google / google/osv-scanner

exit_code_redirect.sh ignores --allow-no-lockfiles=false in multiline Action input

Open
#3,005 6 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
11k
Forks
792
Avg merge
1d 20h
Merged PRs (30d)
48

Description

## Summary

The Docker Action passes multiline `scan-args` as its final argument.
`exit_code_redirect.sh` splits and executes those values from `split_args`,
but when `osv-scanner` exits `128`, the wrapper checks only the separate
prefix-argument collection for `--allow-no-lockfiles=false`.

In standard Action invocation, the prefix collection is empty. The explicit
fail-closed flag is executed but not inspected, and exit `128` is converted
to `0`.

## Current affected source

Reproduced against:

```text
Repository: google/osv-scanner
Commit: c84fa4568f2526d0333e9a914ea8a0a5f74ad68b
Release: v2.5.1
File: exit_code_redirect.sh
Git blob: 2e7482fa65b8b45cf5c0687452d58646c28e0990
File SHA-256: 4b6d24e5727c636a8a897545f2496223c9ed4426e2692dd096b0b4d08fd1725c
```

## Minimal local reproduction

The following reproduction is for a Linux Bash environment. It creates only a
temporary controlled scanner stub and removes it automatically.

```bash
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT

cat >"$tmp/osv-scanner" <<'STUB'
#!/usr/bin/env bash
exit 128
STUB

chmod +x "$tmp/osv-scanner"

scan_args=$'--allow-no-lockfiles=false\n--format=json\n--output=results.json\n-r\n./'

set +e
PATH="$tmp:$PATH" bash ./exit_code_redirect.sh "$scan_args"
wrapper_exit=$?
set -e

printf 'wrapper_exit=%s\n' "$wrapper_exit"
```

Observed:

```text
Exit code: 128
deprecation warning: ...
wrapper_exit=0
```

Expected:

```text
Exit code: 128
wrapper_exit=128
```

## Root cause

The wrapper executes both the prefix arguments and the split multiline Action
input:

```bash
osv-scanner $args "${split_args[@]}"
```

However, the exit-`128` policy loop examines only the prefix argument
collection:

```bash
for value in "${args[@]}"; do
```

The explicit `--allow-no-lockfiles=false` value is normally present in
`split_args`. It is therefore passed to and executed by `osv-scanner`, but it is
omitted from the wrapper's policy decision.

## Security-control effect

The validated composition is:

```text
--allow-no-lockfiles=false executed
scanner exit=128
wrapper exit=0
results.json absent
reporter exit=0
empty SARIF produced
modeled required-check outcome=success
```

This can cause an incomplete dependency scan to be represented as a successful
security check instead of a failed scan.

## Proposed remediation

Inspect the same combined argument set that is passed to `osv-scanner`:

```diff
- for value in "${args[@]}"; do
+ # Inspect both prefix arguments and the split multiline Action input.
+ for value in "${args[@]}" "${split_args[@]}"; do
```

This preserves the existing default and explicit-true behavior while honoring
the explicit false setting.

An in-repository regression test is prepared. It executes the real wrapper with
a controlled scanner stub and covers these paths:

```text
scanner exit 0 -> wrapper 0
scanner exit 1 -> wrapper 1
scanner exit 2 -> wrapper 2
default scanner exit 128 -> wrapper 0 with warning
--allow-no-lockfiles -> wrapper 0
--allow-no-lockfiles=true -> wrapper 0
-allow-no-lockfiles=true -> wrapper 0
--allow-no-lockfiles=false -> wrapper 128
-allow-no-lockfiles=false -> wrapper 128
prefix-form --allow-no-lockfiles=false -> wrapper 128
```

## Validation completed

```text
Baseline negative control:
PASS — the unmodified wrapper failed the new regression as expected

Independent patch applications:
2/2 PASS
patched trees byte-identical

Bash syntax:
2/2 PASS

Targeted Go regression:
2/2 PASS

Go race detector:
2/2 PASS

go vet:
2/2 PASS

gofmt:
2/2 PASS

Behavior matrix:
10/10 PASS on each independent run

Explicit-false stress:
1000/1000 expected results on each independently patched tree
0 unexpected results

Fresh same-input reproduction:
baseline wrapper exit=0
patched wrapper exit=128
executed scanner argument vector byte-identical
```

The full upstream repository test suite and hosted GitHub Actions end-to-end
workflow have not yet been run and are not claimed. Those will be completed
through the normal pull-request and CI process.

## Relationship to prior public work

This issue does not claim novelty for the broader condition where an incomplete
OSV-Scanner workflow can resolve successfully. That family has previously been
discussed in:

- `google/osv-scanner-action#71`
- `google/osv-scanner-action#139`

The narrower residual root described here is that the explicit
`--allow-no-lockfiles=false` option is executed from `split_args` but omitted
from the wrapper's exit-`128` policy inspection.

Preserving the failure also permits failure-conditioned missing-result checks to
execute instead of being bypassed by a prior `128 -> 0` conversion.

## Disclosure and remediation request

This behavior was reported to Google Bug Hunters as case `544980692`. Google
closed the report as below its security escalation threshold and explicitly
permitted public disclosure.

The bounded remediation patch and in-repository regression test are prepared.

Per `CONTRIBUTING.md`, please assign this issue or confirm the preferred test
location and pull-request path so I can submit the remediation through the
repository's normal review process.

No production target, third-party data, hosted workflow, or external repository
was modified during validation.

Contributor guide

Open the contributing guide

Research direction

Start with exit_code_redirect.sh and run the provided Linux Bash reproduction using the controlled scanner stub. Compare the exit-128 policy handling with the combined arguments passed to osv-scanner; done means multiline --allow-no-lockfiles=false returns 128 while the listed behavior matrix and prepared regression test pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
bash, docker, github-actions, go
Domain
ci-cd, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.