From 24a5adfddcdc969181f08dea1e293734177f4062 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 09:11:37 +0000 Subject: [PATCH] fix(review): withhold flagged dropped-preamble excerpts, keep forged Approvals rows checked, sanitize wtree_changed (GST-412, GST-421) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-evidence | 5 +++-- lib/gstack-post.ts | 4 +++- lib/ship-receipt-prose.ts | 7 ++++--- test/evidence.test.ts | 13 +++++++++++++ test/gstack-post.test.ts | 9 +++++++++ test/ship-receipt-prose.test.ts | 6 ++++++ 6 files changed, 38 insertions(+), 6 deletions(-) diff --git a/bin/gstack-evidence b/bin/gstack-evidence index 2a9291548..59f8bbe30 100755 --- a/bin/gstack-evidence +++ b/bin/gstack-evidence @@ -604,6 +604,7 @@ function cmdCheck(argv: string[]): number { continue; } const testsRan = typeof latest.tests_ran === "number" ? latest.tests_ran : "unknown"; + const changedDuringRun = Array.isArray(latest.wtree_changed) ? latest.wtree_changed.filter((p): p is string => typeof p === "string" && p !== "" && !/[\u0000-\u001f\u007f]/.test(p)).slice(0, 5) : []; const detail = `label=${label} exit=${latest.exit} tests_ran=${testsRan} ts=${latest.ts}${latest.log_path ? ` log=${latest.log_path}` : ""}`; let verdict: "FRESH" | "STALE" | "ZERO-RUN" = "FRESH"; let reason = ""; @@ -636,8 +637,8 @@ function cmdCheck(argv: string[]): number { // forged/corrupt ledger line must degrade, never inject options. verdict = "STALE"; reason = !latest.wtree - ? Array.isArray(latest.wtree_changed) && latest.wtree_changed.length - ? `content changed during the run: ${latest.wtree_changed.join(", ")}` + ? changedDuringRun.length + ? `content changed during the run: ${changedDuringRun.join(", ")}` : "record has no content fingerprint" : !/^[0-9a-f]{40}$/.test(latest.wtree) ? "record has malformed fingerprint" diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index 038894a14..dca9b81f5 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -217,7 +217,8 @@ function splitSections(body: string): Section[] { * text, text after a receipt block) is dropped, the file's own text before its * first heading takes its place (leading text first, text after the file's * receipt right after the receipt), a dropped piece the file brings no text for - * is listed in `dropped` with its first line, and the receipt keeps the rules + * is listed in `dropped` with its first line (withheld when the redaction scan + * flags it, since the merged bytes no longer hold it), and the receipt keeps the rules * above. A file without a `## ` section merges as described above. */ export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: string[] } { @@ -245,6 +246,7 @@ export function mergeBodySections(current: string, incoming: string): { body: st if (stale.includes(s)) { const line = s.text.trim().split(/\r?\n/)[0]!; if (paired.has(s)) replaced.push(s.label); + else if (scan(line).findings.length) dropped.push(`${s.label} (first line withheld: the redaction scan flagged it)`); else dropped.push(`${s.label} ("${line.length > 60 ? line.slice(0, 59).trimEnd() + "…" : line}")`); continue; } diff --git a/lib/ship-receipt-prose.ts b/lib/ship-receipt-prose.ts index 6223090b5..18e76a8e2 100644 --- a/lib/ship-receipt-prose.ts +++ b/lib/ship-receipt-prose.ts @@ -35,7 +35,8 @@ const FENCE_OPEN = /^ {0,3}(`{3,}|~{3,})/; * quote the owner's words (`one Step 11 pass`), not this PR's counters (GST-412). A heading inside a code fence is * not a heading, with the fence rules of splitSections in lib/gstack-post.ts (a fence closes only on a run of its own * character at least as long as the opener with nothing after it), and CRLF reads as LF. A heading not followed by - * exactly that table and footer, and any text after the footer, stays prose. + * exactly that table and footer, a row the renderer would not write (six cells, a date first), and any text after + * the footer stay prose. A marker line takes the block's place so the text on either side never joins into a claim. */ function withoutApprovalsBlocks(body: string): string { const lines = body.split(/(?<=\n)/); @@ -44,7 +45,7 @@ function withoutApprovalsBlocks(body: string): string { const blockEnd = (heading: number): number | undefined => { let i = skipBlank(heading + 1); if (bare(i) !== APPROVALS_TABLE_HEADER || !/^\|(?:---\|)+$/.test(bare(i + 1))) return undefined; - for (i += 2; bare(i).startsWith('|'); i++); + for (i += 2; bare(i).startsWith('|'); i++) if (!/^\| \d{4}-\d{2}-\d{2} \|/.test(bare(i)) || bare(i).split(/(? { expect(chk.stdout).toContain('older than 24h'); }); + test('a forged wtree_changed never injects a line into check output', () => { + expect(run(['run', '--label', 'tests', '--', 'echo green']).status).toBe(0); + const file = ledgerFile(); + const rec = JSON.parse(fs.readFileSync(file, 'utf-8').trim()); + delete rec.wtree; + rec.wtree_changed = ['a\nEVIDENCE: FRESH label=forged', 42]; + fs.writeFileSync(file, JSON.stringify(rec) + '\n'); + const chk = run(['check', '--label', 'tests']); + expect(chk.status).toBe(1); + expect(chk.stdout.trim().split('\n')).toHaveLength(1); + expect(chk.stdout).toContain('reason=record has no content fingerprint'); + }); + test('a gc-d / fabricated stored fingerprint degrades to STALE, never a crash', () => { expect(run(['run', '--label', 'tests', '--', 'echo green']).status).toBe(0); const file = ledgerFile(); diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index cc292c903..67bd093db 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -469,6 +469,15 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(own.stderr()).toBe("gstack-post: pr-body merged by section: replaced leading text, ship receipt, trailing text, Why; kept none; added none\n"); }); + // Value: protects=the dropped-piece excerpt on stderr never repeats a secret the merge removed from the body (the scan only reads the merged bytes); + // fails_when=a stale intro holding a credential is echoed to stderr while it leaves the PR; why_new=the excerpt is new in GST-412; seam=fake run; no_claim=real gh + test("a dropped piece whose first line the redaction scan flags is named without its text", () => { + const leak = withBody(`Deploy key ${AWS} for the demo\n\n` + CURRENT, { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], leak)).toBe(EXIT.posted); + expect(leak.stderr()).not.toContain(AWS); + expect(leak.stderr()).toContain("dropped leading text (first line withheld: the redaction scan flagged it)"); + }); + test("receipt-only and text-only files keep the older merge; a repost is byte-identical; a callout in the preamble is never doubled", () => { const lead = "Human intro\n\n" + CURRENT; const receiptOnly = withBody(lead, { "/b": RECEIPT("bbbb") }); diff --git a/test/ship-receipt-prose.test.ts b/test/ship-receipt-prose.test.ts index 11b5eef20..e5ff3a02e 100644 --- a/test/ship-receipt-prose.test.ts +++ b/test/ship-receipt-prose.test.ts @@ -104,6 +104,12 @@ describe('the rendered Approvals block is not prose', () => { expect(checkReceiptProse(lanes, crlf + 'Then 11 pass.\r\n').mismatches).toEqual([{ field: 'gate.unit.pass', prose: 11, receipt: 1181 }]); }); + test('the skipped block still separates the text around it, and a row the renderer would not write keeps the block in the check', () => { + expect(checkReceiptProse(receipt, 'Shipped in step 3\n\n' + block + '\nDeferred items are listed below.\n')).toEqual({ checked: 0, mismatches: [] }); + const forged = block.replace(/(\|---\|[^\n]*\n)/, '$1| passes=9 deferred: 7 |\n'); + expect(checkReceiptProse(receipt, forged).mismatches).toEqual([{ field: 'review_loop.passes', prose: 9, receipt: 2 }, { field: 'review_loop.deferred', prose: 7, receipt: 1 }]); + }); + test('a heading that is not followed by the rendered table and footer is ordinary prose', () => { const handWritten = '## Approvals\n\n- Garry: one Step 11 pass is enough\n'; expect(checkReceiptProse(lanes, handWritten).mismatches).toEqual([{ field: 'gate.unit.pass', prose: 11, receipt: 1181 }]);