From 85e956105ff267107f9a15518c708d2866eac9ee Mon Sep 17 00:00:00 2001 From: ajianaz Date: Fri, 9 Oct 2026 14:19:07 +0700 Subject: [PATCH] refactor(scan): scanners take file lines; MCP snippet scan uses postprocess (#610) Add engine::scan_input::ScanFile as the single scanner input shape (path, language, numbered lines). secrets/security scanners, run_rules and the inline cora-ignore filter get *_in(&[ScanFile]) implementations; the diff entry points are thin adapters, files_as_chunks is removed. cora.check_snippet now runs through postprocess_report (ignore.rules, cora-ignore, FP filters, max_findings). Unit test opening an index session uses a per-test temp DB. brain_search_process sandboxes use unique temp dirs so concurrent test runs cannot delete each other's. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 5 + src/commands/watch.rs | 9 +- src/engine/index_bridge.rs | 8 ++ src/engine/inline_suppress.rs | 65 ++++++------ src/engine/mod.rs | 1 + src/engine/postprocess.rs | 11 ++- src/engine/rules/mod.rs | 83 ++++++++-------- src/engine/scan_input.rs | 132 +++++++++++++++++++++++++ src/engine/scanner.rs | 46 --------- src/engine/secrets_scanner.rs | 111 ++++++++++----------- src/engine/security_scanner.rs | 151 +++++++++++++--------------- src/index/session.rs | 8 ++ src/mcp/tools.rs | 175 ++++++++++++++++++++++++++------- tests/brain_search_process.rs | 17 ++-- 14 files changed, 514 insertions(+), 308 deletions(-) create mode 100644 src/engine/scan_input.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 73e3fde..095cac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Release checksums are signed with cosign keyless and verified by `install.sh` and `cora upgrade` (#591).** `release.yml` now signs `checksums-sha256.txt` (which covers every archive) via Sigstore/GitHub OIDC before creating the release and uploads `checksums-sha256.txt.sigstore.json`; a signing failure aborts the job so no unsigned release is published. `install.sh` and `cora upgrade` verify the bundle with `cosign verify-blob` (identity: this repo's `release.yml` on a `v*` tag, issuer `token.actions.githubusercontent.com`) when `cosign` is on `PATH`; an invalid signature aborts. Without `cosign`, or for releases without a bundle, they print a notice and fall back to checksum-only. `CORA_REQUIRE_SIGNATURE=1` makes a missing `cosign` or bundle fatal. See `docs/installation.md`. +### Changed + +- **Scanners take file lines instead of diffs (#610).** New `engine::scan_input::ScanFile` (`path`, `language`, `(line number, text)` pairs) is the one input shape for `secrets_scanner::scan_secrets_in`, `security_scanner::scan_security_in`, `rules::run_rules_in` and `inline_suppress::apply_lines`. Diff chunks and whole files are thin adapters (`ScanFile::added_lines`, `ScanFile::post_image`, `ScanFile::from_entry`); `scan_secrets`/`scan_security`/`run_rules`/`inline_suppress::apply` keep their signatures. The `files_as_chunks` workaround and the MCP fake diff are gone. Findings are byte-identical for diffs and scans. Unit tests that open an index session now use a per-test temp database instead of the shared global one. + ### Fixed +- **`cora.check_snippet` (MCP) now honors `ignore.rules`, inline `cora-ignore:` markers and the false-positive filters, and `rules_engine.max_findings` (#610).** The tool used to run the secrets and security scanners on a fake diff with a fixed cap of 10 per scanner and no filtering, so a snippet could report findings that `cora scan` suppresses. It now goes through the shared post-processing pipeline as a synthetic `snippet.` file (so path-aware classification applies), loads the project config like the other MCP tools (and fails with the same "Failed to load config" error), and mentions how many findings the cap dropped. Output format is unchanged. - **Secret-detection edge cases (#625, #628, #629, #630).** A string literal inside a constructor wrapper is now a literal: Java `new String("..")`, `Cow::Owned`, `Box::from`, `Secret::new`, Python `str("..")` (plain calls such as `get_password()` stay quiet) (#630). A literal default in the default position of an env read is flagged in code files: `os.environ.get("X", "lit")`, `os.getenv`, `process.env.X || "lit"` / `?? "lit"`, Rust `env::var("X").unwrap_or("lit".into())` / `unwrap_or_else(|_| "lit".to_string())`, Ruby `ENV.fetch`, PHP `getenv() ?:`; the env var name and name-only reads stay quiet (#628). Shell `${VAR:-literal}` / `${VAR:=literal}` is flagged, while `${VAR}`, `${VAR:-}` and `${VAR:?msg}` stay quiet (#629). The unquoted-YAML rule no longer applies to source code (`secret: Secret1Type` in `.ts`/`.py`), YAML flow-style `{ password: hunter2hunter2 }` is flagged, unspaced `password:=os.Getenv("X")` is no longer matched, and SQL `PASSWORD "..."` with double quotes is detected (#625). All-alphabetic unquoted YAML values (`password: correcthorsebattery`) remain undetected, tracked in #635. The LLM secret cross-check uses the same classification. - **`rules_engine.max_findings` is applied after suppression and truncation is reported (#624).** The cap (default 5) used to be applied inside each scanner, before `ignore.rules` / `cora-ignore`, so suppressed findings consumed slots and pushed real findings out, and the rest were dropped silently. Scanners now run uncapped and the cap is enforced once in post-processing on deterministic findings only (LLM issues are never capped), highest severity first. `cora scan` and `cora review` print `⚠ N more deterministic findings not shown (...)` on stderr when findings are cut, and the `cora scan` summary mentions it; stdout formats are unchanged. `max_findings: 0` or `null` now means unlimited (previously `0` hid every finding). diff --git a/src/commands/watch.rs b/src/commands/watch.rs index a68aecb..6e131e1 100644 --- a/src/commands/watch.rs +++ b/src/commands/watch.rs @@ -476,7 +476,14 @@ mod tests { fs::create_dir_all(root.join("src")).unwrap(); fs::write(root.join("src/a.rs"), "pub fn alpha() {}\n").unwrap(); fs::write(root.join("src/b.rs"), "pub fn beta() {}\n").unwrap(); - let session = IndexSession::open_at(root, ConfigSource::ProjectOnlyAt(root)).unwrap(); + // Per-test DB: never touches the shared global index. + let db_dir = TempDir::new().unwrap(); + let session = IndexSession::open_at_db( + &db_dir.path().join("cora.db"), + root, + ConfigSource::ProjectOnlyAt(root), + ) + .unwrap(); session.index(false).unwrap(); let pat = PathPattern::new("a.rs").unwrap(); let s = || scan_files(root, &None, Some(&pat), None).unwrap(); diff --git a/src/engine/index_bridge.rs b/src/engine/index_bridge.rs index 934abb8..1e293cf 100644 --- a/src/engine/index_bridge.rs +++ b/src/engine/index_bridge.rs @@ -108,6 +108,14 @@ impl IndexBridge { Self::from_connection(conn, start) } + /// [`Self::open_or_create`] against an explicit database file instead of + /// the process-global data dir (tests: a per-test temp DB, #610). + #[cfg(test)] + pub fn open_or_create_at(db_path: &Path, start: &Path) -> anyhow::Result { + let conn = crate::index::open_index_at(db_path)?; + Self::from_connection(conn, start) + } + /// [`Self::open_or_create`] for the current working directory. pub fn open_or_create_cwd() -> anyhow::Result { Self::open_or_create(&std::env::current_dir()?) diff --git a/src/engine/inline_suppress.rs b/src/engine/inline_suppress.rs index a7b46ee..a98b00f 100644 --- a/src/engine/inline_suppress.rs +++ b/src/engine/inline_suppress.rs @@ -20,7 +20,8 @@ //! the diff (more than the context window away from the finding) is not seen. use crate::engine::ReviewIssue; -use crate::engine::diff_parser::{DiffLineType, FileChunk}; +use crate::engine::diff_parser::FileChunk; +use crate::engine::scan_input::ScanFile; use std::collections::{HashMap, HashSet}; use tracing::debug; @@ -72,43 +73,41 @@ fn matches_any(issue: &ReviewIssue, rules: &HashSet) -> bool { || issue.also_matches.iter().any(|a| hit(a)) } -/// Drop findings suppressed by an inline `cora-ignore:` marker. -pub fn apply(mut issues: Vec, chunks: &[FileChunk]) -> Vec { +/// Drop findings suppressed by an inline `cora-ignore:` marker, reading the +/// post-change lines of a diff (diff adapter over [`apply_lines`]). +pub fn apply(issues: Vec, chunks: &[FileChunk]) -> Vec { + let files: Vec> = chunks.iter().map(ScanFile::post_image).collect(); + apply_lines(issues, &files) +} + +/// Drop findings suppressed by an inline `cora-ignore:` marker found in +/// `files`' lines. +pub fn apply_lines(mut issues: Vec, files: &[ScanFile<'_>]) -> Vec { // (file, line) -> lowercased rules suppressed there. let mut suppressed: HashMap<(String, u32), HashSet> = HashMap::new(); - for chunk in chunks { - let path = chunk - .new_path - .as_deref() - .or(chunk.old_path.as_deref()) - .unwrap_or(""); - for hunk in &chunk.chunks { - for line in &hunk.lines { - if line.line_type == DiffLineType::Remove { - continue; - } - let Some(ln) = line.new_line_no else { continue }; - let rules = parse_rules(&line.content); - if rules.is_empty() { - if line.content.to_lowercase().contains("cora-ignore") { - debug!( - file = path, - line = ln, - "bare cora-ignore ignored: rule list required" - ); - } - continue; + for file in files { + let path = file.path; + for &(ln, content) in &file.lines { + let rules = parse_rules(content); + if rules.is_empty() { + if content.to_lowercase().contains("cora-ignore") { + debug!( + file = path, + line = ln, + "bare cora-ignore ignored: rule list required" + ); } + continue; + } + suppressed + .entry((path.to_string(), ln)) + .or_default() + .extend(rules.iter().cloned()); + if is_comment_only(content) { suppressed - .entry((path.to_string(), ln)) + .entry((path.to_string(), ln + 1)) .or_default() - .extend(rules.iter().cloned()); - if is_comment_only(&line.content) { - suppressed - .entry((path.to_string(), ln + 1)) - .or_default() - .extend(rules); - } + .extend(rules); } } } diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 63ea8e5..599348f 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -21,6 +21,7 @@ pub mod quality_gate; pub mod review; pub mod review_store; pub mod rules; +pub mod scan_input; pub mod scanner; #[cfg(test)] mod secret_heuristics_tests; diff --git a/src/engine/postprocess.rs b/src/engine/postprocess.rs index b09c02e..22e620b 100644 --- a/src/engine/postprocess.rs +++ b/src/engine/postprocess.rs @@ -39,7 +39,8 @@ use tracing::debug; use crate::config::schema::Config; use crate::engine::diff_parser::FileChunk; -use crate::engine::scanner::{FileEntry, files_as_chunks}; +use crate::engine::scan_input; +use crate::engine::scanner::FileEntry; use crate::engine::types::{ReviewIssue, Severity}; use crate::engine::{inline_suppress, rules, secrets_scanner, security_scanner}; @@ -91,20 +92,20 @@ pub fn postprocess_report( apply_context_line_filter(issues, chunks) } Source::Files(files) => { - let chunks = files_as_chunks(files); + let scan_files = scan_input::from_entries(files); // Scanners run uncapped; the cap is enforced below, after // suppression (#624). let mut merged = issues; for family in [ - secrets_scanner::scan_secrets(&chunks, usize::MAX), - security_scanner::scan_security(&chunks, usize::MAX), + secrets_scanner::scan_secrets_in(&scan_files, usize::MAX), + security_scanner::scan_security_in(&scan_files, usize::MAX), ] { if !family.is_empty() { merged = rules::merge_rule_findings(merged, family); } } let merged = apply_ignore_rules(merged, &config.ignore.rules); - inline_suppress::apply(merged, &chunks) + inline_suppress::apply_lines(merged, &scan_files) } }; let (issues, dropped) = cap_deterministic(issues, config.rules_config.finding_cap()); diff --git a/src/engine/rules/mod.rs b/src/engine/rules/mod.rs index 900bc46..63228c5 100644 --- a/src/engine/rules/mod.rs +++ b/src/engine/rules/mod.rs @@ -6,8 +6,9 @@ pub mod types; use tracing::debug; -use crate::engine::diff_parser::{DiffLineType, FileChunk, parse_diff}; +use crate::engine::diff_parser::{FileChunk, parse_diff}; use crate::engine::rules::types::{RuleFinding, RulesConfig}; +use crate::engine::scan_input::{self, ScanFile}; use crate::engine::types::{ReviewIssue, Severity}; /// Map severity to a numeric rank for sorting (Critical=4, Major=3, Minor=2, Info=1). @@ -20,10 +21,17 @@ fn severity_rank(sev: Severity) -> u8 { } } -/// Run all rules (built-in + custom) against parsed diff chunks. +/// Run all rules (built-in + custom) against the added lines of parsed diff +/// chunks (diff adapter over [`run_rules_in`]). /// /// Returns findings capped by `config.max_findings`. pub fn run_rules(chunks: &[FileChunk], config: &RulesConfig) -> Vec { + run_rules_in(&scan_input::from_chunks(chunks), config) +} + +/// Run all rules (built-in + custom) against file lines. The one implementation +/// behind both the diff and the whole-file entry points. +pub fn run_rules_in(files: &[ScanFile<'_>], config: &RulesConfig) -> Vec { if !config.enabled { debug!("rule engine is disabled"); return Vec::new(); @@ -44,52 +52,39 @@ pub fn run_rules(chunks: &[FileChunk], config: &RulesConfig) -> Vec let mut findings = Vec::new(); - for file in chunks { - let file_path = file - .new_path - .as_deref() - .or(file.old_path.as_deref()) - .unwrap_or("unknown"); - - for hunk in &file.chunks { - for line in &hunk.lines { - // Only check added lines (new code being introduced) - if line.line_type != DiffLineType::Add { + for file in files { + let file_path = file.path; + + for &(line_no, content) in &file.lines { + for rule in &all_rules { + // Check language filter + if !matching::matches_language(rule, file.language) { continue; } - for rule in &all_rules { - // Check language filter - if !matching::matches_language(rule, &file.language) { - continue; - } - - // Check exclude filter - if matching::matches_exclude(rule, file_path) { - continue; - } - - // Check pattern match - if !matching::match_rule_against_line(rule, &line.content) { - continue; - } - - // Post-match filter (e.g., allow localhost URLs) - if builtin::post_match_filter_for_path(&rule.id, &line.content, file_path) { - continue; - } - - let line_no = line.new_line_no.unwrap_or(0); - - findings.push(RuleFinding { - rule_id: rule.id.clone(), - file: file_path.to_string(), - line: line_no, - severity: rule.severity, - title: format!("[{}] Rule: {}", rule.id, rule.id), - body: rule.message.clone(), - }); + // Check exclude filter + if matching::matches_exclude(rule, file_path) { + continue; } + + // Check pattern match + if !matching::match_rule_against_line(rule, content) { + continue; + } + + // Post-match filter (e.g., allow localhost URLs) + if builtin::post_match_filter_for_path(&rule.id, content, file_path) { + continue; + } + + findings.push(RuleFinding { + rule_id: rule.id.clone(), + file: file_path.to_string(), + line: line_no, + severity: rule.severity, + title: format!("[{}] Rule: {}", rule.id, rule.id), + body: rule.message.clone(), + }); } } } diff --git a/src/engine/scan_input.rs b/src/engine/scan_input.rs new file mode 100644 index 0000000..c6700e1 --- /dev/null +++ b/src/engine/scan_input.rs @@ -0,0 +1,132 @@ +//! Scanner input seam (#610): "the lines of a file", independent of how they +//! were obtained. +//! +//! The deterministic scanners (`secrets_scanner`, `security_scanner`, +//! `rules::run_rules`) and the inline `cora-ignore:` filter only need a path, a +//! language and `(line number, text)` pairs. [`ScanFile`] is that shape; the +//! diff and whole-file sources are thin adapters over it: +//! +//! - [`ScanFile::added_lines`]: the added lines of a diff chunk (what the +//! scanners looked at before; `cora review`). +//! - [`ScanFile::post_image`]: added + context lines of a diff chunk (what the +//! inline-suppression filter reads). +//! - [`ScanFile::from_entry`]: every line of a scanned file (`cora scan`, MCP +//! snippet check). + +use std::path::Path; + +use crate::engine::diff_parser::{DiffLineType, FileChunk}; +use crate::engine::scanner::FileEntry; + +/// One file's worth of lines to scan. +#[derive(Debug, Clone)] +pub struct ScanFile<'a> { + /// Path used in findings and for path-aware classification. + pub path: &'a str, + /// Language tag (file extension for files, the chunk language for diffs). + pub language: &'a str, + /// `(line number, text)`. Diff lines without a number carry `0`. + pub lines: Vec<(u32, &'a str)>, +} + +impl<'a> ScanFile<'a> { + /// Added lines of a diff chunk, in diff order. A missing path becomes + /// `"unknown"` and a missing new-side line number becomes `0`, exactly as + /// the scanners treated them before this seam existed. + pub fn added_lines(chunk: &'a FileChunk) -> Self { + let lines = chunk + .chunks + .iter() + .flat_map(|h| &h.lines) + .filter(|l| l.line_type == DiffLineType::Add) + .map(|l| (l.new_line_no.unwrap_or(0), l.content.as_str())) + .collect(); + Self { + path: chunk + .new_path + .as_deref() + .or(chunk.old_path.as_deref()) + .unwrap_or("unknown"), + language: chunk.language.as_str(), + lines, + } + } + + /// Post-change lines (added + context, with a new-side number) of a diff + /// chunk. A missing path becomes `""`. + pub fn post_image(chunk: &'a FileChunk) -> Self { + let lines = chunk + .chunks + .iter() + .flat_map(|h| &h.lines) + .filter(|l| l.line_type != DiffLineType::Remove) + .filter_map(|l| l.new_line_no.map(|n| (n, l.content.as_str()))) + .collect(); + Self { + path: chunk + .new_path + .as_deref() + .or(chunk.old_path.as_deref()) + .unwrap_or(""), + language: chunk.language.as_str(), + lines, + } + } + + /// Every line of a whole file, numbered from 1. The language is the file + /// extension (`""` when there is none). + pub fn from_entry(entry: &'a FileEntry) -> Self { + Self { + path: entry.path.as_str(), + language: Path::new(&entry.path) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or(""), + lines: entry + .content + .lines() + .zip(1u32..) + .map(|(t, n)| (n, t)) + .collect(), + } + } +} + +/// Added-line view of every chunk (diff adapter). +pub fn from_chunks(chunks: &[FileChunk]) -> Vec> { + chunks.iter().map(ScanFile::added_lines).collect() +} + +/// Whole-file view of every entry (file adapter). +pub fn from_entries(entries: &[FileEntry]) -> Vec> { + entries.iter().map(ScanFile::from_entry).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::diff_parser::parse_diff; + + #[test] + fn file_entry_is_numbered_from_one_with_extension_language() { + let e = FileEntry { + path: "src/a.py".into(), + content: "x = 1\ny = 2\n".into(), + lines: 2, + }; + let f = ScanFile::from_entry(&e); + assert_eq!(f.path, "src/a.py"); + assert_eq!(f.language, "py"); + assert_eq!(f.lines, vec![(1, "x = 1"), (2, "y = 2")]); + } + + #[test] + fn diff_adapters_split_added_from_post_image() { + let diff = "diff --git a/a.py b/a.py\n--- a/a.py\n+++ b/a.py\n@@ -1,2 +1,3 @@\n ctx\n-gone\n+added\n tail\n"; + let chunks = parse_diff(diff); + let added = ScanFile::added_lines(&chunks[0]); + assert_eq!(added.lines, vec![(2, "added")]); + let post = ScanFile::post_image(&chunks[0]); + assert_eq!(post.lines, vec![(1, "ctx"), (2, "added"), (3, "tail")]); + } +} diff --git a/src/engine/scanner.rs b/src/engine/scanner.rs index 5b1c1e3..60a515e 100644 --- a/src/engine/scanner.rs +++ b/src/engine/scanner.rs @@ -2,7 +2,6 @@ use std::collections::BTreeSet; use std::io::IsTerminal; use std::path::Path; -use crate::engine::diff_parser::{DiffHunk, DiffLine, DiffLineType, FileChunk}; use crate::engine::path_match::PathMatcher; use crate::error::CoraError; use ignore::WalkBuilder; @@ -20,51 +19,6 @@ pub struct FileEntry { pub lines: usize, } -/// View scanned files as synthetic "new file" diff chunks (every line added), -/// so the diff-based deterministic scanners and the inline `cora-ignore:` -/// filter can run on a full-file scan unchanged (#595). -pub fn files_as_chunks(files: &[FileEntry]) -> Vec { - files - .iter() - .filter(|f| !f.content.is_empty()) - .map(|f| { - let lines: Vec = f - .content - .lines() - .zip(1u32..) - .map(|(text, n)| DiffLine { - line_type: DiffLineType::Add, - content: text.to_string(), - old_line_no: None, - new_line_no: Some(n), - }) - .collect(); - let count = u32::try_from(lines.len()).unwrap_or(u32::MAX); - let language = Path::new(&f.path) - .extension() - .and_then(|e| e.to_str()) - .unwrap_or("") - .to_string(); - FileChunk { - old_path: None, - new_path: Some(f.path.clone()), - language, - chunks: vec![DiffHunk { - old_start: 0, - old_count: 0, - new_start: 1, - new_count: count, - header: String::new(), - lines, - }], - is_binary: false, - is_deleted: false, - is_new: true, - } - }) - .collect() -} - /// File extensions to include in scans by default (source code). const DEFAULT_EXTENSIONS: &[&str] = &[ "rs", "py", "js", "ts", "tsx", "jsx", "go", "java", "kt", "rb", "c", "cpp", "h", "hpp", "cs", diff --git a/src/engine/secrets_scanner.rs b/src/engine/secrets_scanner.rs index 4afc955..c29d718 100644 --- a/src/engine/secrets_scanner.rs +++ b/src/engine/secrets_scanner.rs @@ -5,8 +5,9 @@ use std::sync::LazyLock; use tracing::debug; use crate::engine::Severity; -use crate::engine::diff_parser::{DiffLineType, FileChunk}; +use crate::engine::diff_parser::FileChunk; use crate::engine::rules::types::RuleFinding; +use crate::engine::scan_input::{self, ScanFile}; // ─── Built-in secret patterns ─── @@ -107,80 +108,72 @@ static TEST_FIXTURE_RE: LazyLock = LazyLock::new(|| { // ─── Public API ─── -/// Scan diff chunks for known secret patterns. +/// Scan the added lines of diff chunks for known secret patterns (diff adapter +/// over [`scan_secrets_in`]). /// /// Returns findings capped at `max_findings`. Secret values are masked in the /// message body. pub fn scan_secrets(chunks: &[FileChunk], max_findings: usize) -> Vec { + scan_secrets_in(&scan_input::from_chunks(chunks), max_findings) +} + +/// Scan file lines for known secret patterns. The one implementation behind +/// both the diff and the whole-file entry points. +pub fn scan_secrets_in(files: &[ScanFile<'_>], max_findings: usize) -> Vec { let mut findings = Vec::new(); - for file in chunks { - let file_path = file - .new_path - .as_deref() - .or(file.old_path.as_deref()) - .unwrap_or("unknown"); + for file in files { + let file_path = file.path; // Test/spec/fixture/mock/example paths skip the generic rules (noisy // there) but still get the shared high-confidence list, since real // credentials are often committed in exactly those places (#579). let is_fixture = TEST_FIXTURE_RE.is_match(file_path); - for hunk in &file.chunks { - for line in &hunk.lines { - if line.line_type != DiffLineType::Add { - continue; - } - - let line_no = line.new_line_no.unwrap_or(0); - - if is_fixture { - if let Some((pat, matched)) = - crate::engine::secret_patterns::find_high_confidence(&line.content) - { - findings.push(RuleFinding { - rule_id: pat.id.to_string(), - file: file_path.to_string(), - line: line_no, - severity: Severity::Critical, - title: format!("[{}] {}", pat.id, pat.name), - body: format!( - "{} detected in a test/fixture path — verify it is a fake, \ - otherwise mask with environment variable. Matched: {}", - pat.name, - mask_secret(matched) - ), - }); - if findings.len() >= max_findings { - break; - } + for &(line_no, content) in &file.lines { + if is_fixture { + if let Some((pat, matched)) = + crate::engine::secret_patterns::find_high_confidence(content) + { + findings.push(RuleFinding { + rule_id: pat.id.to_string(), + file: file_path.to_string(), + line: line_no, + severity: Severity::Critical, + title: format!("[{}] {}", pat.id, pat.name), + body: format!( + "{} detected in a test/fixture path — verify it is a fake, \ + otherwise mask with environment variable. Matched: {}", + pat.name, + mask_secret(matched) + ), + }); + if findings.len() >= max_findings { + break; } - continue; } + continue; + } - for (re, pat) in COMPILED.iter() { - if let Some(m) = re.find(&line.content) { - let matched = m.as_str(); - findings.push(RuleFinding { - rule_id: pat.id.to_string(), - file: file_path.to_string(), - line: line_no, - severity: pat.severity, - title: format!("[{}] {}", pat.id, pat.name), - body: format!( - "{} detected — mask with environment variable. Matched: {}", - pat.name, - mask_secret(matched) - ), - }); - if findings.len() >= max_findings { - break; - } + for (re, pat) in COMPILED.iter() { + if let Some(m) = re.find(content) { + let matched = m.as_str(); + findings.push(RuleFinding { + rule_id: pat.id.to_string(), + file: file_path.to_string(), + line: line_no, + severity: pat.severity, + title: format!("[{}] {}", pat.id, pat.name), + body: format!( + "{} detected — mask with environment variable. Matched: {}", + pat.name, + mask_secret(matched) + ), + }); + if findings.len() >= max_findings { + break; } } - if findings.len() >= max_findings { - break; - } } if findings.len() >= max_findings { break; @@ -229,7 +222,7 @@ fn floor_boundary(s: &str, target: usize) -> usize { #[cfg(test)] mod tests { use super::*; - use crate::engine::diff_parser::{DiffHunk, DiffLine}; + use crate::engine::diff_parser::{DiffHunk, DiffLine, DiffLineType}; fn make_chunk(file: &str, added_lines: &[&str]) -> FileChunk { FileChunk { diff --git a/src/engine/security_scanner.rs b/src/engine/security_scanner.rs index a8ee71d..1622216 100644 --- a/src/engine/security_scanner.rs +++ b/src/engine/security_scanner.rs @@ -9,9 +9,10 @@ use std::sync::LazyLock; use tracing::debug; use crate::engine::Severity; -use crate::engine::diff_parser::{DiffLineType, FileChunk}; +use crate::engine::diff_parser::FileChunk; use crate::engine::rules::builtin::post_match_filter_for_path; use crate::engine::rules::types::RuleFinding; +use crate::engine::scan_input::{self, ScanFile}; // ─── Security patterns ─── @@ -160,19 +161,22 @@ static COMPILED_PATTERNS: LazyLock> = Laz .collect() }); -/// Run static security scan on diff chunks. +/// Run static security scan on the added lines of diff chunks (diff adapter +/// over [`scan_security_in`]). /// /// Only scans added lines (not removed/context) to reduce false positives. /// Returns findings sorted by severity (worst first). pub fn scan_security(chunks: &[FileChunk], max_findings: usize) -> Vec { + scan_security_in(&scan_input::from_chunks(chunks), max_findings) +} + +/// Run the static security scan over file lines. The one implementation behind +/// both the diff and the whole-file entry points. +pub fn scan_security_in(files: &[ScanFile<'_>], max_findings: usize) -> Vec { let mut findings = Vec::new(); - for chunk in chunks { - let path = chunk - .new_path - .as_deref() - .or(chunk.old_path.as_deref()) - .unwrap_or("unknown"); + for file in files { + let path = file.path; // Skip the noisy general rules for test/spec/fixture/mock/example files // and for documentation (security patterns are designed for source code, @@ -184,7 +188,7 @@ pub fn scan_security(chunks: &[FileChunk], max_findings: usize) -> Vec= max_findings { findings.sort_by_key(|f| std::cmp::Reverse(f.severity)); return findings; @@ -192,52 +196,45 @@ pub fn scan_security(chunks: &[FileChunk], max_findings: usize) -> Vec= max_findings { - findings.sort_by_key(|f| std::cmp::Reverse(f.severity)); - return findings; - } + continue; + } + + debug!( + rule = %rule_id, + file = path, + line = line_no, + "security pattern match" + ); + findings.push(RuleFinding { + rule_id: rule_id.clone(), + file: path.to_string(), + line: line_no, + severity: *severity, + title: name.clone(), + body: format!( + "Static security scanner detected: {} in {}:{}", + name, path, line_no + ), + }); + + if findings.len() >= max_findings { + findings.sort_by_key(|f| std::cmp::Reverse(f.severity)); + return findings; } } } @@ -248,41 +245,35 @@ pub fn scan_security(chunks: &[FileChunk], max_findings: usize) -> Vec, findings: &mut Vec, max_findings: usize, ) { - for hunk in &chunk.chunks { - for line in &hunk.lines { - if line.line_type != DiffLineType::Add { - continue; - } - let line_no = line.new_line_no.unwrap_or(0); - if line_no == 0 { - continue; - } - // Placeholder keys (e.g. AKIAIOSFODNN7EXAMPLE) are ignored inside. - if crate::engine::secret_patterns::find_high_confidence(&line.content).is_none() { - continue; - } - findings.push(RuleFinding { - rule_id: "secrets/high-confidence-in-test-or-doc".to_string(), - file: path.to_string(), - line: line_no, - severity: Severity::Major, - title: "Credential-like secret in test/doc file".to_string(), - body: format!( - "Static security scanner detected a credential-like value in {path}:{line_no}. \ - Test/doc paths are not exempt from secret scanning; verify it is a fake." - ), - }); - if findings.len() >= max_findings { - return; - } + let path = file.path; + for &(line_no, content) in &file.lines { + if line_no == 0 { + continue; + } + // Placeholder keys (e.g. AKIAIOSFODNN7EXAMPLE) are ignored inside. + if crate::engine::secret_patterns::find_high_confidence(content).is_none() { + continue; + } + findings.push(RuleFinding { + rule_id: "secrets/high-confidence-in-test-or-doc".to_string(), + file: path.to_string(), + line: line_no, + severity: Severity::Major, + title: "Credential-like secret in test/doc file".to_string(), + body: format!( + "Static security scanner detected a credential-like value in {path}:{line_no}. \ + Test/doc paths are not exempt from secret scanning; verify it is a fake." + ), + }); + if findings.len() >= max_findings { + return; } } } @@ -342,7 +333,7 @@ mod tests { use super::*; fn make_chunk(file: &str, added_lines: &[&str]) -> FileChunk { - use crate::engine::diff_parser::{DiffHunk, DiffLine}; + use crate::engine::diff_parser::{DiffHunk, DiffLine, DiffLineType}; FileChunk { old_path: None, new_path: Some(file.to_string()), diff --git a/src/index/session.rs b/src/index/session.rs index 804a086..a774237 100644 --- a/src/index/session.rs +++ b/src/index/session.rs @@ -121,6 +121,14 @@ impl IndexSession { Self::from_bridge(bridge, load_config(source).as_ref()) } + /// [`Self::open_at`] against an explicit database file instead of the + /// process-global data dir (tests: a per-test temp DB, #610). + #[cfg(test)] + pub fn open_at_db(db_path: &Path, start: &Path, source: ConfigSource<'_>) -> Result { + let bridge = IndexBridge::open_or_create_at(db_path, start)?; + Self::from_bridge(bridge, load_config(source).as_ref()) + } + /// Build a session from an already-open bridge and loaded config. pub fn from_bridge(bridge: IndexBridge, config: Option<&Config>) -> Result { let (conn, project_id, root) = bridge.into_strict_parts()?; diff --git a/src/mcp/tools.rs b/src/mcp/tools.rs index 00250cf..3e9b359 100644 --- a/src/mcp/tools.rs +++ b/src/mcp/tools.rs @@ -4,10 +4,10 @@ use std::sync::LazyLock; -use crate::engine::diff_parser; +use crate::engine::postprocess::{Source, postprocess_report}; use crate::engine::profiles; use crate::engine::rules; -use crate::engine::secrets_scanner; +use crate::engine::scanner::FileEntry; use crate::engine::security_scanner; use super::protocol::{Tool, ToolResult}; @@ -314,44 +314,64 @@ fn handle_check_snippet(params: &serde_json::Value) -> ToolResult { .and_then(|v| v.as_str()) .unwrap_or("unknown"); - // Create a fake diff chunk from the snippet - let fake_diff = format!( - "diff --git a/snippet.{lang} b/snippet.{lang}\n--- a/snippet.{lang}\n+++ b/snippet.{lang}\n@@ -0,0 +1,{count} @@\n{added}", - count = code.lines().count(), - added = code - .lines() - .map(|l| format!("+{l}")) - .collect::>() - .join("\n"), - ); + // Same project config the other tools use, so `ignore.rules` and + // `rules_engine.max_findings` apply exactly as in `cora scan` (#610). + let config = match load_project_config() { + Ok(c) => c, + Err(e) => return ToolResult::error(format!("Failed to load config: {e}")), + }; - let chunks = diff_parser::parse_diff(&fake_diff); + check_snippet_with(code, lang, &config) +} - let mut findings = Vec::new(); +/// Check `code` through the shared post-processing pipeline (scanners, +/// `ignore.rules`, inline `cora-ignore:` markers, finding cap) as a synthetic +/// file `snippet.`, so path-aware classification sees the snippet's +/// language/extension. +fn check_snippet_with( + code: &str, + lang: &str, + config: &crate::config::schema::Config, +) -> ToolResult { + // The language becomes a file extension: keep it a single safe path segment. + let ext: String = lang + .trim_start_matches('.') + .chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .collect(); + let entry = FileEntry { + path: format!("snippet.{ext}"), + content: code.to_string(), + lines: code.lines().count(), + }; - // Run secrets scanner - let secrets = secrets_scanner::scan_secrets(&chunks, 10); - findings.extend(secrets); + let report = postprocess_report( + Vec::new(), + &Source::Files(std::slice::from_ref(&entry)), + config, + ); - // Run security scanner - let security = security_scanner::scan_security(&chunks, 10); - findings.extend(security); + if report.issues.is_empty() { + return ToolResult::text("✅ No issues found in snippet by deterministic scanners."); + } - if findings.is_empty() { - ToolResult::text("✅ No issues found in snippet by deterministic scanners.") - } else { - let mut lines = vec![format!("Found {} issue(s):\n", findings.len())]; - for f in &findings { - lines.push(format!( - "- **{}** ({}): {} — {}", - f.rule_id, - f.severity.label(), - f.title, - f.body - )); - } - ToolResult::text(lines.join("\n")) + let mut lines = vec![format!("Found {} issue(s):\n", report.issues.len())]; + for f in &report.issues { + lines.push(format!( + "- **{}** ({}): {} — {}", + f.rule_id.as_deref().unwrap_or(""), + f.severity.label(), + f.title, + f.body + )); } + if report.dropped > 0 { + lines.push(format!( + "\n{} more deterministic finding(s) not shown (rules.max_findings = {}; raise it or set 0/null to show all)", + report.dropped, config.rules_config.max_findings + )); + } + ToolResult::text(lines.join("\n")) } fn handle_get_quality_gate() -> ToolResult { @@ -1148,6 +1168,93 @@ mod tests { assert!(result.content[0].text.contains("issue")); } + // ─── check_snippet through the shared pipeline (#610) ─── + // These call `check_snippet_with` with an explicit Config: no project + // config discovery, no data dir. + + fn snippet_text(code: &str, lang: &str, config: &crate::config::schema::Config) -> String { + let r = check_snippet_with(code, lang, config); + assert!(!r.is_error); + r.content[0].text.clone() + } + + const AWS: &str = "key = 'AKIAIOSFODNN7EXAMPLE'"; + + #[test] + fn snippet_reports_secret_with_rule_id() { + let cfg = crate::config::schema::Config::default(); + let text = snippet_text(AWS, "py", &cfg); + assert!(text.contains("Found 1 issue(s)"), "{text}"); + assert!(text.contains("secrets/aws-access-key"), "{text}"); + } + + #[test] + fn snippet_honors_ignore_rules() { + let mut cfg = crate::config::schema::Config::default(); + cfg.ignore.rules = vec!["secrets/aws-access-key".to_string()]; + let text = snippet_text(AWS, "py", &cfg); + assert!(text.contains("No issues"), "{text}"); + } + + #[test] + fn snippet_honors_inline_cora_ignore() { + let cfg = crate::config::schema::Config::default(); + let same_line = format!("{AWS} # cora-ignore: secrets/aws-access-key"); + assert!(snippet_text(&same_line, "py", &cfg).contains("No issues")); + let next_line = format!("# cora-ignore: secrets/aws-access-key\n{AWS}"); + assert!(snippet_text(&next_line, "py", &cfg).contains("No issues")); + // A marker naming another rule does not hide it. + let other = format!("{AWS} # cora-ignore: crypto/md5-password"); + assert!(snippet_text(&other, "py", &cfg).contains("Found 1 issue(s)")); + } + + #[test] + fn snippet_applies_false_positive_filters() { + let cfg = crate::config::schema::Config::default(); + for line in [ + "password = os.environ[\"DB_PASSWORD\"]", + "const apiKey: string = process.env.API_KEY;", + "password: ${DB_PASSWORD}", + ] { + assert!( + snippet_text(line, "py", &cfg).contains("No issues"), + "{line}" + ); + } + // A literal still fires. + let hit = snippet_text("password = \"hunter2hunter2\"", "py", &cfg); + assert!(hit.contains("Found"), "{hit}"); + } + + #[test] + fn snippet_path_is_derived_from_language() { + let cfg = crate::config::schema::Config::default(); + let text = snippet_text("let p = \"x\";\nlet h = md5(password);", "rs", &cfg); + // The synthetic path carries the extension (shown in scanner bodies). + assert!(text.contains("snippet.rs"), "{text}"); + // Hostile language values cannot escape the single path segment. + let text = snippet_text("let h = md5(password);", "../../etc/x", &cfg); + assert!(!text.contains("../"), "{text}"); + } + + #[test] + fn snippet_respects_max_findings_and_reports_dropped() { + let mut cfg = crate::config::schema::Config::default(); + cfg.rules_config.max_findings = 1; + let code = "a = 'AKIAIOSFODNN7EXAMPLE'\nb = 'AKIAIOSFODNN7EXAMPLF'"; + let text = snippet_text(code, "py", &cfg); + assert!(text.contains("Found 1 issue(s)"), "{text}"); + assert!( + text.contains("1 more deterministic finding(s) not shown"), + "{text}" + ); + + cfg.rules_config.max_findings = 0; // unlimited + let text = snippet_text(code, "py", &cfg); + assert!(text.contains("Found 2 issue(s)"), "{text}"); + assert!(!text.contains("not shown"), "{text}"); + } + #[test] fn handle_check_snippet_missing_code() { let result = handle_tool_call("cora.check_snippet", &serde_json::json!({})); diff --git a/tests/brain_search_process.rs b/tests/brain_search_process.rs index 20b853d..09d3f0c 100644 --- a/tests/brain_search_process.rs +++ b/tests/brain_search_process.rs @@ -12,9 +12,14 @@ fn cora_cmd() -> Command { } /// Isolated CODECORA_HOME + tiny project, so tests never touch real data. -fn sandbox(name: &str) -> (PathBuf, PathBuf) { - let root = std::env::temp_dir().join(format!("cora-545-{name}")); - let _ = std::fs::remove_dir_all(&root); +/// Each call gets a unique temp dir (dropped with the returned guard), so two +/// concurrent `cargo test` runs cannot delete each other's sandbox. +fn sandbox(name: &str) -> (tempfile::TempDir, PathBuf, PathBuf) { + let guard = tempfile::Builder::new() + .prefix(&format!("cora-545-{name}-")) + .tempdir() + .unwrap(); + let root = guard.path().to_path_buf(); let proj = root.join("proj"); std::fs::create_dir_all(&proj).unwrap(); std::fs::write( @@ -29,7 +34,7 @@ fn sandbox(name: &str) -> (PathBuf, PathBuf) { .unwrap(); let home = root.join("home"); std::fs::create_dir_all(&home).unwrap(); - (proj, home) + (guard, proj, home) } fn run(cora: &mut Command) -> String { @@ -39,7 +44,7 @@ fn run(cora: &mut Command) -> String { #[test] fn brain_fresh_process_emits_vector_signal() { - let (proj, home) = sandbox("usearch"); + let (_guard, proj, home) = sandbox("usearch"); // 1. Index in one process (default usearch backend). run(cora_cmd() @@ -67,7 +72,7 @@ fn brain_fresh_process_emits_vector_signal() { #[test] fn brain_vecq_backend_uses_own_extension() { - let (proj, home) = sandbox("vecq"); + let (_guard, proj, home) = sandbox("vecq"); std::fs::write(proj.join(".cora.yaml"), "brain:\n vector_store: vecq\n").unwrap();