fix(hooks): no run evidence for a merged name git reads as a tag or another ref
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
5b9cbeba28
commit
29b1fba9fb
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 299 additions and 9 deletions
|
|
@ -15,6 +15,18 @@
|
|||
// wrong type reads "no run evidence" or "not measured", never a green line.
|
||||
// Text taken from the file prints with every non-printable or non-ASCII
|
||||
// character as `?`.
|
||||
//
|
||||
// A merged name git would read as something other than refs/heads/<name>
|
||||
// gives "no run evidence": git looks a name up as `$GIT_DIR/<name>`,
|
||||
// `refs/<name>` and `refs/tags/<name>` before `refs/heads/<name>`, so the
|
||||
// merge would not take the branch head the evidence was written for. The name
|
||||
// is ambiguous when an entry of any kind (file, directory, link; lstat only)
|
||||
// sits at `<git dir>/<name>`, `<common>/<name>`, `<common>/refs/<name>` or
|
||||
// `<common>/refs/tags/<name>`, or when a `packed-refs` line (not a `^` peeled
|
||||
// line) names exactly `refs/<name>` or `refs/tags/<name>`. A `packed-refs`
|
||||
// that is there but cannot be read within its cap counts as ambiguous (cannot
|
||||
// tell), never as "no tag". The writer (lib/review/merge-evidence.mjs)
|
||||
// refuses the same names.
|
||||
|
||||
import { closeSync, constants, fstatSync, lstatSync, openSync, readFileSync, readSync } from 'node:fs';
|
||||
import { isAbsolute, join, resolve } from 'node:path';
|
||||
|
|
@ -28,7 +40,7 @@ const NOT = 'not measured';
|
|||
const UNDO = 'Undone by: `git revert -m 1 <merge>`';
|
||||
|
||||
// `git merge` options that take a separate value: their value is not the branch.
|
||||
const MERGE_VALUED = ['-m', '-F', '-s', '-X', '--message', '--file', '--strategy', '--strategy-option', '--into-name'];
|
||||
const MERGE_VALUED = ['-m', '-F', '-s', '-X', '--message', '--file', '--strategy', '--strategy-option', '--into-name', '--cleanup'];
|
||||
|
||||
const SHA_RE = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/;
|
||||
const VERDICTS = ['ALLOW', 'WARN', 'BLOCK'];
|
||||
|
|
@ -77,18 +89,66 @@ function smallRegular(path, max) {
|
|||
return readFileSync(path, 'utf8');
|
||||
}
|
||||
|
||||
/** The head sha of `refs/heads/<branch>`: the loose ref, else `packed-refs`; else null. */
|
||||
function headSha(common, branch) {
|
||||
const PACKED_MAX = 16 * 1024 * 1024;
|
||||
|
||||
/**
|
||||
* `packed-refs` of `common`: '' when there is none, its text, or null when it
|
||||
* is there but not a regular file of at most PACKED_MAX bytes, or unreadable.
|
||||
*/
|
||||
function packedRefs(common) {
|
||||
const path = join(common, 'packed-refs');
|
||||
let st;
|
||||
try {
|
||||
st = lstatSync(path);
|
||||
} catch (e) {
|
||||
return e && (e.code === 'ENOENT' || e.code === 'ENOTDIR') ? '' : null;
|
||||
}
|
||||
if (!st.isFile() || st.size > PACKED_MAX) return null;
|
||||
try { return readFileSync(path, 'utf8'); } catch { return null; }
|
||||
}
|
||||
|
||||
/** The ref name of each `packed-refs` line `<sha> <ref>` (a `^` peeled line is none). */
|
||||
function packedLines(packed) {
|
||||
const out = [];
|
||||
for (const line of packed.split('\n')) {
|
||||
const m = line.match(/^([0-9a-f]{40}|[0-9a-f]{64}) (.+)$/);
|
||||
if (m) out.push([m[1], m[2]]);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when git would read `name` as something other than refs/heads/<name>
|
||||
* (see the header): an entry of any kind at one of the four paths, or a
|
||||
* packed `refs/<name>` or `refs/tags/<name>`. Cannot tell counts as true.
|
||||
*/
|
||||
function ambiguousName(gitDir, common, name, packed) {
|
||||
const parts = name.split('/');
|
||||
for (const path of [join(gitDir, ...parts), join(common, ...parts), join(common, 'refs', ...parts), join(common, 'refs', 'tags', ...parts)]) {
|
||||
try { lstatSync(path); return true; } catch (e) {
|
||||
// No entry is ENOENT/ENOTDIR; any other error cannot tell.
|
||||
if (!e || (e.code !== 'ENOENT' && e.code !== 'ENOTDIR')) return true;
|
||||
}
|
||||
}
|
||||
if (packed === null) return true;
|
||||
return packedLines(packed).some(([, ref]) => ref === `refs/${name}` || ref === `refs/tags/${name}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* The head sha of `refs/heads/<branch>`: the loose ref, else `packed-refs`;
|
||||
* null when there is none or the name is ambiguous for git.
|
||||
*/
|
||||
function headSha(gitDir, common, branch) {
|
||||
const packed = packedRefs(common);
|
||||
if (ambiguousName(gitDir, common, branch, packed)) return null;
|
||||
const loose = smallRegular(join(common, 'refs', 'heads', ...branch.split('/')), 4096);
|
||||
if (loose !== null) {
|
||||
const sha = loose.trim();
|
||||
if (SHA_RE.test(sha)) return sha;
|
||||
}
|
||||
const packed = smallRegular(join(common, 'packed-refs'), 16 * 1024 * 1024);
|
||||
if (packed === null) return null;
|
||||
for (const line of packed.split('\n')) {
|
||||
const m = line.match(/^([0-9a-f]{40}|[0-9a-f]{64}) (.+)$/);
|
||||
if (m && m[2] === `refs/heads/${branch}`) return m[1];
|
||||
if (!packed) return null;
|
||||
for (const [sha, ref] of packedLines(packed)) {
|
||||
if (ref === `refs/heads/${branch}`) return sha;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
|
@ -165,7 +225,7 @@ export function evidenceLines(repoRoot, mergeArgs) {
|
|||
const gitDir = gitDirAt(repoRoot);
|
||||
if (!gitDir) return NO_EVIDENCE;
|
||||
const common = commonDirOf(gitDir);
|
||||
const sha = headSha(common, branch);
|
||||
const sha = headSha(gitDir, common, branch);
|
||||
if (!sha) return NO_EVIDENCE;
|
||||
const text = readEvidence(join(common, 'voyage', 'merge-evidence', `${sha}.json`));
|
||||
if (text === 'missing') return NO_EVIDENCE;
|
||||
|
|
|
|||
230
tests/hooks/merge-evidence-ambiguous.test.mjs
Normal file
230
tests/hooks/merge-evidence-ambiguous.test.mjs
Normal file
|
|
@ -0,0 +1,230 @@
|
|||
// tests/hooks/merge-evidence-ambiguous.test.mjs
|
||||
// [AMB] The guard's evidence reader (hooks/lib/merge-evidence.mjs) shows no
|
||||
// run evidence for a merged name git would read as something other than
|
||||
// refs/heads/<name>. git looks a name up as `$GIT_DIR/<name>`, `refs/<name>`,
|
||||
// `refs/tags/<name>` before `refs/heads/<name>`, so with a tag `feat` the
|
||||
// command `git merge feat` merges the tag's commit, while the reader would
|
||||
// show the evidence written for the branch head. An entry of any kind at
|
||||
// `<git dir>/<name>`, `<common>/<name>`, `<common>/refs/<name>` or
|
||||
// `<common>/refs/tags/<name>`, or a `packed-refs` line for `refs/<name>` or
|
||||
// `refs/tags/<name>`, gives "no run evidence". The writer
|
||||
// (lib/review/merge-evidence.mjs) refuses the same names.
|
||||
// [CLU] `--cleanup <mode>` takes a separate value: `git merge --cleanup strip
|
||||
// feat` merges feat, and the reader shows feat's Tested by line.
|
||||
// `keep:` cases hold at the base already and stay untagged.
|
||||
// The reader loads through `import * as m`, so a missing export is a failed
|
||||
// assertion at the red base, never a crash. Every child gets childEnv(); refs
|
||||
// are made with `git update-ref` (and `git pack-refs --all`), never `git tag`.
|
||||
|
||||
import { after, test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { appendFileSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import * as m from '../../hooks/lib/merge-evidence.mjs';
|
||||
import { runHookWithEnv } from '../helpers/hook-helper.mjs';
|
||||
import { childEnv } from '../helpers/child-env.mjs';
|
||||
|
||||
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
||||
const PRE_BASH = join(ROOT, 'hooks', 'scripts', 'pre-bash-executor.mjs');
|
||||
|
||||
const UNDONE_TWO_WAY = 'Undone by: `git revert -m 1 <merge>` (two-way).';
|
||||
const NO_EVIDENCE = 'Tested by: no run evidence.\nUndone by: no run evidence.';
|
||||
|
||||
const made = [];
|
||||
after(() => { for (const b of made) rmSync(b, { recursive: true, force: true }); });
|
||||
|
||||
function g(cwd, ...args) {
|
||||
const r = spawnSync('git', args, { cwd, encoding: 'utf8', env: childEnv() });
|
||||
if (r.status !== 0) throw new Error(`git ${args.join(' ')}: ${r.stderr}`);
|
||||
return r.stdout.trim();
|
||||
}
|
||||
|
||||
/**
|
||||
* A throwaway repo on `main` (README) with a branch `feat` one commit ahead
|
||||
* (a.mjs), checked out on main. No hooks, no signing.
|
||||
* (Copied from tests/hooks/merge-evidence.test.mjs `mevRepo`.)
|
||||
*/
|
||||
function ambRepo() {
|
||||
const base = realpathSync(mkdtempSync(join(tmpdir(), 'merge-evidence-amb-')));
|
||||
made.push(base);
|
||||
const repo = join(base, 'repo');
|
||||
mkdirSync(repo, { recursive: true });
|
||||
mkdirSync(join(base, 'hooks-none'));
|
||||
g(repo, 'init', '-q', '-b', 'main');
|
||||
appendFileSync(join(repo, '.git', 'config'), '[user]\n\tname = Test\n\temail = test@example.invalid\n'
|
||||
+ `[commit]\n\tgpgsign = false\n[core]\n\thooksPath = "${join(base, 'hooks-none')}"\n`);
|
||||
writeFileSync(join(repo, 'README.md'), '# t\n');
|
||||
g(repo, 'add', 'README.md');
|
||||
g(repo, 'commit', '-q', '-m', 'init');
|
||||
const mainSha = g(repo, 'rev-parse', 'HEAD');
|
||||
g(repo, 'checkout', '-q', '-b', 'feat');
|
||||
writeFileSync(join(repo, 'a.mjs'), 'export const a = 1;\n');
|
||||
g(repo, 'add', 'a.mjs');
|
||||
g(repo, 'commit', '-q', '-m', 'feat: the work');
|
||||
const featSha = g(repo, 'rev-parse', 'HEAD');
|
||||
g(repo, 'checkout', '-q', 'main');
|
||||
const common = join(repo, '.git');
|
||||
const evDir = join(common, 'voyage', 'merge-evidence');
|
||||
return {
|
||||
base, repo, mainSha, featSha, common, evDir,
|
||||
/** Write an evidence file by hand (an object as JSON). */
|
||||
put: (body, sha = featSha) => {
|
||||
mkdirSync(evDir, { recursive: true });
|
||||
writeFileSync(join(evDir, `${sha}.json`), JSON.stringify(body, null, 2));
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/** A complete evidence object for `sha` on `branch` (copied from tests/hooks/merge-evidence.test.mjs `evidence`). */
|
||||
function evidence(sha, branch = 'feat') {
|
||||
return {
|
||||
v: 1, branch, branch_head_sha: sha, written_at: '2026-11-02T10:00:00.000Z',
|
||||
end_to_end: { command: '/bin/bash tests/e2e.sh', exit: 0 },
|
||||
verification: { passed: 5, total: 5 },
|
||||
review: { verdict: 'ALLOW', sha },
|
||||
door: { type: 'two-way', text: 'a revert restores the old gate' },
|
||||
};
|
||||
}
|
||||
|
||||
const tested = (sha) =>
|
||||
`Tested by: End-to-end \`/bin/bash tests/e2e.sh\` exit 0; Verification 5/5; review ALLOW at ${sha.slice(0, 8)}.`;
|
||||
|
||||
/** The guard on a Bash call, run in `cwd` (copied from tests/hooks/merge-evidence.test.mjs `ask`). */
|
||||
async function ask(cwd, command = 'git merge --no-ff feat') {
|
||||
const input = { hook_event_name: 'PreToolUse', tool_name: 'Bash', cwd, tool_input: { command } };
|
||||
const r = await runHookWithEnv(PRE_BASH, input, {});
|
||||
let out = null;
|
||||
try { out = JSON.parse(String(r.stdout).trim()); } catch { out = null; }
|
||||
return {
|
||||
...r,
|
||||
decision: out?.hookSpecificOutput?.permissionDecision ?? null,
|
||||
reason: String(out?.hookSpecificOutput?.permissionDecisionReason ?? ''),
|
||||
};
|
||||
}
|
||||
|
||||
/** The repo's evidence for feat's head, and the guard's question and the reader's lines for `git merge --no-ff feat`. */
|
||||
async function asked(fx, command = 'git merge --no-ff feat', args = ['--no-ff', 'feat']) {
|
||||
const r = await ask(fx.repo, command);
|
||||
assert.equal(r.decision, 'ask', `Control: the merge into main is asked about: ${r.stdout}${r.stderr}`);
|
||||
return { reason: r.reason, lines: m.evidenceLines(fx.repo, args) };
|
||||
}
|
||||
|
||||
test('the reader module exports evidenceLines', () => {
|
||||
assert.equal(typeof m.evidenceLines, 'function');
|
||||
assert.equal(typeof m.mergedBranch, 'function');
|
||||
});
|
||||
|
||||
// ---- the ambiguous names ----------------------------------------------------------
|
||||
|
||||
test('[AMB] a loose tag with the branch\'s name gives no run evidence although an evidence file for the branch head exists', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
g(fx.repo, 'update-ref', 'refs/tags/feat', fx.mainSha);
|
||||
assert.equal(g(fx.repo, 'rev-parse', '--verify', 'refs/tags/feat'), fx.mainSha, 'Control: the tag is there');
|
||||
const { reason, lines } = await asked(fx);
|
||||
assert.equal(lines, NO_EVIDENCE, 'the reader shows no evidence for an ambiguous name');
|
||||
assert.ok(reason.includes(NO_EVIDENCE), reason);
|
||||
assert.ok(!reason.includes(tested(fx.featSha)), `no Tested by line for the branch head:\n${reason}`);
|
||||
});
|
||||
|
||||
test('[AMB] a packed tag with the branch\'s name gives no run evidence', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
g(fx.repo, 'update-ref', 'refs/tags/feat', fx.mainSha);
|
||||
g(fx.repo, 'pack-refs', '--all');
|
||||
const packed = readFileSync(join(fx.common, 'packed-refs'), 'utf8');
|
||||
assert.ok(packed.includes(`${fx.mainSha} refs/tags/feat`), `Control: the tag is packed:\n${packed}`);
|
||||
assert.ok(packed.includes(`${fx.featSha} refs/heads/feat`), `Control: the branch is packed too:\n${packed}`);
|
||||
const { reason, lines } = await asked(fx);
|
||||
assert.equal(lines, NO_EVIDENCE);
|
||||
assert.ok(reason.includes(NO_EVIDENCE), reason);
|
||||
assert.ok(!reason.includes(tested(fx.featSha)), reason);
|
||||
});
|
||||
|
||||
test('[AMB] refs/<name> gives no run evidence', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
g(fx.repo, 'update-ref', 'refs/feat', fx.mainSha);
|
||||
assert.equal(g(fx.repo, 'rev-parse', '--verify', 'refs/feat'), fx.mainSha, 'Control: refs/feat is there');
|
||||
const { reason, lines } = await asked(fx);
|
||||
assert.equal(lines, NO_EVIDENCE);
|
||||
assert.ok(reason.includes(NO_EVIDENCE), reason);
|
||||
assert.ok(!reason.includes(tested(fx.featSha)), reason);
|
||||
|
||||
// Control: packed, refs/feat still gives no evidence.
|
||||
g(fx.repo, 'pack-refs', '--all');
|
||||
assert.ok(readFileSync(join(fx.common, 'packed-refs'), 'utf8').includes(`${fx.mainSha} refs/feat`), 'Control: refs/feat is packed');
|
||||
assert.equal(m.evidenceLines(fx.repo, ['--no-ff', 'feat']), NO_EVIDENCE, 'packed refs/<name>');
|
||||
});
|
||||
|
||||
test('[AMB] a file <git dir>/<name> gives no run evidence', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
writeFileSync(join(fx.common, 'feat'), `${fx.mainSha}\n`);
|
||||
const { reason, lines } = await asked(fx);
|
||||
assert.equal(lines, NO_EVIDENCE);
|
||||
assert.ok(reason.includes(NO_EVIDENCE), reason);
|
||||
assert.ok(!reason.includes(tested(fx.featSha)), reason);
|
||||
});
|
||||
|
||||
test('[AMB] a packed-refs that cannot be read within the cap counts as ambiguous, never as "no tag"', () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
assert.equal(m.evidenceLines(fx.repo, ['--no-ff', 'feat']), `${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`, 'Control: shown before');
|
||||
// A packed-refs that is a directory (there, not a regular file): cannot tell.
|
||||
mkdirSync(join(fx.common, 'packed-refs'));
|
||||
assert.equal(m.evidenceLines(fx.repo, ['--no-ff', 'feat']), NO_EVIDENCE);
|
||||
});
|
||||
|
||||
// ---- --cleanup takes a value -------------------------------------------------------
|
||||
|
||||
test('[CLU] git merge --cleanup strip feat shows feat\'s Tested by line', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
// A branch named after the mode, with its own evidence, must not be read as the merged one.
|
||||
g(fx.repo, 'update-ref', 'refs/heads/strip', fx.mainSha);
|
||||
fx.put(evidence(fx.mainSha, 'strip'), fx.mainSha);
|
||||
assert.equal(m.mergedBranch(['--cleanup', 'strip', 'feat']), 'feat');
|
||||
const { reason, lines } = await asked(fx, 'git merge --cleanup strip feat', ['--cleanup', 'strip', 'feat']);
|
||||
assert.equal(lines, `${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`);
|
||||
assert.ok(reason.includes(`${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`), reason);
|
||||
assert.ok(!reason.includes(tested(fx.mainSha)), reason);
|
||||
});
|
||||
|
||||
// ---- keep ----------------------------------------------------------------------
|
||||
|
||||
test('keep: an unambiguous branch still shows its Tested by line', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
// Other refs that are not the name: a tag and a ref under another name, a packed branch.
|
||||
g(fx.repo, 'update-ref', 'refs/tags/v1', fx.mainSha);
|
||||
g(fx.repo, 'update-ref', 'refs/tags/feat-old', fx.mainSha);
|
||||
const { reason, lines } = await asked(fx);
|
||||
assert.equal(lines, `${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`);
|
||||
assert.ok(reason.includes(`${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`), reason);
|
||||
g(fx.repo, 'pack-refs', '--all');
|
||||
assert.equal(m.evidenceLines(fx.repo, ['--no-ff', 'feat']), `${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`, 'packed');
|
||||
});
|
||||
|
||||
test('keep: git merge --no-ff -m "Merge run branch" feat asks and never shows another branch\'s Tested by line', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
// A branch named after a word of the message, with its own evidence.
|
||||
g(fx.repo, 'update-ref', 'refs/heads/Merge', fx.mainSha);
|
||||
fx.put(evidence(fx.mainSha, 'Merge'), fx.mainSha);
|
||||
const r = await ask(fx.repo, 'git merge --no-ff -m "Merge run branch" feat');
|
||||
assert.equal(r.decision, 'ask', `${r.stdout}${r.stderr}`);
|
||||
assert.ok(!r.reason.includes(tested(fx.mainSha)), `not the other branch's line:\n${r.reason}`);
|
||||
assert.ok(r.reason.includes(NO_EVIDENCE) || r.reason.includes(`${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`), r.reason);
|
||||
});
|
||||
|
||||
test('keep: git merge --no-ff feat -m "merge: x" shows feat\'s Tested by line', async () => {
|
||||
const fx = ambRepo();
|
||||
fx.put(evidence(fx.featSha));
|
||||
const r = await ask(fx.repo, 'git merge --no-ff feat -m "merge: x"');
|
||||
assert.equal(r.decision, 'ask', `${r.stdout}${r.stderr}`);
|
||||
assert.ok(r.reason.includes(`${tested(fx.featSha)}\n${UNDONE_TWO_WAY}`), r.reason);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue