Repository navigation
pip-audit-bulk: fail when any scan is incomplete - #340
Conversation
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Add a timeout and handle FileNotFoundError for the osv-scanner call. · pip-audit-bulk:58-69
pip-audit-bulk:58-69
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a timeout and handle
FileNotFoundErrorfor theosv-scannercall.
subprocess.runhas notimeout. A hungosv-scannerblocks the audit indefinitely. This is a likely failure mode when OSV is unreachable, which is the case this PR targets. A hang cannot reach the new failure path. Set a timeout (for example 300s). Catchsubprocess.TimeoutExpiredand record the formula as a failure. Passcheck=Falseexplicitly. A missing binary raisesFileNotFoundErrorand aborts the whole run instead of recording a per-formula failure.Proposed fix
capture_output=True, text=True, + check=False, + timeout=300, )Handle
subprocess.TimeoutExpiredwhere the result is consumed. Convert it to a failed result.Source: Learnings
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
69030a02-ec43-485b-84b3-422474c74672
📒 Files selected for processing: 4
.github/workflows/audit.ymlREADME.mdpip-audit-bulktests/test_pip_audit_bulk.py
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout: 2
- GitHub Check: audit
- GitHub Check: workflow_syntax
🧰 Additional context used
📚 Code guidelines: 1
Generated by CodeRabbit — auto-discovered
📓 Path-based instructions: 3
For every checkout step, set `persist-credentials: false`.
📄 CodeRabbit inference engine (Generated by CodeRabbit)
Files:
.github/workflows/audit.yml
Pin remote GitHub Actions to full commit SHAs and include a version comment; keep local reusable-workflow references as local paths.
📄 CodeRabbit inference engine (Generated by CodeRabbit)
Files:
.github/workflows/audit.yml
Set workflow-level `permissions: {}` and grant only required permissions at the job level; reusable workflow callers should also keep their calling job's permissions restricted.
📄 CodeRabbit inference engine (Generated by CodeRabbit)
Files:
.github/workflows/audit.yml
🪛 ast-grep (0.45.3)
tests/test_pip_audit_bulk.py
[info] 29-29: use jsonify instead of json.dumps for JSON output
Context: json.dumps(data)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 41-41: use jsonify instead of json.dumps for JSON output
Context: json.dumps({formula: [f"{formula}==1.0"] for formula in results})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[error] 62-76: Command coming from incoming request
Context: subprocess.run(
[sys.executable, str(root / SCRIPT.name)],
cwd=root,
env={
**os.environ,
"PATH": str(bin_dir) + os.pathsep + os.environ["PATH"],
"CI": "1",
"GITHUB_STEP_SUMMARY": str(summary),
"QUICK_RUN": "false",
"SCANNER_RESULTS": json.dumps(results),
},
capture_output=True,
text=True,
timeout=10,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[info] 71-71: use jsonify instead of json.dumps for JSON output
Context: json.dumps(results)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.8)
tests/test_pip_audit_bulk.py
[error] 62-62: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments: 5
pip-audit-bulk: 2
84-91: LGTM!
133-144: LGTM!tests/test_pip_audit_bulk.py: 1
1-165: LGTM!README.md: 1
16-23: LGTM!Also applies to: 47-48
.github/workflows/audit.yml: 1
27-29: LGTM!
woodruffw
left a comment
There was a problem hiding this comment.
The core pip-audit-bulk change itself LGTM, but I'd remove almost everything else in this PR personally 🙂
osv-scanner exits 127 with an empty JSON result when it cannot reach OSV, which was recorded as a clean scan and published. Signed-off-by: Patrick Linnane <patrick@linnane.io>
9f58b8c to
2ee4807
Compare
osv-scanner exits 127 with an empty JSON result when OSV is unreachable, which was recorded as a clean scan and published. Any other exit than 0 or 1 now fails the audit and lists the formula in the summary. Scans pass
--no-resolve, since resolving pinned resources through deps.dev also exits 127 for some formulae.GPT-6 Astra drafted the change. Claude Code (Opus 5.5) reviewed it, reproducing the outage behaviour and running the patched audit over all 547 formulae with no failures. I reviewed the diff.