feat(cli): add --scan-timeout guard for large projects - #1100
Conversation
Closes OWASP#983. Large monorepos can stall a scan for minutes with no deterministic guard, forcing CI to rely on the job-level timeout. This adds --scan-timeout (90, 90s, 5m, 1500ms; 1s min, 60m max) plus CVE_LITE_SCAN_TIMEOUT env fallback, raced against both single-folder and multi-folder scans. On expiry the CLI exits 3 with an actionable message instead of hanging.
sonukapoor
left a comment
There was a problem hiding this comment.
The parser here is the best part of this and I want to say so before the asks. Every boundary I tried behaved correctly, 999ms and 3600001ms are both rejected with the right message, 0.5m works, 1e3 and +5s are refused, both the --flag value and --flag=value forms parse, and the env var follows the existing CVE_LITE_AUDIT_LOG convention. No new runtime dependencies.
The output behaviour on timeout is also right, which was my main worry. --json writes nothing rather than an empty array, no SARIF or HTML file is written, and it exits non-zero. A timeout that produced "no vulnerabilities found" would be the worst possible bug in this tool, and you avoided it.
Five things.
1. timer.unref?.() defeats the guard in exactly the case the flag exists for. src/utils/scan-timeout.ts:71. An unref'd timer cannot hold the event loop open, so if the scan stalls on something that is not live I/O, Node has nothing left to run and exits before the timeout can fire. I verified both directions against your built module with a promise that never settles:
with unref: process exits immediately, "TIMEOUT FIRED" never prints
without unref: TIMEOUT FIRED: Scan timed out after 2s...
The unref also buys nothing, since you clear the timer on both settle paths so it can never outlive the race. Deleting line 71 is the whole fix.
2. The feature has no integration coverage, and the suite does not notice. Replacing both wiring sites in index.ts so --scan-timeout does nothing at all leaves all 1783 tests passing. The flag still parses, validates, and prints its banner while enforcing nothing.
Your unit tests are genuinely good, every other mutation I tried was killed. But they only exercise the helper in isolation. What is missing is one spawned-CLI test on a fixture with a sub-second timeout asserting the exit code, empty stdout under --json, and no SARIF file written.
3. Route the timeout through ScanCompleteness instead of process.exit. This is the one I would most like changed, and it is my fault for not saying it on the issue.
A timeout is an incomplete scan, and there is already machinery for that: ScanDiagnosticCode, ScanDiagnostic with severity and impact, and shouldFailForIncompleteScan gated by --incomplete-policy. Right now a bare process.exit(EXIT_ERROR) bypasses all of it, so --incomplete-policy=warn cannot soften a timeout and a CI gate cannot tell a timeout from any other error.
Concretely: add SCAN_TIMEOUT to ScanDiagnosticCode, emit a diagnostic with impact: "detection", and let the existing policy decide the exit code. It is a small change and it keeps the flag's meaning consistent with the rest of the tool.
To be clear about what I am not asking for: I originally described this as wrapping the scan in an abort signal so partial findings survive. That is the better end state, but it means threading a signal through scanPackages, which is 641 lines on the hot path, and that is not a reasonable thing to bolt onto this PR. I will take that on separately. Getting the meaning right now matters more, because changing what a timeout means after release would be a breaking change.
4. No audit-log event on timeout. handleScanTimeout calls process.exit from inside a .catch(), so it never reaches the handler that emits error, and it skips auditLogHandle.close(). A timed-out scan currently leaves a trail identical to a SIGKILL: scan.started and nothing else. For something positioned as compliance evidence, the trail should say it timed out.
5. The docs claim more coverage than the code has. The description says the guard covers "lockfile parsing, OSV batch fan-out, usage scanning", but findNestedLockfiles and loadPackages both run before the wrap, and applyFixesIfRequested, the post-fix rescan, the overrides audit, DM001 and license detection all run after it. --fix --scan-timeout 30s on a large repo can run for many minutes past the deadline. Either extend the coverage or narrow the help text and docs to what is actually guarded.
Smaller things, take or leave. ScanTimeoutError and handleScanTimeout print the same guidance twice. The user-facing strings sit in index.ts rather than a utility module, and moving handleScanTimeout into scan-timeout.ts would also make it testable for item 2. Setting the env var prints (--scan-timeout) as the source even when no flag was passed. The info line is not suppressed under --ratchet, unlike its neighbours. And an invalid env var is not validated in validateOptions, so it falls through to the corporate-proxy hint, which is a pre-existing fallback but now catches a new class of error.
Closes #983.
Summary
Large monorepos can stall a scan for minutes (lockfile parsing, OSV batch fan-out, usage scanning) with no deterministic guard, forcing CI to rely on the job-level timeout.
Changes
--scan-timeout <duration>flag (90,90s,5m,1500ms; 1s min, 60m max) plusCVE_LITE_SCAN_TIMEOUTenv fallback.src/utils/scan-timeout.ts: parser, resolver,withScanTimeout()race,ScanTimeoutErrorwith actionable guidance.scanProject) and multi-folder (handleMultiFolderScan) paths; expiry exits3with guidance instead of hanging.src/cli/validate.ts, help text, andwebsite/docs/cli-reference.md.Verification
tests/scan-timeout.test.ts: 13 tests pass (unit parsing, env resolution, timeout race, CLI wiring, validation).npm run buildpasses.