diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 4ab1c8a55..fb6d70fe2 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "c6f543a29a359ce7738bf215886b53a5081da2cfc5b6f98d4f9416c58d0dfb36", + "registryDigest": "d435bfa6e93138c57bd3043bc20ab71fe83e20f40633b45cbeba7c735d1e693e", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/continuous-learning-v2/hooks/observe.sh b/skills/continuous-learning-v2/hooks/observe.sh index ff0d3235f..e490fea24 100755 --- a/skills/continuous-learning-v2/hooks/observe.sh +++ b/skills/continuous-learning-v2/hooks/observe.sh @@ -123,6 +123,15 @@ if [ -n "$STDIN_CWD" ] && [ -d "$STDIN_CWD" ]; then unset CLAUDE_PROJECT_DIR export CLV2_NO_PROJECT=1 fi +else + # Payload cwd is missing or not a directory. Do not let detect-project.sh + # priority 2 guess from the hook's inherited working directory. Keep a + # valid inherited CLAUDE_PROJECT_DIR (explicit override, including non-git + # directories). Otherwise force the global bucket. + if [ -z "${CLAUDE_PROJECT_DIR:-}" ] || [ ! -d "${CLAUDE_PROJECT_DIR:-}" ]; then + unset CLAUDE_PROJECT_DIR + export CLV2_NO_PROJECT=1 + fi fi # ───────────────────────────────────────────── diff --git a/tests/hooks/observe-missing-cwd.test.js b/tests/hooks/observe-missing-cwd.test.js new file mode 100644 index 000000000..128530745 --- /dev/null +++ b/tests/hooks/observe-missing-cwd.test.js @@ -0,0 +1,268 @@ +/** + * Regression for observe.sh when the hook payload has no usable cwd. + * Issue #2746, defect 1. + * + * The hook must not let detect-project.sh priority 2 (`git rev-parse` + * against the inherited working directory) choose a project. A valid + * inherited CLAUDE_PROJECT_DIR stays an explicit override, including a + * non-git directory (#2469 / #2488). Anything else is forced into the + * global bucket via CLV2_NO_PROJECT. + * + * Run with: node tests/hooks/observe-missing-cwd.test.js + */ + +if (process.platform === 'win32') { + console.log('Skipping bash-dependent observe tests on Windows'); + console.log('Passed: 0'); + console.log('Failed: 0'); + process.exit(0); +} + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +let passed = 0; +let failed = 0; + +const repoRoot = path.resolve(__dirname, '..', '..'); +const observeShPath = path.join( + repoRoot, + 'skills', + 'continuous-learning-v2', + 'hooks', + 'observe.sh' +); + +function test(name, fn) { + try { + fn(); + console.log(`PASS: ${name}`); + passed += 1; + } catch (error) { + console.log(`FAIL: ${name}`); + console.error(` ${error.message}`); + failed += 1; + } +} + +function createTempDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-observe-missing-cwd-')); +} + +function cleanupDir(dir) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + } catch (error) { + console.error(`[cleanupDir] failed to remove ${dir}: ${error.message}`); + } +} + +function normalizeComparablePath(filePath) { + if (!filePath) { + return filePath; + } + const normalized = fs.realpathSync(filePath); + return process.platform === 'win32' ? normalized.toLowerCase() : normalized; +} + +function gitInit(dir) { + const initResult = spawnSync('git', ['init'], { cwd: dir, encoding: 'utf8' }); + assert.strictEqual(initResult.status, 0, initResult.stderr || 'git init failed'); +} + +function homunculusDir(homeDir) { + return path.join(homeDir, '.local', 'share', 'ecc-homunculus'); +} + +// `cwd` omitted when cwdField is the symbol OMIT. +const OMIT = Symbol('omit-cwd'); + +function hookPayload(cwdField) { + const payload = { + tool_name: 'Read', + tool_input: { file_path: 'README.md' }, + tool_response: 'ok', + session_id: 'session-missing-cwd', + }; + if (cwdField !== OMIT) { + payload.cwd = cwdField; + } + return payload; +} + +function runObserve({ homeDir, processCwd, cwdField, claudeProjectDir }) { + const env = { ...process.env }; + // The hook must not see host credentials, and parent project-detection + // knobs must not mask the payload-cwd fallback under test. + for (const key of [ + 'GH_TOKEN', + 'GITHUB_TOKEN', + 'CLV2_NO_PROJECT', + 'CLV2_HOMUNCULUS_DIR', + 'CLV2_CONFIG', + 'XDG_DATA_HOME', + 'GIT_DIR', + 'GIT_WORK_TREE', + ]) { + delete env[key]; + } + env.HOME = homeDir; + env.USERPROFILE = homeDir; + env.CLAUDE_CODE_ENTRYPOINT = 'cli'; + env.ECC_HOOK_PROFILE = 'standard'; + env.ECC_SKIP_OBSERVE = '0'; + if (claudeProjectDir === undefined) { + env.CLAUDE_PROJECT_DIR = ''; + } else { + env.CLAUDE_PROJECT_DIR = claudeProjectDir; + } + + return spawnSync('bash', [observeShPath, 'post'], { + cwd: processCwd, + encoding: 'utf8', + input: JSON.stringify(hookPayload(cwdField)), + env, + timeout: 20000, + }); +} + +function readObservation(filePath) { + assert.ok(fs.existsSync(filePath), `expected an observation at ${filePath}`); + const lines = fs.readFileSync(filePath, 'utf8').trim().split('\n').filter(Boolean); + assert.strictEqual(lines.length, 1, `expected exactly one observation, got ${lines.length}`); + return JSON.parse(lines[0]); +} + +function assertGlobalNotInherited(homeDir) { + const root = homunculusDir(homeDir); + const projectsDir = path.join(root, 'projects'); + const registryPath = path.join(root, 'projects.json'); + const observationsPath = path.join(root, 'observations.jsonl'); + + assert.ok( + !fs.existsSync(registryPath), + 'unusable payload cwd must not register the hook process working directory' + ); + assert.ok( + !fs.existsSync(projectsDir) || fs.readdirSync(projectsDir).length === 0, + 'unusable payload cwd must not create a project directory from the inherited working directory' + ); + + const observation = readObservation(observationsPath); + assert.strictEqual(observation.project_id, 'global', 'observation must be stored in the global bucket'); + assert.strictEqual(observation.project_name, 'global'); + assert.notStrictEqual( + observation.project_name, + 'decoy-repo', + 'observation must not adopt the inherited git checkout name' + ); +} + +function assertProjectScope(homeDir, projectDir, projectName) { + const root = homunculusDir(homeDir); + const projectsDir = path.join(root, 'projects'); + assert.ok(fs.existsSync(projectsDir), 'a valid CLAUDE_PROJECT_DIR should create a project directory'); + const projectIds = fs.readdirSync(projectsDir); + assert.strictEqual(projectIds.length, 1, `expected one project, got ${projectIds.join(',')}`); + + const storedDir = path.join(projectsDir, projectIds[0]); + const metadata = JSON.parse(fs.readFileSync(path.join(storedDir, 'project.json'), 'utf8')); + assert.strictEqual(metadata.name, projectName); + assert.strictEqual( + normalizeComparablePath(metadata.root), + normalizeComparablePath(projectDir), + 'project root must be the inherited CLAUDE_PROJECT_DIR, not the hook working directory' + ); + assert.notStrictEqual(metadata.name, 'decoy-repo'); + + const observation = readObservation(path.join(storedDir, 'observations.jsonl')); + assert.strictEqual(observation.project_name, projectName); + assert.notStrictEqual(observation.project_id, 'global'); + assert.ok(observation.project_id && observation.project_id.length > 0); + + const globalObservations = path.join(root, 'observations.jsonl'); + assert.ok( + !fs.existsSync(globalObservations), + 'a valid CLAUDE_PROJECT_DIR must not also write the global observations file' + ); +} + +function withDecoyRepo(fn) { + const testRoot = createTempDir(); + try { + const homeDir = path.join(testRoot, 'home'); + const decoyRepo = path.join(testRoot, 'decoy-repo'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(decoyRepo, { recursive: true }); + gitInit(decoyRepo); + fn({ testRoot, homeDir, decoyRepo }); + } finally { + cleanupDir(testRoot); + } +} + +console.log('\n=== Observe.sh missing/invalid payload cwd (issue #2746) ===\n'); + +test('omitted payload cwd does not adopt the inherited git working directory', () => { + withDecoyRepo(({ homeDir, decoyRepo }) => { + const result = runObserve({ homeDir, processCwd: decoyRepo, cwdField: OMIT }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout || 'observe.sh failed'); + assertGlobalNotInherited(homeDir); + }); +}); + +test('empty payload cwd does not adopt the inherited git working directory', () => { + withDecoyRepo(({ homeDir, decoyRepo }) => { + const result = runObserve({ homeDir, processCwd: decoyRepo, cwdField: '' }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout || 'observe.sh failed'); + assertGlobalNotInherited(homeDir); + }); +}); + +test('nonexistent payload cwd does not adopt the inherited git working directory', () => { + withDecoyRepo(({ testRoot, homeDir, decoyRepo }) => { + const missing = path.join(testRoot, 'no-such-directory'); + const result = runObserve({ homeDir, processCwd: decoyRepo, cwdField: missing }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout || 'observe.sh failed'); + assertGlobalNotInherited(homeDir); + }); +}); + +test('invalid inherited CLAUDE_PROJECT_DIR still forces global instead of the inherited git checkout', () => { + withDecoyRepo(({ testRoot, homeDir, decoyRepo }) => { + const missingProject = path.join(testRoot, 'missing-claude-project'); + const result = runObserve({ + homeDir, + processCwd: decoyRepo, + cwdField: OMIT, + claudeProjectDir: missingProject, + }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout || 'observe.sh failed'); + assertGlobalNotInherited(homeDir); + }); +}); + +test('omitted payload cwd keeps a valid non-git CLAUDE_PROJECT_DIR', () => { + withDecoyRepo(({ testRoot, homeDir, decoyRepo }) => { + const plainProject = path.join(testRoot, 'plain-project'); + fs.mkdirSync(plainProject, { recursive: true }); + assert.ok(!fs.existsSync(path.join(plainProject, '.git')), 'guard: CLAUDE_PROJECT_DIR must not be a git repo'); + + const result = runObserve({ + homeDir, + processCwd: decoyRepo, + cwdField: OMIT, + claudeProjectDir: plainProject, + }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout || 'observe.sh failed'); + assertProjectScope(homeDir, plainProject, 'plain-project'); + }); +}); + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}`); + +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/hooks/observe-scope-safety.test.js b/tests/hooks/observe-scope-safety.test.js new file mode 100644 index 000000000..0d640eb4c --- /dev/null +++ b/tests/hooks/observe-scope-safety.test.js @@ -0,0 +1,188 @@ +'use strict'; + +// Exercise the public stdin hook with isolated user data and an unrelated cwd. +// These fixtures prove local controls, not native host consent or authentication. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); + +const repo = path.resolve(__dirname, '../..'); +const hook = path.join(repo, 'skills/continuous-learning-v2/hooks/observe.sh'); +let passed = 0; +let failed = 0; + +function test(name, fn) { + if (process.platform === 'win32') return; + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-scope-safety-')); + try { + fn(root); + console.log(`PASS: ${name}`); + passed += 1; + } catch (error) { + console.error(`FAIL: ${name}: ${error.message}`); + failed += 1; + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +function fixture(root) { + const home = path.join(root, 'home'); + const data = path.join(home, '.local/share/ecc-homunculus'); + const decoy = path.join(root, 'decoy'); + const explicit = path.join(root, 'explicit'); + const subdir = path.join(explicit, 'src'); + fs.mkdirSync(subdir, { recursive: true }); + fs.mkdirSync(decoy); + for (const cwd of [decoy, explicit]) { + assert.equal(spawnSync('git', ['init'], { cwd }).status, 0); + } + const files = { + 'observations.jsonl': '{"user":"existing-global"}\n', + 'config.json': '{"observer":{"enabled":false}}\n', + 'projects.json': '{"user-owned":{"name":"preserved"}}\n', + 'instincts/personal/user.md': 'existing global instinct\n', + 'projects/user-owned/observations.jsonl': '{"user":"existing-scoped"}\n', + 'projects/user-owned/instincts/personal/user.md': 'existing scoped instinct\n', + }; + for (const [relative, content] of Object.entries(files)) { + const target = path.join(data, relative); + fs.mkdirSync(path.dirname(target), { recursive: true }); + fs.writeFileSync(target, content); + } + // Allow only runtime environment here, never ambient credentials or scope. + const env = { PATH: process.env.PATH, HOME: home, USERPROFILE: home, + CLAUDE_CODE_ENTRYPOINT: 'cli', ECC_HOOK_PROFILE: 'standard', + ECC_SKIP_OBSERVE: '0', CLV2_CONFIG: path.join(data, 'config.json') }; + return { data, decoy, explicit, subdir, files, env }; +} + +function run(f, payload, extraEnv = {}, phase = 'post', registered = false) { + const command = registered ? process.execPath : 'bash'; + const args = registered + ? [path.join(repo, 'scripts/hooks/run-with-flags.js'), `${phase}:observe`, + 'scripts/hooks/observe-runner.js', 'standard,strict'] + : [hook, phase]; + const result = spawnSync(command, args, { cwd: f.decoy, + input: typeof payload === 'string' ? payload : JSON.stringify(payload), + encoding: 'utf8', timeout: 20000, env: { ...f.env, ...extraEnv } }); + assert.equal(result.status, 0, result.stderr); + assert.doesNotMatch(result.stderr, /runner failed|timed out/i); +} + +function preserved(f, { globalAppend = false, registryUpdate = false } = {}) { + for (const [relative, content] of Object.entries(f.files)) { + const actual = fs.readFileSync(path.join(f.data, relative), 'utf8'); + if (relative === 'observations.jsonl' && globalAppend) { + assert.ok(actual.startsWith(content), 'existing global observations changed'); + } else if (relative === 'projects.json' && registryUpdate) { + assert.deepEqual(JSON.parse(actual)['user-owned'], JSON.parse(content)['user-owned']); + } else assert.equal(actual, content, `user file changed: ${relative}`); + } +} + +const payload = { tool_name: 'Read', session_id: 'scope-safety', + tool_response: { content: 'password = "CANARY_abcdefghijk"' } }; + +for (const cwd of [undefined, '', null, '/no-such-ecc-scope-directory']) { + test(`unusable cwd ${JSON.stringify(cwd)} appends globally and preserves scoped user files`, root => { + const f = fixture(root); + run(f, { ...payload, ...(cwd === undefined ? {} : { cwd }) }); + preserved(f, { globalAppend: true }); + const rows = fs.readFileSync(path.join(f.data, 'observations.jsonl'), 'utf8').trim().split('\n'); + assert.equal(rows.length, 2); + const record = JSON.parse(rows[1]); + assert.equal(record.project_id, 'global'); + assert.ok(record.output.includes('[REDACTED]')); + assert.ok(!record.output.includes('CANARY_abcdefghijk')); + assert.deepEqual(fs.readdirSync(path.join(f.data, 'projects')), ['user-owned']); + assert.ok(!fs.existsSync(path.join(f.data, '.observer.pid'))); + }); +} + +test('explicit payload subdirectory wins over inherited scope and redacts pre input', root => { + const f = fixture(root); + run(f, { ...payload, cwd: f.subdir, + tool_input: { content: 'token = "CANARY_abcdefghijk"' } }, + { CLAUDE_PROJECT_DIR: f.decoy }, 'pre'); + preserved(f, { registryUpdate: true }); + const registry = JSON.parse(fs.readFileSync(path.join(f.data, 'projects.json'))); + const entries = Object.values(registry).filter(entry => entry.root); + assert.equal(entries.length, 1); + assert.equal(fs.realpathSync(entries[0].root), fs.realpathSync(f.explicit)); + const stored = fs.readFileSync(path.join(f.data, 'projects', entries[0].id, 'observations.jsonl'), 'utf8'); + assert.equal(JSON.parse(stored).event, 'tool_start'); + assert.ok(stored.includes('[REDACTED]')); + assert.ok(!stored.includes('CANARY_abcdefghijk')); +}); + +test('malformed stdin is redacted in global fallback without registering decoy', root => { + const f = fixture(root); + run(f, 'invalid JSON token=CANARY_abcdefghijk'); + preserved(f, { globalAppend: true }); + const rows = fs.readFileSync(path.join(f.data, 'observations.jsonl'), 'utf8').trim().split('\n'); + assert.equal(rows.length, 2); + assert.equal(JSON.parse(rows[1]).event, 'parse_error'); + assert.ok(rows[1].includes('[REDACTED]')); + assert.ok(!rows[1].includes('CANARY_abcdefghijk')); + assert.deepEqual(fs.readdirSync(path.join(f.data, 'projects')), ['user-owned']); +}); + +test('missing payload cwd keeps an explicit git project and preserves the global bucket', root => { + const f = fixture(root); + run(f, payload, { CLAUDE_PROJECT_DIR: f.subdir }); + preserved(f, { registryUpdate: true }); + const registry = JSON.parse(fs.readFileSync(path.join(f.data, 'projects.json'))); + const entries = Object.values(registry).filter(entry => entry.root); + assert.equal(entries.length, 1); + assert.equal(fs.realpathSync(entries[0].root), fs.realpathSync(f.explicit)); + const stored = fs.readFileSync(path.join(f.data, 'projects', entries[0].id, 'observations.jsonl'), 'utf8'); + const record = JSON.parse(stored); + assert.equal(record.project_id, entries[0].id); + assert.equal(record.project_name, 'explicit'); +}); + +test('custom config disabled marker stops capture without touching either bucket', root => { + const f = fixture(root); + const configDir = path.join(root, 'custom-config'); + fs.mkdirSync(configDir); + fs.writeFileSync(path.join(configDir, 'config.json'), f.files['config.json']); + fs.writeFileSync(path.join(configDir, 'disabled'), 'user disabled capture\n'); + run(f, { ...payload, cwd: f.subdir }, { CLV2_CONFIG: path.join(configDir, 'config.json') }); + preserved(f); + assert.deepEqual(fs.readdirSync(path.join(f.data, 'projects')), ['user-owned']); + assert.ok(!fs.existsSync(path.join(f.data, '.observer-last-activity'))); +}); + +for (const [label, extraEnv, input, disabled, registered] of [ + ['empty stdin', {}, '', false, false], + ['capture skip', { ECC_SKIP_OBSERVE: '1' }, payload, false, false], + ['minimal profile', { ECC_HOOK_PROFILE: 'minimal' }, payload, false, false], + ['unknown entrypoint', { CLAUDE_CODE_ENTRYPOINT: 'automated-bot' }, payload, false, false], + ['subagent', {}, { ...payload, agent_id: 'fixture-agent' }, false, false], + ['excluded path', {}, { ...payload, cwd: '/fixture/observer-sessions' }, false, false], + ['data disabled marker', {}, payload, true, false], + ['registered hook disabled', { ECC_DISABLED_HOOKS: 'post:observe' }, payload, false, true], +]) { + test(`${label} exits without changing user files or registering inherited cwd`, root => { + const f = fixture(root); + if (disabled) fs.writeFileSync(path.join(f.data, 'disabled'), 'user disabled capture\n'); + let cwd = f.subdir; + if (label === 'excluded path') { + cwd = path.join(f.explicit, 'observer-sessions'); + fs.mkdirSync(cwd); + } + // A valid project exposes guards accidentally moved below registration. + const event = typeof input === 'string' ? input : { ...input, cwd }; + run(f, event, extraEnv, 'post', registered); + preserved(f); + assert.deepEqual(fs.readdirSync(path.join(f.data, 'projects')), ['user-owned']); + assert.ok(!fs.existsSync(path.join(f.data, '.observer-last-activity'))); + }); +} + +console.log(`Passed: ${passed}`); +console.log(`Failed: ${failed}`); +process.exitCode = failed ? 1 : 0;