From 96e7bd43f0513a4d2fd8bc64d84058dc6fa1effd Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 16:06:42 +0000 Subject: [PATCH] fix(ship): a negated subject drops a label only when it names the review's findings (GST-435 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-435-collect-labels.md | 6 +++--- docs/review-policy.md | 6 ++++-- lib/review-ledger.ts | 7 ++++--- test/review-dispatch.test.ts | 5 ++++- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/changelog.d/gst-435-collect-labels.md b/changelog.d/gst-435-collect-labels.md index df39bd8c8..1d2d8494d 100644 --- a/changelog.d/gst-435-collect-labels.md +++ b/changelog.d/gst-435-collect-labels.md @@ -1,9 +1,9 @@ -**/ship's review collection no longer turns a reviewer's "no CRITICAL" summary into a CRITICAL finding.** In the HARN-15 runs (harness-evals #152 and the ship2 pass) the Red Team and fix-check readers opened with lines like "I found three INFORMATIONAL issues and no CRITICAL ones" and "there's nothing at CRITICAL". `collect` stored each of those lines as a CRITICAL finding, and `judge` would not let the lead drop it, so a clean pass read as blocked. The same happened to the fix-check closing line "Neither finding is CRITICAL." on gstack-evals#94. What reviewers see is unchanged: every brief renders byte for byte as before. +**/ship's review collection no longer turns a reviewer's "no CRITICAL" summary into a CRITICAL finding.** In the HARN-15 runs (harness-evals #152 and the ship2 pass) the Red Team and fix-check readers opened with lines like "I found three INFORMATIONAL issues and no CRITICAL ones" and "there's nothing at CRITICAL". `collect` stored each of those lines as a CRITICAL finding, and `judge` would not let the lead drop it, so a clean pass read as blocked. The same happened to the fix-check closing line "Neither finding is CRITICAL." on gstack-evals#94. What reviewers see is unchanged, and every brief renders byte for byte as before. ### Fixed -- **A severity word that a negator directly scopes is no label.** "not CRITICAL", "isn't really High", "no CRITICAL or High findings", "found nothing CRITICAL", "nothing at CRITICAL", "Neither finding is CRITICAL", "none of which is High" and "below High" no longer count. The negation has to apply to the label itself: "Nothing guards the write path the failure of which is CRITICAL" and "None of the writes are atomic and are CRITICAL (queue.ts:78); High confidence" still collect as CRITICAL. A table row is never read for negation. A kept finding never takes a weaker label than the strongest one left on its line, so the table row "| 1 | `CRITICAL` | High | token leak in log.ts:3 |" stays CRITICAL. The native adversarial voice's FIXABLE still collects as CRITICAL. +- **A severity word that a negator directly scopes is no label.** "not CRITICAL", "isn't really High", "no CRITICAL or High findings", "found nothing CRITICAL", "nothing at CRITICAL", "Neither finding is CRITICAL", "none of which is High" and "below High" no longer count. The negation has to apply to the label itself, and a negated subject counts only when it names the review's own findings ("Neither finding", "Nothing here", "none of which"): "No input validation is CRITICAL (upload.ts:4)", "Nothing guards the write path the failure of which is CRITICAL" and "None of the writes are atomic and are CRITICAL (queue.ts:78); High confidence" still collect as CRITICAL. A table row is never read for negation. A kept finding never takes a weaker label than the strongest one left on its line, so the table row "| 1 | `CRITICAL` | High | token leak in log.ts:3 |" stays CRITICAL. The native adversarial voice's FIXABLE still collects as CRITICAL. - **A list item whose label first appears in an indented later paragraph collects as that item.** The ship2 native voice wrote "4. **The host lock is not atomic** (`qm-stack.ts:66-76`)", two detail bullets, and then an indented "Fix: … FIXABLE. Severity: Low." paragraph. The finding was stored as the Fix paragraph alone, without the line that says what it is. It now starts with the item's first line. A labeled item's own labeled later paragraph is still its own finding. -- **No blocker is lost.** Main's parser and this one were run over 2,250 recorded reader outputs: 25 raw outputs from the GSTA-189 and GSTA-196 replays, the 2,172 finding records from arms C6, L6, A3668 (both suites), G1 and L, and 53 HARN-15 and test-fixture outputs. None of the 260 records matched to a judged blocker changes. In the arms, two INVESTIGATE findings, each recorded in both arm G1 and arm L, lose a phantom label: one a CRITICAL from "no CRITICAL" inside its sentence, the other a High from "resolves below High". Every other change is a HARN-15 or GEVA-77 summary line that drops from CRITICAL to INFORMATIONAL or to nothing, or a native finding that gains its title. +- **No blocker is lost.** Main's parser and this one were run over 2,250 recorded reader outputs: 25 raw outputs from the GSTA-189 and GSTA-196 replays, the 2,172 finding records from arms C6, L6, A3668 (both suites), G1 and L, and 53 HARN-15 and test-fixture outputs. None of the 260 records matched to a judged blocker changes. In the arms, two INVESTIGATE findings, each recorded in both arm G1 and arm L, lose a phantom label. One is a CRITICAL from "no CRITICAL" inside its sentence, and the other is a High from "resolves below High". Every other change is a HARN-15 or GEVA-77 summary line that drops from CRITICAL to INFORMATIONAL or to nothing, or a native finding that gains its title. The replay ran before review narrowed the negated-subject form, and that change only keeps more labels. ### Docs - `docs/review-policy.md` (collect) describes the negation rule and the later-paragraph rule. diff --git a/docs/review-policy.md b/docs/review-policy.md index 452d480ac..5d2721964 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -470,8 +470,10 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: wherever they stand, headings and table rows included; a top-level list item under a label-only heading takes the stronger of the two). A label a negator directly scopes is no label: "not CRITICAL", "no CRITICAL or High findings", "found nothing CRITICAL", - "Neither finding is CRITICAL", "below High" (GST-435). A negation of something else - ("Nothing guards the write path the failure of which is CRITICAL") leaves the label, a + "Neither finding is CRITICAL", "below High" (GST-435). A negated subject counts only when it + names the review's own findings ("Neither finding", "Nothing here", "none of which"). A negation of + something else ("No input validation is CRITICAL", "Nothing guards the write path the failure + of which is CRITICAL") leaves the label, a table row is never read for negation, and a kept finding never takes a weaker label than the strongest one left on its line. A `Severity:` or `Blocks:` line continues the finding above it until a list item, heading, bold paragraph opener or a diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 3029e3ccc..1f80aa388 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -62,12 +62,13 @@ const ALL_LABELS = new RegExp(ANY_LABEL.source, 'g'); /** * GST-435 rule (b): a severity word is no label when a negator directly scopes it: "not CRITICAL", "isn't really High", "no CRITICAL * or High findings", "found nothing CRITICAL", "nothing at CRITICAL", "below CRITICAL", "Neither finding is CRITICAL", "none of which is - * High". A negation of something else ("Nothing guards the write path the failure of which is CRITICAL", "None of the writes are atomic - * and are CRITICAL") leaves the word a label, and a table row is never read for negation. + * High". A negated subject scopes the label only when it names the review's own findings ("Neither finding", "Nothing here", "none of + * which"). A negation of something else ("No input validation is CRITICAL (upload.ts:4)", "Nothing guards the write path the failure of + * which is CRITICAL", "None of the writes are atomic and are CRITICAL") leaves the word a label, and a table row is never read for negation. */ const QUOTE = '[`*"\'\u201c\u201d\u2018\u2019]*'; const FILLER = '(?:\\s+(?:really|actually|quite|even|yet|a|an|any|at|as|be|rise|rises|to|reach|reaches|meet|meets|count|counts|qualify|qualifies|rated|marked|called|flagged))'; -const SUBJECT_WORD = "(?!(?:which|that|who|whom|whose|so|and|but|or|nor|because|since|while|if|when|where|then|than|is|are|was|were)\\b)[A-Za-z][\\w'\u2019-]*"; +const SUBJECT_WORD = '(?:the|two|three|four|both|other|remaining|above|listed|reported|new|these|those|them|it|here|else|left|finding|findings|issue|issues|defect|defects|problem|problems|bug|bugs|concern|concerns|item|items|nit|nits|one|ones|note|notes)\\b'; const DIRECT = `(?:\\b(?:not|never|no|nothing|none|neither|nor|cannot|below|beneath|lower\\s+than|less\\s+than|short\\s+of)|n['\u2019]t)${FILLER}{0,3}`; const SUBJECT = `\\b(?:no|none|neither|nothing)(?:\\s+of\\s+(?:the|these|those|them|which|it|this|its|my|our|their))?(?:\\s+${SUBJECT_WORD}){0,3}\\s+(?:is|are|was|were|be|seems?|looks?|reads?\\s+as|counts?\\s+as|rises?\\s+to|ranks?\\s+as)${FILLER}{0,2}`; const NEGATED_BEFORE = new RegExp(`(?:${DIRECT}|${SUBJECT})\\s+(?:${QUOTE}(?:${LABELS})${QUOTE}\\s*(?:,|/|\\bor\\b|\\bnor\\b|\\band\\b)\\s*)*${QUOTE}$`, 'i'); diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 898711076..b474faedb 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -466,12 +466,15 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change ['Nothing checks the token which is CRITICAL (a.ts:3)', 'CRITICAL'], ['No bounds check lets CRITICAL data leak (a.ts:3)', 'CRITICAL'], ['No defects in parsing but the upload handler is CRITICAL.', 'CRITICAL'], ['1. Without auth this is CRITICAL (a.ts:3)', 'CRITICAL'], ['Not checking expiry at auth.ts:9 lets stale tokens through. Severity: High', 'High'], ["The lock isn't released on error (pool.ts:12): High", 'High'], - ['A retry storm could escalate up to CRITICAL.', 'CRITICAL'], ['Low: the parser now maps `FIXABLE` to `CRITICAL` in a comment (a.ts:3)', 'CRITICAL']]; + ['A retry storm could escalate up to CRITICAL.', 'CRITICAL'], ['Low: the parser now maps `FIXABLE` to `CRITICAL` in a comment (a.ts:3)', 'CRITICAL'], + ['No input validation is CRITICAL (upload.ts:4)', 'CRITICAL'], ['No CSRF token is a CRITICAL gap at form.ts:12', 'CRITICAL'], + ['Nothing escaped here is CRITICAL: stored XSS at view.ts:8', 'CRITICAL'], ['No timeout here is High (fetch.ts:3)', 'High']]; for (const [line, label] of kept) expect([line, parseFindings('outside-voice', line).map(f => f.label)]).toEqual([line, [label]]); const native = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/harn15-152-p1-adversarial-native.txt'), 'utf8'); expect(parseFindings('adversarial-native', native).map(f => f.label)).toEqual(['CRITICAL', 'INVESTIGATE', 'INVESTIGATE', 'CRITICAL', 'INVESTIGATE', 'INVESTIGATE', 'CRITICAL']); expect(parseFindings('adversarial-native', '**1. Race in a.ts:3 (`FIXABLE`).**\nSeverity: Low. Blocks: no').map(f => f.label)).toEqual(['CRITICAL']); expect(parseFindings('red-team', '1. Race in a.ts:3 (FIXABLE).').map(f => f.label)).toEqual(['FIXABLE']); + expect(parseFindings('adversarial-native', 'No rate limit is FIXABLE (login.ts:9)').map(f => f.label)).toEqual(['CRITICAL']); }); // Value: protects=a list item whose label first appears in an indented later paragraph ("Fix: ... FIXABLE. Severity: Low.") collects as