diff --git a/skills/corgea/SKILL.md b/skills/corgea/SKILL.md index df32d48..ebc45d0 100644 --- a/skills/corgea/SKILL.md +++ b/skills/corgea/SKILL.md @@ -77,6 +77,7 @@ corgea wait --scan-id SCAN_ID # Wait for specific scan corgea ls # List scans corgea ls --issues --scan-id SCAN_ID # Issues for a scan corgea ls --sca-issues # SCA (dependency) issues +corgea ls --code-quality # Code quality issues corgea ls --issues --page 2 --page-size 10 # Pagination corgea ls --issues --scan-id SCAN_ID --json # JSON output ``` @@ -85,6 +86,7 @@ corgea ls --issues --scan-id SCAN_ID --json # JSON output |------|-------|-------------| | `--issues` | `-i` | List code/SAST issues | | `--sca-issues` | `-c` | List SCA issues | +| `--code-quality` | `-q` | List code quality issues (alias `--quality`) | | `--scan-id` | `-s` | Filter to a scan | | `--page` | `-p` | Page number | | `--page-size` | | Items per page | diff --git a/src/list.rs b/src/list.rs index a497d70..486beab 100644 --- a/src/list.rs +++ b/src/list.rs @@ -9,6 +9,7 @@ use std::path::Path; pub struct ListArgs { pub issues: bool, pub sca_issues: bool, + pub code_quality: bool, pub json: bool, pub page: Option, pub page_size: Option, @@ -20,6 +21,7 @@ pub fn run(config: &Config, args: ListArgs) { let ListArgs { issues, sca_issues, + code_quality, json, page, page_size, @@ -131,8 +133,9 @@ pub fn run(config: &Config, args: ListArgs) { Some(sca_issues_response.page), Some(sca_issues_response.total_pages), ); - } else if issues { - // The --scan-id route hits /scan/{id}/issues and ignores the project. + } else if issues || code_quality { + // The --scan-id route hits /scan/{id}/issues[/quality] and ignores the + // project. let resolved = scan_id .is_none() .then(|| utils::api::resolve_project_or_exit(&config.get_url(), &selector)); @@ -140,13 +143,29 @@ pub fn run(config: &Config, args: ListArgs) { .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, - scan_id.clone(), - ) { + let issue_kind = if code_quality { + "code quality issues" + } else { + "scan issues" + }; + let fetch_result = if code_quality { + utils::api::get_quality_issues( + &config.get_url(), + &project_name, + Some(page.unwrap_or(1)), + page_size, + scan_id.clone(), + ) + } else { + utils::api::get_scan_issues( + &config.get_url(), + &project_name, + Some(page.unwrap_or(1)), + page_size, + scan_id.clone(), + ) + }; + let issues_response = match fetch_result { Ok(response) => response, Err(e) => { debug(&format!("Error Sending Request: {}", e)); @@ -162,7 +181,7 @@ pub fn run(config: &Config, args: ListArgs) { } } else { log::error!( - "Unable to fetch scan issues. Please check your connection and ensure that:\n\ + "Unable to fetch {issue_kind}. 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 {}", @@ -176,7 +195,10 @@ pub fn run(config: &Config, args: ListArgs) { let mut blocking_rules: std::collections::HashMap = std::collections::HashMap::new(); - if let Some(id) = &scan_id { + // Blocking rules are a security-listing concern. Skip the enrichment for + // code quality so a blocking-rules API failure can't take down the CQ + // listing and so Blocking columns aren't driven by non-CQ findings. + if let Some(id) = scan_id.as_ref().filter(|_| !code_quality) { let mut page: u32 = 1; loop { match utils::api::check_blocking_rules(&config.get_url(), id, Some(page)) { diff --git a/src/main.rs b/src/main.rs index 4420b0c..8832fec 100644 --- a/src/main.rs +++ b/src/main.rs @@ -190,6 +190,14 @@ enum Commands { )] sca_issues: bool, + #[arg( + long, + short = 'q', + visible_alias = "quality", + help = "List code quality issues instead of scans" + )] + code_quality: bool, + #[arg(short, long, help = "Specify the scan id to list issues for.")] scan_id: Option, @@ -774,15 +782,23 @@ fn main() { page_size, scan_id, sca_issues, + code_quality, project_name, repo, }) => { verify_token_and_exit_when_fail(&corgea_config); - if *issues && *sca_issues { - ::log::error!("Cannot use both --issues and --sca-issues at the same time."); + if [*issues, *sca_issues, *code_quality] + .iter() + .filter(|flag| **flag) + .count() + > 1 + { + ::log::error!( + "Cannot use more than one of --issues, --sca-issues, and --code-quality at the same time." + ); std::process::exit(1); } - if scan_id.is_some() && !*issues && !*sca_issues { + if scan_id.is_some() && !*issues && !*sca_issues && !*code_quality { println!("scan_id option is only supported for issues list command."); std::process::exit(1); } @@ -791,6 +807,7 @@ fn main() { list::ListArgs { issues: *issues, sca_issues: *sca_issues, + code_quality: *code_quality, json: *json, page: *page, page_size: *page_size, diff --git a/src/utils/api.rs b/src/utils/api.rs index ae164cd..e4c4f74 100644 --- a/src/utils/api.rs +++ b/src/utils/api.rs @@ -517,6 +517,89 @@ pub fn get_scan_issues( } } +/// Endpoint and query for a code quality listing. The backend serves code +/// quality from paths parallel to — but not named like — the security routes: +/// `/scan/{id}/issues/quality` for a scan, `/issues/code-quality` otherwise. +fn quality_issues_request( + url: &str, + project: &str, + page: Option, + page_size: Option, + scan_id: Option<&str>, +) -> (String, Vec<(&'static str, String)>) { + // Project names can contain `&`/`?`/`#`, so use `query`, not `format!`. + let (endpoint, mut query_params) = match scan_id { + Some(scan_id) => ( + format!("{}{}/scan/{}/issues/quality", url, API_BASE, scan_id), + vec![], + ), + None => ( + format!("{}{}/issues/code-quality", url, API_BASE), + vec![("project", project.to_string())], + ), + }; + if let Some(p) = page { + query_params.push(("page", p.to_string())); + } + query_params.push(("page_size", page_size.unwrap_or(30).to_string())); + (endpoint, query_params) +} + +pub fn get_quality_issues( + url: &str, + project: &str, + page: Option, + page_size: Option, + scan_id: Option, +) -> Result> { + let (endpoint, query_params) = + quality_issues_request(url, project, page, page_size, scan_id.as_deref()); + let client = http_client(); + + debug(&format!("Sending request to URL: {}", endpoint)); + debug(&format!("Query params: {:?}", query_params)); + + let response = match client.get(&endpoint).query(&query_params).send() { + Ok(res) => { + check_for_warnings(res.headers(), res.status()); + res + } + Err(e) => return Err(format!("Failed to send request: {}", e).into()), + }; + // Unlike the security routes, these endpoints answer a missing scan with a + // bare HTTP 404 rather than a `no_project_found` body, so the status has to + // be read before the parse or the miss surfaces as a parse failure. + let status = response.status(); + if !status.is_success() { + let body = response.text().unwrap_or_default(); + debug(&format!( + "Code quality request failed: HTTP {}. Response body: {}", + status, body + )); + if status == StatusCode::NOT_FOUND { + return Err("Code quality issues not found 404".into()); + } + return Err(format!("Request failed with status: {}", status).into()); + } + let response_text = response.text()?; + let project_issues_response: ProjectIssuesResponse = serde_json::from_str(&response_text) + .map_err(|e| { + debug(&format!( + "Failed to parse response: {}. Response body: {}", + e, response_text + )); + format!("Failed to parse response: {}", e) + })?; + + if project_issues_response.status == "ok" { + Ok(project_issues_response) + } else if project_issues_response.status == "no_project_found" { + Err("Project not found 404".into()) + } else { + Err("Server error 500".into()) + } +} + pub fn get_scan(url: &str, scan_id: &str) -> Result> { let url = format!("{}{}/scan/{}", url, API_BASE, scan_id); @@ -1491,6 +1574,85 @@ mod tests { assert!(headers.get("CORGEA-SOURCE").is_some()); } + #[test] + fn deserializes_code_quality_issue_response() { + // Code quality issues carry a free-form classification label (no CWE) and + // must deserialize into the same Issue struct used for security issues. + let body = r#"{ + "status": "ok", + "page": 1, + "total_pages": 1, + "total_issues": 1, + "issues": [ + { + "id": "11111111-1111-1111-1111-111111111111", + "urgency": "ME", + "created_at": "2026-01-01T00:00:00Z", + "status": "open", + "classification": { + "id": "Maintainability", + "name": "Maintainability", + "description": null + }, + "location": { + "file": {"name": "app.py", "language": "python", "path": "app/app.py"}, + "project": {"name": "proj", "branch": "main", "git_sha": "abc"}, + "line_number": 20 + }, + "auto_triage": {"false_positive_detection": {"status": "valid"}}, + "auto_fix_suggestion": {"status": "no_fix"} + } + ] + }"#; + + let parsed: ProjectIssuesResponse = + serde_json::from_str(body).expect("should parse code quality response"); + assert_eq!(parsed.status, "ok"); + let issues = parsed.issues.expect("issues present"); + assert_eq!(issues.len(), 1); + let issue = &issues[0]; + assert_eq!(issue.classification.id, "Maintainability"); + assert_eq!(issue.classification.name, "Maintainability"); + assert!(issue.classification.description.is_none()); + } + + #[test] + fn quality_issues_request_targets_the_documented_paths() { + // The two code quality routes are named asymmetrically on the backend, + // so the paths are pinned here rather than derived from each other. + let (endpoint, query) = + quality_issues_request("https://api.example.com", "proj", Some(2), Some(10), None); + assert_eq!( + endpoint, + "https://api.example.com/api/v1/issues/code-quality" + ); + assert_eq!( + query, + vec![ + ("project", "proj".to_string()), + ("page", "2".to_string()), + ("page_size", "10".to_string()), + ] + ); + + let (endpoint, query) = quality_issues_request( + "https://api.example.com", + "proj", + Some(1), + None, + Some("scan-123"), + ); + assert_eq!( + endpoint, + "https://api.example.com/api/v1/scan/scan-123/issues/quality" + ); + // A scan selects its own project, and the page size defaults to 30. + assert_eq!( + query, + vec![("page", "1".to_string()), ("page_size", "30".to_string())] + ); + } + #[test] fn should_warn_deprecated_false_when_no_warning_header() { let headers = HeaderMap::new(); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 83068c9..77d4be4 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -559,11 +559,15 @@ pub struct Routes { pub scans: Option, pub issues: Option, pub sca_issues: Option, + /// `GET /issues/code-quality` — the project-scoped `--code-quality` route. + pub code_quality_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, + /// `GET /scan/{id}/issues/quality` — the `--code-quality --scan-id` route. + pub scan_quality_issues: Option, } #[allow(dead_code)] @@ -579,10 +583,14 @@ impl Routes { self.scans.clone() } else if path.starts_with("/api/v1/issues/sca") { self.sca_issues.clone() + } else if path.starts_with("/api/v1/issues/code-quality") { + self.code_quality_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") { + if path.contains("/issues/quality") { + self.scan_quality_issues.clone() + } else if path.contains("/issues") { self.scan_issues.clone() } else { self.scan.clone() diff --git a/tests/list_code_quality.rs b/tests/list_code_quality.rs new file mode 100644 index 0000000..e213196 --- /dev/null +++ b/tests/list_code_quality.rs @@ -0,0 +1,209 @@ +//! End-to-end tests for `corgea list --code-quality`. +//! +//! The code quality routes are named asymmetrically on the backend +//! (`/issues/code-quality` for a project, `/scan/{id}/issues/quality` for a +//! scan), so the request targets are asserted rather than assumed. Stubs route +//! on the request-target path PREFIX. + +mod common; + +use common::{projects_empty, projects_match, Hits, Routes, CANON, REMOTE}; +use std::path::Path; +use std::process::Output; + +// --- stub bodies ----------------------------------------------------------- + +/// A code quality page: the classification is a label (`Maintainability`), +/// not a CWE, and carries no description. +fn quality_one() -> String { + r#"{"status":"ok","page":1,"total_pages":1,"total_issues":1,"issues":[{"id":"quality-abc","scan_id":"scan-123","status":"open","urgency":"medium","created_at":"2026-01-01T00:00:00Z","classification":{"id":"Maintainability","name":"Maintainability","description":null},"location":{"file":{"name":"app.py","language":"python","path":"src/app.py"},"line_number":20,"project":{"name":"bohappdev/dotnet-azure-web-tsb","branch":null,"git_sha":null}},"details":null,"auto_triage":{"false_positive_detection":{"status":"valid","reasoning":null}},"auto_fix_suggestion":null}]}"#.to_string() +} + +/// `/issues` returning one security issue, so a test can tell the two listings +/// apart by which id was rendered. +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() +} + +// --- harness --------------------------------------------------------------- + +/// Serves the project-scoped code quality route plus the security routes it +/// must not fall back to. +fn spawn_project_stub(projects: String) -> (String, Hits) { + common::spawn_resolution_stub(Routes { + projects: Some(projects), + issues: Some(issues_one()), + code_quality_issues: Some(quality_one()), + ..Default::default() + }) +} + +fn run_list(args: &[&str], url: &str, cwd: &Path) -> Output { + common::run_corgea("list", args, url, cwd) +} + +fn assert_exit(out: &Output, code: i32) { + assert_eq!( + out.status.code(), + Some(code), + "stdout: {}\nstderr: {}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); +} + +// --- tests ----------------------------------------------------------------- + +#[test] +fn code_quality_reads_the_code_quality_endpoint_not_the_security_one() { + let (url, hits) = spawn_project_stub(projects_empty()); + let (_tmp, repo) = common::temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--code-quality"], &url, &repo); + assert_exit(&out, 0); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("quality-abc"), "stdout: {stdout}"); + // The label stands in for the CWE column. + assert!(stdout.contains("Maintainability"), "stdout: {stdout}"); + assert!( + !stdout.contains("issue-abc"), + "the security listing must not answer --code-quality; stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/issues/code-quality?")), + "expected the code quality endpoint; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/issues?")), + "the security issue endpoint must not be dialed; hits: {hits:?}" + ); +} + +#[test] +fn code_quality_alias_and_short_flag_reach_the_same_endpoint() { + for flag in ["--quality", "-q"] { + let (url, hits) = spawn_project_stub(projects_empty()); + let (_tmp, repo) = common::temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&[flag], &url, &repo); + assert_exit(&out, 0); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/issues/code-quality?")), + "{flag} should list code quality; hits: {hits:?}" + ); + } +} + +#[test] +fn code_quality_scopes_to_the_project_resolved_from_the_repo() { + // The checkout is `build-123`, so a canonical `project=` can only have come + // from /projects resolution — the same path `--issues` takes. (COR-1577) + let (url, hits) = spawn_project_stub(projects_match()); + let (_tmp, repo) = common::temp_git_repo("build-123", REMOTE); + let out = run_list(&["--code-quality"], &url, &repo); + assert_exit(&out, 0); + let encoded = CANON.replace('/', "%2F"); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/issues/code-quality?") + && h.contains(&format!("project={encoded}"))), + "the canonical project must scope the code quality request; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.contains("project=build-123")), + "the checkout dir name must not be queried; hits: {hits:?}" + ); +} + +#[test] +fn code_quality_percent_encodes_the_project_name() { + // Interpolated raw, an `&` would split the query and address `foo` instead. + let (url, hits) = spawn_project_stub(projects_empty()); + let (_tmp, dir) = common::temp_plain_dir("whatever"); + let out = run_list( + &["--code-quality", "--project-name", "foo&bar#baz"], + &url, + &dir, + ); + assert_exit(&out, 0); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/issues/code-quality?") + && h.contains("project=foo%26bar%23baz")), + "the delimiters must be encoded, not split the query; hits: {hits:?}" + ); +} + +#[test] +fn code_quality_with_a_scan_id_uses_the_scan_route_and_skips_blocking_rules() { + // Blocking rules are a security concern: with `check_blocking_rules` + // unstubbed (404), reaching it would exit 1 even though the code quality + // fetch succeeded. + let (url, hits) = common::spawn_resolution_stub(Routes { + scan_issues: Some(issues_one()), + scan_quality_issues: Some(quality_one()), + ..Default::default() + }); + let (_tmp, repo) = common::temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--code-quality", "--scan-id", "scan-123"], &url, &repo); + assert_exit(&out, 0); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("quality-abc"), "stdout: {stdout}"); + assert!( + !stdout.contains("Blocking"), + "no blocking columns on a code quality table; stdout: {stdout}" + ); + let hits = hits.lock().unwrap(); + assert!( + hits.iter() + .any(|h| h.starts_with("/api/v1/scan/scan-123/issues/quality")), + "expected the scan-scoped code quality endpoint; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.contains("check_blocking_rules")), + "blocking rules must not be checked for code quality; hits: {hits:?}" + ); + assert!( + !hits.iter().any(|h| h.starts_with("/api/v1/projects")), + "no /projects resolution on the --scan-id route; hits: {hits:?}" + ); +} + +#[test] +fn code_quality_reports_a_missing_scan_rather_than_a_parse_failure() { + // These endpoints answer a missing scan with a bare HTTP 404, so the status + // has to be read before the body or the miss surfaces as "Failed to parse". + let (url, _hits) = common::spawn_resolution_stub(Routes::default()); + let (_tmp, repo) = common::temp_git_repo("dotnet-azure-web-tsb", REMOTE); + let out = run_list(&["--code-quality", "--scan-id", "nope"], &url, &repo); + assert_exit(&out, 1); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stderr.contains("Scan with ID 'nope'"), "stderr: {stderr}"); + assert!( + !stderr.contains("Failed to parse"), + "a 404 must not read as a parse failure; stderr: {stderr}" + ); +} + +#[test] +fn issue_kind_flags_are_mutually_exclusive() { + for args in [ + ["--issues", "--code-quality"], + ["--sca-issues", "--code-quality"], + ["--issues", "--sca-issues"], + ] { + let (url, _hits) = common::spawn_resolution_stub(Routes::default()); + let (_tmp, dir) = common::temp_plain_dir("whatever"); + let out = run_list(&args, &url, &dir); + assert_exit(&out, 1); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("Cannot use more than one of"), + "{args:?} should be rejected; stderr: {stderr}" + ); + } +}