diff --git a/README.md b/README.md index 44ee09d..153f1e4 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,16 @@ Once the binary is installed, login with your token from the Corgea app. corgea login ``` +## Scanning + +`corgea scan` uploads your project and waits for results. `corgea wait [scan_id]` +attaches to a running scan; `corgea upload --wait` does the same for a +third-party report. + +All three exit 1 if the scan fails, printing the reason and the scanners that hit +problems. A scan that completes with a scanner missing exits 0 with a warning. +Waiting gives up after 10 hours; override with `CORGEA_SCAN_TIMEOUT_SECONDS`. + ## Dependency Inventory (offline) `corgea deps` builds a dependency inventory from npm, Python, and Java manifests diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index bbf8d9e..75175d8 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -74,9 +74,14 @@ By default `upload` prints the scan page URL so you can track the results. Pass ```bash corgea wait # Wait for latest scan -corgea wait --scan-id SCAN_ID # Wait for specific scan +corgea wait SCAN_ID # Wait for a specific scan ``` +Waiting (`corgea scan`, `corgea wait`, `corgea upload --wait`) exits 1 if the +scan fails, printing why. A scan missing one scanner's results exits 0 with a +warning. Polling gives up after 10 hours; override with +`CORGEA_SCAN_TIMEOUT_SECONDS`. + ### List — `corgea list` (alias: `corgea ls`) ```bash diff --git a/src/scan.rs b/src/scan.rs index ce078c5..d77dd24 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -53,24 +53,23 @@ pub struct ScanUploadResult { } /// Build the URL to the Corgea scan page so users can track results. -pub fn build_scan_url(config: &Config, result: &ScanUploadResult) -> String { - build_scan_url_from_base(&config.get_url(), result) -} - -/// Build the scan page URL from an explicit base URL. Split out from -/// `build_scan_url` so the project-segment encoding contract can be unit -/// tested without constructing a `Config`. -fn build_scan_url_from_base(base_url: &str, result: &ScanUploadResult) -> String { - match &result.project_id { - Some(pid) => format!("{}/project/{}/?scan_id={}", base_url, pid, result.scan_id), - // The project name is a free-form path segment. In CI it is - // `{owner/repo}-{pr}`, so it must be percent-encoded or the `/` - // would route the tracking link to the wrong project. +/// +/// The project-name fallback is a free-form path segment — uploads from CI name +/// the project `{owner/repo}-{pr}` — so it must be percent-encoded or the `/` +/// would route the link to the wrong project. +pub fn build_scan_url( + base_url: &str, + project_id: Option<&str>, + project_name: &str, + scan_id: &str, +) -> String { + match project_id { + Some(pid) => format!("{}/project/{}/?scan_id={}", base_url, pid, scan_id), None => format!( "{}/project/{}?scan_id={}", base_url, - urlencoding::encode(&result.project_name), - result.scan_id + urlencoding::encode(project_name), + scan_id ), } } @@ -78,7 +77,12 @@ fn build_scan_url_from_base(base_url: &str, result: &ScanUploadResult) -> String /// Print the scan page URL so the user can track the scan results without /// waiting for it to complete. pub fn print_scan_tracking_url(config: &Config, result: &ScanUploadResult) { - let scan_url = build_scan_url(config, result); + let scan_url = build_scan_url( + &config.get_url(), + result.project_id.as_deref(), + &result.project_name, + &result.scan_id, + ); print!( "\n\nScan has started with ID: {}.\n\nYou can view it populate at the link:\n{}\n\n", result.scan_id, @@ -594,18 +598,14 @@ pub fn upload_scan( mod tests { use super::*; - fn result(project_id: Option<&str>, project_name: &str) -> ScanUploadResult { - ScanUploadResult { - scan_id: "scan-123".to_string(), - project_id: project_id.map(|p| p.to_string()), - project_name: project_name.to_string(), - } - } - #[test] fn scan_url_prefers_project_id_when_present() { - let url = - build_scan_url_from_base("https://www.corgea.app", &result(Some("42"), "some/name")); + let url = build_scan_url( + "https://www.corgea.app", + Some("42"), + "some/name", + "scan-123", + ); assert_eq!(url, "https://www.corgea.app/project/42/?scan_id=scan-123"); } @@ -613,11 +613,22 @@ mod tests { fn scan_url_percent_encodes_project_name_fallback() { // CI project names are `{owner/repo}-{pr}`; the `/` must be encoded so // the link resolves to the intended project rather than a nested path. - let url = - build_scan_url_from_base("https://www.corgea.app", &result(None, "corgea/cli-15")); + let url = build_scan_url("https://www.corgea.app", None, "corgea/cli-15", "scan-123"); assert_eq!( url, "https://www.corgea.app/project/corgea%2Fcli-15?scan_id=scan-123" ); } + + #[test] + fn scan_url_leaves_locally_derived_project_names_untouched() { + // `determine_project_name` already reduces names to `[alphanumeric-_.]`, + // which encoding leaves alone, so sharing this builder with the upload + // paths cannot change the links they were printing before. + let url = build_scan_url("https://www.corgea.app", None, "corgea_cli-1.0", "scan-123"); + assert_eq!( + url, + "https://www.corgea.app/project/corgea_cli-1.0?scan_id=scan-123" + ); + } } diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 4923f05..a967c21 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -1,15 +1,19 @@ use crate::config::Config; +use crate::scan::build_scan_url; use crate::targets; use crate::utils; use crate::utils::api::SCAIssue; use std::collections::HashMap; use std::env; -use std::error::Error; use std::fs; use std::sync::{Arc, Mutex}; use std::thread; +use std::time::{Duration, Instant}; use uuid::Uuid; +/// Overrides how long `wait_for_scan` polls before giving up. +const SCAN_TIMEOUT_ENV: &str = "CORGEA_SCAN_TIMEOUT_SECONDS"; + #[allow(clippy::too_many_arguments)] pub fn run( config: &Config, @@ -237,15 +241,12 @@ pub fn run( }; let scan_id = upload_result.scan_id; - let scan_url = match &upload_result.project_id { - Some(pid) => format!("{}/project/{}/?scan_id={}", config.get_url(), pid, scan_id), - None => format!( - "{}/project/{}?scan_id={}", - config.get_url(), - project_name, - scan_id - ), - }; + let scan_url = build_scan_url( + &config.get_url(), + upload_result.project_id.as_deref(), + &project_name, + &scan_id, + ); let _ = utils::generic::delete_directory(&temp_dir); print!( @@ -627,50 +628,207 @@ pub fn triggered_slug_summary(issues: &[utils::api::BlockingIssue]) -> String { names.join(", ") } +/// Whether a scan status means the scan has stopped, and if so, how it ended. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum ScanState { + Completed, + Failed, + Running, +} + +/// Classify the scan status reported by the API. +/// +/// The contract is a frozen four-value set: `complete`, `incomplete`, +/// `processing`, `scanning`. `incomplete` is terminal, and treating it as +/// running is what made the CLI poll a finished scan forever. The extra +/// aliases are defensive: terminal-sounding statuses must stop the loop. +pub fn classify_scan_status(status: &str) -> ScanState { + match status.trim().to_lowercase().as_str() { + "complete" | "completed" => ScanState::Completed, + "incomplete" | "failed" | "error" | "cancelled" | "canceled" => ScanState::Failed, + _ => ScanState::Running, + } +} + +/// How long to poll before giving up, parsed from `SCAN_TIMEOUT_ENV`. +/// +/// Backstop for a scan that never reports a terminal status. Scans that run +/// past the default raise the override. Takes the raw value so it is testable +/// without touching the environment. +fn parse_poll_timeout(raw: Option<&str>) -> Duration { + const DEFAULT_SECONDS: u64 = 10 * 60 * 60; + let seconds = match raw { + Some(raw) => match raw.trim().parse::() { + Ok(seconds) if seconds > 0 => seconds, + _ => { + log::warn!( + "Ignoring {}='{}': expected a positive whole number of seconds. Waiting up to {} hours instead.", + SCAN_TIMEOUT_ENV, + raw, + DEFAULT_SECONDS / 3600 + ); + DEFAULT_SECONDS + } + }, + None => DEFAULT_SECONDS, + }; + Duration::from_secs(seconds) +} + +/// Human-readable explanation of why a scan failed, for the terminal. +/// +/// Falls back from `failed_reason` to the reported scanner problems, so the +/// output is always more specific than "the scan failed". +pub fn format_scan_failure(scan: &utils::api::ScanResponse) -> String { + let mut lines = vec![format!("Scan {} did not complete.", scan.id)]; + if let Some(reason) = scan + .failed_reason + .as_deref() + .map(str::trim) + .filter(|reason| !reason.is_empty()) + { + lines.push(format!("Reason: {}", reason)); + } + let problems = scan_problem_lines(scan); + if !problems.is_empty() { + lines.push(String::from("Scanner problems:")); + lines.extend(problems); + } + lines.join("\n") +} + +/// Warnings for a scan that finished but is missing some scanner's results. +/// +/// `None` when nothing degraded. A secondary scanner breaking no longer fails +/// the scan, so this is the only place coverage loss surfaces. +pub fn format_scan_warnings(scan: &utils::api::ScanResponse) -> Option { + let problems = scan_problem_lines(scan); + if problems.is_empty() { + return None; + } + let mut lines = vec![String::from( + "Some scanners reported problems, so this scan may be missing results:", + )]; + lines.extend(problems); + Some(lines.join("\n")) +} + +/// One bullet per scanner problem, capped so dozens of file-level errors +/// cannot bury the rest of the output. +fn scan_problem_lines(scan: &utils::api::ScanResponse) -> Vec { + const MAX_LINES: usize = 10; + let problems: Vec<&utils::api::ScanErrorSummary> = scan + .scan_errors + .iter() + .filter(|error| error.is_problem()) + .collect(); + let mut lines: Vec = problems + .iter() + .take(MAX_LINES) + .map(|error| format_scan_error_line(error)) + .collect(); + if problems.len() > MAX_LINES { + lines.push(format!( + " ...and {} more; see the scan page for the full list.", + problems.len() - MAX_LINES + )); + } + lines +} + +fn format_scan_error_line(error: &utils::api::ScanErrorSummary) -> String { + let message = error + .message + .as_deref() + .map(str::trim) + .filter(|m| !m.is_empty()) + .unwrap_or("No details provided."); + let mut prefix = String::new(); + if let Some(scan_type) = error.scan_type.as_deref().filter(|s| !s.is_empty()) { + prefix.push_str(scan_type); + } + if let Some(location) = error.location.as_deref().filter(|l| !l.is_empty()) { + if prefix.is_empty() { + prefix.push_str(location); + } else { + prefix.push_str(&format!(" @ {}", location)); + } + } + if prefix.is_empty() { + format!(" - {}", message) + } else { + format!(" - [{}] {}", prefix, message) + } +} + +/// Block until the scan reaches a terminal state, then report it. +/// +/// Exits non-zero on failure or poll timeout, so CI cannot mistake a broken +/// scan for a clean one. pub fn wait_for_scan(config: &Config, scan_id: &str) { - // Create loading animation let stop_signal = Arc::new(Mutex::new(false)); - - // Spawn a new thread for the spinner animation let stop_signal_clone = Arc::clone(&stop_signal); - thread::spawn(move || { + let spinner = thread::spawn(move || { utils::terminal::show_loading_message( "Scanning... The Hunt Is On! ([T]s)", stop_signal_clone, ); }); - loop { - std::thread::sleep(std::time::Duration::from_secs(1)); - match check_scan_status(scan_id, &config.get_url()) { - Ok(true) => { - *stop_signal.lock().unwrap() = true; - break; - } - Ok(false) => {} + let timeout = parse_poll_timeout(env::var(SCAN_TIMEOUT_ENV).ok().as_deref()); + let started_at = Instant::now(); + + let result = loop { + thread::sleep(Duration::from_secs(1)); + match utils::api::get_scan(&config.get_url(), scan_id) { + Ok(scan) => match classify_scan_status(&scan.status) { + ScanState::Completed => break Ok(scan), + ScanState::Failed => break Err(format_scan_failure(&scan)), + ScanState::Running if started_at.elapsed() >= timeout => { + break Err(format!( + "Stopped waiting for scan {} after {}s; it is still reported as '{}'.\n\ + The scan may still finish in the Corgea cloud — check the scan page, \ + or set {} to wait longer.", + scan_id, + timeout.as_secs(), + scan.status, + SCAN_TIMEOUT_ENV + )) + } + ScanState::Running => {} + }, Err(e) => { - log::error!( - "\n\nUnable to check the scan status for scan ID '{}'.\nPlease verify that: - - The server URL '{}' is reachable. - - Your authentication token is valid. - - The scan ID '{}' exists and is correct. - - Check out our docs at https://docs.corgea.app/install_cli#login-with-the-cli - - Error details:\n{}", + break Err(format!( + "Unable to check the status of scan '{}'.\n\ + Please verify that:\n\ + - The server URL '{}' is reachable.\n\ + - Your authentication token is valid.\n\ + - The scan ID is correct.\n\n\ + Check out our docs at https://docs.corgea.app/install_cli#login-with-the-cli\n\n\ + Error details: {}", scan_id, config.get_url(), - scan_id, e - ); - std::process::exit(1); + )) } } - } + }; + + *stop_signal.lock().unwrap() = true; + let _ = spinner.join(); print!( - "{}", + "\r{}", utils::terminal::set_text_color("", utils::terminal::TerminalColor::Reset) ); + + let scan = match result { + Ok(scan) => scan, + Err(message) => { + log::error!("\n\n{}\n", message); + std::process::exit(1); + } + }; + println!( "\r╭────────────────────────────────────────────╮\n\ │ {: <42} │\n\ @@ -679,11 +837,14 @@ pub fn wait_for_scan(config: &Config, scan_id: &str) { ╰────────────────────────────────────────────╯\n", " ", " " ); + if let Some(warnings) = format_scan_warnings(&scan) { + log::warn!("{}\n", warnings); + } } /// Match doghouse `LICENSE_DEPS_WAIT_TIMEOUT` (15 minutes). -const BLOCKING_RULES_WAIT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15 * 60); -const BLOCKING_RULES_POLL_INTERVAL: std::time::Duration = std::time::Duration::from_secs(2); +const BLOCKING_RULES_WAIT_TIMEOUT: Duration = Duration::from_secs(15 * 60); +const BLOCKING_RULES_POLL_INTERVAL: Duration = Duration::from_secs(2); fn stop_blocking_rules_spinner(stop_signal: &Arc>, spinner: thread::JoinHandle<()>) { *stop_signal.lock().unwrap() = true; @@ -708,8 +869,8 @@ const BLOCKING_RULES_TIMEOUT_MESSAGE: &str = /// retry until timeout; permanent errors fail immediately. fn decide_blocking_rules_poll( result: Result, - elapsed: std::time::Duration, - timeout: std::time::Duration, + elapsed: Duration, + timeout: Duration, ) -> BlockingRulesPollDecision { match result { Ok(rules) if rules.is_complete() => BlockingRulesPollDecision::Complete(rules), @@ -763,7 +924,7 @@ fn wait_for_blocking_rules( ); }); - let started = std::time::Instant::now(); + let started = Instant::now(); loop { // Do not start another request after the deadline. if started.elapsed() >= BLOCKING_RULES_WAIT_TIMEOUT { @@ -790,14 +951,7 @@ fn wait_for_blocking_rules( std::process::exit(1); } } - std::thread::sleep(BLOCKING_RULES_POLL_INTERVAL); - } -} - -pub fn check_scan_status(scan_id: &str, url: &str) -> Result> { - match utils::api::get_scan(url, scan_id) { - Ok(scan) => Ok(scan.status == "complete"), - Err(e) => Err(e), + thread::sleep(BLOCKING_RULES_POLL_INTERVAL); } } @@ -888,7 +1042,6 @@ mod tests { BlockOnError, BlockingIssue, BlockingRuleResponse, BlockingRuleStats, SCAIssue, SCALocation, SCAPackage, BLOCKING_RULES_STATUS_COMPLETE, BLOCKING_RULES_STATUS_PENDING, }; - use std::time::Duration; fn counts(pairs: &[(&str, usize)]) -> HashMap { pairs.iter().map(|(k, v)| (k.to_string(), *v)).collect() @@ -1275,4 +1428,268 @@ mod tests { "block_on was provided but contained no rule slugs." ); } + + fn scan_with( + status: &str, + failed_reason: Option<&str>, + scan_errors: Vec, + ) -> utils::api::ScanResponse { + utils::api::ScanResponse { + id: "scan-123".to_string(), + project: "proj".to_string(), + repo: None, + branch: None, + status: status.to_string(), + engine: "corgea-blast".to_string(), + created_at: "2026-01-01T00:00:00Z".to_string(), + git_sha: None, + metadata: None, + failed_reason: failed_reason.map(|r| r.to_string()), + scan_errors, + } + } + + fn scan_error( + scan_type: Option<&str>, + level: Option<&str>, + location: Option<&str>, + message: Option<&str>, + ) -> utils::api::ScanErrorSummary { + utils::api::ScanErrorSummary { + scan_type: scan_type.map(|s| s.to_string()), + level: level.map(|s| s.to_string()), + location: location.map(|s| s.to_string()), + message: message.map(|s| s.to_string()), + } + } + + #[test] + fn incomplete_status_is_terminal_not_still_running() { + // The hang: "incomplete" is terminal, and treating it as running meant + // polling a finished scan indefinitely. + assert_eq!(classify_scan_status("incomplete"), ScanState::Failed); + } + + #[test] + fn classify_scan_status_covers_the_api_contract() { + assert_eq!(classify_scan_status("complete"), ScanState::Completed); + assert_eq!(classify_scan_status("processing"), ScanState::Running); + assert_eq!(classify_scan_status("scanning"), ScanState::Running); + assert_eq!(classify_scan_status("incomplete"), ScanState::Failed); + } + + #[test] + fn classify_scan_status_ignores_case_and_padding() { + // `corgea wait` compared against "Complete" while the API sends + // lowercase, so a finished scan was polled again. + assert_eq!(classify_scan_status("Complete"), ScanState::Completed); + assert_eq!(classify_scan_status(" COMPLETE "), ScanState::Completed); + assert_eq!(classify_scan_status("Incomplete"), ScanState::Failed); + } + + #[test] + fn unknown_status_keeps_waiting_rather_than_failing_the_build() { + assert_eq!(classify_scan_status("queued"), ScanState::Running); + assert_eq!(classify_scan_status(""), ScanState::Running); + } + + #[test] + fn poll_timeout_rejects_overrides_that_are_not_a_positive_number() { + // A bad value must not shorten or disable the wait: anything that is + // not a positive count of seconds falls back to the default. + let default = Duration::from_secs(10 * 60 * 60); + assert_eq!(parse_poll_timeout(None), default); + assert_eq!(parse_poll_timeout(Some("")), default); + assert_eq!(parse_poll_timeout(Some("abc")), default); + assert_eq!(parse_poll_timeout(Some("0")), default); + assert_eq!(parse_poll_timeout(Some("-30")), default); + assert_eq!(parse_poll_timeout(Some("1.5")), default); + assert_eq!(parse_poll_timeout(Some(" 90 ")), Duration::from_secs(90)); + } + + #[test] + fn scan_failure_reports_reason_and_errors() { + let scan = scan_with( + "incomplete", + Some("Dependency Analysis did not finish."), + vec![scan_error( + Some("sca"), + Some("error"), + Some("Project-wide"), + Some("Could not reach the public package registry."), + )], + ); + + let output = format_scan_failure(&scan); + + assert!(output.contains("scan-123")); + assert!(output.contains("Dependency Analysis did not finish.")); + assert!(output.contains("[sca @ Project-wide]")); + assert!(output.contains("Could not reach the public package registry.")); + } + + #[test] + fn scan_failure_without_reason_still_explains_itself() { + let output = format_scan_failure(&scan_with("incomplete", None, vec![])); + + assert!(output.contains("did not complete")); + assert!(!output.contains("Reason:")); + } + + #[test] + fn scan_failure_omits_blank_reason() { + let output = format_scan_failure(&scan_with("incomplete", Some(" "), vec![])); + + assert!(!output.contains("Reason:")); + } + + #[test] + fn scan_output_skips_informational_notes() { + // `info` entries are bookkeeping, not missing results. + let scan = scan_with( + "incomplete", + None, + vec![ + scan_error(Some("sca"), Some("info"), None, Some("some info")), + scan_error(Some("sca"), Some("warning"), None, Some("a warning")), + scan_error(Some("iac"), Some("error"), None, Some("a real error")), + ], + ); + + let output = format_scan_failure(&scan); + + assert!(output.contains("a real error")); + assert!(output.contains("a warning")); + assert!(!output.contains("some info")); + } + + #[test] + fn info_only_scan_produces_no_warnings() { + let scan = scan_with( + "complete", + None, + vec![scan_error(Some("sca"), Some("info"), None, Some("skipped"))], + ); + + assert!(format_scan_warnings(&scan).is_none()); + } + + #[test] + fn missing_or_unknown_level_is_still_reported() { + // The server defaults an absent level to "error" and may add levels + // this client does not know; neither may drop a real failure. + assert!(scan_error(Some("sca"), None, None, Some("boom")).is_problem()); + assert!(scan_error(Some("sca"), Some("critical"), None, Some("boom")).is_problem()); + assert!(!scan_error(Some("sca"), Some("INFO"), None, Some("fyi")).is_problem()); + } + + #[test] + fn long_error_lists_are_capped() { + let errors = (0..25) + .map(|i| { + scan_error( + Some("sast"), + Some("error"), + None, + Some(&format!("boom {}", i)), + ) + }) + .collect(); + + let output = format_scan_failure(&scan_with("incomplete", None, errors)); + + assert!(output.contains("boom 9")); + assert!(!output.contains("boom 10")); + assert!(output.contains("...and 15 more")); + } + + #[test] + fn completed_scan_with_scanner_errors_produces_warnings() { + // Warnings on a completed scan are the only signal coverage dropped. + let scan = scan_with( + "complete", + None, + vec![scan_error( + Some("sca"), + Some("error"), + Some("Project-wide"), + Some("Dependency Analysis did not finish."), + )], + ); + + let warnings = format_scan_warnings(&scan).expect("expected warnings"); + + assert!(warnings.contains("may be missing results")); + assert!(warnings.contains("Dependency Analysis did not finish.")); + } + + #[test] + fn clean_scan_produces_no_warnings() { + assert!(format_scan_warnings(&scan_with("complete", None, vec![])).is_none()); + } + + #[test] + fn scan_error_line_survives_missing_fields() { + assert_eq!( + format_scan_error_line(&scan_error(None, None, None, None)), + " - No details provided." + ); + assert_eq!( + format_scan_error_line(&scan_error(None, None, Some("pom.xml"), Some("bad"))), + " - [pom.xml] bad" + ); + assert_eq!( + format_scan_error_line(&scan_error(Some("sca"), None, None, Some("bad"))), + " - [sca] bad" + ); + } + + #[test] + fn scan_response_deserializes_without_new_fields() { + // Older servers do not send failed_reason/scan_errors; the client must + // still parse their responses. + let json = r#"{ + "id": "abc", + "project": "p", + "repo": null, + "branch": "main", + "status": "complete", + "engine": "corgea-blast", + "created_at": "2026-01-01T00:00:00Z" + }"#; + + let scan: utils::api::ScanResponse = serde_json::from_str(json).unwrap(); + + assert_eq!(classify_scan_status(&scan.status), ScanState::Completed); + assert!(scan.failed_reason.is_none()); + assert!(scan.scan_errors.is_empty()); + // `corgea ls --json` serializes from the scan list, which never carries + // these fields; empty ones there would claim a scan had no problems. + let round_tripped = serde_json::to_string(&scan).unwrap(); + assert!(!round_tripped.contains("scan_errors")); + assert!(!round_tripped.contains("failed_reason")); + } + + #[test] + fn scan_response_deserializes_null_scan_errors() { + // The API sends `"scan_errors": null` when there is nothing to report, + // including on failed scans, where a parse error would hide the reason. + let json = r#"{ + "id": "abc", + "project": "p", + "repo": null, + "branch": "main", + "status": "incomplete", + "engine": "corgea-blast", + "created_at": "2026-01-01T00:00:00Z", + "failed_reason": "the scanner ran out of memory", + "scan_errors": null + }"#; + + let scan: utils::api::ScanResponse = serde_json::from_str(json).unwrap(); + + assert_eq!(classify_scan_status(&scan.status), ScanState::Failed); + assert!(scan.scan_errors.is_empty()); + assert!(format_scan_failure(&scan).contains("the scanner ran out of memory")); + } } diff --git a/src/utils/api.rs b/src/utils/api.rs index 6bfc784..fbdd8da 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -1351,7 +1351,46 @@ pub fn get_all_sca_issues( Ok(all_issues) } -#[derive(Deserialize, Serialize, Debug)] +/// One scanner problem reported against a scan, already sanitized server-side. +/// +/// Fields are optional so older servers still deserialize. +#[derive(Deserialize, Serialize, Debug, Clone)] +pub struct ScanErrorSummary { + #[serde(default)] + pub scan_type: Option, + #[serde(default)] + pub level: Option, + #[serde(default)] + pub location: Option, + #[serde(default)] + pub message: Option, +} + +impl ScanErrorSummary { + /// Whether this entry means the scan is missing results. + /// + /// `info` entries are notes. Everything else counts, including absent or + /// unrecognized levels, which the server treats as `error`. + pub fn is_problem(&self) -> bool { + !self + .level + .as_deref() + .is_some_and(|level| level.trim().eq_ignore_ascii_case("info")) + } +} + +/// Reads a missing field, an explicit `null`, and a list all as a list. +/// +/// `#[serde(default)]` alone only covers the missing case, and the API sends +/// `"scan_errors": null` for scans with nothing to report. +fn scan_errors_or_empty<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + Ok(Option::>::deserialize(deserializer)?.unwrap_or_default()) +} + +#[derive(Deserialize, Serialize, Debug, Clone)] pub struct ScanResponse { pub id: String, pub project: String, @@ -1364,6 +1403,18 @@ pub struct ScanResponse { pub git_sha: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub metadata: Option, + /// Why a scan ended without finishing. Only set for failed scans. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub failed_reason: Option, + /// Per-scanner problems, present on completed scans too, where they mean a + /// scanner's results are missing. Skipped when empty, since the scan list + /// never carries them. + #[serde( + default, + deserialize_with = "scan_errors_or_empty", + skip_serializing_if = "Vec::is_empty" + )] + pub scan_errors: Vec, } #[derive(Serialize, Deserialize, Debug)] diff --git a/src/wait.rs b/src/wait.rs index 3ef5eca..0b83912 100644 --- a/src/wait.rs +++ b/src/wait.rs @@ -1,4 +1,5 @@ use crate::config::Config; +use crate::scan::build_scan_url; use crate::scanners::blast; use crate::utils; use crate::utils::api::ProjectSelector; @@ -12,131 +13,135 @@ pub struct WaitArgs { pub project_id: Option, } +/// Most recent scan of the resolved project, or a hard exit naming what was +/// tried when it has none. +fn latest_scan_id(config: &Config, resolved: &utils::api::ResolvedProject) -> String { + let scans = match utils::api::query_scan_list( + &config.get_url(), + Some(&resolved.query_name), + Some(1), + None, + ) { + Ok(result) => result.scans.unwrap_or_default(), + Err(e) => { + log::error!( + "Unable to query the scan list. Please check your connection and ensure that:\n\ + - The server URL is reachable.\n\ + - Your authentication token is valid.\n\n\ + Check out our docs at https://docs.corgea.app/install_cli#login-with-the-cli\n\n\ + Error details: {}", + e + ); + std::process::exit(1); + } + }; + match scans.first() { + Some(scan) => scan.id.clone(), + None => { + if resolved.confirmed { + log::error!( + "Project '{}' has no scans yet. Run 'corgea scan' to start one.", + resolved.query_name + ); + } else { + log::error!( + "No scans found for {}. Run 'corgea scan', or pass --scan-id.", + resolved.tried_label + ); + } + std::process::exit(1); + } + } +} + pub fn run(config: &Config, args: WaitArgs) { let WaitArgs { scan_id, selector, project_id, } = args; - // A scan id alone leaves nothing to resolve: everything below keys off - // the scan, and `blast::check_scan_status`/`report_scan_status` fetch by - // scan id regardless of whether the project id came along too. - let resolved = if scan_id.is_some() { - let name = selector - .name - .clone() - .unwrap_or_else(|| utils::generic::determine_project_name(None)); - utils::api::ResolvedProject { - // `confirmed`/`tried_label` are only read on the no-scan-id path. - tried_label: format!("project '{}'", name), - query_name: name, - confirmed: false, - } - } else { - utils::api::resolve_project_or_exit(&config.get_url(), &selector) - }; - let project_name = resolved.query_name.clone(); - // Only the scan-less path reads the listing. - let scans: Vec = if scan_id.is_some() { - Vec::new() - } else { - match utils::api::query_scan_list(&config.get_url(), Some(&project_name), Some(1), None) { - Ok(result) => result.scans.unwrap_or_default(), - Err(e) => { - log::error!( - "Unable to query the scan list. Please check your connection and ensure that: - - The server URL is reachable. - - Your authentication token is valid. - - Check out our docs at https://docs.corgea.app/install_cli#login-with-the-cli - - Error details: {}", - e - ); - std::process::exit(1); - } + // A scan id alone leaves nothing to resolve: the scan and issue endpoints + // both fetch by scan id, so neither /projects nor the listing is dialed. + let (scan_id, resolved_name) = match scan_id { + Some(scan_id) => (scan_id, None), + None => { + let resolved = utils::api::resolve_project_or_exit(&config.get_url(), &selector); + let scan_id = latest_scan_id(config, &resolved); + (scan_id, Some(resolved.query_name)) } }; - let (scan_id, processed, project_name) = match scan_id { - Some(scan_id) => { - let scan = match utils::api::get_scan(&config.get_url(), &scan_id) { - Ok(scan) => scan, - Err(_) => { - log::error!( - "\nOops! Something went wrong. Please try again later or check your setup.\n" - ); - std::process::exit(1); - } - }; - let processed = scan.status == "complete"; - // Explicit `--project-name` (or an uploaded name passed in) wins; - // otherwise trust the canonical project the backend just - // returned for this scan over the locally recomputed name. - let project_name = selector.name.clone().unwrap_or_else(|| { - let canonical = scan.project.trim(); - if canonical.is_empty() { - project_name.clone() - } else { - canonical.to_string() - } - }); - (scan_id.to_string(), processed, project_name) + + // Read the scan itself: the listing omits failed_reason and scan_errors, + // which are the only record of why a scan ended badly. + let scan = match utils::api::get_scan(&config.get_url(), &scan_id) { + Ok(scan) => scan, + Err(e) => { + log::error!( + "\nUnable to read scan '{}'. Please check your connection and token, then try again.\n\nError details: {}\n", + scan_id, + e + ); + std::process::exit(1); } - None => match scans.first() { - Some(scan) => (scan.id.clone(), scan.status == "Complete", project_name), - None => { - if resolved.confirmed { - log::error!( - "Project '{}' has no scans yet. Run 'corgea scan' to start one.", - project_name - ); - } else { - log::error!( - "No scans found for {}. Run 'corgea scan', or pass --scan-id.", - resolved.tried_label - ); - } - std::process::exit(1); - } - }, }; - let scan_url = match &project_id { - Some(pid) => format!("{}/project/{}/?scan_id={}", config.get_url(), pid, scan_id), - None => { - // The project name is a free-form path segment (canonical names - // contain `/`, e.g. `bohappdev/dotnet-azure-web-tsb`), so it must - // be percent-encoded — see `build_scan_url_from_base` in scan.rs. - let name = project_name.trim().trim_matches('/'); - if name.is_empty() { - log::error!( - "Cannot build the scan URL: no Corgea project resolved. Pass --project-name ." - ); - std::process::exit(1); + // A resolved name already drove the listing query, so keep it. Otherwise an + // explicit `--project-name` (or an uploaded name passed in) wins, then the + // canonical project the backend returned for this scan, in preference to a + // name recomputed from the checkout. + let project_name = resolved_name + .or_else(|| selector.name.clone()) + .unwrap_or_else(|| { + let canonical = scan.project.trim(); + if canonical.is_empty() { + utils::generic::determine_project_name(None) + } else { + canonical.to_string() } - format!( - "{}/project/{}?scan_id={}", - config.get_url(), - urlencoding::encode(name), - scan_id - ) - } - }; + }); - if !processed { - print!( - "\n\nWaiting for scan with ID: {}.\n\nYou can view it populate at the link:\n{}\n\n", - scan_id, - utils::terminal::set_text_color(&scan_url, utils::terminal::TerminalColor::Green) + // Canonical names contain `/` (e.g. `bohappdev/dotnet-azure-web-tsb`), which + // `build_scan_url` percent-encodes into a single path segment. + let url_name = project_name.trim().trim_matches('/'); + if project_id.is_none() && url_name.is_empty() { + log::error!( + "Cannot build the scan URL: no Corgea project resolved. Pass --project-name ." ); - print!( - "{}", - utils::terminal::set_text_color("Your scan will continue securely in the Corgea cloud.\nYou can safely exit the process now if you prefer not to wait for it to complete.\n\n", utils::terminal::TerminalColor::Blue) - ); - blast::wait_for_scan(config, &scan_id); - } else { - println!("Scan has been processed successfully!"); + std::process::exit(1); + } + let scan_url = build_scan_url(&config.get_url(), project_id.as_deref(), url_name, &scan_id); + + // The API reports lowercase statuses, so comparing against "Complete" + // never matched and finished scans were polled again. + match blast::classify_scan_status(&scan.status) { + blast::ScanState::Running => { + print!( + "\n\nWaiting for scan with ID: {}.\n\nYou can view it populate at the link:\n{}\n\n", + scan_id, + utils::terminal::set_text_color(&scan_url, utils::terminal::TerminalColor::Green) + ); + print!( + "{}", + utils::terminal::set_text_color("Your scan will continue securely in the Corgea cloud.\nYou can safely exit the process now if you prefer not to wait for it to complete.\n\n", utils::terminal::TerminalColor::Blue) + ); + blast::wait_for_scan(config, &scan_id); + } + blast::ScanState::Completed => { + println!("Scan has been processed successfully!"); + if let Some(warnings) = blast::format_scan_warnings(&scan) { + log::warn!("\n{}\n", warnings); + } + } + // Report the failure rather than claim success or poll forever. + blast::ScanState::Failed => { + log::error!("\n\n{}\n", blast::format_scan_failure(&scan)); + println!( + "\nYou can view the scan details at the following link:\n{}", + utils::terminal::set_text_color(&scan_url, utils::terminal::TerminalColor::Blue) + ); + std::process::exit(1); + } } match blast::report_scan_status(&config.get_url(), &project_name, &scan_id) { diff --git a/tests/cli_scan_wait_terminal_state.rs b/tests/cli_scan_wait_terminal_state.rs new file mode 100644 index 0000000..7df14e1 --- /dev/null +++ b/tests/cli_scan_wait_terminal_state.rs @@ -0,0 +1,225 @@ +//! End-to-end coverage for how `corgea wait` handles terminal scan states. +//! +//! The CLI's terminal check was `status == "complete"`, so `incomplete` fell +//! through to the still-running branch of an untimed loop and a failed scan +//! polled forever. These tests drive the real binary against a stubbed scan API +//! and kill it if it outlives `MAX_RUNTIME`, so a hang fails a test instead of +//! stalling the suite. + +mod common; + +use std::process::Stdio; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +const SCAN_ID: &str = "5ba108cb-fc2e-4f2e-a3ba-d5e4fbfe77ac"; + +/// Bound every run: a hang is the bug under test, so exceeding this is failure. +const MAX_RUNTIME: Duration = Duration::from_secs(30); + +fn scan_json(status: &str, failed_reason: &str, scan_errors: &str) -> String { + let reason = if failed_reason.is_empty() { + String::from("null") + } else { + format!("\"{}\"", failed_reason) + }; + // The API sends `null`, not `[]`, when a scan has no problems to report. + let errors = if scan_errors.is_empty() { + String::from("null") + } else { + format!("[{}]", scan_errors) + }; + format!( + r#"{{"id":"{SCAN_ID}","project":"proj","repo":null,"branch":"main", + "status":"{status}","engine":"corgea-blast", + "created_at":"2026-08-01T15:16:31Z","time_taken":480, + "git_sha":"abc123","metadata":null, + "failed_reason":{reason},"scan_errors":{errors}}}"# + ) +} + +fn issues_json() -> String { + String::from(r#"{"status":"ok","issues":[],"page":1,"total_pages":1,"total_issues":0}"#) +} + +/// Stub the scan API, walking `statuses` one entry per scan read and holding on +/// the last, so one stub can model a scan that is already terminal or one that +/// turns terminal mid-poll. +/// +/// The issues route must be matched first: it lives under `/api/v1/scan/`. +fn spawn_scan_api( + statuses: &'static [&'static str], + reason: &'static str, + errors: &'static str, +) -> String { + let reads = Arc::new(AtomicUsize::new(0)); + common::spawn_http_stub(move |path| { + if path.contains("/issues") { + return ("200 OK", issues_json()); + } + if path.starts_with("/api/v1/scan/") { + let read = reads.fetch_add(1, Ordering::SeqCst); + let status = statuses[read.min(statuses.len() - 1)]; + return ("200 OK", scan_json(status, reason, errors)); + } + ("200 OK", String::from(r#"{"status":"ok"}"#)) + }) +} + +/// Run `corgea wait ` against `url`, failing rather than blocking +/// forever if the command does not exit within `MAX_RUNTIME`. +fn run_wait(url: &str, env: &[(&str, &str)]) -> (Option, String) { + let (mut cmd, _home) = common::corgea_isolated(); + cmd.env("CORGEA_URL", url) + .env("CORGEA_TOKEN", "test-token") + .args(["wait", SCAN_ID]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + for (key, value) in env { + cmd.env(key, value); + } + + let mut child = cmd.spawn().expect("run corgea wait"); + let deadline = Instant::now() + MAX_RUNTIME; + let status = loop { + match child.try_wait().expect("poll corgea wait") { + Some(status) => break status, + None if Instant::now() >= deadline => { + let _ = child.kill(); + let _ = child.wait(); + panic!("`corgea wait` did not exit within {MAX_RUNTIME:?} — it is hanging again"); + } + None => std::thread::sleep(Duration::from_millis(50)), + } + }; + + let out = child + .wait_with_output() + .expect("collect corgea wait output"); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + (status.code(), combined) +} + +const SCA_FAILURE: &str = r#"{"scan_type":"sca","level":"error","location":"Project-wide", + "message":"Could not read dependency metadata from the package registry."}"#; + +#[test] +fn wait_on_already_failed_scan_exits_nonzero() { + let url = spawn_scan_api( + &["incomplete"], + "Dependency Analysis did not finish.", + SCA_FAILURE, + ); + + let (code, output) = run_wait(&url, &[]); + + assert_eq!( + code, + Some(1), + "a failed scan must fail the command: {output}" + ); + assert!( + output.contains("Dependency Analysis did not finish."), + "failure reason must reach the user: {output}" + ); + assert!( + output.contains("Could not read dependency metadata from the package registry."), + "scanner error must reach the user: {output}" + ); + assert!( + !output.contains("Scan Completed Successfully"), + "a failed scan must never print the success banner: {output}" + ); +} + +#[test] +fn failed_scan_without_scanner_errors_still_reports_the_reason() { + // A failed scan with nothing per-scanner to report carries + // `"scan_errors": null`. Rejecting that shape while parsing would turn the + // failure into a generic read error and lose the reason entirely. + let url = spawn_scan_api(&["incomplete"], "The scan worker ran out of memory.", ""); + + let (code, output) = run_wait(&url, &[]); + + assert_eq!( + code, + Some(1), + "a failed scan must fail the command: {output}" + ); + assert!( + output.contains("The scan worker ran out of memory."), + "failure reason must reach the user: {output}" + ); +} + +#[test] +fn scan_that_fails_while_being_polled_exits_nonzero() { + // The reported shape: still running at first and `incomplete` later, so the + // terminal check inside the poll loop is the one that matters. + let url = spawn_scan_api( + &["processing", "incomplete"], + "Dependency Analysis did not finish.", + SCA_FAILURE, + ); + + let (code, output) = run_wait(&url, &[]); + + assert_eq!( + code, + Some(1), + "a scan that fails mid-poll must fail the command: {output}" + ); + assert!( + output.contains("Dependency Analysis did not finish."), + "failure reason must reach the user: {output}" + ); +} + +#[test] +fn wait_on_completed_scan_succeeds() { + let url = spawn_scan_api(&["complete"], "", ""); + + let (code, output) = run_wait(&url, &[]); + + assert_eq!(code, Some(0), "a clean scan must succeed: {output}"); + assert!( + !output.contains("may be missing results"), + "a clean scan must not warn: {output}" + ); +} + +#[test] +fn completed_scan_reports_missing_scanner_results() { + // This warning is the only place the user learns coverage dropped. + let degraded = r#"{"scan_type":"sca","level":"error","location":"Project-wide", + "message":"Dependency Analysis did not finish, so those results are missing."}"#; + let url = spawn_scan_api(&["processing", "complete"], "", degraded); + + let (code, output) = run_wait(&url, &[]); + + assert_eq!(code, Some(0), "a degraded scan still succeeds: {output}"); + assert!( + output.contains("Dependency Analysis did not finish, so those results are missing."), + "degraded coverage must be reported: {output}" + ); +} + +#[test] +fn wait_stops_polling_a_scan_that_never_finishes() { + // Guards the timeout: without it, a scan stuck in a non-terminal status + // polls forever. + let url = spawn_scan_api(&["processing"], "", ""); + + let (code, output) = run_wait(&url, &[("CORGEA_SCAN_TIMEOUT_SECONDS", "3")]); + + assert_eq!(code, Some(1), "a timeout must fail the command: {output}"); + assert!( + output.contains("Stopped waiting"), + "timeout must explain itself: {output}" + ); +} diff --git a/tests/cloud_commands_e2e/upload_wait.rs b/tests/cloud_commands_e2e/upload_wait.rs index 994ff77..05c545d 100644 --- a/tests/cloud_commands_e2e/upload_wait.rs +++ b/tests/cloud_commands_e2e/upload_wait.rs @@ -145,5 +145,8 @@ fn wait_exits_one_when_scan_detail_fails() { let context = output_context(&output, &transcript); assert_eq!(output.status.code(), Some(1), "{context}"); let stderr = String::from_utf8_lossy(&output.stderr); - assert!(stderr.contains("Oops! Something went wrong"), "{context}"); + assert!( + stderr.contains(&format!("Unable to read scan '{scan_id}'")), + "{context}" + ); }