refactor(scan): scanners take file lines; MCP snippet scan uses postprocess (#610)

CHANGELOG.md

@@ -19,8 +19,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.<language>` 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).
 

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();

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<Self> {
+        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> {
         Self::open_or_create(&std::env::current_dir()?)

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<String>) -> bool {
         || issue.also_matches.iter().any(|a| hit(a))
 }
 
-/// Drop findings suppressed by an inline `cora-ignore:` marker.
-pub fn apply(mut issues: Vec<ReviewIssue>, chunks: &[FileChunk]) -> Vec<ReviewIssue> {
+/// 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<ReviewIssue>, chunks: &[FileChunk]) -> Vec<ReviewIssue> {
+    let files: Vec<ScanFile<'_>> = 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<ReviewIssue>, files: &[ScanFile<'_>]) -> Vec<ReviewIssue> {
     // (file, line) -> lowercased rules suppressed there.
     let mut suppressed: HashMap<(String, u32), HashSet<String>> = 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);
             }
         }
     }

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;

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());

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<RuleFinding> {
+    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<RuleFinding> {
     if !config.enabled {
         debug!("rule engine is disabled");
         return Vec::new();
@@ -44,52 +52,39 @@ pub fn run_rules(chunks: &[FileChunk], config: &RulesConfig) -> Vec<RuleFinding>
 
     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(),
+                });
             }
         }
     }