diff --git a/src/list.rs b/src/list.rs index 751a00f..a497d70 100644 --- a/src/list.rs +++ b/src/list.rs @@ -1,37 +1,57 @@ use crate::config::Config; use crate::log::debug; use crate::utils; +use crate::utils::api::ProjectSelector; use serde_json::json; use std::path::Path; -#[allow(clippy::too_many_arguments)] -pub fn run( - config: &Config, - issues: &bool, - sca_issues: &bool, - json: &bool, - page: &Option, - page_size: &Option, - scan_id: &Option, - project_name_override: Option, - repo_override: Option, -) { +#[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; println!(); - if *sca_issues { - // SCA has no project parameter; this name is only error copy. - let project_name = utils::generic::determine_project_name(None); + 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 name below 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::determine_project_name(None)); 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); } @@ -48,7 +68,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, @@ -111,15 +131,11 @@ pub fn run( Some(sca_issues_response.page), Some(sca_issues_response.total_pages), ); - } else if *issues { + } else if issues { // The --scan-id route hits /scan/{id}/issues and ignores the project. - let resolved = scan_id.is_none().then(|| { - utils::api::resolve_project_or_exit( - &config.get_url(), - project_name_override.as_deref(), - repo_override.as_deref(), - ) - }); + 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()) @@ -127,8 +143,8 @@ pub fn run( 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, @@ -160,14 +176,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; @@ -190,7 +202,7 @@ pub fn run( } } - if *json { + if json { let mut json = serde_json::json!({ "page": issues_response.page, "total_pages": issues_response.total_pages, @@ -279,17 +291,13 @@ 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(), - project_name_override.as_deref(), - repo_override.as_deref(), - ); + 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, + page, + page_size, ) { Ok(scans) => { let page = scans.page; @@ -316,7 +324,7 @@ pub fn run( std::process::exit(1); } }; - if *json { + if json { let output = json!({ "page": page, "total_pages": total_pages, @@ -330,14 +338,14 @@ pub fn run( // 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. - if scans.is_empty() && !resolved.confirmed && project_name_override.is_none() { + 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 { + if json { return; } if scans.is_empty() { diff --git a/src/main.rs b/src/main.rs index c309606..2431e1c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -160,6 +160,13 @@ enum Commands { 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")] @@ -719,14 +726,19 @@ fn main() { scan_id, project_name, repo, + project_id, }) => { verify_token_and_exit_when_fail(&corgea_config); wait::run( &corgea_config, - scan_id.clone(), - project_name.clone(), - repo.clone(), - None, + 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 { @@ -750,14 +762,18 @@ fn main() { } list::run( &corgea_config, - issues, - sca_issues, - json, - page, - page_size, - scan_id, - project_name.clone(), - repo.clone(), + 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 6d389bb..15f701f 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -110,10 +110,14 @@ pub fn run_semgrep(config: &Config, project_name: Option) { if let Some(result) = parse_scan(config, output, true, project_name.clone()) { crate::wait::run( config, - Some(result.scan_id), - project_name, - None, - result.project_id, + crate::wait::WaitArgs { + scan_id: Some(result.scan_id), + selector: crate::utils::api::ProjectSelector { + name: project_name, + ..Default::default() + }, + project_id: result.project_id, + }, ); } } @@ -131,10 +135,14 @@ pub fn run_snyk(config: &Config, project_name: Option) { if let Some(result) = parse_scan(config, output, true, project_name.clone()) { crate::wait::run( config, - Some(result.scan_id), - project_name, - None, - result.project_id, + 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 d75335f..722ca8b 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 @@ -742,10 +744,24 @@ pub struct ProjectSummary { #[derive(Deserialize, Debug)] pub struct ProjectsResponse { - #[serde(default)] + /// 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; + /// 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 @@ -760,29 +776,35 @@ fn repo_url_matches_path(repo_url: &str, expected_path: &str) -> bool { r == expected_path } -/// Resolve the canonical project for a repo path via GET /api/v1/projects?repo_url=… -/// -/// 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 rather than listing a stranger's -/// scans. +/// 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=… /// -/// `Err` for hard failures (network/auth/5xx); a clean "no match" (or a 404 -/// from a backend without the endpoint) is a soft `Ok(None)`. -pub fn resolve_project_by_repo( +/// `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, -) -> Result, Box> { + page: u32, +) -> Result, Box> { let request_url = format!("{}{}/projects", url, API_BASE); let client = http_client(); debug(&format!( - "Resolving project via {} (repo_url={})", - request_url, repo_path + "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)]) + .query(&[ + ("repo_url", repo_path), + ("page", &page), + ("page_size", &page_size), + ]) .send()?; check_for_warnings(response.headers(), response.status()); let status = response.status(); @@ -793,22 +815,95 @@ pub fn resolve_project_by_repo( return Err(format!("/projects request failed: HTTP {}", status).into()); } let text = response.text()?; - let parsed: ProjectsResponse = match serde_json::from_str(&text) { - Ok(parsed) => parsed, + match serde_json::from_str(&text) { + Ok(parsed) => Ok(Some(parsed)), Err(e) => { - debug(&format!( - "Failed to parse /projects response: {} | body: {}", - e, text - )); - return Ok(None); + 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(); - Ok(parsed.projects.into_iter().find(|p| { - p.repo_url - .as_deref() - .is_some_and(|r| repo_url_matches_path(r, &expected)) - })) + 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); + if parsed.projects.is_empty() { + break; + } + for project in parsed.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 urls: 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, + urls.join(", ") + ) + .into()); + } + Ok(candidates.pop()) } /// What `list`/`wait` need to drive the existing name-based queries. @@ -822,6 +917,24 @@ pub struct ResolvedProject { 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 @@ -829,10 +942,10 @@ pub struct ResolvedProject { /// (network/auth/5xx from /projects). pub fn resolve_project( url: &str, - project_name_override: Option<&str>, - repo_override: Option<&str>, + selector: &ProjectSelector, ) -> Result> { - if let Some(name) = project_name_override { + 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('/'); @@ -849,17 +962,22 @@ pub fn resolve_project( // --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. - let repo_path = match repo_override { - Some(r) => { - Some(utils::generic::extract_repo_path(r).unwrap_or_else(|| r.trim().to_string())) - } - None => { - utils::generic::discover_repo_url().and_then(|u| utils::generic::extract_repo_path(&u)) - } + 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(r.trim().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)? { + if let Some(project) = resolve_project_by_repo(url, &repo_path, repo_host.as_deref())? { return Ok(ResolvedProject { query_name: project.name, confirmed: true, @@ -891,12 +1009,8 @@ pub fn resolve_project( /// `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, - project_name_override: Option<&str>, - repo_override: Option<&str>, -) -> ResolvedProject { - match resolve_project(url, project_name_override, repo_override) { +pub fn resolve_project_or_exit(url: &str, selector: &ProjectSelector) -> ResolvedProject { + match resolve_project(url, selector) { Ok(resolved) => resolved, Err(e) => { log::error!( @@ -1021,6 +1135,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![]; @@ -1030,6 +1145,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) @@ -1093,11 +1213,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; @@ -1565,7 +1692,7 @@ mod tests { let base = spawn_projects_stub( r#"{"status":"ok","projects":[{"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").unwrap(); + 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") @@ -1581,7 +1708,7 @@ mod tests { r#"{"status":"ok","projects":[{"name":"other/repo","repo_url":"https://github.com/other/repo"},{"name":"misc/thing","repo_url":"https://github.com/misc/thing"}]}"#, ); assert!( - resolve_project_by_repo(&base, "bohappdev/dotnet-azure-web-tsb") + resolve_project_by_repo(&base, "bohappdev/dotnet-azure-web-tsb", None) .unwrap() .is_none() ); @@ -1593,7 +1720,7 @@ mod tests { let base = spawn_projects_stub( r#"{"status":"ok","projects":[{"name":"acme/api-v2","repo_url":"https://github.com/acme/api-v2"}]}"#, ); - assert!(resolve_project_by_repo(&base, "acme/api") + assert!(resolve_project_by_repo(&base, "acme/api", None) .unwrap() .is_none()); } @@ -1601,12 +1728,12 @@ mod tests { #[test] fn resolve_project_by_repo_empty_and_404_are_soft_none() { let base = spawn_projects_stub(r#"{"status":"ok","projects":[]}"#); - assert!(resolve_project_by_repo(&base, "org/repo") + assert!(resolve_project_by_repo(&base, "org/repo", None) .unwrap() .is_none()); // /projects absent on a very old backend -> soft miss, not a failure. let base = spawn_projects_stub_status("404 Not Found", r#"{"message":"not found"}"#); - assert!(resolve_project_by_repo(&base, "org/repo") + assert!(resolve_project_by_repo(&base, "org/repo", None) .unwrap() .is_none()); } @@ -1615,17 +1742,131 @@ mod tests { 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").is_err()); + assert!(resolve_project_by_repo(&base, "org/repo", None).is_err()); + } + + // Serves `page_one` until a request carries `page=2`, then `page_two`. + 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_walks_past_the_first_page() { + // `repo_url__icontains` over a 20-per-page list: enough `acme/api-*` + // siblings push the exact `acme/api` onto page 2, and stopping at page + // 1 would recreate the very miss this resolves. + let base = spawn_paged_projects_stub( + r#"{"status":"ok","total_pages":2,"projects":[{"name":"acme/api-v2","repo_url":"https://github.com/acme/api-v2"}]}"#, + r#"{"status":"ok","total_pages":2,"projects":[{"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":[{"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"), "{err}"); + } + + #[test] + fn resolve_project_by_repo_bad_envelope_is_hard_err() { + // `@paginated` always emits `projects`, empty array included, so a 200 + // without it — or one that is not JSON at all — is an error envelope or + // a foreign responder, not a clean miss. + for body in [ + r#"{"status":"error","message":"boom"}"#, + "Access denied", + ] { + let base = spawn_projects_stub(body); + assert!( + resolve_project_by_repo(&base, "org/repo", None).is_err(), + "{body}" + ); + } + } + + #[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":[{"name":"gl","repo_url":"https://gitlab.com/acme/api"},{"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. + let base = spawn_projects_stub( + r#"{"status":"ok","projects":[{"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":[{"name":"gl","repo_url":"https://gitlab.com/acme/api"},{"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"), "{err}"); } #[test] fn resolve_project_name_override_is_normalized_and_never_empty() { // The name goes straight into `?project=`, which the backend matches // exactly, so a trailing slash would miss the project. - let r = resolve_project("http://127.0.0.1:1", Some("foo/"), None).unwrap(); + let r = resolve_project( + "http://127.0.0.1:1", + &ProjectSelector { + name: Some("foo/".into()), + repo: None, + }, + ) + .unwrap(); assert_eq!(r.query_name, "foo"); assert!(!r.confirmed); - assert!(resolve_project("http://127.0.0.1:1", Some("/"), None).is_err()); + assert!(resolve_project( + "http://127.0.0.1:1", + &ProjectSelector { + name: Some("/".into()), + repo: None + } + ) + .is_err()); } #[test] diff --git a/src/utils/generic.rs b/src/utils/generic.rs index bf4bdd9..718981c 100644 --- a/src/utils/generic.rs +++ b/src/utils/generic.rs @@ -330,6 +330,13 @@ 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 /// nothing marks the value as a network remote. diff --git a/src/wait.rs b/src/wait.rs index 1e293a1..45b2160 100644 --- a/src/wait.rs +++ b/src/wait.rs @@ -1,19 +1,29 @@ 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_name_override: Option, - repo_override: Option, - project_id: Option, -) { +#[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, +} + +pub fn run(config: &Config, args: WaitArgs) { + let WaitArgs { + scan_id, + selector, + project_id, + } = args; // A scan id plus the project id from the upload response 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() && project_id.is_some() { - let name = project_name_override + let name = selector + .name .clone() .unwrap_or_else(|| utils::generic::determine_project_name(None)); utils::api::ResolvedProject { @@ -23,11 +33,7 @@ pub fn run( confirmed: false, } } else { - utils::api::resolve_project_or_exit( - &config.get_url(), - project_name_override.as_deref(), - repo_override.as_deref(), - ) + utils::api::resolve_project_or_exit(&config.get_url(), &selector) }; let project_name = resolved.query_name.clone(); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 0969519..3d99a04 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -531,3 +531,79 @@ pub fn run_corgea( .current_dir(cwd); cmd.output().expect("spawn corgea") } + +/// 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, + pub sca_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/sca") { + self.sca_issues.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 recording stub; returns the base URL and the hit log. +#[allow(dead_code)] +pub fn spawn_resolution_stub(routes: Routes) -> (String, Hits) { + spawn_recording_http_stub(move |path| routes.answer(path)) +} + +/// An empty page of SCA issues. +#[allow(dead_code)] +pub fn sca_issues_empty() -> String { + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"#.to_string() +} + +/// An empty page of scan-scoped issues. +#[allow(dead_code)] +pub fn scan_issues_empty() -> String { + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"#.to_string() +} + +/// `GET /api/v1/scan/{id}` returning a completed scan (`check_scan_status` +/// checks the lowercase `complete`). +#[allow(dead_code)] +pub 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() +} diff --git a/tests/list_resolution.rs b/tests/list_resolution.rs index fe3a79b..0f36a35 100644 --- a/tests/list_resolution.rs +++ b/tests/list_resolution.rs @@ -6,7 +6,7 @@ mod common; use common::{ projects_empty, projects_match, scans_empty, scans_one, temp_git_repo, temp_plain_dir, Hits, - CANON, NOT_FOUND_JSON, REMOTE, + Routes, CANON, REMOTE, }; use std::path::Path; use std::process::Output; @@ -25,21 +25,21 @@ fn issues_miss() -> String { // --- harness --------------------------------------------------------------- -/// Stub serving verify + the three listing endpoints, and recording every -/// request target so a test can assert what the CLI actually sent. +/// The three listing endpoints `list` reads. fn spawn_stub(projects: String, scans: String, issues: String) -> (String, Hits) { - common::spawn_recording_http_stub(move |path| { - if path.starts_with("/api/v1/verify") { - ("200 OK", r#"{"status":"ok"}"#.to_string()) - } else if path.starts_with("/api/v1/projects?repo_url=") { - ("200 OK", projects.clone()) - } else if path.starts_with("/api/v1/scans?") { - ("200 OK", scans.clone()) - } else if path.starts_with("/api/v1/issues?") { - ("200 OK", issues.clone()) - } else { - ("404 Not Found", NOT_FOUND_JSON.to_string()) - } + common::spawn_resolution_stub(Routes { + projects: Some(projects), + scans: Some(scans), + issues: Some(issues), + ..Default::default() + }) +} + +/// Stub serving verify + the SCA listing endpoint only. +fn spawn_sca_stub() -> (String, Hits) { + common::spawn_resolution_stub(Routes { + sca_issues: Some(common::sca_issues_empty()), + ..Default::default() }) } @@ -229,6 +229,72 @@ fn list_project_name_override_skips_resolution() { ); } +#[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. + let (url, hits) = spawn_stub(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_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`. + let (url, hits) = spawn_sca_stub(); + 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) = spawn_sca_stub(); + 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"); @@ -277,22 +343,19 @@ fn list_json_miss_is_valid_empty_envelope() { 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) - 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.contains("/check_blocking_rules") { + // `projects` stays unset: dialing it would 404 and show up in the hits. + let routes = Routes { + scan_issues: Some(common::scan_issues_empty()), + ..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 if path.starts_with("/api/v1/scan/") && path.contains("/issues") { - ( - "200 OK", - r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"# - .to_string(), - ) } else { - ("404 Not Found", NOT_FOUND_JSON.to_string()) + routes.answer(path) } }); let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE); diff --git a/tests/wait_resolution.rs b/tests/wait_resolution.rs index 2a42e48..4428a2a 100644 --- a/tests/wait_resolution.rs +++ b/tests/wait_resolution.rs @@ -6,37 +6,22 @@ mod common; use common::{ projects_empty, projects_match, scans_empty, scans_one, temp_git_repo, temp_plain_dir, Hits, - CANON, NOT_FOUND_JSON, REMOTE, + Routes, CANON, REMOTE, }; use std::path::Path; use std::process::Output; // --- harness --------------------------------------------------------------- -/// `GET /api/v1/scan/{id}` returning a completed scan (`check_scan_status` -/// checks the lowercase `complete`); `/issues` under it returns an empty page, -/// enough for `report_scan_status` to succeed and print the result link. +/// The endpoints a `wait` run reads: resolution, the listing, and — once it +/// has a scan id — the single-scan routes. fn spawn_stub(projects: String, scans: String) -> (String, Hits) { - common::spawn_recording_http_stub(move |path| { - if path.starts_with("/api/v1/verify") { - ("200 OK", r#"{"status":"ok"}"#.to_string()) - } else if path.starts_with("/api/v1/projects?repo_url=") { - ("200 OK", projects.clone()) - } else if path.starts_with("/api/v1/scans?") { - ("200 OK", scans.clone()) - } else if path.starts_with("/api/v1/scan/") { - if path.contains("/issues") { - ( - "200 OK", - r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"# - .to_string(), - ) - } else { - ("200 OK", 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()) - } - } else { - ("404 Not Found", NOT_FOUND_JSON.to_string()) - } + common::spawn_resolution_stub(Routes { + projects: Some(projects), + scans: Some(scans), + scan: Some(common::scan_complete()), + scan_issues: Some(common::scan_issues_empty()), + ..Default::default() }) } @@ -144,16 +129,64 @@ fn wait_with_scan_id_does_not_list_scans() { ); } +#[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 dialed, and the id-form + // URL still comes out. + let (url, hits) = spawn_stub(projects_match(), scans_one(CANON)); + 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. + 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() { - let (url, hits) = common::spawn_recording_http_stub(|path| { - if path.starts_with("/api/v1/verify") - || path.starts_with("/api/v1/code-upload") - || path.starts_with("/api/v1/git-config-upload") + // Neither `projects` nor `scans` is served — the assertions are that + // neither is dialed. The upload endpoints are this test's own. + let routes = Routes { + scan: Some(common::scan_complete()), + scan_issues: Some(common::scan_issues_empty()), + ..Default::default() + }; + 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") { @@ -161,18 +194,8 @@ fn scan_post_wait_uses_upload_project_id_without_resolving() { "200 OK", r#"{"status":"ok","sast_scan_id":"scan-123","project_id":42}"#.to_string(), ) - } else if path.starts_with("/api/v1/scan/") { - if path.contains("/issues") { - ( - "200 OK", - r#"{"status":"ok","page":1,"total_pages":1,"total_issues":0,"issues":[]}"# - .to_string(), - ) - } else { - ("200 OK", r#"{"id":"scan-123","project":"p","repo":null,"branch":"main","status":"complete","engine":"blast","created_at":"2026-01-01T00:00:00Z"}"#.to_string()) - } } else { - ("404 Not Found", NOT_FOUND_JSON.to_string()) + routes.answer(path) } }); let (_tmp, repo) = temp_git_repo("dotnet-azure-web-tsb", REMOTE);