From c520d57d5b85bcaa3f07b9b449a848f13e3ffc77 Mon Sep 17 00:00:00 2001
From: ajianaz <ajianaz@users.noreply.github.com>
Date: Thu, 23 Jul 2026 20:29:34 +0700
Subject: [PATCH] fix: suppress false positives in sec-hardcoded-url and
 crypto/hardcoded-secret (#357, #364)

- Extend post_match_filter with SVG xmlns, xlink:href, Docker hostnames,
  comments, docstrings, loopback, empty values, Svelte bindings, variable refs
- Wire post_match_filter into security_scanner.rs scan loop
- Add 17 unit + integration tests

Closes #357. Closes #364.
---
 src/engine/rules/builtin.rs    | 284 ++++++++++++++++++++++++++++++++-
 src/engine/security_scanner.rs |  62 +++++++
 2 files changed, 339 insertions(+), 7 deletions(-)

diff --git a/src/engine/rules/builtin.rs b/src/engine/rules/builtin.rs
index 96011cb..bc63410 100644
--- a/src/engine/rules/builtin.rs
+++ b/src/engine/rules/builtin.rs
@@ -127,13 +127,283 @@ pub fn builtin_rules() -> Vec<CustomRule> {
 /// Returns `true` to suppress a finding that the regex matched but should be ignored.
 pub fn post_match_filter(rule_id: &str, line: &str) -> bool {
     match rule_id {
-        "sec-hardcoded-url" => {
-            let lower = line.to_lowercase();
-            lower.contains("http://localhost")
-                || lower.contains("http://127.0.0.1")
-                || lower.contains("http://0.0.0.0")
-                || lower.contains("http://[::1]")
-        }
+        "sec-hardcoded-url" => is_false_positive_url(line),
+        "crypto/hardcoded-secret" => is_false_positive_secret(line),
         _ => false,
     }
 }
+
+/// Check if an `http://` URL match is a false positive.
+///
+/// Suppresses: XML/SVG namespaces, Docker internal hostnames, comment/docstring lines,
+/// and other non-connection uses of `http://` URLs.
+fn is_false_positive_url(line: &str) -> bool {
+    let trimmed = line.trim();
+
+    // Skip lines that are comments or docstrings
+    let is_comment = trimmed.starts_with("//")
+        || trimmed.starts_with('#')
+        || trimmed.starts_with("<!--")
+        || trimmed.starts_with('*')
+        || trimmed.starts_with("///")
+        || trimmed.starts_with("//!")
+        || trimmed.contains("\"\"\"") // Python/Rust docstrings
+        || trimmed.contains("'''"  ); // Python docstrings
+    if is_comment {
+        return true;
+    }
+
+    let lower = line.to_lowercase();
+
+    // XML/SVG namespace URIs are identifiers, not network connections
+    if lower.contains("xmlns=") || lower.contains("xlink:href=") {
+        return true;
+    }
+
+    // Loopback addresses
+    if lower.contains("http://localhost")
+        || lower.contains("http://127.0.0.1")
+        || lower.contains("http://0.0.0.0")
+        || lower.contains("http://[::1]")
+    {
+        return true;
+    }
+
+    // Docker internal hostnames (no TLS needed for container-to-container on same host)
+    // Match http://<simple-hostname>:port pattern — no dots (not a public domain)
+    static DOCKER_HOST_RE: std::sync::LazyLock<regex::Regex> =
+        std::sync::LazyLock::new(|| regex::Regex::new(r"(?i)http://[a-z][\w-]*:\d+").unwrap());
+    if DOCKER_HOST_RE.is_match(line) {
+        // Only suppress if hostname has no dots (public domains have dots)
+        if let Some(caps) = DOCKER_HOST_RE.captures(line) {
+            let host = &caps[0];
+            // Extract hostname part between "http://" and ":"
+            if let Some(start) = host.find("//") {
+                let host_part = &host[start + 2..];
+                if let Some(colon) = host_part.find(':') {
+                    let name = &host_part[..colon];
+                    if !name.contains('.') {
+                        return true;
+                    }
+                }
+            }
+        }
+    }
+
+    false
+}
+
+/// Check if a `hardcoded-secret` match is a false positive.
+///
+/// Suppresses: empty string values, variable references (no literal secret),
+/// and UI binding patterns (Svelte $state, bind:value, form fields).
+fn is_false_positive_secret(line: &str) -> bool {
+    let lower = line.to_lowercase();
+
+    // Empty string or empty-ish values: = ''  = ""  = $state('')
+    if lower.contains("= ''") || lower.contains("= \"\"") || lower.contains("= $state('')") {
+        return true;
+    }
+
+    // UI/framework binding patterns — variable names containing "secret"/"password"
+    // that are form state, not actual credentials
+    if lower.contains("$state(") || lower.contains("bind:") {
+        return true;
+    }
+
+    // Object shorthand where the value is a variable reference, not a literal
+    // e.g., { app_secret: formAppSecret } — RHS is a variable name, not a secret value
+    // Variable references: no quotes, no digits mixed with special chars
+    if let Some(colon_pos) = line.find(':') {
+        let after_colon = line[colon_pos + 1..].trim();
+        // If the RHS is a bare identifier (variable reference), it's not a hardcoded secret
+        if !after_colon.is_empty()
+            && !after_colon.starts_with('"')
+            && !after_colon.starts_with('\'')
+            && !after_colon.starts_with('`')
+            && after_colon
+                .chars()
+                .next()
+                .is_some_and(|c| c.is_alphabetic() || c == '_' || c == '$')
+        {
+            // Check that the rest is also identifier-like (no string literals)
+            let is_bare_identifier = after_colon
+                .split(|c: char| !c.is_alphanumeric() && c != '_' && c != '-' && c != '$')
+                .all(|part| {
+                    part.is_empty()
+                        || part
+                            .chars()
+                            .all(|c| c.is_alphanumeric() || c == '_' || c == '-' || c == '$')
+                });
+            if is_bare_identifier {
+                return true;
+            }
+        }
+    }
+
+    false
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // ─── sec-hardcoded-url false positive tests (issue #357, #364) ───
+
+    #[test]
+    fn url_svg_xmlns_is_false_positive() {
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            r#"<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">"#
+        ));
+    }
+
+    #[test]
+    fn url_xlink_href_is_false_positive() {
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            r#"<use xlink:href="http://www.w3.org/1999/xlink">"#
+        ));
+    }
+
+    #[test]
+    fn url_docker_hostname_no_dots_is_false_positive() {
+        assert!(post_match_filter("sec-hardcoded-url", "http://uteke:8767"));
+        assert!(post_match_filter("sec-hardcoded-url", "http://redis:6379"));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "http://postgres-db:5432"
+        ));
+    }
+
+    #[test]
+    fn url_docker_hostname_with_dots_is_not_suppressed() {
+        assert!(!post_match_filter(
+            "sec-hardcoded-url",
+            "http://example.com:8080"
+        ));
+        assert!(!post_match_filter(
+            "sec-hardcoded-url",
+            "http://api.staging.internal:3000"
+        ));
+    }
+
+    #[test]
+    fn url_in_comment_is_false_positive() {
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "// Default: http://example.com:8080"
+        ));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "# Server URL: http://127.0.0.1:8767"
+        ));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "<!-- See http://www.w3.org/TR/ -->"
+        ));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "/// Uses http://localhost for development"
+        ));
+    }
+
+    #[test]
+    fn url_in_docstring_is_false_positive() {
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "    \"\"\"...default: http://127.0.0.1:8767...\"\"\""
+        ));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "    '''UTEKE_SERVER_URL — http://uteke:8767'''"
+        ));
+    }
+
+    #[test]
+    fn url_public_domain_is_real_finding() {
+        assert!(!post_match_filter(
+            "sec-hardcoded-url",
+            "fetch('http://api.example.com/data')"
+        ));
+        assert!(!post_match_filter(
+            "sec-hardcoded-url",
+            "let url = 'http://evil.com/steal'"
+        ));
+    }
+
+    #[test]
+    fn url_loopback_still_suppressed() {
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "http://localhost:3000"
+        ));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "http://127.0.0.1:5432"
+        ));
+        assert!(post_match_filter(
+            "sec-hardcoded-url",
+            "http://0.0.0.0:8080"
+        ));
+    }
+
+    #[test]
+    fn unknown_rule_id_never_suppressed() {
+        assert!(!post_match_filter("some-other-rule", "http://evil.com"));
+    }
+
+    // ─── crypto/hardcoded-secret false positive tests (issue #357) ───
+
+    #[test]
+    fn secret_empty_string_is_false_positive() {
+        assert!(post_match_filter(
+            "crypto/hardcoded-secret",
+            "let formAppSecret = $state('');"
+        ));
+        assert!(post_match_filter(
+            "crypto/hardcoded-secret",
+            "let password = '';"
+        ));
+        assert!(post_match_filter(
+            "crypto/hardcoded-secret",
+            "let api_key = \"\";"
+        ));
+    }
+
+    #[test]
+    fn secret_svelte_state_is_false_positive() {
+        assert!(post_match_filter(
+            "crypto/hardcoded-secret",
+            "let formPassword = $state('default12345678');"
+        ));
+    }
+
+    #[test]
+    fn secret_bind_is_false_positive() {
+        assert!(post_match_filter(
+            "crypto/hardcoded-secret",
+            "<input bind:value={formSecret} />"
+        ));
+    }
+
+    #[test]
+    fn secret_variable_reference_is_false_positive() {
+        assert!(post_match_filter(
+            "crypto/hardcoded-secret",
+            "...(formAppSecret && { app_secret: formAppSecret })"
+        ));
+    }
+
+    #[test]
+    fn secret_actual_hardcoded_is_real_finding() {
+        assert!(!post_match_filter(
+            "crypto/hardcoded-secret",
+            "let password = supersecret12345"
+        ));
+        assert!(!post_match_filter(
+            "crypto/hardcoded-secret",
+            "const API_KEY = \"sk-abc123def456gh\""
+        ));
+    }
+}
diff --git a/src/engine/security_scanner.rs b/src/engine/security_scanner.rs
index dbea263..2dfbec9 100644
--- a/src/engine/security_scanner.rs
+++ b/src/engine/security_scanner.rs
@@ -10,6 +10,7 @@ use tracing::debug;
 
 use crate::engine::Severity;
 use crate::engine::diff_parser::{DiffLineType, FileChunk};
