ci: gate on detection coverage against the testbed corpus - #98
Merged
Conversation
Adds a CI job that scores the CLI against profullstack/malware-test-prs — the public catalog of vulnerable/safe line pairs the rule set has been tuned against all along — and fails if the true-positive rate falls or the false-positive rate rises. Unit tests prove a rule fires on one hand-written line. This proves the whole set still catches what it caught across 67 real cases, and still stays silent on the 78 corrected implementations sitting beside them. A change that trades one false positive for three misses passes every unit test and fails here. At the pinned corpus commit the CLI scores TPR 65.9% (85/129) and FPR 0% (0/78) — up from the 15.6% baseline recorded in code-rules.ts when the code rules were only secret detection, with the false-positive rate held at zero throughout. The remaining misses are dominated by classes KNOWN_GAPS deliberately declines to approximate with line-oriented matching — CSRF, IDOR, TOCTOU, NoSQL injection, session fixation — so the floor is set at TPR 60 / FPR 2, under the current result with room for noise but not for a real regression. The corpus is pinned to a commit, not tracked to its default branch: a new upstream test case would otherwise lower the rate and fail this repository's gate on someone else's change. Bump the pin in a PR whose diff is the rule that covers the new case. Verified locally against the pinned commit: gate passes at 60/2, fails at 90/5, and scores identically from a clean master build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThreatCrush Security Scan157 finding(s) HIGH/CRITICAL: 14 | MEDIUM: 107 | LOW: 36
…and 107 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires up the "vuln db" —
profullstack/malware-test-prs, the public testbed catalog the rule set has been tuned against all along — as a coverage gate in CI.What the testbed is
vulns/VULNERABILITY_CATALOG.json: 67 test cases across seven languages, each with line-level attribution —vulnerable_linesa scanner must flag,safe_linesit must not. The SAFE lines are a control group: every one is a correct implementation of the exact pattern the neighbouring vulnerable code gets wrong. A scanner that flags them is matching syntax, not following data.The repo ships a scorer (
scripts/validate-coverage.py) that already grades inbound SARIF against the catalog. This PR points it the other way: threatcrush scoring itself on every change.The number
At the pinned corpus commit, the CLI scores:
code-rules.tsrecords the starting point: 15.6% true-positive when the code rules were still only secret detection. The shell, PHP, Java and Go rule sets added since moved it to 65.9% — and the false-positive rate sat at 0% throughout. That second number is the one that was hard to hold: every rule was built against the corrected shape as well as the vulnerable one precisely so the control group stays silent, and across the whole corpus it does.Why not higher
The 44 misses are spread across ~32 CWE classes, almost all singletons, and they are dominated by the classes
KNOWN_GAPSincode-rules.tsexplicitly declines to approximate with line-oriented matching: CSRF, IDOR, mass assignment, NoSQL injection, session fixation, TOCTOU, missing authorization. A regex that "caught" those would catch every session read and everyObject.assignin the codebase — it would buy true positives with the false-positive rate this whole effort exists to protect.The one recoverable cluster is CWE-327 / CWE-338 (weak crypto, predictable PRNG — 9 lines), where threatcrush has rules but the credential-context guard is too tight for the testbed's multi-line Python fixtures. That is a real follow-up; it is a rule-tuning task, not a new class.
The gate
Floors at TPR 60 / FPR 2, under the current result with room for ordinary noise but not for a real regression — a rule that stops firing, or one that starts flagging the control group, moves the number past a floor and fails the job.
The corpus is pinned to a commit, not tracked to its default branch. A new upstream test case would otherwise lower the rate and fail this repository's gate on someone else's change. The pin is bumped deliberately, in a PR whose diff is the rule that covers the new case.
Fail-closed: an empty SARIF scores as 0% and would read as a total regression, so the scan step distinguishes "scanner produced nothing" from "scanner regressed" and names the right cause.
Verification
Reproduced the CI steps locally against the pinned commit
f9f4fce:masterbuild → scanvulns/→ score: TPR 65.9%, FPR 0%, PASSThe scorer's Markdown summary lands in the PR's job summary, so every run shows the rate and the full miss list inline.