fix(scan): soften findings inside Rust inline #[cfg(test)] blocks - #174
Merged
Conversation
The three structural false-positive causes fixed in #154 and #158 all key on where a *file* sits: `_test.go`, `testutils/`, `docs/`. Rust does not work that way. `cargo test` compiles unit tests from a `#[cfg(test)] mod tests` block at the bottom of the very file they cover, so production code and its fixtures share one path and `isTestPath` can never separate them. Measured on ferriskey/ferriskey (Rust IAM, 689 stars) at 0.11.8: 135 findings, zero true positives, of which 15 are exactly this — a fixture password in a test module reported at `high` from a path that looks like production. The sharpest is `core/src/domain/trident/services.rs:2719`, where the test module opens at 2137 of 4042 lines; being a *good* fake password is what kept it from being softened by any of the existing value-side rules. Adds `inlineTestLines()`, which returns the line indices a file's own test blocks occupy, and a `test-block` softening reason alongside `test`. Keyed on lines rather than on the file, because the file is half production code: a to-end-of-file rule would soften nearly 2000 lines of `services.rs` and hide a real credential committed below the test module. Finding the end of a block means counting braces, and counting braces in Rust means lexing it first — `format!("{}", x)` would otherwise close the module early and undo the fix from the inside. `rustCodeLines()` blanks comments, strings and char literals, handling the three things a generic stripper gets wrong: nested block comments, raw strings (`r#"a "quoted" string"#`), and `'a` lifetimes that are not char literals. An unbalanced file claims only its attribute line, so a parse that has gone wrong cannot quietly silence the rest of the file. Only Rust gets this. Go's toolchain will not run a test outside a `_test.go` file, and Python and JavaScript convention give tests their own files — all three already read by `isTestPath`. Verified end to end through the built CLI: - ferriskey: 135 findings before and after, nothing dropped, high 39 -> 24. All 15 moved lines confirmed inside a `#[cfg(test)]` module by an independent check; no production line moved. - malware-test-prs: 134 findings, 46 critical, identical before and after, zero severity moves. Detection is unchanged. - packages/scan 347/347 and apps/cli 73/73 green. As with every other softening here, this moves severity and never drops a finding: the count, the SARIF and `--fail-on low` are all unaffected. The fixture exemption that *skips* a finding stays keyed on `isTestPath` alone — dropping is a verdict, and a block boundary inferred from brace counting is evidence for a severity, not for silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVMa6BbfoMTWAiGvkFXAdk
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ThreatCrush Security Scan12 finding(s) HIGH/CRITICAL: 1 | MEDIUM: 6 | LOW: 5
Snippets are redacted; ThreatCrush never prints matched credential material. |
…omment The scanner flagged its own new doc comment: `text.ts` quoted the ferriskey line it was written to explain, credential and all, and `text.ts` is a production path where no softener applies. That is the rule working exactly as intended, on the commit that shipped it. Describes the measurement instead of transcribing it. Also corrects 22 to 15 — 22 was the count of findings sitting inside a `#[cfg(test)]` block, but 7 of those were already `low` from a value-side rule, so 15 is the number this change actually moves. Comment only; ferriskey scans identically before and after (135 findings, high 24), and `packages/scan` stays 347/347. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVMa6BbfoMTWAiGvkFXAdk
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
2 participants
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.
Cause 4: a language whose tests live inside the file they test
The three structural false-positive causes fixed in #154 and #158 all key on where a file sits —
_test.go,testutils/,docs/. Rust does not work that way.cargo testcompiles unit tests from a#[cfg(test)] mod testsblock at the bottom of the very file they cover, so production code and its fixtures share one path andisTestPathcan never separate them.Measured on ferriskey/ferriskey (Rust IAM server, 689 stars) at 0.11.8: 135 findings, zero true positives, of which 15 are exactly this — a fixture password inside a test module, reported at
highfrom a path that looks like production.The sharpest is
core/src/domain/trident/services.rs:2719:Its
#[cfg(test)]opens at line 2137 of 4042. Being a good fake password is what kept it from being softened by any of the existing value-side rules —describesItsOwnKeycatcheslet password = "my_password"and correctly ignores this one. Only the block it sits in says what it is.What this adds
inlineTestLines()returns the line indices a file's own test blocks occupy, and a newtest-blocksoftening reason sits alongsidetest.Keyed on lines, not on the file.
services.rsis half production code. A "everything after the first#[cfg(test)]" rule would soften nearly 2000 lines of it, and a real credential committed below the test module would report atlowfor a reason nobody could see.Finding the end of a block means counting braces, and counting braces in Rust means lexing it first.
format!("{}", x)is on more lines than most constructs this engine looks for; counted naively it closes the module early and undoes the fix from the inside.rustCodeLines()blanks comments, strings and char literals, handling the three things a generic stripper gets wrong:*/does not necessarily end oner#"a "quoted" string"#'is far more often a lifetime (&'a str) than a char literalAn unbalanced file claims only its attribute line, so a parse that has already gone wrong cannot quietly silence the rest of the file.
Only Rust gets this. Go's toolchain will not run a test outside a
_test.gofile, and Python and JavaScript convention give tests their own files — all three already read byisTestPath.Two things it deliberately does not do:
#[cfg(not(test))]gates code compiled when tests are off. It is production code by definition and is excluded.#[cfg(feature = "test-util")]is a feature flag that merely has "test" in its name. The string blanking is what keeps it from reading as a gate.Verification
End to end through the built CLI, not the source:
highAll 15 moved lines were confirmed to sit inside a
#[cfg(test)]module by a check independent of the implementation (no top-level}closes the module between the attribute and the finding). No production line moved. Scan wall clock on ferriskey is unchanged at ~4.0s.packages/scan347/347 andapps/cli73/73 green.The line this does not cross
As with every other softening here, this moves severity and never drops a finding — the count, the SARIF and
--fail-on loware all unaffected. The fixture exemption that actually skips a finding stays keyed onisTestPathalone: dropping is a verdict, and a block boundary inferred from brace counting is evidence for a severity, not for silence.Credential fixtures in the new test file are assembled at runtime rather than written out, since the
gitleaksjob reads every ref and a password-shaped literal on this branch would redden unrelated PRs.What this does not fix
ferriskey still scores 0 true positives out of 135, and this change does not make it pitchable — the remaining 120 are the same
secret-database-urlon local dev DSNs,js-open-redirecton a server-supplied URL the backend validates, andjs-jwt-decode-without-verifyon a browser reading its own token. Rust is 63% of that repository and the engine has no Rust rules at all. This fixes one structural cause; it does not change the verdict on Rust-majority repos.🤖 Generated with Claude Code
https://claude.ai/code/session_01CVMa6BbfoMTWAiGvkFXAdk