From acfefffe6fac2a119db1ce1c2c3a43fce654629a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 7 Oct 2026 07:05:28 +0000 Subject: [PATCH 1/3] Register --only-uncommitted scans as partial scans Pack the whole project and send the uncommitted files as files_to_scan with partial_scan=true, the way pull request scans are registered, instead of an archive of only those files that the server took for the branch's full state. --- src/main.rs | 7 +- src/manifest.rs | 13 +- src/scanners/blast.rs | 219 ++++++++++++++++++++++++-------- src/utils/api.rs | 19 +++ src/utils/generic.rs | 14 +- tests/cli_scan_include_image.rs | 42 +++++- 6 files changed, 249 insertions(+), 65 deletions(-) diff --git a/src/main.rs b/src/main.rs index 3d8456b..f4c574d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -96,12 +96,15 @@ enum Commands { )] fail_on: Option, - #[arg(long, help = "Only scan uncommitted changes.")] + #[arg( + long, + help = "Only scan uncommitted changes. The whole project is still uploaded, so the analysis has the surrounding code for context, but only staged, modified and untracked files are analyzed. Corgea records this as a partial scan, like a pull request scan, so it never replaces the branch's latest full scan." + )] 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 or --target already narrow the upload to chosen files. With --exclude it still runs, from the stored file checksums of an earlier scan rather than a git diff." + 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 --target narrows the upload to chosen files or --only-uncommitted narrows the analysis to them. With --exclude it still runs, from the stored file checksums of an earlier scan rather than a git diff." )] disable_incremental: bool, diff --git a/src/manifest.rs b/src/manifest.rs index 07fd02e..2b5ed70 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -40,12 +40,13 @@ //! analyzed again. //! //! The archive has to be the whole project under whatever exclude set built -//! it. A `--target` or `--only-uncommitted` run is not that: it names the files -//! to pack, so the archive holds what someone pointed at this once, and a -//! manifest of it reads as every other file having been deleted -- every one -//! of their findings dropped without anything having looked at them. Those -//! runs build no manifest, which is the same reason they already skip -//! incremental. +//! it. A `--target` run is not that: it names the files to pack, so the +//! archive holds what someone pointed at this once, and a manifest of it reads +//! as every other file having been deleted -- every one of their findings +//! dropped without anything having looked at them. Those runs build no +//! manifest, which is the same reason they already skip incremental. An +//! `--only-uncommitted` run packs the whole project but is a partial scan, which +//! the server never stores a manifest for, so it skips hashing too. use flate2::read::GzDecoder; use flate2::write::GzEncoder; diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index b375ff8..cf20306 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -5,7 +5,7 @@ use crate::scan::build_scan_url; use crate::targets; use crate::utils; use crate::utils::api::SCAIssue; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::env; use std::fs; use std::path::{Path, PathBuf}; @@ -17,6 +17,9 @@ use std::time::{Duration, Instant}; /// Same shape as the `--target` file preview. const FORCE_INCLUDE_PREVIEW: usize = 20; +/// What `--only-uncommitted` analyzes. +const UNCOMMITTED_SELECTORS: &str = "git:staged,git:modified,git:untracked"; + /// Overrides how long `wait_for_scan` polls before giving up. const SCAN_TIMEOUT_ENV: &str = "CORGEA_SCAN_TIMEOUT_SECONDS"; const DEFAULT_SCAN_TIMEOUT: Duration = Duration::from_secs(10 * 60 * 60); @@ -315,19 +318,59 @@ pub fn run( } } -/// Repo-relative paths as the `/`-separated strings the server's file lists use. +/// A repo-relative path as the `/`-separated string the server's file lists use. +fn repo_relative_string(path: &Path) -> String { + path.components() + .map(|component| component.as_os_str().to_string_lossy()) + .collect::>() + .join("/") +} + fn repo_relative_strings(paths: &[PathBuf]) -> Vec { paths .iter() - .map(|path| { - path.components() - .map(|component| component.as_os_str().to_string_lossy()) - .collect::>() - .join("/") - }) + .map(|path| repo_relative_string(path)) .collect() } +/// The files a partial scan asks the server to analyze, as the archive names +/// them, and the selected files the archive does not hold. +/// +/// `selected` are absolute paths, matched against the archive relative to the +/// first of `roots` they sit under. The archive leaves out what packaging +/// filters (vendored, generated and other unscannable files) and anything +/// outside the directory it walked; naming one of those would fail the whole +/// upload server-side, so they are reported as skipped instead. Force-included +/// files follow the selection: every run analyzes them. +fn partial_scan_files( + selected: &[PathBuf], + roots: &[PathBuf], + force_included: &[PathBuf], + archived: &HashSet, +) -> (Vec, Vec) { + let mut to_scan = Vec::new(); + let mut skipped = Vec::new(); + for file in selected { + let Some(relative) = roots.iter().find_map(|root| file.strip_prefix(root).ok()) else { + skipped.push(file.display().to_string()); + continue; + }; + let name = repo_relative_string(relative); + if archived.contains(&name) { + to_scan.push(name); + } else { + skipped.push(name); + } + } + let mut seen: HashSet = to_scan.iter().cloned().collect(); + to_scan.extend( + repo_relative_strings(force_included) + .into_iter() + .filter(|name| archived.contains(name) && seen.insert(name.clone())), + ); + (to_scan, skipped) +} + /// Package the project, upload it, and wait for the scan to finish. /// /// Returns the new scan's id and, when the server reported one, its project id. @@ -393,10 +436,28 @@ fn start_new_scan( ); }); - let target_str: Option<&str> = if *only_uncommitted { - Some("git:staged,git:modified,git:untracked") + // Only --target narrows the archive. --only-uncommitted packs the whole + // project and names its files in the upload, the way a pull request scan is + // registered: an archive of just those files reads to the server as the + // branch's full state, hiding every finding outside them. + let target_str: Option<&str> = target.as_deref(); + let uncommitted_files = if *only_uncommitted { + match targets::resolve_targets_with_exclude(UNCOMMITTED_SELECTORS, exclude.as_deref()) { + Ok(result) => Some(result.files), + Err(e) => { + *stop_signal.lock().unwrap() = true; + let _ = packaging_thread.join(); + print!( + "\r{}", + utils::terminal::set_text_color("", utils::terminal::TerminalColor::Reset) + ); + log::error!("\n\nError resolving uncommitted changes: {}\n", e); + let _ = utils::generic::delete_directory(&temp_dir); + std::process::exit(1); + } + } } else { - target.as_deref() + None }; // Before packaging: mid-pack HEAD move must not look like a clean new SHA. @@ -488,36 +549,22 @@ fn start_new_scan( ); } else { let file_count = result.files.len(); - if *only_uncommitted { - println!("\rFiles to be submitted for partial scan:\n"); - for (index, file) in result.files.iter().enumerate() { - if let Ok(relative) = - file.strip_prefix(std::env::current_dir().unwrap_or_default()) - { - println!("{}: {}", index + 1, relative.display()); - } else { - println!("{}: {}", index + 1, file.display()); - } + println!("Scanning {} files (target mode)", file_count); + + let display_count = std::cmp::min(20, file_count); + for file in result.files.iter().take(display_count) { + if let Ok(relative) = + file.strip_prefix(std::env::current_dir().unwrap_or_default()) + { + println!(" {}", relative.display()); + } else { + println!(" {}", file.display()); } - println!(); - } else { - println!("Scanning {} files (target mode)", file_count); - - let display_count = std::cmp::min(20, file_count); - for file in result.files.iter().take(display_count) { - if let Ok(relative) = - file.strip_prefix(std::env::current_dir().unwrap_or_default()) - { - println!(" {}", relative.display()); - } else { - println!(" {}", file.display()); - } - } - if file_count > display_count { - println!(" (+{} more)", file_count - display_count); - } - println!(); } + if file_count > display_count { + println!(" (+{} more)", file_count - display_count); + } + println!(); } } Err(e) => { @@ -541,7 +588,7 @@ fn start_new_scan( exclude.as_deref(), &force_included, &extra_zip_files, - !*disable_incremental, + !*disable_incremental && !*only_uncommitted, ) { Ok(archive) => { if archive.added_files.is_empty() { @@ -551,13 +598,7 @@ fn start_new_scan( "\r{}", utils::terminal::set_text_color("", utils::terminal::TerminalColor::Reset) ); - if *only_uncommitted { - log::error!( - "\n\nOops! It seems there are no scannable uncommitted changes in your project.\nYou may have uncommitted changes, but none match the types of files we can scan.\n\n" - ); - } else { - log::error!("\n\nOops! No valid files found to scan after filtering.\n\n"); - } + log::error!("\n\nOops! No valid files found to scan after filtering.\n\n"); let _ = utils::generic::delete_directory(&temp_dir); std::process::exit(1); } @@ -584,6 +625,53 @@ fn start_new_scan( "\r{}Project packaged successfully.\n", utils::terminal::set_text_color("", utils::terminal::TerminalColor::Green) ); + let partial_files = uncommitted_files.map(|selected| { + let cwd = env::current_dir().unwrap_or_default(); + // Git reports paths under the repository's resolved root, which differs + // from the working directory when it is reached through a symlink. + let roots: Vec = cwd.canonicalize().into_iter().chain([cwd]).collect(); + let (files, skipped) = partial_scan_files( + &selected, + &roots, + &force_included, + &archive_contents.entry_names, + ); + if !skipped.is_empty() { + log::warn!( + "\n{}", + utils::terminal::set_text_color( + "⚠️ Skipping uncommitted files Corgea does not analyze (vendored, generated, outside this directory, or not source code):", + utils::terminal::TerminalColor::Yellow + ) + ); + for file in &skipped { + log::warn!(" • {file}"); + } + } + if files.is_empty() { + if image_archives.is_empty() { + log::error!( + "\n\nOops! It seems there are no scannable uncommitted changes in your project.\nYou may have uncommitted changes, but none match the types of files we can scan.\n\n" + ); + let _ = utils::generic::delete_directory(&temp_dir); + std::process::exit(1); + } + log::warn!( + "\n{}", + utils::terminal::set_text_color( + "⚠️ No scannable uncommitted changes, so this scan covers only the included container image(s).", + utils::terminal::TerminalColor::Yellow + ) + ); + } else { + println!("\nFiles to be submitted for partial scan:\n"); + for (index, file) in files.iter().enumerate() { + println!("{}: {}", index + 1, file); + } + println!(); + } + files + }); let repo_after = utils::generic::get_repo_info_for_scan("./").unwrap_or_default(); // Notice = what `git status` shows, from the raw samples (so neither // --target/--exclude nor SHA drift, which the upload flag also covers). @@ -611,8 +699,10 @@ fn start_new_scan( // --exclude that is the whole of what it costs: the flag rules this scan // out as a *commit* baseline, which is right because a git diff from its // commit would describe files it never uploaded, while the checksums it - // stores still make it one a later run can subtract from. - if target_str.is_some() || exclude.is_some() { + // stores still make it one a later run can subtract from. A partial scan + // covers only its files, so it must not pass for the commit's full scan: + // the server dedupes a scan only against clean uploads of the same commit. + if target_str.is_some() || exclude.is_some() || partial_files.is_some() { if let Some(ref mut info) = repo_info { info.dirty = true; } @@ -623,9 +713,10 @@ fn start_new_scan( // "scanning every file" either, so no message is honest. --exclude is not // one of them -- it narrows the same whole-project walk, which the archive's // own checksums describe exactly -- so those runs resolve a plan like any - // other and say what came of it. + // other and say what came of it. A partial scan already names what to + // analyze and carries nothing forward, so it is silent too. let targeted_archive = target_str.is_some(); - let incremental = if *disable_incremental || targeted_archive { + let incremental = if *disable_incremental || targeted_archive || partial_files.is_some() { None } else { // Reconciled repo info, so a tree that turned out dirty — or a HEAD @@ -672,6 +763,7 @@ fn start_new_scan( incremental, include_paths: include_rules.cli_patterns, file_manifest, + partial_files, }, ) { Ok(result) => result, @@ -2149,4 +2241,29 @@ mod tests { assert!(warnings.contains(" - pom.xml could not be resolved")); assert!(warnings.contains(" - No details provided.")); } + + #[test] + fn partial_scan_files_names_archived_files_and_reports_the_rest() { + let root = PathBuf::from("/work/repo"); + let archived: HashSet = ["src/app.py", "src/forced.py", "src/also.py"] + .into_iter() + .map(String::from) + .collect(); + let selected = [ + root.join("src/app.py"), + root.join("vendor/lib.js"), + PathBuf::from("/elsewhere/other.py"), + ]; + let force_included = [PathBuf::from("src/forced.py"), PathBuf::from("src/app.py")]; + + let (to_scan, skipped) = partial_scan_files( + &selected, + &[PathBuf::from("/resolved/repo"), root.clone()], + &force_included, + &archived, + ); + + assert_eq!(to_scan, ["src/app.py", "src/forced.py"]); + assert_eq!(skipped, ["vendor/lib.js", "/elsewhere/other.py"]); + } } diff --git a/src/utils/api.rs b/src/utils/api.rs index 1226f36..7526de7 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -555,6 +555,11 @@ pub struct UploadOptions { /// Digests of everything in the archive, stored with the scan for a later /// run to diff against. Set only when the archive is the whole project. pub file_manifest: Option, + /// Zip entry names to analyze out of a whole-project archive. Set, even + /// when empty, for a partial scan, which the server registers the way it + /// does a pull request scan: findings for these files only, never standing + /// in for the branch's state. + pub partial_files: Option>, } pub fn upload_zip( @@ -571,6 +576,7 @@ pub fn upload_zip( incremental, include_paths, file_manifest, + partial_files, } = options; let include_paths_field = match include_paths.is_empty() { true => None, @@ -582,6 +588,14 @@ pub fn upload_zip( } }, }; + // JSON rather than the comma-separated form the server also accepts: a + // path may contain a comma. Failing here, rather than dropping the list, + // keeps a partial scan from registering as a full one. + let partial_files_field = partial_files + .as_ref() + .map(serde_json::to_string) + .transpose() + .map_err(|e| format!("Could not serialize the files to scan: {e}"))?; 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(); @@ -726,6 +740,11 @@ pub fn upload_zip( if let Some(patterns) = &include_paths_field { form = form.part("include_paths", multipart::Part::text(patterns.clone())); } + if let Some(files) = &partial_files_field { + form = form + .part("partial_scan", multipart::Part::text("true")) + .part("files_to_scan", multipart::Part::text(files.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 diff --git a/src/utils/generic.rs b/src/utils/generic.rs index a2d70f7..03457d9 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -66,11 +66,14 @@ const DEFAULT_EXCLUDE_GLOBS: &[&str] = &[ pub struct ArchiveContents { /// Source paths of everything added, in the order they were written. pub added_files: Vec, + /// Zip entry names of the project files added, which is how the server + /// names a file in a partial scan's list. + pub entry_names: HashSet, /// Digest of every archived file, keyed by its zip entry name. /// - /// Only built for a whole-project archive. A `--target` or - /// `--only-uncommitted` run packs a chosen handful of files, and a manifest - /// of those reads to the server as every other file having been deleted. + /// Only built for a whole-project archive. A `--target` run packs a chosen + /// handful of files, and a manifest of those reads to the server as every + /// other file having been deleted. pub manifest: Option, } @@ -252,6 +255,7 @@ pub fn create_zip_from_target>( .unix_permissions(0o755); let mut added_files = Vec::new(); + let mut entry_names = HashSet::new(); let mut excluded_files = Vec::new(); // Hashing rides along on the copy that compresses each file, so the archive // is still read once. Skipped outright when the caller has already decided @@ -273,13 +277,14 @@ pub fn create_zip_from_target>( Some(manifest) => { let mut tee = TeeWriter::new(&mut zip); io::copy(&mut file, &mut tee)?; - manifest.insert(entry_name, tee.finish()); + manifest.insert(entry_name.clone(), tee.finish()); } None => { io::copy(&mut file, &mut zip)?; } } added_files.push(path); + entry_names.insert(entry_name); } else if path.is_dir() { zip.add_directory(directory_entry_name(&relative_path), options)?; } @@ -334,6 +339,7 @@ pub fn create_zip_from_target>( zip.finish()?; Ok(ArchiveContents { added_files, + entry_names, manifest, }) } diff --git a/tests/cli_scan_include_image.rs b/tests/cli_scan_include_image.rs index 5073fa1..7f81f8a 100644 --- a/tests/cli_scan_include_image.rs +++ b/tests/cli_scan_include_image.rs @@ -2,7 +2,8 @@ //! binary through the blast scan flow against a stubbed HTTP server with a //! stubbed container CLI, and asserts the exported image archive is bundled //! into the uploaded project zip under the name the backend greps for -//! (`corgea-image-scanning-*.tar`). +//! (`corgea-image-scanning-*.tar`). Also covers how `--only-uncommitted` +//! registers its upload as a partial scan. mod common; @@ -200,7 +201,44 @@ fn scan_only_uncommitted_with_include_image_uploads_the_archive() { stderr.contains("only the included container image"), "should say the scan covers only the image, got:\n{stderr}" ); - assert!(uploaded_text(&uploads).contains("corgea-image-scanning-myapp-1.0.tar")); + let uploaded = uploaded_text(&uploads); + assert!(uploaded.contains("corgea-image-scanning-myapp-1.0.tar")); + assert!( + uploaded.contains("name=\"files_to_scan\"\r\n\r\n[]"), + "still a partial scan, of no source files" + ); +} + +/// `--only-uncommitted` uploads the whole project and names the changed files, +/// the way a pull request scan is registered, so the server records a partial +/// scan rather than a branch state holding only those files. +#[test] +fn scan_only_uncommitted_uploads_the_project_and_names_the_changed_files() { + let (base_url, uploads) = spawn_recording_scan_stub("scan-uncommitted"); + let project = stub_project(); + commit_everything(project.path()); + fs::write(project.path().join("edited.py"), "print(2)\n").expect("write new file"); + + let (mut cmd, _home) = corgea_isolated(); + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan", "--only-uncommitted"]); + + let output = cmd.output().expect("run corgea scan --only-uncommitted"); + assert!( + output.status.success(), + "stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + let uploaded = uploaded_text(&uploads); + assert!(uploaded.contains("main.py"), "committed files upload too"); + assert!(uploaded.contains("name=\"partial_scan\"\r\n\r\ntrue")); + assert!(uploaded.contains("name=\"files_to_scan\"\r\n\r\n[\"edited.py\"]")); + assert!(uploaded.contains("name=\"dirty\"\r\n\r\ntrue")); + assert!(!uploaded.contains("name=\"file_manifest\"")); } /// A copy of an archive living in the repository must not ride along: it would put From b552c45045ed23256daf6454b654ed5d6e00cd1e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 18 Oct 2026 12:41:07 +0000 Subject: [PATCH 2/3] Pack uncommitted files the project walk skips The whole-project walk leaves out hidden paths and tracked files matching an ignore rule, so --only-uncommitted dropped changes to files like .github/workflows/ci.yml from the analysis, and a commit touching only those failed with no scannable changes. Add the uncommitted files to the archive, still under the default excludes, so the partial scan can name them. Co-authored-by: ibrahim --- src/scanners/blast.rs | 19 +++++++--- src/utils/generic.rs | 33 ++++++++++++----- tests/cli_scan_include_image.rs | 64 ++++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 14 deletions(-) diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index fd45d0f..0130173 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -460,6 +460,20 @@ fn start_new_scan( } else { None }; + let cwd = env::current_dir().unwrap_or_default(); + // Git reports paths under the repository's resolved root, which differs + // from the working directory when it is reached through a symlink. + let roots: Vec = cwd.canonicalize().into_iter().chain([cwd]).collect(); + let uncommitted_in_walk: Vec = uncommitted_files + .iter() + .flatten() + .filter_map(|file| { + roots + .iter() + .find_map(|root| file.strip_prefix(root).ok()) + .map(Path::to_path_buf) + }) + .collect(); // Before packaging: mid-pack HEAD move must not look like a clean new SHA. let repo_before = utils::generic::get_repo_info_for_scan("./").unwrap_or_default(); @@ -588,6 +602,7 @@ fn start_new_scan( None, exclude.as_deref(), &force_included, + &uncommitted_in_walk, &extra_zip_files, !*disable_incremental && !*only_uncommitted, ) { @@ -627,10 +642,6 @@ fn start_new_scan( utils::terminal::set_text_color("", utils::terminal::TerminalColor::Green) ); let partial_files = uncommitted_files.map(|selected| { - let cwd = env::current_dir().unwrap_or_default(); - // Git reports paths under the repository's resolved root, which differs - // from the working directory when it is reached through a symlink. - let roots: Vec = cwd.canonicalize().into_iter().chain([cwd]).collect(); let (files, skipped) = partial_scan_files( &selected, &roots, diff --git a/src/utils/generic.rs b/src/utils/generic.rs index ca2e73a..8081cec 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -163,18 +163,26 @@ fn directory_entry_name(relative_path: &Path) -> String { /// `--include` matched. They override every filter here — the default /// excludes, `--exclude`, and `.gitignore` — because a file left out of the /// archive cannot be scanned whatever the engine later decides about it. +/// - `uncommitted` are paths, relative to the walk, that git reports changed. +/// The whole-project walk skips hidden paths and `.gitignore` matches even +/// when git tracks them, so a change to `.github/workflows/ci.yml` would +/// never reach the archive a partial scan names its files from. They still +/// go through the default excludes; `--exclude` was applied when they were +/// resolved. /// - `extra_files` are staged files added to the root of the zip as /// `(source path, zip entry name)`. They come from explicit flags such as /// `--include-image`, so exclude rules don't apply to them. /// - `want_manifest` is the caller saying it has a use for the digests. A run /// that cannot scan incrementally should pass `false` and not pay to hash /// every file it packs. +#[allow(clippy::too_many_arguments)] pub fn create_zip_from_target>( target: Option<&str>, output_zip: P, exclude_globs: Option<&[&str]>, user_exclude: Option<&str>, force_include: &[PathBuf], + uncommitted: &[PathBuf], extra_files: &[(PathBuf, String)], want_manifest: bool, ) -> Result> { @@ -234,15 +242,13 @@ pub fn create_zip_from_target>( }; let forced: HashSet<&Path> = force_include.iter().map(PathBuf::as_path).collect(); - let already_present: HashSet = files_to_zip + let mut already_present: HashSet = files_to_zip .iter() .map(|(_, relative)| relative.clone()) .collect(); - for relative in force_include { - if already_present.contains(relative) { - continue; - } - if relative.is_file() { + let uncommitted = if target.is_none() { uncommitted } else { &[] }; + for relative in force_include.iter().chain(uncommitted) { + if relative.is_file() && already_present.insert(relative.clone()) { files_to_zip.push((relative.clone(), relative.clone())); } } @@ -261,7 +267,11 @@ pub fn create_zip_from_target>( // is still read once. Skipped outright when the caller has already decided // it wants no manifest, rather than hashing every file in the project for // a value that is then dropped. - let mut manifest = (want_manifest && archives_whole_project(".", target)).then(Manifest::new); + // Uncommitted paths can add files the walk skipped, making the archive more + // than the project under its exclude set, which is what a manifest describes. + let mut manifest = + (want_manifest && uncommitted.is_empty() && archives_whole_project(".", target)) + .then(Manifest::new); for (path, relative_path) in files_to_zip { // Match repo-relative paths so abs `/tmp/...` targets don't hit `**/tmp/**`. @@ -1090,6 +1100,7 @@ mod tests { None, &[], &[], + &[], true, ) .expect("zip creation should succeed"); @@ -1237,6 +1248,7 @@ mod tests { None, std::slice::from_ref(&forced), &[], + &[], false, ) .expect("zip creation should succeed") @@ -1281,6 +1293,7 @@ mod tests { Some(&[]), None, &[], + &[], &extra_files, true, ) @@ -1327,6 +1340,7 @@ mod tests { Some(&[]), None, &[], + &[], &extra_files, true, ) @@ -1460,7 +1474,8 @@ mod tests { let output_zip = root.join("out.zip"); let previous = env::current_dir().unwrap(); env::set_current_dir(root).unwrap(); - let contents = create_zip_from_target(None, "out.zip", None, None, &[], &extra_files, true); + let contents = + create_zip_from_target(None, "out.zip", None, None, &[], &[], &extra_files, true); env::set_current_dir(previous).unwrap(); let manifest = contents @@ -1520,7 +1535,7 @@ mod tests { let previous = env::current_dir().unwrap(); env::set_current_dir(root).unwrap(); let contents = - create_zip_from_target(None, "out.zip", None, None, &[], &extra_files, true); + create_zip_from_target(None, "out.zip", None, None, &[], &[], &extra_files, true); env::set_current_dir(previous).unwrap(); let archive = contents.expect("zip creation should succeed"); diff --git a/tests/cli_scan_include_image.rs b/tests/cli_scan_include_image.rs index 7f81f8a..97f2190 100644 --- a/tests/cli_scan_include_image.rs +++ b/tests/cli_scan_include_image.rs @@ -212,11 +212,22 @@ fn scan_only_uncommitted_with_include_image_uploads_the_archive() { /// `--only-uncommitted` uploads the whole project and names the changed files, /// the way a pull request scan is registered, so the server records a partial /// scan rather than a branch state holding only those files. +/// +/// Hidden paths and tracked files that match an ignore rule are left out of the +/// whole-project walk, yet git still reports their changes; they have to be in +/// the archive for the server to accept them in the list. #[test] fn scan_only_uncommitted_uploads_the_project_and_names_the_changed_files() { let (base_url, uploads) = spawn_recording_scan_stub("scan-uncommitted"); let project = stub_project(); + let workflow = project.path().join(".github/workflows/ci.yml"); + fs::create_dir_all(workflow.parent().unwrap()).expect("create workflow dir"); + fs::write(&workflow, "on: push\n").expect("write workflow"); + fs::write(project.path().join("ignored.py"), "print(1)\n").expect("write ignored file"); commit_everything(project.path()); + fs::write(project.path().join(".git/info/exclude"), "ignored.py\n").expect("write exclude"); + fs::write(&workflow, "on: pull_request\n").expect("edit workflow"); + fs::write(project.path().join("ignored.py"), "print(3)\n").expect("edit ignored file"); fs::write(project.path().join("edited.py"), "print(2)\n").expect("write new file"); let (mut cmd, _home) = corgea_isolated(); @@ -236,11 +247,62 @@ fn scan_only_uncommitted_uploads_the_project_and_names_the_changed_files() { let uploaded = uploaded_text(&uploads); assert!(uploaded.contains("main.py"), "committed files upload too"); assert!(uploaded.contains("name=\"partial_scan\"\r\n\r\ntrue")); - assert!(uploaded.contains("name=\"files_to_scan\"\r\n\r\n[\"edited.py\"]")); + let field = "name=\"files_to_scan\"\r\n\r\n"; + let list = uploaded + .split_once(field) + .and_then(|(_, rest)| rest.split_once("\r\n")) + .map(|(list, _)| list) + .expect("files_to_scan field"); + let mut named: Vec = serde_json::from_str(list).expect("files_to_scan is JSON"); + named.sort(); + assert_eq!( + named, + [".github/workflows/ci.yml", "edited.py", "ignored.py"] + ); + let archive = uploaded.replace(&format!("{field}{list}"), ""); + for name in &named { + assert!(archive.contains(name.as_str()), "{name} is in the archive"); + } assert!(uploaded.contains("name=\"dirty\"\r\n\r\ntrue")); assert!(!uploaded.contains("name=\"file_manifest\"")); } +/// An uncommitted file is still held to the default excludes, and one that is +/// also force-included is packed once: a second entry of the same name would +/// fail the archive. +#[test] +fn scan_only_uncommitted_keeps_default_excludes_and_packs_a_forced_file_once() { + let (base_url, uploads) = spawn_recording_scan_stub("scan-uncommitted-forced"); + let project = stub_project(); + let script = project.path().join(".github/deploy.sh"); + fs::create_dir_all(script.parent().unwrap()).expect("create .github"); + fs::write(&script, "echo 1\n").expect("write script"); + fs::write(project.path().join("ignored.py"), "print(1)\n").expect("write ignored file"); + commit_everything(project.path()); + fs::write(project.path().join(".git/info/exclude"), "ignored.py\n").expect("write exclude"); + fs::write(&script, "echo 2\n").expect("edit script"); + fs::write(project.path().join("ignored.py"), "print(3)\n").expect("edit ignored file"); + + let (mut cmd, _home) = corgea_isolated(); + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan", "--only-uncommitted", "--include", "ignored.py"]); + + let output = cmd.output().expect("run corgea scan --only-uncommitted"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "stdout:\n{}\nstderr:\n{stderr}", + String::from_utf8_lossy(&output.stdout), + ); + assert!(stderr.contains(".github/deploy.sh"), "reported as skipped"); + + let uploaded = uploaded_text(&uploads); + assert!(uploaded.contains("name=\"files_to_scan\"\r\n\r\n[\"ignored.py\"]")); + assert!(!uploaded.contains(".github/deploy.sh")); +} + /// A copy of an archive living in the repository must not ride along: it would put /// the backend into image-scanning mode on scans that never asked for it. #[test] From a2ceed259fd44604822133129bf56a657f74d4d3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 21 Oct 2026 10:11:06 +0000 Subject: [PATCH 3/3] Keep --only-uncommitted working on webapps older than v1.82.0 Read GET /api/version and send the partial-scan upload only when the server is v1.82.0 or newer, the release that registers a JSON files_to_scan. Older servers, and any server that does not report a version, still receive an archive of just the changed files. Co-authored-by: ibrahim --- skills/corgea/SKILL.md | 2 + src/main.rs | 2 +- src/manifest.rs | 6 +- src/scanners/blast.rs | 126 +++++++++++++++++++++++++++++--- tests/cli_scan_include_image.rs | 68 ++++++++++++++++- 5 files changed, 191 insertions(+), 13 deletions(-) diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index 1cecb72..58387c0 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -95,6 +95,8 @@ Removing, disabling or editing a project ignore rule (scan settings → File Ign `--only-uncommitted` and `--target` are mutually exclusive. `--fail-on`, `--fail`, and `--block-on` are mutually exclusive. +On Corgea webapp v1.82.0 or newer, `--only-uncommitted` uploads the whole project and names the changed files, so the server records a partial scan and does not replace the branch's latest full scan. An older webapp is sent only the changed files — the previous behavior — which it records as the branch's latest scan. The CLI reads `GET /api/version` to choose; a missing or unreadable version takes the older upload, because that server would reject the partial-scan one. + `--out-format`/`--out-file` and `--sbom` are honored regardless of the gate: the report and the SBOM are written before `--fail`/`--block-on` are evaluated, so a scan that exits 1 on a blocking rule still leaves the report file behind for the pipeline to ingest. `--skip-if-commit-scanned-recently` reuses the project's most recent reusable scan of the current commit instead of starting a duplicate, when one ran inside the `--scanned-within` window (default `24h`; accepts `90s`, `30m`, `4h`, `7d`, and a bare number as hours). The reused scan takes the new scan's place for the rest of the command — results table, `--block-on` gate and its exit code, `--out-file` report — so the pipeline behaves the same either way. It prints `CORGEA_SCAN_SKIPPED=true` plus `CORGEA_SCAN_ID=` on a reuse and `CORGEA_SCAN_SKIPPED=false` when a scan runs, so a later step can branch on it. diff --git a/src/main.rs b/src/main.rs index f182b30..9563185 100644 --- a/src/main.rs +++ b/src/main.rs @@ -101,7 +101,7 @@ enum Commands { #[arg( long, - help = "Only scan uncommitted changes. The whole project is still uploaded, so the analysis has the surrounding code for context, but only staged, modified and untracked files are analyzed. Corgea records this as a partial scan, like a pull request scan, so it never replaces the branch's latest full scan." + help = "Only scan uncommitted changes. On Corgea webapp v1.82.0 or newer, the whole project is still uploaded so the analysis has the surrounding code for context, but only staged, modified and untracked files are analyzed, and Corgea records this as a partial scan, like a pull request scan, so it never replaces the branch's latest full scan. An older webapp is sent only the changed files, which it records as the branch's latest scan." )] only_uncommitted: bool, diff --git a/src/manifest.rs b/src/manifest.rs index d4798c8..d61a98e 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -45,8 +45,10 @@ //! as every other file having been deleted -- every one of their findings //! dropped without anything having looked at them. Those runs build no //! manifest, which is the same reason they already skip incremental. An -//! `--only-uncommitted` run packs the whole project but is a partial scan, which -//! the server never stores a manifest for, so it skips hashing too. +//! `--only-uncommitted` run against a webapp that registers partial scans packs +//! the whole project but is a partial scan, which the server never stores a +//! manifest for, so it skips hashing too. An older webapp is sent only the +//! changed files, the same shape as `--target`, and builds no manifest either. use flate2::read::GzDecoder; use flate2::write::GzEncoder; diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 7448a4b..72a13f2 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -21,6 +21,12 @@ const FORCE_INCLUDE_PREVIEW: usize = 20; /// What `--only-uncommitted` analyzes. const UNCOMMITTED_SELECTORS: &str = "git:staged,git:modified,git:untracked"; +/// Doghouse first registers a CLI partial scan — `partial_scan` plus a JSON +/// `files_to_scan` — in this release (Corgea/doghouse#2348, tagged v1.82.0). +/// An older server splits that field on commas, cannot find the JSON text in +/// the archive, and rejects the upload. +const PARTIAL_SCAN_WEBAPP_VERSION: &str = "v1.82.0"; + /// Overrides how long `wait_for_scan` polls before giving up. const SCAN_TIMEOUT_ENV: &str = "CORGEA_SCAN_TIMEOUT_SECONDS"; const DEFAULT_SCAN_TIMEOUT: Duration = Duration::from_secs(10 * 60 * 60); @@ -335,6 +341,54 @@ fn repo_relative_strings(paths: &[PathBuf]) -> Vec { .collect() } +/// Whether `version` is a webapp release that registers CLI partial scans. +/// +/// `None`, and any text that is not a release number, count as too old. +/// Sending the partial-scan upload to a server that cannot read it fails +/// `--only-uncommitted` outright, including the pre-commit hook. The previous +/// upload still succeeds on a newer server; it is just recorded as the +/// branch's latest full scan. +fn webapp_registers_cli_partial_scans(version: Option<&str>) -> bool { + let Some(running) = version.and_then(crate::version_check::extract_version) else { + return false; + }; + let Some(required) = crate::version_check::extract_version(PARTIAL_SCAN_WEBAPP_VERSION) else { + return false; + }; + running >= required +} + +/// Read the webapp version and decide whether this `--only-uncommitted` run +/// can be registered as a partial scan. Warns when it cannot, because that run +/// then uploads only the changed files and the server records them as the +/// branch's latest full scan. +fn server_registers_cli_partial_scans(corgea_url: &str) -> bool { + let reported = match utils::api::get_webapp_version(corgea_url) { + Ok(version) => version, + Err(error) => { + log::warn!( + "Could not read the Corgea server version ({error}). Uploading only the uncommitted files; partial scans need webapp {PARTIAL_SCAN_WEBAPP_VERSION} or newer." + ); + return false; + } + }; + match reported.as_deref() { + Some(version) if webapp_registers_cli_partial_scans(Some(version)) => true, + Some(version) => { + log::warn!( + "This Corgea server ({version}) does not register partial scans yet (needs {PARTIAL_SCAN_WEBAPP_VERSION}). Uploading only the uncommitted files; this scan stands in for the branch's latest full scan until the server is upgraded." + ); + false + } + None => { + log::warn!( + "This Corgea server did not report a version this CLI understands. Uploading only the uncommitted files; partial scans need webapp {PARTIAL_SCAN_WEBAPP_VERSION} or newer." + ); + false + } + } +} + /// The files a partial scan asks the server to analyze, as the archive names /// them, and the selected files the archive does not hold. /// @@ -438,12 +492,19 @@ fn start_new_scan( ); }); - // Only --target narrows the archive. --only-uncommitted packs the whole - // project and names its files in the upload, the way a pull request scan is - // registered: an archive of just those files reads to the server as the - // branch's full state, hiding every finding outside them. - let target_str: Option<&str> = target.as_deref(); - let uncommitted_files = if *only_uncommitted { + // A server that registers partial scans gets the whole project, with the + // changed files named in the upload: an archive of just those files reads + // as the branch's full state and hides every finding outside them. An + // older server rejects that upload, so it is sent only the changed files, + // which is how `--only-uncommitted` behaved before partial scans existed. + let register_partial = + *only_uncommitted && server_registers_cli_partial_scans(&config.get_url()); + let target_str: Option<&str> = if *only_uncommitted && !register_partial { + Some(UNCOMMITTED_SELECTORS) + } else { + target.as_deref() + }; + let uncommitted_files = if register_partial { match targets::resolve_targets_with_exclude(UNCOMMITTED_SELECTORS, exclude.as_deref()) { Ok(result) => Some(result.files), Err(e) => { @@ -563,6 +624,18 @@ fn start_new_scan( utils::terminal::TerminalColor::Yellow ) ); + } else if *only_uncommitted { + println!("\rFiles to be submitted for partial scan:\n"); + for (index, file) in result.files.iter().enumerate() { + if let Ok(relative) = + file.strip_prefix(std::env::current_dir().unwrap_or_default()) + { + println!("{}: {}", index + 1, relative.display()); + } else { + println!("{}: {}", index + 1, file.display()); + } + } + println!(); } else { let file_count = result.files.len(); println!("Scanning {} files (target mode)", file_count); @@ -605,7 +678,7 @@ fn start_new_scan( &force_included, &uncommitted_in_walk, &extra_zip_files, - !*disable_incremental && !*only_uncommitted, + !*disable_incremental && !register_partial, ) { Ok(archive) => { if archive.added_files.is_empty() { @@ -615,7 +688,13 @@ fn start_new_scan( "\r{}", utils::terminal::set_text_color("", utils::terminal::TerminalColor::Reset) ); - log::error!("\n\nOops! No valid files found to scan after filtering.\n\n"); + if *only_uncommitted && !register_partial { + log::error!( + "\n\nOops! It seems there are no scannable uncommitted changes in your project.\nYou may have uncommitted changes, but none match the types of files we can scan.\n\n" + ); + } else { + log::error!("\n\nOops! No valid files found to scan after filtering.\n\n"); + } let _ = utils::generic::delete_directory(&temp_dir); std::process::exit(1); } @@ -736,7 +815,11 @@ fn start_new_scan( } else if target_str.is_some() { Err(FullScanReason::new( FullScanCause::TargetedUpload, - "--target narrowed the upload to the files it selects", + if *only_uncommitted { + "--only-uncommitted narrowed the upload to uncommitted files" + } else { + "--target narrowed the upload to the files it selects" + }, )) } else if partial_files.is_some() { Err(FullScanReason::new( @@ -2295,4 +2378,29 @@ mod tests { assert_eq!(to_scan, ["src/app.py", "src/forced.py"]); assert_eq!(skipped, ["vendor/lib.js", "/elsewhere/other.py"]); } + + #[test] + fn partial_scans_start_at_webapp_1_82_0() { + for version in ["v1.82.0", "v1.82.2", "v1.82", "v1.82.0-eu", "v2.0.0"] { + assert!( + webapp_registers_cli_partial_scans(Some(version)), + "{version} registers partial scans" + ); + } + // A suffixed build counts as the release it was cut from, so a + // customer build of v1.81.3 is still too old, and a version this CLI + // cannot read is too — sending the new upload there fails the scan. + for version in [ + "v1.81.3", + "v1.81.3-client-a", + "v1.81.1-sonae", + "not-a-version", + ] { + assert!( + !webapp_registers_cli_partial_scans(Some(version)), + "{version} keeps the previous upload" + ); + } + assert!(!webapp_registers_cli_partial_scans(None)); + } } diff --git a/tests/cli_scan_include_image.rs b/tests/cli_scan_include_image.rs index 97f2190..d40001b 100644 --- a/tests/cli_scan_include_image.rs +++ b/tests/cli_scan_include_image.rs @@ -33,13 +33,23 @@ exit 1 /// Stub container CLI that has nothing and can't pull. const BROKEN_ENGINE: &str = "#!/bin/sh\necho 'no such image' 1>&2\nexit 1\n"; +/// The blast scan route table against a webapp new enough to register partial +/// scans (doghouse v1.82.0). +fn spawn_recording_scan_stub(scan_id: &'static str) -> (String, Uploads) { + spawn_recording_scan_stub_at(scan_id, Some("v1.82.0")) +} + /// The blast scan route table (verify -> upload -> poll -> issues), with the /// upload chunk bodies captured so a test can inspect what was bundled. -fn spawn_recording_scan_stub(scan_id: &'static str) -> (String, Uploads) { +/// +/// `version` is the release `GET /api/version` reports. `None` answers 404, +/// the way a webapp predating that endpoint does. +fn spawn_recording_scan_stub_at(scan_id: &'static str, version: Option<&str>) -> (String, Uploads) { let listener = TcpListener::bind("127.0.0.1:0").expect("bind stub"); let base_url = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); let uploads: Uploads = Default::default(); let recorder = Arc::clone(&uploads); + let version_body = version.map(common::webapp_version); std::thread::spawn(move || { for stream in listener.incoming() { @@ -77,6 +87,11 @@ fn spawn_recording_scan_stub(scan_id: &'static str) -> (String, Uploads) { r#"{"status":"ok","issues":[],"page":1,"total_pages":1,"total_issues":0}"# .to_string(), ) + } else if path == "/api/version" { + match &version_body { + Some(body) => ("200 OK", body.clone()), + None => ("404 Not Found", r#"{"message":"not found"}"#.to_string()), + } } else { ("404 Not Found", r#"{"message":"not found"}"#.to_string()) }; @@ -267,6 +282,57 @@ fn scan_only_uncommitted_uploads_the_project_and_names_the_changed_files() { assert!(!uploaded.contains("name=\"file_manifest\"")); } +/// A webapp older than v1.82.0 splits a JSON `files_to_scan` on commas and +/// rejects the upload, so `--only-uncommitted` keeps the previous request: +/// an archive of only the changed files, with no partial-scan marker. A +/// missing version takes the same path, because that server would reject the +/// new upload too. +#[test] +fn scan_only_uncommitted_on_an_older_webapp_uploads_only_the_changed_files() { + for version in [Some("v1.81.3"), None] { + let (base_url, uploads) = spawn_recording_scan_stub_at("scan-uncommitted-old", version); + let project = stub_project(); + commit_everything(project.path()); + fs::write(project.path().join("edited.py"), "print(2)\n").expect("write new file"); + + let (mut cmd, _home) = corgea_isolated(); + cmd.current_dir(project.path()) + .env("CORGEA_URL", &base_url) + .env("CORGEA_TOKEN", "test-token") + .args(["scan", "--only-uncommitted"]); + + let output = cmd.output().expect("run corgea scan --only-uncommitted"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "version {version:?}\nstdout:\n{}\nstderr:\n{stderr}", + String::from_utf8_lossy(&output.stdout), + ); + assert!( + stderr.contains("Uploading only the uncommitted files"), + "version {version:?} should explain the fallback, got:\n{stderr}" + ); + if let Some(version) = version { + assert!( + stderr.contains(version), + "the warning should name the server version, got:\n{stderr}" + ); + } + + let uploaded = uploaded_text(&uploads); + assert!( + uploaded.contains("edited.py"), + "the changed file is uploaded" + ); + assert!( + !uploaded.contains("main.py"), + "a committed file stays out of the archive on an older server" + ); + assert!(!uploaded.contains("name=\"partial_scan\"")); + assert!(!uploaded.contains("name=\"files_to_scan\"")); + } +} + /// An uncommitted file is still held to the default excludes, and one that is /// also force-included is packed once: a second entry of the same name would /// fail the archive.