Skip to content

fix: skip taint pass instead of erroring when a language has no taint rules - #74

Open
juangaitanv wants to merge 1 commit into
mainfrom
fix-html-taint-regression
Open

fix: skip taint pass instead of erroring when a language has no taint rules#74
juangaitanv wants to merge 1 commit into
mainfrom
fix-html-taint-regression

Conversation

@juangaitanv

Copy link
Copy Markdown
Contributor

Problem

Scanning any single .html file failed with exit 1:

Error: No taint flow rules found. Please ensure your rules contain rules with mode='taint'.

Our release benchmark saw 179/179 html file scans error out.

Root cause

src/scanner/modes.rs:544-548run_taint_analysis_with_verbosity returned a hard Err when the loaded rule set contained zero mode = "taint" rules. The default scan mode (src/main.rs:119-130) runs the search pass first and the taint pass second; the ? on the taint call discarded the already-computed search findings and aborted the process.

rules/html (7 rules) and rules/objectscript (5 rules) are search-mode only, so every single-file scan of those languages hit the guard. Directory scans were unaffected — the guard counts over the merged rule set for all detected languages, so any python/javascript file in the tree masked it. That is why this shipped unnoticed; the per-file invocation contract (sighthound --output-format json <file>) is what bites.

Fix

A rule pack with no taint rules is a corpus property, not a failure. The guard now emits a show_progress-gated notice on stderr and returns Ok(Vec::new()), so the search-pass findings survive and the process exits 0. This matches vulnerability_scanner.rs:777-783, which already answers the strictly worse condition (both rule halves empty) with Ok(Vec::new()).

No language is special-cased — the condition is a count over the merged rule set — so objectscript is fixed by the same change. No new CLI flag; the invocation contract is unchanged. stdout stays pure JSON under --output-format json.

Tests

Two feature-gated regression tests in tests/strictness/language_coverage.rs, one for html and one for objectscript. Each scans a single file — load-bearing, since a directory scan merges in python/javascript taint rules and would pass without the fix. Verified by stashing the modes.rs hunk: both tests fail (left: Some(1) right: Some(0)), and pass with it.

They assert exit 0, stdout parses as a JSON array, the notice appears on stderr only, and the old error string is gone.

Verification

  • cargo test — 258 passed, exit 0
  • make ci — exit 0 (only pre-existing advisory CRAP/complexity lines)
  • sighthound --output-format json <pygoat .html> — prints [], exit 0 (was exit 1)
  • Release benchmark over realvuln-pygoat + realvuln-lets-be-bad-guys: No taint flow rules found warnings 148 → 0, zero failed scans. Every scoreboard row is numerically identical to the pre-fix baseline, including language:javascript (TP=1 PREC=1.000 REC=1.000) and language:python (TP=13 FN=70) — no behavior change for languages that do have taint rules.
  • Findings for a javascript file (datasets/insecure-js/server.js) are byte-identical before and after.

Known gap (separate work, not this PR)

The language:html scoreboard row stays TP=0 FP=0 FN=17 TN=6. That row is unchanged because the benchmark already scored a failed scan as "found nothing" — TP cases became FN, hard negatives became TN. The observable win here is exit 1 → exit 0 and 148 warnings → 0.

html recall is a separate rules-coverage gap, not part of this regression. main's html pack is Thymeleaf / inline-<script> tuned and verifiably fires on those shapes; the benchmark corpus is Django/Jinja templates ({{ query|safe }}, <form method="POST"> with no {% csrf_token %}), which no current html rule matches. The 7 html TPs the 0.1.2 release binary scored came from a generic rule pack (rules/html/html_security.ron) that only ever existed on the orphan release lineage, and they were line-window coincidences — e.g. CASE-1536 expects CWE-79 at xss_lab.html:27 (the |safe line) and 0.1.2 matched it with an inline onclick handler finding at line 36. Porting that pack back would trade precision on the curated hard negatives for coincidental recall, so it is deliberately out of scope.

… rules

run_taint_analysis_with_verbosity returned a hard Err when the merged rule
set contained zero mode="taint" rules. In the default scan mode the taint
pass runs second, so the `?` discarded the already-computed search findings
and aborted with exit 1.

rules/html and rules/objectscript are search-mode only, so every single-file
scan of those languages hit the guard. Directory scans masked it because the
count is over the merged rule set for all detected languages.

A rule pack with no taint rules is a corpus property, not a failure. Emit a
show_progress-gated notice on stderr and return Ok(Vec::new()), matching
vulnerability_scanner.rs which already answers the strictly worse condition
(both rule halves empty) the same way.

Adds two feature-gated single-file regression tests (html, objectscript);
a directory scan would pass without the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants