From ab54cbb5e13a55a3da98dc2fec2bd12a8c2b794d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 4 Nov 2026 19:37:28 -0800 Subject: [PATCH] fix(review): lead judgment may lower a native finding to the Severity it stated beside Blocks: no (GST-1406) (#4017) * fix(review): lead judgment may lower a native finding to the Severity it stated beside Blocks: no (GST-1406) Collect keeps the FIXABLE raise on a native adversarial finding whose Severity and Blocks: no follow prose, because it can't tell a finding's own fields from another subject's. judge() and ledgerBlockers() now let lead judgment lower such a finding to its one stated positive-rank Severity, never below it, only with a reason, recorded as dismissed. Carried from gst-1406-fixable-blocks-no (daf4fc2) without the GST-1409 fence pre-scan. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes verify checks each merged row's reader against the output it came from, and ledgerBlockers takes the reader from the output event, so a row relabelled adversarial-native can't take the native lowering and replay clean. The JUDGMENT_REJECTED hint names the native route; the fragment says the blocker count is unchanged rather than that it follows. Review-Finding: v1/outside-voice#1 reader=outside-voice Review-Finding: v1/adversarial-native#2 reader=adversarial-native Review-Finding: v1/adversarial-native#4 reader=adversarial-native Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore: name the GST-1406 fragment after its branch Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-dispatch | 2 +- changelog.d/gst-1406-judge-route.md | 3 ++ docs/review-policy.md | 5 ++- lib/review-ledger.ts | 15 ++++++--- test/review-gst1031.test.ts | 49 +++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 changelog.d/gst-1406-judge-route.md diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index b1c20a026..2cc9fa31d 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -495,7 +495,7 @@ function main(): number { if (nothing && r.ok) { process.stdout.write(`JUDGMENT: ok nothing to judge (${merged} holds no findings; wrote an empty ${judged})\n`); return EXIT.ok; } if (nothing) fs.rmSync(judged, { force: true }); process.stdout.write(r.ok ? `JUDGMENT: ok added=${r.added.length} promoted=${r.promoted.length} folded=${r.folded.length}${r.dismissed.length ? ` dismissed=${r.dismissed.length}` : ''}\n` - : `JUDGMENT_REJECTED: dropped=${JSON.stringify(r.dropped)} downgraded=${JSON.stringify(r.downgraded)} unexplained=${JSON.stringify(r.unexplained)} invalid=${JSON.stringify(r.invalid)} (lead judgment may add or promote a finding, never drop or downgrade one; a summary_candidate line may be lowered with a reason)\n`); + : `JUDGMENT_REJECTED: dropped=${JSON.stringify(r.dropped)} downgraded=${JSON.stringify(r.downgraded)} unexplained=${JSON.stringify(r.unexplained)} invalid=${JSON.stringify(r.invalid)} (lead judgment may add or promote a finding, never drop or downgrade one; a summary_candidate line may be lowered with a reason, and so may a native adversarial line stating Blocks: no, to its one stated Severity and no lower)\n`); return r.ok ? EXIT.ok : EXIT.fail; } if (cmd === 'verify') { diff --git a/changelog.d/gst-1406-judge-route.md b/changelog.d/gst-1406-judge-route.md new file mode 100644 index 000000000..5df44d08b --- /dev/null +++ b/changelog.d/gst-1406-judge-route.md @@ -0,0 +1,3 @@ +### Fixed + +- **Lead judgment can lower a native adversarial finding to the Severity its reader stated beside `Blocks: no` (GST-1406).** When the native reader wrote its `Severity` and `Blocks: no` after prose (`… so the docs are wrong. Severity: Medium. Blocks: no.`), collect kept the heading's FIXABLE as CRITICAL. Judge refused any lowering, so the native record never resolved and /ship's Step 11.5 couldn't bind. Collect still can't tell those fields from another subject's written under the same heading, so it keeps the label. Now `gstack-review-dispatch judge` lets lead judgment lower such a finding to its stated Severity, never below, with a reason. It's recorded as `dismissed`. The ledger's blocker count already left such a finding out (GST-1031) and still does, and `verify` now also checks each merged row's reader against the output it came from, so a row relabelled as native fails replay. Other readers, a finding that states `Blocks: yes`, no `Blocks` or a Severity that isn't a severity (such as `Severity: FIXABLE`), and a lowering below the stated Severity or with no reason are still refused as downgrades. See docs/review-policy.md. diff --git a/docs/review-policy.md b/docs/review-policy.md index 880187dce..00d941011 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -757,7 +757,10 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: found nothing CRITICAL") is a known phantom: it collects at that label. When it is a summary candidate (no lead label, no `Severity` or `Blocks` field, and no file:line or exception clause in any sentence naming a label: "so nothing is CRITICAL", "the test covers the CRITICAL fix"), collect marks it `summary_candidate` and lead judgment may lower it - with a reason, which judge records as `dismissed` (GST-1031); otherwise lead judgment can fold it only into a finding + with a reason, which judge records as `dismissed` (GST-1031). A native adversarial finding whose own text states + `Blocks: no` and one `Severity` field may also be lowered with a reason to that Severity, never below it, and is + recorded as `dismissed` too (GST-1406): collect keeps the FIXABLE raise when those fields follow prose, because it + can't tell a finding's own fields from another subject's. Otherwise lead judgment can fold it only into a finding at least as severe, and it counts toward what blocks. GST-766 skipped negated labels and lost real blockers named in an exception clause ("No CRITICAL issues except the token leak in log.ts:3"); GSTA-167 reverted that. Each prose finding's line also diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index bea5628c0..d010c8fb1 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -86,6 +86,11 @@ const LEAD_LABEL = new RegExp(`^(?:[^\\p{L}\\p{N}\\s]\\s*|\\d+[.)]\\s*)*(?:\\w[\ const STATED_BLOCKS = /(? { + const ls = [...raw.matchAll(SEVERITY_FIELD)].flatMap(m => (m[1] ?? m[2]!).match(FIELD_LABELS)!); + return reader.split('/')[0] === 'adversarial-native' && statesNoBlocks(raw) && ls.length === 1 && to >= (rank(ls[0]!) || Infinity); +}; export const statesNoBlocks = (raw: string) => { if (raw.startsWith('{') || BLOCKS_YES.test(raw)) return false; const said = [...raw.matchAll(BLOCKS_FIELD)].map(m => m[1]!.toLowerCase()); return said.length > 0 && !said.includes('yes'); }; /** * A summary candidate (GST-1031): a prose finding whose every severity word is denied ("so nothing is CRITICAL", "No CRITICAL issues") @@ -339,7 +344,7 @@ export function judge(merged: MergedLine[], judged: JudgedLine[]): Judgment { const from = rank(m.label) ?? 0; const to = rank(target.severity); if (to === null) continue; - if (to < from) (!j.duplicate_of && j.reason?.trim() && summaryCandidate(m.raw) ? res.dismissed : res.downgraded).push(m.id); + if (to < from) (!j.duplicate_of && j.reason?.trim() && (summaryCandidate(m.raw) || statedLowering(m.reader, m.raw, to)) ? res.dismissed : res.downgraded).push(m.id); else if (j.duplicate_of) res.folded.push(m.id); else if (to > from) { res.promoted.push(m.id); if (!j.reason?.trim()) res.unexplained.push(m.id); } } @@ -408,6 +413,7 @@ export function ledgerBlockers(ledger: string): LedgerBlocker[] { const ranks = new Map(); const unblocking = new Map(); const raws = new Map(); + const readers = new Map(); const loci = new Map(); for (const o of events.filter(e => e.event === 'output' && Array.isArray(e.ids) && e.ids.length)) { const lines = jsonLines(read(o.merged, `merged file of ${String(o.reader)}`), String(o.merged)) as MergedLine[]; @@ -416,6 +422,7 @@ export function ledgerBlockers(ledger: string): LedgerBlocker[] { ranks.set(id, rank(line.label) ?? 0); if (statesNoBlocks(line.raw)) unblocking.set(id, rank(line.label) ?? 0); raws.set(id, line.raw); + readers.set(id, String(o.reader)); loci.set(id, lineLocus(line.raw)); } } @@ -425,7 +432,7 @@ export function ledgerBlockers(ledger: string): LedgerBlocker[] { if (promptHash(text) !== verdict.judged_sha256) fail(`judged set ${String(verdict.judged)} changed since judge`); for (const j of jsonLines(text, String(verdict.judged)) as JudgedLine[]) { if (j.duplicate_of) { ranks.delete(j.id); continue; } - const dismissed = !!j.reason?.trim() && summaryCandidate(raws.get(j.id) ?? ''); + const dismissed = !!j.reason?.trim() && (summaryCandidate(raws.get(j.id) ?? '') || statedLowering(readers.get(j.id) ?? '', raws.get(j.id) ?? '', rank(j.severity) ?? 0)); ranks.set(j.id, dismissed ? rank(j.severity) ?? 0 : Math.max(ranks.get(j.id) ?? 0, rank(j.severity) ?? 0)); if (!loci.has(j.id)) loci.set(j.id, lineLocus(j.reason ?? '')); } @@ -510,14 +517,14 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri try { lines = typeof o.merged === 'string' ? readJsonl(o.merged).filter(l => ids.includes(l.id)) : []; } catch { notes.push('merged_unreadable'); status = 'MISMATCH'; } if (lines.length !== ids.length) { status = 'MISMATCH'; notes.push(`merged_lines=${lines.length}/${ids.length}`); } if (lines.some(l => l.output_sha256 !== o.sha256)) { status = 'MISMATCH'; notes.push('merged_line_from_other_output'); } - // Each merged row must be the finding collect parses at that id's index from the unchanged output, label and raw text byte for byte. + // Each merged row must be the finding collect parses at that id's index from the unchanged output, its reader, label and raw text byte for byte. if (text === undefined) { status = 'MISMATCH'; notes.push(typeof o.file === 'string' ? 'output_file_missing' : 'output_file_unrecorded'); differs.push(...ids); } else if (promptHash(text) === o.sha256) { const parsed = parseFindings(reader, text, o.fixable_raises !== false); - const off = ids.filter((id, k) => { const m = lines.find(l => l.id === id); return !m || parsed[k]?.label !== m.label || parsed[k]?.raw !== m.raw; }); + const off = ids.filter((id, k) => { const m = lines.find(l => l.id === id); return !m || m.reader !== reader || parsed[k]?.label !== m.label || parsed[k]?.raw !== m.raw; }); if (unavailableOutput(o) && parsed.length) { status = 'MISMATCH'; notes.push('unavailable_output_has_findings'); } else if (!unavailableOutput(o)) { if (off.length) { status = 'MISMATCH'; notes.push(`merged_line_differs=${JSON.stringify(off)}`); differs.push(...off); } diff --git a/test/review-gst1031.test.ts b/test/review-gst1031.test.ts index c8a4b76f0..7c8cfda05 100644 --- a/test/review-gst1031.test.ts +++ b/test/review-gst1031.test.ts @@ -88,6 +88,55 @@ describe('a closing summary is a candidate lead judgment may dismiss on record ( }); }); +describe('lead judgment may lower a native finding to the Severity its reader stated beside Blocks: no (GST-1031 ruling A, GST-1406)', () => { + // Value: protects=a native finding whose reader wrote its Severity and Blocks: no after prose (so collect keeps the FIXABLE raise) + // can be lowered by lead judgment to that Severity, on record with a reason, so the native record can bind; fails_when=that finding + // is an unlowerable CRITICAL (GSTA-275 GST-1276 v4 stopped Step 11.5), or the route lowers a non-native finding, one stating Blocks: + // yes or no Blocks, one below its stated Severity, or one with no reason; why_new=collect cannot tell a heading's own fields from + // another subject's (GST-1406 red team), lead judgment can; seam=judge, ledgerBlockers; no_claim=that a lowering is right (it is on record) + const OUT = ['**1. Title (FIXABLE).** body in lib/x.ts:', '- a', '- b', '', 'Before this diff it stopped. Fix: refuse it. Severity: Medium. Blocks: no, because a reader still reads it.', ''].join('\n'); + const run = (reader: string, output: string, severity: string, reason?: string) => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'gst1406-')); + const ledger = path.join(d, 'l.jsonl'); + const merged = path.join(d, 'm.jsonl'); + fs.writeFileSync(path.join(d, 'out.txt'), output); + const c = collect(ledger, reader, output, merged, path.join(d, 'out.txt')); + fs.writeFileSync(path.join(d, 'j.jsonl'), c.lines.map(l => JSON.stringify({ id: l.id, severity, ...(reason ? { reason } : {}) })).join('\n') + '\n'); + const r = recordJudgment(ledger, c.lines, readJsonl(path.join(d, 'j.jsonl')), path.join(d, 'j.jsonl'), merged); + const out = { labels: c.lines.map(l => l.label), ok: r.ok, dismissed: r.dismissed.length, blockers: ledgerBlockers(ledger).length }; + fs.rmSync(d, { recursive: true, force: true }); + return out; + }; + test('lowered to its stated Severity with a reason, it is dismissed on record and leaves the blocker count', () => { + expect(run('adversarial-native', OUT, 'Medium', 'its reader stated Severity: Medium, Blocks: no')).toEqual({ labels: ['CRITICAL'], ok: true, dismissed: 1, blockers: 0 }); + }); + test('negative controls: no reason, below the stated Severity, Blocks: yes, no Blocks, or a non-native reader stay downgrades', () => { + expect(run('adversarial-native', OUT, 'Medium')).toMatchObject({ ok: false, dismissed: 0 }); + expect(run('adversarial-native', OUT, 'Low', 'r')).toMatchObject({ ok: false, dismissed: 0 }); + expect(run('adversarial-native', OUT.replace('Blocks: no', 'Blocks: yes'), 'Medium', 'r')).toMatchObject({ ok: false, dismissed: 0 }); + expect(run('adversarial-native', OUT.replace(' Blocks: no, because a reader still reads it.', ''), 'Medium', 'r')).toMatchObject({ ok: false, dismissed: 0 }); + expect(run('red-team', '**1. Auth bypass lets any user read admin data (CRITICAL).**\n\nThe earlier triage filed it lower. Severity: Low. Blocks: no.\n', 'Low', 'r')).toMatchObject({ ok: false, dismissed: 0 }); + expect(run('adversarial-native', OUT.replace('Severity: Medium', 'Severity: FIXABLE'), 'Low', 'r')).toMatchObject({ ok: false, dismissed: 0 }); + }); + test('a merged row whose reader was edited to adversarial-native is caught by verify', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'gst1406r-')); + const ledger = path.join(d, 'l.jsonl'); + const merged = path.join(d, 'm.jsonl'); + const output = 'CRITICAL: admin.ts:9 skips authentication. Severity: Low. Blocks: no.\n'; + fs.writeFileSync(path.join(d, 'out.txt'), output); + const c = collect(ledger, 'outside-voice', output, merged, path.join(d, 'out.txt')); + const forged = c.lines.map(l => ({ ...l, reader: 'adversarial-native' })); + fs.writeFileSync(merged, forged.map(l => JSON.stringify(l)).join('\n') + '\n'); + fs.writeFileSync(path.join(d, 'j.jsonl'), forged.map(l => JSON.stringify({ id: l.id, severity: 'Low', reason: 'r' })).join('\n') + '\n'); + recordJudgment(ledger, forged, readJsonl(path.join(d, 'j.jsonl')), path.join(d, 'j.jsonl'), merged); + const v = verify(ledger); + expect(v.clean).toBe(false); + expect(v.rows.find(r => r.kind === 'OUTPUT')!.detail).toContain('merged_line_differs=["outside-voice#1"]'); + expect(v.rows.find(r => r.kind === 'JUDGE')!.status).toBe('MISMATCH'); + fs.rmSync(d, { recursive: true, force: true }); + }); +}); + describe('the blocker count keys on Blocks (GST-1031 case 3)', () => { // Value: protects=a CRITICAL its reader marked Blocks: no is not counted as a blocker by the ledger count, the row count or Step // 9.4's classify, while Blocks: yes, an unstated Blocks and a lead promotion still block; fails_when=REVIEW_LOOP reports a