-
Notifications
You must be signed in to change notification settings - Fork 1
Add code quality issue listing to the CLI #130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
05e2c18
ecf51c0
c72c3d6
e368bd4
6b56917
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,25 @@ use crate::utils; | |
| use serde_json::json; | ||
| use std::path::Path; | ||
|
|
||
| #[allow(clippy::too_many_arguments)] | ||
| pub fn run( | ||
| config: &Config, | ||
| issues: &bool, | ||
| sca_issues: &bool, | ||
| code_quality: &bool, | ||
| json: &bool, | ||
| page: &Option<u16>, | ||
| page_size: &Option<u16>, | ||
| scan_id: &Option<String>, | ||
| project_name: &Option<String>, | ||
| ) { | ||
| let project_name = | ||
| utils::generic::get_current_working_directory().unwrap_or("unknown".to_string()); | ||
| // Resolve the project name the same way `corgea scan` does: honor an explicit | ||
| // --project-name, otherwise prefer the Git remote repository name and fall | ||
| // back to the directory name. This matches the project key scans are stored | ||
| // under; using the bare directory basename caused "Project not found" | ||
| // whenever the checkout directory differed from the repository name (e.g. | ||
| // Git worktrees). | ||
| let project_name = utils::generic::determine_project_name(project_name.as_deref()); | ||
| println!(); | ||
| if *sca_issues { | ||
| let sca_issues_response = match utils::api::get_sca_issues( | ||
|
|
@@ -108,14 +116,30 @@ pub fn run( | |
| Some(sca_issues_response.page), | ||
| Some(sca_issues_response.total_pages), | ||
| ); | ||
| } else if *issues { | ||
| let issues_response = match utils::api::get_scan_issues( | ||
| &config.get_url(), | ||
| &project_name, | ||
| Some((*page).unwrap_or(1)), | ||
| *page_size, | ||
| scan_id.clone(), | ||
| ) { | ||
| } else if *issues || *code_quality { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Folding CQ into the Impact: A blocking-rules API blip (or security-only Fix: Gate enrichment on security listing only, e.g. |
||
| let fetch_result = if *code_quality { | ||
| utils::api::get_quality_issues( | ||
| &config.get_url(), | ||
| &project_name, | ||
|
Ibrahimrahhal marked this conversation as resolved.
|
||
| 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 issue_kind = if *code_quality { | ||
| "code quality issues" | ||
| } else { | ||
| "scan issues" | ||
| }; | ||
| let issues_response = match fetch_result { | ||
| Ok(response) => response, | ||
| Err(e) => { | ||
| debug(&format!("Error Sending Request: {}", e)); | ||
|
|
@@ -127,7 +151,7 @@ pub fn run( | |
| } | ||
| } 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 {}", | ||
|
|
@@ -141,7 +165,10 @@ pub fn run( | |
| let mut blocking_rules: std::collections::HashMap<String, String> = | ||
| std::collections::HashMap::new(); | ||
|
|
||
| if scan_id.is_some() { | ||
| // 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 scan_id.is_some() && !*code_quality { | ||
| let mut page: u32 = 1; | ||
| loop { | ||
| match utils::api::check_blocking_rules( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -513,6 +513,62 @@ pub fn get_scan_issues( | |
| } | ||
| } | ||
|
|
||
| pub fn get_quality_issues( | ||
| url: &str, | ||
| project: &str, | ||
| page: Option<u16>, | ||
| page_size: Option<u16>, | ||
| scan_id: Option<String>, | ||
| ) -> Result<ProjectIssuesResponse, Box<dyn std::error::Error>> { | ||
| let mut seperator = "?"; | ||
| let mut url = match scan_id { | ||
| Some(scan_id) => format!("{}{}/scan/{}/issues/quality", url, API_BASE, scan_id), | ||
| None => { | ||
| seperator = "&"; | ||
| format!( | ||
| "{}{}/issues/code-quality?project={}", | ||
| url, API_BASE, project | ||
| ) | ||
| } | ||
| }; | ||
| 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"); | ||
| } | ||
| let client = http_client(); | ||
|
|
||
| debug(&format!("Sending request to URL: {}", url)); | ||
|
|
||
| let response = match client.get(&url).send() { | ||
| Ok(res) => { | ||
| check_for_warnings(res.headers(), res.status()); | ||
| res | ||
| } | ||
| Err(e) => return Err(format!("Failed to send request: {}", e).into()), | ||
| }; | ||
| let response_text = response.text()?; | ||
|
Comment on lines
+546
to
+553
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New client ignores HTTP status; non-JSON errors become opaque parse/auth failures.
Impact: During companion rollout (or any 401/403/404/500 HTML/JSON error body), users get Fix: Mirror |
||
| 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<ScanResponse, Box<dyn std::error::Error>> { | ||
| let url = format!("{}{}/scan/{}", url, API_BASE, scan_id); | ||
|
|
||
|
|
@@ -1136,6 +1192,48 @@ 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"); | ||
|
Comment on lines
+1196
to
+1227
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test passes without exercising the new behavior (endpoint contract / CLI wiring).
Impact: False confidence for a net-new production API surface. Fix: Add a unit/integration test that builds the request URL (or uses a mock HTTP server) and asserts both endpoint variants + query params; add a clap/CLI test that |
||
| 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 should_warn_deprecated_false_when_no_warning_header() { | ||
| let headers = HeaderMap::new(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--project-nameis ignored in SCA mode, socorgea list --sca-issues --project-name foodoes not filter byfoo. Handling this combination explicitly would prevent the flag from implying filtering that does not occur.