diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index d675396..bfbedd0 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,13 +83,15 @@ 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. `--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/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..0790112 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." @@ -180,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, @@ -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..7975052 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()); @@ -674,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.