fix(rules): secret rules ignore name: value entries with non-literal values (#661)

CHANGELOG.md

@@ -41,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`crypto/hardcoded-secret` / `sec-hardcoded-secret` no longer flag `name: value` entries whose value is not a literal (#661).** In source files, `webhook_secret: env::var("X")`, `refresh_token: prev?.refresh_token`, `token: Option<&'static str>`, `"password": ACCOUNT_PW` and `"clientSecret": j["clientSecret"]` are references or types, and format placeholders (`api_key = "{}"`, `"%s"`) are not values. Quoted literals (`token: "ghp_..."`) and `=` assignments are judged as before. On two real codebases the Critical/Major findings dropped from 38 to 20 (all remaining ones are test fixtures, SQL and test URLs, tracked in #662/#663).
+
 - **Agent-mode review no longer hides capped findings without saying so.** `cora.review_prepare` listed every deterministic finding but `cora.review_finalize` showed only `rules_engine.max_findings` (default 5) of them with no explanation; found by running a real agent on a real project. `review_prepare` now reports `deterministic_findings_total` and the cap, and `review_finalize` / `scan_finalize` / `cora.review_diff` report `not_shown` (with a note).
 
 - **The MCP `cora.review_diff` tool no longer crashes the server (#647).** Every call panicked with "Cannot start a runtime from within a runtime" because the handler blocked on a second runtime from a tokio runtime thread. Handlers now go through `block_in_place`.

src/engine/rules/builtin.rs

@@ -284,6 +284,7 @@ fn quoted_placeholder_value(line: &str) -> bool {
         let v = &cap[1];
         let inner = &v[1..v.len() - 1];
         let placeholder = inner.is_empty()
+            || is_format_placeholder(inner)
             || (inner.starts_with("${") && inner.ends_with('}') && !shell_secret_default(inner))
             || (inner.starts_with("{{") && inner.ends_with("}}"))
             || (inner.starts_with('$')
@@ -361,12 +362,64 @@ fn secret_assign_rhs(line: &str) -> Option<&str> {
     Some(rest.trim())
 }
 
-/// A quoted value that is nothing but a reference: `${X}`, `$X`, `$(cmd)`.
+/// A format-string placeholder standing in for a value: `{}`, `{name}`,
+/// `{0}`, `{:?}`, `%s`, `%v`. Not a literal secret.
+fn is_format_placeholder(inner: &str) -> bool {
+    static RE: LazyLock<Regex> = LazyLock::new(|| {
+        // Digits only as a positional index (`{0}`): a name with digits inside
+        // braces (`{hunter2hunter2}`) is more likely a value than a template.
+        Regex::new(r"^(?:\{(?:\d+|[A-Za-z_][A-Za-z_]{0,31})?(?::[^{}]*)?\}|%[sdvq])$")
+            .expect("placeholder regex must compile")
+    });
+    RE.is_match(inner)
+}
+
+/// `name: value` entries (struct-literal fields, object/dict entries, type
+/// annotations) in SOURCE CODE whose value is not a quoted literal:
+/// `webhook_secret: env::var("X")`, `refresh_token: prev?.refresh_token`,
+/// `token: Option<&'static str>`, `"password": ACCOUNT_PW`. In code a bare
+/// expression after the colon is a reference or a type, never a secret value; a
+/// quoted literal (`token: "abc12345"`) is not covered here and stays flagged.
+/// Config-style files (YAML, TOML, ...) keep their own judgement.
+fn colon_value_is_not_a_literal(line: &str, path: &str) -> bool {
+    if rhs_style(path) != RhsStyle::Code {
+        return false;
+    }
+    static COLON_RE: LazyLock<Regex> = LazyLock::new(|| {
+        Regex::new(r#"(?i)(?:password|passwd|pwd|secret|api_?key|token)["']?\s*:"#)
+            .expect("colon regex must compile")
+    });
+    // EVERY entry on the line must be a non-literal: `{ token: id, password:
+    // "hunter2hunter2" }` has a real secret after a harmless first entry.
+    let mut entries = 0;
+    for m in COLON_RE.find_iter(line) {
+        let rest = &line[m.end()..];
+        // `a::token`, `x := ...` and `token:=` are not `name: value` entries.
+        if rest.starts_with([':', '=']) {
+            continue;
+        }
+        let rhs = rest.trim().trim_end_matches([';', ',']).trim_end();
+        if rhs.is_empty() || starts_with_literal(rhs) {
+            return false;
+        }
+        // An env read / wrapper with a literal fallback is judged on the
+        // literal: `secret: env::var("X").unwrap_or("hunter2hunter2".into())`.
+        if embedded_literal(rhs) == Some(true) {
+            return false;
+        }
+        entries += 1;
+    }
+    entries > 0
+}
+
+/// A quoted value that is nothing but a reference: `${X}`, `$X`, `$(cmd)`,
+/// or a format placeholder.
 fn is_pure_reference(inner: &str) -> bool {
-    (inner.starts_with("${")
-        && inner.ends_with('}')
-        && inner.matches("${").count() == 1
-        && !shell_secret_default(inner))
+    is_format_placeholder(inner)
+        || (inner.starts_with("${")
+            && inner.ends_with('}')
+            && inner.matches("${").count() == 1
+            && !shell_secret_default(inner))
         || (inner.starts_with("$(") && inner.ends_with(')'))
         || (inner.starts_with('$')
             && inner.len() > 1
@@ -474,7 +527,7 @@ fn embedded_literal(rhs: &str) -> Option<bool> {
 ///   files nothing changes.
 fn non_literal_rhs(line: &str, path: &str) -> bool {
     let Some(rhs) = secret_assign_rhs(line) else {
-        return false;
+        return colon_value_is_not_a_literal(line, path);
     };
     let rhs = rhs.trim_end_matches([';', ',']).trim_end();
     if rhs.is_empty() {
@@ -1387,4 +1440,148 @@ mod tests {
             assert!(!has_secret_literal(line, path), "{line}");
         }
     }
+
+    // ─── #661: `name: value` entries in code that are not secret literals ───
+
+    fn secret_fp(line: &str, path: &str) -> bool {
+        post_match_filter_for_path("crypto/hardcoded-secret", line, path)
+            && post_match_filter_for_path("sec-hardcoded-secret", line, path)
+    }
+
+    #[test]
+    fn colon_entries_with_non_literal_values_are_not_secrets() {
+        for (line, path) in [
+            (
+                r#"webhook_secret: env::var("POLAR_WEBHOOK_SECRET")"#,
+                "config.rs",
+            ),
+            (
+                "access_token: env::var(\"POLAR_ACCESS_TOKEN\")",
+                "config.rs",
+            ),
+            (
+                "refresh_token: typeof b.refresh_token === 'string' ? b.refresh_token : prev?.refresh_token,",
+                "oidc.ts",
+            ),
+            (
+                "fn with_auth(token: Option<&'static str>) -> Self {",
+                "mod.rs",
+            ),
+            ("admin_token: Option<&'static str>,", "mod.rs"),
+            (
+                "api_key: pick(\"UTEKE_EMBED_FALLBACK_API_KEY\", self.api_key),",
+                "lib.rs",
+            ),
+            ("\"RS_CLIENT_SECRET\": rs[\"clientSecret\"],", "gh_setup.py"),
+            (
+                "json_body={\"email\": ADMIN, \"password\": ACCOUNT_PW})",
+                "lib.py",
+            ),
+            (
+                "let ent = {\"clientSecret\": j[\"clientSecret\"]}",
+                "gh_setup.py",
+            ),
+        ] {
+            assert!(secret_fp(line, path), "should be suppressed: {line}");
+        }
+    }
+
+    #[test]
+    fn format_placeholders_are_not_secret_values() {
+        for line in [
+            "api_key = \"{}\"",
+            "api_key = \"{key}\"",
+            "token = \"%s\"",
+            "password = \"{0}\"",
+            "api_key: \"{}\",",
+        ] {
+            assert!(
+                secret_fp(line, "onboard.rs"),
+                "should be suppressed: {line}"
+            );
+        }
+    }
+
+    #[test]
+    fn real_secret_literals_in_colon_entries_are_still_flagged() {
+        for (line, path) in [
+            ("password: \"hunter2hunter2\",", "config.ts"),
+            ("api_key: 'sk_live_abcdefghijkl',", "client.js"),
+            ("const cfg = { token: \"ghp_abcdefghijklmnop\" };", "a.ts"),
+            ("password = \"hunter2hunter2\"", "app.py"),
+            ("let token: &str = \"abcdefgh12345678\";", "main.rs"),
+            ("api_key: \"k\".into(),", "lib.rs"), // short literal: left to test-code awareness (#663)
+        ] {
+            assert!(
+                !post_match_filter_for_path("crypto/hardcoded-secret", line, path),
+                "must stay flagged: {line}"
+            );
+        }
+    }
+
+    #[test]
+    fn rust_path_segments_named_like_secrets_are_not_entries() {
+        // `Token::secret: ...` style paths and `:=` are not `name: value` entries.
+        assert!(!colon_value_is_not_a_literal(
+            "let s = a::token::new();",
+            "x.rs"
+        ));
+        assert!(!colon_value_is_not_a_literal(
+            "token := \"abcdefgh12345678\"",
+            "main.go"
+        ));
+    }
+
+    #[test]
+    fn config_style_files_keep_their_own_judgement() {
+        // YAML: an unquoted value after the colon is still judged by the
+        // plain-scalar rule, not waved through by the code-only rule.
+        assert!(!colon_value_is_not_a_literal(
+            "password: hunter2hunter2",
+            "app.yaml"
+        ));
+    }
+
+    #[test]
+    fn a_harmless_first_entry_does_not_hide_a_later_real_secret() {
+        for (line, path) in [
+            (
+                "const c = { token: ident, password: \"hunter2hunter2\" };",
+                "a.ts",
+            ),
+            (
+                "Cfg { user: name, api_key: \"sk_live_abcdefghijkl\" }",
+                "a.rs",
+            ),
+        ] {
+            assert!(
+                !post_match_filter_for_path("crypto/hardcoded-secret", line, path),
+                "must stay flagged: {line}"
+            );
+        }
+    }
+
+    #[test]
+    fn env_reads_with_a_literal_fallback_stay_flagged_in_colon_entries() {
+        let line = "webhook_secret: env::var(\"X\").unwrap_or(\"hunter2hunter2\".into()),";
+        assert!(!post_match_filter_for_path(
+            "crypto/hardcoded-secret",
+            line,
+            "config.rs"
+        ));
+    }
+
+    #[test]
+    fn braced_values_with_digits_are_not_placeholders() {
+        assert!(!is_format_placeholder("{hunter2hunter2}"));
+        assert!(!is_format_placeholder("{Passw0rd!}"));
+        for ok in ["{}", "{0}", "{name}", "{api_key}", "{:?}", "{0:>8}", "%s"] {
+            assert!(is_format_placeholder(ok), "{ok}");
+        }
+        assert!(!post_match_filter_for_path(
+            "crypto/hardcoded-secret",
+            "password = \"{hunter2hunter2}\"",
+            "app.py"
+        ));
+    }
 }