Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
284 changes: 277 additions & 7 deletions src/engine/rules/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\""
));
}
}
62 changes: 62 additions & 0 deletions src/engine/security_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ───
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"
);
}
}
Loading