fix(security): tighten command injection regex + add ignore rules logging

src/engine/review.rs

@@ -478,6 +478,7 @@ fn apply_ignore_rules(mut issues: Vec<ReviewIssue>, ignore_rules: &[String]) ->
         return issues;
     }
 
+    let before = issues.len();
     issues.retain(|issue| {
         !ignore_rules.iter().any(|pattern| {
             let pattern_lower = pattern.to_lowercase();
@@ -486,6 +487,15 @@ fn apply_ignore_rules(mut issues: Vec<ReviewIssue>, ignore_rules: &[String]) ->
                 || issue.title.to_lowercase().contains(&pattern_lower)
         })
     });
+    let filtered = before - issues.len();
+    if filtered > 0 {
+        debug!(
+            filtered,
+            remaining = issues.len(),
+            rules = ignore_rules.len(),
+            "filtered issues via ignore rules"
+        );
+    }
 
     issues
 }
@@ -695,4 +705,83 @@ mod tests {
             "unknown lines should be kept (better safe than sorry)"
         );
     }
+
+    #[test]
+    fn ignore_rules_filters_by_title_match() {
+        let issues = vec![
+            ReviewIssue {
+                file: "cli.rs".to_string(),
+                line: Some(236),
+                severity: Severity::Critical,
+                issue_type: Some("rule".to_string()),
+                title: "Command injection via exec/system with dynamic input".to_string(),
+                body: "Static security scanner detected...".to_string(),
+                suggested_fix: None,
+            },
+            ReviewIssue {
+                file: "main.rs".to_string(),
+                line: Some(10),
+                severity: Severity::Major,
+                issue_type: Some("security".to_string()),
+                title: "SQL injection via string concatenation".to_string(),
+                body: "...".to_string(),
+                suggested_fix: None,
+            },
+        ];
+
+        let rules = vec!["Command injection via exec/system with dynamic input".to_string()];
+        let result = apply_ignore_rules(issues, &rules);
+        assert_eq!(result.len(), 1);
+        assert_eq!(result[0].title, "SQL injection via string concatenation");
+    }
+
+    #[test]
+    fn ignore_rules_filters_by_issue_type_match() {
+        let issues = vec![ReviewIssue {
+            file: "test.py".to_string(),
+            line: Some(50),
+            severity: Severity::Minor,
+            issue_type: Some("style".to_string()),
+            title: "Some style issue".to_string(),
+            body: "...".to_string(),
+            suggested_fix: None,
+        }];
+
+        let rules = vec!["style".to_string()];
+        let result = apply_ignore_rules(issues, &rules);
+        assert!(result.is_empty());
+    }
+
+    #[test]
+    fn ignore_rules_empty_keeps_all() {
+        let issues = vec![ReviewIssue {
+            file: "f.rs".to_string(),
+            line: Some(1),
+            severity: Severity::Critical,
+            issue_type: Some("rule".to_string()),
+            title: "Any finding".to_string(),
+            body: "...".to_string(),
+            suggested_fix: None,
+        }];
+
+        let result = apply_ignore_rules(issues, &[]);
+        assert_eq!(result.len(), 1);
+    }
+
+    #[test]
+    fn ignore_rules_case_insensitive() {
+        let issues = vec![ReviewIssue {
+            file: "f.rs".to_string(),
+            line: Some(1),
+            severity: Severity::Critical,
+            issue_type: Some("rule".to_string()),
+            title: "HARDCODED password or SECRET in variable".to_string(),
+            body: "...".to_string(),
+            suggested_fix: None,
+        }];
+
+        let rules = vec!["Hardcoded Password Or Secret".to_string()];
+        let result = apply_ignore_rules(issues, &rules);
+        assert!(result.is_empty());
+    }
 }

src/engine/security_scanner.rs

@@ -63,7 +63,10 @@ pub static PATTERNS: &[SecurityPattern] = &[
     SecurityPattern {
         id: "injection/exec",
         name: "Command injection via exec/system with dynamic input",
-        regex: r"(?i)(?:exec|system|popen|subprocess\.(?:call|run))\s*\(",
+        // Only flag when there's a dynamic input signal on the same line
+        // (f-string, format(), string concat with +, shell=True, or raw user input).
+        // subprocess.run(["cmd", "arg"]) with literal list is safe and should not trigger.
+        regex: r#"(?i)(?:exec|system|popen|subprocess\.(?:call|run))\s*\((?:[^)]*(?:f"|f'|format\(|\.format\(|shell\s*=\s*True|\+\s*(?:req|request|input|params|data|user|query)|%s|%\(.*\)))"#,
         severity: Severity::Critical,
     },
     // ── Auth issues ──
@@ -312,4 +315,57 @@ mod tests {
         // Critical (hardcoded-secret) should come before Minor (debug)
         assert!(findings[0].severity >= findings[findings.len() - 1].severity);
     }
+
+    #[test]
+    fn subprocess_run_with_literal_list_no_false_positive() {
+        let chunks = vec![make_chunk(
+            "src/provider.py",
+            &["cmd = [self._bin, 'recall', query]", "subprocess.run(cmd)"],
+        )];
+        let findings = scan_security(&chunks, 10);
+        let exec_findings: Vec<_> = findings
+            .iter()
+            .filter(|f| f.rule_id == "injection/exec")
+            .collect();
+        assert!(
+            exec_findings.is_empty(),
+            "subprocess.run(cmd) with controlled list should not trigger"
+        );
+    }
+
+    #[test]
+    fn subprocess_run_with_fstring_triggers() {
+        let chunks = vec![make_chunk(
+            "src/handler.py",
+            &["subprocess.run(f'cat {user_input}')"],
+        )];
+        let findings = scan_security(&chunks, 10);
+        let exec_findings: Vec<_> = findings
+            .iter()
+            .filter(|f| f.rule_id == "injection/exec")
+            .collect();
+        assert_eq!(
+            exec_findings.len(),
+            1,
+            "subprocess.run with f-string should trigger"
+        );
+    }
+
+    #[test]
+    fn subprocess_run_with_shell_true_triggers() {
+        let chunks = vec![make_chunk(
+            "src/handler.py",
+            &["subprocess.run(cmd, shell=True)"],
+        )];
+        let findings = scan_security(&chunks, 10);
+        let exec_findings: Vec<_> = findings
+            .iter()
+            .filter(|f| f.rule_id == "injection/exec")
+            .collect();
+        assert_eq!(
+            exec_findings.len(),
+            1,
+            "subprocess.run with shell=True should trigger"
+        );
+    }
 }