Wave-1 order 4 (P7, P12, the P9 test lock, the P3 plan-approval guard). All command hooks; none runs a model. - P12: pre-bash-executor answers permissionDecision "ask" before merge into main, push, tag/publish and destructive git (holds in auto mode). post-approval-log writes exactly what the person was asked to the K2 approval log: merge/push to main as `merge`, tag/publish as `release`, with a one-hour expiry; nothing when no person was asked. - P3: pre-plan-gate runs the K3 plan gate on a Voyage plan leaving plan mode and sends a refused plan back; "not measured" asks, never passes. - P9: lib/util/test-lock.mjs (K8). pre-write-executor refuses edits to locked test files; stop-test-lock refuses "done" for the locking session (stop_hook_active guarded). - P7: post-edit-check runs the target repo's own formatter/linter per edit, only during a Voyage plan run or after opt-in (.claude/voyage-guards.json); lib/verification/step-check.mjs (K9) fails the step on findings; no tool reads "not measured", never green. - STORM cap matcher also names `Agent` (the tool's name since 2.1.63). - Mod precondition stated in README, SECURITY.md and every PreToolUse stop guard: Voyage's guards are deterministic only when no installed mod handles `tool.check`. K2, K3 and K5 are stubbed in tests until orders 2 and 3 are merged. Two independent review rounds; every finding has a regression test. Suite 1600 (1598 pass / 0 fail / 2 skipped). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
301 lines
16 KiB
JavaScript
301 lines
16 KiB
JavaScript
// tests/hooks/guards-review-regressions.test.mjs
|
|
// Regressions from the independent review of the wave-1 guards (06.10). Each
|
|
// test reproduces one finding the first test set did not cover; all failed
|
|
// before the fix.
|
|
|
|
import { test } from 'node:test';
|
|
import { strict as assert } from 'node:assert';
|
|
import { spawnSync } from 'node:child_process';
|
|
import { writeFileSync, mkdirSync, realpathSync } from 'node:fs';
|
|
import { join } from 'node:path';
|
|
import { runHook, runHookWithEnv } from '../helpers/hook-helper.mjs';
|
|
import { ROOT, makeRepo, gitIn, fakeTool, tmp, pluginCopy, readStubLog, parseJsonOut } from './guard-helpers.mjs';
|
|
|
|
const PRE_BASH = join(ROOT, 'hooks', 'scripts', 'pre-bash-executor.mjs');
|
|
const PRE_WRITE = join(ROOT, 'hooks', 'scripts', 'pre-write-executor.mjs');
|
|
const STOP = join(ROOT, 'hooks', 'scripts', 'stop-test-lock.mjs');
|
|
const LOCK = join(ROOT, 'lib', 'util', 'test-lock.mjs');
|
|
const STEP = join(ROOT, 'lib', 'verification', 'step-check.mjs');
|
|
const { classifyGitApproval } = await import(join(ROOT, 'hooks', 'lib', 'git-approval.mjs'));
|
|
|
|
const FEAT = makeRepo('main');
|
|
gitIn(FEAT, 'checkout', '-q', '-b', 'feat');
|
|
const MAIN = makeRepo('main');
|
|
|
|
async function decision(command, cwd = FEAT) {
|
|
const r = await runHook(PRE_BASH, { hook_event_name: 'PreToolUse', tool_name: 'Bash', cwd, tool_input: { command } });
|
|
let out = null;
|
|
try { out = parseJsonOut(r.stdout); } catch { out = null; }
|
|
return out?.hookSpecificOutput?.permissionDecision ?? (r.code === 2 ? 'block' : null);
|
|
}
|
|
|
|
// 1. Quotes that are not quotes
|
|
for (const cmd of [
|
|
"# push it, it's ready\ngit push origin main",
|
|
"echo don\\'t; git push origin main",
|
|
"echo 'unterminated; git push origin main",
|
|
]) {
|
|
test(`review 1: an apostrophe that is not a closed quote does not hide a push: ${JSON.stringify(cmd)}`, async () => {
|
|
assert.equal(await decision(cmd), 'ask');
|
|
});
|
|
}
|
|
|
|
// 2. Quoted program, subcommand or ref
|
|
for (const cmd of ['git "push" origin feat', '"git" push', "'git' push", "git 'push' origin feat"]) {
|
|
test(`review 2: a quoted word is still the word: ${cmd}`, async () => {
|
|
assert.equal(await decision(cmd), 'ask');
|
|
});
|
|
}
|
|
test('review 2: a quoted ref keeps its kind (push to "main" is a merge approval)', () => {
|
|
assert.deepEqual(classifyGitApproval('git push origin "main"', { branch: 'feat' }).map((m) => m.kind), ['merge']);
|
|
});
|
|
test('review 2: quoted data with spaces stays data', async () => {
|
|
assert.equal(await decision('echo "git push origin main"'), null);
|
|
assert.equal(await decision('git commit -m "wip"'), null);
|
|
});
|
|
|
|
// 3. Wrappers
|
|
for (const cmd of ['timeout 60 git push', 'nice git push', 'nice -n 5 git push', 'env -i git push', 'env -u HOME git push',
|
|
'sudo -u bob git push', 'command -p git push', 'time -p git push', 'eval git push', 'xargs -n 1 git push']) {
|
|
test(`review 3: wrapper seen through: ${cmd}`, async () => {
|
|
assert.equal(await decision(cmd), 'ask');
|
|
});
|
|
}
|
|
|
|
// 8. One branch model for the whole command
|
|
test('review 8: switching to main then pushing is a push to main', () => {
|
|
assert.deepEqual(classifyGitApproval('git checkout main && git push', { branch: 'feat' }).map((m) => m.kind), ['merge']);
|
|
});
|
|
test('review 8: switching away from main then merging is not a merge into main', () => {
|
|
assert.deepEqual(classifyGitApproval('git checkout feat && git merge x', { branch: 'main' }), []);
|
|
});
|
|
test('review 8: git -C <dir> reads that repo\'s branch', () => {
|
|
assert.deepEqual(classifyGitApproval(`git -C ${MAIN} merge x`, { cwd: FEAT }).map((m) => m.kind), ['merge']);
|
|
});
|
|
test('review 8: cd <dir> && ... reads that repo\'s branch', () => {
|
|
assert.deepEqual(classifyGitApproval(`cd ${MAIN} && git merge x`, { cwd: FEAT }).map((m) => m.kind), ['merge']);
|
|
});
|
|
|
|
test('review 8: the log records what the person was asked, not a re-reading after the command', async () => {
|
|
const plugin = pluginCopy({ 'lib/util/approval-log.mjs': 'stub' });
|
|
const repo = makeRepo('main');
|
|
gitIn(repo, 'checkout', '-q', '-b', 'feat');
|
|
const cmd = 'git checkout main && git merge feat';
|
|
const base = { session_id: 's-8', tool_use_id: 'toolu_8', cwd: repo, tool_name: 'Bash', tool_input: { command: cmd } };
|
|
const pre = await runHook(join(plugin, 'hooks', 'scripts', 'pre-bash-executor.mjs'), { ...base, hook_event_name: 'PreToolUse' });
|
|
assert.match(pre.stdout, /merge into main/);
|
|
gitIn(repo, 'checkout', '-q', 'main'); // the command ran
|
|
const log = join(tmp(), 'stub.jsonl');
|
|
await runHookWithEnv(join(plugin, 'hooks', 'scripts', 'post-approval-log.mjs'), { ...base, hook_event_name: 'PostToolUse' }, { STUB_LOG: log });
|
|
const calls = readStubLog(log);
|
|
assert.equal(calls.length, 1);
|
|
assert.equal(calls[0].argv[calls[0].argv.indexOf('--kind') + 1], 'merge');
|
|
});
|
|
|
|
test('review 8: a command nobody was asked about is not logged, even if it reads like one afterwards', async () => {
|
|
const plugin = pluginCopy({ 'lib/util/approval-log.mjs': 'stub' });
|
|
const repo = makeRepo('main');
|
|
const log = join(tmp(), 'stub.jsonl');
|
|
await runHookWithEnv(join(plugin, 'hooks', 'scripts', 'post-approval-log.mjs'), {
|
|
hook_event_name: 'PostToolUse', session_id: 's-9', tool_use_id: 'toolu_never_asked', cwd: repo,
|
|
tool_name: 'Bash', tool_input: { command: 'git push origin main' },
|
|
}, { STUB_LOG: log });
|
|
assert.equal(readStubLog(log).length, 0);
|
|
});
|
|
|
|
test('uncertain (bypass mode): an approval that no person gave is not logged', async () => {
|
|
const plugin = pluginCopy({ 'lib/util/approval-log.mjs': 'stub' });
|
|
const repo = makeRepo('main');
|
|
const base = { session_id: 's-b', tool_use_id: 'toolu_b', cwd: repo, tool_name: 'Bash', tool_input: { command: 'git push origin main' }, permission_mode: 'bypassPermissions' };
|
|
await runHook(join(plugin, 'hooks', 'scripts', 'pre-bash-executor.mjs'), { ...base, hook_event_name: 'PreToolUse' });
|
|
const log = join(tmp(), 'stub.jsonl');
|
|
const r = await runHookWithEnv(join(plugin, 'hooks', 'scripts', 'post-approval-log.mjs'), { ...base, hook_event_name: 'PostToolUse' }, { STUB_LOG: log });
|
|
assert.equal(readStubLog(log).length, 0);
|
|
assert.match(parseJsonOut(r.stdout).hookSpecificOutput.additionalContext, /not logged/i);
|
|
});
|
|
|
|
// 9. Classification
|
|
const kinds = (cmd, branch = 'feat', cwd) => classifyGitApproval(cmd, { branch, cwd }).map((m) => m.kind);
|
|
for (const cmd of ['git tag -v v1', 'git tag --verify v1', 'git push -n', 'git push --dry-run origin main']) {
|
|
test(`review 9: not asked: ${cmd}`, () => assert.deepEqual(kinds(cmd), []));
|
|
}
|
|
for (const cmd of ['npm --tag beta publish', 'pnpm -r publish', 'yarn npm publish', 'npx npm publish', 'gh release create v1']) {
|
|
test(`review 9: release: ${cmd}`, () => assert.deepEqual(kinds(cmd), ['release']));
|
|
}
|
|
test('review 9: pushing an existing tag by name is a release', () => {
|
|
const repo = makeRepo('main');
|
|
gitIn(repo, 'tag', 'v1.0');
|
|
assert.deepEqual(kinds('git push origin v1.0', 'main', repo), ['release']);
|
|
});
|
|
for (const cmd of ['git checkout HEAD file.txt', 'git switch -f main', 'git switch --discard-changes main', 'git branch -M main']) {
|
|
test(`review 9: destructive: ${cmd}`, () => assert.deepEqual(kinds(cmd, 'feat'), [null]));
|
|
}
|
|
test('review 9: deleting main on the remote is destructive, not a merge approval', () => {
|
|
assert.deepEqual(kinds('git push origin :main'), [null]);
|
|
assert.deepEqual(kinds('git push origin --delete main'), [null]);
|
|
});
|
|
|
|
// 5 + 6 + 7. The write guard
|
|
test('review 5: a lock module that cannot load blocks the write (fail closed), the path rules still run', async () => {
|
|
const plugin = pluginCopy({ 'lib/util/test-lock.mjs': 'missing' });
|
|
const hook = join(plugin, 'hooks', 'scripts', 'pre-write-executor.mjs');
|
|
const r = await runHook(hook, { tool_name: 'Edit', tool_input: { file_path: join(makeRepo('main'), 'a.js') } });
|
|
assert.equal(r.code, 2);
|
|
assert.match(r.stderr, /could not load/i);
|
|
});
|
|
test('review 5: a guard module that cannot load makes the Bash guard ask, never pass silently', async () => {
|
|
const plugin = pluginCopy({ 'hooks/lib/git-approval.mjs': 'missing' });
|
|
const r = await runHook(join(plugin, 'hooks', 'scripts', 'pre-bash-executor.mjs'), { tool_name: 'Bash', tool_input: { command: 'ls' } });
|
|
assert.equal(parseJsonOut(r.stdout)?.hookSpecificOutput?.permissionDecision, 'ask');
|
|
});
|
|
|
|
function lockIn(repo, rel, env = {}) {
|
|
const r = spawnSync(process.execPath, [LOCK, 'lock', '--files', rel, '--step', '1'], { cwd: repo, encoding: 'utf8', env: { ...process.env, ...env } });
|
|
assert.equal(r.status, 0, r.stderr);
|
|
}
|
|
|
|
test('review 6: another letter case does not get past the lock (case-insensitive file systems)', async () => {
|
|
const repo = makeRepo('main');
|
|
mkdirSync(join(repo, 'tests'));
|
|
writeFileSync(join(repo, 'tests', 'a.test.mjs'), 'x');
|
|
lockIn(repo, 'tests/a.test.mjs');
|
|
const upper = join(repo, 'Tests', 'a.test.mjs');
|
|
let caseInsensitive = false;
|
|
try { realpathSync(upper); caseInsensitive = true; } catch { caseInsensitive = false; }
|
|
const r = await runHook(PRE_WRITE, { tool_name: 'Edit', tool_input: { file_path: upper } });
|
|
if (caseInsensitive) assert.equal(r.code, 2, 'same file, other case');
|
|
else assert.equal(r.code, 0, 'a different file on a case-sensitive file system');
|
|
});
|
|
|
|
test('review 7: the lock file itself cannot be written with the file tools (repo and worktree git dirs)', async () => {
|
|
for (const p of ['/r/.git/voyage/test-lock.json', '/r/.git/worktrees/wt/voyage/test-lock.json', '/r/.git/voyage/post-edit.jsonl']) {
|
|
const r = await runHook(PRE_WRITE, { tool_name: 'Write', tool_input: { file_path: p, content: '{}' } });
|
|
assert.equal(r.code, 2, p);
|
|
}
|
|
});
|
|
|
|
// 10. The stop guard finds the session's lock wherever the session now stands
|
|
const SESS_MOVED = `sess-moved-${process.pid}-${Date.now()}`;
|
|
test('review 10: the stop guard finds the lock taken by this session from another directory', async () => {
|
|
const repo = makeRepo('main');
|
|
writeFileSync(join(repo, 'a.test.js'), 'x');
|
|
lockIn(repo, 'a.test.js', { CLAUDE_CODE_SESSION_ID: SESS_MOVED });
|
|
const r = await runHook(STOP, { hook_event_name: 'Stop', session_id: SESS_MOVED, cwd: tmp(), stop_hook_active: false });
|
|
assert.equal(parseJsonOut(r.stdout)?.decision, 'block');
|
|
spawnSync(process.execPath, [LOCK, 'unlock'], { cwd: repo });
|
|
const after = await runHook(STOP, { hook_event_name: 'Stop', session_id: SESS_MOVED, cwd: tmp(), stop_hook_active: false });
|
|
assert.equal(parseJsonOut(after.stdout), null, 'unlock clears it');
|
|
});
|
|
|
|
// 4, 11, 12. The step check
|
|
function runStep(repo) {
|
|
const r = spawnSync(process.execPath, [STEP, '--cwd', repo], { encoding: 'utf8', env: { ...process.env, PATH: '/usr/bin:/bin' } });
|
|
return { code: r.status, json: JSON.parse(r.stdout) };
|
|
}
|
|
function jsRepo() {
|
|
const repo = makeRepo('main');
|
|
writeFileSync(join(repo, '.prettierrc'), '{}');
|
|
writeFileSync(join(repo, '.gitignore'), 'node_modules/\n');
|
|
gitIn(repo, 'add', '.prettierrc', '.gitignore');
|
|
gitIn(repo, 'commit', '-q', '-m', 'tools');
|
|
fakeTool(join(repo, 'node_modules', '.bin'), 'prettier');
|
|
return repo;
|
|
}
|
|
test('review 4: a configured tool that crashed is not green', () => {
|
|
const repo = jsRepo();
|
|
writeFileSync(join(repo, 'eslint.config.js'), 'export default []');
|
|
// eslint is configured and installed, but crashes (exit 2 = config error) on every call
|
|
writeFileSync(join(repo, 'node_modules', '.bin', 'eslint'), '#!/bin/sh\nexit 2\n', { mode: 0o755 });
|
|
writeFileSync(join(repo, 'a.js'), 'ok'); // prettier: clean
|
|
const r = runStep(repo);
|
|
assert.equal(r.code, 2, JSON.stringify(r.json));
|
|
assert.equal(r.json.status, 'not_measured');
|
|
});
|
|
test('review 4: changed files no tool covers are listed, never silently dropped', () => {
|
|
const repo = jsRepo();
|
|
writeFileSync(join(repo, 'a.js'), 'ok');
|
|
writeFileSync(join(repo, 'b.py'), 'x = 1');
|
|
const r = runStep(repo);
|
|
assert.deepEqual(r.json.not_covered, ['b.py']);
|
|
});
|
|
test('review 11: a changed file whose name starts with "-" reaches the tool as a file, not a flag', () => {
|
|
const repo = jsRepo();
|
|
writeFileSync(join(repo, '-x.js'), 'BAD');
|
|
const r = runStep(repo);
|
|
assert.equal(r.code, 1, JSON.stringify(r.json));
|
|
});
|
|
test('review 12: a changed file with a non-ASCII name is checked', () => {
|
|
const repo = jsRepo();
|
|
writeFileSync(join(repo, 'blåbær.js'), 'BAD');
|
|
const r = runStep(repo);
|
|
assert.equal(r.code, 1, JSON.stringify(r.json));
|
|
});
|
|
|
|
test('uncertain (relative plan path): the plan gate resolves planFilePath against the session cwd', async () => {
|
|
const plugin = pluginCopy({ 'lib/validators/plan-gate.mjs': 'stub' });
|
|
const dir = tmp();
|
|
writeFileSync(join(dir, 'plan.md'), '---\nplan_version: 1.7\n---\n');
|
|
const log = join(tmp(), 'stub.jsonl');
|
|
const r = await runHookWithEnv(join(plugin, 'hooks', 'scripts', 'pre-plan-gate.mjs'), {
|
|
hook_event_name: 'PreToolUse', tool_name: 'ExitPlanMode', cwd: dir,
|
|
tool_input: { plan: '---\nplan_version: 1.7\n---\n', planFilePath: 'plan.md' },
|
|
}, { STUB_LOG: log, STUB_EXIT: '1' });
|
|
assert.equal(r.code, 2, r.stdout);
|
|
assert.equal(readStubLog(log)[0].argv[0], join(dir, 'plan.md'));
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Second review round (06.10): what the first fixes left open or introduced.
|
|
// ---------------------------------------------------------------------------
|
|
for (const cmd of [
|
|
'git commit -m "fix \\"x\\" bug" && git push origin main && echo "ok"',
|
|
'echo "a\\"b" ; git push origin main ; echo "c"',
|
|
'if git push origin main; then echo ok; fi',
|
|
'while git push origin main; do sleep 1; done',
|
|
"eval 'git push origin main'",
|
|
'echo a\\ #b; git push origin main',
|
|
]) {
|
|
test(`review 2nd round: still asks: ${JSON.stringify(cmd)}`, async () => {
|
|
assert.equal(await decision(cmd), 'ask');
|
|
});
|
|
}
|
|
|
|
test('review 2nd round (7): the .git/voyage block ignores letter case', async () => {
|
|
for (const p of ['/r/.GIT/voyage/test-lock.json', '/r/.git/VOYAGE/test-lock.json', '/r/.Git/hooks/pre-commit']) {
|
|
const r = await runHook(PRE_WRITE, { tool_name: 'Write', tool_input: { file_path: p, content: '{}' } });
|
|
assert.equal(r.code, 2, p);
|
|
}
|
|
});
|
|
|
|
test('review 2nd round (N1): redirections are not file arguments', () => {
|
|
assert.deepEqual(kinds('git checkout main 2>/dev/null && git merge x', 'feat'), ['merge']);
|
|
assert.deepEqual(kinds('git checkout feat2 2>&1', 'feat'), []);
|
|
assert.deepEqual(kinds('git push origin 2>&1', 'main'), ['merge']);
|
|
assert.deepEqual(kinds('git checkout main > /dev/null && git merge x', 'feat'), ['merge']);
|
|
});
|
|
|
|
for (const cmd of ['npm publish --dry-run', 'cargo publish --dry-run', 'npm view x publish', 'yarn info publish']) {
|
|
test(`review 2nd round (N2): not a release: ${cmd}`, () => assert.deepEqual(kinds(cmd), []));
|
|
}
|
|
|
|
test('review 2nd round (N5): a detached switch leaves no branch to merge into', () => {
|
|
assert.deepEqual(kinds('git switch --detach main && git merge x', 'feat'), []);
|
|
assert.deepEqual(kinds('git checkout --detach main && git merge x', 'feat'), []);
|
|
});
|
|
|
|
test('review 2nd round: a push option value is not the remote', () => {
|
|
assert.deepEqual(kinds('git push -o ci.skip origin main', 'feat'), ['merge']);
|
|
});
|
|
|
|
test('review 2nd round (N4): when the record cannot be written, the prompt says the approval will not be logged', async () => {
|
|
const plugin = pluginCopy({});
|
|
// TMPDIR points at a file, so the record directory cannot be created.
|
|
const fileAsDir = join(tmp(), 'not-a-dir');
|
|
writeFileSync(fileAsDir, 'x');
|
|
const r = await runHookWithEnv(join(plugin, 'hooks', 'scripts', 'pre-bash-executor.mjs'), {
|
|
hook_event_name: 'PreToolUse', session_id: 's-n4', tool_use_id: 'toolu_n4', cwd: makeRepo('main'),
|
|
tool_name: 'Bash', tool_input: { command: 'git push origin main' },
|
|
}, { TMPDIR: fileAsDir });
|
|
const reason = parseJsonOut(r.stdout).hookSpecificOutput.permissionDecisionReason;
|
|
assert.match(reason, /will not be logged/i);
|
|
});
|