fix(detectors): validate structure instead of matching shape alone - #85
Draft
pixincreate wants to merge 1 commit into
Draft
fix(detectors): validate structure instead of matching shape alone#85pixincreate wants to merge 1 commit into
pixincreate wants to merge 1 commit into
Conversation
Four detectors matched far more than they should, or nothing at all. CreditCardDetector accepted any 13-16 digit run with arbitrary spaces, so commit hashes, timestamps and ids were reported HIGH — 1611 of them in one payments repository, and one match even spanned the gap between two unrelated numbers. Detectors can now declare a structural check (validate = "luhn"), and the pattern additionally requires an issuer prefix in 4-digit groups. Luhn alone was not enough: about one in ten random digit runs satisfies it. HighEntropyDetector required entropy 4.0 from hex, whose 16-symbol alphabet caps entropy at exactly 4.0 — reachable only by a perfectly uniform string, so it had never fired (measured max over 20k random 64-char samples: 3.97). Lowering it to 3.6 alone produced 758 hits in a hashing-heavy repository, because a bare 64-char hex string is a SHA-256 digest as often as a 32-byte token and no threshold separates them. It now relies on the keyword prefilter for credential context. PKCS#8 headers matched nothing: the pattern's algorithm group was effectively mandatory, so 'BEGIN PRIVATE KEY' and 'BEGIN ENCRYPTED PRIVATE KEY' — what openssl genpkey and service-account JSON emit — were invisible. PhoneNumberDetector matched any 10-digit run, making every unix timestamp a phone number. Measured across seven working repositories: -22%, -28%, -11%, -6%, -5% findings, with detection of real cards, keys and tokens verified by test.
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.
Stacked on #84 — review that one first; this PR's diff is only the detector changes.
Summary
Four detectors matched on shape alone, so they either flooded reports or never fired. Detectors can now declare a structural check, and the worst patterns are tightened.
Changes
\b(?:\d[ -]*?){13,16}\baccepted any 13–16 digit run with arbitrary spaces — commit hashes, timestamps, ids, all reported HIGH. One match spanned the gap between two unrelated numbers (index aabbcc0..1111111 100644). Now requires an issuer prefix in 4-digit groups and a Luhn checksum, via a newvalidate = "luhn"field. Luhn alone was insufficient: ~1 in 10 random digit runs passes it.BEGIN PRIVATE KEYandBEGIN ENCRYPTED PRIVATE KEY— whatopenssl genpkeyand GCP/Azure service-account JSON emit — matched nothing.Effect
Measured on seven working repositories, same corpus before and after:
The card vault is flat because the now-working high-entropy detector adds back roughly what the card fix removes — in credential context only.
Tests
Four regression tests assert both directions: real Visa/Mastercard/Amex numbers, separated phone numbers, all three private-key headers, and hex in credential context are still reported; Luhn-invalid numbers, prefix-less digit runs, bare timestamps, and bare digests are not. 181 tests pass.