From 15bdbeba59e19a96b755c9c3845f4ae3eba6dcdf Mon Sep 17 00:00:00 2001 From: ajianaz Date: Tue, 27 Oct 2026 15:40:42 +0700 Subject: [PATCH] feat(ignore): path-scoped ignore.rules entries (#680) Signed-off-by: ajianaz --- CHANGELOG.md | 1 + docs/configuration.md | 17 +++++ src/config/schema.rs | 99 +++++++++++++++++++++++++-- src/engine/postprocess.rs | 139 +++++++++++++++++++++++++++++++------- tests/scoped_ignore.rs | 43 ++++++++++++ 5 files changed, 270 insertions(+), 29 deletions(-) create mode 100644 tests/scoped_ignore.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5216df5..5b974f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Path-scoped `ignore.rules` (#680).** An entry can be an object, `- rule: bug-println` with `files: ["crates/cli/**"]` and an optional `reason`, to silence a rule in some directories only while it keeps firing elsewhere; plain strings still silence a rule everywhere. An empty `files` list is a config error. Works in `review`, `scan` and the MCP tools. See `docs/configuration.md`. - **Cora Review can use Anthropic workload identity federation (no stored API key).** The review job exchanges its GitHub OIDC token for a short-lived Anthropic access token (when `ANTHROPIC_FEDERATION_RULE_ID`, `ANTHROPIC_ORGANIZATION_ID`, `ANTHROPIC_SERVICE_ACCOUNT_ID` and `ANTHROPIC_WORKSPACE_ID` are set) and sends it only to `api.anthropic.com`; otherwise it falls back to `CORA_API_KEY`, `CORA_BASE_URL` and `CORA_MODEL`. A smoke test workflow verifies the exchange and a Bearer call to `/v1/chat/completions`. See `docs/examples.md`. - **Agents validate before they commit (#674).** A review of the staged diff, by an agent (`cora.review_finalize`) or by `cora review --staged`, now leaves a marker bound to that exact change set. `cora hook verify` reports it (exit 0 validated, 10 validated but blocking, 11 not validated) and `cora hook claude-guard` is a Claude Code `PreToolUse` hook that denies a `git commit` that is unreviewed, blocking, uses `--no-verify`, or is not a plain commit of the staged change. The pre-commit hook (now v2, upgraded in place by `cora hook install`) accepts a validated diff and otherwise falls back to `cora review --staged --ci --severity major`; a tooling error is reported as "not a pass". The `/cora-review` skill tells agents to review before every commit. See `docs/usage.md`. diff --git a/docs/configuration.md b/docs/configuration.md index b8cc097..5937ed6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -568,6 +568,23 @@ const next = compute(); - `cora review` reads markers from the diff (added and context lines); a marker outside the diff's context window is not seen. `cora scan` reads markers from the full file contents. - `cora scan` also runs the deterministic secrets and security scanners on every scanned file and applies `ignore.rules`, so those findings are reported even when the LLM call fails. +## Path-scoped ignore rules + +`ignore.rules` entries can be plain strings (the rule is silenced everywhere) or objects that silence a rule only under some paths: + +```yaml +ignore: + rules: + - "Hardcoded password or secret" # whole project + - rule: bug-println + files: ["crates/cli/**", "evals/**"] # only these paths + - rule: bug-expect + files: ["**/subprocess.rs"] + reason: "expects state a local invariant" # optional, for reviewers +``` + +`rule` is matched like a plain entry (rule id or title fragment). `files` uses the same globs as `ignore.files` and must not be empty (an empty list is a config error). It applies to `review`, `scan` and the MCP tools, before the `max_findings` cap. `reason` is documentation for the people reading the config. + ## Ignore Files Exclude files or directories from **all** cora operations — review, scan, and indexing. This is the broadest exclusion mechanism. diff --git a/src/config/schema.rs b/src/config/schema.rs index 9eb333f..f21bc94 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -90,6 +90,30 @@ pub struct ProviderConfig { pub struct IgnoreConfig { pub files: Vec, pub rules: Vec, + /// Rules silenced only for some paths (`ignore.rules` object entries, #680). + #[serde(default)] + pub scoped: Vec, +} + +/// One `ignore.rules` entry in object form: silence `rule` only under `files`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ScopedIgnore { + /// Rule id or title fragment, matched like a plain `ignore.rules` string. + pub rule: String, + /// Glob patterns (same syntax as `ignore.files`) the silencing is limited to. + pub files: Vec, + /// Why the rule is silenced here (documentation for reviewers; optional). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reason: Option, +} + +/// An `ignore.rules` entry: a plain string (whole project) or a scoped object. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum IgnoreRuleEntry { + Plain(String), + Scoped(ScopedIgnore), } /// Hook configuration. @@ -131,6 +155,7 @@ impl Default for Config { ".git/**".into(), ], rules: vec![], + scoped: vec![], }, hook: HookConfig { mode: "warn".to_string(), @@ -296,6 +321,19 @@ impl Config { )); } + // ── scoped ignore rules (#680) ── + for (i, s) in self.ignore.scoped.iter().enumerate() { + if s.rule.trim().is_empty() { + errs.push(format!("ignore.rules[{i}]: rule must not be empty")); + } + if s.files.is_empty() { + errs.push(format!( + "ignore.rules ({}): files is empty, so the rule would apply nowhere; list globs or use the plain string form", + s.rule + )); + } + } + // ── custom rules (#684) ── errs.extend(crate::engine::rules::validate_custom_rules( &self.rules_config.custom_rules, @@ -405,7 +443,7 @@ pub struct IgnoreSection { #[serde(skip_serializing_if = "Option::is_none")] pub files: Option>, #[serde(skip_serializing_if = "Option::is_none")] - pub rules: Option>, + pub rules: Option>, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -731,9 +769,16 @@ impl CoraFile { } } } - if let Some(v) = &ig.rules { - // ignore.rules defaults to empty, so clone is safe here. - config.ignore.rules.clone_from(v); + if let Some(entries) = &ig.rules { + // ignore.rules defaults to empty, so replacing is safe here. + config.ignore.rules.clear(); + config.ignore.scoped.clear(); + for entry in entries { + match entry { + IgnoreRuleEntry::Plain(rule) => config.ignore.rules.push(rule.clone()), + IgnoreRuleEntry::Scoped(s) => config.ignore.scoped.push(s.clone()), + } + } } } if let Some(h) = &self.hook { @@ -1098,7 +1143,7 @@ provider: zai let cora = CoraFile { ignore: Some(IgnoreSection { files: Some(vec!["vendor/**".to_string()]), - rules: Some(vec!["skip-rule-1".to_string()]), + rules: Some(vec![IgnoreRuleEntry::Plain("skip-rule-1".to_string())]), }), ..Default::default() }; @@ -1982,4 +2027,48 @@ bundling: let yaml = "\nllm:\n temperature: 0.5\n max_tokens: 8192\n"; assert!(CoraFile::from_str(yaml).is_ok()); } + + #[test] + fn ignore_rules_accept_strings_and_scoped_objects() { + let yaml = r#" +ignore: + rules: + - "Hardcoded password or secret" + - rule: bug-println + files: ["crates/cli/**", "evals/**"] + - rule: bug-expect + files: ["**/subprocess.rs"] + reason: "expects state a local invariant" +"#; + let cora: CoraFile = serde_yaml_ng::from_str(yaml).unwrap(); + let mut cfg = Config::default(); + cora.merge_into(&mut cfg).unwrap(); + assert_eq!(cfg.ignore.rules, vec!["Hardcoded password or secret"]); + assert_eq!(cfg.ignore.scoped.len(), 2); + assert_eq!(cfg.ignore.scoped[0].rule, "bug-println"); + assert_eq!( + cfg.ignore.scoped[0].files, + vec!["crates/cli/**", "evals/**"] + ); + assert_eq!( + cfg.ignore.scoped[1].reason.as_deref(), + Some("expects state a local invariant") + ); + assert!(cfg.validate().is_ok()); + } + + #[test] + fn a_scoped_ignore_without_files_is_a_config_error() { + let mut cfg = Config::default(); + cfg.ignore.scoped.push(ScopedIgnore { + rule: "bug-println".into(), + files: vec![], + reason: None, + }); + let err = cfg.validate().unwrap_err().to_string(); + assert!( + err.contains("bug-println") && err.contains("files is empty"), + "{err}" + ); + } } diff --git a/src/engine/postprocess.rs b/src/engine/postprocess.rs index c02609c..fbce856 100644 --- a/src/engine/postprocess.rs +++ b/src/engine/postprocess.rs @@ -84,7 +84,7 @@ pub fn postprocess_report( Source::Diff(chunks) => { let issues = apply_llm_secret_fp_filter(issues, chunks); let issues = apply_markdown_code_block_filter(issues, chunks); - let issues = apply_ignore_rules(issues, &config.ignore.rules); + let issues = apply_ignore_rules(issues, &config.ignore.rules, &config.ignore.scoped); // Rule-scoped inline suppression (`cora-ignore: `, #554), // shared by deterministic and LLM findings. let (issues, unmatched) = inline_suppress::apply(issues, chunks); @@ -97,7 +97,7 @@ pub fn postprocess_report( // below, after suppression (#624). let merged = deterministic::run_files(files, config).merge_into(issues); let scan_files = scan_input::from_entries(files); - let merged = apply_ignore_rules(merged, &config.ignore.rules); + let merged = apply_ignore_rules(merged, &config.ignore.rules, &config.ignore.scoped); let (kept, unmatched) = inline_suppress::apply_lines(merged, &scan_files); warn_unmatched(&unmatched); kept @@ -326,26 +326,48 @@ fn apply_markdown_code_block_filter( issues } -/// Filter out issues whose `issue_type` matches any ignored rule pattern. -fn apply_ignore_rules(mut issues: Vec, ignore_rules: &[String]) -> Vec { - if ignore_rules.is_empty() { +/// Does `pattern` (an `ignore.rules` string) name this issue? +fn ignore_pattern_matches(issue: &ReviewIssue, pattern: &str) -> bool { + let pattern_lower = pattern.to_lowercase(); + let issue_type_lower = issue.issue_type.clone().unwrap_or_default().to_lowercase(); + let exact = pattern.trim().to_lowercase(); + let id_hit = |s: &str| s.trim().to_lowercase() == exact; + issue_type_lower.contains(&pattern_lower) + || issue.title.to_lowercase().contains(&pattern_lower) + // Rule ids match exactly, including ids of scanner findings + // merged into this issue (#597). + || issue.rule_id.as_deref().is_some_and(id_hit) + || issue.also_matches.iter().any(|a| id_hit(a)) +} + +/// Filter out issues whose `issue_type` matches any ignored rule pattern, or +/// that match a scoped entry (`rule` + `files`) in a file under its globs (#680). +fn apply_ignore_rules( + mut issues: Vec, + ignore_rules: &[String], + scoped: &[crate::config::schema::ScopedIgnore], +) -> Vec { + if ignore_rules.is_empty() && scoped.is_empty() { return issues; } + let scoped: Vec<(&str, crate::engine::path_match::PathMatcher)> = scoped + .iter() + .map(|s| { + ( + s.rule.as_str(), + crate::engine::path_match::PathMatcher::new(&s.files), + ) + }) + .collect(); let before = issues.len(); issues.retain(|issue| { - !ignore_rules.iter().any(|pattern| { - let pattern_lower = pattern.to_lowercase(); - let issue_type_lower = issue.issue_type.clone().unwrap_or_default().to_lowercase(); - let exact = pattern.trim().to_lowercase(); - let id_hit = |s: &str| s.trim().to_lowercase() == exact; - issue_type_lower.contains(&pattern_lower) - || issue.title.to_lowercase().contains(&pattern_lower) - // Rule ids match exactly, including ids of scanner findings - // merged into this issue (#597). - || issue.rule_id.as_deref().is_some_and(id_hit) - || issue.also_matches.iter().any(|a| id_hit(a)) - }) + !ignore_rules + .iter() + .any(|pattern| ignore_pattern_matches(issue, pattern)) + && !scoped.iter().any(|(rule, files)| { + files.is_match(&issue.file) && ignore_pattern_matches(issue, rule) + }) }); let filtered = before - issues.len(); if filtered > 0 { @@ -932,7 +954,7 @@ mod tests { ]; let rules = vec!["Command injection via exec/system with dynamic input".to_string()]; - let result = apply_ignore_rules(issues, &rules); + let result = apply_ignore_rules(issues, &rules, &[]); assert_eq!(result.len(), 1); assert_eq!(result[0].title, "SQL injection via string concatenation"); } @@ -946,7 +968,7 @@ mod tests { ]; let rules = vec!["style".to_string()]; - let result = apply_ignore_rules(issues, &rules); + let result = apply_ignore_rules(issues, &rules, &[]); assert!(result.is_empty()); } @@ -958,7 +980,7 @@ mod tests { .with_body("..."), ]; - let result = apply_ignore_rules(issues, &[]); + let result = apply_ignore_rules(issues, &[], &[]); assert_eq!(result.len(), 1); } @@ -976,7 +998,7 @@ mod tests { ]; let rules = vec!["Hardcoded Password Or Secret".to_string()]; - let result = apply_ignore_rules(issues, &rules); + let result = apply_ignore_rules(issues, &rules, &[]); assert!(result.is_empty()); } @@ -1184,15 +1206,15 @@ mod tests { let mut a = ReviewIssue::new("a.rs", Some(1), Severity::Major, "Plain title") .with_type("rule") .with_rule_id("sec-hardcoded-secret"); - let kept = apply_ignore_rules(vec![a.clone()], &["SEC-Hardcoded-Secret".to_string()]); + let kept = apply_ignore_rules(vec![a.clone()], &["SEC-Hardcoded-Secret".to_string()], &[]); assert!(kept.is_empty()); // ids are exact, not substring - let kept = apply_ignore_rules(vec![a.clone()], &["hardcoded".to_string()]); + let kept = apply_ignore_rules(vec![a.clone()], &["hardcoded".to_string()], &[]); assert_eq!(kept.len(), 1); // merged-away scanner id a.rule_id = None; a.also_matches = vec!["sec-hardcoded-secret".to_string()]; - let kept = apply_ignore_rules(vec![a], &["sec-hardcoded-secret".to_string()]); + let kept = apply_ignore_rules(vec![a], &["sec-hardcoded-secret".to_string()], &[]); assert!(kept.is_empty()); } @@ -1416,4 +1438,73 @@ mod tests { let out = postprocess(issues, &Source::Diff(&chunks), &Config::default()); assert!(out.is_empty(), "got {out:?}"); } + + // ─── path-scoped ignore.rules (#680) ─── + + fn scoped(rule: &str, files: &[&str]) -> crate::config::schema::ScopedIgnore { + crate::config::schema::ScopedIgnore { + rule: rule.to_string(), + files: files.iter().map(|f| f.to_string()).collect(), + reason: None, + } + } + + fn issue_in(file: &str, rule_id: &str) -> ReviewIssue { + let mut i = ReviewIssue::new( + file.to_string(), + Some(1), + Severity::Minor, + format!("[{rule_id}] x"), + ); + i.rule_id = Some(rule_id.to_string()); + i + } + + #[test] + fn a_scoped_rule_is_silenced_only_under_its_globs() { + let issues = vec![ + issue_in("crates/cli/src/main.rs", "bug-println"), + issue_in("crates/core/src/lib.rs", "bug-println"), + issue_in("crates/cli/src/main.rs", "bug-unwrap"), + ]; + let kept = apply_ignore_rules(issues, &[], &[scoped("bug-println", &["crates/cli/**"])]); + let ids: Vec<(String, String)> = kept + .iter() + .map(|i| (i.file.clone(), i.rule_id.clone().unwrap())) + .collect(); + assert_eq!( + ids, + vec![ + ( + "crates/core/src/lib.rs".to_string(), + "bug-println".to_string() + ), + ( + "crates/cli/src/main.rs".to_string(), + "bug-unwrap".to_string() + ), + ] + ); + } + + #[test] + fn plain_strings_still_silence_a_rule_everywhere() { + let issues = vec![ + issue_in("a/x.rs", "bug-println"), + issue_in("b/y.rs", "bug-println"), + ]; + let kept = apply_ignore_rules(issues, &["bug-println".to_string()], &[]); + assert!(kept.is_empty()); + } + + #[test] + fn scoped_globs_support_double_star_and_file_names() { + let issues = vec![ + issue_in("pkg/deep/subprocess.rs", "bug-expect"), + issue_in("pkg/other.rs", "bug-expect"), + ]; + let kept = apply_ignore_rules(issues, &[], &[scoped("bug-expect", &["**/subprocess.rs"])]); + assert_eq!(kept.len(), 1); + assert_eq!(kept[0].file, "pkg/other.rs"); + } } diff --git a/tests/scoped_ignore.rs b/tests/scoped_ignore.rs new file mode 100644 index 0000000..615f077 --- /dev/null +++ b/tests/scoped_ignore.rs @@ -0,0 +1,43 @@ +//! `ignore.rules` entries in object form silence a rule under some paths only (#680). +use std::process::Command; + +fn scan(config: &str) -> String { + let d = tempfile::tempdir().unwrap(); + for dir in ["crates/cli/src", "crates/core/src"] { + std::fs::create_dir_all(d.path().join(dir)).unwrap(); + } + std::fs::write(d.path().join(".cora.yaml"), config).unwrap(); + for file in ["crates/cli/src/out.rs", "crates/core/src/lib.rs"] { + std::fs::write(d.path().join(file), "pub fn f() { println!(\"x\"); }\n").unwrap(); + } + let out = Command::new(env!("CARGO_BIN_EXE_cora")) + .args(["scan", "--no-llm", "--format", "compact"]) + .current_dir(d.path()) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("HOME", d.path()) + .output() + .unwrap(); + String::from_utf8_lossy(&out.stdout).to_string() + &String::from_utf8_lossy(&out.stderr) +} + +#[test] +fn a_rule_is_silenced_in_one_subtree_and_still_fires_in_another() { + let text = scan( + "ignore:\n rules:\n - rule: bug-println\n files: [\"crates/cli/**\"]\n reason: stdout is the cli interface\n", + ); + assert!( + text.contains("crates/core/src/lib.rs"), + "core must still fire: {text}" + ); + assert!( + !text.contains("crates/cli/src/out.rs"), + "cli must be silenced: {text}" + ); +} + +#[test] +fn the_plain_string_form_still_silences_everywhere() { + let text = scan("ignore:\n rules:\n - bug-println\n"); + assert!(!text.contains("bug-println"), "{text}"); +}