fix(detectors): validate structure instead of matching shape alone

.keywatch-baseline.json

@@ -1491,6 +1491,125 @@
       "finding_type": "Generic Key/Secret",
       "matched_content_hash": "650dfe7ddce0129182c60029ba66733efa412ba0824523dd6b096482c4eb8ac9",
       "plugin_name": "GenericKeyValueDetector"
+    },
+    {
+      "file_path": "./detectors.toml",
+      "line_number": 275,
+      "finding_type": "Private Key Content",
+      "matched_content_hash": "f59444f5756fc6dd701c599452c1752e64a97ecb0447ce41a1e19eb15293ef9a",
+      "plugin_name": "PrivateKeyDetector"
+    },
+    {
+      "file_path": "./src/scanner.rs",
+      "line_number": 1014,
+      "finding_type": "Private Key Content",
+      "matched_content_hash": "f91082d1cbd2032b5ea19f2bbf6b3f88e02c12ae320fc9e08e6dc0f73d82bdd1",
+      "plugin_name": "PrivateKeyDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 300,
+      "finding_type": "Credit Card Number",
+      "matched_content_hash": "4541206d542811878a9374508fe296fa321a8b56c2902736a02f388836f6e108",
+      "plugin_name": "CreditCardDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 301,
+      "finding_type": "Credit Card Number",
+      "matched_content_hash": "e2e5b50c7d336fb0ed238a9c1dd7520b847fe55b990c8f90ecb810a398854d52",
+      "plugin_name": "CreditCardDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 301,
+      "finding_type": "Aadhaar Card Number",
+      "matched_content_hash": "14270b9138e72cd99e52cab8f9df9ab6cfcec3d2d8d743ca83774ae0d6f1faed",
+      "plugin_name": "AadhaarCardDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 302,
+      "finding_type": "Credit Card Number",
+      "matched_content_hash": "13ae894eedbfba2dbd06400ba5b215ffd661885646ab86e050fb1a0d192c1c5b",
+      "plugin_name": "CreditCardDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 302,
+      "finding_type": "Aadhaar Card Number",
+      "matched_content_hash": "f326bb251646469912e5296e014c0c226ff0e95d866381c1b0d060e7a8bf30ab",
+      "plugin_name": "AadhaarCardDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 303,
+      "finding_type": "Credit Card Number",
+      "matched_content_hash": "7eafa99f1c4d8c35d2a84d390f9e1a9806fb518aaad3bd0c6dde7c9669e1ab97",
+      "plugin_name": "CreditCardDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 326,
+      "finding_type": "Phone Number",
+      "matched_content_hash": "33360227b18134e6e594a3df05104457bac9713999bb6374dc9bd74fd54c49ee",
+      "plugin_name": "PhoneNumberDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 326,
+      "finding_type": "Phone Number",
+      "matched_content_hash": "cc8546d7c697eb4e2aecde0bfa6ece05bcd1323722b22e34f7dba70c97c706fd",
+      "plugin_name": "PhoneNumberDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 326,
+      "finding_type": "Phone Number",
+      "matched_content_hash": "c7730d0abc47a6b7fddd39f1c461ca7e582e5b27b3e38ee4b2dfdec1558d4023",
+      "plugin_name": "PhoneNumberDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 343,
+      "finding_type": "SSH Private Key",
+      "matched_content_hash": "1b01887f477e98dd56ec542c12433dee8c323176bd532c05163823079263ba31",
+      "plugin_name": "SSHPrivateKeyDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 344,
+      "finding_type": "SSH Private Key",
+      "matched_content_hash": "32006da0b4e4851aa7946ffa4f040f364cac7c317f312b5d742ca5833dd8760f",
+      "plugin_name": "SSHPrivateKeyDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 345,
+      "finding_type": "SSH Private Key",
+      "matched_content_hash": "678a65e8968aabff441076ae306e13d4fc85b1d36d8036a10d2100c1dc40d251",
+      "plugin_name": "SSHPrivateKeyDetector"
+    },
+    {
+      "file_path": "./tests/detector_tests.rs",
+      "line_number": 356,
+      "finding_type": "Random String",
+      "matched_content_hash": "e51298df0e431de2bfdf6180e3a7b9f3f092c3e9a912facd350e8c7179936e75",
+      "plugin_name": "RandomString"
+    },
+    {
+      "file_path": "./tests/scanner_tests.rs",
+      "line_number": 210,
+      "finding_type": "Private Key Content",
+      "matched_content_hash": "b94c156724c086556087773aad998c0791dc79172caa4b3184a7330cfa7c1d76",
+      "plugin_name": "PrivateKeyDetector"
+    },
+    {
+      "file_path": "./tests/scanner_tests.rs",
+      "line_number": 212,
+      "finding_type": "Private Key Content",
+      "matched_content_hash": "43531827dd6142886cfeb10207f046021a4eb6c575828583ad2cb20d9430c72f",
+      "plugin_name": "PrivateKeyDetector"
     }
   ]
 }

