Fix scan fail-on bypass via repository-controlled suppressions#25
Fix scan fail-on bypass via repository-controlled suppressions#25tachyon-beep wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd64010099
ℹ️ 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".
| findings = apply_suppressions(raw, baseline, waivers, today=date.today(), judged=judged) | ||
| # Keep a separate gate population that applies only operator-supplied scan | ||
| # scoping (for example --new-since), not repository-controlled suppressions. | ||
| gate_findings = list(raw) |
There was a problem hiding this comment.
When a rule or local pack emits a normal-file DEFECT with line_start=None, apply_suppressions intentionally replaces it with a WLN-ENGINE-LINELESS-DEFECT FACT so the invalid, collision-prone defect is not treated as an active finding. Building gate_findings directly from raw bypasses that tool-owned invariant, so scan --fail-on can now fail on a defect that is omitted from the emitted findings and was previously quarantined for fingerprint-safety reasons; only repository-controlled baseline/waiver/judged suppressions should be skipped here.
Useful? React with 👍 / 👎.
| if fail_on is None: | ||
| return GateDecision(tripped=False, fail_on=None, exit_class=0) | ||
| tripped = gate_trips(result.findings, fail_on) | ||
| gate_findings = result.gate_findings or result.findings |
There was a problem hiding this comment.
When the only above-threshold defect is baselined, waived, or judged, this switch makes gate.tripped true while the MCP response still emits that finding as suppressed; _scan only attaches inline explanations for SuppressionState.ACTIVE findings (src/wardline/mcp/server.py:160-164). Agents using scan with explain: true and fail_on can therefore receive a failing gate with no explained active defect to fix, so the gate-relevant population needs to be exposed/explained separately or the emitted finding needs a gate-relevant marker.
Useful? React with 👍 / 👎.
| findings = apply_suppressions(raw, baseline, waivers, today=date.today(), judged=judged) | ||
| # Keep a separate gate population that applies only operator-supplied scan | ||
| # scoping (for example --new-since), not repository-controlled suppressions. | ||
| gate_findings = list(raw) |
There was a problem hiding this comment.
This treats every waiver as untrusted by gating on the completely raw findings, but run_scan(..., config_path=...) and the CLI --config path can be an operator-supplied config outside the repository. In that CI setup, centrally managed waivers in the explicit config are no longer honored by --fail-on, even though the nearby config handling explicitly preserves operator-provided policy; only waivers loaded from the repository's default wardline.yaml should be excluded from the gate population.
Useful? React with 👍 / 👎.
…ail-on) (#28) 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: John Morrissey <john@wardline.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed ourselves in The vulnerability is real and HIGH-severity, and your scope was right: all three repo-controlled suppression sources (baseline / waiver / judged) must stop clearing the We wrote our own rather than merge this directly because of two issues:
Also folded in #24's Verified: full suite 2406 passed, ruff + mypy clean; live repro shows a baselined ERROR trips the gate by default and clears under |
1 participant
Motivation
--fail-ongate, allowing an attacker-controlled repo entry to hide defects from CI gates.Description
ScanResult.gate_findingsthat is derived from the raw (pre-suppression) findings so gates evaluate an unsuppressed view while emitted findings remain annotated (file:src/wardline/core/run.py).--new-sincedelta scoping to both the emitted findings and the separate gate population so operator scoping still scopes the gate but repository suppressions do not (file:src/wardline/core/run.py).gate_decisionto evaluate against thegate_findingspopulation when present, falling back toresult.findingsotherwise (file:src/wardline/core/run.py).--fail-ongate (files:tests/unit/core/test_run.py,tests/unit/cli/test_cli.py).Testing
uv run ruff checkand Python compilation withpython -m py_compile, both succeeded.tests/unit/core/test_run.py::test_gate_decision_uses_unsuppressed_gate_populationandtests/unit/core/test_suppression.py::test_gate_trips_only_on_active_defect_at_or_above_threshold, which passed.--extra scanner) but some runs were blocked by dependency downloads (PyYAML/click) due to external network/tunnel failures and thus could not complete; this is an environment issue and not a code regression.Codex Task