mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-07 17:28:23 +01:00
fix(review): lead judgment may lower a native finding to the Severity it stated beside Blocks: no; collect keeps its label (GST-1406 review fix)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
11283d4ef0
commit
07005f3b28
4 files changed
+45
-23
No files matched your search
@@ -1,3 +1,3 @@
|
||||
### Fixed
|
||||
|
||||
- **A native adversarial finding that says `Blocks: no` after some prose is no longer collected as CRITICAL (GST-1406).** `gstack-review-dispatch collect` honored GST-1031's rule (a FIXABLE finding with its own `Severity` and `Blocks: no` collects at that severity) only when the fields started their own line. When the reader wrote them after prose (`… so the docs are wrong. Severity: Medium. Blocks: no.`), the heading's FIXABLE won. If the line was in the heading's paragraph, the fields split off as a second record. Lead judgment can't lower a CRITICAL, so the native record stayed unresolved and /ship's Step 11.5 couldn't bind. Now such a line counts as the finding's fields. A finding with no `Blocks: no`, or with `Blocks: yes`, still collects CRITICAL.
|
||||
- **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`, and the ledger's blocker count follows it. Other readers, a finding that states `Blocks: yes` or no `Blocks`, or a lowering below the stated Severity or with no reason are still refused as downgrades. See 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
|
||||
|
||||
@@ -86,6 +86,11 @@ const LEAD_LABEL = new RegExp(`^(?:[^\\p{L}\\p{N}\\s]\\s*|\\d+[.)]\\s*)*(?:\\w[\
|
||||
const STATED_BLOCKS = /(?<![A-Za-z0-9])Blocks\**\s*:?\s*\**\s*(yes|no)(?![A-Za-z0-9])/gi;
|
||||
/** A prose reader's `Blocks` field as it can lower a count (GST-1031): its colon required, so prose ("blocks no other writer") never does, and a JSON finding (no Blocks field in its schema) never has one. */
|
||||
const BLOCKS_FIELD = /(?<![A-Za-z0-9])Blocks\**\s*:\s*\**\s*(yes|no)(?![A-Za-z0-9-])/gi;
|
||||
/** A native voice's finding whose reader stated `Blocks: no` and one Severity (GST-1031 ruling A): lead judgment may lower it to that Severity, never below, on record with a reason (GST-1406). */
|
||||
const statedLowering = (reader: string, raw: string, to: number) => {
|
||||
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")
|
||||
@@ -222,12 +227,6 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr
|
||||
}
|
||||
const heading = LABEL_HEADING.exec(t);
|
||||
if (heading) { section = fixableRaises ? named(heading[1]!) : heading[1]!; current = null; head = ''; continue; }
|
||||
if (current?.heading && !current.fields.size && fieldOf(t) !== undefined && !LEAD_LABEL.test(t) && !LIST_ITEM.test(t) && !HEADING.test(t) && !NUMBERED_HEAD.test(t) && !LOCUS.test(t)) {
|
||||
current.fields.add('severity');
|
||||
current.label = blocks ? 'CRITICAL' : critical(current.raw) ? stronger(current.label, label!) : strongestOf([current.lead, label])!;
|
||||
current.raw += ` | ${t}`;
|
||||
continue;
|
||||
}
|
||||
if (current?.heading && !current.fields.size && !label && !blocks && !/^#{1,6}\s/.test(t) && !NUMBERED_HEAD.test(t) && !t.startsWith('{')) { current.raw += ` | ${t}`; continue; }
|
||||
if (!indented && HEADING.test(t)) { section = null; current = null; }
|
||||
if (t.startsWith('{')) {
|
||||
@@ -345,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); }
|
||||
}
|
||||
@@ -414,6 +413,7 @@ export function ledgerBlockers(ledger: string): LedgerBlocker[] {
|
||||
const ranks = new Map<string, number>();
|
||||
const unblocking = new Map<string, number>();
|
||||
const raws = new Map<string, string>();
|
||||
const readers = new Map<string, string>();
|
||||
const loci = new Map<string, string | null>();
|
||||
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[];
|
||||
@@ -422,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, line.reader);
|
||||
loci.set(id, lineLocus(line.raw));
|
||||
}
|
||||
}
|
||||
@@ -431,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 ?? ''));
|
||||
}
|
||||
|
||||
+31
-13
@@ -39,19 +39,6 @@ describe('one record per native finding at its stated severity (GST-1031 case 1,
|
||||
}
|
||||
expect(labels('adversarial-native', '## FIXABLE\n1. Leak in a.ts:3\n2. Race in b.ts:4. Blocks: no', true)).toEqual(['CRITICAL', 'CRITICAL']);
|
||||
});
|
||||
test('fields that follow body prose on their line still count as the fields (GST-1406)', () => {
|
||||
// Value: protects=ruling A when the reader writes its Severity and Blocks: no after prose on the same line, in the heading's
|
||||
// paragraph or after a blank line; fails_when=the heading's FIXABLE collects CRITICAL (an unlowerable blocker that stops Step
|
||||
// 11.5) or the field line splits off as a second record; why_new=GSTA-275 GST-1276 v4 native pass, finding 1; seam=parseFindings
|
||||
const head = '**1. Title (FIXABLE).** body in lib/x.ts.';
|
||||
for (const raises of [true, false]) {
|
||||
expect(labels('adversarial-native', [head, '', 'More prose. Severity: Medium. Blocks: no.'].join('\n'), raises)).toEqual(['Medium']);
|
||||
expect(labels('adversarial-native', [head, '- a', '- b', '', 'More prose. Severity: Medium. Blocks: no, because a reader still reads it.'].join('\n'), raises)).toEqual(['Medium']);
|
||||
expect(labels('adversarial-native', [head, 'More prose. Severity: Medium. Blocks: no.'].join('\n'), raises)).toEqual(['Medium']);
|
||||
expect(labels('adversarial-native', [head, '', 'More prose. Severity: Medium.'].join('\n'), raises)).toEqual(['CRITICAL']);
|
||||
expect(labels('adversarial-native', [head, '', 'More prose. Severity: Medium. Blocks: yes.'].join('\n'), raises)).toEqual(['CRITICAL']);
|
||||
}
|
||||
});
|
||||
test('collect records the stated severity beside the one record', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gst1031-'));
|
||||
const c = collect(path.join(dir, 'l.jsonl'), 'adversarial-native', fixture('gst-1031-native.txt'), path.join(dir, 'm.jsonl'));
|
||||
@@ -101,6 +88,37 @@ 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 });
|
||||
});
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
Reference in new issue
Block a user