From a210bd3644f98829a4e7005b249dfbc6fc191161 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 14:04:53 +0000 Subject: [PATCH 1/5] fix(sealed): waivers must name the full commit sha (GST-268) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 11 ++++++ docs/pregate.md | 6 ++- docs/troubleshooting.md | 3 +- lib/result-codes.ts | 2 +- lib/sealed-guard.ts | 20 +++++----- test/sealed-guard.test.ts | 79 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 105 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f49fe21d4..cfce25163 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [Unreleased] + +**A sealed-guard waiver now counts only when it names the full commit sha.** A `.gstack/sealed-allow.tsv` line used to accept a sha as short as 7 hex characters and waive any commit whose sha started with it. A PR author could generate a commit with the same first 7 characters in minutes and publish different content at the same path under an old, reviewed waiver. A waiver now matches only the finding's full commit sha (40 hex characters, 64 in a SHA-256 repository). A shortened, uppercase or mistyped sha is ignored, so the finding still refuses, and stderr says which line was ignored and which full sha to write. + +### Changed +- `lib/sealed-guard.ts`: waivers compare the whole commit sha; `gstack-pregate sealed --json` adds `ignoredWaivers` (rule, path, the sha as written, the finding's commit) for waiver lines that name a finding's rule and path but not its commit. Lines that match no finding stay silent. +- The refusal hint, the `SEALED_OUTPUT_REFUSED` fix text, docs/pregate.md and docs/troubleshooting.md say the sha must be the full commit sha. + +### Tests +- `test/sealed-guard.test.ts`: 7-, 12- and 39-character prefixes, an uppercase sha and one extra character do not waive and are named; the full sha waives, trailing whitespace included; another commit's full sha does not waive; a short line with no matching finding prints nothing; a SHA-256 repository accepts 64 characters and rejects 40, 63 and 65; error reports carry an empty list. The two existing waiver tests now write the full sha. + ## [1.91.330.0] - 2026-10-30 **/ship reviewers now check a suspected defect against the code it depends on before they report it or let it go.** A reviewer packet used to allow at most two more ranges of up to 200 lines, from files in the diff, in one turn. In GSTA-167's no-loss replay, 7 recorded blockers needed context outside those ranges, and no reviewer found them. Each reviewer now confirms or rules out a suspected defect by reading the callers and callees of changed functions, the tests that cover them, and the config and contracts they read. It uses grep and the read tool, at most 8 reads in at most 2 turns of parallel calls. The coverage audit keeps its own budget. diff --git a/docs/pregate.md b/docs/pregate.md index 1cab42ce0..65ea249ac 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -840,7 +840,11 @@ gstack-pregate sealed [--range ] [--remote origin] [--base ] [--js `git ls-remote `. A malformed or unreadable policy refuses. Waivers come only from the base branch's `.gstack/sealed-allow.tsv` (`rulepathshareason`) and are - listed on every push. + listed on every push. A waiver counts only when `sha` is the finding's full + commit sha (40 hex characters, 64 in a SHA-256 repository); a shortened or + mistyped one is ignored, so the finding still refuses, and stderr names it + (`ignored waiver sha `) with the full commit to write. + `--json` lists them as `ignoredWaivers`. - **Visibility.** `sealed.visibility` from the base branch, else a cache under the state root (`sealed-visibility.json`: a `public` answer for 24 hours, a `private` one for 1 hour before it may turn a refusal into a warning), else diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 6379e72fd..3ecc60217 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3825,7 +3825,8 @@ and counts) can be exempted from the path rules with a `sealed.allow_paths` glob in the base branch's `.gstack/pregate.json`; the text rules still read it ([docs/pregate.md#sealed](pregate.md#sealed)). A reviewed false positive gets one line in the base branch's -`.gstack/sealed-allow.tsv`: `rulepathshareason`. A private +`.gstack/sealed-allow.tsv`: `rulepathshareason`, where `sha` is +the full commit sha (a shortened one is ignored and named on stderr). A private destination sets `"visibility": "private"` under `sealed` in the base branch's `.gstack/pregate.json`. Capy's pull-request tool may not run local hooks, so run `gstack-pregate sealed` yourself before any push or PR creation. diff --git a/lib/result-codes.ts b/lib/result-codes.ts index 80e6f16f1..04dfa6408 100644 --- a/lib/result-codes.ts +++ b/lib/result-codes.ts @@ -513,7 +513,7 @@ export const RESULT_CODES = { SEALED_OUTPUT_REFUSED: { anchor: 'sealed-output-refused', summary: 'a push or PR would publish sealed or eval output (a sealed path, a transcript, a quoted answer, a seed or a local home path) to a public or unknown destination, or the sealed policy is unreadable', - fix: 'remove it from every published commit, or add one reviewed `rulepathshareason` line to the base branch\'s .gstack/sealed-allow.tsv; a private destination sets sealed.visibility "private" in the base branch\'s .gstack/pregate.json', + fix: 'remove it from every published commit, or add one reviewed `rulepathfull-commit-shareason` line to the base branch\'s .gstack/sealed-allow.tsv (a shortened sha is ignored); a private destination sets sealed.visibility "private" in the base branch\'s .gstack/pregate.json', }, EVAL_GATE_UNPRICED: { anchor: 'eval-gate-unpriced', diff --git a/lib/sealed-guard.ts b/lib/sealed-guard.ts index cda351a97..69a4aec60 100644 --- a/lib/sealed-guard.ts +++ b/lib/sealed-guard.ts @@ -22,7 +22,7 @@ * destination base branch's `sealed` and the pushed one (a push can tighten, * never relax); `allow_paths` exceptions come only from the base branch's * policy and remove path findings before waivers, which come only from the - * base branch's `.gstack/sealed-allow.tsv`. A malformed or unreadable policy refuses (fail + * base branch's `.gstack/sealed-allow.tsv` and must name the full commit sha. A malformed or unreadable policy refuses (fail * closed). Visibility: the base override, else a cache under the state root * (a `public` answer for 24 h, a `private` one for 1 h before it may * downgrade a refusal), else `gh repo view` (5 s); unknown counts as public. @@ -104,13 +104,13 @@ export function unionPolicies(base: SealedPolicy | null, pushed: SealedPolicy | return { paths: u('paths'), transcripts: u('transcripts'), answer_markers: u('answer_markers'), seed_patterns: u('seed_patterns'), local_paths: !!(base?.local_paths || pushed?.local_paths), allow_paths: base?.allow_paths ?? [], case_insensitive: !!(base?.case_insensitive || pushed?.case_insensitive), visibility }; } -/** Reviewed waivers: `rulepathshareason` lines in the base branch's allow file. */ +/** Reviewed waivers: `rulepathshareason` lines in the base branch's allow file; a waiver counts only when its sha equals the finding's full commit sha. */ export function allowEntriesAt(repoRoot: string, treeish: string | null): AllowEntry[] { if (!treeish) return []; const show = git(repoRoot, ['cat-file', 'blob', `${treeish}:${SEALED_ALLOW_PATH}`]); if (!show.ok) return []; - return show.out.split('\n').filter(l => l.trim() && !l.startsWith('#')).map(l => l.split('\t')).filter(f => f.length >= 4 && /^[0-9a-f]{7,64}$/.test(f[2]!) && f[3]!.trim()) - .map(f => ({ rule: f[0]!.trim(), path: f[1]!.trim(), sha: f[2]!, reason: f.slice(3).join('\t').trim() })); + return show.out.split('\n').filter(l => l.trim() && !l.startsWith('#')).map(l => l.split('\t')).filter(f => f.length >= 4 && f[2]!.trim() && f[3]!.trim()) + .map(f => ({ rule: f[0]!.trim(), path: f[1]!.trim(), sha: f[2]!.trim(), reason: f.slice(3).join('\t').trim() })); } /** Commits a push publishes: local minus the remote's refs (and its tip for this ref); null remote and no tip ā†’ everything reachable. */ @@ -229,7 +229,7 @@ export function resolveVisibility(url: string | null, o: VisibilityOptions = {}) export interface SealedReport { status: 'no-policy' | 'clear' | 'refused' | 'warned' | 'error'; - policyLabels: string[]; commits: number; findings: SealedFinding[]; exempt: ExemptFinding[]; waived: Array; + policyLabels: string[]; commits: number; findings: SealedFinding[]; exempt: ExemptFinding[]; waived: Array; ignoredWaivers: Array<{ rule: string; path: string; sha: string; commit: string }>; visibility: Visibility; visibilitySource: string; target: string; error?: string; } @@ -240,7 +240,7 @@ export interface EvaluateOptions { /** A report for a failure before any policy is read (an unlisted range, an unreadable push); renders as a fail-closed refusal. */ export function sealedErrorReport(error: string, target: string): SealedReport { - return { status: 'error', error, policyLabels: [], commits: 0, findings: [], exempt: [], waived: [], visibility: 'unknown', visibilitySource: '', target }; + return { status: 'error', error, policyLabels: [], commits: 0, findings: [], exempt: [], waived: [], ignoredWaivers: [], visibility: 'unknown', visibilitySource: '', target }; } /** The whole guard for one push (or range): policies, commits, findings, waivers, visibility, verdict. */ @@ -271,13 +271,14 @@ export function evaluateSealed(o: EvaluateOptions): SealedReport { const allow = allowEntriesAt(o.repoRoot, o.baseRef); const waived: SealedReport['waived'] = []; const findings = found.filter(f => { - const entry = allow.find(a => a.rule === f.rule && a.path === f.file && f.commit.startsWith(a.sha)); + const entry = allow.find(a => a.rule === f.rule && a.path === f.file && a.sha === f.commit); if (entry) waived.push({ ...f, reason: entry.reason }); return !entry; }); + const ignoredWaivers = findings.flatMap(f => allow.filter(a => a.rule === f.rule && a.path === f.file).map(a => ({ rule: a.rule, path: a.path, sha: a.sha, commit: f.commit }))); const vis = policy.visibility ? { visibility: policy.visibility as Visibility, source: `${VISIBILITY_OVERRIDE_KEY} override` } : resolveVisibility(o.remoteUrl, { env: o.env, gh: o.gh }); const status = findings.length === 0 ? 'clear' : vis.visibility === 'private' ? 'warned' : 'refused'; - return { status, policyLabels: labels, commits: commits.length, findings, exempt, waived, visibility: vis.visibility, visibilitySource: vis.source, target: o.target }; + return { status, policyLabels: labels, commits: commits.length, findings, exempt, waived, ignoredWaivers, visibility: vis.visibility, visibilitySource: vis.source, target: o.target }; } const where = (f: SealedFinding) => `${f.file || '(unknown file)'}${f.line ? `:${f.line}` : ''}`; @@ -287,6 +288,7 @@ export function renderSealed(r: SealedReport, context: string): string { const lines: string[] = []; for (const e of r.exempt) lines.push(`${context}: exempt ${e.rule} ${where(e)} commit ${e.commit.slice(0, 12)} (sealed.allow_paths ${e.glob})`); for (const w of r.waived) lines.push(`${context}: waived ${w.rule} ${where(w)} commit ${w.commit.slice(0, 12)} (${SEALED_ALLOW_PATH}: ${w.reason})`); + for (const i of r.ignoredWaivers) lines.push(`${context}: ignored waiver ${i.rule} ${i.path} sha ${i.sha} (${SEALED_ALLOW_PATH} must name the full commit sha ${i.commit})`); const vis = `visibility: ${r.visibility}${r.visibility === 'unknown' ? ', counted public' : ''}; ${r.visibilitySource}`; const body = r.findings.map(f => ` ${f.rule} ${where(f)} commit ${f.commit.slice(0, 12)}`); if (r.status === 'error') { @@ -294,7 +296,7 @@ export function renderSealed(r: SealedReport, context: string): string { lines.push(`Fix the sealed policy (gstack-pregate validate), then retry; an unreadable policy never passes (SEALED_OUTPUT_REFUSED)`); } else if (r.status === 'refused') { lines.push(`\nā›” ${context} BLOCKED the publication of ${r.target} (${vis}) ā€” sealed or eval output in ${r.commits} published commit(s):`, ...body); - lines.push(`Remove it from every published commit before this branch reaches a public repository, exempt a publishable path with a sealed.allow_paths glob in the base branch's ${SEALED_CONFIG_PATH}, or waive one reviewed finding with a \`rulepathshareason\` line in the base branch's ${SEALED_ALLOW_PATH}. A private destination: set ${VISIBILITY_OVERRIDE_KEY} "private" in the base branch's ${SEALED_CONFIG_PATH} (SEALED_OUTPUT_REFUSED)`); + lines.push(`Remove it from every published commit before this branch reaches a public repository, exempt a publishable path with a sealed.allow_paths glob in the base branch's ${SEALED_CONFIG_PATH}, or waive one reviewed finding with a \`rulepathfull-commit-shareason\` line in the base branch's ${SEALED_ALLOW_PATH}. A private destination: set ${VISIBILITY_OVERRIDE_KEY} "private" in the base branch's ${SEALED_CONFIG_PATH} (SEALED_OUTPUT_REFUSED)`); } else if (r.status === 'warned') { lines.push(`${context}: warning ā€” sealed or eval output in a push to a private destination (${vis}); it would be refused for a public one:`, ...body); } diff --git a/test/sealed-guard.test.ts b/test/sealed-guard.test.ts index 00874c0a9..4a9cdaeaa 100644 --- a/test/sealed-guard.test.ts +++ b/test/sealed-guard.test.ts @@ -41,12 +41,12 @@ function sh(cwd: string, env: Record, args: string[]): string { } /** A bare origin with main carrying the given base files, and a clone on branch `results`. */ -function scratch(baseFiles: Record): Scratch { +function scratch(baseFiles: Record, objectFormat: 'sha1' | 'sha256' = 'sha1'): Scratch { const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'sealed-guard-'))); const work = path.join(root, 'work'); const origin = path.join(root, 'origin.git'); const state = path.join(root, 'state'); fs.mkdirSync(work); fs.mkdirSync(state); const env = { HOME: root, GSTACK_HOME: state, GSTACK_STATE_ROOT: state, GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1' }; - fs.writeFileSync(env.GIT_CONFIG_GLOBAL, '[user]\n\tname = t\n\temail = t@example.com\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n'); + fs.writeFileSync(env.GIT_CONFIG_GLOBAL, `[user]\n\tname = t\n\temail = t@example.com\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n\tdefaultObjectFormat = ${objectFormat}\n`); spawnSync('git', ['init', '-q', '--bare', origin], { env: { ...process.env, ...env }, timeout: 30_000 }); sh(work, env, ['init', '-q', '-b', 'main']); write(work, { 'README.md': '# evals\n', ...baseFiles }); @@ -145,7 +145,7 @@ describe('policy union, waivers, private destinations and fail-closed parsing', test('a waiver counts only from the base branch allow file and is listed; the same line committed in the push is ignored', () => { const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); const sha = commit(s, { 'runs/c1/transcript.jsonl': '{}\n' }); - const allow = `# rule\tpath\tsha\treason\nsealed.transcript\truns/c1/transcript.jsonl\t${sha.slice(0, 12)}\tsynthetic fixture, no sealed content\n`; + const allow = `# rule\tpath\tsha\treason\nsealed.transcript\truns/c1/transcript.jsonl\t${sha}\tsynthetic fixture, no sealed content\n`; commit(s, { '.gstack/sealed-allow.tsv': allow }); expect(hook(s).code).toBe(1); sh(s.work, s.env, ['checkout', '-q', 'main']); @@ -267,7 +267,7 @@ describe('sealed.allow_paths: base-branch exceptions applied after the deny glob test('an exception covers sealed.transcript too and is counted before a base sha waiver, never both', () => { const s = scratch({ '.gstack/pregate.json': policy({ paths: ['**/sealed/**'], transcripts: ['**/transcript*.jsonl'], allow_paths: ['**/sealed/*/transcript-public.jsonl'] }) }); const sha = commit(s, { 'sealed/s1/transcript-public.jsonl': '{}\n' }); - landOnMain(s, { '.gstack/sealed-allow.tsv': `sealed.transcript\tsealed/s1/transcript-public.jsonl\t${sha.slice(0, 12)}\tsynthetic fixture\n` }); + landOnMain(s, { '.gstack/sealed-allow.tsv': `sealed.transcript\tsealed/s1/transcript-public.jsonl\t${sha}\tsynthetic fixture\n` }); const pre = preflight(s); expect(pre.code).toBe(0); expect(pre.err).toContain('exempt sealed.transcript sealed/s1/transcript-public.jsonl'); @@ -372,3 +372,74 @@ describe('gstack-pregate sealed is a check, not a push (#3565)', () => { expect(before).not.toContain('refs/heads/results'); }); }); + +describe('waivers name the full commit sha (GST-268)', () => { + const TRANSCRIPT = 'runs/c1/transcript.jsonl'; + const line = (sha: string) => `sealed.transcript\t${TRANSCRIPT}\t${sha}\tsynthetic fixture\n`; + + test('a 7-, 12- or 39-character prefix, an uppercase sha or a sha with one extra character does not waive; each is named as ignored with the full commit', () => { + for (const cut of [(x: string) => x.slice(0, 7), (x: string) => x.slice(0, 12), (x: string) => x.slice(0, 39), (x: string) => x.toUpperCase(), (x: string) => `${x}a`]) { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); + const h = hook(s); + expect(h.code).toBe(1); + expect(h.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${cut(sha)}`); + expect(h.err).toContain(`full commit sha ${sha}`); + const pre = preflight(s); + expect(pre.code).toBe(1); + expect(pre.out).toMatch(/^SEALED: refused findings=1 .* waived=0 /m); + const j = JSON.parse(preflight(s, ['--json']).out); + expect(j.ignoredWaivers).toEqual([{ rule: 'sealed.transcript', path: TRANSCRIPT, sha: cut(sha), commit: sha }]); + } + }); + + test('the full 40-character sha waives, trailing whitespace included, with no ignored note', () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(`${sha} `) }); + const r = hook(s); + expect(r.code).toBe(0); + expect(r.err).toContain(`waived sealed.transcript ${TRANSCRIPT}`); + expect(r.err).not.toContain('ignored waiver'); + expect(JSON.parse(preflight(s, ['--json']).out).ignoredWaivers).toEqual([]); + }); + + test("the full sha of a different commit does not waive this commit's file at the same path", () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const first = commit(s, { [TRANSCRIPT]: '{}\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(first) }); + sh(s.work, s.env, ['reset', '-q', '--hard', 'origin/main']); + const second = commit(s, { [TRANSCRIPT]: '{"other":1}\n' }); + const r = hook(s); + expect(r.code).toBe(1); + expect(r.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${first}`); + expect(r.err).toContain(`full commit sha ${second}`); + }); + + test('a short waiver line on the base branch prints nothing on a push with no matching finding', () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED), '.gstack/sealed-allow.tsv': line('abc1234') }); + commit(s, { 'notes.md': 'ok\n' }); + const r = hook(s); + expect(r.code).toBe(0); + expect(r.err).not.toContain('ignored waiver'); + }); + + test('a SHA-256 repository waives with the full 64-character sha; a 40-, 63- or 65-character value does not', () => { + for (const [cut, waives] of [[(x: string) => x, true], [(x: string) => x.slice(0, 40), false], [(x: string) => x.slice(0, 63), false], [(x: string) => `${x}a`, false]] as Array<[(x: string) => string, boolean]>) { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }, 'sha256'); + const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); + expect(sha).toMatch(/^[0-9a-f]{64}$/); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); + const pre = preflight(s); + expect(pre.code).toBe(waives ? 0 : 1); + expect(pre.err.includes('ignored waiver')).toBe(!waives); + } + }); + + test('an error report carries an empty ignoredWaivers list', () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + commit(s, { 'notes.md': 'ok\n' }); + expect(JSON.parse(preflight(s, ['--range', 'nosuch..HEAD', '--json']).out).ignoredWaivers).toEqual([]); + }); +}); From 68674cc316e7fe4fceda7a928cc2ec94b663328c Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 14:11:54 +0000 Subject: [PATCH 2/5] fix(sealed): one ignored-waiver note per line and commit; split the waiver loops into cases Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 +-- lib/sealed-guard.ts | 10 ++++-- test/sealed-guard.test.ts | 69 ++++++++++++++++++++++++--------------- 3 files changed, 53 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfce25163..b851c2c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,11 @@ **A sealed-guard waiver now counts only when it names the full commit sha.** A `.gstack/sealed-allow.tsv` line used to accept a sha as short as 7 hex characters and waive any commit whose sha started with it. A PR author could generate a commit with the same first 7 characters in minutes and publish different content at the same path under an old, reviewed waiver. A waiver now matches only the finding's full commit sha (40 hex characters, 64 in a SHA-256 repository). A shortened, uppercase or mistyped sha is ignored, so the finding still refuses, and stderr says which line was ignored and which full sha to write. ### Changed -- `lib/sealed-guard.ts`: waivers compare the whole commit sha; `gstack-pregate sealed --json` adds `ignoredWaivers` (rule, path, the sha as written, the finding's commit) for waiver lines that name a finding's rule and path but not its commit. Lines that match no finding stay silent. +- `lib/sealed-guard.ts`: waivers compare the whole commit sha; `gstack-pregate sealed --json` adds `ignoredWaivers` (rule, path, the sha as written, the finding's commit) for waiver lines that name a finding's rule and path but not its commit, once per line and commit. Lines that match no finding, and waivers that applied to another commit in the push, stay silent. - The refusal hint, the `SEALED_OUTPUT_REFUSED` fix text, docs/pregate.md and docs/troubleshooting.md say the sha must be the full commit sha. ### Tests -- `test/sealed-guard.test.ts`: 7-, 12- and 39-character prefixes, an uppercase sha and one extra character do not waive and are named; the full sha waives, trailing whitespace included; another commit's full sha does not waive; a short line with no matching finding prints nothing; a SHA-256 repository accepts 64 characters and rejects 40, 63 and 65; error reports carry an empty list. The two existing waiver tests now write the full sha. +- `test/sealed-guard.test.ts`: 7-, 12- and 39-character prefixes, an uppercase sha and one extra character do not waive and are named; the full sha waives, trailing whitespace included; another commit's full sha does not waive; a short line with no matching finding prints nothing; a waiver applied to one commit is not reported for a later one; a text rule with two matching lines names a short waiver once; a SHA-256 repository accepts 64 characters and rejects 40, 63 and 65; error reports carry an empty list. The two existing waiver tests now write the full sha. ## [1.91.330.0] - 2026-10-30 diff --git a/lib/sealed-guard.ts b/lib/sealed-guard.ts index 69a4aec60..6bc9272dc 100644 --- a/lib/sealed-guard.ts +++ b/lib/sealed-guard.ts @@ -275,12 +275,18 @@ export function evaluateSealed(o: EvaluateOptions): SealedReport { if (entry) waived.push({ ...f, reason: entry.reason }); return !entry; }); - const ignoredWaivers = findings.flatMap(f => allow.filter(a => a.rule === f.rule && a.path === f.file).map(a => ({ rule: a.rule, path: a.path, sha: a.sha, commit: f.commit }))); + const ignored = new Map(); + for (const f of findings) for (const a of allow) { + if (a.rule !== f.rule || a.path !== f.file || waived.some(w => w.commit === a.sha && w.rule === a.rule && w.file === a.path)) continue; + ignored.set(`${a.rule}\t${a.path}\t${a.sha}\t${f.commit}`, { rule: a.rule, path: a.path, sha: a.sha, commit: f.commit }); + } + const ignoredWaivers = [...ignored.values()]; const vis = policy.visibility ? { visibility: policy.visibility as Visibility, source: `${VISIBILITY_OVERRIDE_KEY} override` } : resolveVisibility(o.remoteUrl, { env: o.env, gh: o.gh }); const status = findings.length === 0 ? 'clear' : vis.visibility === 'private' ? 'warned' : 'refused'; return { status, policyLabels: labels, commits: commits.length, findings, exempt, waived, ignoredWaivers, visibility: vis.visibility, visibilitySource: vis.source, target: o.target }; } +const FULL_SHA = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; const where = (f: SealedFinding) => `${f.file || '(unknown file)'}${f.line ? `:${f.line}` : ''}`; /** stderr text for a report: findings one per line (` commit `), never the matched text. */ @@ -288,7 +294,7 @@ export function renderSealed(r: SealedReport, context: string): string { const lines: string[] = []; for (const e of r.exempt) lines.push(`${context}: exempt ${e.rule} ${where(e)} commit ${e.commit.slice(0, 12)} (sealed.allow_paths ${e.glob})`); for (const w of r.waived) lines.push(`${context}: waived ${w.rule} ${where(w)} commit ${w.commit.slice(0, 12)} (${SEALED_ALLOW_PATH}: ${w.reason})`); - for (const i of r.ignoredWaivers) lines.push(`${context}: ignored waiver ${i.rule} ${i.path} sha ${i.sha} (${SEALED_ALLOW_PATH} must name the full commit sha ${i.commit})`); + for (const i of r.ignoredWaivers) lines.push(`${context}: ignored waiver ${i.rule} ${i.path} sha ${i.sha} (${FULL_SHA.test(i.sha) ? `it names another commit; this finding is in commit ${i.commit}` : `${SEALED_ALLOW_PATH} must name the full commit sha ${i.commit}`})`); const vis = `visibility: ${r.visibility}${r.visibility === 'unknown' ? ', counted public' : ''}; ${r.visibilitySource}`; const body = r.findings.map(f => ` ${f.rule} ${where(f)} commit ${f.commit.slice(0, 12)}`); if (r.status === 'error') { diff --git a/test/sealed-guard.test.ts b/test/sealed-guard.test.ts index 4a9cdaeaa..aa866aae5 100644 --- a/test/sealed-guard.test.ts +++ b/test/sealed-guard.test.ts @@ -377,21 +377,19 @@ describe('waivers name the full commit sha (GST-268)', () => { const TRANSCRIPT = 'runs/c1/transcript.jsonl'; const line = (sha: string) => `sealed.transcript\t${TRANSCRIPT}\t${sha}\tsynthetic fixture\n`; - test('a 7-, 12- or 39-character prefix, an uppercase sha or a sha with one extra character does not waive; each is named as ignored with the full commit', () => { - for (const cut of [(x: string) => x.slice(0, 7), (x: string) => x.slice(0, 12), (x: string) => x.slice(0, 39), (x: string) => x.toUpperCase(), (x: string) => `${x}a`]) { - const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); - const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); - landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); - const h = hook(s); - expect(h.code).toBe(1); - expect(h.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${cut(sha)}`); - expect(h.err).toContain(`full commit sha ${sha}`); - const pre = preflight(s); - expect(pre.code).toBe(1); - expect(pre.out).toMatch(/^SEALED: refused findings=1 .* waived=0 /m); - const j = JSON.parse(preflight(s, ['--json']).out); - expect(j.ignoredWaivers).toEqual([{ rule: 'sealed.transcript', path: TRANSCRIPT, sha: cut(sha), commit: sha }]); - } + const SHORT: Array<[string, (x: string) => string]> = [['a 7-character prefix', x => x.slice(0, 7)], ['a 12-character prefix', x => x.slice(0, 12)], ['a 39-character prefix', x => x.slice(0, 39)], ['an uppercase sha', x => x.toUpperCase()], ['one extra character', x => `${x}a`]]; + test.each(SHORT)('%s does not waive and is named as ignored with the full commit', (_label, cut) => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); + const h = hook(s); + expect(h.code).toBe(1); + expect(h.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${cut(sha)}`); + expect(h.err).toContain(`full commit sha ${sha}`); + const pre = preflight(s); + expect(pre.code).toBe(1); + expect(pre.out).toMatch(/^SEALED: refused findings=1 .* waived=0 /m); + expect(JSON.parse(preflight(s, ['--json']).out).ignoredWaivers).toEqual([{ rule: 'sealed.transcript', path: TRANSCRIPT, sha: cut(sha), commit: sha }]); }); test('the full 40-character sha waives, trailing whitespace included, with no ignored note', () => { @@ -414,7 +412,7 @@ describe('waivers name the full commit sha (GST-268)', () => { const r = hook(s); expect(r.code).toBe(1); expect(r.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${first}`); - expect(r.err).toContain(`full commit sha ${second}`); + expect(r.err).toContain(`it names another commit; this finding is in commit ${second}`); }); test('a short waiver line on the base branch prints nothing on a push with no matching finding', () => { @@ -425,16 +423,35 @@ describe('waivers name the full commit sha (GST-268)', () => { expect(r.err).not.toContain('ignored waiver'); }); - test('a SHA-256 repository waives with the full 64-character sha; a 40-, 63- or 65-character value does not', () => { - for (const [cut, waives] of [[(x: string) => x, true], [(x: string) => x.slice(0, 40), false], [(x: string) => x.slice(0, 63), false], [(x: string) => `${x}a`, false]] as Array<[(x: string) => string, boolean]>) { - const s = scratch({ '.gstack/pregate.json': policy(SEALED) }, 'sha256'); - const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); - expect(sha).toMatch(/^[0-9a-f]{64}$/); - landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); - const pre = preflight(s); - expect(pre.code).toBe(waives ? 0 : 1); - expect(pre.err.includes('ignored waiver')).toBe(!waives); - } + const SHA256: Array<[string, (x: string) => string, boolean]> = [['the full 64 characters waive', x => x, true], ['a 40-character prefix does not', x => x.slice(0, 40), false], ['a 63-character prefix does not', x => x.slice(0, 63), false], ['65 characters do not', x => `${x}a`, false]]; + test.each(SHA256)('a SHA-256 repository: %s', (_label, cut, waives) => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }, 'sha256'); + const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); + expect(sha).toMatch(/^[0-9a-f]{64}$/); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); + const pre = preflight(s); + expect(pre.code).toBe(waives ? 0 : 1); + expect(pre.err.includes('ignored waiver')).toBe(!waives); + }); + + test('a waiver that applied in one commit is not reported as ignored for a later commit at the same path', () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const first = commit(s, { 'notes/a.md': 'SEALED ANSWER: b\n' }); + const second = commit(s, { 'notes/a.md': 'SEALED ANSWER: b\nSEALED ANSWER: c\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': `sealed.answer\tnotes/a.md\t${first}\tsynthetic fixture\n` }); + const j = JSON.parse(preflight(s, ['--json']).out); + expect(j.waived.map((w: { commit: string }) => w.commit)).toEqual([first]); + expect(j.findings.map((f: { commit: string }) => f.commit)).toEqual([second]); + expect(j.ignoredWaivers).toEqual([]); + }); + + test('a short waiver for a text rule with several matching lines is named once', () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const sha = commit(s, { 'notes/a.md': 'SEALED ANSWER: b\nSEALED ANSWER: c\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': `sealed.answer\tnotes/a.md\t${sha.slice(0, 7)}\tsynthetic fixture\n` }); + const j = JSON.parse(preflight(s, ['--json']).out); + expect(j.findings).toHaveLength(2); + expect(j.ignoredWaivers).toEqual([{ rule: 'sealed.answer', path: 'notes/a.md', sha: sha.slice(0, 7), commit: sha }]); }); test('an error report carries an empty ignoredWaivers list', () => { From 124feda2fef7017d8d24d6dcd78bc5cda07bee6a Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 14:21:29 +0000 Subject: [PATCH 3/5] docs(pregate): quote the ignored-waiver stderr line as printed Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/pregate.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/pregate.md b/docs/pregate.md index 65ea249ac..a3427f145 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -843,7 +843,8 @@ gstack-pregate sealed [--range ] [--remote origin] [--base ] [--js listed on every push. A waiver counts only when `sha` is the finding's full commit sha (40 hex characters, 64 in a SHA-256 repository); a shortened or mistyped one is ignored, so the finding still refuses, and stderr names it - (`ignored waiver sha `) with the full commit to write. + (`ignored waiver sha (...)`) with the full commit to + write. `--json` lists them as `ignoredWaivers`. - **Visibility.** `sealed.visibility` from the base branch, else a cache under the state root (`sealed-visibility.json`: a `public` answer for 24 hours, a From 9cb07426dba4c2ea5c4295a4bfe75750827baa63 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 14:31:37 +0000 Subject: [PATCH 4/5] test(sealed): create SHA-256 scratch repos with --object-format, not the init config Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/sealed-guard.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/sealed-guard.test.ts b/test/sealed-guard.test.ts index aa866aae5..3c6afcedf 100644 --- a/test/sealed-guard.test.ts +++ b/test/sealed-guard.test.ts @@ -46,9 +46,9 @@ function scratch(baseFiles: Record, objectFormat: 'sha1 const work = path.join(root, 'work'); const origin = path.join(root, 'origin.git'); const state = path.join(root, 'state'); fs.mkdirSync(work); fs.mkdirSync(state); const env = { HOME: root, GSTACK_HOME: state, GSTACK_STATE_ROOT: state, GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1' }; - fs.writeFileSync(env.GIT_CONFIG_GLOBAL, `[user]\n\tname = t\n\temail = t@example.com\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n\tdefaultObjectFormat = ${objectFormat}\n`); - spawnSync('git', ['init', '-q', '--bare', origin], { env: { ...process.env, ...env }, timeout: 30_000 }); - sh(work, env, ['init', '-q', '-b', 'main']); + fs.writeFileSync(env.GIT_CONFIG_GLOBAL, `[user]\n\tname = t\n\temail = t@example.com\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n`); + spawnSync('git', ['init', '-q', '--bare', `--object-format=${objectFormat}`, origin], { env: { ...process.env, ...env }, timeout: 30_000 }); + sh(work, env, ['init', '-q', '-b', 'main', `--object-format=${objectFormat}`]); write(work, { 'README.md': '# evals\n', ...baseFiles }); sh(work, env, ['add', '-A']); sh(work, env, ['commit', '-q', '-m', 'base']); sh(work, env, ['remote', 'add', 'origin', origin]); From 9a5474e4666d4b82163539604d25f086701d39e8 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 15:30:13 +0000 Subject: [PATCH 5/5] fix(sealed): ignored-waiver hint names the finding's commit to review, not a sha to write Choose the shortened-vs-another-commit wording by prefix against the finding's commit (fixes the SHA-256 40-hex mislabel and non-prefix short waivers), and check applied waivers with a Set instead of rescanning. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 +- docs/pregate.md | 4 ++-- lib/sealed-guard.ts | 9 ++++++--- test/sealed-guard.test.ts | 22 +++++++++++++++++++--- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b851c2c8b..6e66e077c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [Unreleased] -**A sealed-guard waiver now counts only when it names the full commit sha.** A `.gstack/sealed-allow.tsv` line used to accept a sha as short as 7 hex characters and waive any commit whose sha started with it. A PR author could generate a commit with the same first 7 characters in minutes and publish different content at the same path under an old, reviewed waiver. A waiver now matches only the finding's full commit sha (40 hex characters, 64 in a SHA-256 repository). A shortened, uppercase or mistyped sha is ignored, so the finding still refuses, and stderr says which line was ignored and which full sha to write. +**A sealed-guard waiver now counts only when it names the full commit sha.** A `.gstack/sealed-allow.tsv` line used to accept a sha as short as 7 hex characters and waive any commit whose sha started with it. A PR author could generate a commit with the same first 7 characters in minutes and publish different content at the same path under an old, reviewed waiver. A waiver now matches only the finding's full commit sha (40 hex characters, 64 in a SHA-256 repository). A shortened, uppercase or mistyped sha is ignored, so the finding still refuses, and stderr says which line was ignored and which commit the finding is in, to review before anyone waives it. ### Changed - `lib/sealed-guard.ts`: waivers compare the whole commit sha; `gstack-pregate sealed --json` adds `ignoredWaivers` (rule, path, the sha as written, the finding's commit) for waiver lines that name a finding's rule and path but not its commit, once per line and commit. Lines that match no finding, and waivers that applied to another commit in the push, stay silent. diff --git a/docs/pregate.md b/docs/pregate.md index a3427f145..6ad67bece 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -843,8 +843,8 @@ gstack-pregate sealed [--range ] [--remote origin] [--base ] [--js listed on every push. A waiver counts only when `sha` is the finding's full commit sha (40 hex characters, 64 in a SHA-256 repository); a shortened or mistyped one is ignored, so the finding still refuses, and stderr names it - (`ignored waiver sha (...)`) with the full commit to - write. + (`ignored waiver sha (...)`) with the finding's full + commit, which needs a review before anyone waives it. `--json` lists them as `ignoredWaivers`. - **Visibility.** `sealed.visibility` from the base branch, else a cache under the state root (`sealed-visibility.json`: a `public` answer for 24 hours, a diff --git a/lib/sealed-guard.ts b/lib/sealed-guard.ts index 6bc9272dc..976a44860 100644 --- a/lib/sealed-guard.ts +++ b/lib/sealed-guard.ts @@ -275,9 +275,10 @@ export function evaluateSealed(o: EvaluateOptions): SealedReport { if (entry) waived.push({ ...f, reason: entry.reason }); return !entry; }); + const applied = new Set(waived.map(w => `${w.rule}\t${w.file}\t${w.commit}`)); const ignored = new Map(); for (const f of findings) for (const a of allow) { - if (a.rule !== f.rule || a.path !== f.file || waived.some(w => w.commit === a.sha && w.rule === a.rule && w.file === a.path)) continue; + if (a.rule !== f.rule || a.path !== f.file || applied.has(`${a.rule}\t${a.path}\t${a.sha}`)) continue; ignored.set(`${a.rule}\t${a.path}\t${a.sha}\t${f.commit}`, { rule: a.rule, path: a.path, sha: a.sha, commit: f.commit }); } const ignoredWaivers = [...ignored.values()]; @@ -286,7 +287,6 @@ export function evaluateSealed(o: EvaluateOptions): SealedReport { return { status, policyLabels: labels, commits: commits.length, findings, exempt, waived, ignoredWaivers, visibility: vis.visibility, visibilitySource: vis.source, target: o.target }; } -const FULL_SHA = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; const where = (f: SealedFinding) => `${f.file || '(unknown file)'}${f.line ? `:${f.line}` : ''}`; /** stderr text for a report: findings one per line (` commit `), never the matched text. */ @@ -294,7 +294,10 @@ export function renderSealed(r: SealedReport, context: string): string { const lines: string[] = []; for (const e of r.exempt) lines.push(`${context}: exempt ${e.rule} ${where(e)} commit ${e.commit.slice(0, 12)} (sealed.allow_paths ${e.glob})`); for (const w of r.waived) lines.push(`${context}: waived ${w.rule} ${where(w)} commit ${w.commit.slice(0, 12)} (${SEALED_ALLOW_PATH}: ${w.reason})`); - for (const i of r.ignoredWaivers) lines.push(`${context}: ignored waiver ${i.rule} ${i.path} sha ${i.sha} (${FULL_SHA.test(i.sha) ? `it names another commit; this finding is in commit ${i.commit}` : `${SEALED_ALLOW_PATH} must name the full commit sha ${i.commit}`})`); + for (const i of r.ignoredWaivers) { + const why = i.commit.startsWith(i.sha.toLowerCase()) ? `${SEALED_ALLOW_PATH} must name the full lowercase commit sha` : 'it names another commit'; + lines.push(`${context}: ignored waiver ${i.rule} ${i.path} sha ${i.sha} (${why}; this finding is in commit ${i.commit}, review it before waiving)`); + } const vis = `visibility: ${r.visibility}${r.visibility === 'unknown' ? ', counted public' : ''}; ${r.visibilitySource}`; const body = r.findings.map(f => ` ${f.rule} ${where(f)} commit ${f.commit.slice(0, 12)}`); if (r.status === 'error') { diff --git a/test/sealed-guard.test.ts b/test/sealed-guard.test.ts index 3c6afcedf..1d159e69e 100644 --- a/test/sealed-guard.test.ts +++ b/test/sealed-guard.test.ts @@ -377,15 +377,17 @@ describe('waivers name the full commit sha (GST-268)', () => { const TRANSCRIPT = 'runs/c1/transcript.jsonl'; const line = (sha: string) => `sealed.transcript\t${TRANSCRIPT}\t${sha}\tsynthetic fixture\n`; - const SHORT: Array<[string, (x: string) => string]> = [['a 7-character prefix', x => x.slice(0, 7)], ['a 12-character prefix', x => x.slice(0, 12)], ['a 39-character prefix', x => x.slice(0, 39)], ['an uppercase sha', x => x.toUpperCase()], ['one extra character', x => `${x}a`]]; - test.each(SHORT)('%s does not waive and is named as ignored with the full commit', (_label, cut) => { + const PREFIX = 'must name the full lowercase commit sha'; + const OTHER = 'it names another commit'; + const SHORT: Array<[string, (x: string) => string, string]> = [['a 7-character prefix', x => x.slice(0, 7), PREFIX], ['a 12-character prefix', x => x.slice(0, 12), PREFIX], ['a 39-character prefix', x => x.slice(0, 39), PREFIX], ['an uppercase sha', x => x.toUpperCase(), PREFIX], ['one extra character', x => `${x}a`, OTHER]]; + test.each(SHORT)('%s does not waive and is named as ignored with the full commit', (_label, cut, why) => { const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); const sha = commit(s, { [TRANSCRIPT]: '{}\n' }); landOnMain(s, { '.gstack/sealed-allow.tsv': line(cut(sha)) }); const h = hook(s); expect(h.code).toBe(1); expect(h.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${cut(sha)}`); - expect(h.err).toContain(`full commit sha ${sha}`); + expect(h.err).toContain(`${why}; this finding is in commit ${sha}, review it before waiving)`); const pre = preflight(s); expect(pre.code).toBe(1); expect(pre.out).toMatch(/^SEALED: refused findings=1 .* waived=0 /m); @@ -415,6 +417,19 @@ describe('waivers name the full commit sha (GST-268)', () => { expect(r.err).toContain(`it names another commit; this finding is in commit ${second}`); }); + test("a short waiver for an older commit is named as another commit, never as this commit's shortened sha", () => { + const s = scratch({ '.gstack/pregate.json': policy(SEALED) }); + const first = commit(s, { [TRANSCRIPT]: '{}\n' }); + landOnMain(s, { '.gstack/sealed-allow.tsv': line(first.slice(0, 7)) }); + sh(s.work, s.env, ['reset', '-q', '--hard', 'origin/main']); + const second = commit(s, { [TRANSCRIPT]: '{"other":1}\n' }); + expect(second.startsWith(first.slice(0, 7))).toBe(false); + const r = hook(s); + expect(r.code).toBe(1); + expect(r.err).toContain(`ignored waiver sealed.transcript ${TRANSCRIPT} sha ${first.slice(0, 7)} (${OTHER}; this finding is in commit ${second}, review it before waiving)`); + expect(r.err).not.toContain(PREFIX); + }); + test('a short waiver line on the base branch prints nothing on a push with no matching finding', () => { const s = scratch({ '.gstack/pregate.json': policy(SEALED), '.gstack/sealed-allow.tsv': line('abc1234') }); commit(s, { 'notes.md': 'ok\n' }); @@ -432,6 +447,7 @@ describe('waivers name the full commit sha (GST-268)', () => { const pre = preflight(s); expect(pre.code).toBe(waives ? 0 : 1); expect(pre.err.includes('ignored waiver')).toBe(!waives); + if (!waives) expect(pre.err).toContain(`${cut(sha).length < 64 ? PREFIX : OTHER}; this finding is in commit ${sha}, review it before waiving)`); }); test('a waiver that applied in one commit is not reported as ignored for a later commit at the same path', () => {