fix(scan): gate on the unsuppressed population by default (secure --fail-on) — supersedes #24, #25#28
Conversation
…ail-on) Close a HIGH-severity CI-gate bypass. `wardline scan --fail-on` applied repository-controlled suppressions (`.wardline/baseline.yaml`, `wardline.yaml` waivers, `.wardline/judged.yaml`) to findings BEFORE evaluating the gate, so a malicious PR could commit a suppression keyed to its own new defect's fingerprint and clear the gate. All three sources are committed repo content and equally exploitable. Reproduced live (baselining the sole ERROR zeroed the gate). Secure-by-default model (combines #24 + #25): - `gate_decision` now evaluates a separate UNSUPPRESSED population (`ScanResult.gate_findings`). baseline/waiver/judged still ANNOTATE the emitted findings (`suppressed=…` stays visible) but no longer clear the gate. - The gate population is built with apply_suppressions over EMPTY baseline + waivers + judged, NOT `list(raw)`, so the lineless-DEFECT→non-gating-FACT downgrade is preserved (no spurious gate trips). - `--new-since <ref>` (operator-supplied, unforgeable) scopes BOTH the emitted and gate populations — the secure CI ratchet. - `--trust-suppressions` (CLI) / `trust_suppressions` (run_scan, MCP scan tool), default False, restores the local ratchet / judge DX for trusted checkouts (None sentinel → gate falls back to the suppressed findings). `run_judge` passes True so judge/triage/persist are unchanged. - `load_judged` now requires `verdict: FALSE_POSITIVE` (rejects a hand-edited TRUE_POSITIVE / missing verdict smuggled in as a silent suppression). BREAKING (noted in CHANGELOG, acceptable at 0.x): baseline-gated CI goes green→red on upgrade until `--new-since` or `--trust-suppressions` is added. Docs updated (suppression.md): the secure CI ratchet is `--new-since`. Combines and supersedes #24 (judged-only) and #25 (no escape hatch + a lineless-DEFECT gate bug). Full suite green (2394 passed), ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9aea1afde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name="scan", | ||
| description="Whole-program taint scan of the project. Returns structured " | ||
| "findings, the suppression summary (active = the gate population), " | ||
| "findings, the suppression summary (active = unsuppressed defects; " |
There was a problem hiding this comment.
Correct MCP summary active semantics
For scans where a repository baseline/waiver/judged record suppresses the only defect, the MCP response still reports summary.active from the emitted, suppressed result.findings population, so it will be 0 while the new unsuppressed gate can still trip. This description tells agents that active is the unsuppressed gate population, which makes the structured tool contract misleading precisely in the new secure-default scenario and can cause callers to conclude there are no defects to address even though gate.tripped is true.
Useful? React with 👍 / 👎.
Two CI jobs (`ruff format --check src tests`, bare `mypy`) were red on main. Causes were pre-existing/mechanical, not behavioural: - `ruff format`: `src/wardline/core/legis.py` + `tests/conformance/test_legis_intake_contract.py` (from the legis merge 948daa4) and `src/wardline/core/judged.py` + `tests/unit/cli/test_cli.py` (from #28) were never `ruff format`-reflowed. - `mypy`: `tests/unit/core/test_legis_artifact.py:175` carried a stale `# type: ignore[arg-type]` for a `subprocess.run(cwd=<object>)` call mypy now reports as `call-overload`; corrected the ignore code. Formatting-only + an ignore-code correction; no logic changes. Full suite 2406 passed, ruff check/format + mypy strict all green. Co-authored-by: John Morrissey <john@wardline.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Closes a HIGH-severity CI-gate bypass.
wardline scan --fail-onapplied repository-controlled suppressions (.wardline/baseline.yaml,wardline.yamlwaivers,.wardline/judged.yaml) to findings before evaluating the gate. Since all three are committed repo content, a malicious PR could add a suppression keyed to its own new defect's fingerprint and clear the gate. Reproduced live (baselining the sole ERROR defect zeroed the gate).Model (maintainer-chosen): secure-by-default + opt-in
gate_decisionnow evaluates a separate unsuppressed population (ScanResult.gate_findings). baseline/waiver/judged still annotate the emitted findings (suppressed=…stays visible) but no longer clear the gate.apply_suppressionsover empty baseline + waivers + judged (notlist(raw)), preserving the lineless-DEFECT→non-gating-FACT downgrade (no spurious trips).--new-since <ref>(operator-supplied, unforgeable) scopes both populations — the secure CI ratchet.--trust-suppressions(CLI) /trust_suppressions(run_scan, MCPscan), default False, restores the local ratchet / judge DX (None sentinel → gate falls back to suppressed findings).run_judgepassesTrue.load_judgednow requiresverdict: FALSE_POSITIVE(rejects a hand-edited TRUE_POSITIVE / missing verdict).Why this and not #24 / #25
gate_findings = list(raw)(a lineless-DEFECT gate bug).This combines them: #25's mechanism (fixed) + #24's verdict-hardening + the
--trust-suppressionsescape hatch + docs.A CI job relying on a committed baseline to keep
scan --fail-ongreen goes red on upgrade until--new-since <merge-base>(recommended) or--trust-suppressions(trusted checkouts) is added. legis's artifact + the 'one judge' property derive from the annotatedfindings, so they're unchanged; only the local--fail-onexit code changed.Verification
--trust-suppressions, while staying annotatedsuppressed=baselined.--trust-suppressionsvariants; CLI + MCP parity covered.🤖 Generated with Claude Code