From 5d2ba4f6f2b4e97f9ede69f6f1d9e4c747b7c81a Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 08:55:59 +0000 Subject: [PATCH] fix(review-ledger): judge binding compares real paths, so a run reached through a symlink verifies clean Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-ledger.ts | 5 ++++- test/review-dispatch.test.ts | 14 +++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index d9de23553..f6091f2b1 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -248,6 +248,9 @@ function unchangedOutput(o: LedgerEvent): string | undefined { const treeHolds = (render: LedgerEvent | undefined, reply: string | undefined) => !render || !('tree' in render) || (reply !== undefined && checkReviewedTree(reply, typeof render.tree === 'string' ? render.tree : null).ok); +/** One file however its path is spelled: collect and judge may reach a run directory through a symlink. */ +const realPath = (f: string) => { try { return fs.realpathSync(f); } catch { return path.resolve(f); } }; + /** * Whether a recorded output was a review: its unchanged reply names its render's tree, and it has findings or carries the reader's * clean marker, so a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own @@ -360,7 +363,7 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri if (missing.length) notes.push(`merged_ids_missing=${missing.join(',')}`); // The judged set must come from the merged file collect wrote, holding only rows collect recorded. const outputs = events.slice(0, lastJudge).filter(e => e.event === 'output' && Array.isArray(e.ids) && e.ids.length); - if (outputs.some(e => typeof e.merged !== 'string' || path.resolve(e.merged) !== path.resolve(String(v.merged)))) notes.push('judge_merged_not_collected'); + if (outputs.some(e => typeof e.merged !== 'string' || realPath(e.merged) !== realPath(String(v.merged)))) notes.push('judge_merged_not_collected'); let rerun: Judgment | null = null; try { const judgedText = fs.readFileSync(String(v.judged), 'utf8'); diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 3640a6909..6a86647c3 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -686,7 +686,8 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change // Value: protects=verify re-derives every merged row from the reader's unchanged output by its index, so a merged.jsonl edited // after collect cannot lower a finding or swap a blocker's text for a summary and still verify clean; fails_when=a forged label - // or a copied raw line on a real blocker's id verifies clean (GST-541: both did at main c830c9b); why_new=verify compared only + // or a copied raw line on a real blocker's id verifies clean (GST-541: both did at main c830c9b), or a clean run collected through a + // symlinked run directory and judged there with relative paths verifies dirty (GST-541 ship2-p11); why_new=verify compared only // output hashes and line counts; seam=verify; no_claim=that an output collected without a recorded file can be re-derived test('verify re-derives each merged row by index: a forged label or a summary copied onto a blocker id is dirty', () => { const blocker = '{"severity":"CRITICAL","path":"log.ts","line":3,"category":"security","summary":"bearer token written to a public log","specialist":"testing"}'; @@ -724,6 +725,17 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect(run(['judge', '--ledger', copy.ledger, '--merged', forgedCopy, '--judged', copy.w('judged.jsonl', readJsonl(copy.merged).map(m => JSON.stringify({ id: m.id, severity: 'INFORMATIONAL' })).join('\n') + '\n')]).status).toBe(0); const viaCopy = run(['verify', '--ledger', copy.ledger]); expect([viaCopy.status, viaCopy.stdout]).toEqual([1, expect.stringMatching(/^JUDGE - MISMATCH .*judge_merged_not_collected/m)]); + const relinked = pass(); + const linkDir = path.join(tmp(), 'run'); + fs.symlinkSync(relinked.out, linkDir); + const ledgerViaLink = path.join(linkDir, 'review-ledger.jsonl'); + run(['sent', '--ledger', ledgerViaLink, '--role', 'specialist', '--name', 'testing', '--sent', relinked.file]); + const linkOut = relinked.w('spec.out', blocker + '\n' + summary + '\n'); + expect(run(['collect', '--ledger', ledgerViaLink, '--role', 'specialist', '--name', 'testing', '--output', path.join(linkDir, path.basename(linkOut)), '--merged', path.join(linkDir, 'merged.jsonl')]).status).toBe(0); + relinked.w('judged.jsonl', readJsonl(relinked.merged).map(m => JSON.stringify({ id: m.id, severity: m.label })).join('\n') + '\n'); + expect(run(['judge', '--ledger', 'review-ledger.jsonl', '--merged', 'merged.jsonl', '--judged', 'judged.jsonl'], linkDir).status).toBe(0); + const linked = run(['verify', '--ledger', ledgerViaLink]); + expect([linked.status, linked.stdout.includes('judge_merged_not_collected'), linked.stdout.includes('VERIFY: clean')]).toEqual([0, false, true]); const quiet = pass(); run(['sent', '--ledger', quiet.ledger, '--role', 'specialist', '--name', 'testing', '--sent', quiet.file]); const none = quiet.w('none.out', 'NO FINDINGS\n');