CHANGELOG.md

@@ -18,6 +18,11 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 
+- `CreditCardDetector` requires an issuer prefix and a valid Luhn checksum, instead of matching any 13-16 digit run
+- `HighEntropyDetector` could never fire (its 4.0 threshold is the ceiling for hex) and now runs, restricted to lines naming a credential
+- PKCS#8 private key headers (`BEGIN PRIVATE KEY`, `BEGIN ENCRYPTED PRIVATE KEY`) are detected
+- `PhoneNumberDetector` needs punctuation or a country code, so unix timestamps are not phone numbers
+- Detectors can require a structural check via `validate = "luhn"`
 - Hooks use built-in detectors, so a `detectors.toml` committed to a scanned repository can no longer replace the detector set and disable its own scan
 - Files git renders as binary (including text marked `-diff` in `.gitattributes`) are read from the index instead of being reported clean
 - `Base64Detector` matches from 28 characters, the length where entropy can actually separate base64 from identifiers

detectors.toml

@@ -36,14 +36,14 @@ keywords = ["eyJ"]
 
 [[detectors]]
 name = "SSHPrivateKeyDetector"
-pattern = "-----BEGIN (RSA|DSA|EC|OPENSSH) PRIVATE KEY-----"
+pattern = "-----BEGIN (?:(?:RSA|DSA|EC|OPENSSH|ENCRYPTED) )?PRIVATE KEY-----"
 finding_type = "SSH Private Key"
 severity = "HIGH"
-keywords = ["BEGIN RSA PRIVATE KEY", "BEGIN DSA PRIVATE KEY", "BEGIN EC PRIVATE KEY", "BEGIN OPENSSH PRIVATE KEY"]
+keywords = ["PRIVATE KEY"]
 
 [[detectors]]
 name = "PrivateKeyDetector"
-pattern = "(?s)-----BEGIN (RSA|DSA|EC|PGP)? PRIVATE KEY-----\\n[\\s\\S]*?-----END (RSA|DSA|EC|PGP)? PRIVATE KEY-----"
+pattern = "(?s)-----BEGIN (?:(?:RSA|DSA|EC|PGP|OPENSSH|ENCRYPTED) )?PRIVATE KEY-----[\\s\\S]*?-----END (?:(?:RSA|DSA|EC|PGP|OPENSSH|ENCRYPTED) )?PRIVATE KEY-----"
 finding_type = "Private Key Content"
 severity = "HIGH"
 
@@ -71,13 +71,21 @@ severity = "LOW"
 
 [[detectors]]
 name = "PhoneNumberDetector"
