diff --git a/CHANGELOG.md b/CHANGELOG.md index 5216df5..efaac0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **`--help` no longer prints `CORA_BASE_URL`, `CORA_MODEL` or `CORA_PROVIDER` values (#687).** A base URL can carry credentials (`user:pass@host`, `?key=...`) and the other two name internal deployments; like the API key, their `[env: ...]` values are hidden (clap `hide_env_values`). +- **`cora scan` is quieter and says what it found (#683).** The `qual-clone` hint (`.clone()` in Rust, 306 info findings on one workspace) is now opt-in via `rules_engine.clone_hints: true`. New `cora scan --min-severity ` hides lower findings in every output format and in the count (blocking is still decided by `hook.min_severity`), and the summary counts by severity: `found 457 issues (1 critical, 150 minor, 306 info)`. - **Models that reject `temperature` work (#701).** cora always sent `temperature` (default `0.0`); newer models such as `claude-opus-5-5` answer HTTP 400 "`temperature` is deprecated for this model" and the review ended as "could not complete". A 400 that names `temperature` is now retried once without it and remembered for the model for the rest of the process; every other 400 is returned unchanged, and providers that accept it keep the configured value. - **`crypto/hardcoded-secret` no longer flags `CancelToken::default()` and `review_prepare` matches `review_finalize` (#697).** A path such as `Token::default()` was read as `token: `; the value after a single `:` can no longer start with another `:`. `cora.review_prepare` now applies `ignore.rules`, inline `cora-ignore` markers and the context filter to `deterministic_findings`, so the agent no longer sees (and fixes) a finding that `review_finalize` would drop. - **`cora.scan_finalize` reports how much of the project the verdict covers (#685).** New optional input `batches_judged`; the result has `coverage` (`complete`, `partial`, or `unverified` when not declared), `total_batches`, `batches_judged`, `files_judged` and `files_total`. A partial run says `Judged 1/11 batches (3/23 files)` and `should_block` is null instead of false. The `/cora-review` skill now passes `batches_judged`. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 801d7bb..1c7b1d7 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -91,6 +91,7 @@ Complete command reference for the cora CLI. | `cora scan --exclude` `"vendor/**"` | Exclude glob patterns | | `cora scan --extensions` `"ts,js"` | Additional file extensions to scan | | `cora scan --incremental` | Scan only files changed since last scan | +| `cora scan --min-severity` `minor` | Hide findings below `info`, `minor`, `major` or `critical` in every output format and in the count (blocking still follows `hook.min_severity`) | | `cora scan --focus` `security` | Override focus areas | | `cora scan --batch-files` `N` | Max files per LLM batch (default: 20) | | `cora scan --no-continue-on-batch-error` | Abort on batch failure (default: skip and continue) | diff --git a/docs/configuration.md b/docs/configuration.md index b8cc097..147d4df 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -494,6 +494,7 @@ rules_engine: | `enabled` | `bool` | `true` | Enable/disable the rule engine | | `max_findings` | `int` | `50` | Max deterministic findings shown per run; Critical and Major are never hidden (`0` or `null` = unlimited). See below | | `include_tests` | `bool` | `false` | Scan test code like any other code (see "Test code" below) | +| `clone_hints` | `bool` | `false` | Run the `qual-clone` hint for Rust `.clone()` calls (info; noisy on idiomatic Rust, so off by default) | | `index_skip_files` | `[string]` | *(see below)* | Glob patterns for files to skip during index scanning | #### Test code diff --git a/src/commands/scan.rs b/src/commands/scan.rs index 5f68060..8650615 100644 --- a/src/commands/scan.rs +++ b/src/commands/scan.rs @@ -44,6 +44,33 @@ pub struct ScanOptions { /// When true (default), a failed batch is skipped with a warning and the /// rest of the scan continues. When false, a failed batch aborts the run. pub continue_on_batch_error: bool, + /// Hide findings below this severity (`info`, `minor`, `major`, + /// `critical`) in every output format and in the summary count. Blocking + /// is still decided by `hook.min_severity`. + pub min_severity: Option, +} + +/// ` (1 critical, 2 major, 306 info)`, or an empty string when there are no +/// findings, so a long list of info findings cannot hide a critical one. +fn severity_breakdown(issues: &[crate::engine::ReviewIssue]) -> String { + use crate::engine::types::Severity; + let parts: Vec = [ + Severity::Critical, + Severity::Major, + Severity::Minor, + Severity::Info, + ] + .into_iter() + .filter_map(|sev| { + let n = issues.iter().filter(|i| i.severity == sev).count(); + (n > 0).then(|| format!("{n} {}", sev.label().to_lowercase())) + }) + .collect(); + if parts.is_empty() { + String::new() + } else { + format!(" ({})", parts.join(", ")) + } } /// Execute the scan command. @@ -188,7 +215,6 @@ pub async fn execute_scan( if let Some(w) = dropped_warning(dropped, config) { eprintln!("{}", w.yellow()); } - let issue_count = all_issues.len(); // Scan lists every finding; only `hook.min_severity` decides blocking. let shaped = finalize::shape_issues( &mut all_issues, @@ -201,6 +227,13 @@ pub async fn execute_scan( }, ); let should_block = shaped.should_block; + // `--min-severity` only changes what is shown, never the blocking decision. + if let Some(min) = opts.min_severity.as_deref() { + let min = crate::engine::types::Severity::from_str_lossy(min); + all_issues.retain(|i| i.severity <= min); + } + let issue_count = all_issues.len(); + let by_severity = severity_breakdown(&all_issues); // A clean deterministic result is not an AI scan: say so in the summary. let no_judge_note = if matches!(judge, JudgeChoice::Deterministic) { @@ -214,10 +247,11 @@ pub async fn execute_scan( files_scanned: files.len(), lines_scanned: total_lines, summary: format!( - "Scanned {} files ({} lines), found {} issues{}.", + "Scanned {} files ({} lines), found {} issues{}{}.", files.len(), total_lines, issue_count, + by_severity, if dropped > 0 { format!( " ({dropped} more deterministic findings not shown, rules.max_findings cap)" @@ -542,6 +576,7 @@ mod tests { focus: vec![], batch_files: 0, continue_on_batch_error: true, + min_severity: None, }; let code = execute_scan(&config, &JudgeChoice::Llm(llm), &opts, OutputFormat::Json) .await @@ -662,6 +697,7 @@ mod tests { focus: vec![], batch_files: 0, continue_on_batch_error: true, + min_severity: None, }; let code = execute_scan( &config, diff --git a/src/config/schema.rs b/src/config/schema.rs index 9eb333f..eccf43d 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -653,6 +653,9 @@ pub struct RulesSection { /// `RulesConfig::include_tests`, #663). #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub include_tests: bool, + /// Run the `qual-clone` hint for Rust `.clone()` calls (off by default, #683). + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub clone_hints: bool, } /// File bundling configuration section for `.cora.yaml`. @@ -810,6 +813,7 @@ impl CoraFile { config.rules_config.enabled = re.enabled; config.rules_config.max_findings = re.max_findings; config.rules_config.include_tests = re.include_tests; + config.rules_config.clone_hints = re.clone_hints; if !re.custom.is_empty() { config.rules_config.custom_rules = re.custom.clone(); } @@ -1189,6 +1193,7 @@ provider: zai custom: Vec::new(), index_skip_files: vec!["my-generated/**".to_string()], include_tests: false, + clone_hints: false, }), ..Default::default() }; diff --git a/src/engine/rules/mod.rs b/src/engine/rules/mod.rs index d65d65f..c610d45 100644 --- a/src/engine/rules/mod.rs +++ b/src/engine/rules/mod.rs @@ -97,6 +97,10 @@ pub fn run_rules_in(files: &[ScanFile<'_>], config: &RulesConfig) -> Vec) { let b = a.clone(); }\n"); + let off = rule_ids(&entry, &default_config()); + assert!(!off.iter().any(|i| i.starts_with("qual-clone")), "{off:?}"); + let mut cfg = default_config(); + cfg.clone_hints = true; + let on = rule_ids(&entry, &cfg); + assert!(on.iter().any(|i| i.starts_with("qual-clone")), "{on:?}"); + } + fn rule(id: &str, pattern: &str, languages: &[&str]) -> CustomRule { CustomRule { id: id.into(), @@ -426,6 +441,7 @@ mod tests { custom_rules: Vec::new(), index_skip_files: super::types::default_index_skip_files(), include_tests: false, + clone_hints: false, } } diff --git a/src/engine/rules/types.rs b/src/engine/rules/types.rs index bb64456..4ccf80b 100644 --- a/src/engine/rules/types.rs +++ b/src/engine/rules/types.rs @@ -26,6 +26,8 @@ pub struct RulesConfig { /// high-confidence patterns on inline test blocks (#663). #[serde(default)] pub include_tests: bool, + /// Run the noisy `qual-clone` hint (`.clone()` in Rust). Off by default (#683). + pub clone_hints: bool, } /// Default skip patterns for index scanners — common false-positive sources. @@ -75,6 +77,7 @@ impl Default for RulesConfig { custom_rules: Vec::new(), index_skip_files: default_index_skip_files(), include_tests: false, + clone_hints: false, } } } diff --git a/src/main.rs b/src/main.rs index f92eba6..a5802c4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -386,6 +386,11 @@ enum Command { #[clap(long)] no_continue_on_batch_error: bool, + /// Hide findings below this severity in the output and the summary + /// count (blocking is still decided by `hook.min_severity`) + #[clap(long, value_name = "LEVEL", value_parser = ["info", "minor", "major", "critical"])] + min_severity: Option, + /// Deterministic checks only: never call an LLM (also the automatic /// behavior when no API key is configured) #[clap(long)] @@ -1248,6 +1253,7 @@ async fn main() -> Result<()> { focus, batch_files, no_continue_on_batch_error, + min_severity, no_llm, } => { cmd_scan( @@ -1261,6 +1267,7 @@ async fn main() -> Result<()> { focus, batch_files, continue_on_batch_error: !no_continue_on_batch_error, + min_severity, no_llm, }, ) @@ -1531,6 +1538,7 @@ struct ScanOpts { focus: Vec, batch_files: usize, continue_on_batch_error: bool, + min_severity: Option, no_llm: bool, } @@ -1769,6 +1777,7 @@ async fn cmd_scan(globals: &GlobalOptions, opts: ScanOpts) -> Result { focus: opts.focus, batch_files: opts.batch_files, continue_on_batch_error: opts.continue_on_batch_error, + min_severity: opts.min_severity, }; scan::execute_scan(&config, &judge, &scan_opts, format).await diff --git a/tests/scan_noise.rs b/tests/scan_noise.rs new file mode 100644 index 0000000..df1dd91 --- /dev/null +++ b/tests/scan_noise.rs @@ -0,0 +1,48 @@ +//! `cora scan` noise controls (#683): clone hints are opt-in, `--min-severity` +//! trims every output, and the summary counts findings by severity. +use std::process::Command; + +fn scan(format: &str, args: &[&str]) -> String { + let d = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(d.path().join("src")).unwrap(); + std::fs::write( + d.path().join("src/a.rs"), + "fn f(a: &Arc) { let b = a.clone(); }\nfn g() { let x = y.unwrap(); }\nlet password = \"hunter2hunter2\";\n", + ) + .unwrap(); + let out = Command::new(env!("CARGO_BIN_EXE_cora")) + .args(["scan", "--no-llm", "--format", format]) + .args(args) + .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_default_scan_does_not_list_clone_sites() { + let text = scan("compact", &[]); + assert!(!text.contains("qual-clone"), "{text}"); + assert!(text.contains("hardcoded-secret"), "{text}"); +} + +#[test] +fn the_summary_counts_findings_by_severity() { + let text = scan("json", &[]); + assert!(text.contains("1 critical"), "{text}"); + assert!(text.contains("minor"), "unwrap is minor: {text}"); +} + +#[test] +fn min_severity_hides_lower_findings_everywhere() { + let text = scan("compact", &["--min-severity", "major"]); + assert!(text.contains("hardcoded-secret"), "{text}"); + assert!( + !text.contains("bug-unwrap"), + "minor finding must be hidden: {text}" + ); + assert!(!text.contains("minor"), "{text}"); +}