From fca43f74198d9a774d1593aa7a3946859252be8c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 31 Aug 2026 11:55:00 +0000 Subject: [PATCH 1/2] Add force-include rules to the scan packager and a --include flag A file the CLI leaves out of the archive cannot be scanned whatever the engine later decides about it, so DEFAULT_EXCLUDE_GLOBS and .gitignore silently overruled any platform-side attempt to force-scan misclassified proprietary code. Read the project's include rules from GET /api/v1/scan-settings before packaging, union them with the new repeatable --include flag, and add every matched file to the zip regardless of the default excludes, --exclude, or .gitignore. Force-included files also join the incremental changed-file list. The server carries findings forward for whatever the diff omits, and an include rule exists precisely because the file was never scanned, so there is nothing to carry. Only the --include values travel with the upload; the project's own rules are already stored server-side. Co-authored-by: ibrahim --- skills/corgea/SKILL.md | 5 + src/include_rules.rs | 228 ++++++++++++++++++ src/incremental.rs | 80 +++++++ src/main.rs | 20 ++ src/scanners/blast.rs | 49 ++++ src/utils/api.rs | 74 ++++++ src/utils/generic.rs | 64 +++++- tests/cli_scan_include.rs | 229 +++++++++++++++++++ tests/cloud_commands_e2e/common/mod.rs | 20 +- tests/cloud_commands_e2e/scan_incremental.rs | 10 +- tests/cloud_commands_e2e/scan_skip.rs | 6 +- 11 files changed, 777 insertions(+), 8 deletions(-) create mode 100644 src/include_rules.rs create mode 100644 tests/cli_scan_include.rs diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index d675396..c57502d 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -48,6 +48,9 @@ corgea scan --target "src/**/*.py" # Glob patterns corgea scan --target git:diff=origin/main...HEAD # Git diff range corgea scan --target git:staged,git:modified # Git selectors corgea scan --target - # File list from stdin +corgea scan --exclude "tests/**,*.md" # Exclude glob patterns (comma-separated) +corgea scan --include src/myProj/MyClass.java # Force a file in that Corgea would skip as vendored +corgea scan --include 'vendor/our-fork/**' --include generated/ # Repeatable; path, directory or glob corgea scan --scan-type secrets # Single scan type corgea scan --scan-type blast,policy,secrets,pii # Multiple scan types corgea scan --scan-type policy --policy 1 # Specific policy ID @@ -80,6 +83,8 @@ Scan types: `blast` (base AI), `policy` (PolicyIQ), `malicious`, `secrets`, `pii An included image is enough on its own: when it is combined with `--only-uncommitted` or `--target` and no source files match (a clean working tree, for example), the scan warns and covers just the image rather than failing. An archive named `corgea-image-scanning-*.tar` that is committed to the repository is ignored — only images passed on the command line are scanned. +`--include` forces files into the scan that Corgea would otherwise skip because it classified them as vendored, third-party, generated or test code. It overrides the CLI's own packaging filters, `.gitignore`, `--exclude`, and the engine's classification, and force-included files are analyzed on every run — including incremental ones, where an unchanged file would normally have its previous findings carried forward. Use it when proprietary code lives somewhere Corgea assumes dependencies live, e.g. a fork checked into `vendor/`. Project-level include rules configured in the web app (scan settings → File Include Rules) are fetched before packaging and applied too; `--include` adds to them for one run. + `--only-uncommitted` and `--target` are mutually exclusive. `--fail-on`, `--fail`, and `--block-on` are mutually exclusive. `--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. diff --git a/src/include_rules.rs b/src/include_rules.rs new file mode 100644 index 0000000..ee14355 --- /dev/null +++ b/src/include_rules.rs @@ -0,0 +1,228 @@ +//! Force-include rules: files Corgea must scan even though it would skip them. +//! +//! Corgea leaves out vendored, third-party, test and generated code in two +//! places: this CLI's packaging filters (`DEFAULT_EXCLUDE_GLOBS`, `.gitignore`) +//! and the engine's own classification of what it extracted. When either gets +//! that call wrong for proprietary code, an include rule overrides it. +//! +//! Rules come from two places and are unioned: the project's rules on the +//! platform, fetched here before packaging, and `--include` on this command +//! line. Both matter locally — a file the packager leaves out of the zip cannot +//! be scanned whatever the engine later decides — and only the flag values +//! travel with the upload, since the platform already knows its own rules. + +use crate::config::Config; +use crate::utils::api; +use globset::{Glob, GlobSet, GlobSetBuilder}; +use ignore::WalkBuilder; +use std::path::{Path, PathBuf}; + +/// Ceiling on files one run may force into the archive. A rule like `**/*.js` +/// would otherwise pull an entire `node_modules` tree into the upload. +const MAX_FORCE_INCLUDED_FILES: usize = 5_000; + +/// The force-include rules in effect for one scan. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct IncludeRules { + /// Every pattern in effect: the project's rules plus `--include`. + pub patterns: Vec, + /// Just the `--include` values, the ones the server does not know yet. + pub cli_patterns: Vec, +} + +impl IncludeRules { + pub fn is_empty(&self) -> bool { + self.patterns.is_empty() + } + + /// Paths under `root` that the rules match, relative to `root`. + /// + /// Walks with the standard ignore filters off, since the point is to reach + /// files `.gitignore` and the default excludes hide. `.git` is still + /// skipped: it holds no source and its object store is large. + pub fn matching_files(&self, root: &Path) -> Vec { + let Some(glob_set) = build_glob_set(&self.patterns) else { + return Vec::new(); + }; + let mut matches = Vec::new(); + let walker = WalkBuilder::new(root) + .standard_filters(false) + .filter_entry(|entry| entry.file_name() != ".git") + .build(); + for entry in walker.flatten() { + if !entry.file_type().is_some_and(|kind| kind.is_file()) { + continue; + } + let Ok(relative) = entry.path().strip_prefix(root) else { + continue; + }; + if glob_set.is_match(relative) { + matches.push(relative.to_path_buf()); + } + if matches.len() >= MAX_FORCE_INCLUDED_FILES { + log::warn!( + "Include rules matched more than {} files; only the first {} are forced into this scan.", + MAX_FORCE_INCLUDED_FILES, + MAX_FORCE_INCLUDED_FILES + ); + break; + } + } + matches.sort(); + matches + } +} + +/// Build a matcher, dropping patterns globset cannot compile. +/// +/// One unparseable pattern must not discard the rest: the others are still a +/// clear instruction, and silently scanning less than asked is the failure this +/// whole feature exists to fix. +fn build_glob_set(patterns: &[String]) -> Option { + let mut builder = GlobSetBuilder::new(); + let mut usable = 0; + for pattern in patterns { + match Glob::new(pattern) { + Ok(glob) => { + builder.add(glob); + usable += 1; + } + Err(e) => log::warn!("Ignoring include rule '{pattern}': {e}"), + } + // A bare path or directory prefix should match what is under it, which + // is how the same pattern reads in the platform's ignore rules. + if !pattern.contains('*') { + let descendants = format!("{}/**", pattern.trim_end_matches('/')); + if let Ok(glob) = Glob::new(&descendants) { + builder.add(glob); + usable += 1; + } + } + } + if usable == 0 { + return None; + } + builder.build().ok() +} + +/// Collect the rules for this run: the project's, plus `--include`. +/// +/// A failed lookup is a warning, not a failure. It leaves the project's rules +/// unapplied for this run, which is the behavior every release before this one +/// had; refusing to scan would be worse. +pub fn resolve( + config: &Config, + project_name: &str, + repo_url: Option<&str>, + cli_include: &[String], +) -> IncludeRules { + let cli_patterns = normalize_patterns(cli_include); + let mut patterns = Vec::new(); + + match api::query_scan_settings(&config.get_url(), project_name, repo_url) { + Ok(Some(settings)) => { + for pattern in normalize_patterns(&settings.include_paths) { + push_unique(&mut patterns, pattern); + } + if !patterns.is_empty() { + println!( + "Applying {} project include rule(s) from Corgea: {}.", + patterns.len(), + patterns.join(", ") + ); + } + } + // A backend without the endpoint has no include rules to apply either. + Ok(None) => {} + Err(e) => log::warn!( + "Could not read the project's include rules, so only --include applies to this run: {e}" + ), + } + + for pattern in &cli_patterns { + push_unique(&mut patterns, pattern.clone()); + } + IncludeRules { + patterns, + cli_patterns, + } +} + +fn normalize_patterns(patterns: &[String]) -> Vec { + let mut normalized = Vec::new(); + for pattern in patterns { + let trimmed = pattern.trim(); + if !trimmed.is_empty() { + push_unique(&mut normalized, trimmed.to_string()); + } + } + normalized +} + +fn push_unique(patterns: &mut Vec, pattern: String) { + if !patterns.contains(&pattern) { + patterns.push(pattern); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + fn rules(patterns: &[&str]) -> IncludeRules { + IncludeRules { + patterns: patterns.iter().map(|p| p.to_string()).collect(), + cli_patterns: Vec::new(), + } + } + + fn write(root: &TempDir, relative: &str) { + let path = root.path().join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, "class A {}\n").unwrap(); + } + + #[test] + fn normalize_trims_blanks_and_dedupes() { + let input = ["a/**".to_string(), " a/** ".to_string(), " ".to_string()]; + assert_eq!(normalize_patterns(&input), vec!["a/**".to_string()]); + } + + #[test] + fn no_patterns_matches_nothing() { + let root = TempDir::new().unwrap(); + write(&root, "src/App.java"); + assert!(IncludeRules::default() + .matching_files(root.path()) + .is_empty()); + assert!(build_glob_set(&[]).is_none()); + } + + #[test] + fn unparseable_pattern_does_not_discard_the_others() { + assert!(build_glob_set(&["src/**".to_string(), "[".to_string()]).is_some()); + } + + #[test] + fn matching_files_reaches_gitignored_and_vendored_paths() { + let root = TempDir::new().unwrap(); + write(&root, "vendor/mylib/Payments.java"); + write(&root, "vendor/other/Other.java"); + write(&root, "node_modules/pkg/index.js"); + write(&root, ".git/objects/blob"); + fs::write(root.path().join(".gitignore"), "vendor/\nnode_modules/\n").unwrap(); + + let matched = + rules(&["vendor/mylib", "node_modules/pkg/index.js"]).matching_files(root.path()); + + assert_eq!( + matched, + vec![ + PathBuf::from("node_modules/pkg/index.js"), + PathBuf::from("vendor/mylib/Payments.java"), + ] + ); + } +} diff --git a/src/incremental.rs b/src/incremental.rs index 8a8523b..56fbe51 100644 --- a/src/incremental.rs +++ b/src/incremental.rs @@ -60,6 +60,41 @@ pub struct IncrementalPlan { pub covers_worktree: bool, } +impl IncrementalPlan { + /// Add force-included files to what the server will analyze. + /// + /// The server carries findings forward for every file the diff omits, so a + /// force-included file that has not changed would never be looked at — and + /// the reason to add an include rule is precisely that the file was never + /// scanned before, so there is nothing to carry forward. Returns `None` + /// when the combined list outgrows an incremental scan, which falls back to + /// scanning everything. + pub fn including(mut self, forced: &[String]) -> Option { + let mut listed: BTreeSet = self.changed_files.iter().cloned().collect(); + let additions: Vec = forced + .iter() + .filter(|path| listed.insert((*path).clone())) + .cloned() + .collect(); + if additions.is_empty() { + return Some(self); + } + if self.changed_files.len() + additions.len() > MAX_CHANGED_FILES { + explain_full_scan( + "the include rules cover more files than an incremental scan is worth", + ); + return None; + } + match additions.len() { + 1 => println!("Incremental scan: also analyzing 1 force-included file."), + count => println!("Incremental scan: also analyzing {count} force-included files."), + } + self.changed_files.extend(additions); + self.changed_files.sort(); + Some(self) + } +} + /// What an incremental scan of this commit would cover, or `None` to scan /// everything. pub fn resolve_incremental_plan( @@ -442,6 +477,51 @@ mod tests { assert_eq!(short_sha("ααααααααα"), "ααααααα"); } + fn plan(changed: &[&str]) -> IncrementalPlan { + IncrementalPlan { + base_sha: "abc123".to_string(), + changed_files: changed.iter().map(|f| f.to_string()).collect(), + covers_worktree: false, + } + } + + #[test] + fn including_adds_force_included_files_the_diff_left_out() { + let forced = vec![ + "src/app.py".to_string(), + "vendor/mylib/Payments.java".to_string(), + ]; + + let widened = plan(&["src/app.py"]) + .including(&forced) + .expect("still worth it"); + + assert_eq!( + widened.changed_files, + vec![ + "src/app.py".to_string(), + "vendor/mylib/Payments.java".to_string() + ] + ); + } + + #[test] + fn including_nothing_new_leaves_the_plan_alone() { + let original = plan(&["src/app.py"]); + assert_eq!( + original.clone().including(&["src/app.py".to_string()]), + Some(original) + ); + } + + #[test] + fn including_too_many_files_falls_back_to_a_full_scan() { + let forced: Vec = (0..=MAX_CHANGED_FILES) + .map(|i| format!("v/{i}.js")) + .collect(); + assert_eq!(plan(&["src/app.py"]).including(&forced), None); + } + #[test] fn a_completed_clean_blast_scan_is_a_baseline() { assert!(is_usable_baseline(&scan("main", "abc"))); diff --git a/src/main.rs b/src/main.rs index 5e9cd0f..3a356d4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,7 @@ mod authorize; mod cicd; mod config; mod images; +mod include_rules; mod incremental; mod inspect; mod list; @@ -39,6 +40,11 @@ struct Cli { args: Vec, } +// `Scan` carries by far the largest flag set of any subcommand, and exactly one +// `Commands` value exists per process — parsed once at startup and destructured +// immediately — so the wasted stack in the other variants costs nothing that +// boxing would recover. +#[allow(clippy::large_enum_variant)] #[derive(Subcommand, Debug)] enum Commands { /// Authenticate to Corgea @@ -156,6 +162,13 @@ enum Commands { )] exclude: Option, + #[arg( + long = "include", + value_name = "PATH", + help = "Force files into the scan that Corgea would otherwise skip as vendored, third-party, generated or test code (repeatable), e.g. --include src/myProj/MyClass.java --include 'vendor/our-fork/**'. Accepts a path, a directory, or a glob pattern, and overrides this command's packaging filters, .gitignore, --exclude and the engine's own classification. Force-included files are analyzed on every run, including incremental ones. Your project's include rules in Corgea apply too; this flag adds to them for one run." + )] + include: Vec, + #[arg( long, help = "The name of the Corgea project. Defaults to git repository name if found, otherwise to the current directory name." @@ -683,6 +696,7 @@ fn main() { out_file, target, exclude, + include, project_name, sbom, include_image, @@ -812,6 +826,11 @@ fn main() { std::process::exit(1); } + if !include.is_empty() && *scanner != Scanner::Blast { + ::log::error!("--include is only supported with the blast scanner."); + std::process::exit(1); + } + if sbom.is_some() && *scanner != Scanner::Blast { ::log::error!("sbom is only supported with blast scanner."); std::process::exit(1); @@ -867,6 +886,7 @@ fn main() { out_file.clone(), target.clone(), exclude.clone(), + include.clone(), project_name.clone(), sbom.clone(), include_images, diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 3c368fa..b5e9518 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -62,6 +62,7 @@ pub fn run( out_file: Option, target: Option, exclude: Option, + include: Vec, project_name: Option, sbom: Option, include_images: Vec, @@ -118,6 +119,7 @@ pub fn run( policy, target, exclude, + include, include_images, ), }; @@ -277,6 +279,19 @@ pub fn run( } } +/// Repo-relative paths as the `/`-separated strings the server's file lists use. +fn repo_relative_strings(paths: &[PathBuf]) -> Vec { + paths + .iter() + .map(|path| { + path.components() + .map(|component| component.as_os_str().to_string_lossy()) + .collect::>() + .join("/") + }) + .collect() +} + /// 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. @@ -292,6 +307,7 @@ fn start_new_scan( policy: Option, target: Option, exclude: Option, + include: Vec, include_images: Vec, ) -> (String, Option) { println!("\nScanning with BLAST 🚀🚀🚀"); @@ -350,6 +366,33 @@ fn start_new_scan( // 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(); + // Resolved before packaging: the rules decide what goes into the archive, + // and a file left out of it cannot be scanned however the engine classifies + // what it did receive. + let include_rules = crate::include_rules::resolve( + config, + project_name, + repo_before + .as_ref() + .and_then(|info| info.repo_url.as_deref()), + &include, + ); + let force_included = include_rules.matching_files(Path::new(".")); + if !include_rules.is_empty() && force_included.is_empty() { + log::warn!( + "\n{}", + utils::terminal::set_text_color( + "⚠️ No files matched your include rules, so nothing was forced into this scan.", + utils::terminal::TerminalColor::Yellow + ) + ); + } else if !force_included.is_empty() { + println!( + "Force-including {} file(s) Corgea would otherwise skip.", + force_included.len() + ); + } + if target_str.is_none() && exclude.is_some() { println!("Excluding files matching: {}", exclude.as_deref().unwrap()); } @@ -449,6 +492,7 @@ fn start_new_scan( &zip_path, None, exclude.as_deref(), + &force_included, &extra_zip_files, ) { Ok(added_files) => { @@ -541,6 +585,10 @@ fn start_new_scan( repo_info.as_ref().is_some_and(|info| info.dirty), *ignore_dirty_worktree, ) + // Force-included files are usually unchanged, and the server carries + // findings forward for whatever the diff omits — so without this an + // include rule would never get the file looked at on an incremental run. + .and_then(|plan| plan.including(&repo_relative_strings(&force_included))) }; println!("\n\nSubmitting scan to Corgea:"); let upload_result = match utils::api::upload_zip( @@ -553,6 +601,7 @@ fn start_new_scan( policy, metadata, incremental: incremental_plan, + include_paths: include_rules.cli_patterns, }, ) { Ok(result) => result, diff --git a/src/utils/api.rs b/src/utils/api.rs index f02db74..9b1aa1b 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -243,6 +243,9 @@ pub struct UploadOptions { /// Set when this run resolved a diff for the server to analyze instead of /// the whole project. pub incremental: Option, + /// `--include` patterns for this run. The project's own include rules are + /// already stored server-side, so only the flag values are sent. + pub include_paths: Vec, } pub fn upload_zip( @@ -257,7 +260,18 @@ pub fn upload_zip( policy, metadata, incremental, + include_paths, } = options; + let include_paths_field = match include_paths.is_empty() { + true => None, + false => match serde_json::to_string(&include_paths) { + Ok(json) => Some(json), + Err(e) => { + debug(&format!("Could not serialize the --include patterns: {e}")); + None + } + }, + }; 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(); @@ -386,6 +400,9 @@ pub fn upload_zip( if let Some(meta) = &metadata { form = form.part("metadata", multipart::Part::text(meta.clone())); } + if let Some(patterns) = &include_paths_field { + form = form.part("include_paths", multipart::Part::text(patterns.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 @@ -949,6 +966,63 @@ fn request_scan_list( } } +/// Project-level path rules a client needs before it packages a scan. +#[derive(Deserialize, Debug, Default, PartialEq, Eq)] +pub struct ScanSettings { + /// Patterns that force files into the scan even when Corgea would classify + /// them as vendored, third-party, generated or test code. + #[serde(default)] + pub include_paths: Vec, + #[serde(default)] + pub ignore_paths: Vec, +} + +#[derive(Deserialize, Debug, Default)] +struct ScanSettingsResponse { + #[serde(default)] + settings: ScanSettings, +} + +/// GET /api/v1/scan-settings — the project's ignore and include rules. +/// +/// `Ok(None)` only for a 404, which is a backend predating the endpoint: it has +/// no rules to apply, so the caller proceeds with just its own flags. Anything +/// else is an `Err`, because reading zero rules from a broken lookup and +/// reading zero rules from a project that has none are not the same thing. +pub fn query_scan_settings( + url: &str, + project_name: &str, + repo_url: Option<&str>, +) -> Result, Box> { + let request_url = format!("{}{}/scan-settings", url, API_BASE); + let client = http_client(); + let mut query = vec![("project_name", project_name.to_string())]; + if let Some(repo_url) = repo_url { + query.push(("repo_url", repo_url.to_string())); + } + debug(&format!( + "Reading project scan settings from {} ({:?})", + request_url, query + )); + let response = client.get(&request_url).query(&query).send()?; + check_for_warnings(response.headers(), response.status()); + let status = response.status(); + if status == StatusCode::NOT_FOUND { + return Ok(None); + } + if !status.is_success() { + return Err(format!("/scan-settings request failed: HTTP {}", status).into()); + } + let text = response.text()?; + match serde_json::from_str::(&text) { + Ok(parsed) => Ok(Some(parsed.settings)), + Err(e) => { + debug(&format!("/scan-settings response body: {}", text)); + Err(format!("Failed to parse the /scan-settings response: {}", e).into()) + } + } +} + #[derive(Deserialize, Debug)] pub struct ProjectSummary { pub name: String, diff --git a/src/utils/generic.rs b/src/utils/generic.rs index 274ccd6..4fb8150 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -2,6 +2,7 @@ use crate::utils::terminal::{set_text_color, TerminalColor}; use git2::{Repository, StatusOptions}; use globset::{Glob, GlobSetBuilder}; use ignore::WalkBuilder; +use std::collections::HashSet; use std::env; use std::fs::{self, File}; use std::io; @@ -67,6 +68,10 @@ const DEFAULT_EXCLUDE_GLOBS: &[&str] = &[ /// - If `target` is `Some(target_str)`, resolves the target using the targets module and creates zip from those files. /// The target string can be a comma-separated list of files, directories, globs, or git selectors. /// - `user_exclude` is an optional comma-separated list of glob patterns from `--exclude`. +/// - `force_include` are repo-relative paths the project's include rules or +/// `--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. /// - `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. @@ -75,6 +80,7 @@ pub fn create_zip_from_target>( output_zip: P, exclude_globs: Option<&[&str]>, user_exclude: Option<&str>, + force_include: &[PathBuf], extra_files: &[(PathBuf, String)], ) -> Result, Box> { let exclude_globs = exclude_globs.unwrap_or(DEFAULT_EXCLUDE_GLOBS); @@ -88,7 +94,7 @@ pub fn create_zip_from_target>( let user_exclude_glob_set = crate::targets::build_user_exclude_glob_set(user_exclude) .map_err(|e| format!("Failed to build exclude patterns: {}", e))?; - let files_to_zip: Vec<(PathBuf, PathBuf)> = if let Some(target_str) = target { + let mut files_to_zip: Vec<(PathBuf, PathBuf)> = if let Some(target_str) = target { let current_dir = env::current_dir()?; let result = crate::targets::resolve_targets_with_exclude(target_str, user_exclude) .map_err(|e| format!("Failed to resolve targets: {}", e))?; @@ -132,6 +138,20 @@ pub fn create_zip_from_target>( files }; + let forced: HashSet<&Path> = force_include.iter().map(PathBuf::as_path).collect(); + let 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() { + files_to_zip.push((relative.clone(), relative.clone())); + } + } + let zip_file = File::create(output_zip.as_ref())?; let mut zip = ZipWriter::new(zip_file); @@ -144,7 +164,8 @@ pub fn create_zip_from_target>( for (path, relative_path) in files_to_zip { // Match repo-relative paths so abs `/tmp/...` targets don't hit `**/tmp/**`. - let is_excluded = glob_set.is_match(&relative_path); + let is_excluded = + glob_set.is_match(&relative_path) && !forced.contains(relative_path.as_path()); if (path.is_file() || path.is_dir()) && !is_excluded { if path.is_file() { @@ -918,8 +939,9 @@ mod tests { // which would exclude *everything*. The filter + warn path under test // is identical either way. let excludes: &[&str] = &["**/node_modules/**"]; - let added = create_zip_from_target(Some(&target), &output_zip, Some(excludes), None, &[]) - .expect("zip creation should succeed"); + let added = + create_zip_from_target(Some(&target), &output_zip, Some(excludes), None, &[], &[]) + .expect("zip creation should succeed"); assert!( added.iter().any(|p| p.ends_with("src/main.py")), @@ -933,6 +955,38 @@ mod tests { ); } + /// A force-include rule is the customer overruling Corgea's own judgement + /// about a file, so it has to beat the default excludes. + #[test] + fn create_zip_from_target_keeps_force_included_files() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + + let node_modules = root.join("node_modules"); + fs::create_dir_all(&node_modules).unwrap(); + let forced = node_modules.join("internal-sdk.js"); + fs::write(&forced, "console.log(1)").unwrap(); + let excluded = node_modules.join("third-party.js"); + fs::write(&excluded, "console.log(2)").unwrap(); + + let output_zip = root.join("out.zip"); + let target = format!("{},{}", forced.display(), excluded.display()); + // Explicit file targets outside the cwd keep their absolute paths as + // zip entry names, so that is the shape the exemption check compares. + let added = create_zip_from_target( + Some(&target), + &output_zip, + Some(&["**/node_modules/**"]), + None, + std::slice::from_ref(&forced), + &[], + ) + .expect("zip creation should succeed"); + + assert!(added.contains(&forced), "force-included: {:?}", added); + assert!(!added.contains(&excluded), "still excluded: {:?}", added); + } + /// The staging directory holds the project zip and exported images, so other /// local users must not be able to read it. #[cfg(unix)] @@ -967,6 +1021,7 @@ mod tests { &output_zip, Some(&[]), None, + &[], &extra_files, ) .expect("zip creation should succeed"); @@ -1009,6 +1064,7 @@ mod tests { &output_zip, Some(&[]), None, + &[], &extra_files, ) .expect("a >4 GiB entry needs ZIP64, not an error"); diff --git a/tests/cli_scan_include.rs b/tests/cli_scan_include.rs new file mode 100644 index 0000000..97d1a6a --- /dev/null +++ b/tests/cli_scan_include.rs @@ -0,0 +1,229 @@ +//! End-to-end coverage for force-include rules: drives the real binary through +//! the blast scan flow against a stubbed HTTP server and asserts that files the +//! packager would normally leave out — `node_modules`, `.gitignore`d paths, +//! `--exclude`d paths — are bundled when `--include` or the project's own +//! include rules name them, and that the rules travel with the upload. + +mod common; + +use common::corgea_isolated; +use std::fs; +use std::io::Write; +use std::net::TcpListener; +use std::sync::{Arc, Mutex}; +use tempfile::TempDir; + +/// Raw bodies of the chunk uploads the CLI sent. +type Uploads = Arc>>>; + +/// The blast scan route table, answering `/scan-settings` with `include_paths` +/// so a test can exercise the platform-configured rules as well as the flag. +fn spawn_scan_stub( + scan_id: &'static str, + project_include_paths: &'static 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); + + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + let request = corgea::vuln_api_stub::read_http_request(&mut stream); + let request_line = String::from_utf8_lossy(&request[..request.len().min(1024)]) + .lines() + .next() + .unwrap_or("") + .to_string(); + let target = request_line.split_whitespace().nth(1).unwrap_or(""); + let path = target.split('?').next().unwrap_or(target); + + let (status, body) = if path == "/api/v1/verify" { + ("200 OK", r#"{"status":"ok"}"#.to_string()) + } else if path == "/api/v1/scan-settings" { + ( + "200 OK", + format!( + r#"{{"status":"ok","project":null,"settings":{{"include_paths":{},"ignore_paths":[]}}}}"#, + project_include_paths + ), + ) + } else if path == "/api/v1/start-scan" { + ("200 OK", r#"{"transfer_id":"transfer-1"}"#.to_string()) + } else if path == "/api/v1/start-scan/transfer-1/" { + recorder.lock().unwrap().push(request.clone()); + ( + "200 OK", + format!(r#"{{"scan_id":"{}","project_id":"1"}}"#, scan_id), + ) + } else if path == format!("/api/v1/scan/{}", scan_id) { + ( + "200 OK", + format!( + r#"{{"id":"{}","project":"proj","repo":null,"branch":null,"status":"complete","engine":"blast","created_at":"2026-01-01T00:00:00Z"}}"#, + scan_id + ), + ) + } else if path == format!("/api/v1/scan/{}/issues", scan_id) { + ( + "200 OK", + r#"{"status":"ok","issues":[],"page":1,"total_pages":1,"total_issues":0}"# + .to_string(), + ) + } else { + ("404 Not Found", r#"{"message":"not found"}"#.to_string()) + }; + + let response = corgea::vuln_api_stub::http_response(status, "", &body); + let _ = stream.write_all(response.as_bytes()); + } + }); + + (base_url, uploads) +} + +/// Everything the CLI uploaded, as lossy text. Zip entry names are stored +/// verbatim in each local file header, so searching for a path here proves it +/// was bundled. +fn uploaded_text(uploads: &Uploads) -> String { + let uploads = uploads.lock().expect("upload log"); + assert!(!uploads.is_empty(), "no chunk upload was recorded"); + uploads + .iter() + .map(|chunk| String::from_utf8_lossy(chunk).into_owned()) + .collect() +} + +/// A project whose proprietary code sits where Corgea assumes dependencies live. +fn stub_project() -> TempDir { + let project = TempDir::new().expect("project dir"); + let root = project.path(); + fs::write(root.join("main.py"), "print(1)\n").expect("write source file"); + fs::create_dir_all(root.join("node_modules/internal-sdk")).expect("create vendor dir"); + fs::write( + root.join("node_modules/internal-sdk/index.js"), + "module.exports = 1;\n", + ) + .expect("write force-include candidate"); + fs::create_dir_all(root.join("node_modules/third-party")).expect("create dependency dir"); + fs::write( + root.join("node_modules/third-party/index.js"), + "module.exports = 2;\n", + ) + .expect("write third-party file"); + project +} + +fn scan(base_url: &str, project: &TempDir, args: &[&str]) -> std::process::Output { + let (mut cmd, _home) = corgea_isolated(); + cmd.current_dir(project.path()) + .env("CORGEA_URL", base_url) + .env("CORGEA_TOKEN", "test-token") + .arg("scan") + .args(args); + let output = cmd.output().expect("run corgea scan"); + assert!( + output.status.success(), + "stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + output +} + +#[test] +fn include_flag_bundles_a_file_the_default_excludes_would_drop() { + let (base_url, uploads) = spawn_scan_stub("scan-include-flag", "[]"); + let project = stub_project(); + + let output = scan( + &base_url, + &project, + &["--include", "node_modules/internal-sdk/index.js"], + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Force-including 1 file(s)"), + "should report what it forced in, got:\n{stdout}" + ); + + let uploaded = uploaded_text(&uploads); + assert!( + uploaded.contains("node_modules/internal-sdk/index.js"), + "the force-included file should be bundled" + ); + assert!( + !uploaded.contains("node_modules/third-party/index.js"), + "other node_modules files stay excluded" + ); + assert!(uploaded.contains("main.py"), "source files still upload"); + // The server does not know this run's flag values, so they travel with it. + assert!( + uploaded.contains(r#"["node_modules/internal-sdk/index.js"]"#), + "the --include patterns should be sent with the upload" + ); +} + +#[test] +fn project_include_rules_from_the_platform_are_applied() { + let (base_url, uploads) = + spawn_scan_stub("scan-include-project", r#"["node_modules/internal-sdk"]"#); + let project = stub_project(); + + let output = scan(&base_url, &project, &[]); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Applying 1 project include rule(s) from Corgea"), + "should say the rules came from the platform, got:\n{stdout}" + ); + + let uploaded = uploaded_text(&uploads); + assert!(uploaded.contains("node_modules/internal-sdk/index.js")); + assert!(!uploaded.contains("node_modules/third-party/index.js")); + // Already stored server-side, so nothing to send back. + assert!(!uploaded.contains(r#"name="include_paths""#)); +} + +#[test] +fn an_include_rule_overrides_exclude_patterns() { + let (base_url, uploads) = spawn_scan_stub("scan-include-exclude", "[]"); + let project = stub_project(); + fs::write(project.path().join(".gitignore"), "generated/\n").expect("write gitignore"); + fs::create_dir_all(project.path().join("generated")).expect("create generated dir"); + fs::write( + project.path().join("generated/Payments.java"), + "class Payments {}\n", + ) + .expect("write generated file"); + + scan( + &base_url, + &project, + &[ + "--exclude", + "generated/**", + "--include", + "generated/Payments.java", + ], + ); + + assert!(uploaded_text(&uploads).contains("generated/Payments.java")); +} + +#[test] +fn an_include_rule_that_matches_nothing_warns_and_still_scans() { + let (base_url, uploads) = spawn_scan_stub("scan-include-nomatch", "[]"); + let project = stub_project(); + + let output = scan(&base_url, &project, &["--include", "no/such/path.java"]); + + assert!( + String::from_utf8_lossy(&output.stderr).contains("No files matched your include rules"), + "stderr:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(uploaded_text(&uploads).contains("main.py")); +} diff --git a/tests/cloud_commands_e2e/common/mod.rs b/tests/cloud_commands_e2e/common/mod.rs index b9548d3..8325441 100644 --- a/tests/cloud_commands_e2e/common/mod.rs +++ b/tests/cloud_commands_e2e/common/mod.rs @@ -596,6 +596,24 @@ pub(crate) fn verify_request() -> ExpectedRequest { ) } +/// Every new BLAST scan reads the project's include rules before packaging, so +/// files Corgea would classify away can still be forced into the archive. +pub(crate) fn scan_settings_request(project: &str) -> ExpectedRequest { + let project = project.to_string(); + expected_request( + "read project include rules", + move |request| { + assert_authenticated_request(request, Method::GET, "/api/v1/scan-settings")?; + assert_query(request, "project_name", &project) + }, + json_response(json!({ + "status": "ok", + "project": null, + "settings": {"include_paths": [], "ignore_paths": []} + })), + ) +} + pub(crate) fn scan_response(scan_id: &str, project: &str, status: &str) -> Value { json!({ "id": scan_id, @@ -814,7 +832,7 @@ 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(), scan_settings_request("cloud-e2e")]; // 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 diff --git a/tests/cloud_commands_e2e/scan_incremental.rs b/tests/cloud_commands_e2e/scan_incremental.rs index c9385ba..21d9972 100644 --- a/tests/cloud_commands_e2e/scan_incremental.rs +++ b/tests/cloud_commands_e2e/scan_incremental.rs @@ -130,6 +130,7 @@ fn the_upload_carries_the_baseline_commit_and_the_files_that_changed_since_it() let expected_base = base_sha.clone(); let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), baseline_lookup("main", vec![baseline_scan(&base_sha)]), start_upload(), expected_request( @@ -180,7 +181,7 @@ fn a_project_with_no_baseline_scan_uploads_without_a_diff() { let head_sha = second_commit(&project); let patch_sha = head_sha.clone(); - let mut plan = vec![verify_request()]; + let mut plan = vec![verify_request(), scan_settings_request(PROJECT)]; plan.extend(baseline_lookups_finding_nothing()); plan.extend([ start_upload(), @@ -234,6 +235,7 @@ fn a_baseline_on_a_later_page_is_still_found() { let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), baseline_lookup_page("main", 1, 2, vec![unusable]), baseline_lookup_page("main", 2, 2, vec![baseline_scan(&base_sha)]), start_upload(), @@ -273,6 +275,7 @@ fn a_failed_lookup_is_not_reported_as_a_missing_baseline() { let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), expected_request( "fail the baseline lookup", |request| assert_baseline_lookup_request(request, PROJECT, "main"), @@ -323,6 +326,7 @@ fn disable_incremental_does_not_even_look_for_a_baseline() { let patch_sha = head_sha.clone(); let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), start_upload(), expected_request( "upload BLAST archive with no diff", @@ -372,6 +376,7 @@ fn a_narrowed_archive_skips_incremental_without_claiming_a_full_scan() { let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), start_upload(), expected_request( "upload narrowed BLAST archive", @@ -420,6 +425,7 @@ fn a_directory_that_is_not_a_git_repository_scans_everything() { let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), start_upload(), expected_request( "upload BLAST archive with no repo metadata", @@ -466,6 +472,7 @@ fn ignore_dirty_worktree_diffs_the_working_tree_instead_of_refusing() { let expected_base = base_sha.clone(); let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), baseline_lookup("main", vec![baseline_scan(&base_sha)]), start_upload(), expected_request( @@ -524,6 +531,7 @@ fn a_dirty_worktree_skips_the_baseline_lookup_and_scans_everything() { let patch_sha = head_sha.clone(); let mut plan = vec![ verify_request(), + scan_settings_request(PROJECT), start_upload(), expected_request( "upload BLAST archive with no diff", diff --git a/tests/cloud_commands_e2e/scan_skip.rs b/tests/cloud_commands_e2e/scan_skip.rs index 6b073b1..6a292cd 100644 --- a/tests/cloud_commands_e2e/scan_skip.rs +++ b/tests/cloud_commands_e2e/scan_skip.rs @@ -446,9 +446,11 @@ 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"); + // blast_upload_plan already holds verify then the include-rule lookup; the + // baselines follow it and the reuse lookup precedes it. 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(2, baseline_lookup_for_branch("master", vec![])); + plan.insert(2, 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()); From 183121f785fc154ead5a1201cbaa60b8a3198d1e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 31 Aug 2026 12:03:49 +0000 Subject: [PATCH 2/2] Refuse scan reuse alongside an include rule A reused scan predates the include rule, so skipping would leave the very file the run mandated unscanned. Under-reporting, unlike --exclude's over-reporting, so clap refuses the combination rather than warning. Co-authored-by: ibrahim --- skills/corgea/SKILL.md | 2 +- src/main.rs | 4 ++-- tests/cloud_commands_e2e/scan_skip.rs | 27 +++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index c57502d..bfbedd0 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -91,7 +91,7 @@ An included image is enough on its own: when it is combined with `--only-uncommi `--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. -Reuse requires a candidate that answers the same question: a completed `corgea-blast` scan of that commit, on a branch rather than a pull request, from an explicitly clean worktree, reporting no scanner problems. Anything else runs a real scan (nothing in the window, a failed or still-running scan, a worktree `git status` reports changes in, or a failed lookup). `--ignore-dirty-worktree` (requires `--skip-if-commit-scanned-recently`) overrides the dirty-worktree half of that test: reuse proceeds even if this worktree is dirty or the prior scan recorded `worktree_dirty=true`. A prior scan that never reported the flag is still not reused. A new scan still reports the real dirty status. An unresolvable commit is a hard error (exit 1). Because the API exposes neither a scan's configured scan types and target policies nor whether it bundled a container image, a run that changes what gets scanned cannot be matched against a candidate, so the flag cannot be combined with `--scan-type`, `--policy`, `--include-image`, `--only-uncommitted`, or `--target`. `--exclude` is allowed but warns on a skip: what gets reused is a scan of the whole commit, so the results and the gate can cover files the run would have skipped (over-reporting, never under-reporting). +Reuse requires a candidate that answers the same question: a completed `corgea-blast` scan of that commit, on a branch rather than a pull request, from an explicitly clean worktree, reporting no scanner problems. Anything else runs a real scan (nothing in the window, a failed or still-running scan, a worktree `git status` reports changes in, or a failed lookup). `--ignore-dirty-worktree` (requires `--skip-if-commit-scanned-recently`) overrides the dirty-worktree half of that test: reuse proceeds even if this worktree is dirty or the prior scan recorded `worktree_dirty=true`. A prior scan that never reported the flag is still not reused. A new scan still reports the real dirty status. An unresolvable commit is a hard error (exit 1). Because the API exposes neither a scan's configured scan types and target policies nor whether it bundled a container image, a run that changes what gets scanned cannot be matched against a candidate, so the flag cannot be combined with `--scan-type`, `--policy`, `--include-image`, `--include`, `--only-uncommitted`, or `--target`. `--exclude` is allowed but warns on a skip: what gets reused is a scan of the whole commit, so the results and the gate can cover files the run would have skipped (over-reporting, never under-reporting). ### Upload — `corgea upload [report]` diff --git a/src/main.rs b/src/main.rs index 3a356d4..0790112 100644 --- a/src/main.rs +++ b/src/main.rs @@ -193,8 +193,8 @@ enum Commands { #[arg( long = "skip-if-commit-scanned-recently", - 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." + conflicts_with_all = ["only_uncommitted", "target", "scan_type", "policy", "include_image", "include", "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, --include 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, diff --git a/tests/cloud_commands_e2e/scan_skip.rs b/tests/cloud_commands_e2e/scan_skip.rs index 6a292cd..7975052 100644 --- a/tests/cloud_commands_e2e/scan_skip.rs +++ b/tests/cloud_commands_e2e/scan_skip.rs @@ -676,6 +676,33 @@ fn the_window_cannot_be_set_without_the_skip_flag() { ); } +/// A force-include rule widens what gets scanned, and the reused candidate was +/// scanned without it — so reuse would silently skip the very file the run +/// mandated. That is under-reporting, which the flag refuses rather than warns. +#[test] +fn reuse_is_refused_alongside_an_include_rule() { + let api = ApiStub::start(Vec::new()); + let project = git_project(); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args([ + "scan", + "blast", + "--skip-if-commit-scanned-recently", + "--include", + "vendor/our-fork/**", + ]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + + assert_eq!(output.status.code(), Some(2), "{context}"); + assert!( + String::from_utf8_lossy(&output.stderr).contains("--include"), + "{context}" + ); +} + /// `--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.