Skip to content
Closed
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
89 changes: 89 additions & 0 deletions src/engine/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
}
Expand Down Expand Up @@ -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());
}
}
61 changes: 60 additions & 1 deletion src/engine/security_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ──
Expand Down Expand Up @@ -302,6 +305,62 @@ mod tests {
assert_eq!(findings.len(), 2);
}

#[test]
fn subprocess_run_with_literal_list_no_false_positive() {
// subprocess.run(cmd) where cmd is a variable (controlled list) should NOT trigger.
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() {
// subprocess.run(f"...") with f-string (dynamic interpolation) SHOULD trigger.
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() {
// subprocess.run(..., shell=True) SHOULD trigger.
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"
);
}

#[test]
fn sorts_by_severity() {
let chunks = vec![make_chunk(
Expand Down
Loading
Loading