feat(ci): add fail_on_severity so the scan gate can actually fail - #169
Conversation
`panic-attack assail` has no non-zero exit path of its own — no process::exit, no ExitCode, no --fail-on anywhere in its 3,132-line src/main.rs. It always exits 0. Consumers of this reusable therefore ran a scanner whose only value was the cross-repo report; when the dispatch is skipped (no VERISIMDB_PAT) the job scans, discards the result, and reports success regardless of findings. Adds a `fail_on_severity` input (none | low | medium | high | critical) and a gate step that fails the job when an unsuppressed finding sits at or above the threshold. Design notes: * **Default is 'none'**, so this is a no-op for every existing caller until it opts in. verisimdb, echidna and ambientops all stay green on merge. * **Counts only unsuppressed findings.** `suppressed` is set by the repo's own classification registry and by test-context detection, and is *omitted* rather than set false — hence `select(.suppressed != true)` rather than `== false`. * **An unrecognised threshold aborts.** A typo like 'hgih' must not degrade to 'threshold 0' and pass everything; that silent-pass behaviour is exactly what this change exists to remove. * The failure message points at the classification registry, not at this workflow — suppression belongs next to the evidence and in review, not buried in CI config. Verified against a real scan (verisimdb origin/main, 113 findings): critical -> 0 offending, high -> 1, medium -> 39; the one High was a genuine finding live only because a classification key had gone stale after a rename. YAML parses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review✅ Approved2 resolved / 2 findingsAdds a fail_on_severity input and gate step to the CI scan to enforce unsuppressed finding thresholds. The change requests fixes for case-sensitive threshold validation causing valid inputs to abort and ensuring scan gate failures do not prematurely short-circuit the report dispatch step. ✅ 2 resolved✅ Edge Case: Capitalized 'None' aborts the job instead of no-op
✅ Edge Case: Gate failure suppresses cross-repo report dispatch
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Uh oh!
There was an error while loading. Please reload this page.
The problem
panic-attack assailhas no non-zero exit path of its own — noprocess::exit, noExitCode, no--fail-onanywhere in its 3,132-linesrc/main.rs. It always exits 0.So a consumer of
scan-and-report.ymlgets a job whose only real output is thecross-repo report. When that dispatch is skipped — which is the documented,
supported path when
VERISIMDB_PATis absent — the job scans, throws theresult away, and reports success no matter what it found.
Measured on
hyperpolymath/verisimdbatorigin/main: 113 findings, includingone live High, with the Security Scan reporting green throughout.
The change
A
fail_on_severityinput (none | low | medium | high | critical) plus a gatestep that fails when an unsuppressed finding sits at or above the threshold.
Four things worth reviewing specifically:
none— this is a no-op until opted into. verisimdb, echidna andambientops all stay exactly as green on merge as they are now. Nobody gets a
surprise red from this landing.
suppressedcomes from the repo's ownclassification registry and from test-context detection, and it is omitted
rather than set to
false— so the filter isselect(.suppressed != true),not
== false. Getting that backwards would make the gate count nothing.hgihmust notfall through to "threshold 0" and pass everything. A gate that silently
accepts a malformed configuration is the same class of defect this PR exists
to remove, so the
casehas an explicit*)that errors.workflow. Suppression should live next to the evidence with an audit
rationale, in review — not get buried in CI config where nobody re-reads it.
Verification
Gate logic run against a real 113-finding scan (verisimdb
origin/main):criticalhighmediumThe single High was genuine, and instructive: it was live only because a
classification key still named
VeriSimVcl.res's pre-rename path, so a renamehad silently un-suppressed it. Exactly the kind of drift a real gate catches and
an always-exit-0 scan cannot. That key is fixed in verisimdb#201, which takes
that repo to 0 High / 0 Critical — so it can adopt
fail_on_severity: highimmediately without a flag day.
A malformed threshold was confirmed to abort rather than pass. YAML parses.
Not included
No baseline/grandfathering file. Suppression already has a home — the
per-repo classification registry — and adding a second, CI-local mechanism
would split the audit trail across two places. Happy to add one if you'd rather
have a fast path for bulk-accepting an existing backlog.
🤖 Generated with Claude Code