+use crate::engine::rules::builtin::post_match_filter;
 use crate::engine::rules::types::RuleFinding;
 
 // ─── Security patterns ───
@@ -142,6 +143,17 @@ pub fn scan_security(chunks: &[FileChunk], max_findings: usize) -> Vec<RuleFindi
 
                 for (rule_id, name, regex, severity) in COMPILED_PATTERNS.iter() {
                     if regex.is_match(&line.content) {
+                        // Apply post-match filter to suppress known false positives
+                        if post_match_filter(rule_id, &line.content) {
+                            debug!(
+                                rule = %rule_id,
+                                file = path,
+                                line = line_no,
+                                "security pattern suppressed by post-match filter"
+                            );
+                            continue;
+                        }
+
                         debug!(
                             rule = %rule_id,
                             file = path,
@@ -400,4 +412,54 @@ mod tests {
             "subprocess.run with shell=True should trigger"
         );
     }
+
+    // ─── False positive suppression via post_match_filter (issue #357) ───
+
+    #[test]
+    fn svg_xmlns_url_suppressed_in_security_scan() {
+        let chunks = vec![make_chunk(
+            "src/icon.svg",
+            &[r#"<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">"#],
+        )];
+        let findings = scan_security(&chunks, 10);
+        assert!(
+            findings.is_empty(),
+            "SVG xmlns should not trigger security findings"
+        );
+    }
+
+    #[test]
+    fn empty_secret_value_suppressed_in_security_scan() {
+        let chunks = vec![make_chunk(
+            "src/Form.svelte",
+            &["let formAppSecret = $state('');"],
+        )];
+        let findings = scan_security(&chunks, 10);
+        let secret_findings: Vec<_> = findings
+            .iter()
+            .filter(|f| f.rule_id == "crypto/hardcoded-secret")
+            .collect();
+        assert!(
+            secret_findings.is_empty(),
+            "Empty $state('') secret should not trigger"
+        );
+    }
+
+    #[test]
+    fn real_hardcoded_secret_still_detected_after_filter() {
+        let chunks = vec![make_chunk(
+            "src/config.py",
+            &["API_KEY = sk_live_abc123def456"],
+        )];
+        let findings = scan_security(&chunks, 10);
+        let secret_findings: Vec<_> = findings
+            .iter()
+            .filter(|f| f.rule_id == "crypto/hardcoded-secret")
+            .collect();
+        assert_eq!(
+            secret_findings.len(),
+            1,
+            "Real hardcoded secret should still be detected"
+        );
+    }
 }