diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 4923f05..e63d432 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -64,7 +64,6 @@ pub fn run( fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); let project_name = utils::generic::determine_project_name(project_name.as_deref()); let zip_path = format!("{}/{}.zip", temp_dir.display(), project_name); - let repo_info = utils::generic::get_repo_info("./").unwrap_or_default(); match utils::generic::create_path_if_not_exists(&temp_dir) { Ok(_) => (), Err(e) => { @@ -91,6 +90,10 @@ pub fn run( target.as_deref() }; + // Sample before packaging so a mid-pack commit cannot advertise a new clean HEAD + // against an archive built from the previous (or mixed) tree. + let repo_before = utils::generic::get_repo_info("./").unwrap_or_default(); + if target_str.is_none() && exclude.is_some() { println!("Excluding files matching: {}", exclude.as_deref().unwrap()); } @@ -207,6 +210,35 @@ pub fn run( "\r{}Project packaged successfully.\n", utils::terminal::set_text_color("", utils::terminal::TerminalColor::Green) ); + let repo_after = utils::generic::get_repo_info("./").unwrap_or_default(); + // User notice reflects actual worktree dirtiness, not upload fail-safes + // (--target/--exclude or before/after SHA drift). + let worktree_dirty = repo_before.as_ref().is_some_and(|i| i.dirty) + || repo_after.as_ref().is_some_and(|i| i.dirty); + if worktree_dirty { + let notice_sha = repo_after + .as_ref() + .and_then(|i| i.sha.as_deref()) + .or_else(|| repo_before.as_ref().and_then(|i| i.sha.as_deref())); + match notice_sha { + Some(sha) => { + let short_sha = &sha[..sha.len().min(7)]; + println!( + "Working tree has uncommitted changes - scanning your local files, not commit {short_sha}." + ); + } + None => { + println!("Working tree has uncommitted changes - scanning your local files.") + } + } + } + let mut repo_info = utils::generic::reconcile_repo_info_for_upload(repo_before, repo_after); + // Partial archives are not an exact HEAD snapshot even on a clean tree. + if target_str.is_some() || exclude.is_some() { + if let Some(ref mut info) = repo_info { + info.dirty = true; + } + } println!("\n\nSubmitting scan to Corgea:"); let upload_result = match utils::api::upload_zip( &zip_path, diff --git a/src/utils/api.rs b/src/utils/api.rs index 6bfc784..627e169 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -18,6 +18,8 @@ use std::path::Path; const CHUNK_SIZE: usize = 50 * 1024 * 1024; // 50 MB const API_BASE: &str = "/api/v1"; +const DIRTY_TRUE: &str = "true"; +const DIRTY_FALSE: &str = "false"; fn auth_headers(token: &str) -> HeaderMap { let mut headers = HeaderMap::new(); @@ -336,6 +338,11 @@ pub fn upload_zip( if let Some(sha) = &info.sha { form = form.part("sha", multipart::Part::text(sha.to_string())); } + // Always send dirty: omitted = old CLI; false = exact clean HEAD snapshot. + form = form.part( + "dirty", + multipart::Part::text(if info.dirty { DIRTY_TRUE } else { DIRTY_FALSE }), + ); } if let Some(scan_type) = scan_type.clone() { let scan_type = if scan_type.contains("blast") { diff --git a/src/utils/generic.rs b/src/utils/generic.rs index 3d28131..b7ddaa9 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -1,5 +1,5 @@ use crate::utils::terminal::{set_text_color, TerminalColor}; -use git2::Repository; +use git2::{Repository, StatusOptions}; use globset::{Glob, GlobSetBuilder}; use ignore::WalkBuilder; use std::env; @@ -109,7 +109,9 @@ pub fn create_zip_from_target>( let mut excluded_files = Vec::new(); for (path, relative_path) in files_to_zip { - let is_excluded = glob_set.is_match(&path); + // Match against the repo-relative path. Absolute paths (target mode) + // can live under `/tmp/...` on Linux and would falsely hit `**/tmp/**`. + let is_excluded = glob_set.is_match(&relative_path); if (path.is_file() || path.is_dir()) && !is_excluded { if path.is_file() { @@ -297,13 +299,62 @@ pub fn get_repo_info(dir: &str) -> Result, git2::Error> { .map(|commit| commit.id().to_string()) }); + let dirty = is_worktree_dirty(&repo); + Ok(Some(RepoInfo { branch, repo_url: origin_url(&repo), sha, + dirty, })) } +/// True when the worktree has modified, staged, or untracked files (including +/// dirty submodules). Gitignored paths alone do not count. +/// +/// Untracked paths that packaging would later drop via `DEFAULT_EXCLUDE_GLOBS` +/// still count as dirty (false-positive dirty costs a full scan, not a miss). +/// Status errors also treat the tree as dirty so we never claim clean HEAD. +fn is_worktree_dirty(repo: &Repository) -> bool { + let mut opts = StatusOptions::new(); + // Include submodule status: packaging walks into submodule dirs, so a + // modified checkout must not be advertised as clean parent HEAD. + opts.include_untracked(true) + .recurse_untracked_dirs(true) + .include_ignored(false); + repo.statuses(Some(&mut opts)) + .map(|s| !s.is_empty()) + .unwrap_or(true) +} + +/// Merge before/after packaging samples into upload metadata. +/// +/// Only `dirty=false` when both samples exist, are clean, and share the same +/// SHA. Any missing sample, dirty sample, or SHA drift fails safe to dirty. +/// Prefer the post-packaging sample for branch/url/sha display. +pub fn reconcile_repo_info_for_upload( + before: Option, + after: Option, +) -> Option { + match (before, after) { + (None, None) => None, + (Some(sample), None) | (None, Some(sample)) => Some(RepoInfo { + dirty: true, + ..sample + }), + (Some(before), Some(after)) => { + let stable_clean = + !before.dirty && !after.dirty && before.sha.is_some() && before.sha == after.sha; + Some(RepoInfo { + branch: after.branch.or(before.branch), + repo_url: after.repo_url.or(before.repo_url), + sha: after.sha.or(before.sha), + dirty: !stable_clean, + }) + } + } +} + /// `origin`'s URL, or None when the remote is missing or carries no URL. fn origin_url(repo: &Repository) -> Option { repo.find_remote("origin") @@ -407,11 +458,13 @@ pub fn get_status(status: &str) -> &str { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct RepoInfo { pub branch: Option, pub repo_url: Option, pub sha: Option, + /// True when the upload must not be treated as an exact clean HEAD snapshot. + pub dirty: bool, } #[cfg(test)] @@ -440,12 +493,7 @@ mod tests { fn get_repo_info_at_root_only_not_nested_cwd() { let dir = tempfile::tempdir().unwrap(); let root = dir.path(); - git(root, &["init"]); - git(root, &["config", "user.email", "test@example.com"]); - git(root, &["config", "user.name", "Test"]); - fs::write(root.join("README"), "hi").unwrap(); - git(root, &["add", "README"]); - git(root, &["commit", "-m", "init"]); + init_committed_repo(root); let root_s = root.to_str().unwrap(); let nested = root.join("pkg").join("inner"); @@ -456,6 +504,7 @@ mod tests { .unwrap() .expect("repo root should yield SHA metadata"); assert!(info.sha.is_some()); + assert!(!info.dirty, "clean commit should report dirty=false"); assert!(is_at_repo_root(root_s)); assert!( @@ -465,6 +514,169 @@ mod tests { assert!(!is_at_repo_root(nested_s)); } + fn init_committed_repo(root: &std::path::Path) { + git(root, &["init"]); + git(root, &["config", "user.email", "test@example.com"]); + git(root, &["config", "user.name", "Test"]); + fs::write(root.join("README"), "hi").unwrap(); + git(root, &["add", "README"]); + git(root, &["commit", "-m", "init"]); + } + + #[test] + fn get_repo_info_dirty_true_when_tracked_file_modified() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_committed_repo(root); + fs::write(root.join("README"), "changed").unwrap(); + let info = get_repo_info(root.to_str().unwrap()) + .unwrap() + .expect("repo info"); + assert!(info.dirty); + } + + #[test] + fn get_repo_info_dirty_true_when_change_staged() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_committed_repo(root); + fs::write(root.join("README"), "staged").unwrap(); + git(root, &["add", "README"]); + let info = get_repo_info(root.to_str().unwrap()) + .unwrap() + .expect("repo info"); + assert!(info.dirty); + } + + #[test] + fn get_repo_info_dirty_true_when_untracked_file() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_committed_repo(root); + fs::write(root.join("new.py"), "print(1)").unwrap(); + let info = get_repo_info(root.to_str().unwrap()) + .unwrap() + .expect("repo info"); + assert!(info.dirty); + } + + #[test] + fn get_repo_info_dirty_false_when_only_gitignored_file() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_committed_repo(root); + fs::write(root.join(".gitignore"), "ignored.txt\n").unwrap(); + git(root, &["add", ".gitignore"]); + git(root, &["commit", "-m", "ignore"]); + fs::write(root.join("ignored.txt"), "secret").unwrap(); + let info = get_repo_info(root.to_str().unwrap()) + .unwrap() + .expect("repo info"); + assert!(!info.dirty); + } + + fn sample_info(sha: &str, dirty: bool) -> RepoInfo { + RepoInfo { + branch: Some("main".into()), + repo_url: Some("https://github.com/org/repo.git".into()), + sha: Some(sha.into()), + dirty, + } + } + + #[test] + fn reconcile_clean_same_sha_stays_clean() { + let before = sample_info("aaa", false); + let after = sample_info("aaa", false); + let out = reconcile_repo_info_for_upload(Some(before), Some(after)).unwrap(); + assert!(!out.dirty); + assert_eq!(out.sha.as_deref(), Some("aaa")); + } + + #[test] + fn reconcile_sha_drift_marks_dirty() { + let before = sample_info("aaa", false); + let after = sample_info("bbb", false); + let out = reconcile_repo_info_for_upload(Some(before), Some(after)).unwrap(); + assert!(out.dirty); + assert_eq!(out.sha.as_deref(), Some("bbb")); + } + + #[test] + fn reconcile_either_dirty_marks_dirty() { + let before = sample_info("aaa", true); + let after = sample_info("aaa", false); + let out = reconcile_repo_info_for_upload(Some(before), Some(after)).unwrap(); + assert!(out.dirty); + + let before = sample_info("aaa", false); + let after = sample_info("aaa", true); + let out = reconcile_repo_info_for_upload(Some(before), Some(after)).unwrap(); + assert!(out.dirty); + } + + #[test] + fn reconcile_missing_sample_marks_dirty() { + let only = sample_info("aaa", false); + assert!( + reconcile_repo_info_for_upload(Some(only.clone()), None) + .unwrap() + .dirty + ); + assert!( + reconcile_repo_info_for_upload(None, Some(only)) + .unwrap() + .dirty + ); + assert!(reconcile_repo_info_for_upload(None, None).is_none()); + } + + #[test] + fn get_repo_info_dirty_when_submodule_content_modified() { + let parent_dir = tempfile::tempdir().unwrap(); + let parent = parent_dir.path(); + init_committed_repo(parent); + + // Keep the submodule source outside the parent so it is not an + // untracked sibling that would itself mark the tree dirty. + let sub_dir = tempfile::tempdir().unwrap(); + let sub_src = sub_dir.path(); + git(sub_src, &["init"]); + git(sub_src, &["config", "user.email", "test@example.com"]); + git(sub_src, &["config", "user.name", "Test"]); + fs::write(sub_src.join("lib.py"), "v1\n").unwrap(); + git(sub_src, &["add", "lib.py"]); + git(sub_src, &["commit", "-m", "sub init"]); + + // Modern git blocks file:// clones unless explicitly allowed. + git( + parent, + &[ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + sub_src.to_str().unwrap(), + "vendor", + ], + ); + git(parent, &["commit", "-m", "add submodule"]); + + let clean = get_repo_info(parent.to_str().unwrap()) + .unwrap() + .expect("repo info"); + assert!(!clean.dirty, "committed submodule should be clean"); + + fs::write(parent.join("vendor").join("lib.py"), "v2\n").unwrap(); + let dirty = get_repo_info(parent.to_str().unwrap()) + .unwrap() + .expect("repo info"); + assert!( + dirty.dirty, + "modified submodule checkout must mark parent dirty" + ); + } + #[test] fn create_zip_from_target_excludes_default_globs() { let dir = tempfile::tempdir().unwrap(); @@ -508,6 +720,26 @@ mod tests { ); } + #[test] + fn default_exclude_globs_match_abs_tmp_but_not_repo_relative_paths() { + // Linux CI tempdirs are `/tmp/...`. Matching DEFAULT_EXCLUDE_GLOBS on the + // absolute path made `--target` drop every file via `**/tmp/**`. + let mut builder = GlobSetBuilder::new(); + for &pattern in DEFAULT_EXCLUDE_GLOBS { + builder.add(Glob::new(pattern).unwrap()); + } + let set = builder.build().unwrap(); + assert!( + set.is_match(Path::new("/tmp/proj/app.py")), + "absolute /tmp paths hit **/tmp/** (the CI failure mode)" + ); + assert!( + !set.is_match(Path::new("app.py")), + "repo-relative paths must stay scannable" + ); + assert!(!set.is_match(Path::new("src/app.py"))); + } + #[test] fn extract_repo_path_handles_common_remote_forms() { for url in [ diff --git a/tests/cloud_commands_e2e/common/mod.rs b/tests/cloud_commands_e2e/common/mod.rs index 3cd26a8..b475a28 100644 --- a/tests/cloud_commands_e2e/common/mod.rs +++ b/tests/cloud_commands_e2e/common/mod.rs @@ -767,12 +767,17 @@ pub(crate) fn assert_issue_summary(stdout: &str, context: &str) { } pub(crate) fn blast_plan(sha: &str) -> Vec { + blast_upload_plan(sha, false, true) +} + +/// BLAST upload contract. `include_sca` covers `--fail-on malicious` (SCA fetch). +pub(crate) fn blast_upload_plan(sha: &str, dirty: bool, include_sca: bool) -> Vec { let patch_sha = sha.to_string(); + let dirty_value = if dirty { "true" } else { "false" }.to_string(); 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 sca_path = "/api/v1/scan/blast-scan-123/issues/sca".to_string(); - vec![ + let mut plan = vec![ verify_request(), expected_request( "start BLAST upload", @@ -808,6 +813,7 @@ pub(crate) fn blast_plan(sha: &str) -> Vec { "https://github.com/corgea/cloud-e2e.git", )?; assert_multipart_text_field(request, "sha", &patch_sha)?; + assert_multipart_text_field(request, "dirty", &dirty_value)?; assert_body_contains(request, b"name=\"chunk_data\"") }, json_response(json!({ @@ -829,7 +835,10 @@ pub(crate) fn blast_plan(sha: &str) -> Vec { }, json_response(empty_issue_page()), ), - expected_request( + ]; + if include_sca { + let sca_path = "/api/v1/scan/blast-scan-123/issues/sca".to_string(); + plan.push(expected_request( "read malicious SCA issues", move |request| { assert_authenticated_request(request, Method::GET, &sca_path)?; @@ -837,6 +846,7 @@ pub(crate) fn blast_plan(sha: &str) -> Vec { assert_query(request, "page_size", "30") }, json_response(malicious_sca_issue_page()), - ), - ] + )); + } + plan } diff --git a/tests/cloud_commands_e2e/scan_list.rs b/tests/cloud_commands_e2e/scan_list.rs index 9632472..d715a16 100644 --- a/tests/cloud_commands_e2e/scan_list.rs +++ b/tests/cloud_commands_e2e/scan_list.rs @@ -26,6 +26,10 @@ fn scan_fail_on_malicious_sends_sha_and_list_renders_it() { scan_stdout.contains("matched --fail-on malicious"), "{scan_context}" ); + assert!( + !scan_stdout.contains("Working tree has uncommitted changes"), + "clean tree must not print dirty notice\n{scan_context}" + ); let list_response_sha = project.sha.clone(); let list_api = ApiStub::start(vec![ @@ -72,6 +76,97 @@ fn scan_fail_on_malicious_sends_sha_and_list_renders_it() { assert!(list_stdout.contains(&project.sha[..8]), "{list_context}"); } +#[test] +fn scan_dirty_worktree_sends_dirty_true_and_prints_notice() { + let project = git_project(); + std::fs::write(project.path().join("main.py"), "print('dirty')\n") + .expect("modify tracked file"); + let short_sha = &project.sha[..7]; + let scan_api = ApiStub::start(blast_upload_plan(&project.sha, true, false)); + let (mut scan_command, _scan_home) = cloud_command(&scan_api, project.path()); + scan_command.args(["scan", "blast", "--project-name", "cloud-e2e"]); + + let scan_output = run_with_timeout(scan_command, &scan_api); + let scan_transcript = scan_api.assert_finished(); + let scan_context = output_context(&scan_output, &scan_transcript); + assert_eq!(scan_output.status.code(), Some(0), "{scan_context}"); + let scan_stdout = String::from_utf8_lossy(&scan_output.stdout); + assert!( + scan_stdout.contains(&format!( + "Working tree has uncommitted changes - scanning your local files, not commit {short_sha}." + )), + "{scan_context}" + ); +} + +#[test] +fn scan_clean_target_upload_sends_dirty_true_without_worktree_notice() { + let project = git_project(); + std::fs::write(project.path().join("other.py"), "print('other')\n").expect("write other"); + run_git(project.path(), &["add", "other.py"]); + run_git(project.path(), &["commit", "-m", "add other"]); + let sha = String::from_utf8(run_git(project.path(), &["rev-parse", "HEAD"]).stdout) + .expect("UTF-8 SHA") + .trim() + .to_string(); + + let scan_api = ApiStub::start(blast_upload_plan(&sha, true, false)); + let (mut scan_command, _scan_home) = cloud_command(&scan_api, project.path()); + scan_command.args([ + "scan", + "blast", + "--target", + "main.py", + "--project-name", + "cloud-e2e", + ]); + + let scan_output = run_with_timeout(scan_command, &scan_api); + let scan_context_early = output_context(&scan_output, &scan_api.transcript()); + assert_eq!(scan_output.status.code(), Some(0), "{scan_context_early}"); + let scan_transcript = scan_api.assert_finished(); + let scan_context = output_context(&scan_output, &scan_transcript); + let scan_stdout = String::from_utf8_lossy(&scan_output.stdout); + assert!( + !scan_stdout.contains("Working tree has uncommitted changes"), + "clean partial target must not print dirty worktree notice\n{scan_context}" + ); +} + +#[test] +fn scan_clean_exclude_upload_sends_dirty_true_without_worktree_notice() { + let project = git_project(); + std::fs::write(project.path().join("other.py"), "print('other')\n").expect("write other"); + run_git(project.path(), &["add", "other.py"]); + run_git(project.path(), &["commit", "-m", "add other"]); + let sha = String::from_utf8(run_git(project.path(), &["rev-parse", "HEAD"]).stdout) + .expect("UTF-8 SHA") + .trim() + .to_string(); + + let scan_api = ApiStub::start(blast_upload_plan(&sha, true, false)); + let (mut scan_command, _scan_home) = cloud_command(&scan_api, project.path()); + scan_command.args([ + "scan", + "blast", + "--exclude", + "other.py", + "--project-name", + "cloud-e2e", + ]); + + let scan_output = run_with_timeout(scan_command, &scan_api); + let scan_context_early = output_context(&scan_output, &scan_api.transcript()); + assert_eq!(scan_output.status.code(), Some(0), "{scan_context_early}"); + let scan_transcript = scan_api.assert_finished(); + let scan_context = output_context(&scan_output, &scan_transcript); + let scan_stdout = String::from_utf8_lossy(&scan_output.stdout); + assert!( + !scan_stdout.contains("Working tree has uncommitted changes"), + "clean exclude scan must not print dirty worktree notice\n{scan_context}" + ); +} + #[test] fn list_json_returns_filtered_scan_contract() { let project = TempDir::new().expect("create list project");