fix(verdict-gate): accept only a full 40- or 64-hex reviewed commit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-26 13:13:22 +01:00
commit cf7af30b46
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
3 changed files with 77 additions and 15 deletions

View file

@ -27,7 +27,12 @@
import { spawnSync } from 'node:child_process';
import { LESSONS_FILE_NAME, MARK_START, MARK_END, withPointer } from './recurrence.mjs';
const SHA_RE = /^[0-9a-f]{7,40}$/i;
/**
* A full commit id: 40 hex (SHA-1) or 64 hex (SHA-256), nothing shorter (M11).
* A short id can also be a branch name, which resolves to wherever the branch
* points now (DJ3).
*/
export const FULL_SHA_RE = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i;
const LESSONS_REL = `.claude/rules/${LESSONS_FILE_NAME}`; // posix, as `git diff` prints it
const CLAUDE_REL = 'CLAUDE.md';
const SYMLINK_MODE = '120000';
@ -41,8 +46,9 @@ export function reviewedSha(reviewText) {
const fm = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/.exec(String(reviewText ?? ''));
const m = fm && /^scope_sha_end:[ \t]*(["']?)([^"'\s#]*)\1[ \t]*(?:#.*)?$/m.exec(fm[1]);
// A commit sha only: a ref name (HEAD, main, a tag) resolves to whatever it
// points at now, and coverage would pass whatever was reviewed (round 4).
return m && SHA_RE.test(m[2]) ? m[2] : '';
// points at now, and coverage would pass whatever was reviewed (round 4);
// so can a short id that is also a branch name (M11).
return m && FULL_SHA_RE.test(m[2]) ? m[2] : '';
}
function git(repo, args) {
@ -101,14 +107,17 @@ export function checkReviewedCommit({ repo, sha } = {}) {
if (typeof repo !== 'string' || repo === '') return stale('no repository given');
const given = String(sha ?? '');
const notCommit = `reviewed commit ${given} is not a commit in this repository`;
// 1. Hex only, so a value like `--output=/x` never reaches git as an option.
if (!SHA_RE.test(given)) return stale(notCommit);
// 1. Full hex only, so a value like `--output=/x` never reaches git as an option.
if (!FULL_SHA_RE.test(given)) return stale(notCommit);
const resolved = git(repo, ['rev-parse', '--verify', '--quiet', `${given}^{commit}`]);
if (resolved.status !== 0) {
const note = gitNote(resolved);
return stale(note ? `${notCommit} (${note})` : notCommit);
}
const full = resolved.stdout.trim();
// A ref named like an id (64 hex in a SHA-1 repository) also resolves: the
// id must be the commit itself, not a name that points at one (M11).
if (full !== given.toLowerCase()) return stale(`${given} is not a full commit id in this repository`);
// 2. The review is of HEAD itself.
const head = git(repo, ['rev-parse', '--verify', 'HEAD']);

View file

@ -272,13 +272,13 @@ after(() => {
for (const seed of seeds.values()) rmSync(seed.base, { recursive: true, force: true });
});
function seedRepo({ sub, claude, claudeLink }) {
function seedRepo({ sub, claude, claudeLink, objectFormat }) {
const base = mkdtempSync(join(tmpdir(), 'verdict-gate-r6-seed-'));
const repo = join(base, 'repo');
const dir = sub ? join(repo, sub) : repo;
mkdirSync(join(dir, '.claude', 'rules'), { recursive: true });
mkdirSync(join(base, 'hooks-none'));
g(repo, 'init', '-q', '-b', 'main');
g(repo, 'init', '-q', '-b', 'main', ...(objectFormat ? [`--object-format=${objectFormat}`] : []));
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`);
putIn(dir, 'a.mjs', 'export const a = 1;\n');
@ -297,12 +297,13 @@ function seedRepo({ sub, claude, claudeLink }) {
/**
* A throwaway repo with one reviewed commit (a.mjs, CLAUDE.md, the lessons
* file), no git hooks. `sub` puts the project in a subdirectory of the repo;
* `claudeLink` makes CLAUDE.md a symlink to AGENTS.md. Each variant is built
* once; every call gets its own copy of it.
* `claudeLink` makes CLAUDE.md a symlink to AGENTS.md; `objectFormat`
* (`sha256`) picks the repository's hash. Each variant is built once; every
* call gets its own copy of it.
*/
function gateRepo({ sub = '', claude = '# Project\n', claudeLink = false } = {}) {
const key = JSON.stringify([sub, claude, claudeLink]);
if (!seeds.has(key)) seeds.set(key, seedRepo({ sub, claude, claudeLink }));
function gateRepo({ sub = '', claude = '# Project\n', claudeLink = false, objectFormat = '' } = {}) {
const key = JSON.stringify([sub, claude, claudeLink, objectFormat]);
if (!seeds.has(key)) seeds.set(key, seedRepo({ sub, claude, claudeLink, objectFormat }));
const seed = seeds.get(key);
const base = mkdtempSync(join(tmpdir(), 'verdict-gate-r6-'));
cpSync(seed.base, base, { recursive: true, verbatimSymlinks: true });
@ -578,3 +579,49 @@ test('verdict-gate --repo — a scope_sha_end like --output=<file> never reaches
assert.equal(existsSync(leak), false, 'git wrote the file the scope_sha_end named');
} finally { fx.cleanup(); }
});
// ---- M11: only a full 40- or 64-hex reviewed commit opens the gate ----
/** Can the local git create a SHA-256 repository? Probed once, in a throwaway directory. */
const sha256Ok = (() => {
const dir = mkdtempSync(join(tmpdir(), 'verdict-gate-sha256-probe-'));
try {
return spawnSync('git', ['init', '-q', '--object-format=sha256', dir], { encoding: 'utf8', env: childEnv() }).status === 0;
} finally {
rmSync(dir, { recursive: true, force: true });
}
})();
test('[M11] a 64-hex reviewed commit opens the gate in a SHA-256 repository', { skip: !sha256Ok && 'git cannot create a SHA-256 repository' }, () => {
const fx = gateRepo({ objectFormat: 'sha256' });
try {
assert.match(fx.reviewed, /^[0-9a-f]{64}$/, `the fixture is a SHA-256 repository: ${fx.reviewed}`);
const r = fx.gate();
assert.equal(r.status, 0, r.stdout + r.stderr);
} finally { fx.cleanup(); }
});
test('[M11] a 7-character reviewed commit that is also a branch at HEAD is stale', () => {
// DJ3: a short id that names a branch resolves to wherever the branch points
// now, so the review would cover work it never saw.
const fx = gateRepo();
try {
fx.commit('fix: more work', { 'a.mjs': 'export const a = 2;\n' });
const name = 'deadbee';
fx.git('branch', name, 'HEAD');
assertStale(fx.gate({ sha: name }));
} finally { fx.cleanup(); }
});
test('[M11] a 64-hex name of a branch in a SHA-1 repository is stale with the reason "not a full commit id"', () => {
// In a SHA-1 repository a 64-hex name is no object id; git resolves it as a
// ref name to whatever the branch points at (here HEAD).
const fx = gateRepo();
try {
const name = 'c0ffee'.padEnd(64, '0');
fx.git('branch', name, 'HEAD');
const r = fx.gate({ sha: name });
assertStale(r, /not a full commit id/);
assert.ok(r.stdout.includes(name), `the reason names the id as written: ${r.stdout}`);
} finally { fx.cleanup(); }
});

View file

@ -406,10 +406,16 @@ test('release-check r3 — the review and the approval log committed after the r
} finally { fx.cleanup(); }
});
test('release-check r3 — scope_sha_end is read as written: an all-digit sha keeps its leading zero', () => {
test('[M11] sibling: release-check r3 — scope_sha_end is read as written: an all-digit 40-character sha keeps its leading zero, a 7-character id is refused', () => {
assert.equal(typeof mod.reviewedSha, 'function');
assert.equal(mod.reviewedSha('---\ntype: trekreview\nscope_sha_end: 0123456\n---\nbody\n'), '0123456');
assert.equal(mod.reviewedSha('---\nscope_sha_end: "00ab12c"\n---\n'), '00ab12c');
const digits = '0123456789012345678901234567890123456789';
assert.equal(mod.reviewedSha(`---\ntype: trekreview\nscope_sha_end: ${digits}\n---\nbody\n`), digits);
const quoted = '00ab12c'.padEnd(40, '0');
assert.equal(mod.reviewedSha(`---\nscope_sha_end: "${quoted}"\n---\n`), quoted);
const sha256 = '00ab12c'.padEnd(64, 'f');
assert.equal(mod.reviewedSha(`---\nscope_sha_end: ${sha256}\n---\n`), sha256, 'a 64-hex (SHA-256) id is read');
assert.equal(mod.reviewedSha('---\ntype: trekreview\nscope_sha_end: 0123456\n---\nbody\n'), '', 'a 7-character id is refused');
assert.equal(mod.reviewedSha('---\nscope_sha_end: "00ab12c"\n---\n'), '', 'a quoted 7-character id is refused');
assert.equal(mod.reviewedSha('---\ntype: trekreview\n---\nscope_sha_end: 1\n'), '', 'only the frontmatter counts');
});