-
Notifications
You must be signed in to change notification settings - Fork 1
Add --wait to corgea upload and print scan page URL when not waiting
#131
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
Changes from all commits
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 | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -65,6 +65,12 @@ enum Commands { | |||||||||||||||
| help = "The name of the Corgea project. Defaults to git repository name if found, otherwise to the current directory name." | ||||||||||||||||
| )] | ||||||||||||||||
| project_name: Option<String>, | ||||||||||||||||
|
|
||||||||||||||||
| #[arg( | ||||||||||||||||
| long, | ||||||||||||||||
| help = "Wait for the uploaded scan to complete and print the results. Without this flag, the command prints the scan page URL so you can track the results." | ||||||||||||||||
| )] | ||||||||||||||||
| wait: bool, | ||||||||||||||||
| }, | ||||||||||||||||
| /// Scan the current directory. Supports blast, semgrep and snyk. | ||||||||||||||||
| Scan { | ||||||||||||||||
|
|
@@ -469,18 +475,25 @@ fn main() { | |||||||||||||||
| Some(Commands::Upload { | ||||||||||||||||
| report, | ||||||||||||||||
| project_name, | ||||||||||||||||
| wait, | ||||||||||||||||
| }) => { | ||||||||||||||||
| verify_token_and_exit_when_fail(&corgea_config); | ||||||||||||||||
| match report { | ||||||||||||||||
| let result = match report { | ||||||||||||||||
| Some(report) => { | ||||||||||||||||
| if report.ends_with(".fpr") { | ||||||||||||||||
| fortify_parse(&corgea_config, report, project_name.clone()); | ||||||||||||||||
| fortify_parse(&corgea_config, report, project_name.clone()) | ||||||||||||||||
| } else { | ||||||||||||||||
| scan::read_file_report(&corgea_config, report, project_name.clone()); | ||||||||||||||||
| scan::read_file_report(&corgea_config, report, project_name.clone()) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| None => { | ||||||||||||||||
| scan::read_stdin_report(&corgea_config, project_name.clone()); | ||||||||||||||||
| None => scan::read_stdin_report(&corgea_config, project_name.clone()), | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| if let Some(result) = result { | ||||||||||||||||
| if *wait { | ||||||||||||||||
| wait::run(&corgea_config, Some(result.scan_id), result.project_id); | ||||||||||||||||
|
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.
Impact: Concrete fix: pass |
||||||||||||||||
| } else { | ||||||||||||||||
| scan::print_scan_tracking_url(&corgea_config, &result); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
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.
If Impact: Users/CI that pass Fix: When 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.
This gate silently does nothing when Impact: CI can invoke A narrow fix is to fail when waiting was requested but the API did not provide the ID:
Suggested change
Also cover this branch with a CLI-level regression test so |
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,48 @@ pub fn run_command(base_cmd: &String, mut command: Command) -> String { | |
| pub struct ScanUploadResult { | ||
| pub scan_id: String, | ||
| pub project_id: Option<String>, | ||
| pub project_name: String, | ||
| } | ||
|
|
||
| /// Build the URL to the Corgea scan page so users can track results. | ||
| pub fn build_scan_url(config: &Config, result: &ScanUploadResult) -> String { | ||
| build_scan_url_from_base(&config.get_url(), result) | ||
| } | ||
|
|
||
| /// Build the scan page URL from an explicit base URL. Split out from | ||
| /// `build_scan_url` so the project-segment encoding contract can be unit | ||
| /// tested without constructing a `Config`. | ||
| fn build_scan_url_from_base(base_url: &str, result: &ScanUploadResult) -> String { | ||
| match &result.project_id { | ||
| Some(pid) => format!("{}/project/{}/?scan_id={}", base_url, pid, result.scan_id), | ||
| // The project name is a free-form path segment. In CI it is | ||
| // `{owner/repo}-{pr}`, so it must be percent-encoded or the `/` | ||
| // would route the tracking link to the wrong project. | ||
| None => format!( | ||
| "{}/project/{}?scan_id={}", | ||
| base_url, | ||
| urlencoding::encode(&result.project_name), | ||
| result.scan_id | ||
| ), | ||
|
Ibrahimrahhal marked this conversation as resolved.
|
||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /// Print the scan page URL so the user can track the scan results without | ||
| /// waiting for it to complete. | ||
| pub fn print_scan_tracking_url(config: &Config, result: &ScanUploadResult) { | ||
| let scan_url = build_scan_url(config, result); | ||
| print!( | ||
| "\n\nScan has started with ID: {}.\n\nYou can view it populate at the link:\n{}\n\n", | ||
| result.scan_id, | ||
| utils::terminal::set_text_color(&scan_url, utils::terminal::TerminalColor::Green) | ||
| ); | ||
| print!( | ||
| "{}", | ||
| utils::terminal::set_text_color( | ||
| "Your scan will continue securely in the Corgea cloud.\n", | ||
| utils::terminal::TerminalColor::Blue | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| pub fn run_semgrep(config: &Config, project_name: Option<String>) { | ||
|
|
@@ -85,14 +127,21 @@ pub fn run_snyk(config: &Config, project_name: Option<String>) { | |
| } | ||
| } | ||
|
|
||
| pub fn read_stdin_report(config: &Config, project_name: Option<String>) { | ||
| pub fn read_stdin_report( | ||
| config: &Config, | ||
| project_name: Option<String>, | ||
| ) -> Option<ScanUploadResult> { | ||
| let mut input = String::new(); | ||
| let _ = io::stdin().read_to_string(&mut input); | ||
|
|
||
| let _ = parse_scan(config, input, false, project_name); | ||
| parse_scan(config, input, false, project_name) | ||
| } | ||
|
|
||
| pub fn read_file_report(config: &Config, file_path: &str, project_name: Option<String>) { | ||
| pub fn read_file_report( | ||
| config: &Config, | ||
| file_path: &str, | ||
| project_name: Option<String>, | ||
| ) -> Option<ScanUploadResult> { | ||
| let input = match std::fs::read_to_string(file_path) { | ||
| Ok(input) => input, | ||
| Err(e) => { | ||
|
|
@@ -101,7 +150,7 @@ pub fn read_file_report(config: &Config, file_path: &str, project_name: Option<S | |
| } | ||
| }; | ||
|
|
||
| let _ = parse_scan(config, input, false, project_name); | ||
| parse_scan(config, input, false, project_name) | ||
| } | ||
|
|
||
| pub fn parse_scan( | ||
|
|
@@ -517,5 +566,38 @@ pub fn upload_scan( | |
| sast_scan_id.map(|scan_id| ScanUploadResult { | ||
|
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. Stale generic “Go to {base_url}” undercuts the new tracking URL
Impact: Misleading output (and noisier CI logs) right as this PR’s value prop is “print the scan page URL.” Fix: Drop or gate this generic line when a |
||
| scan_id, | ||
| project_id, | ||
| project_name: project.clone(), | ||
| }) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| fn result(project_id: Option<&str>, project_name: &str) -> ScanUploadResult { | ||
| ScanUploadResult { | ||
| scan_id: "scan-123".to_string(), | ||
| project_id: project_id.map(|p| p.to_string()), | ||
| project_name: project_name.to_string(), | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn scan_url_prefers_project_id_when_present() { | ||
| let url = | ||
| build_scan_url_from_base("https://www.corgea.app", &result(Some("42"), "some/name")); | ||
| assert_eq!(url, "https://www.corgea.app/project/42/?scan_id=scan-123"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn scan_url_percent_encodes_project_name_fallback() { | ||
| // CI project names are `{owner/repo}-{pr}`; the `/` must be encoded so | ||
| // the link resolves to the intended project rather than a nested path. | ||
| let url = | ||
| build_scan_url_from_base("https://www.corgea.app", &result(None, "corgea/cli-15")); | ||
| assert_eq!( | ||
| url, | ||
| "https://www.corgea.app/project/corgea%2Fcli-15?scan_id=scan-123" | ||
| ); | ||
| } | ||
| } | ||
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.
--waituses the wrong project identity (breaks--project-nameand CI uploads)ScanUploadResultnow carriesproject_name, but this call discards it:wait::runalways re-derives the project from the CWD and unconditionally queries that project's scan list before using the providedscan_id:Impact
corgea upload report.json --project-name my-svc --waituploads tomy-svc, then waits against the CWD project. If that CWD project is missing/unreachable, wait exits 1 after a successful upload.upload_scannames the project{GITHUB_REPOSITORY}-{GITHUB_PR}(src/scan.rs~204–214), which almost never matches the checkout directory name — soupload --waitin Actions is likely to fail or print the wrong fallback URL whenproject_idis absent.Fix
wait::runto accept the upload’sproject_name(or skipquery_scan_listentirely whenscan_idisSome).result.project_namethrough from this call site (and ideally fromrun_semgrep/run_snyktoo).--project-name≠ CWD +--waitmust not query/exit on the CWD project.