diff --git a/src/list.rs b/src/list.rs index f0b66c0..ca2b671 100644 --- a/src/list.rs +++ b/src/list.rs @@ -1,33 +1,61 @@ use crate::config::Config; use crate::log::debug; use crate::utils; +use crate::utils::api::ProjectSelector; use serde_json::json; use std::path::Path; -pub fn run( - config: &Config, - issues: &bool, - sca_issues: &bool, - json: &bool, - page: &Option, - page_size: &Option, - scan_id: &Option, -) { - let project_name = utils::generic::determine_project_name(None); - println!(); - if *sca_issues { +#[derive(Default)] +pub struct ListArgs { + pub issues: bool, + pub sca_issues: bool, + pub json: bool, + pub page: Option, + pub page_size: Option, + pub scan_id: Option, + pub selector: ProjectSelector, +} + +pub fn run(config: &Config, args: ListArgs) { + let ListArgs { + issues, + sca_issues, + json, + page, + page_size, + scan_id, + selector, + } = args; + if !json { + println!(); + } + if sca_issues { + // Only an explicit --project-name/--repo scopes the SCA listing: the + // endpoint takes `project`, but unflagged `--sca-issues` has always + // returned the company-wide latest scan and narrowing that silently is + // not this change's to make. Without a selector the CWD basename stays + // what it was — error copy only. + let resolved = (scan_id.is_none() && selector.is_set()) + .then(|| utils::api::resolve_project_or_exit(&config.get_url(), &selector)); + let project_name = resolved + .as_ref() + .map(|r| r.query_name.clone()) + .unwrap_or_else(|| { + utils::generic::get_current_working_directory().unwrap_or("unknown".to_string()) + }); let sca_issues_response = match utils::api::get_sca_issues( &config.get_url(), - Some((*page).unwrap_or(1)), - *page_size, + Some(page.unwrap_or(1)), + page_size, scan_id.clone(), + resolved.as_ref().map(|r| r.query_name.as_str()), ) { Ok(response) => response, Err(e) => { debug(&format!("Error Sending Request: {}", e)); if e.to_string().contains("404") { - if scan_id.is_some() { - log::error!("Scan with ID '{}' doesn't exist or has no SCA issues. Please run 'corgea scan' to create a new scan for this project.", scan_id.as_ref().unwrap()); + if let Some(id) = &scan_id { + log::error!("Scan with ID '{}' doesn't exist or has no SCA issues. Please run 'corgea scan' to create a new scan for this project.", id); } else { log::error!("No SCA issues found for project '{}'. Please run 'corgea scan' to create a new scan for this project.", project_name); } @@ -44,7 +72,7 @@ pub fn run( } }; - if *json { + if json { let output = serde_json::json!({ "page": sca_issues_response.page, "total_pages": sca_issues_response.total_pages, @@ -107,22 +135,38 @@ pub fn run( Some(sca_issues_response.page), Some(sca_issues_response.total_pages), ); - } else if *issues { + return; + } + + if issues { + // The --scan-id issue route hits /scan/{id}/issues and ignores the + // project, so it needs no resolution. + let resolved = scan_id + .is_none() + .then(|| utils::api::resolve_project_or_exit(&config.get_url(), &selector)); + let project_name = resolved + .as_ref() + .map(|r| r.query_name.clone()) + .unwrap_or_default(); let issues_response = match utils::api::get_scan_issues( &config.get_url(), &project_name, - Some((*page).unwrap_or(1)), - *page_size, + Some(page.unwrap_or(1)), + page_size, scan_id.clone(), ) { Ok(response) => response, Err(e) => { debug(&format!("Error Sending Request: {}", e)); if e.to_string().contains("404") { - if scan_id.is_some() { - log::error!("Scan with ID '{}' doesn't exist. Please run 'corgea scan' to create a new scan for this project.", scan_id.as_ref().unwrap()); - } else { - log::error!("Project with name '{}' doesn't exist. Please run 'corgea scan' to create a new scan for this project.", project_name); + // `resolved` is None exactly on the --scan-id route. + match &resolved { + None => log::error!("Scan with ID '{}' doesn't exist. Please run 'corgea scan' to create a new scan for this project.", scan_id.as_ref().unwrap()), + Some(r) if r.confirmed => log::error!("Project '{}' has no issues yet. Run 'corgea scan' to create a scan for this project.", project_name), + Some(r) => log::error!( + "No Corgea project found for {}. Run 'corgea scan' to create one, or pass --project-name .", + r.tried_label + ), } } else { log::error!( @@ -140,14 +184,10 @@ pub fn run( let mut blocking_rules: std::collections::HashMap = std::collections::HashMap::new(); - if scan_id.is_some() { + if let Some(id) = &scan_id { let mut page: u32 = 1; loop { - match utils::api::check_blocking_rules( - &config.get_url(), - scan_id.as_ref().unwrap(), - Some(page), - ) { + match utils::api::check_blocking_rules(&config.get_url(), id, Some(page)) { Ok(rules) => { if rules.block { render_blocking_rules = true; @@ -170,7 +210,7 @@ pub fn run( } } - if *json { + if json { let mut json = serde_json::json!({ "page": issues_response.page, "total_pages": issues_response.total_pages, @@ -259,26 +299,28 @@ pub fn run( utils::terminal::print_table(table, issues_response.page, issues_response.total_pages); } else { + let resolved = utils::api::resolve_project_or_exit(&config.get_url(), &selector); + let project_name = &resolved.query_name; let (scans, page, total_pages) = match utils::api::query_scan_list( &config.get_url(), - Some(&project_name), - *page, - *page_size, + Some(project_name), + page, + page_size, ) { Ok(scans) => { let page = scans.page; let total_pages = scans.total_pages; - let filtered_scans: Vec = scans - .scans - .unwrap_or_default() - .into_iter() - .filter(|scan| scan.project == project_name) - .collect(); - (filtered_scans, page, total_pages) + // The server already filtered by the resolved project; the old + // client-side `scan.project == cwd_basename` pass would discard + // every repo-resolved scan. (COR-1577) + (scans.scans.unwrap_or_default(), page, total_pages) } Err(e) => { if e.to_string().contains("404") { - log::error!("Project with name '{}' doesn't exist. Please run 'corgea scan' to create a new scan for this project.", project_name); + log::error!( + "No Corgea project found for {}. Run 'corgea scan' to create one, or pass --project-name .", + resolved.tried_label + ); } else { log::error!( "Unable to fetch scans. Please check your connection and ensure that:\n\ @@ -290,13 +332,36 @@ pub fn run( std::process::exit(1); } }; - if *json { + if json { let output = json!({ "page": page, "total_pages": total_pages, "results": scans }); + // The envelope prints first so JSON consumers get valid stdout + // even when the miss below exits 1. println!("{}", serde_json::to_string_pretty(&output).unwrap()); + } + // An unresolved project is a miss (exit 1, as --issues and `wait`); a + // confirmed project with no scans is a valid empty result. So is an + // explicit --project-name: /scans answers 200-empty either way, so the + // caller's own exact name is the better authority than a guess that it + // does not exist. + if scans.is_empty() && !resolved.confirmed && selector.name.is_none() { + log::error!( + "No Corgea project found for {}. Run 'corgea scan' to create one, or pass --project-name .", + resolved.tried_label + ); + std::process::exit(1); + } + if json { + return; + } + if scans.is_empty() { + println!( + "Project '{}' has no scans yet. Run 'corgea scan' to create one.", + project_name + ); return; } let mut table = vec![vec![ @@ -321,6 +386,7 @@ pub fn run( } else { formatted_repo }; + table.push(vec![ scan.id.clone(), scan.project.clone(), diff --git a/src/main.rs b/src/main.rs index 9618721..96d2c46 100644 --- a/src/main.rs +++ b/src/main.rs @@ -141,7 +141,27 @@ enum Commands { project_name: Option, }, /// Wait for the latest in progress scan - Wait { scan_id: Option }, + Wait { + scan_id: Option, + #[arg( + long, + conflicts_with = "repo", + help = "Query this exact Corgea project name directly (skips repo auto-resolution)." + )] + project_name: Option, + #[arg( + long, + help = "Resolve the project from this repo (org/repo slug or remote URL) instead of the git remote." + )] + repo: Option, + #[arg( + long, + requires = "scan_id", + value_parser = clap::builder::NonEmptyStringValueParser::new(), + help = "Use this known Corgea project id for the result link, skipping project resolution. Requires a scan id, which is what the id then belongs to." + )] + project_id: Option, + }, /// List something, by default it lists the scans #[command(alias = "ls")] List { @@ -166,6 +186,19 @@ enum Commands { #[arg(long, value_parser = clap::value_parser!(u16), help = "Number of items per page")] page_size: Option, + + #[arg( + long, + conflicts_with = "repo", + help = "Query this exact Corgea project name directly (skips repo auto-resolution)." + )] + project_name: Option, + + #[arg( + long, + help = "Resolve the project from this repo (org/repo slug or remote URL) instead of the git remote." + )] + repo: Option, }, /// Inspect something, by default it will inspect a scan Inspect { @@ -663,9 +696,24 @@ fn main() { ), } } - Some(Commands::Wait { scan_id }) => { + Some(Commands::Wait { + scan_id, + project_name, + repo, + project_id, + }) => { verify_token_and_exit_when_fail(&corgea_config); - wait::run(&corgea_config, scan_id.clone(), None); + wait::run( + &corgea_config, + wait::WaitArgs { + scan_id: scan_id.clone(), + selector: utils::api::ProjectSelector { + name: project_name.clone(), + repo: repo.clone(), + }, + project_id: project_id.clone(), + }, + ); } Some(Commands::List { issues, @@ -674,6 +722,8 @@ fn main() { page_size, scan_id, sca_issues, + project_name, + repo, }) => { verify_token_and_exit_when_fail(&corgea_config); if *issues && *sca_issues { @@ -686,12 +736,18 @@ fn main() { } list::run( &corgea_config, - issues, - sca_issues, - json, - page, - page_size, - scan_id, + list::ListArgs { + issues: *issues, + sca_issues: *sca_issues, + json: *json, + page: *page, + page_size: *page_size, + scan_id: scan_id.clone(), + selector: utils::api::ProjectSelector { + name: project_name.clone(), + repo: repo.clone(), + }, + }, ); } Some(Commands::Inspect { diff --git a/src/scan.rs b/src/scan.rs index af1fd4a..cd0af92 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -65,8 +65,18 @@ pub fn run_semgrep(config: &Config, project_name: Option) { let output = run_command(&base_command.to_string(), command); - if let Some(result) = parse_scan(config, output, true, project_name) { - crate::wait::run(config, Some(result.scan_id), result.project_id); + if let Some(result) = parse_scan(config, output, true, project_name.clone()) { + crate::wait::run( + config, + crate::wait::WaitArgs { + scan_id: Some(result.scan_id), + selector: crate::utils::api::ProjectSelector { + name: project_name, + ..Default::default() + }, + project_id: result.project_id, + }, + ); } } @@ -80,8 +90,18 @@ pub fn run_snyk(config: &Config, project_name: Option) { let output = run_command(&base_command.to_string(), command); - if let Some(result) = parse_scan(config, output, true, project_name) { - crate::wait::run(config, Some(result.scan_id), result.project_id); + if let Some(result) = parse_scan(config, output, true, project_name.clone()) { + crate::wait::run( + config, + crate::wait::WaitArgs { + scan_id: Some(result.scan_id), + selector: crate::utils::api::ProjectSelector { + name: project_name, + ..Default::default() + }, + project_id: result.project_id, + }, + ); } } diff --git a/src/utils/api.rs b/src/utils/api.rs index 2356b6a..a6ea454 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -471,27 +471,29 @@ pub fn get_scan_issues( page_size: Option, scan_id: Option, ) -> Result> { - let mut seperator = "?"; - let mut url = match scan_id { - Some(scan_id) => format!("{}{}/scan/{}/issues", url, API_BASE, scan_id), - None => { - seperator = "&"; - format!("{}{}/issues?project={}", url, API_BASE, project) - } + // Built with `query`, not `format!`: a project name is user- and + // server-supplied, so an `&`/`#`/`?` in it would otherwise split the query + // and address a different project. + let (url, mut query_params) = match scan_id { + Some(scan_id) => ( + format!("{}{}/scan/{}/issues", url, API_BASE, scan_id), + vec![], + ), + None => ( + format!("{}{}/issues", url, API_BASE), + vec![("project", project.to_string())], + ), }; if let Some(p) = page { - url.push_str(&format!("{}page={}", seperator, p)); - } - if let Some(p_size) = page_size { - url.push_str(&format!("&page_size={}", p_size)); - } else { - url.push_str("&page_size=30"); + query_params.push(("page", p.to_string())); } + query_params.push(("page_size", page_size.unwrap_or(30).to_string())); let client = http_client(); debug(&format!("Sending request to URL: {}", url)); + debug(&format!("Query params: {:?}", query_params)); - let response = match client.get(&url).send() { + let response = match client.get(&url).query(&query_params).send() { Ok(res) => { check_for_warnings(res.headers(), res.status()); res @@ -733,6 +735,319 @@ pub fn query_scan_list( } } +#[derive(Deserialize, Debug)] +pub struct ProjectSummary { + // Tolerant: an unexpected string id (or none) must not fail the parse. + #[serde(default)] + pub id: serde_json::Value, + pub name: String, + #[serde(default)] + pub repo_url: Option, +} + +#[derive(Deserialize, Debug)] +pub struct ProjectsResponse { + /// Required: doghouse's `@paginated` emits this key on every 200, empty + /// array included (`api/decorators.py:228,241,259`). A 200 without it is + /// not this endpoint answering, so it must fail the parse rather than read + /// as "no matches" and take the legacy-name fallback. + pub projects: Vec, + /// Absent on a backend that does not paginate -> treated as a single page. + #[serde(default)] + pub total_pages: Option, +} + +/// The backend's `@paginated` `max_page_size` for /projects; a larger value is +/// clamped server-side. +const PROJECTS_PAGE_SIZE: u16 = 50; + +/// Ceiling on pages walked looking for an exact repo match, so a bogus +/// server-reported `total_pages` cannot drive an unbounded request loop. +const PROJECTS_MAX_PAGES: u32 = 20; + +/// Stringify a scalar JSON id; None otherwise, so no `/project/null/` URL. +fn id_to_string(v: &serde_json::Value) -> Option { + match v { + serde_json::Value::String(s) => Some(s.clone()), + serde_json::Value::Number(n) => Some(n.to_string()), + _ => None, + } +} + +/// True when a stored `repo_url` points at exactly `expected_path` (a whole +/// post-host path, already lowercased). Comparing whole paths keeps the +/// backend's `repo_url__icontains` results honest: neither the sibling +/// `acme/api-v2` nor the nested `…/mirrors/acme/api` passes for `acme/api`. +/// Falls back to a normalized compare for a stored bare `acme/api`. +fn repo_url_matches_path(repo_url: &str, expected_path: &str) -> bool { + if let Some(path) = utils::generic::extract_repo_path(repo_url) { + return path == expected_path; + } + let r = repo_url.trim().trim_end_matches('/'); + let r = r.strip_suffix(".git").unwrap_or(r).to_lowercase(); + r == expected_path +} + +/// True when a candidate is stored on exactly `expected_host`. +fn repo_url_on_host(repo_url: &str, expected_host: &str) -> bool { + utils::generic::extract_repo_host(repo_url).as_deref() == Some(expected_host) +} + +/// One page of GET /api/v1/projects?repo_url=… +/// +/// `Ok(None)` only for a 404 (a backend without the endpoint). A 5xx or a body +/// that does not parse is an `Err`: both are hard failures, and treating them +/// as a clean miss would silently fall back to the CWD-name path. +fn fetch_projects_page( + url: &str, + repo_path: &str, + page: u32, +) -> Result, Box> { + let request_url = format!("{}{}/projects", url, API_BASE); + let client = http_client(); + debug(&format!( + "Resolving project via {} (repo_url={}, page={})", + request_url, repo_path, page + )); + let (page, page_size) = (page.to_string(), PROJECTS_PAGE_SIZE.to_string()); + let response = client + .get(&request_url) + .query(&[ + ("repo_url", repo_path), + ("page", &page), + ("page_size", &page_size), + ]) + .send()?; + check_for_warnings(response.headers(), response.status()); + let status = response.status(); + if status == reqwest::StatusCode::NOT_FOUND { + return Ok(None); + } + if !status.is_success() { + return Err(format!("/projects request failed: HTTP {}", status).into()); + } + let text = response.text()?; + match serde_json::from_str(&text) { + Ok(parsed) => Ok(Some(parsed)), + Err(e) => { + debug(&format!("/projects response body: {}", text)); + Err(format!("Failed to parse the /projects response: {}", e).into()) + } + } +} + +/// Resolve the canonical project for a repo path via GET /api/v1/projects?repo_url=… +/// +/// The backend filters `repo_url__icontains` over a paginated list, so the +/// exact repo can sit behind a page of siblings (`acme/api-v2`, …) — pages are +/// walked until it turns up or they run out. +/// +/// Old-backend safety guard: a pre-COR-1426 backend ignores the unknown +/// `repo_url` param and returns ALL company projects, so every candidate is +/// re-checked against the path here — on such a backend none match, and the +/// caller falls back to the CWD-name path. +/// +/// The host is a tie-breaker, not a gate: it settles which of several +/// same-path candidates is ours (`github.com/acme/api` is not +/// `gitlab.com/acme/api`), but a lone path match is accepted whatever its +/// host — an SSH-config alias origin (`corp-github:acme/api`) never matches +/// the stored `github.com` and must still resolve. Several path matches with +/// none on our host is genuinely ambiguous and errors rather than guessing. +/// +/// `Err` for hard failures (network/auth/5xx/unparseable body/ambiguity); a +/// clean "no match" (or a 404 from a backend without the endpoint) is a soft +/// `Ok(None)`. +pub fn resolve_project_by_repo( + url: &str, + repo_path: &str, + expected_host: Option<&str>, +) -> Result, Box> { + let expected = repo_path.to_lowercase(); + let mut candidates: Vec = Vec::new(); + let mut page = 1; + loop { + let Some(parsed) = fetch_projects_page(url, repo_path, page)? else { + return Ok(None); + }; + let total_pages = parsed.total_pages.unwrap_or(1); + let projects = parsed.projects; + if projects.is_empty() { + break; + } + for project in projects { + let Some(repo_url) = project.repo_url.as_deref() else { + continue; + }; + if !repo_url_matches_path(repo_url, &expected) { + continue; + } + // Our own host settles it; no need to read further pages. + if expected_host.is_some_and(|h| repo_url_on_host(repo_url, h)) { + return Ok(Some(project)); + } + candidates.push(project); + } + if page >= total_pages { + break; + } + // Every reported page was NOT searched, so this is not a clean miss: + // saying so would send the caller to the legacy-name fallback, which + // can list a different same-basename project and exit 0. + if page >= PROJECTS_MAX_PAGES { + return Err(format!( + "/projects reported {} pages; refusing to guess after searching {}", + total_pages, PROJECTS_MAX_PAGES + ) + .into()); + } + page += 1; + } + + if candidates.len() > 1 { + let hosts: Vec<&str> = candidates + .iter() + .filter_map(|p| p.repo_url.as_deref()) + .collect(); + return Err(format!( + "{} Corgea projects claim repo '{}' ({}); pass --project-name to choose one", + candidates.len(), + repo_path, + hosts.join(", ") + ) + .into()); + } + Ok(candidates.pop()) +} + +/// What `list`/`wait` need to drive the existing name-based queries. +#[derive(Debug)] +pub struct ResolvedProject { + /// Sent as `?project=` to the listing endpoints. + pub query_name: String, + /// True only when /projects confirmed a backend project. + pub confirmed: bool, + pub project_id: Option, + /// Pre-formatted subject of the miss message ("repo 'org/repo'", …). + pub tried_label: String, +} + +/// How the caller asked for a project: `--project-name` and `--repo` are +/// mutually exclusive at the CLI, but both travel together to the resolver. +/// One struct so the two same-typed options cannot be transposed at a call +/// site. +#[derive(Default, Clone)] +pub struct ProjectSelector { + pub name: Option, + pub repo: Option, +} + +impl ProjectSelector { + /// True when the caller named a project or repo explicitly, rather than + /// leaving it to auto-detection. + pub fn is_set(&self) -> bool { + self.name.is_some() || self.repo.is_some() + } +} + +/// Resolve which project `list`/`wait` should query: `--project-name` verbatim, +/// else the repo path from `--repo` or the discovered remote. Unconfirmed, an +/// explicit `--repo` queries that path as a name and everything else falls back +/// to the CWD basename (today's behavior). `Err` only for a hard resolver +/// failure (network/auth/5xx from /projects). +pub fn resolve_project( + url: &str, + selector: &ProjectSelector, +) -> Result> { + let repo_override = selector.repo.as_deref(); + if let Some(name) = selector.name.as_deref() { + // Normalize before it reaches `?project=`, which the backend matches + // exactly: `--project-name foo/` must not miss the project `foo`. + let name = name.trim().trim_matches('/'); + if name.is_empty() { + return Err("--project-name must name a project".into()); + } + return Ok(ResolvedProject { + query_name: name.to_string(), + confirmed: false, + project_id: None, + tried_label: format!("project '{}'", name), + }); + } + + // --repo may be a bare path (`org/repo`, or a GitLab `group/subgroup/repo`) + // rather than a URL; `extract_repo_path` returns None for those, so the + // whole value is the path and there is no host to hold candidates to. + let (repo_path, repo_host) = match repo_override { + Some(r) => match utils::generic::extract_repo_path(r) { + Some(path) => (Some(path), utils::generic::extract_repo_host(r)), + None => (Some(utils::generic::strip_git_suffix(r).to_string()), None), + }, + None => match utils::generic::discover_repo_url() { + Some(u) => ( + utils::generic::extract_repo_path(&u), + utils::generic::extract_repo_host(&u), + ), + None => (None, None), + }, + }; + + if let Some(repo_path) = repo_path { + if let Some(project) = resolve_project_by_repo(url, &repo_path, repo_host.as_deref())? { + return Ok(ResolvedProject { + query_name: project.name, + confirmed: true, + project_id: id_to_string(&project.id), + tried_label: format!("repo '{}'", repo_path), + }); + } + // Unconfirmed: an explicit --repo queries that path as a name, and an + // auto-detected remote queries exactly what the pre-COR-1577 CLI did + // (repo basename at the worktree root, else the CWD name) so an old or + // not-yet-onboarded backend still resolves. + let query_name = if repo_override.is_some() { + repo_path.clone() + } else { + utils::generic::determine_project_name(None) + }; + return Ok(ResolvedProject { + query_name, + confirmed: false, + project_id: None, + tried_label: format!("repo '{}'", repo_path), + }); + } + + let cwd = + utils::generic::get_current_working_directory().unwrap_or_else(|| "unknown".to_string()); + Ok(ResolvedProject { + tried_label: format!("directory '{}'", cwd), + // Same legacy query as above: the sanitized basename here, since a + // directory named `my app` was onboarded as `my_app`. + query_name: utils::generic::determine_project_name(None), + confirmed: false, + project_id: None, + }) +} + +/// `resolve_project`, or a hard exit with the shared failure copy. Every +/// caller treats a resolver error as fatal. +pub fn resolve_project_or_exit(url: &str, selector: &ProjectSelector) -> ResolvedProject { + match resolve_project(url, selector) { + Ok(resolved) => resolved, + Err(e) => { + log::error!( + "Unable to resolve the Corgea project. 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); + } + } +} + pub fn exchange_code_for_token( base_url: &str, code: &str, @@ -842,6 +1157,7 @@ pub fn get_sca_issues( page: Option, page_size: Option, scan_id: Option, + project: Option<&str>, ) -> Result> { let client = http_client(); let mut query_params = vec![]; @@ -851,6 +1167,11 @@ pub fn get_sca_issues( if let Some(page_size) = page_size { query_params.push(("page_size", page_size.to_string())); } + // Scopes the scan-less route to one project (doghouse `list_sca_issues` + // reads `project`); the scan route already keys off the scan. + if let Some(project) = project { + query_params.push(("project", project.to_string())); + } let endpoint = if let Some(scan_id) = scan_id { format!("{}{}/scan/{}/issues/sca", url, API_BASE, scan_id) @@ -914,11 +1235,18 @@ pub fn get_all_sca_issues( let mut current_page: u32 = 1; loop { - let response = - match get_sca_issues(url, Some(current_page as u16), Some(30), scan_id.clone()) { - Ok(response) => response, - Err(e) => return Err(format!("Failed to get SCA issues: {}", e).into()), - }; + // No project scope: every caller passes a scan id, which selects the + // scan on its own. + let response = match get_sca_issues( + url, + Some(current_page as u16), + Some(30), + scan_id.clone(), + None, + ) { + Ok(response) => response, + Err(e) => return Err(format!("Failed to get SCA issues: {}", e).into()), + }; if response.issues.is_empty() { break; @@ -1339,4 +1667,276 @@ mod tests { assert!(result.is_err()); assert_eq!(attempts.get(), RETRY_BACKOFF_SECS.len() + 1); } + + // Single-response-per-connection JSON stub on an ephemeral port; returns base URL. + fn spawn_projects_stub(body: &'static str) -> String { + spawn_projects_stub_status("200 OK", body) + } + + // As `spawn_projects_stub` but with a caller-chosen status line, for the + // resolver error-path tests (404 soft-miss vs 5xx hard error). + fn spawn_projects_stub_status(status_line: &'static str, body: &'static str) -> String { + use std::io::Write; + let listener = TcpListener::bind("127.0.0.1:0").expect("bind stub"); + let base = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); + thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + // Drain the request headers before responding. Closing the + // socket with an unread request still in the kernel buffer + // triggers a TCP RST that surfaces on the client as hyper + // `UnexpectedMessage` (flaky, timing-dependent). + let _ = corgea::vuln_api_stub::read_http_request(&mut stream); + let resp = corgea::vuln_api_stub::http_response(status_line, "", body); + let _ = stream.write_all(resp.as_bytes()); + } + }); + base + } + + // Serves `page_one` until a request carries `page=2`, then `page_two` — + // for the pagination walk. + fn spawn_paged_projects_stub(page_one: &'static str, page_two: &'static str) -> String { + use std::io::Write; + let listener = TcpListener::bind("127.0.0.1:0").expect("bind stub"); + let base = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); + 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 body = if String::from_utf8_lossy(&request).contains("page=2") { + page_two + } else { + page_one + }; + let resp = corgea::vuln_api_stub::http_response("200 OK", "", body); + let _ = stream.write_all(resp.as_bytes()); + } + }); + base + } + + #[test] + fn resolve_project_by_repo_keeps_only_repo_url_matches() { + // New backend: filter applied, one matching project returned. + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"id":7,"name":"bohappdev/dotnet-azure-web-tsb","repo_url":"https://github.com/bohappdev/dotnet-azure-web-tsb"}]}"#, + ); + let got = resolve_project_by_repo(&base, "bohappdev/dotnet-azure-web-tsb", None).unwrap(); + assert_eq!( + got.map(|p| p.name).as_deref(), + Some("bohappdev/dotnet-azure-web-tsb") + ); + } + + #[test] + fn resolve_project_by_repo_guards_against_old_backend_returning_all() { + // Old backend ignores ?repo_url and returns unrelated projects -> guard -> None. + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"id":1,"name":"other/repo","repo_url":"https://github.com/other/repo"},{"id":2,"name":"misc/thing","repo_url":"https://github.com/misc/thing"}]}"#, + ); + let got = resolve_project_by_repo(&base, "bohappdev/dotnet-azure-web-tsb", None).unwrap(); + assert!(got.is_none(), "non-matching projects must be discarded"); + } + + #[test] + fn resolve_project_by_repo_empty_projects_is_none() { + let base = spawn_projects_stub(r#"{"status":"ok","projects":[]}"#); + assert!(resolve_project_by_repo(&base, "org/repo", None) + .unwrap() + .is_none()); + } + + #[test] + fn repo_url_matches_path_compares_the_whole_path() { + // Same repo across scheme/.git/trailing-slash/port variants. + assert!(repo_url_matches_path( + "https://github.com/acme/api", + "acme/api" + )); + assert!(repo_url_matches_path( + "https://github.com/acme/api.git", + "acme/api" + )); + assert!(repo_url_matches_path("git@github.com:acme/api", "acme/api")); + assert!(repo_url_matches_path("acme/api", "acme/api")); + // Sibling / prefix repo, and a different org. + assert!(!repo_url_matches_path( + "https://github.com/acme/api-v2", + "acme/api" + )); + assert!(!repo_url_matches_path( + "https://github.com/notacme/api", + "acme/api" + )); + // The owner must be top-level on the host: a nested mirror is a + // different repository, as is `org/team/repo` for `team/repo`. + assert!(!repo_url_matches_path( + "https://github.com/mirrors/acme/api", + "acme/api" + )); + assert!(!repo_url_matches_path( + "https://github.com/org/team/repo", + "team/repo" + )); + // Multi-segment paths compare in full. + assert!(repo_url_matches_path( + "https://dev.azure.com/org/project/_git/repo", + "org/project/_git/repo" + )); + assert!(repo_url_matches_path( + "git@gitlab.com:group/subgroup/repo.git", + "group/subgroup/repo" + )); + } + + #[test] + fn repo_url_on_host_reads_through_url_forms() { + assert!(repo_url_on_host( + "https://github.com/acme/api", + "github.com" + )); + assert!(repo_url_on_host( + "git@github.com:acme/api.git", + "github.com" + )); + assert!(repo_url_on_host( + "https://gitlab.acme.com:8443/acme/api", + "gitlab.acme.com" + )); + assert!(!repo_url_on_host( + "https://gitlab.com/acme/api", + "github.com" + )); + // A stored bare `acme/api` is on no host at all. + assert!(!repo_url_on_host("acme/api", "github.com")); + } + + #[test] + fn resolve_project_by_repo_rejects_sibling_prefix_repo() { + // Backend `repo_url__icontains` returns the sibling `acme/api-v2` for + // `acme/api`; the path guard must reject it rather than confirming the + // wrong project (COR-1577 review). + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"id":9,"name":"acme/api-v2","repo_url":"https://github.com/acme/api-v2"}]}"#, + ); + let got = resolve_project_by_repo(&base, "acme/api", None).unwrap(); + assert!(got.is_none(), "a prefix sibling repo must not be confirmed"); + } + + #[test] + fn resolve_project_by_repo_404_is_soft_none() { + // /projects absent on a very old backend -> soft miss (Ok(None)). + let base = spawn_projects_stub_status("404 Not Found", r#"{"message":"not found"}"#); + assert!(resolve_project_by_repo(&base, "org/repo", None) + .unwrap() + .is_none()); + } + + #[test] + fn resolve_project_by_repo_server_error_is_hard_err() { + // 5xx must surface, not silently fall back to the local-dir project. + let base = spawn_projects_stub_status("500 Internal Server Error", r#"{"error":"boom"}"#); + assert!(resolve_project_by_repo(&base, "org/repo", None).is_err()); + } + + #[test] + fn resolve_project_by_repo_unparseable_body_is_hard_err() { + // A 200 carrying HTML (proxy / captive portal) or an incompatible + // schema is a hard failure too: silently reporting "no match" would + // resolve a DIFFERENT project via the CWD-name fallback (COR-1577 + // review). + let base = spawn_projects_stub("Access denied"); + assert!(resolve_project_by_repo(&base, "org/repo", None).is_err()); + } + + #[test] + fn resolve_project_by_repo_walks_past_the_first_page() { + // The backend filters `repo_url__icontains` over a 20-per-page list, so + // enough `acme/api-*` siblings push the exact `acme/api` onto page 2. + // Stopping at page 1 would recreate the very miss this resolves. + let base = spawn_paged_projects_stub( + r#"{"status":"ok","total_pages":2,"projects":[{"id":1,"name":"acme/api-v2","repo_url":"https://github.com/acme/api-v2"},{"id":2,"name":"acme/api-gw","repo_url":"https://github.com/acme/api-gw"}]}"#, + r#"{"status":"ok","total_pages":2,"projects":[{"id":9,"name":"acme/api","repo_url":"https://github.com/acme/api"}]}"#, + ); + let got = resolve_project_by_repo(&base, "acme/api", None).unwrap(); + assert_eq!(got.map(|p| p.name).as_deref(), Some("acme/api")); + } + + #[test] + fn resolve_project_by_repo_truncated_search_is_hard_err() { + // The ceiling stops the walk before every reported page was searched, + // so "no match" would be a guess — and the caller acts on it by + // querying the legacy name, which can list a different project. + let base = spawn_projects_stub( + r#"{"status":"ok","total_pages":999,"projects":[{"id":1,"name":"acme/api-v2","repo_url":"https://github.com/acme/api-v2"}]}"#, + ); + let err = resolve_project_by_repo(&base, "acme/api", None).unwrap_err(); + assert!( + err.to_string().contains("999 pages"), + "error should name the reported page count: {err}" + ); + } + + #[test] + fn resolve_project_by_repo_missing_projects_key_is_hard_err() { + // `@paginated` always emits `projects`, empty array included, so a 200 + // without it is an error envelope or a foreign responder. Reading it as + // "no matches" would take the legacy-name fallback and quietly query a + // different project (PR #122 review). + let base = spawn_projects_stub(r#"{"status":"error","message":"boom"}"#); + assert!(resolve_project_by_repo(&base, "org/repo", None).is_err()); + } + + #[test] + fn resolve_project_by_repo_picks_the_candidate_on_the_origin_host() { + // `?repo_url=acme/api` is hostless, so `icontains` returns both forges; + // the origin host is what says which one is ours. + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"id":3,"name":"gl","repo_url":"https://gitlab.com/acme/api"},{"id":4,"name":"gh","repo_url":"https://github.com/acme/api"}]}"#, + ); + let got = resolve_project_by_repo(&base, "acme/api", Some("github.com")).unwrap(); + assert_eq!(got.map(|p| p.name).as_deref(), Some("gh")); + let got = resolve_project_by_repo(&base, "acme/api", Some("gitlab.com")).unwrap(); + assert_eq!(got.map(|p| p.name).as_deref(), Some("gl")); + } + + #[test] + fn resolve_project_by_repo_accepts_a_lone_match_from_an_unknown_host() { + // An SSH-config alias origin (`corp-github:acme/api`) never equals the + // stored `github.com`, but there is nothing to disambiguate — holding + // out for a host match would leave COR-1577 unfixed for exactly the + // alias remotes this repo itself uses (PR #122 review). + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"id":4,"name":"acme/api","repo_url":"https://github.com/acme/api"}]}"#, + ); + let got = resolve_project_by_repo(&base, "acme/api", Some("corp-github")).unwrap(); + assert_eq!(got.map(|p| p.name).as_deref(), Some("acme/api")); + } + + #[test] + fn resolve_project_by_repo_errors_when_the_path_is_claimed_twice() { + // Two forges, neither ours: picking either would be a coin flip, and + // reporting no match would quietly list a third project's scans. + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"id":3,"name":"gl","repo_url":"https://gitlab.com/acme/api"},{"id":4,"name":"gh","repo_url":"https://github.com/acme/api"}]}"#, + ); + let err = resolve_project_by_repo(&base, "acme/api", Some("corp-github")).unwrap_err(); + assert!( + err.to_string().contains("--project-name"), + "the error should say how to disambiguate: {err}" + ); + } + + #[test] + fn resolve_project_by_repo_stops_at_the_last_page() { + // `total_pages: 1` (and its absence, as in the other stubs here) must + // end the walk rather than request page 2 forever. + let base = spawn_projects_stub( + r#"{"status":"ok","total_pages":1,"projects":[{"id":1,"name":"acme/api-v2","repo_url":"https://github.com/acme/api-v2"}]}"#, + ); + assert!(resolve_project_by_repo(&base, "acme/api", None) + .unwrap() + .is_none()); + } } diff --git a/src/utils/generic.rs b/src/utils/generic.rs index c316d17..04c11a9 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -260,6 +260,68 @@ fn extract_repo_name_from_url(url: &str) -> Option { None } +/// The whole repository path after the host, lowercased (`org/repo`, +/// `group/subgroup/repo`, Azure `org/project/_git/repo`). The host is excluded +/// so SSH/HTTPS/port variants of one remote compare equal; None when fewer than +/// two path segments follow it. The full path — not a trailing `org/repo` slug +/// — is what doghouse `normalize_repo_url` stores (`heeler/models.py:201-246`), +/// so it is what an equality compare needs. Azure SSH remotes +/// (`ssh.dev.azure.com/v3/org/…`, no `_git` segment) remain a known limitation. +pub fn extract_repo_path(url: &str) -> Option { + Some(split_remote(url)?[1..].join("/").to_lowercase()) +} + +/// The host of a git remote (`github.com`), lowercased and without userinfo or +/// port. None for a hostless value such as a bare `org/repo` — the same inputs +/// `extract_repo_path` rejects. +pub fn extract_repo_host(url: &str) -> Option { + Some(split_remote(url)?[0].to_lowercase()) +} + +/// Split a git remote into `[host, path segments…]`, dropping scheme, userinfo +/// and port. None when fewer than two path segments follow the host, or when +/// the value carries no host at all. +fn split_remote(url: &str) -> Option> { + let url = strip_git_suffix(url); + let (had_scheme, url) = match url.split_once("://") { + Some((_, rest)) => (true, rest), + None => (false, url), + }; + // Drop userinfo (`git@`, `oauth2:token@`) so it is never read as the host. + let host_end = url.find('/').unwrap_or(url.len()); + let (had_userinfo, url) = match url[..host_end].rfind('@') { + Some(at) => (true, &url[at + 1..]), + None => (false, url), + }; + // A colon before the first '/' is the scp-style `host:org/repo` separator. + let first_slash = url.find('/').unwrap_or(url.len()); + let had_scp_colon = url[..first_slash].contains(':'); + // URL forms split host from path on '/', scp-like `git@host:org/repo` on ':'. + let mut segments: Vec<&str> = url.split(['/', ':']).filter(|s| !s.is_empty()).collect(); + // segments[0] is the host; an all-digit segment right after it is a port. + if segments.len() >= 4 && segments[1].chars().all(|c| c.is_ascii_digit()) { + segments.remove(1); + } + // Need host + at least org + repo. + if segments.len() < 3 { + return None; + } + // A scheme, userinfo or scp colon is what marks a network remote. Without + // one this is a bare path — a GitLab `group/subgroup/repo`, whose namespace + // may itself contain dots — so every segment belongs to the path and there + // is no host to take. + if !had_scheme && !had_userinfo && !had_scp_colon { + return None; + } + Some(segments) +} + +/// Trim surrounding space, a trailing `/`, and a `.git` suffix. +pub fn strip_git_suffix(url: &str) -> &str { + let url = url.trim().trim_end_matches('/'); + url.strip_suffix(".git").unwrap_or(url) +} + pub fn get_env_var_if_exists(var_name: &str) -> Option { match env::var(var_name) { Ok(value) if !value.trim().is_empty() => Some(value), @@ -297,18 +359,28 @@ pub fn get_repo_info(dir: &str) -> Result, git2::Error> { .map(|commit| commit.id().to_string()) }); - let repo_url = repo - .find_remote("origin") - .ok() - .and_then(|remote| remote.url().map(|url| url.to_string())); - Ok(Some(RepoInfo { branch, - repo_url, + repo_url: origin_url(&repo), sha, })) } +/// `origin`'s URL, or None when the remote is missing or carries no URL. +fn origin_url(repo: &Repository) -> Option { + repo.find_remote("origin") + .ok() + .and_then(|remote| remote.url().map(|url| url.to_string())) +} + +/// The enclosing repository's `origin` remote URL, searched upward from the +/// current directory so `corgea list`/`wait` also resolve from a subdirectory. +/// `get_repo_info` deliberately returns None outside the worktree root; this +/// does not. None outside a git repo or when `origin` carries no URL. +pub fn discover_repo_url() -> Option { + origin_url(&Repository::discover(Path::new(".")).ok()?) +} + /// True when `dir` is the repository worktree root (not a subdirectory). fn is_at_repo_root(dir: &str) -> bool { let Ok(repo) = Repository::discover(Path::new(dir)) else { @@ -354,41 +426,34 @@ mod tests { use std::fs; use std::process::Command; - #[test] - fn get_repo_info_at_root_only_not_nested_cwd() { - let dir = tempfile::tempdir().unwrap(); - let root = dir.path(); - assert!(Command::new("git") - .args(["init"]) - .current_dir(root) - .status() - .unwrap() - .success()); + /// `git ` in `dir`. Scrubs the GIT_* env a parent git process + /// injects (e.g. when these tests run from a pre-commit hook): an + /// inherited GIT_DIR would point `git init` at the developer's repo + /// instead of `dir`. Same scrub as `tests/cli_deps.rs::run_git`. + fn git(dir: &Path, args: &[&str]) { assert!(Command::new("git") - .args(["config", "user.email", "test@example.com"]) - .current_dir(root) - .status() - .unwrap() - .success()); - assert!(Command::new("git") - .args(["config", "user.name", "Test"]) - .current_dir(root) + .args(args) + .current_dir(dir) + .env_remove("GIT_DIR") + .env_remove("GIT_WORK_TREE") + .env_remove("GIT_COMMON_DIR") + .env_remove("GIT_INDEX_FILE") + .env_remove("GIT_PREFIX") .status() .unwrap() .success()); + } + + #[test] + fn get_repo_info_at_root_only_not_nested_cwd() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + git(root, &["init"]); + git(root, &["config", "user.email", "test@example.com"]); + git(root, &["config", "user.name", "Test"]); fs::write(root.join("README"), "hi").unwrap(); - assert!(Command::new("git") - .args(["add", "README"]) - .current_dir(root) - .status() - .unwrap() - .success()); - assert!(Command::new("git") - .args(["commit", "-m", "init"]) - .current_dir(root) - .status() - .unwrap() - .success()); + git(root, &["add", "README"]); + git(root, &["commit", "-m", "init"]); let root_s = root.to_str().unwrap(); let nested = root.join("pkg").join("inner"); @@ -450,4 +515,117 @@ mod tests { added ); } + + #[test] + fn extract_repo_path_handles_common_remote_forms() { + assert_eq!( + extract_repo_path("https://github.com/org/repo.git").as_deref(), + Some("org/repo") + ); + assert_eq!( + extract_repo_path("https://github.com/org/repo").as_deref(), + Some("org/repo") + ); + assert_eq!( + extract_repo_path("git@github.com:org/repo.git").as_deref(), + Some("org/repo") + ); + assert_eq!( + extract_repo_path("git@github.com:org/repo").as_deref(), + Some("org/repo") + ); + assert_eq!( + extract_repo_path("ssh://git@github.com/org/repo").as_deref(), + Some("org/repo") + ); + assert_eq!( + extract_repo_path("https://github.com/org/repo/").as_deref(), + Some("org/repo") + ); + assert_eq!( + extract_repo_path("https://github.com/Org/Repo").as_deref(), + Some("org/repo") + ); + // host:port must not leak the port into the path + assert_eq!( + extract_repo_path("https://git.example.com:8443/org/repo").as_deref(), + Some("org/repo") + ); + // token userinfo must not be read as the host + assert_eq!( + extract_repo_path("https://oauth2:tok@gitlab.com/org/repo").as_deref(), + Some("org/repo") + ); + // Bank of Hope case + assert_eq!( + extract_repo_path("git@github.com:bohappdev/dotnet-azure-web-tsb.git").as_deref(), + Some("bohappdev/dotnet-azure-web-tsb") + ); + // Whole path is kept: Azure `_git` and GitLab subgroups + assert_eq!( + extract_repo_path("https://dev.azure.com/org/project/_git/repo").as_deref(), + Some("org/project/_git/repo") + ); + assert_eq!( + extract_repo_path("git@gitlab.com:group/subgroup/repo.git").as_deref(), + Some("group/subgroup/repo") + ); + } + + #[test] + fn extract_repo_path_returns_none_for_unsplittable_input() { + assert_eq!(extract_repo_path("not a url"), None); + assert_eq!(extract_repo_path(""), None); + assert_eq!(extract_repo_path("github.com"), None); // host only + } + + #[test] + fn extract_repo_path_leaves_unmarked_bare_paths_alone() { + // Nothing marks these as a network remote, so they are paths in full — + // the caller keeps the value verbatim. A GitLab namespace may contain + // dots, so a dotted leading segment is no evidence of a host. + assert_eq!(extract_repo_path("group/subgroup/repo"), None); + assert_eq!(extract_repo_path("my.group/sub/repo"), None); + assert_eq!(extract_repo_host("my.group/sub/repo"), None); + // A scheme, userinfo or scp colon still marks one. + assert_eq!( + extract_repo_path("https://my.group/sub/repo").as_deref(), + Some("sub/repo") + ); + assert_eq!( + extract_repo_path("git@my.group:sub/repo").as_deref(), + Some("sub/repo") + ); + // An SSH-config host alias carries no userinfo and no dot, but the + // colon before the first slash still marks it scp-style. + assert_eq!( + extract_repo_path("corp-github:bohappdev/dotnet-azure-web-tsb.git").as_deref(), + Some("bohappdev/dotnet-azure-web-tsb") + ); + assert_eq!( + extract_repo_host("corp-github:bohappdev/dotnet-azure-web-tsb.git").as_deref(), + Some("corp-github") + ); + } + + #[test] + fn extract_repo_host_reads_the_host_from_marked_remotes() { + assert_eq!( + extract_repo_host("https://github.com/acme/api").as_deref(), + Some("github.com") + ); + assert_eq!( + extract_repo_host("git@gitlab.com:group/subgroup/repo.git").as_deref(), + Some("gitlab.com") + ); + // Port and userinfo are not part of the host. + assert_eq!( + extract_repo_host("https://git.example.com:8443/org/repo").as_deref(), + Some("git.example.com") + ); + assert_eq!( + extract_repo_host("https://oauth2:tok@gitlab.com/org/repo").as_deref(), + Some("gitlab.com") + ); + } } diff --git a/src/wait.rs b/src/wait.rs index dafb59c..d4add4b 100644 --- a/src/wait.rs +++ b/src/wait.rs @@ -1,26 +1,61 @@ use crate::config::Config; use crate::scanners::blast; use crate::utils; +use crate::utils::api::ProjectSelector; -pub fn run(config: &Config, scan_id: Option, project_id: Option) { - let project_name = utils::generic::determine_project_name(None); +#[derive(Default)] +pub struct WaitArgs { + pub scan_id: Option, + pub selector: ProjectSelector, + /// Known project id — `--project-id`, or straight from an upload response. + /// Paired with a scan id it skips resolution entirely. + pub project_id: Option, +} - let scans_result = - utils::api::query_scan_list(&config.get_url(), Some(&project_name), Some(1), None); - let scans: Vec = match scans_result { - Ok(result) => result.scans.unwrap_or_default(), - Err(e) => { - log::error!( - "Unable to query the scan list. Please check your connection and ensure that: +pub fn run(config: &Config, args: WaitArgs) { + let WaitArgs { + scan_id, + selector, + project_id: known_project_id, + } = args; + + // A scan id plus a known project id leaves nothing to resolve: everything + // below keys off the scan, and the id-form URL is already known. + let resolved = if scan_id.is_some() && known_project_id.is_some() { + let name = selector.name.clone().unwrap_or_else(|| { + utils::generic::get_current_working_directory().unwrap_or_else(|| "unknown".to_string()) + }); + 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, + project_id: None, + } + } 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); + e + ); + std::process::exit(1); + } } }; let (scan_id, processed) = match scan_id { @@ -39,20 +74,37 @@ pub fn run(config: &Config, scan_id: Option, project_id: Option) None => match scans.first() { Some(scan) => (scan.id.clone(), scan.status == "Complete"), None => { - log::error!("Error querying scan list"); + if resolved.confirmed { + log::error!( + "Project '{}' has no scans yet. Run 'corgea scan' to start one.", + project_name + ); + } else { + log::error!( + "No scan to wait for: no Corgea project found for {}. Run 'corgea scan', or pass --scan-id / --project-name.", + resolved.tried_label + ); + } std::process::exit(1); } }, }; + let project_id = known_project_id.or(resolved.project_id); 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 - ), + None => { + // Without an id the name goes into the path; an empty or slash-only + // name would yield `/project//?scan_id=…`, which resolves nowhere. + 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); + } + format!("{}/project/{}?scan_id={}", config.get_url(), name, scan_id) + } }; if !processed { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index f02b6c1..786f4d7 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -127,6 +127,27 @@ where base_url } +/// Every request target a stub was asked for, in order. +#[allow(dead_code)] +pub type Hits = std::sync::Arc>>; + +/// `spawn_http_stub` that also records every request target, so a test can +/// assert which endpoints were (not) dialed. Mirrors +/// `vuln_api_stub::spawn_capturing_vuln_api_stub`. +#[allow(dead_code)] +pub fn spawn_recording_http_stub(route: F) -> (String, Hits) +where + F: Fn(&str) -> (&'static str, String) + Send + 'static, +{ + let hits: Hits = Default::default(); + let recorder = hits.clone(); + let base_url = spawn_http_stub(move |path| { + recorder.lock().unwrap().push(path.to_string()); + route(path) + }); + (base_url, hits) +} + /// Registry stub serving `/pypi/oldpkg/json` (pypi) and `/oldpkg` (npm /// packument), both published 2020 → never recent. Everything else 404s. #[allow(dead_code)] @@ -433,3 +454,141 @@ pub fn tree_harness( .vuln_statuses(statuses) .build() } + +// --- project-resolution e2e fixtures (shared by list_resolution.rs and +// wait_resolution.rs) ------------------------------------------------------- + +/// Canonical project name for the Bank-of-Hope resolution case: the dir +/// basename (`dotnet-azure-web-tsb`) differs from the stored project name. +#[allow(dead_code)] +pub const CANON: &str = "bohappdev/dotnet-azure-web-tsb"; +/// Git remote whose slug resolves to `CANON`. +#[allow(dead_code)] +pub const REMOTE: &str = "https://github.com/bohappdev/dotnet-azure-web-tsb.git"; + +/// `/projects` hit returning the canonical project whose `repo_url` contains +/// the slug (id 7) — the new-backend confirmed path. +#[allow(dead_code)] +pub fn projects_match() -> String { + r#"{"status":"ok","projects":[{"id":7,"name":"bohappdev/dotnet-azure-web-tsb","repo_url":"https://github.com/bohappdev/dotnet-azure-web-tsb"}]}"#.to_string() +} + +/// `/projects` miss (repo not onboarded / pre-COR-1426 backend filtered out). +#[allow(dead_code)] +pub fn projects_empty() -> String { + r#"{"status":"ok","projects":[]}"#.to_string() +} + +/// `/scans` returning one `Complete` scan under `project`. +#[allow(dead_code)] +pub fn scans_one(project: &str) -> String { + format!( + r#"{{"status":"ok","page":1,"total_pages":1,"scans":[{{"id":"scan-123","project":"{project}","repo":"https://github.com/bohappdev/dotnet-azure-web-tsb","branch":"main","status":"Complete","engine":"blast","created_at":"2026-01-01T00:00:00Z"}}]}}"# + ) +} + +/// `/scans` returning an empty page. +#[allow(dead_code)] +pub fn scans_empty() -> String { + r#"{"status":"ok","page":1,"total_pages":1,"scans":[]}"#.to_string() +} + +/// Temp git repo at `/` with `origin` set to `remote`. The dir +/// basename is the caller's to choose so it can differ from the stored name. +#[allow(dead_code)] +pub fn temp_git_repo(dirname: &str, remote: &str) -> (TempDir, std::path::PathBuf) { + let tmp = TempDir::new().expect("temp dir"); + let repo_dir = tmp.path().join(dirname); + std::fs::create_dir(&repo_dir).expect("create repo dir"); + let repo = git2::Repository::init(&repo_dir).expect("git init"); + repo.remote("origin", remote).expect("set origin"); + (tmp, repo_dir) +} + +/// Temp NON-git dir at `/` (no remote). +#[allow(dead_code)] +pub fn temp_plain_dir(dirname: &str) -> (TempDir, std::path::PathBuf) { + let tmp = TempDir::new().expect("temp dir"); + let dir = tmp.path().join(dirname); + std::fs::create_dir(&dir).expect("create dir"); + (tmp, dir) +} + +/// The endpoints the `list`/`wait` resolution tests stub. `/verify` is always +/// answered `ok`; a field left `None` 404s, as does any other path — so a test +/// asserting an endpoint was never dialed simply leaves it unset. +/// +/// Routing is on the request-target PREFIX: `/projects` carries a +/// percent-encoded query, so the full target is not a stable key. +#[allow(dead_code)] +#[derive(Default, Clone)] +pub struct Routes { + pub projects: Option, + pub scans: Option, + pub issues: Option, + /// `GET /scan/{id}` — `check_scan_status`. + pub scan: Option, + /// `GET /scan/{id}/issues` — `report_scan_status` and the `--scan-id` + /// issue route. + pub scan_issues: Option, +} + +#[allow(dead_code)] +impl Routes { + /// The stub answer for `path`: a served body, else 404. Tests needing an + /// endpoint outside this table match it first and delegate here. + pub fn answer(&self, path: &str) -> (&'static str, String) { + let body = if path.starts_with("/api/v1/verify") { + Some(r#"{"status":"ok"}"#.to_string()) + } else if path.starts_with("/api/v1/projects?repo_url=") { + self.projects.clone() + } else if path.starts_with("/api/v1/scans?") { + self.scans.clone() + } else if path.starts_with("/api/v1/issues?") { + self.issues.clone() + } else if path.starts_with("/api/v1/scan/") { + if path.contains("/issues") { + self.scan_issues.clone() + } else { + self.scan.clone() + } + } else { + None + }; + match body { + Some(body) => ("200 OK", body), + None => ("404 Not Found", NOT_FOUND_JSON.to_string()), + } + } +} + +/// `Routes` behind a plain stub; returns the base URL. +#[allow(dead_code)] +pub fn spawn_resolution_stub(routes: Routes) -> String { + spawn_http_stub(move |path| routes.answer(path)) +} + +/// `Routes` behind a recording stub; returns the base URL and the hit log. +#[allow(dead_code)] +pub fn spawn_recording_resolution_stub(routes: Routes) -> (String, Hits) { + spawn_recording_http_stub(move |path| routes.answer(path)) +} + +/// Run `corgea ` against `url` from `cwd`, isolated from +/// the host (temp HOME). `CORGEA_URL`/`CORGEA_TOKEN` are layered back on after +/// `corgea_isolated` strips them. +#[allow(dead_code)] +pub fn run_corgea( + subcommand: &str, + args: &[&str], + url: &str, + cwd: &std::path::Path, +) -> std::process::Output { + let (mut cmd, _home) = corgea_isolated(); + cmd.arg(subcommand); + cmd.args(args); + cmd.env("CORGEA_URL", url) + .env("CORGEA_TOKEN", "test-token") + .current_dir(cwd); + cmd.output().expect("spawn corgea") +} diff --git a/tests/list_resolution.rs b/tests/list_resolution.rs new file mode 100644 index 0000000..ca5b52b --- /dev/null +++ b/tests/list_resolution.rs @@ -0,0 +1,584 @@ +//! End-to-end tests for `corgea list` repo-URL project resolution (COR-1577). +//! The stub route table lives in `common::Routes`. + +mod common; + +use common::{ + projects_empty, projects_match, scans_empty, scans_one, temp_git_repo, temp_plain_dir, Routes, + CANON, REMOTE, +}; +use std::path::Path; +use std::process::Output; + +// --- stub bodies ----------------------------------------------------------- + +/// `/issues` returning one issue (status `ok`). +fn issues_one() -> String { + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":1,"issues":[{"id":"issue-abc","scan_id":"scan-123","status":"open","urgency":"high","created_at":"2026-01-01T00:00:00Z","classification":{"id":"CWE-89","name":"SQL Injection","description":null},"location":{"file":{"name":"app.py","language":"python","path":"src/app.py"},"line_number":42,"project":{"name":"bohappdev/dotnet-azure-web-tsb","branch":null,"git_sha":null}},"details":null,"auto_triage":{"false_positive_detection":{"status":"none","reasoning":null}},"auto_fix_suggestion":null}]}"#.to_string() +} + +/// `/issues` exact-name miss (HTTP 200 `no_project_found`, mapped to 404). +fn issues_miss() -> String { + r#"{"status":"no_project_found"}"#.to_string() +} + +// --- harness --------------------------------------------------------------- + +/// The three listing endpoints `list` reads. +fn routes(projects: String, scans: String, issues: String) -> Routes { + Routes { + projects: Some(projects), + scans: Some(scans), + issues: Some(issues), + ..Default::default() + } +} + +fn spawn_stub(projects: String, scans: String, issues: String) -> String { + common::spawn_resolution_stub(routes(projects, scans, issues)) +} + +fn run_list(args: &[&str], url: &str, cwd: &Path) -> Output { + common::run_corgea("list", args, url, cwd) +} + +// --- tests ----------------------------------------------------------------- + +#[test] +fn list_uses_canonical_name_from_repo() { + // The checkout is `build-123`, so the canonical name can only come from + // resolution — and the assertion is on the query the CLI SENT, since the + // stub answers every /scans the same way and stdout alone would stay green + // through a regression. (PR #122 review) + let (url, hits) = common::spawn_recording_resolution_stub(routes( + projects_match(), + scans_one(CANON), + issues_one(), + )); + let (_tmp, repo) = temp_git_repo("build-123", REMOTE); + let out = run_list(&[], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("bohappdev/dotnet-azure-web-tsb"), + "stdout: {stdout}" + ); + assert!(stdout.contains("scan-123"), "stdout: {stdout}"); + let hits = hits.lock().unwrap(); + assert!( + hits.iter().any(|h| h.starts_with("/api/v1/scans?") + && h.contains("project=bohappdev%2Fdotnet-azure-web-tsb")), + "the canonical project must drive /scans; hits: {hits:?}" + ); +} + +#[test] +fn list_issues_shows_repo_resolved_issue() { + // Same shape as above: `build-123` plus an assertion on the sent query, so + // a fallback to the checkout name cannot pass. + let (url, hits) = common::spawn_recording_resolution_stub(routes( + projects_match(), + scans_one(CANON), + issues_one(), + )); + let (_tmp, repo) = temp_git_repo("build-123", REMOTE); + let out = run_list(&["--issues"], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(stdout.contains("issue-abc"), "stdout: {stdout}"); + let hits = hits.lock().unwrap(); + assert!( + hits.iter().any(|h| h.starts_with("/api/v1/issues?") + && h.contains("project=bohappdev%2Fdotnet-azure-web-tsb")), + "the canonical project must drive /issues; hits: {hits:?}" + ); +} + +#[test] +fn list_issues_percent_encodes_the_project_name() { + // A project name is user- and server-supplied; interpolated raw, an `&` + // would split the query and address the project `foo` instead. (PR #122 + // review) + let (url, hits) = common::spawn_recording_resolution_stub(routes( + projects_empty(), + scans_empty(), + issues_one(), + )); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_list(&["--issues", "--project-name", "foo&bar#baz"], &url, &dir); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter().any(|h| h.contains("project=foo%26bar%23baz")), + "the delimiters must be encoded, not split the query; hits: {hits:?}" + ); +} + +#[test] +fn list_resolves_through_an_ssh_config_host_alias() { + // `git@github.com-corgea:org/repo.git` — the shape this very repository's + // origin uses. Its host never equals the stored `github.com`, so treating + // the host as a gate rather than a tie-breaker would leave COR-1577 + // unfixed for every aliased remote. (PR #122 review) + let (url, hits) = common::spawn_recording_resolution_stub(routes( + projects_match(), + scans_one(CANON), + issues_one(), + )); + let (_tmp, repo) = temp_git_repo( + "build-123", + "git@github.com-corgea:bohappdev/dotnet-azure-web-tsb.git", + ); + let out = run_list(&[], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("bohappdev/dotnet-azure-web-tsb"), + "stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter().any(|h| h.starts_with("/api/v1/scans?") + && h.contains("project=bohappdev%2Fdotnet-azure-web-tsb")), + "the alias must still resolve the canonical project; hits: {hits:?}" + ); +} + +#[test] +fn list_repo_flag_resolves_from_flag_not_remote() { + // Records every request target so we can prove the slug came from --repo. + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + projects: Some(projects_match()), + scans: Some(scans_one(CANON)), + ..Default::default() + }); + // Non-git dir: no remote, so a resolved slug can ONLY have come from --repo. + let (_tmp, dir) = temp_plain_dir("unrelated-dir"); + let out = run_list(&["--repo", "bohappdev/dotnet-azure-web-tsb"], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("bohappdev/dotnet-azure-web-tsb"), + "stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/projects?repo_url=bohappdev%2Fdotnet-azure-web-tsb")), + "expected a /projects hit carrying the flag slug; hits: {hits:?}" + ); +} + +#[test] +fn list_miss_names_repo_no_empty_table() { + let url = spawn_stub(projects_empty(), scans_empty(), issues_miss()); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&[], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "stderr: {stderr}"); + assert!( + stderr.contains("repo 'bohappdev/dotnet-azure-web-tsb'"), + "stderr: {stderr}" + ); + assert!( + !stdout.contains("Scan ID"), + "should not render a table; stdout: {stdout}" + ); +} + +#[test] +fn list_confirmed_project_with_no_scans_exits_zero() { + // A confirmed project that simply has no scans is a valid empty result — + // failing it would break CI polling. + let url = spawn_stub(projects_match(), scans_empty(), issues_miss()); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&[], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(stdout.contains("has no scans yet"), "stdout: {stdout}"); +} + +#[test] +fn list_no_remote_falls_back_to_cwd_name() { + // Regression: non-git dir whose basename matches a project the scans stub + // serves under that exact name still lists scans (CWD-name fallback). + let url = spawn_stub(projects_empty(), scans_one("myproject"), issues_one()); + let (_tmp, dir) = temp_plain_dir("myproject"); + let out = run_list(&[], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(stdout.contains("myproject"), "stdout: {stdout}"); + assert!(stdout.contains("scan-123"), "stdout: {stdout}"); +} + +#[test] +fn list_project_name_override() { + let url = spawn_stub(projects_empty(), scans_one("some/name"), issues_one()); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_list(&["--project-name", "some/name"], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(stdout.contains("some/name"), "stdout: {stdout}"); +} + +#[test] +fn list_unconfirmed_falls_back_to_the_repo_name_not_the_checkout_dir() { + // Cloned into `build-123`: on a /projects soft miss (old or not-yet- + // onboarded backend) the query must stay what the pre-COR-1577 CLI sent — + // the repo basename — or a working setup starts missing. (PR #122 review) + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + projects: Some(projects_empty()), + scans: Some(scans_one("dotnet-azure-web-tsb")), + ..Default::default() + }); + let (_tmp, repo) = temp_git_repo("build-123", REMOTE); + let out = run_list(&[], &url, &repo); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.contains("project=dotnet-azure-web-tsb")), + "expected the repo basename, not the checkout dir; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.contains("project=build-123")), + "the checkout dir name must not be queried; hits: {hits:?}" + ); +} + +#[test] +fn list_repo_flag_keeps_every_segment_of_a_bare_slug() { + // A GitLab `group/subgroup/repo` has no host: all three segments are the + // path. Reading `group` as the host would query `subgroup/repo` and hit a + // different project. (PR #122 review) + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + projects: Some(projects_empty()), + scans: Some(scans_empty()), + ..Default::default() + }); + let (_tmp, dir) = temp_plain_dir("unrelated-dir"); + let out = run_list(&["--repo", "group/subgroup/repo.git"], &url, &dir); + assert_eq!(out.status.code(), Some(1), "an empty miss still exits 1"); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/projects?repo_url=group%2Fsubgroup%2Frepo&")), + "every segment must survive, .git trimmed; hits: {hits:?}" + ); +} + +#[test] +fn list_repo_flag_keeps_a_dotted_bare_namespace_whole() { + // GitLab namespaces may contain dots, so a dotted leading segment is no + // evidence of a host: `my.group/sub/repo` is a path in full. (PR #122 + // review) + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + projects: Some(projects_empty()), + scans: Some(scans_empty()), + ..Default::default() + }); + let (_tmp, dir) = temp_plain_dir("unrelated-dir"); + let out = run_list(&["--repo", "my.group/sub/repo"], &url, &dir); + assert_eq!(out.status.code(), Some(1), "an empty miss still exits 1"); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/projects?repo_url=my.group%2Fsub%2Frepo&")), + "the dotted namespace must not be read as a host; hits: {hits:?}" + ); +} + +#[test] +fn list_no_remote_falls_back_to_the_sanitized_dir_name() { + // `determine_project_name` sanitized the basename before this PR, so a + // project onboarded from `my app` is stored as `my_app` — query that, not + // the raw name. (PR #122 review) + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + scans: Some(scans_one("my_app")), + ..Default::default() + }); + let (_tmp, dir) = temp_plain_dir("my app"); + let out = run_list(&[], &url, &dir); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter().any(|h| h.contains("project=my_app")), + "expected the sanitized dir name; hits: {hits:?}" + ); +} + +#[test] +fn list_project_name_with_no_scans_exits_zero() { + // /scans answers 200-empty both for "project has no scans" and "no such + // project", so an explicit --project-name is the better authority: report + // the empty result, do not claim the project does not exist. (PR #122 + // review) + let url = spawn_stub(projects_empty(), scans_empty(), issues_miss()); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_list(&["--project-name", "some/name"], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(stdout.contains("has no scans yet"), "stdout: {stdout}"); +} + +#[test] +fn list_sca_issues_scopes_to_an_explicit_project_name() { + // The flags are offered on every `list` mode, so with --sca-issues they + // must actually scope the request rather than silently return every + // project's findings. `list_sca_issues` reads `project`. (PR #122 review) + let (url, hits) = common::spawn_recording_http_stub(|path| { + if path.starts_with("/api/v1/verify") { + ("200 OK", r#"{"status":"ok"}"#.to_string()) + } else if path.starts_with("/api/v1/issues/sca") { + ( + "200 OK", + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"# + .to_string(), + ) + } else { + ("404 Not Found", common::NOT_FOUND_JSON.to_string()) + } + }); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_list(&["--sca-issues", "--project-name", "some/name"], &url, &dir); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/issues/sca") && h.contains("project=some%2Fname")), + "the SCA request must carry the named project; hits: {hits:?}" + ); +} + +#[test] +fn list_sca_issues_without_a_selector_stays_unscoped() { + // Unflagged --sca-issues has always returned the company-wide latest scan; + // adding the flags must not silently narrow it. + let (url, hits) = common::spawn_recording_http_stub(|path| { + if path.starts_with("/api/v1/verify") { + ("200 OK", r#"{"status":"ok"}"#.to_string()) + } else if path.starts_with("/api/v1/issues/sca") { + ( + "200 OK", + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"# + .to_string(), + ) + } else { + ("404 Not Found", common::NOT_FOUND_JSON.to_string()) + } + }); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--sca-issues"], &url, &repo); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let hits = hits.lock().unwrap(); + assert!( + !hits.iter().any(|h| h.contains("project=")), + "no selector means no project scope; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/projects")), + "and no resolution round trip; hits: {hits:?}" + ); +} + +#[test] +fn list_project_name_and_repo_are_mutually_exclusive() { + let (_tmp, dir) = temp_plain_dir("whatever"); + let (mut cmd, _home) = common::corgea_isolated(); + cmd.args(["list", "--project-name", "a", "--repo", "b"]) + .env("CORGEA_URL", "http://127.0.0.1:1") + .env("CORGEA_TOKEN", "test-token") + .current_dir(&dir); + let out = cmd.output().expect("spawn corgea"); + // clap rejects conflicting args at parse time with a usage error, exit 2. + assert_eq!( + out.status.code(), + Some(2), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn list_json_miss_is_valid_empty_envelope() { + let url = spawn_stub(projects_empty(), scans_empty(), issues_miss()); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--json"], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + // Envelope on stdout, miss on stderr, exit 1. + assert_eq!( + out.status.code(), + Some(1), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let v: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("stdout not JSON ({e}): {stdout}")); + assert_eq!( + v["results"].as_array().map(|a| a.len()), + Some(0), + "results should be empty; stdout: {stdout}" + ); + assert!( + !stdout.contains("No Corgea project"), + "no human prose on stdout; stdout: {stdout}" + ); +} + +#[test] +fn list_issues_with_scan_id_skips_project_resolution() { + // The scan-id issue route ignores the project, so no /projects call should + // be made even from a real git repo where a remote IS present. (COR-1577) + // `projects` stays unset: dialing it would 404 and show up in the hits. + let routes = Routes { + scan_issues: Some( + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"#.to_string(), + ), + ..Default::default() + }; + let (url, hits) = common::spawn_recording_http_stub(move |path| { + if path.contains("/check_blocking_rules") { + ( + "200 OK", + r#"{"block":false,"blocking_issues":[],"total_pages":1}"#.to_string(), + ) + } else { + routes.answer(path) + } + }); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--issues", "--scan-id", "scan-xyz"], &url, &repo); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let hits = hits.lock().unwrap(); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/projects")), + "no /projects resolution for --issues --scan-id; hits: {hits:?}" + ); + assert!( + hits.iter().any(|h| h.contains("/scan/scan-xyz/issues")), + "expected the scan-id issue route; hits: {hits:?}" + ); +} + +#[test] +fn list_resolves_from_subdirectory() { + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + projects: Some(projects_match()), + scans: Some(scans_one(CANON)), + ..Default::default() + }); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let subdir = repo.join("src"); + std::fs::create_dir(&subdir).expect("create subdir"); + let out = run_list(&[], &url, &subdir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + // Discovery walks up from src/ to the repo root, so the remote slug — not + // the `src` basename — drives resolution: a /projects hit proves it. + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/projects?repo_url=bohappdev%2Fdotnet-azure-web-tsb")), + "expected /projects resolution from the subdir; hits: {hits:?}" + ); + assert!( + stdout.contains("bohappdev/dotnet-azure-web-tsb"), + "stdout: {stdout}" + ); +} + +#[test] +fn list_issues_json_miss_keeps_stdout_clean() { + let url = spawn_stub(projects_empty(), scans_empty(), issues_miss()); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--issues", "--json"], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + // The --issues miss is a hard error via log::error! (stderr); stdout stays + // clean so JSON consumers never see corrupt output. + assert_eq!(out.status.code(), Some(1), "stderr: {stderr}"); + assert!( + stdout.trim().is_empty(), + "stdout must be clean; stdout: {stdout}" + ); + assert!( + stderr.contains("No Corgea project found"), + "stderr should name the miss; stderr: {stderr}" + ); +} diff --git a/tests/wait_resolution.rs b/tests/wait_resolution.rs new file mode 100644 index 0000000..c8c5206 --- /dev/null +++ b/tests/wait_resolution.rs @@ -0,0 +1,367 @@ +//! End-to-end tests for `corgea wait` repo-URL project resolution (COR-1577). +//! The stub route table lives in `common::Routes`, whose `scan`/`scan_issues` +//! arms serve `check_scan_status` and `report_scan_status` respectively. + +mod common; + +use common::{ + projects_empty, projects_match, scans_empty, scans_one, temp_git_repo, temp_plain_dir, Routes, + CANON, REMOTE, +}; +use std::path::Path; +use std::process::Output; + +// --- stub bodies ----------------------------------------------------------- + +/// `GET /api/v1/scan/{id}` returning a completed scan (`check_scan_status` +/// checks the lowercase `complete`). +fn scan_complete() -> String { + r#"{"id":"scan-123","project":"bohappdev/dotnet-azure-web-tsb","repo":"https://github.com/bohappdev/dotnet-azure-web-tsb","branch":"main","status":"complete","engine":"blast","created_at":"2026-01-01T00:00:00Z"}"#.to_string() +} + +/// `GET /api/v1/scan/{id}/issues` returning an empty page — enough for +/// `report_scan_status` to succeed and print the result link. +fn scan_issues_empty() -> String { + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"#.to_string() +} + +// --- harness --------------------------------------------------------------- + +/// The single-scan endpoints every `wait` run reads once it has a scan id. +fn scan_routes() -> Routes { + Routes { + scan: Some(scan_complete()), + scan_issues: Some(scan_issues_empty()), + ..Default::default() + } +} + +fn routes(projects: String, scans: String) -> Routes { + Routes { + projects: Some(projects), + scans: Some(scans), + ..scan_routes() + } +} + +fn spawn_stub(projects: String, scans: String) -> String { + common::spawn_resolution_stub(routes(projects, scans)) +} + +fn run_wait(args: &[&str], url: &str, cwd: &Path) -> Output { + common::run_corgea("wait", args, url, cwd) +} + +// --- tests ----------------------------------------------------------------- + +#[test] +fn wait_uses_canonical_project_and_numeric_id_scan_url() { + let url = spawn_stub(projects_match(), scans_one(CANON)); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_wait(&[], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + // Numeric project id (7) from /projects, not the dir basename. + assert!( + stdout.contains("/project/7/?scan_id=scan-123"), + "stdout: {stdout}" + ); +} + +#[test] +fn wait_repo_flag_resolves_from_flag_not_remote() { + // Records every request target so we can prove the slug came from --repo. + let (url, hits) = + common::spawn_recording_resolution_stub(routes(projects_match(), scans_one(CANON))); + // Non-git dir: no remote, so a resolved slug can ONLY have come from --repo. + let (_tmp, dir) = temp_plain_dir("unrelated-dir"); + let out = run_wait(&["--repo", "bohappdev/dotnet-azure-web-tsb"], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("/project/7/?scan_id=scan-123"), + "stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/projects?repo_url=bohappdev%2Fdotnet-azure-web-tsb")), + "expected a /projects hit carrying the flag slug; hits: {hits:?}" + ); +} + +#[test] +fn wait_miss_names_repo_no_bare_error() { + let url = spawn_stub(projects_empty(), scans_empty()); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_wait(&[], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!( + out.status.code(), + Some(1), + "stdout: {stdout}\nstderr: {stderr}" + ); + assert!( + stderr.contains("repo 'bohappdev/dotnet-azure-web-tsb'"), + "stderr should name the repo; stderr: {stderr}" + ); + assert!( + !stdout.contains("Error querying scan list") + && !stderr.contains("Error querying scan list"), + "the bare error must be absent; stdout: {stdout}\nstderr: {stderr}" + ); +} + +#[test] +fn wait_project_name_override() { + let url = spawn_stub(projects_empty(), scans_one("some/name")); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_wait(&["--project-name", "some/name"], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + // No confirmed id, so the URL keeps the exact `--project-name` value. + assert!( + stdout.contains("/project/some/name?scan_id=scan-123"), + "stdout: {stdout}" + ); +} + +#[test] +fn wait_project_name_trailing_slash_is_trimmed() { + // The trailing slash must go before the name reaches `?project=`, which the + // backend matches exactly — asserting only the printed URL would pass even + // if `project=foo%2F` were sent. (PR #122 review) + let (url, hits) = + common::spawn_recording_resolution_stub(routes(projects_empty(), scans_one("foo"))); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_wait(&["--project-name", "foo/"], &url, &dir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("/project/foo?scan_id=scan-123"), + "stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + // `project` is the last query param, so `ends_with` also proves the value + // is not the un-trimmed `foo%2F`. + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/scans?") && h.ends_with("project=foo")), + "the scan listing must be queried for `foo`; hits: {hits:?}" + ); +} + +#[test] +fn wait_project_name_slash_only_is_rejected() { + let url = common::spawn_resolution_stub(scan_routes()); + let (_tmp, dir) = temp_plain_dir("whatever"); + let out = run_wait(&["--project-name", "/"], &url, &dir); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "stderr: {stderr}"); + assert!( + stderr.contains("--project-name must name a project"), + "stderr: {stderr}" + ); +} + +#[test] +fn wait_resolves_from_subdirectory() { + let (url, hits) = + common::spawn_recording_resolution_stub(routes(projects_match(), scans_one(CANON))); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let subdir = repo.join("src"); + std::fs::create_dir(&subdir).expect("create subdir"); + let out = run_wait(&[], &url, &subdir); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + // Discovery walks up from src/, so the remote — not the `src` basename — + // drives resolution. + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/projects?repo_url=bohappdev%2Fdotnet-azure-web-tsb")), + "expected /projects resolution from the subdir; hits: {hits:?}" + ); + assert!( + stdout.contains("/project/7/?scan_id=scan-123"), + "stdout: {stdout}" + ); +} + +#[test] +fn wait_with_scan_id_does_not_list_scans() { + // `scans` stays unset — the assertion is that it is never dialed. + let (url, hits) = common::spawn_recording_resolution_stub(Routes { + projects: Some(projects_match()), + ..scan_routes() + }); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_wait(&["scan-123"], &url, &repo); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + // The listing is only read when no scan id was given. + let hits = hits.lock().unwrap(); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/scans")), + "no scan listing with --scan-id; hits: {hits:?}" + ); +} + +#[test] +fn wait_project_id_flag_skips_resolution() { + // A caller who already knows the id (CI passing it between steps) needs no + // lookup at all: neither `projects` nor `scans` is served, and the id-form + // URL still comes out. (PR #122 review) + let (url, hits) = common::spawn_recording_resolution_stub(scan_routes()); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_wait(&["scan-123", "--project-id", "42"], &url, &repo); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("/project/42/?scan_id=scan-123"), + "stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + !hits + .iter() + .any(|h| h.starts_with("/api/v1/projects") || h.starts_with("/api/v1/scans")), + "--project-id must resolve nothing; hits: {hits:?}" + ); +} + +#[test] +fn wait_project_id_requires_a_scan_id() { + // Without a scan id the scan is still picked by the resolved project name, + // so a lone --project-id would only relabel the link — pointing at a + // different project than the scan. clap rejects it. (PR #122 review) + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_wait(&["--project-id", "42"], "http://127.0.0.1:1", &repo); + assert_eq!( + out.status.code(), + Some(2), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +/// `corgea scan semgrep` with a fake `semgrep` on PATH: the post-scan wait gets +/// the project id straight from the upload response, so it must resolve nothing +/// — no `/projects`, no `/scans` — and still link the id-form URL. +#[cfg(unix)] +#[test] +fn scan_post_wait_uses_upload_project_id_without_resolving() { + // Neither `projects` nor `scans` is served — the assertions are that + // neither is dialed. The upload endpoints are this test's own. + let routes = scan_routes(); + let (url, hits) = common::spawn_recording_http_stub(move |path| { + if path.starts_with("/api/v1/code-upload") || path.starts_with("/api/v1/git-config-upload") + { + ("200 OK", r#"{"status":"ok"}"#.to_string()) + } else if path.starts_with("/api/v1/scan-upload") { + ( + "200 OK", + r#"{"status":"ok","sast_scan_id":"scan-123","project_id":42}"#.to_string(), + ) + } else { + routes.answer(path) + } + }); + let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); + std::fs::write(repo.join("app.py"), "x = 1\n").expect("write source"); + let bin = repo.join("fakebin"); + std::fs::create_dir(&bin).expect("create fake bin dir"); + let report = r#"{"version":"1.0.0","errors":[],"results":[{"check_id":"rule","path":"app.py","start":{"line":1},"end":{"line":1},"extra":{"message":"m","severity":"ERROR","metadata":{"source":"https://semgrep.dev/r/rule"}}}]}"#; + common::write_script( + &bin, + "semgrep", + &format!("#!/bin/sh\nprintf '%s' '{report}'\n"), + ); + + let (mut cmd, _home) = common::corgea_isolated(); + let out = cmd + .args(["scan", "semgrep"]) + .env("PATH", &bin) + .env("CORGEA_URL", &url) + .env("CORGEA_TOKEN", "test-token") + // `running_in_ci()` is true under Actions, which sends `upload_scan` + // down the GITHUB_REPOSITORY branch. Pin the non-CI path either way. + .env_remove("CI") + .env_remove("GITHUB_ACTIONS") + .current_dir(&repo) + .output() + .expect("spawn corgea"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stdout: {stdout}\nstderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + stdout.contains("/project/42/?scan_id=scan-123"), + "expected the id-form scan URL; stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/projects")), + "no /projects resolution after an upload that carried the id; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/scans")), + "no scan listing after an upload that carried the id; hits: {hits:?}" + ); +} + +#[test] +fn wait_project_name_and_repo_are_mutually_exclusive() { + let (_tmp, dir) = temp_plain_dir("whatever"); + let (mut cmd, _home) = common::corgea_isolated(); + cmd.args(["wait", "--project-name", "a", "--repo", "b"]) + .env("CORGEA_URL", "http://127.0.0.1:1") + .env("CORGEA_TOKEN", "test-token") + .current_dir(&dir); + let out = cmd.output().expect("spawn corgea"); + // clap rejects conflicting args at parse time with a usage error, exit 2. + assert_eq!( + out.status.code(), + Some(2), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +}