Skip to content

Fail the job when a scan does not complete - #139

Open
alimony wants to merge 1 commit into
google:mainfrom
alimony:fail-closed-on-incomplete-scan
Open

Fail the job when a scan does not complete#139
alimony wants to merge 1 commit into
google:mainfrom
alimony:fail-closed-on-incomplete-scan

Conversation

@alimony

@alimony alimony commented Jul 30, 2026

Copy link
Copy Markdown

Both reusable workflows run the scanner under continue-on-error: true, because osv-scanner exits non-zero when it finds vulnerabilities and the osv-reporter step is what decides whether to fail the job:

- name: "Run scanner"
uses: google/osv-scanner-action/osv-scanner-action@8dc09193bb540e09b23da07ad7e30bd33bf87018 # v2.3.8
env:
GOTOOLCHAIN: auto
with:
scan-args: |-
--output=${{ inputs.matrix-property }}results.json
--format=json
${{ inputs.scan-args }}
continue-on-error: true
- name: "Run osv-scanner-reporter"
uses: google/osv-scanner-action/osv-reporter-action@8dc09193bb540e09b23da07ad7e30bd33bf87018 # v2.3.8
with:
scan-args: |-
--output=${{ inputs.matrix-property }}${{ inputs.results-file-name }}
--new=${{ inputs.matrix-property }}results.json
--gh-annotations=false
--fail-on-vuln=${{ inputs.fail-on-vuln }}

That also swallows a scan that failed outright, and the step outcome alone cannot distinguish the two — both are just failure. The consequence is that the fail/pass decision is made from a results file that a failed scan never wrote, and osv-reporter treats a missing --new file as "no vulnerabilities found" and exits 0.

So anything that stops the scanner from completing produces a green check and an empty Code Scanning upload instead of a build failure. The existing Error troubleshooter step doesn't catch it: the reporter still writes an empty results.sarif, so the artifact upload succeeds.

Measured

Replaying osv-scanner-reusable.yml step for step against the images it pins (ghcr.io/google/osv-scanner-action:v2.3.8). "Poisoned" is the same lockfile as "vulnerable" plus one entry that crashes the npm parser ({"version": "npm:"}, google/osv-scalibr#2331):

repo scanner exit results.json before after
no lockfiles 0 missing SUCCESS SUCCESS
clean lockfile 0 present SUCCESS SUCCESS
10 known vulns (2 critical, 2 high) 1 present FAILURE FAILURE
same 10 vulns + crash entry 2 missing SUCCESS FAILURE

Only the last row changes. Before, the poisoned repo reports No issues found, uploads 0 findings to Code Scanning, and merges green while carrying the same dependencies that fail the build one row above.

osv-scanner-reusable-pr.yml is the more exposed of the two, since its new-code scan runs against the contents of the pull request. With a clean base and a PR head carrying vulnerable dependencies plus the crash entry:

scan-old: exit=0  old-results.json=present
scan-new: exit=2  new-results.json=MISSING
reporter: exit=0  said: No issues found

The fix

Give the scanner steps ids and check that a step which reported failure actually produced its results file; fail the job if it didn't.

This keeps every legitimate path intact:

  • exit 1 (vulnerabilities found) still writes the file, so the reporter keeps its role of applying --fail-on-vuln and the diffing in the PR workflow is untouched;
  • a repository with no lockfiles exits 0 — the action's exit_code_redirect.sh remaps 128 to 0 — so the guard's if: never fires and osv-scanner-action 2.0.0 errors if no lockfiles found #65's behaviour is preserved;
  • a genuinely failed scan (crash, or exit 129 on an OSV API failure) now fails the job instead of reporting a clean result, which also means "no vulnerabilities" is no longer reported when the vulnerability database was unreachable.

Not reformatted with Prettier: both files already differ from .prettierrc.json on main, so running it would bury the change in an unrelated diff.

Disclosure

Reported to Google's OSS VRP first (issue 536144722). Closed as not meeting the bar for a security bug, with an explicit invitation to file publicly, which is what this is.

The scanner steps run under continue-on-error because osv-scanner exits
non-zero when it finds vulnerabilities and the reporter step is what decides
whether to fail the job. That also swallows a scan that failed outright, and
those two cases are not distinguishable from the step outcome alone.

A scan that fails writes no results file, and osv-reporter treats a missing
--new file as "no vulnerabilities found" and exits 0. So anything that stops
the scanner from completing produces a green check and an empty Code Scanning
upload rather than a build failure. In osv-scanner-reusable-pr.yml the
new-code scan runs against the contents of the pull request, so a change that
crashes the scanner passes as a clean diff.

Check that a scanner step which reported failure actually produced its results
file, and fail the job if it did not. A scan that exits non-zero because it
found vulnerabilities still writes the file, so the reporter keeps its role of
applying --fail-on-vuln, and a repository with no lockfiles still exits 0 and
is unaffected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant