Skip to content

Stop waiting forever on scans that already ended - #144

Open
leenk7991 wants to merge 7 commits into
mainfrom
fix/maven-project-scan
Open

Stop waiting forever on scans that already ended#144
leenk7991 wants to merge 7 commits into
mainfrom
fix/maven-project-scan

Conversation

@leenk7991

@leenk7991 leenk7991 commented Aug 2, 2026

Copy link
Copy Markdown
Member

corgea scan polled until status == "complete", so a scan that ended as incomplete was polled forever — the CI job hung until its own timeout with no indication anything had failed.

  • Scan status is classified into completed / failed / running, with incomplete, failed, error, and cancelled all terminal, matched case-insensitively (corgea wait compared against "Complete" and never matched).
  • A failed scan prints the reason and each scanner problem, then exits 1, so CI fails loudly.
    • A scan that completes with a scanner missing exits 0 and warns, since the other results are still valid.
  • Polling gives up after 4 hours (CORGEA_SCAN_TIMEOUT_SECONDS overrides) rather than burning a job's whole budget on a scan that never reaches a terminal state.
  • corgea scan, corgea wait, and corgea upload --wait all share this path.
    • Problem lists are capped at 10 lines with a pointer to the scan page.
  • Backward compatible: failed_reason and scan_errors are optional, so responses from servers without the doghouse change still parse and behave as before.

Merge order: doghouse first (the API fields the CLI reads), then fusion (writes the messages doghouse sanitizes), then cli. The CLI tolerates servers without the new fields, so the order is about when the output becomes useful, not about breakage.

Linear ticket

Test results

Before fix

2026-08-01 19_48_17-Downloads - File Explorer

After

Screenshot 2026-08-02 at 10 37 05 AM

Comment thread src/utils/api.rs Outdated
Comment thread src/wait.rs Outdated

@corgea-security corgea-security left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review risk: 4/5.

The core failure-reporting path breaks when the API returns scan_errors as null. The scan-details fallback URL also mishandles project names containing slashes.

Critical or high-priority changes must be addressed.

Automatic approval was not submitted: automated review found critical or high-priority findings.

@corgea-security corgea-security added the dennis-reviewed Dennis completed an automated review label Aug 2, 2026

@Ibrahimrahhal Ibrahimrahhal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we check for all scan status why do we need CORGEA_SCAN_TIMEOUT_SECONDS? The issue with this is that we have scans that can take 10 hours or more for big clients. maybe we can set it as 10hours for now and document that clients can override that in the docs

@leenk7991

Copy link
Copy Markdown
Member Author

if we check for all scan status why do we need CORGEA_SCAN_TIMEOUT_SECONDS? The issue with this is that we have scans that can take 10 hours or more for big clients. maybe we can set it as 10hours for now and document that clients can override that in the docs

done, it's set to 10 hours, and documentation updates in https://github.com/Corgea/docs/pull/303

@corgea-security corgea-security left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review risk: 2/5.

The diff addresses the existing timeout, null deserialization, and URL-encoding comments with targeted tests. No actionable critical, high-priority, or nitpick findings remain in the supplied diff.

No critical or high-priority changes were found.

Automatic approval was not submitted: changes have been requested.

Comment thread src/scanners/blast.rs

let result = loop {
thread::sleep(Duration::from_secs(1));
match utils::api::get_scan(&config.get_url(), scan_id) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a stalled status read can exceed CORGEA_SCAN_TIMEOUT_SECONDS because requests use the 150-second client timeout; could we start one deadline before the initial read and cap requests to its remaining time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dennis-reviewed Dennis completed an automated review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants