feat: corpus-driven generic secret rules, cache permissions, report integrity (1.5.0) - #42
Merged
Conversation
…ntegrity (1.5.0) The generic secret rules added in 1.4.0 and tuned in 1.4.1 were written against no corpus. An independent gate found seven blockers in them. This release builds the corpus first and rebuilds the rules against it, so the defect class cannot recur silently. - crates/siloscan-core/tests/corpus: 21 files, 172 positives and 121 individually justified negatives, with a harness asserting recall and precision floors. Recall on generic shapes 0.58 -> 0.99, precision 1.00. Synthetic credentials are assembled at run time from split constants so no file spells a credential; the corpus is excluded from the package. - 40-character values are no longer blanket-allowlisted; passwords with punctuation, sub-20-character values, empty-username URLs, unencoded / and @ in URLs, Authorization Bearer and Basic headers, and XML and .NET config forms are now detected - the 1.4.1 placeholder fix now reaches generic-secret-assignment, not only the credentialed-URL rule - (?-i:) on the value and terminator classes: under (?i) the regex crate folded the Unicode terminator range onto k, a-ring and sharp-s, so a value both continued and terminated at the same byte - cache directories are created 0700 and files 0600 explicitly rather than via umask, and a cache directory or salt not owned by the current user, or writable by group or other, is not trusted - --min-severity is stated in human output and the TUI when it withholds findings; regex rules gain an opt-in redact switch
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
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.
Closes seven confirmed blockers from the independent v1.4.1 production gate, plus the false-positive regression the fix itself introduced.
Root cause
siloscan's vendor-prefixed detection was already excellent - 5/5 GitHub variants, 10/10 across major vendors, 5/5 private key formats. The generic safety net beneath it was four regexes written in one pass against no corpus, and it had structural holes: every exactly-40-character secret was allowlisted (killing GitHub OAuth client secrets), passwords with punctuation were unmatchable, the identifier allowlist widened in 1.4.1 swallowed ~20% of base64url tokens, Authorization headers were unmatched outside curl, empty-username URLs (the canonical Redis form) were never reported, and 1.4.1's placeholder fix reached only the URL rule despite its commit message.
The fix is not seven patches. It is a committed detection corpus - 21 files, 172 positives, 121 individually justified negatives - and a harness that measures recall and precision as tests. The rules were then rebuilt against it. Recall on generic shapes went 0.5814 to 0.9884; precision holds at 1.0000.
The regression this caught
The first rewrite traded precision for recall: findings on 1.8 GB of published Rust and C source went 325 to 1222, with ~870 indefensible - numeric constants (PPPIOCSPASS: u32 = 2148561991), C identifiers matched on a bare 'pass' substring (addrBypass), and every Rust scope-resolution path (Literal::u8_unsuffixed). The corpus could not see it: 21 configuration files, no C amalgamations, no generated bindings. Adversarial review caught it and it was fixed to 410 - the two rebuilt rules went from 324 and 569 findings on that tree down to 4 and 77, while both real credentials in it are still reported. The +85 versus the 1.4.1 baseline is entirely vendor rules with fixed shapes (private-key, age-secret-key), not generic-rule noise.
Review also found a genuinely subtle bug: under (?i) the regex crate case-folds the Unicode terminator class onto k/K (U+212A KELVIN SIGN), a-ring (U+212B) and sharp-s (U+1E9E), all of which are value characters - so a value simultaneously continued and terminated at the same byte, producing truncated spans and double reports.
Security
The relocated per-user cache was created with the process umask: world-readable at the default (and a cache entry is an inventory of a private tree - paths, rule ids, line and column, each secret's byte length), and world-writable under a permissive umask with .salt validated by mode only, no ownership check, restoring the tag-forging primitive the 1.4.0 relocation removed. Directories are now created 0700 and files 0600 explicitly, with ownership and mode enforced on open; an untrusted cache is ignored (cold, correct scans) rather than repaired or errored.
Also
Verification