From 4422008ddfd7ebe6a33ff4f37ea13005a3bbfbb5 Mon Sep 17 00:00:00 2001 From: Leen Kilani Date: Sun, 2 Aug 2026 10:40:53 +0300 Subject: [PATCH 1/3] Stop waiting forever on scans that already ended --- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 10 + skills/corgea/SKILL.md | 7 +- src/scanners/blast.rs | 449 ++++++++++++++++++++++++-- src/utils/api.rs | 38 ++- src/wait.rs | 112 ++++--- tests/cli_scan_wait_terminal_state.rs | 199 ++++++++++++ 8 files changed, 737 insertions(+), 82 deletions(-) create mode 100644 tests/cli_scan_wait_terminal_state.rs diff --git a/Cargo.lock b/Cargo.lock index e35738f..f5c08d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -369,7 +369,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" [[package]] name = "corgea" -version = "1.9.3" +version = "1.9.4" dependencies = [ "chrono", "clap", diff --git a/Cargo.toml b/Cargo.toml index 2543dc4..173fabf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "corgea" -version = "1.9.3" +version = "1.9.4" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/README.md b/README.md index 44ee09d..1781540 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 4 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 ddff6c3..1b08ee5 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -66,9 +66,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 4 hours; override with +`CORGEA_SCAN_TIMEOUT_SECONDS`. + ### List — `corgea list` (alias: `corgea ls`) ```bash diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index a4c1384..9d7104f 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -4,12 +4,15 @@ 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, @@ -538,50 +541,207 @@ pub fn fail_on_gate_trips( }) } +/// 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, + } +} + +/// Upper bound on polling, overridable with `SCAN_TIMEOUT_ENV`. +/// +/// A scan that never reaches a terminal state (dropped worker, superseded scan) +/// would otherwise burn a CI job's whole time budget. The default sits far +/// above any real scan. +fn scan_poll_timeout() -> Duration { + const DEFAULT_SECONDS: u64 = 4 * 60 * 60; + let seconds = match env::var(SCAN_TIMEOUT_ENV) { + Ok(raw) => match raw.trim().parse::() { + Ok(seconds) if seconds > 0 => seconds, + _ => { + log::warn!( + "Ignoring {}='{}': expected a positive whole number of seconds. Waiting up to {}s instead.", + SCAN_TIMEOUT_ENV, + raw, + DEFAULT_SECONDS + ); + DEFAULT_SECONDS + } + }, + Err(_) => 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 = scan_poll_timeout(); + 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\ @@ -590,12 +750,8 @@ pub fn wait_for_scan(config: &Config, scan_id: &str) { ╰────────────────────────────────────────────╯\n", " ", " " ); -} - -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), + if let Some(warnings) = format_scan_warnings(&scan) { + log::warn!("{}\n", warnings); } } @@ -847,4 +1003,231 @@ mod tests { assert_eq!(map.get("k").and_then(|v| v.as_str()), Some("")); assert!(metadata_json_from_pairs(&[]).unwrap().is_none()); } + + 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 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")); + } } diff --git a/src/utils/api.rs b/src/utils/api.rs index 2356b6a..b169864 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -935,7 +935,35 @@ 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")) + } +} + +#[derive(Deserialize, Serialize, Debug, Clone)] pub struct ScanResponse { pub id: String, pub project: String, @@ -948,6 +976,14 @@ 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, 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 dafb59c..152bdb5 100644 --- a/src/wait.rs +++ b/src/wait.rs @@ -2,48 +2,55 @@ use crate::config::Config; use crate::scanners::blast; use crate::utils; -pub fn run(config: &Config, scan_id: Option, project_id: Option) { +/// Most recent scan of the project in the current directory. +fn latest_scan_id(config: &Config) -> String { let project_name = utils::generic::determine_project_name(None); - - let scans_result = - utils::api::query_scan_list(&config.get_url(), Some(&project_name), Some(1), None); - let scans: Vec = match scans_result { + let scans = 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 + "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 => { + log::error!("No scans found for project '{}'.", project_name); + std::process::exit(1); + } + } +} - Error details: {}", +pub fn run(config: &Config, scan_id: Option, project_id: Option) { + let scan_id = scan_id.unwrap_or_else(|| latest_scan_id(config)); + // Read the scan itself: the scan list omits failed_reason and scan_errors. + 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); } }; - let (scan_id, processed) = match scan_id { - Some(scan_id) => { - let processed = match blast::check_scan_status(&scan_id, &config.get_url()) { - Ok(processed) => processed, - Err(_) => { - log::error!( - "\nOops! Something went wrong. Please try again later or check your setup.\n" - ); - std::process::exit(1); - } - }; - (scan_id.to_string(), processed) - } - None => match scans.first() { - Some(scan) => (scan.id.clone(), scan.status == "Complete"), - None => { - log::error!("Error querying scan list"); - std::process::exit(1); - } - }, - }; + let project_name = scan.project.clone(); + // The API reports lowercase statuses, so comparing against "Complete" + // never matched and finished scans were polled again. + let state = blast::classify_scan_status(&scan.status); let scan_url = match &project_id { Some(pid) => format!("{}/project/{}/?scan_id={}", config.get_url(), pid, scan_id), @@ -55,19 +62,34 @@ pub fn run(config: &Config, scan_id: Option, project_id: Option) ), }; - 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) - ); - 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!"); + match state { + 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..75d83bb --- /dev/null +++ b/tests/cli_scan_wait_terminal_state.rs @@ -0,0 +1,199 @@ +//! 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) + }; + 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":[{scan_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 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}" + ); +} From 55992096d2ea7433f0b1d3b38afed88e8be4660e Mon Sep 17 00:00:00 2001 From: Leen Kilani Date: Sun, 2 Aug 2026 14:25:35 +0300 Subject: [PATCH 2/3] Address comments --- src/scan.rs | 67 ++++++++++++++----------- src/scanners/blast.rs | 72 ++++++++++++++++++++------- src/utils/api.rs | 17 ++++++- src/wait.rs | 16 +++--- tests/cli_scan_wait_terminal_state.rs | 28 ++++++++++- 5 files changed, 143 insertions(+), 57 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index c533ac7..f2e5357 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, @@ -574,18 +578,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"); } @@ -593,11 +593,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 a72c587..2c2391b 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -1,4 +1,5 @@ use crate::config::Config; +use crate::scan::build_scan_url; use crate::targets; use crate::utils; use crate::utils::api::SCAIssue; @@ -239,15 +240,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!( @@ -581,27 +579,28 @@ pub fn classify_scan_status(status: &str) -> ScanState { } } -/// Upper bound on polling, overridable with `SCAN_TIMEOUT_ENV`. +/// Upper bound on polling, from the raw `SCAN_TIMEOUT_ENV` value. /// /// A scan that never reaches a terminal state (dropped worker, superseded scan) /// would otherwise burn a CI job's whole time budget. The default sits far -/// above any real scan. -fn scan_poll_timeout() -> Duration { +/// above any real scan. Takes the raw value rather than reading the environment +/// so the override can be tested without mutating the process. +fn parse_poll_timeout(raw: Option<&str>) -> Duration { const DEFAULT_SECONDS: u64 = 4 * 60 * 60; - let seconds = match env::var(SCAN_TIMEOUT_ENV) { - Ok(raw) => match raw.trim().parse::() { + 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 {}s instead.", + "Ignoring {}='{}': expected a positive whole number of seconds. Waiting up to {} hours instead.", SCAN_TIMEOUT_ENV, raw, - DEFAULT_SECONDS + DEFAULT_SECONDS / 3600 ); DEFAULT_SECONDS } }, - Err(_) => DEFAULT_SECONDS, + None => DEFAULT_SECONDS, }; Duration::from_secs(seconds) } @@ -706,7 +705,7 @@ pub fn wait_for_scan(config: &Config, scan_id: &str) { ); }); - let timeout = scan_poll_timeout(); + let timeout = parse_poll_timeout(env::var(SCAN_TIMEOUT_ENV).ok().as_deref()); let started_at = Instant::now(); let result = loop { @@ -1086,6 +1085,20 @@ mod tests { 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(4 * 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( @@ -1248,4 +1261,27 @@ mod tests { 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 b169864..62a0cec 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -963,6 +963,17 @@ impl ScanErrorSummary { } } +/// 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, @@ -982,7 +993,11 @@ pub struct ScanResponse { /// 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, skip_serializing_if = "Vec::is_empty")] + #[serde( + default, + deserialize_with = "scan_errors_or_empty", + skip_serializing_if = "Vec::is_empty" + )] pub scan_errors: Vec, } diff --git a/src/wait.rs b/src/wait.rs index 152bdb5..ed38c52 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; @@ -52,15 +53,12 @@ pub fn run(config: &Config, scan_id: Option, project_id: Option) // never matched and finished scans were polled again. let state = blast::classify_scan_status(&scan.status); - let scan_url = match &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(), + project_id.as_deref(), + &project_name, + &scan_id, + ); match state { blast::ScanState::Running => { diff --git a/tests/cli_scan_wait_terminal_state.rs b/tests/cli_scan_wait_terminal_state.rs index 75d83bb..7df14e1 100644 --- a/tests/cli_scan_wait_terminal_state.rs +++ b/tests/cli_scan_wait_terminal_state.rs @@ -24,12 +24,18 @@ fn scan_json(status: &str, failed_reason: &str, scan_errors: &str) -> String { } 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":[{scan_errors}]}}"# + "failed_reason":{reason},"scan_errors":{errors}}}"# ) } @@ -131,6 +137,26 @@ fn wait_on_already_failed_scan_exits_nonzero() { ); } +#[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 From 22728d1c8892d7cc3193e8a2273f0c92a59ec380 Mon Sep 17 00:00:00 2001 From: Leen Kilani Date: Mon, 3 Aug 2026 19:15:02 +0300 Subject: [PATCH 3/3] bump timeout to 10 hours and update documentation --- README.md | 2 +- skills/corgea/SKILL.md | 2 +- src/scanners/blast.rs | 13 ++++++------- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 1781540..153f1e4 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ 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 4 hours; override with `CORGEA_SCAN_TIMEOUT_SECONDS`. +Waiting gives up after 10 hours; override with `CORGEA_SCAN_TIMEOUT_SECONDS`. ## Dependency Inventory (offline) diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index 7c09d5f..30366cb 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -73,7 +73,7 @@ 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 4 hours; override with +warning. Polling gives up after 10 hours; override with `CORGEA_SCAN_TIMEOUT_SECONDS`. ### List — `corgea list` (alias: `corgea ls`) diff --git a/src/scanners/blast.rs b/src/scanners/blast.rs index 2c2391b..8b55c3a 100644 --- a/src/scanners/blast.rs +++ b/src/scanners/blast.rs @@ -579,14 +579,13 @@ pub fn classify_scan_status(status: &str) -> ScanState { } } -/// Upper bound on polling, from the raw `SCAN_TIMEOUT_ENV` value. +/// How long to poll before giving up, parsed from `SCAN_TIMEOUT_ENV`. /// -/// A scan that never reaches a terminal state (dropped worker, superseded scan) -/// would otherwise burn a CI job's whole time budget. The default sits far -/// above any real scan. Takes the raw value rather than reading the environment -/// so the override can be tested without mutating the process. +/// 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 = 4 * 60 * 60; + const DEFAULT_SECONDS: u64 = 10 * 60 * 60; let seconds = match raw { Some(raw) => match raw.trim().parse::() { Ok(seconds) if seconds > 0 => seconds, @@ -1089,7 +1088,7 @@ mod tests { 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(4 * 60 * 60); + 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);