fix(gate): the threat model applies to new findings only (decision 221)
Undone: item 2 of the Part 2 fix round (1be961a). recordVerdict no longer
drops a copy rule A carried as counted when a finding with the same id is
backed this round, and no longer recomputes the counts, `lowered` and the
stop-rule summary for it; the isLowered helper and the SEVERITIES import it
needed are gone. Why: the id is (file, line, rule_key), so a DIFFERENT
backed security finding at the same line and key knocked a counted security
finding inside the threat model out of the count (round 2 PASS, its text
gone), against decisions 218 and 220.
The rule now, as decided: the threat model applies only to this round's
new findings (the deduped findings, before countRound); a finding counted
in an earlier round stays counted, through rule A's carry, until a reviewer
marks it resolved. The gate never counts less. Items 1 and 3-8 of the fix
round are untouched.
Tests: the fix-round case for item 2 now asserts the decided rule (the
carried copy stays counted, BLOCK, the stop rule fires not-fewer, the gate
stops); a new case pins that a different backed finding at the same line
and key leaves the counted one, with its own text, in the count.
The plan-gate header and the known_outside comment, and Handover 4's
known_outside sentence, state the rule. CHANGELOG Unreleased, Known issues:
a finding counted earlier stays counted when an Outside: line approved
later puts it outside, or when it moves to another plan line; the way on is
the operator's choice at the stop rule.
Part 2 of the E plan, decision 221.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
1be961aab1
commit
ff46f57a89
4 changed files with 45 additions and 29 deletions
|
|
@ -123,9 +123,11 @@
|
|||
// `threat_model_ref` equals an `Outside:` line's text exactly — leaves the
|
||||
// counted set and the stop rule and goes to the optional verdict field
|
||||
// `known_outside` ({id, rule_key, rule_keys, text, line}), carried to later
|
||||
// rounds until a reviewer marks it resolved (or it counts again under its id);
|
||||
// a copy of it that rule A carried from an earlier round as counted leaves the
|
||||
// count too. The verdict stores `brief_path`, relative to the repo's top level
|
||||
// rounds until a reviewer marks it resolved (or it counts again under its id).
|
||||
// The threat model applies to this round's new findings only (decision 221): a
|
||||
// finding counted in an earlier round stays counted, through rule A's carry,
|
||||
// until a reviewer marks it resolved; the gate never counts less. The verdict
|
||||
// stores `brief_path`, relative to the repo's top level
|
||||
// (carried when a later round has no --brief), and the gate reads it from
|
||||
// there whatever its working directory. A threat
|
||||
// model that is not usable backs nothing: every security finding counts, and
|
||||
|
|
@ -143,7 +145,7 @@ import { createHash } from 'node:crypto';
|
|||
import { dirname, join, resolve } from 'node:path';
|
||||
import { validatePlanContent } from './plan-validator.mjs';
|
||||
import { splitSections, fenceTracker, extractPlanVersion, versionAtLeast } from '../parsers/plan-schema.mjs';
|
||||
import { normalizeSeverity, isSecurity, SEVERITIES } from '../review/plan-review-dedup.mjs';
|
||||
import { normalizeSeverity, isSecurity } from '../review/plan-review-dedup.mjs';
|
||||
import { countRound, lineDiff, oneLine } from '../review/critic-diff.mjs';
|
||||
import { roundSummary, decideStop, describeStop, STOP_REASONS } from '../review/stop-rule.mjs';
|
||||
import { usableInRepo, repoRelativeBrief, backedLine } from '../review/threat-model.mjs';
|
||||
|
|
@ -292,13 +294,6 @@ function okKnownOutside(k) {
|
|||
typeof e.text === 'string' && typeof e.line === 'string' && e.line !== '');
|
||||
}
|
||||
|
||||
/** A counted finding a reviewer rated higher than it counts (rule A or B). */
|
||||
const isLowered = x => {
|
||||
const o = normalizeSeverity(x.original_severity);
|
||||
const s = normalizeSeverity(x.severity);
|
||||
return o !== null && s !== null && SEVERITIES.indexOf(o) < SEVERITIES.indexOf(s);
|
||||
};
|
||||
|
||||
/** Optional `brief_path`: a non-empty string. */
|
||||
const okBriefPath = p => p === undefined || (typeof p === 'string' && p !== '');
|
||||
|
||||
|
|
@ -437,12 +432,7 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
findings: counted,
|
||||
marks: Array.isArray(dedup.marks) ? dedup.marks : [],
|
||||
});
|
||||
// A previous finding countRound carried (rule A) whose id is backed this
|
||||
// round leaves the counted set too: listed once, in known_outside, and out
|
||||
// of the counts, the verdict and the stop rule's summary.
|
||||
const leaving = c.findings.filter(x => x.carried_from !== undefined && x.carried_from !== null && backedIds.has(x.id));
|
||||
const findings = c.findings.filter(x => !leaving.includes(x));
|
||||
const lowered = c.lowered - leaving.filter(isLowered).length;
|
||||
const findings = c.findings;
|
||||
const counts = { blocker: 0, major: 0, minor: 0 };
|
||||
for (const f of findings) counts[f.severity] += 1;
|
||||
const verdict = {
|
||||
|
|
@ -461,7 +451,9 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
if (c.resolved !== null) verdict.resolved = c.resolved;
|
||||
// known_outside: this round's backed findings, after the previous round's
|
||||
// entries that no reviewer marked resolved and that are not reported again
|
||||
// under their id (backed again: listed once; counted: in full, not listed).
|
||||
// under their id. The threat model applies to this round's new findings only
|
||||
// (decision 221): a finding counted in an earlier round stays counted, through
|
||||
// rule A's carry, until a reviewer marks it resolved.
|
||||
const marks = Array.isArray(dedup.marks) ? dedup.marks : [];
|
||||
const countedIds = new Set(findings.map(x => x.id).filter(id => typeof id === 'string'));
|
||||
const kept = (Array.isArray(prev.verdict?.known_outside) ? prev.verdict.known_outside : [])
|
||||
|
|
@ -480,7 +472,7 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
? { applied: false, why: 'no --brief given' }
|
||||
: { applied: use.usable === true, why: use.usable === true ? null : (use.why ?? 'the threat model is not usable') };
|
||||
return {
|
||||
code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered, threat_model: threat,
|
||||
code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered: c.lowered, threat_model: threat,
|
||||
message: `round ${round} recorded: ${verdict.verdict}` +
|
||||
(knownOutside.length > 0 ? `; ${knownOutside.length} finding(s) outside the threat model listed as known issues` : '') +
|
||||
(use !== undefined && !threat.applied ? `; threat model not applied, every security finding counts: ${threat.why}` : '') +
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue