mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
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>
This commit is contained in:
1 parent
b46ac1a390
commit
96e7bd43f0
4 files changed
+15
-9
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user