fix(rules): detect Go short declarations, YAML/JSON keys and SQL PASSWORD literals (#618, #619, #620)

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **Hardcoded secrets in Go short declarations, YAML/JSON keys and SQL `PASSWORD` literals are now detected.** Go `password := "..."` (with or without a trailing comment) and `var apiKey string = "..."` (#618); unquoted YAML `password: hunter2hunter2` and JSON quoted keys `"password": "..."` / `"apiKey": "..."` (#619); SQL `CREATE USER app WITH PASSWORD '<literal>'` and `IDENTIFIED BY '<literal>'` (#620). References stay quiet: `${VAR}`/`$VAR`, `!tag`, `*alias`, `&anchor`, paths, `null`/`required`-style bare words, and SQL placeholders (`PASSWORD '%s'`). The LLM secret cross-check uses the same pattern, so LLM findings on these shapes are no longer dropped.
+
 ### Changed
 
 - **One shared post-processing module for `cora review` and `cora scan`.** The finding filter pipeline (LLM secret false-positive cross-check, Markdown code blocks, `ignore.rules`, inline `cora-ignore:`, context-line filter, scanner merge for scans) now lives in `engine::postprocess`, so a fix lands in one place. Internal refactor with no CLI or output change (#610).

src/engine/postprocess.rs

@@ -99,13 +99,6 @@ fn apply_llm_secret_fp_filter(
 ) -> Vec<ReviewIssue> {
     use crate::engine::diff_parser::DiffLineType;
 
-    // Lazy-compiled regex matching the built-in sec-hardcoded-secret pattern.
-    // Only triggers for actual value assignments like `api_key = "sk-..."`.
-    static RE_SECRET_LITERAL: std::sync::LazyLock<regex::Regex> = std::sync::LazyLock::new(|| {
-        regex::Regex::new(r#"(?i)(?:password|api_?key|token|secret)(?:\s*:\s*[&\w<>\[\].?|]+)?\s*=\s*(?:"[^"]+"|'[^']+')"#)
-            .expect("hardcoded secret regex must compile")
-    });
-
     // Keywords that indicate an LLM finding is about hardcoded secrets.
     static SECRET_KEYWORDS: &[&str] = &[
         "hardcoded password",
@@ -156,7 +149,7 @@ fn apply_llm_secret_fp_filter(
         let line_num = issue.line.unwrap_or(0);
         let key = (issue.file.clone(), line_num);
         if let Some(actual_line) = added_lines.get(&key) {
-            if !RE_SECRET_LITERAL.is_match(actual_line) {
+            if !crate::engine::rules::builtin::has_secret_literal(actual_line) {
                 debug!(
                     file = %issue.file,
                     line = line_num,
@@ -446,6 +439,58 @@ mod tests {
         assert_eq!(result.len(), 1, "actual hardcoded secret should be kept");
     }
 
+    /// LLM findings on the shapes added in #618-#620 are kept; SQL placeholders
+    /// and references are still dropped.
+    #[test]
+    fn secret_fp_filter_matches_go_yaml_json_sql_shapes() {
+        use crate::engine::diff_parser::*;
+
+        let cases: &[(&str, bool)] = &[
+            (r#"password := "hunter2hunter2""#, true),
+            (r#"var apiKey string = "abcd1234efgh5678""#, true),
+            ("password: hunter2hunter2", true),
+            (r#"  "password": "hunter2hunter2","#, true),
+            ("CREATE USER app WITH PASSWORD 'hunter2hunter2';", true),
+            ("CREATE USER app WITH PASSWORD '%s';", false),
+            ("password: required", false),
+            (r#"  "password": "${DB_PASSWORD}","#, false),
+        ];
+        for (line, keep) in cases {
+            let chunks = vec![FileChunk {
+                old_path: None,
+                new_path: Some("src/x.txt".to_string()),
+                language: "txt".to_string(),
+                chunks: vec![DiffHunk {
+                    old_start: 0,
+                    old_count: 0,
+                    new_start: 1,
+                    new_count: 1,
+                    header: "".to_string(),
+                    lines: vec![DiffLine {
+                        line_type: DiffLineType::Add,
+                        content: line.to_string(),
+                        old_line_no: None,
+                        new_line_no: Some(1),
+                    }],
+                }],
+                is_binary: false,
+                is_deleted: false,
+                is_new: false,
+            }];
+            let issues = vec![
+                ReviewIssue::new(
+                    "src/x.txt",
+                    Some(1),
+                    Severity::Critical,
+                    "Hardcoded password in config",
+                )
+                .with_type("security"),
+            ];
+            let kept = apply_llm_secret_fp_filter(issues, &chunks).len() == 1;
+            assert_eq!(kept, *keep, "{line}");
+        }
+    }
+
     #[test]
     fn secret_fp_filter_keeps_non_security_findings() {
         use crate::engine::diff_parser::*;

src/engine/rules/builtin.rs

@@ -11,8 +11,7 @@ pub fn builtin_rules() -> Vec<CustomRule> {
         // --- Security ---
         CustomRule {
             id: "sec-hardcoded-secret".to_string(),
-            pattern: r#"(?i)(?:password|api_?key|token|secret)(?:\s*:\s*[&\w<>\[\].?|]+)?\s*=\s*(?:"[^"]+"|'[^']+')"#
-                .to_string(),
+            pattern: secret_literal_pattern(),
             severity: Severity::Critical,
             message: "Possible hardcoded secret/credential detected. Use environment variables or a secrets manager.".to_string(),
             languages: vec!["all".to_string()],
@@ -126,6 +125,134 @@ pub fn builtin_rules() -> Vec<CustomRule> {
     ]
 }
 
+/// Regex source shared by the `sec-hardcoded-secret` rule and the LLM secret
+/// cross-check in `postprocess`, so a shape detected here is never dropped
+/// there as a false positive (#607, #618-#620). Alternatives:
+///
+/// 1. `name [: type] = "lit"` / `name := "lit"` (Go) / `"name": "lit"` (JSON
+///    quoted key); the key may be quoted.
+/// 2. Go `var name type = "lit"` (colon-free type between name and `=`).
+/// 3. SQL `PASSWORD '<lit>'` / `IDENTIFIED BY '<lit>'` (no `=`).
+pub(crate) fn secret_literal_pattern() -> String {
+    let kw = r"(?:password|api_?key|token|secret)";
+    let ty = r"[&*\w<>\[\].?|]+";
+    let lit = r#"(?:"[^"]+"|'[^']+')"#;
+    format!(
+        r#"(?i){kw}["']?(?:\s*:\s*{ty})?\s*(?::=|=|["']\s*:)\s*{lit}|(?i){kw}["']?\s+{ty}\s*=\s*{lit}|{SQL_PASSWORD_PATTERN}"#
+    )
+}
+
+/// SQL password literal without `=`: `... WITH PASSWORD 'x'`, `IDENTIFIED BY 'x'`.
+const SQL_PASSWORD_PATTERN: &str = r"(?i)\b(?:password|identified\s+by)\s+'([^']+)'";
+
+static SQL_PASSWORD_RE: LazyLock<Regex> =
+    LazyLock::new(|| Regex::new(SQL_PASSWORD_PATTERN).expect("sql password regex must compile"));
+
+static SECRET_LITERAL_RE: LazyLock<Regex> = LazyLock::new(|| {
+    Regex::new(&secret_literal_pattern()).expect("hardcoded secret regex must compile")
+});
+
+/// Does `line` show a hardcoded secret literal the deterministic detectors
+/// report? Used by the LLM cross-check so LLM findings on the same shapes are
+/// kept. A SQL placeholder (`PASSWORD '%s'`) is not a literal.
+pub(crate) fn has_secret_literal(line: &str) -> bool {
+    if let Some(c) = SQL_PASSWORD_RE.captures(line)
+        && is_sql_placeholder(&c[1])
+    {
+        return false;
+    }
+    if quoted_placeholder_value(line) {
+        return false;
+    }
+    SECRET_LITERAL_RE.is_match(line) || plain_scalar_secret(strip_trailing_comment(line))
+}
+
+/// A SQL literal that is a bind/format placeholder, not a password:
+/// `%s`, `%v`, `$1`, `?`, `:name`, `@name`, `{}` / `{pw}` / `${PW}`, `<password>`.
+fn is_sql_placeholder(lit: &str) -> bool {
+    let lit = lit.trim();
+    lit.starts_with(['$', ':', '@', '?'])
+        || lit.contains('%')
+        || (lit.contains('{') && lit.contains('}'))
+        || (lit.starts_with('<') && lit.ends_with('>'))
+}
+
+/// Value of a whole-line `key: value` entry (YAML, JSON, `- key: value`) whose
+/// key ends in a secret word. The line must be nothing but the entry (an
+/// optional `- ` bullet, an optional trailing comma), so object shorthand inside
+/// code (`{ password: formPassword }`, `pub password: String,`) never qualifies.
+fn secret_kv_value(line: &str) -> Option<&str> {
+    static KV_RE: LazyLock<Regex> = LazyLock::new(|| {
+        Regex::new(
+            r#"(?i)^\s*(?:-\s+)?["']?[\w.\-]*(?:password|passwd|pwd|secret|api_?key|token)["']?\s*:\s+(\S.*?)\s*,?\s*$"#,
+        )
+        .expect("kv regex must compile")
+    });
+    KV_RE
+        .captures(line)
+        .and_then(|c| c.get(1))
+        .map(|m| m.as_str())
+}
+
+/// Unquoted YAML scalar that looks like a real secret (#619). Conservative:
+/// a single token of >= 8 chars with a digit or symbol in it, so bare words
+/// (`required`, `formPassword`, `vault_db_password`) stay "references". Never
+/// a YAML tag/anchor/alias/block indicator, `${..}`/`$VAR`, a path, or a
+/// keyword (`true`, `null`, ...).
+fn plain_scalar_secret(line: &str) -> bool {
+    let Some(v) = secret_kv_value(line) else {
+        return false;
+    };
+    let lower = v.to_ascii_lowercase();
+    v.len() >= 8
+        && !v.contains(char::is_whitespace)
+        && !v.starts_with([
+            '"', '\'', '`', '$', '!', '&', '*', '|', '>', '%', '@', '{', '[', '/', '~', '<', '.',
+            '?', '-',
+        ])
+        && !v.contains("://")
+        && !v.contains(['{', '}', '(', ')', ';', '#'])
+        && !matches!(
+            lower.as_str(),
+            "true" | "false" | "null" | "none" | "nil" | "yes" | "no"
+        )
+        && v.chars()
+            .any(|c| c.is_ascii_digit() || "!@#%^&*+=".contains(c))
+}
+
+/// Quoted `key: "value"` entry whose value is empty or an interpolation /
+/// template placeholder (`${X}`, `$X`, `{{ x }}`): a reference, not a secret.
+fn quoted_placeholder_value(line: &str) -> bool {
+    // Unanchored on purpose: inline JSON (`{"password": "${X}", "apiKey": ""}`)
+    // and call arguments carry the pair in the middle of the line. Every quoted
+    // secret pair on the line must be a placeholder to suppress the finding, so a
+    // real literal next to a placeholder is still reported.
+    static PAIR_RE: LazyLock<Regex> = LazyLock::new(|| {
+        Regex::new(
+            r#"(?i)["']?[\w.\-]*(?:password|passwd|pwd|secret|api_?key|token)["']?\s*:\s*("[^"]*"|'[^']*')"#,
+        )
+        .expect("pair regex must compile")
+    });
+    let mut seen = false;
+    for cap in PAIR_RE.captures_iter(line) {
+        let v = &cap[1];
+        let inner = &v[1..v.len() - 1];
+        let placeholder = inner.is_empty()
+            || (inner.starts_with("${") && inner.ends_with('}'))
+            || (inner.starts_with("{{") && inner.ends_with("}}"))
+            || (inner.starts_with('$')
+                && inner.len() > 1
+                && inner[1..]
+                    .chars()
+                    .all(|c| c.is_ascii_alphanumeric() || c == '_'));
+        if !placeholder {
+            return false;
+        }
+        seen = true;
+    }
+    seen
+}
+
 /// Post-match filter for rules that need additional validation after regex match.
 /// 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 {
@@ -285,6 +412,29 @@ fn is_false_positive_secret(line: &str) -> bool {
         return true;
     }
 
+    // SQL placeholders in format strings: PASSWORD '%s', IDENTIFIED BY '$1' (#620)
+    if let Some(c) = SQL_PASSWORD_RE.captures(line)
+        && is_sql_placeholder(&c[1])
+    {
+        return true;
+    }
+
+    // `"password": "${X}"` / `password: "{{ x }}"`: a reference, not a secret (#619)
+    if quoted_placeholder_value(line) {
+        return true;
+    }
+
+    // YAML alias / anchor / tag / path / `~` reference (`password: *dbpass`,
+    // `password: /run/secrets/db`): points at another value, not a secret (#619)
+    if secret_kv_value(line).is_some_and(|v| v.starts_with(['*', '&', '!', '/', '~'])) {
+        return true;
+    }
+
+    // Unquoted YAML `password: hunter2hunter2` is a real secret, not shorthand (#619)
+    if plain_scalar_secret(line) {
+        return false;
+    }
+
     // 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

src/engine/secret_heuristics_tests.rs

@@ -141,6 +141,17 @@ const CASES: &[Row] = &[
     ("go", "const token = `abcd1234efgh5678`", true),
     ("go", r#"password := os.Getenv("DB_PASSWORD")"#, false),
     ("go", r#"password := """#, false),
+    // #618: short declarations and colon-free types
+    ("go", r#"password := "hunter2hunter2""#, true),
+    ("go", r#"var apiKey string = "abcd1234efgh5678""#, true),
+    ("go", r#"password := "hunter2hunter2" // note: temp"#, true),
+    (
+        "go",
+        r#"password := "hunter2hunter2" // cora-ignore: crypto/hardcoded-secret"#,
+        false,
+    ),
+    ("go", r#"var apiKey string = """#, false),
+    ("go", r#"var apiKey string = os.Getenv("API_KEY")"#, false),
     // ── java / kotlin ──
     (
         "java",
@@ -162,8 +173,44 @@ const CASES: &[Row] = &[
     ("yaml", "api_key: 'abcd1234efgh5678'", true),
     ("yaml", "password: ${DB_PASSWORD}", false),
     ("yaml", "password: \"\"", false),
+    // #619: unquoted YAML scalars
+    ("yaml", "password: hunter2hunter2", true),
+    ("yaml", "  db_password: hunter2hunter2", true),
+    ("yaml", "- token: abcd1234efgh5678", true),
+    ("yaml", "password: hunter2hunter2 # prod db", true),
+    ("yaml", "password: !secret db", false),
+    ("yaml", "password: !vault db_password_1", false),
+    ("yaml", "password: null", false),
+    ("yaml", "password: ~", false),
+    ("yaml", "password: required", false),
+    ("yaml", "password: *dbpass1", false),
+    ("yaml", "password: &dbpass1 hunter2", false),
+    ("yaml", "password: $DB_PASSWORD_1", false),
+    ("yaml", "password: \"${DB_PASSWORD}\"", false),
+    ("yaml", "password_file: /run/secrets/db_password", false),
+    ("yaml", "password: /run/secrets/db_password", false),
+    ("yaml", "secretName: db-credentials-secret", false),
+    ("ts", "  secret: Secret1Type", true),
+    // #619: quoted JSON keys
+    ("json", r#"  "password": "hunter2hunter2","#, true),
+    ("json", r#"  "apiKey": "abcd1234efgh5678""#, true),
+    ("json", r#"  "password": "hunter2hunter2" "#, true),
+    ("json", r#"  "apiKey": "${API_KEY}""#, false),
+    ("json", r#"  "password": "$DB_PASSWORD","#, false),
+    ("json", r#"  "password": "{{ db_password }}","#, false),
+    ("json", r#"  "password": null,"#, false),
     ("json", r#"  "password": "","#, false),
     ("json", r#"  "password": "${DB_PASSWORD}","#, false),
+    // inline JSON: placeholders stay quiet, a real literal beside one is still flagged
+    ("json", r#"{"password": "${X}"}"#, false),
+    ("json", r#"{"password": "${X}", "apiKey": ""}"#, false),
+    ("json", r#"{"password": "", "token": "${T}"}"#, false),
+    (
+        "json",
+        r#"{"password": "${X}", "token": "hunter2hunter2xx"}"#,
+        true,
+    ),
+    ("json", r#"{"password": "hunter2hunter2xx"}"#, true),
     // ── env / .properties ──
     ("env", "DB_PASSWORD=hunter2hunter2", true),
     ("env", "API_KEY=abcd1234efgh5678", true),
@@ -186,6 +233,36 @@ const CASES: &[Row] = &[
         true,
     ),
     ("sql", "ALTER ROLE app SET password = '';", false),
+    // #620: PASSWORD '<literal>' without `=`
+    (
+        "sql",
+        "CREATE USER app WITH PASSWORD 'hunter2hunter2';",
+        true,
+    ),
+    ("sql", "ALTER USER app PASSWORD 'hunter2hunter2';", true),
+    (
+        "sql",
+        "CREATE USER app IDENTIFIED BY 'hunter2hunter2';",
+        true,
+    ),
+    (
+        "sql",
+        "CREATE USER app WITH PASSWORD 'hunter2hunter2'; -- note: temp",
+        true,
+    ),
+    ("sql", "CREATE USER app WITH PASSWORD '';", false),
+    ("sql", "CREATE USER app WITH PASSWORD '%s';", false),
+    ("sql", "CREATE USER app WITH PASSWORD '$1';", false),
+    ("sql", "CREATE USER app WITH PASSWORD ':pw';", false),
+    ("sql", "CREATE USER app WITH PASSWORD '{password}';", false),
+    ("sql", "CREATE USER app IDENTIFIED BY '?';", false),
+    ("sql", "CREATE USER app WITH PASSWORD NULL;", false),
+    ("go", r#"q := "CREATE USER app WITH PASSWORD '%s'""#, false),
+    (
+        "py",
+        r#"q = f"CREATE USER app WITH PASSWORD '{pw}'""#,
+        false,
+    ),
 ];
 
 /// Rows exposing a real false negative/positive that is not fixed yet. Each
@@ -220,20 +297,6 @@ const KNOWN_GAPS: &[Row] = &[
     ("env", "DB_PASSWORD=${DB_PASSWORD_FROM_VAULT}", true),
     ("properties", "db.password=${DB_PASSWORD}", true),
     ("sh", r#"export DB_PASSWORD="$VAULT_DB_PASSWORD""#, true),
-    // KNOWN GAP #618 - FN: Go `:=` short declarations are never matched by the regexes (ideal: flagged)
-    ("go", r#"password := "hunter2hunter2""#, false),
-    ("go", r#"var apiKey string = "abcd1234efgh5678""#, false),
-    ("go", r#"password := "hunter2hunter2" // note: temp"#, false),
-    // KNOWN GAP #619 - FN: unquoted YAML scalar is mistaken for object shorthand; quoted JSON keys never match (ideal: flagged)
-    ("yaml", "password: hunter2hunter2", false),
-    ("json", r#"  "password": "hunter2hunter2","#, false),
-    ("json", r#"  "apiKey": "abcd1234efgh5678""#, false),
-    // KNOWN GAP #620 - FN: SQL `PASSWORD '<literal>'` (no `=`) is not matched (ideal: flagged)
-    (
-        "sql",
-        "CREATE USER app WITH PASSWORD 'hunter2hunter2';",
-        false,
-    ),
 ];
 
 #[test]