diff --git a/src/incremental.rs b/src/incremental.rs new file mode 100644 index 0000000..8a8523b --- /dev/null +++ b/src/incremental.rs @@ -0,0 +1,682 @@ +//! Incremental scans: upload whole project, analyze only what changed. +//! +//! The server already does this, but it derives the diff through the project's +//! SCM integration, which leaves out every project that integration cannot +//! answer for: zip-only projects, unreachable self-hosted hosts, unpushed +//! commits. This module diffs in the clone the scan already reads from. +//! +//! Runs by default, so it must be safe on a repo never set up for it. Every +//! refusal falls through to the full scan that run would have done anyway. +//! `--disable-incremental` forces it. +//! +//! `base_sha` travels with the file list because the server carries findings +//! forward for every file the list omits. Copy from a different baseline than +//! the one diffed here and files changed between the two keep stale findings, +//! reported as current. The server copies from exactly this scan, or refuses. +//! +//! The archive is unchanged. Fusion reads unchanged files for cross-file +//! context, and a finding can only carry forward for a file the archive still +//! holds. Analysis shrinks, not the upload. + +use crate::config::Config; +use crate::scanners::blast::{classify_scan_status, ScanState}; +use crate::utils::api::{self, ScanResponse}; +use git2::Repository; +use std::collections::BTreeSet; + +/// How many of the project's scans to read at a time, newest first. +const SCAN_LOOKUP_PAGE_SIZE: u16 = 30; + +/// Backstop on pages walked looking for a baseline. +/// +/// The server filters out scans that cannot be a baseline, so the answer is +/// normally the first entry of page one and this never iterates. Kept for a +/// backend predating those filters: it ignores unknown parameters and returns +/// scans of every kind, so heavy pull-request traffic can fill a page with +/// nothing usable. +const SCAN_LOOKUP_MAX_PAGES: u16 = 3; + +/// Engine every blast scan carries. An uploaded third-party report describes +/// someone else's analysis and cannot be a baseline for ours. +const BLAST_ENGINE: &str = "corgea-blast"; + +/// Payload guard, not policy. The server applies the real ceiling +/// (`INCREMENTAL_SCAN_MAX_FILES`, 300) and falls back to a full scan above it. +/// This only avoids building a multi-megabyte form field to be refused. +const MAX_CHANGED_FILES: usize = 5_000; + +/// A diff the server can turn into an incremental scan. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IncrementalPlan { + /// Commit this diff was measured from. The server carries its findings + /// forward for every file the diff does not name. + pub base_sha: String, + /// Repo-relative paths differing from `base_sha`, including deletions and + /// both sides of a rename. + pub changed_files: Vec, + /// Whether the diff measured the working tree rather than a commit. The + /// server refuses a dirty upload otherwise, because a commit-to-commit diff + /// cannot describe one. + pub covers_worktree: bool, +} + +/// What an incremental scan of this commit would cover, or `None` to scan +/// everything. +pub fn resolve_incremental_plan( + config: &Config, + project_name: &str, + branch: Option<&str>, + head_sha: Option<&str>, + worktree_dirty: bool, + ignore_dirty_worktree: bool, +) -> Option { + // A commit-to-commit diff cannot see uncommitted edits, so on a dirty tree + // it leaves modified files off the list and their old findings are copied + // forward as current. --ignore-dirty-worktree does not paper over that; it + // switches the diff to measure the working tree, so those files are named + // and rescanned like any other change. + let covers_worktree = worktree_dirty; + if worktree_dirty && !ignore_dirty_worktree { + explain_full_scan( + "this worktree has uncommitted changes, and a commit-to-commit diff cannot \ + see them. Pass --ignore-dirty-worktree to diff the working tree instead", + ); + return None; + } + + // Nothing to diff from. Covers a non-git directory, a repo with no commit, + // a detached HEAD, and a scan started below the repo root — none of which + // report RepoInfo to the upload either. + let (Some(_branch), Some(head_sha)) = (branch, head_sha) else { + explain_full_scan( + "no git branch and commit to diff from (not a git repository, no commit \ + yet, a detached HEAD, or a scan started below the repository root)", + ); + return None; + }; + + let repo = match Repository::discover(".") { + Ok(repo) => repo, + Err(e) => { + explain_full_scan(&format!("this directory is not a git repository ({e})")); + return None; + } + }; + + let trunks = baseline_branches(&repo); + let base_sha = match find_baseline_sha(config, project_name, &trunks) { + Baseline::Found(sha) => sha, + Baseline::NotFound => { + explain_full_scan(&format!( + "project '{project_name}' has no completed scan of a clean worktree on \ + {}, so there is nothing stable to diff against", + join_or(&trunks) + )); + return None; + } + Baseline::LookupFailed => { + explain_full_scan(&format!( + "the earlier scans of project '{project_name}' could not be looked up, \ + so there is nothing to diff against. Run with --verbose for the error" + )); + return None; + } + }; + + let changed_files = match changed_files_since(&repo, &base_sha, head_sha, covers_worktree) { + Ok(files) => files, + Err(reason) => { + explain_full_scan(&reason); + return None; + } + }; + + if changed_files.len() > MAX_CHANGED_FILES { + explain_full_scan(&format!( + "{} files changed since {}, which is more than an incremental scan is worth", + changed_files.len(), + short_sha(&base_sha) + )); + return None; + } + + let since = if covers_worktree { + format!( + "commit {} and your uncommitted changes", + short_sha(&base_sha) + ) + } else { + format!("commit {}", short_sha(&base_sha)) + }; + match changed_files.len() { + 0 => println!("Incremental scan: nothing changed since {since}."), + 1 => println!("Incremental scan: 1 file changed since {since}."), + count => println!("Incremental scan: {count} files changed since {since}."), + } + + Some(IncrementalPlan { + base_sha, + changed_files, + covers_worktree, + }) +} + +/// Say why this run scans everything. Never fatal — a full scan is correct, +/// only slower, so the run continues. +fn explain_full_scan(reason: &str) { + println!("Scanning every file: {reason}."); +} + +/// Outcome of looking for a scan to diff against. +/// +/// `NotFound` and `LookupFailed` both mean a full scan, but they are different +/// things to tell someone: one says this project has no scan history to build +/// on, the other says we could not read the history it may well have. +#[derive(Debug, PartialEq, Eq)] +enum Baseline { + Found(String), + NotFound, + LookupFailed, +} + +/// The branches a baseline may come from, best first. +/// +/// Only trunk qualifies. Any completed clean scan is a *correct* thing to diff +/// against, but not a *stable* one: a scan of someone else's feature branch is +/// a baseline whose contents nobody can predict, and the findings copied +/// forward from it would be that branch's, not this project's. Trunk is the +/// line every branch descends from, so it is the only shared reference point. +/// +/// `origin/HEAD` records what the remote advertised as its default when this +/// clone was made. It is absent from single-branch and `actions/checkout` +/// checkouts and is never refreshed after a rename, so `main` and `master` +/// follow it rather than replace it. +fn baseline_branches(repo: &Repository) -> Vec { + let mut branches: Vec = default_branch(repo).into_iter().collect(); + for fallback in ["main", "master"] { + if !branches.iter().any(|branch| branch == fallback) { + branches.push(fallback.to_string()); + } + } + branches +} + +/// Default branch this clone recorded, or None when it recorded none. +fn default_branch(repo: &Repository) -> Option { + let reference = repo.find_reference("refs/remotes/origin/HEAD").ok()?; + let name = reference + .symbolic_target() + .ok() + .flatten()? + .strip_prefix("refs/remotes/origin/")?; + (!name.is_empty() && name != "HEAD").then(|| name.to_string()) +} + +fn join_or(branches: &[String]) -> String { + match branches.split_last() { + Some((last, [])) => last.clone(), + Some((last, rest)) => format!("{} or {last}", rest.join(", ")), + None => "any branch".to_string(), + } +} + +/// Commit of the newest scan on the first trunk branch that has one. +/// +/// One query per branch, because the branch filter is server-side: a project +/// with heavy feature-branch traffic can push trunk's newest scan far past any +/// page limit, and asking for trunk directly cannot miss it that way. The page +/// budget is shared across branches so the worst case stays bounded. +fn find_baseline_sha(config: &Config, project_name: &str, branches: &[String]) -> Baseline { + let url = config.get_url(); + let mut budget = SCAN_LOOKUP_MAX_PAGES; + + for branch in branches { + let mut page = 1; + while budget > 0 { + budget -= 1; + let response = match api::query_baseline_scans( + &url, + project_name, + BLAST_ENGINE, + branch, + page, + SCAN_LOOKUP_PAGE_SIZE, + ) { + Ok(response) => response, + Err(e) => { + // Proves nothing about this project's history, so it is a + // full scan rather than an error -- but it is a different + // answer from "trunk has no scan", so say which it was. + // Whatever failed is the endpoint, not the branch, so the + // remaining candidates would fail the same way. + crate::log::debug(&format!("Baseline scan lookup failed: {e}")); + return Baseline::LookupFailed; + } + }; + + let scans = response.scans.unwrap_or_default(); + if scans.is_empty() { + break; + } + // Newest first, so the first match on this branch is the best + // available and no later page can improve on it. Matched + // client-side too: a backend that ignored the branch filter would + // otherwise hand back another branch's scan. + if let Some(sha) = branch_baseline(&scans, branch) { + return Baseline::Found(sha); + } + if response + .total_pages + .is_some_and(|total| u32::from(page) >= total) + { + break; + } + page += 1; + } + } + + Baseline::NotFound +} + +/// Newest usable scan of `branch` on this page. +fn branch_baseline(scans: &[ScanResponse], branch: &str) -> Option { + usable_baselines(scans) + .find(|scan| scan.branch.as_deref() == Some(branch)) + .and_then(|scan| scan.git_sha.clone()) +} + +/// Scans on one page that can serve as a baseline, newest first. +fn usable_baselines(scans: &[ScanResponse]) -> impl Iterator { + scans.iter().filter(|scan| is_usable_baseline(scan)) +} + +/// Whether `scan` may be diffed against. +/// +/// Client-side half of the filter doghouse applies picking a baseline itself: a +/// completed blast scan of a whole, clean, non-pull-request commit. +/// `worktree_dirty` must be an explicit `false` — `None` means never reported, +/// and unknown scope is not clean, so the server rejects it as a baseline too. +/// +/// `query_baseline_scans` asks the server for exactly these, which keeps the +/// page walk from iterating. This stays because a backend predating those +/// parameters ignores them, and a dirty or pull-request scan's commit would +/// diff against the wrong tree. +fn is_usable_baseline(scan: &ScanResponse) -> bool { + classify_scan_status(&scan.status) == ScanState::Completed + && scan.engine.eq_ignore_ascii_case(BLAST_ENGINE) + && scan.pull_request_id.is_none() + && scan.worktree_dirty == Some(false) + && scan.git_sha.as_deref().is_some_and(|sha| !sha.is_empty()) +} + +/// Every repo-relative path differing from the baseline commit. +/// +/// `include_worktree` decides what the far side of the diff is. False compares +/// two commits, which is exact when the tree is clean. True compares the +/// baseline against the index and working tree, which is what makes a dirty +/// tree scannable: a file edited but not committed differs from the baseline +/// and has to be named, or its old findings would be carried forward over +/// content nothing analyzed. Untracked files count for the same reason — the +/// archive contains them. +/// +/// Both sides of every delta, no status filtered out, because the list decides +/// which findings are *not* carried forward. A deleted file left off keeps its +/// findings in a tree no longer holding it; a rename is a delete plus an add +/// whose old path needs the same. `--target`'s `git:diff=` selector wants the +/// opposite — paths still on disk, to archive — hence no reuse. +/// +/// Untracked files are not a gap: they make the worktree dirty, already +/// refused above. +/// +/// Submodules are the one thing this cannot describe. A committed pointer bump +/// is one gitlink delta naming the submodule directory, while packaging walks +/// into it and uploads the files inside, so those files would be missing from +/// the list and keep old findings. Diffing the two submodule commits means +/// opening a repo that may not be checked out, so this fails closed. +fn changed_files_since( + repo: &Repository, + base_sha: &str, + head_sha: &str, + include_worktree: bool, +) -> Result, String> { + let base_tree = commit_tree(repo, base_sha).map_err(|e| { + format!( + "commit {}, the one the last scan covered, is not in this clone ({e}). A shallow \ + clone cannot diff against it — fetch more history (for example `actions/checkout` \ + with `fetch-depth: 0`) to scan incrementally", + short_sha(base_sha) + ) + })?; + + let diff = if include_worktree { + let mut options = git2::DiffOptions::new(); + options.include_untracked(true).recurse_untracked_dirs(true); + repo.diff_tree_to_workdir_with_index(Some(&base_tree), Some(&mut options)) + } else { + let head_tree = commit_tree(repo, head_sha) + .map_err(|e| format!("commit {} could not be read ({e})", short_sha(head_sha)))?; + repo.diff_tree_to_tree(Some(&base_tree), Some(&head_tree), None) + } + .map_err(|e| format!("the diff against {} failed ({e})", short_sha(base_sha)))?; + + // Sorted and deduplicated: a rename reports one path per side, and stable + // order keeps the uploaded list reproducible for the same two commits. + let mut files = BTreeSet::new(); + for delta in diff.deltas() { + if delta.old_file().mode() == git2::FileMode::Commit + || delta.new_file().mode() == git2::FileMode::Commit + { + let name = delta + .new_file() + .path() + .or_else(|| delta.old_file().path()) + .map(|path| path.to_string_lossy().into_owned()) + .unwrap_or_else(|| "a submodule".to_string()); + return Err(format!( + "submodule {name} moved to a different commit, and the diff names only \ + the submodule itself rather than the files inside it that this scan \ + uploads" + )); + } + for file in [delta.old_file(), delta.new_file()] { + if let Some(path) = file.path() { + // Byte-for-byte. Git stores `/` as its separator on every + // platform, so a backslash here is part of the filename, and + // translating it would name a file that did not change. + let path = path.to_string_lossy().into_owned(); + if !path.is_empty() { + files.insert(path); + } + } + } + } + Ok(files.into_iter().collect()) +} + +fn commit_tree<'repo>( + repo: &'repo Repository, + rev: &str, +) -> Result, git2::Error> { + repo.revparse_single(rev)?.peel_to_commit()?.tree() +} + +/// First 7 characters, by char boundary rather than byte index. The value comes +/// from the API, so a non-ASCII one must shorten, not panic mid-scan. +fn short_sha(sha: &str) -> &str { + match sha.char_indices().nth(7) { + Some((byte, _)) => &sha[..byte], + None => sha, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use std::path::Path; + + fn scan(branch: &str, sha: &str) -> ScanResponse { + ScanResponse { + id: format!("scan-{sha}"), + project: "proj".to_string(), + repo: None, + branch: Some(branch.to_string()), + status: "complete".to_string(), + engine: BLAST_ENGINE.to_string(), + created_at: "2026-01-01T00:00:00Z".to_string(), + git_sha: Some(sha.to_string()), + worktree_dirty: Some(false), + pull_request_id: None, + metadata: None, + failed_reason: None, + scan_errors: Vec::new(), + } + } + + #[test] + fn short_sha_shortens_a_non_ascii_value_instead_of_panicking() { + // The API supplies git_sha; a malformed one must not kill the scan. + assert_eq!(short_sha("0123456789abcdef"), "0123456"); + assert_eq!(short_sha("abc"), "abc"); + assert_eq!(short_sha(""), ""); + assert_eq!(short_sha("ααααααααα"), "ααααααα"); + } + + #[test] + fn a_completed_clean_blast_scan_is_a_baseline() { + assert!(is_usable_baseline(&scan("main", "abc"))); + } + + #[test] + fn scans_that_cannot_describe_a_whole_clean_commit_are_rejected() { + // The server refuses each of these too, so diffing against them narrows + // a scan the server then widens. + let mut running = scan("main", "abc"); + running.status = "processing".to_string(); + assert!(!is_usable_baseline(&running)); + + let mut third_party = scan("main", "abc"); + third_party.engine = "semgrep".to_string(); + assert!(!is_usable_baseline(&third_party)); + + let mut pr = scan("main", "abc"); + pr.pull_request_id = Some("42".to_string()); + assert!(!is_usable_baseline(&pr)); + + let mut dirty = scan("main", "abc"); + dirty.worktree_dirty = Some(true); + assert!(!is_usable_baseline(&dirty)); + + // Never reported is not known clean. + let mut unknown = scan("main", "abc"); + unknown.worktree_dirty = None; + assert!(!is_usable_baseline(&unknown)); + + let mut no_commit = scan("main", "abc"); + no_commit.git_sha = None; + assert!(!is_usable_baseline(&no_commit)); + } + + #[test] + fn the_newest_usable_scan_on_the_branch_wins() { + let scans = vec![scan("main", "newest"), scan("main", "older")]; + assert_eq!(branch_baseline(&scans, "main").as_deref(), Some("newest")); + } + + #[test] + fn a_scan_on_another_branch_is_never_the_baseline() { + // A backend that ignored the branch filter would otherwise hand back a + // feature branch's scan as trunk's. + let scans = vec![scan("feature", "on-feature")]; + assert_eq!(branch_baseline(&scans, "main"), None); + } + + #[test] + fn unusable_scans_on_the_branch_are_skipped() { + let mut dirty = scan("main", "dirty"); + dirty.worktree_dirty = Some(true); + let scans = vec![dirty, scan("main", "clean")]; + assert_eq!(branch_baseline(&scans, "main").as_deref(), Some("clean")); + } + + #[test] + fn a_page_of_nothing_usable_yields_no_baseline() { + let mut pr = scan("main", "pr"); + pr.pull_request_id = Some("42".to_string()); + assert_eq!(branch_baseline(&[pr], "main"), None); + } + + #[test] + fn trunk_candidates_fall_back_to_main_then_master() { + let dir = tempfile::tempdir().expect("tempdir"); + let repo = Repository::init(dir.path()).expect("init"); + // No origin/HEAD: single-branch and actions/checkout clones have none. + assert_eq!(default_branch(&repo), None); + assert_eq!(baseline_branches(&repo), vec!["main", "master"]); + } + + #[test] + fn a_recorded_default_branch_leads_and_is_not_repeated() { + let dir = tempfile::tempdir().expect("tempdir"); + let repo = Repository::init(dir.path()).expect("init"); + repo.reference_symbolic( + "refs/remotes/origin/HEAD", + "refs/remotes/origin/trunk", + true, + "test", + ) + .expect("set origin/HEAD"); + + assert_eq!(default_branch(&repo).as_deref(), Some("trunk")); + assert_eq!(baseline_branches(&repo), vec!["trunk", "main", "master"]); + + repo.reference_symbolic( + "refs/remotes/origin/HEAD", + "refs/remotes/origin/main", + true, + "test", + ) + .expect("set origin/HEAD"); + assert_eq!(baseline_branches(&repo), vec!["main", "master"]); + } + + /// Two commits: three files, then one that adds, edits and deletes. + fn repo_with_history() -> (tempfile::TempDir, Repository, String, String) { + let dir = tempfile::tempdir().expect("tempdir"); + let repo = Repository::init(dir.path()).expect("init"); + let sig = git2::Signature::now("t", "t@example.com").expect("sig"); + + let commit_all = + |repo: &Repository, message: &str, parent: Option| -> git2::Oid { + let mut index = repo.index().expect("index"); + index + .add_all(["*"], git2::IndexAddOption::DEFAULT, None) + .expect("add"); + index.write().expect("write index"); + let tree = repo + .find_tree(index.write_tree().expect("tree")) + .expect("find tree"); + let parents: Vec = parent + .map(|oid| vec![repo.find_commit(oid).expect("parent")]) + .unwrap_or_default(); + let parent_refs: Vec<&git2::Commit> = parents.iter().collect(); + repo.commit(Some("HEAD"), &sig, &sig, message, &tree, &parent_refs) + .expect("commit") + }; + + let write = |name: &str, body: &str| { + fs::write(dir.path().join(name), body).expect("write file"); + }; + + write("keep.txt", "same"); + write("edit.txt", "before"); + write("gone.txt", "doomed"); + let base = commit_all(&repo, "base", None); + + write("edit.txt", "after"); + write("added.txt", "new"); + fs::remove_file(dir.path().join("gone.txt")).expect("remove"); + // add_all does not stage a deletion on its own. + let mut index = repo.index().expect("index"); + index + .remove_path(Path::new("gone.txt")) + .expect("stage delete"); + index.write().expect("write index"); + let head = commit_all(&repo, "head", Some(base)); + + (dir, repo, base.to_string(), head.to_string()) + } + + #[test] + fn the_diff_names_added_edited_and_deleted_files_but_not_untouched_ones() { + let (_dir, repo, base, head) = repo_with_history(); + let files = changed_files_since(&repo, &base, &head, false).expect("diff"); + // Deleted file must be listed, else its findings carry into a tree that + // no longer holds it. + assert_eq!(files, vec!["added.txt", "edit.txt", "gone.txt"]); + } + + #[test] + fn a_commit_diffed_against_itself_reports_nothing_changed() { + let (_dir, repo, _base, head) = repo_with_history(); + assert!(changed_files_since(&repo, &head, &head, false) + .expect("diff") + .is_empty()); + } + + #[test] + fn a_commit_range_diff_cannot_see_uncommitted_work() { + // Why a dirty tree may not use one: keep.txt differs from what will be + // uploaded, yet the diff does not name it, so its findings would be + // carried forward over content nothing analyzed. + let (dir, repo, _base, head) = repo_with_history(); + fs::write(dir.path().join("keep.txt"), "edited").expect("edit"); + fs::write(dir.path().join("brand-new.txt"), "new").expect("add"); + + let committed = changed_files_since(&repo, &head, &head, false).expect("diff"); + assert!(committed.is_empty()); + } + + #[test] + fn a_worktree_diff_names_edited_and_untracked_files() { + let (dir, repo, _base, head) = repo_with_history(); + fs::write(dir.path().join("keep.txt"), "edited").expect("edit"); + fs::write(dir.path().join("brand-new.txt"), "new").expect("add"); + + let files = changed_files_since(&repo, &head, &head, true).expect("diff"); + + assert_eq!(files, vec!["brand-new.txt", "keep.txt"]); + } + + #[test] + fn a_worktree_diff_still_spans_the_commits_behind_it() { + // The baseline is a commit, so committed changes since it count too -- + // the working tree is the far side of the diff, not the whole of it. + let (dir, repo, base, _head) = repo_with_history(); + fs::write(dir.path().join("keep.txt"), "edited").expect("edit"); + + let files = changed_files_since(&repo, &base, "unused", true).expect("diff"); + + assert_eq!(files, vec!["added.txt", "edit.txt", "gone.txt", "keep.txt"]); + } + + /// Commit whose tree carries a `vendor` gitlink pointing at `target`. + fn commit_with_gitlink(repo: &Repository, parent: git2::Oid, target: git2::Oid) -> git2::Oid { + let sig = git2::Signature::now("t", "t@example.com").expect("sig"); + let parent_commit = repo.find_commit(parent).expect("parent"); + let mut builder = repo + .treebuilder(Some(&parent_commit.tree().expect("parent tree"))) + .expect("treebuilder"); + builder + .insert("vendor", target, i32::from(git2::FileMode::Commit)) + .expect("insert gitlink"); + let tree_oid = builder.write().expect("write tree"); + let tree = repo.find_tree(tree_oid).expect("find tree"); + repo.commit(None, &sig, &sig, "gitlink", &tree, &[&parent_commit]) + .expect("commit") + } + + #[test] + fn a_moved_submodule_pointer_refuses_the_diff() { + // Packaging uploads the files inside the submodule, but the diff names + // only `vendor`, so those files would keep unexamined findings. + let (_dir, repo, base, head) = repo_with_history(); + let base_oid = git2::Oid::from_str(&base).expect("base oid"); + let head_oid = git2::Oid::from_str(&head).expect("head oid"); + let before = commit_with_gitlink(&repo, base_oid, base_oid); + let after = commit_with_gitlink(&repo, before, head_oid); + + let err = changed_files_since(&repo, &before.to_string(), &after.to_string(), false) + .expect_err("a moved submodule must refuse the diff"); + + assert!(err.contains("submodule vendor"), "{err}"); + } + + #[test] + fn a_base_commit_this_clone_does_not_have_is_reported_not_panicked() { + let (_dir, repo, _base, head) = repo_with_history(); + let err = changed_files_since(&repo, &"0".repeat(40), &head, false) + .expect_err("unknown base must fail"); + assert!(err.contains("shallow clone"), "{err}"); + } +} diff --git a/src/main.rs b/src/main.rs index e2920e6..5e9cd0f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,7 @@ mod authorize; mod cicd; mod config; mod images; +mod incremental; mod inspect; mod list; mod log; @@ -89,6 +90,12 @@ enum Commands { #[arg(long, help = "Only scan uncommitted changes.")] only_uncommitted: bool, + #[arg( + long = "disable-incremental", + help = "Analyze every file, even when Corgea could have analyzed only what changed. Scans are incremental by default: the whole project is still uploaded, but only files that changed since this project's last scan are analyzed, and unchanged files keep their existing findings, so the result is a full picture either way. Use this to force a fresh analysis of every file — after changing scanner configuration outside corgea.yaml, for example. Incremental is skipped on its own, with a reason, when there is no git repository or commit to diff from, when the worktree is dirty, when no earlier scan of a clean worktree exists, or when the last scanned commit is missing from a shallow clone; and silently when --only-uncommitted, --target or --exclude already narrow the upload." + )] + disable_incremental: bool, + #[arg( long = "metadata", value_name = "KEY=VALUE", @@ -173,8 +180,8 @@ enum Commands { #[arg( long = "skip-if-commit-scanned-recently", - conflicts_with_all = ["only_uncommitted", "target", "scan_type", "policy", "include_image"], - help = "Do not start a new scan when this commit already has a recent completed scan in the project. That scan then drives the rest of the command — results table, --block-on gate, --out-file report — so the pipeline behaves the same either way. Prints CORGEA_SCAN_SKIPPED=true/false so a pipeline can tell the two apart, and fails if no git commit can be resolved. What can be reused is a scan of the whole commit, and no API tells this run how a past scan was scoped or configured, so the flag is refused with --only-uncommitted, --target, --scan-type, --policy and --include-image; with --exclude it warns instead, since a reused scan covers files this run would have skipped." + conflicts_with_all = ["only_uncommitted", "target", "scan_type", "policy", "include_image", "disable_incremental"], + help = "Do not start a new scan when this commit already has a recent completed scan in the project. That scan then drives the rest of the command — results table, --block-on gate, --out-file report — so the pipeline behaves the same either way. Prints CORGEA_SCAN_SKIPPED=true/false so a pipeline can tell the two apart, and fails if no git commit can be resolved. What can be reused is a scan of the whole commit, and no API tells this run how a past scan was scoped or configured, so the flag is refused with --only-uncommitted, --target, --scan-type, --policy, --include-image and --disable-incremental; with --exclude it warns instead, since a reused scan covers files this run would have skipped." )] skip_if_commit_scanned_recently: bool, @@ -188,8 +195,7 @@ enum Commands { #[arg( long = "ignore-dirty-worktree", - requires = "skip_if_commit_scanned_recently", - help = "With --skip-if-commit-scanned-recently, reuse a recent scan of this commit even if this worktree is dirty or the prior scan recorded worktree_dirty. A new scan still reports the real dirty status." + help = "Do not let uncommitted changes stop this run from taking a shortcut. For an incremental scan, the diff is measured against the working tree instead of the last commit, so edited and untracked files are analyzed rather than skipped. With --skip-if-commit-scanned-recently, a recent scan of this commit may be reused even though this worktree is dirty or the prior scan recorded worktree_dirty. A new scan still reports the real dirty status." )] ignore_dirty_worktree: bool, }, @@ -669,6 +675,7 @@ fn main() { fail, block_on, only_uncommitted, + disable_incremental, metadata, scan_type, policy, @@ -710,6 +717,11 @@ fn main() { std::process::exit(1); } + if *disable_incremental && *scanner != Scanner::Blast { + ::log::error!("--disable-incremental is only supported with blast scanner."); + std::process::exit(1); + } + if !metadata.is_empty() && *scanner != Scanner::Blast { ::log::error!("--metadata is only supported with the blast scanner."); std::process::exit(1); @@ -846,6 +858,8 @@ fn main() { fail, block_on, only_uncommitted, + disable_incremental, + ignore_dirty_worktree, metadata_json, scan_type.clone(), policy.clone(), diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 12421b7..3c368fa 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -53,6 +53,8 @@ pub fn run( fail: &bool, block_on: Option, only_uncommitted: &bool, + disable_incremental: &bool, + ignore_dirty_worktree_for_run: &bool, metadata: Option, scan_type: Option, policy: Option, @@ -109,6 +111,8 @@ pub fn run( config, &project_name, only_uncommitted, + disable_incremental, + ignore_dirty_worktree_for_run, metadata, scan_type, policy, @@ -281,6 +285,8 @@ fn start_new_scan( config: &Config, project_name: &str, only_uncommitted: &bool, + disable_incremental: &bool, + ignore_dirty_worktree: &bool, metadata: Option, scan_type: Option, policy: Option, @@ -514,15 +520,40 @@ fn start_new_scan( info.dirty = true; } } + // Incremental is the default, so this asks what took it off the table. A + // narrowed archive is the silent case: carrying findings forward for files + // the archive no longer holds would be wrong, but those runs are not + // "scanning every file" either, so no message is honest. + let narrowed_archive = target_str.is_some() || exclude.is_some(); + let incremental_plan = if *disable_incremental || narrowed_archive { + None + } else { + // Reconciled repo info, so a tree that turned out dirty — or a HEAD + // that moved mid-packaging — refuses rather than diffing against a + // commit this upload is not a snapshot of. + crate::incremental::resolve_incremental_plan( + config, + project_name, + repo_info.as_ref().and_then(|info| info.branch.as_deref()), + repo_info.as_ref().and_then(|info| info.sha.as_deref()), + // Missing repo info is not dirtiness; it is the missing + // branch/commit the resolver reports next, by its real name. + repo_info.as_ref().is_some_and(|info| info.dirty), + *ignore_dirty_worktree, + ) + }; println!("\n\nSubmitting scan to Corgea:"); let upload_result = match utils::api::upload_zip( &zip_path, &config.get_url(), project_name, repo_info, - scan_type, - policy, - metadata, + utils::api::UploadOptions { + scan_type, + policy, + metadata, + incremental: incremental_plan, + }, ) { Ok(result) => result, Err(e) => { diff --git a/src/skip_scan.rs b/src/skip_scan.rs index 1acc416..aa75b98 100644 --- a/src/skip_scan.rs +++ b/src/skip_scan.rs @@ -454,8 +454,13 @@ fn format_age(age: Duration) -> String { format!("{}s", seconds) } +/// First 7 characters, by char boundary rather than byte index. The value comes +/// from the API, so a non-ASCII one must shorten, not panic mid-scan. fn short_sha(sha: &str) -> &str { - &sha[..sha.len().min(7)] + match sha.char_indices().nth(7) { + Some((byte, _)) => &sha[..byte], + None => sha, + } } #[cfg(test)] diff --git a/src/utils/api.rs b/src/utils/api.rs index 3f8d4f8..29d3272 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -1,3 +1,4 @@ +use crate::incremental::IncrementalPlan; use crate::log::debug; use crate::utils; use corgea::vuln_api::{auth_header, source}; @@ -233,15 +234,30 @@ pub struct UploadZipResult { pub project_id: Option, } +/// Per-scan settings travelling with the archive without being part of it. +#[derive(Debug, Default)] +pub struct UploadOptions { + pub scan_type: Option, + pub policy: Option, + pub metadata: Option, + /// Set when this run resolved a diff for the server to analyze instead of + /// the whole project. + pub incremental: Option, +} + pub fn upload_zip( file_path: &str, url: &str, project_name: &str, repo_info: Option, - scan_type: Option, - policy: Option, - metadata: Option, + options: UploadOptions, ) -> Result> { + let UploadOptions { + scan_type, + policy, + metadata, + incremental, + } = options; let client = http_client(); let file_size = std::fs::metadata(file_path)?.len(); let file_name = Path::new(file_path).file_name().unwrap().to_str().unwrap(); @@ -370,6 +386,34 @@ pub fn upload_zip( if let Some(meta) = &metadata { form = form.part("metadata", multipart::Part::text(meta.clone())); } + // Both fields or neither: the list is only safe next to the commit it + // was measured from, and a server seeing one without the other would + // guess a baseline. A list that will not serialize drops both, leaving + // a full scan. + if let Some(plan) = &incremental { + match serde_json::to_string(&plan.changed_files) { + Ok(changed_files) => { + form = form.part( + "incremental_base_sha", + multipart::Part::text(plan.base_sha.clone()), + ); + form = form.part( + "incremental_changed_files", + multipart::Part::text(changed_files), + ); + // Tells the server the list describes the working tree, not + // just a commit range, which is the only way it can accept a + // diff from a dirty upload. + if plan.covers_worktree { + form = + form.part("incremental_covers_worktree", multipart::Part::text("true")); + } + } + Err(e) => debug(&format!( + "Could not serialize the incremental file list, scanning every file: {e}" + )), + } + } let response = match client .patch(format!("{}{}/start-scan/{}/", url, API_BASE, transfer_id)) @@ -819,6 +863,37 @@ pub fn query_scan_list( request_scan_list(url, query_params) } +/// One page of the project's scans that could be diffed against, newest first. +/// +/// Filters are server-side, so the answer is usually the first entry of page +/// one. A backend predating them ignores the unknown parameters and returns +/// scans of every kind, so the caller must still re-check each scan it acts on +/// — see `incremental::is_usable_baseline`. +pub fn query_baseline_scans( + url: &str, + project: &str, + engine: &str, + branch: &str, + page: u16, + page_size: u16, +) -> Result> { + request_scan_list( + url, + vec![ + ("page", page.to_string()), + ("page_size", page_size.to_string()), + ("project", project.to_string()), + ("engine", engine.to_string()), + ("branch", branch.to_string()), + ("status", "complete".to_string()), + ("exclude_pull_requests", "true".to_string()), + // Explicitly clean only. A scan that never reported the flag is + // unknown scope, which the server rejects as a baseline. + ("worktree_dirty", "false".to_string()), + ], + ) +} + /// One page of the project's scans at exactly `sha`, newest first. /// /// The `sha` filter is server-side, but a backend that predates it ignores the diff --git a/tests/cloud_commands_e2e/common/mod.rs b/tests/cloud_commands_e2e/common/mod.rs index b475a28..76c5f55 100644 --- a/tests/cloud_commands_e2e/common/mod.rs +++ b/tests/cloud_commands_e2e/common/mod.rs @@ -310,6 +310,25 @@ pub(crate) fn assert_scan_list_request( assert_query(request, "project", project) } +/// One baseline lookup an incremental scan makes before uploading. +/// +/// Asserting the filters is the point: they keep this to one request per trunk +/// branch instead of a page walk, and a server dropping them silently returns +/// pull-request and dirty scans for the client to reject. `branch` is asserted +/// because a baseline may only come from trunk. +pub(crate) fn assert_baseline_lookup_request( + request: &CapturedRequest, + project: &str, + branch: &str, +) -> Result<(), String> { + assert_scan_list_request(request, project)?; + assert_query(request, "engine", "corgea-blast")?; + assert_query(request, "status", "complete")?; + assert_query(request, "exclude_pull_requests", "true")?; + assert_query(request, "worktree_dirty", "false")?; + assert_query(request, "branch", branch) +} + pub(crate) fn query_value(request: &CapturedRequest, key: &str) -> Result { let (_, query) = target_path_and_query(&request.target); query @@ -381,6 +400,23 @@ pub(crate) fn assert_multipart_text_field( } } +/// Proves a field was left off the form entirely. Some fields are only safe in +/// pairs, so "absent" is as much the contract as any value. +pub(crate) fn assert_no_multipart_field( + request: &CapturedRequest, + name: &str, +) -> Result<(), String> { + let needle = format!("name=\"{name}\""); + if request + .body + .windows(needle.len()) + .any(|window| window == needle.as_bytes()) + { + return Err(format!("unexpected multipart field {name}")); + } + Ok(()) +} + pub(crate) fn format_transcript(requests: &[CapturedRequest]) -> String { if requests.is_empty() { return "".to_string(); @@ -777,8 +813,22 @@ pub(crate) fn blast_upload_plan(sha: &str, dirty: bool, include_sca: bool) -> Ve let patch_path = "/api/v1/start-scan/transfer-123/".to_string(); let detail_path = "/api/v1/scan/blast-scan-123".to_string(); let issue_path = "/api/v1/scan/blast-scan-123/issues".to_string(); - let mut plan = vec![ - verify_request(), + let mut plan = vec![verify_request()]; + // Scans are incremental by default, so every clean-tree run looks for a + // baseline before uploading -- once per trunk branch, since the fixture + // records no origin/HEAD. Answering with no scans keeps this the full-scan + // contract: nothing to diff from, no incremental fields on the upload. A + // dirty tree never asks. + if !dirty { + for branch in ["main", "master"] { + plan.push(expected_request( + "look up a baseline scan to diff against", + move |request| assert_baseline_lookup_request(request, "cloud-e2e", branch), + json_response(scans_response(Vec::new())), + )); + } + } + plan.extend([ expected_request( "start BLAST upload", |request| { @@ -835,7 +885,7 @@ pub(crate) fn blast_upload_plan(sha: &str, dirty: bool, include_sca: bool) -> Ve }, json_response(empty_issue_page()), ), - ]; + ]); if include_sca { let sca_path = "/api/v1/scan/blast-scan-123/issues/sca".to_string(); plan.push(expected_request( diff --git a/tests/cloud_commands_e2e/main.rs b/tests/cloud_commands_e2e/main.rs index eded107..439f10a 100644 --- a/tests/cloud_commands_e2e/main.rs +++ b/tests/cloud_commands_e2e/main.rs @@ -4,6 +4,7 @@ mod repo_common; mod block_on_report; mod common; mod inspect; +mod scan_incremental; mod scan_list; mod scan_skip; mod upload_wait; diff --git a/tests/cloud_commands_e2e/scan_incremental.rs b/tests/cloud_commands_e2e/scan_incremental.rs new file mode 100644 index 0000000..c9385ba --- /dev/null +++ b/tests/cloud_commands_e2e/scan_incremental.rs @@ -0,0 +1,560 @@ +//! Incremental scans, attempted by default on every `corgea scan blast`: find +//! the project's last clean scan, diff this commit against it locally, send the +//! changed-file list with the archive. +//! +//! The stub asserts the exact wire contract because those two fields are what +//! the server acts on: `incremental_base_sha` picks whose findings carry +//! forward, `incremental_changed_files` picks which files are excluded from +//! that and analyzed instead. +//! +//! Being the default, the ways it declines matter as much as the way it works, +//! so each is a case here: stay correct, and do not even look for a baseline +//! when it already cannot be used. + +use crate::common::*; +use hyper::{Method, StatusCode}; +use serde_json::{json, Value}; + +const PROJECT: &str = "cloud-e2e"; +const BASELINE_SCAN: &str = "baseline-scan-123"; + +fn baseline_scan(sha: &str) -> Value { + json!({ + "id": BASELINE_SCAN, + "project": PROJECT, + "repo": null, + "branch": "main", + "status": "complete", + "engine": "corgea-blast", + "created_at": "2026-07-30T12:00:00Z", + "git_sha": sha, + "worktree_dirty": false + }) +} + +fn baseline_lookup(branch: &'static str, scans: Vec) -> ExpectedRequest { + expected_request( + "look up a baseline scan to diff against", + move |request| assert_baseline_lookup_request(request, PROJECT, branch), + json_response(scans_response(scans)), + ) +} + +/// The lookups a fixture repo makes when no trunk branch has a baseline. It +/// records no origin/HEAD, so the candidates are `main` then `master`. +fn baseline_lookups_finding_nothing() -> Vec { + vec![ + baseline_lookup("main", vec![]), + baseline_lookup("master", vec![]), + ] +} + +/// One page of the baseline lookup, for the walk an old backend forces. +fn baseline_lookup_page( + branch: &'static str, + page: u16, + total_pages: u32, + scans: Vec, +) -> ExpectedRequest { + expected_request( + "look up a baseline scan to diff against", + move |request| { + assert_authenticated_request(request, Method::GET, "/api/v1/scans")?; + assert_query(request, "project", PROJECT)?; + assert_query(request, "branch", branch)?; + assert_query(request, "page", &page.to_string())?; + assert_query(request, "engine", "corgea-blast") + }, + json_response(json!({ + "status": "ok", + "page": page, + "total_pages": total_pages, + "scans": scans, + })), + ) +} + +/// Everything after the archive upload, which incremental does not change. +fn scan_tail() -> Vec { + let detail_path = "/api/v1/scan/blast-scan-123".to_string(); + let issue_path = "/api/v1/scan/blast-scan-123/issues".to_string(); + vec![ + expected_request( + "read completed BLAST scan", + move |request| assert_authenticated_request(request, Method::GET, &detail_path), + json_response(scan_response("blast-scan-123", PROJECT, "complete")), + ), + expected_request( + "read regular BLAST issues", + move |request| { + assert_authenticated_request(request, Method::GET, &issue_path)?; + assert_query(request, "page", "1")?; + assert_query(request, "page_size", "30") + }, + json_response(empty_issue_page()), + ), + ] +} + +fn start_upload() -> ExpectedRequest { + expected_request( + "start BLAST upload", + |request| { + assert_authenticated_request(request, Method::POST, "/api/v1/start-scan")?; + assert_query(request, "scan_type", "blast") + }, + json_response(json!({"transfer_id": "transfer-123"})), + ) +} + +/// Adds a file and edits another, so the diff has more than one entry and a +/// file the baseline already contained. +fn second_commit(project: &GitProject) -> String { + std::fs::write(project.path().join("helper.py"), "print('helper')\n").expect("write helper"); + std::fs::write(project.path().join("main.py"), "print('edited')\n").expect("edit main"); + run_git(project.path(), &["add", "."]); + run_git(project.path(), &["commit", "-m", "second"]); + String::from_utf8(run_git(project.path(), &["rev-parse", "HEAD"]).stdout) + .expect("UTF-8 SHA") + .trim() + .to_string() +} + +#[test] +fn the_upload_carries_the_baseline_commit_and_the_files_that_changed_since_it() { + let project = git_project(); + let base_sha = project.sha.clone(); + let head_sha = second_commit(&project); + + let patch_sha = head_sha.clone(); + let expected_base = base_sha.clone(); + let mut plan = vec![ + verify_request(), + baseline_lookup("main", vec![baseline_scan(&base_sha)]), + start_upload(), + expected_request( + "upload BLAST archive with the diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_multipart_text_field(request, "sha", &patch_sha)?; + assert_multipart_text_field(request, "dirty", "false")?; + assert_multipart_text_field(request, "incremental_base_sha", &expected_base)?; + // Both sides of the diff, sorted, as JSON — a path may contain a + // comma, so the list is never a delimited string. + assert_multipart_text_field( + request, + "incremental_changed_files", + r#"["helper.py","main.py"]"#, + ) + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!( + stdout.contains("Incremental scan: 2 files changed since commit"), + "{context}" + ); +} + +/// No scan to diff against is a full scan, not an error. Every project's first +/// scan takes this path and must still produce a complete result. +#[test] +fn a_project_with_no_baseline_scan_uploads_without_a_diff() { + let project = git_project(); + let head_sha = second_commit(&project); + + let patch_sha = head_sha.clone(); + let mut plan = vec![verify_request()]; + plan.extend(baseline_lookups_finding_nothing()); + plan.extend([ + start_upload(), + expected_request( + "upload BLAST archive with no diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_multipart_text_field(request, "sha", &patch_sha)?; + // Neither field alone, nor at all: a base commit without a + // list lets the server carry everything forward. + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]); + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!( + stdout.contains("has no completed scan of a clean worktree on main or master"), + "{context}" + ); +} + +/// A backend predating the server-side filters returns scans of every kind, so +/// a page can hold nothing usable. The walk is what stops that project from +/// being permanently unable to find a baseline it has. +#[test] +fn a_baseline_on_a_later_page_is_still_found() { + let project = git_project(); + let base_sha = project.sha.clone(); + let head_sha = second_commit(&project); + + let mut unusable = baseline_scan(&head_sha); + unusable["worktree_dirty"] = json!(true); + let expected_base = base_sha.clone(); + + let mut plan = vec![ + verify_request(), + baseline_lookup_page("main", 1, 2, vec![unusable]), + baseline_lookup_page("main", 2, 2, vec![baseline_scan(&base_sha)]), + start_upload(), + expected_request( + "upload BLAST archive with the diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_multipart_text_field(request, "incremental_base_sha", &expected_base) + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + + assert_eq!(output.status.code(), Some(0), "{context}"); +} + +/// A lookup that failed says so. Reporting it as "no earlier scan" tells someone +/// with years of scan history that they have none, and now that incremental is +/// the default, any network blip would say it. +#[test] +fn a_failed_lookup_is_not_reported_as_a_missing_baseline() { + let project = git_project(); + second_commit(&project); + + let mut plan = vec![ + verify_request(), + expected_request( + "fail the baseline lookup", + |request| assert_baseline_lookup_request(request, PROJECT, "main"), + json_response_with_status(StatusCode::INTERNAL_SERVER_ERROR, json!({"error": "boom"})), + ), + start_upload(), + expected_request( + "upload BLAST archive with no diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!(stdout.contains("could not be looked up"), "{context}"); + assert!( + !stdout.contains("no completed scan of a clean worktree"), + "a lookup failure must not claim the project has no scan history\n{context}" + ); +} + +/// The opt-out is absolute: no baseline lookup, no fields, no message. Someone +/// reaching for it wants every file analyzed, usually because something outside +/// `corgea.yaml` changed that the server's baseline checks cannot see. +#[test] +fn disable_incremental_does_not_even_look_for_a_baseline() { + let project = git_project(); + let head_sha = second_commit(&project); + + let patch_sha = head_sha.clone(); + let mut plan = vec![ + verify_request(), + start_upload(), + expected_request( + "upload BLAST archive with no diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_multipart_text_field(request, "sha", &patch_sha)?; + assert_multipart_text_field(request, "dirty", "false")?; + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args([ + "scan", + "blast", + "--disable-incremental", + "--project-name", + PROJECT, + ]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!(!stdout.contains("Scanning every file:"), "{context}"); + assert!(!stdout.contains("Incremental scan:"), "{context}"); +} + +/// `--target` already uploads a chosen subset. Carrying findings forward for +/// files the archive no longer holds would be wrong, so incremental is skipped +/// — silently, since "scanning every file" would be a lie here. +#[test] +fn a_narrowed_archive_skips_incremental_without_claiming_a_full_scan() { + let project = git_project(); + second_commit(&project); + + let mut plan = vec![ + verify_request(), + start_upload(), + expected_request( + "upload narrowed BLAST archive", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + // A partial archive is never an exact snapshot of the commit. + assert_multipart_text_field(request, "dirty", "true")?; + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args([ + "scan", + "blast", + "--target", + "main.py", + "--project-name", + PROJECT, + ]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!(!stdout.contains("Scanning every file:"), "{context}"); +} + +/// No git repository must not stall or fail: no commit to diff from, so skip +/// the lookup and scan everything. +#[test] +fn a_directory_that_is_not_a_git_repository_scans_everything() { + let project = tempfile::TempDir::new().expect("create project"); + std::fs::write(project.path().join("main.py"), "print('hi')\n").expect("write source"); + + let mut plan = vec![ + verify_request(), + start_upload(), + expected_request( + "upload BLAST archive with no repo metadata", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!( + stdout.contains("Scanning every file: no git branch and commit to diff from"), + "{context}" + ); +} + +/// `--ignore-dirty-worktree` does not pretend the tree is clean: it moves the +/// far side of the diff to the working tree, so the edited file is named and +/// rescanned rather than keeping findings nothing analyzed. +#[test] +fn ignore_dirty_worktree_diffs_the_working_tree_instead_of_refusing() { + let project = git_project(); + let base_sha = project.sha.clone(); + std::fs::write(project.path().join("main.py"), "print('uncommitted')\n") + .expect("dirty the tree"); + + let expected_base = base_sha.clone(); + let mut plan = vec![ + verify_request(), + baseline_lookup("main", vec![baseline_scan(&base_sha)]), + start_upload(), + expected_request( + "upload BLAST archive with a worktree diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + // Still reported dirty: the upload is not a snapshot of the + // commit, and the scan must never become a baseline itself. + assert_multipart_text_field(request, "dirty", "true")?; + assert_multipart_text_field(request, "incremental_base_sha", &expected_base)?; + assert_multipart_text_field( + request, + "incremental_changed_files", + r#"["main.py"]"#, + )?; + assert_multipart_text_field(request, "incremental_covers_worktree", "true") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args([ + "scan", + "blast", + "--ignore-dirty-worktree", + "--project-name", + PROJECT, + ]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!(stdout.contains("and your uncommitted changes"), "{context}"); +} + +/// The server refuses a dirty tree too, and the refusal must come before the +/// baseline lookup: a commit-to-commit diff cannot see uncommitted edits, so no +/// baseline makes the list correct. +#[test] +fn a_dirty_worktree_skips_the_baseline_lookup_and_scans_everything() { + let project = git_project(); + let head_sha = second_commit(&project); + std::fs::write(project.path().join("main.py"), "print('uncommitted')\n") + .expect("dirty the tree"); + + let patch_sha = head_sha.clone(); + let mut plan = vec![ + verify_request(), + start_upload(), + expected_request( + "upload BLAST archive with no diff", + move |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_multipart_text_field(request, "sha", &patch_sha)?; + assert_multipart_text_field(request, "dirty", "true")?; + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!( + stdout.contains("Scanning every file: this worktree has uncommitted changes"), + "{context}" + ); +} diff --git a/tests/cloud_commands_e2e/scan_skip.rs b/tests/cloud_commands_e2e/scan_skip.rs index f02183b..6b073b1 100644 --- a/tests/cloud_commands_e2e/scan_skip.rs +++ b/tests/cloud_commands_e2e/scan_skip.rs @@ -32,6 +32,16 @@ fn prior_scan(sha: &str, created_at: &str) -> Value { }) } +/// The trunk baseline lookup a dirty run makes once --ignore-dirty-worktree +/// puts incremental back on the table. +fn baseline_lookup_for_branch(branch: &'static str, scans: Vec) -> ExpectedRequest { + expected_request( + "look up a baseline scan to diff against", + move |request| assert_baseline_lookup_request(request, PROJECT, branch), + json_response(scans_response(scans)), + ) +} + fn commit_lookup(sha: &str, scans: Vec) -> ExpectedRequest { let sha = sha.to_string(); expected_request( @@ -428,14 +438,17 @@ fn ignore_dirty_worktree_reuses_a_prior_dirty_scan() { assert!(!stdout.contains("Scanning with BLAST"), "{context}"); } -/// The override is only a reuse rule. When nothing can be reused, the new -/// scan still sends the real dirty status. +/// When nothing can be reused, the new scan still sends the real dirty status. +/// The override does not launder it -- it only lets the diff measure the +/// working tree, which is why a baseline lookup follows the reuse lookup here. #[test] fn ignore_dirty_worktree_still_uploads_dirty_when_nothing_is_reused() { let project = git_project(); std::fs::write(project.path().join("main.py"), "print('dirty')\n") .expect("modify tracked file"); let mut plan = blast_upload_plan(&project.sha, true, false); + plan.insert(1, baseline_lookup_for_branch("master", vec![])); + plan.insert(1, baseline_lookup_for_branch("main", vec![])); plan.insert(1, commit_lookup(&project.sha, vec![])); let api = ApiStub::start(plan); let (mut command, _home) = cloud_command(&api, project.path()); @@ -661,25 +674,27 @@ fn the_window_cannot_be_set_without_the_skip_flag() { ); } -/// `--ignore-dirty-worktree` only changes reuse; it is meaningless without -/// `--skip-if-commit-scanned-recently`. +/// `--ignore-dirty-worktree` stands alone now: it governs the incremental diff +/// as well as reuse, so a run may pass it without the reuse flag. Covered end +/// to end in `scan_incremental`; this only asserts clap accepts it. #[test] -fn ignore_dirty_worktree_cannot_be_set_without_the_skip_flag() { - let api = ApiStub::start(Vec::new()); +fn ignore_dirty_worktree_may_be_set_without_the_skip_flag() { let project = git_project(); + let api = ApiStub::start(blast_upload_plan(&project.sha, false, false)); let (mut command, _home) = cloud_command(&api, project.path()); - command.args(["scan", "blast", "--ignore-dirty-worktree"]); + command.args([ + "scan", + "blast", + "--ignore-dirty-worktree", + "--project-name", + PROJECT, + ]); let output = run_with_timeout(command, &api); let transcript = api.assert_finished(); let context = output_context(&output, &transcript); - let stderr = String::from_utf8_lossy(&output.stderr); - assert_eq!(output.status.code(), Some(2), "{context}"); - assert!( - stderr.contains("--skip-if-commit-scanned-recently"), - "{context}" - ); + assert_eq!(output.status.code(), Some(0), "{context}"); } /// A backend that predates the `sha` filter answers with the project's scans