Four scanners could not say they had not looked, and guarddog could not say it had found something - #156
Conversation
…ot say it had found something `tool()` mapped every non-zero exit to a verdict, which files "I could not run" under "you are out of step". Running all five scanners to check that assumption found four of them breaking it, and one breaking it the other way. guarddog is the serious one. `verify` exits 0 whether it found three high-severity risks or none, in both ecosystems, so every guarddog finding in this fleet was reported clean. That is a false negative on the one scanner here whose subject is malware and typosquats. `--exit-non-zero-on-finding` is not the remedy: it counts `issues`, which includes capability matches, and `six` reports `issues: 2` with `risks: []` under guarddog's own label `no_risks_detected`, so the flag fails a package guarddog itself calls clean. guarddog now runs with `--output-format json` and its own `risks` list is counted. This is the one scanner whose findings are read here, against the rule the module header otherwise holds, and the module header now says so: the alternative was running it for nothing. The count is reported, never recomputed. The other three, all verified by running them rather than by reading docs: - cargo-vet answers 255 both for an unvetted dependency and for a store it could not open. A run that reported prints on stdout; one that could not start leaves stdout empty and puts its diagnostic on stderr. - cargo-deny returns a bitmask, 1 advisories, 2 bans, 4 licenses, 8 sources, so a matched RUSTSEC advisory and an advisory database it could not fetch share the 1. Same stdout test. - osv-scanner separates them itself at 127 and 128, and both were being read as refusals. Only 1 is a finding. - zizmor answers 11 through 14 by highest severity, so any other non-zero code audited nothing. Its dangerous case is exit 0: handed one unparseable workflow among good ones it skips that one, audits the rest and reports no findings, and its SARIF asserts executionSuccessful true. The stderr line is the only witness, and this section hands zizmor a list of files. `tool_read()` hands each scanner its exit code, stdout and stderr and takes a reader that may name a could-not-look. The reader is consulted before the zero is believed, because two of these failures wear exit 0. Eleven tests, each with its control. Every reader was reverted in turn to confirm the test that covers it fails without it: three fail without the guarddog risks read, four without the other readers, and the controls holding that a genuine finding stays a finding pass in both directions. Seven existing tests had stubs answering a bare `exit 0` for guarddog. That stopped modelling the tool once the invocation gained `--output-format json`, since a real clean run prints a report whose `risks` list is empty. One of them also grepped `"$3"` for the requirements path, which the new flag shifts to `"$5"`; it would have passed while asserting nothing. ADR 0007 records the orchestrator survey this came out of, and why the command is written here rather than replaced by MegaLinter, trunk, SARIF aggregation or pre-commit. The XCCDF citation moves from r3 to r4 and gains the scoring asymmetry `verdict()` implements. Claude-Session: https://claude.ai/code/session_01YE1DDjVegJr2MZ12TjPW6e
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (88.73%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #156 +/- ##
==========================================
- Coverage 93.26% 93.21% -0.06%
==========================================
Files 38 38
Lines 15276 15396 +120
==========================================
+ Hits 14247 14351 +104
- Misses 1029 1045 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tool()mapped every non-zero exit to a verdict, which files "I could not run" under "you are out of step". Checking that assumption meant running all five scanners rather than reading their docs, and four of them break it. One breaks it the other way.guarddog was reporting findings as clean
guarddog verifyexits0whether it found three high-severity risks or none, in both ecosystems. Verified:pyyaml==6.0.2gives three high risks at score 8.8 / high_risk, and exits0. npmnode-gypgives 2 risks, exit0.This section answered by exit code, so every guarddog finding in the fleet was reported clean. That is a false negative on the one scanner here whose subject is malware and typosquats.
--exit-non-zero-on-findingis not the remedy. It countsissues, which includes capability matches.six==1.17.0reportsissues: 2withrisks: []and guarddog's own labelno_risks_detected, so the flag fails a package guarddog itself calls clean. It trades a false negative for a false positive, and there is a test pinning that.guarddog now runs with
--output-format jsonand its ownriskslist is counted. This is the one scanner whose findings are read here, against the rule the module header otherwise holds, and the header now states the exception: the alternative was running it for nothing. The count is reported, never recomputed. The same report carries the could-not-look, since a dependency it could not download populateserrorsand dropsresultswhile still exiting0.The other four
255is an unvetted dependency or a store it could not open1is a matched advisory or a database it could not fetch127/128, both read as refusals1is a finding11-14are findings by severity; exit0when it skips an unparseable workflowfailed to parse input:on stderrzizmor's is the subtle one. Handed one workflow it cannot parse alongside workflows it can, it skips that one, audits the rest, reports no findings and exits
0. Its SARIF assertsexecutionSuccessful: truein exactly that case, so the structured output is worse than useless. This section hands zizmor a list of files, which is precisely the shape that triggers it.tool_read()now hands each scanner its exit code, stdout and stderr and takes a reader that may name a could-not-look. The reader is consulted before the zero is believed, because two of these failures wear exit0. A tool whose exit code already separates the two passes a reader that never fires.Evidence
Every contract above was measured by running the installed tool, not by reading its documentation. Versions: osv-scanner 2.5.1, zizmor 1.30.0, cargo-deny 0.20.2, cargo-vet 0.10.2, guarddog 3.2.0.
Eleven new tests, each with a control. Every reader was reverted in turn to confirm the test covering it fails without it: three fail without the guarddog risks read, four without the other readers, and the controls holding that a genuine finding stays a finding pass in both directions.
A note on the seven tests this changed
Seven existing tests had guarddog stubs answering a bare
exit 0. That stopped modelling the tool once the invocation gained--output-format json, because a real clean run prints a report whoseriskslist is empty. Their stubs were updated, not the code they test.One of them also grepped
"$3"for the requirements-file path, which the new flag shifts to"$5". It would have kept passing while asserting nothing.ADR 0007
Records the orchestrator survey this came out of: whether
supply-chainshould be retired in favour of MegaLinter, trunk, SARIF aggregation, Scorecard, Trivy or pre-commit. None can express the contract, and no candidate reaches more than two of the five scanners. The four scanner defects above were the survey's actual yield.principles/explicit-unknown.tomlmoves its XCCDF citation from r3 to r4 and gains the scoring asymmetryverdict()implements.cargo clippy --all-targetsclean under pedantic,cargo fmt --checkclean, all suites pass,uphold scanpasses.https://claude.ai/code/session_01YE1DDjVegJr2MZ12TjPW6e