-pattern = "\\b(\\+?1\\s*)?(\\(\\d{3}\\)|\\d{3})[-.\\s]?\\d{3}[-.\\s]?\\d{4}\\b"
+# Requires punctuation or a country code: an unseparated 10-digit run is a
+# unix timestamp or numeric id far more often than a phone number.
+pattern = "(?:\\+1[-.\\s]?)?(?:\\(\\d{3}\\)[-.\\s]*|\\b\\d{3}[-.\\s])\\d{3}[-.\\s]?\\d{4}\\b"
 finding_type = "Phone Number"
 severity = "LOW"
 
 [[detectors]]
 name = "CreditCardDetector"
-pattern = "\\b(?:\\d[ -]*?){13,16}\\b"
+# Issuer prefixes (Visa/Mastercard/Amex/Discover/UnionPay) in 4-digit groups.
+# The old "any 13-16 digits with arbitrary spaces" shape matched commit
+# hashes, timestamps and ids, and even spanned the gap between two unrelated
+# numbers ("index aabbcc0..1111111 100644"). Luhn on its own does not fix
+# that: roughly one in ten random digit runs satisfies it.
+pattern = "\\b(?:4\\d{3}|5[1-5]\\d{2}|2[2-7]\\d{2}|6011|3[47]\\d{2})[ -]?\\d{4}[ -]?\\d{4}[ -]?\\d{1,4}\\b"
+validate = "luhn"
 finding_type = "Credit Card Number"
 severity = "HIGH"
 
@@ -134,7 +142,15 @@ name = "HighEntropyDetector"
 pattern = "\\b[a-f0-9]{48,}\\b|\\b[A-Fa-f0-9]{48,}\\b"
 finding_type = "High Entropy String"
 severity = "MEDIUM"
-entropy = 4.0
+# Hex has 16 symbols, so entropy cannot exceed log2(16) = 4.0 and reaches it
+# only on a perfectly uniform string: at 4.0 this detector could never fire
+# (measured max over 20k random 64-char hex strings: 3.97).
+entropy = 3.6
+# A bare 64-char hex string is a SHA-256 digest as often as it is a 32-byte
+# token, and nothing about the content tells them apart — enabling this
+# detector without context produced 758 hits in one hashing-heavy repository.
+# The keyword prefilter restricts it to lines that name a credential.
+keywords = ["key", "secret", "token", "password", "credential", "auth", "bearer"]
 
 [[detectors]]
 name = "StripeAPIKeyDetector"

src/detector.rs

@@ -23,6 +23,10 @@ pub enum DetectorError {
         detector: String,
         source: ParseSeverityError,
     },
+    InvalidValidator {
+        detector: String,
+        source: ParseValidatorError,
+    },
 }
 
 impl fmt::Display for DetectorError {
@@ -49,6 +53,13 @@ impl fmt::Display for DetectorError {
                     detector, source
                 )
             }
+            DetectorError::InvalidValidator { detector, source } => {
+                write!(
+                    formatter,
+                    "invalid validator in detector '{}': {}",
+                    detector, source
+                )
+            }
         }
     }
 }
@@ -60,10 +71,66 @@ impl std::error::Error for DetectorError {
                 Some(source)
             }
             Self::InvalidSeverity { source, .. } => Some(source),
+            Self::InvalidValidator { source, .. } => Some(source),
         }
     }
 }
 
+/// Extra structural check a detector can require of its matches, for
+/// patterns whose shape alone is too permissive.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum ContentValidator {
+    /// Payment card numbers carry a Luhn check digit. Without it, a 13-16
+    /// digit pattern matches every commit hash fragment, timestamp and
+    /// numeric id in a codebase.
+    Luhn,
+}
+
+impl FromStr for ContentValidator {
+    type Err = ParseValidatorError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        match value.trim().to_lowercase().as_str() {
+            "luhn" => Ok(Self::Luhn),
+            other => Err(ParseValidatorError {
+                value: other.to_string(),
+            }),
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct ParseValidatorError {
+    value: String,
+}
+
+impl fmt::Display for ParseValidatorError {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(formatter, "unknown validator '{}'", self.value)
+    }
+}
+
+impl std::error::Error for ParseValidatorError {}
+
+/// Luhn checksum, ignoring embedded separators.
+fn passes_luhn(matched: &str) -> bool {
+    let digits: Vec<u32> = matched.chars().filter_map(|c| c.to_digit(10)).collect();
+    if !(13..=19).contains(&digits.len()) {
+        return false;
+    }
+    let sum: u32 = digits
+        .iter()
+        .rev()
+        .enumerate()
+        .map(|(index, digit)| match index % 2 {
+            1 if *digit > 4 => digit * 2 - 9,
+            1 => digit * 2,
+            _ => *digit,
+        })
+        .sum();
+    sum % 10 == 0
+}
+
 pub struct Detector {
     pub name: String,
     pub regex: Regex,
@@ -72,6 +139,7 @@ pub struct Detector {
     pub allowlist: Vec<Regex>,
     pub keywords: Vec<String>,
     pub entropy_threshold: Option<f64>,
+    pub validator: Option<ContentValidator>,
 }
 
 impl Detector {
@@ -116,9 +184,25 @@ impl Detector {
                 .map(|keyword| keyword.to_lowercase())
                 .collect(),
             entropy_threshold,
+            validator: None,
         })
     }
 
+    /// Attaches a structural validator. Kept separate from `new` so adding a
+    /// check does not touch every construction site.
+    pub fn with_validator(mut self, validator: Option<ContentValidator>) -> Self {
+        self.validator = validator;
+        self
+    }
+
+    /// Whether a match satisfies the detector's structural validator.
+    pub fn passes_validation(&self, matched: &str) -> bool {
+        match self.validator {
+            Some(ContentValidator::Luhn) => passes_luhn(matched),
+            None => true,
+        }
+    }
+
     /// `lowercase_content` must already be lowercased. Keywords are stored
     /// lowercased at construction so callers can lowercase once per line
     /// instead of once per detector.
@@ -168,6 +252,7 @@ struct DetectorConfig {
     allowlist: Option<Vec<String>>,
     keywords: Option<Vec<String>>,
     entropy: Option<f64>,
+    validate: Option<String>,
 }
 
 fn find_detectors_config(include_repository_config: bool) -> Option<std::path::PathBuf> {
@@ -239,6 +324,15 @@ fn initialize_detectors_from_config(
         .map(|detector_config| {
             let allowlist = detector_config.allowlist.as_deref().unwrap_or_default();
             let keywords = detector_config.keywords.as_deref().unwrap_or_default();
+            let validator = detector_config
+                .validate
+                .as_deref()
+                .map(ContentValidator::from_str)
+                .transpose()
+                .map_err(|source| DetectorError::InvalidValidator {
+                    detector: detector_config.name.clone(),
+                    source,
+                })?;
             Detector::new(
                 &detector_config.name,
                 &detector_config.pattern,
@@ -248,6 +342,7 @@ fn initialize_detectors_from_config(
                 keywords,
                 detector_config.entropy,
             )
+            .map(|detector| detector.with_validator(validator))
         })
         .collect::<Result<Vec<_>, _>>()
         .map_err(|source| DetectorInitError::InvalidDetector { source })

src/scanner.rs

@@ -149,6 +149,7 @@ fn scan_line_detectors(
         for mat in detector.regex.find_iter(line) {
             if !is_allowlisted(mat.as_str(), detector)
                 && detector.has_sufficient_entropy(mat.as_str())
+                && detector.passes_validation(mat.as_str())
             {
                 findings.push(Finding {
                     file_path: path.to_string(),
@@ -187,6 +188,7 @@ fn scan_multiline_chunk(
                 if !line_is_suppressed
                     && !is_allowlisted(mat.as_str(), detector)
                     && detector.has_sufficient_entropy(mat.as_str())
+                    && detector.passes_validation(mat.as_str())
                 {
                     findings.push(Finding {
                         file_path: path.to_string(),