From 3b69f7378c98190e0690516ff6440997592e7b4b Mon Sep 17 00:00:00 2001 From: Chris Graf Date: Wed, 1 Jul 2026 07:46:50 -0500 Subject: [PATCH] Never let `verify --json` fail without a diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary emit a synthetic diagnostic for any non-zero verifier run that produced no parsable diagnostics, not just when stderr is non-empty. `_run_project_tool` only synthesized a diagnostic when the tool exited non-zero AND had no parsed diagnostics AND wrote to stderr. A verifier that exited non-zero with unparsable stdout and empty stderr therefore failed silently — a failing `rc` with zero diagnostics and no reason. Broaden the fallback to always surface one diagnostic: prefer the last stderr line, then the last stdout line, then a generic exit-status message. Testing - checkrun test suite: all suites pass (verify-test 107 passed) - new verify-test cases: non-zero exit with unparsable stdout + empty stderr yields a diagnostic from stdout; non-zero exit with no output yields a generic "exited with status N" diagnostic - checkrun lint clean on both files --- lib/checkrun/verify.py | 26 +++++++++++++++++++------- test/suites/verify-test | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/lib/checkrun/verify.py b/lib/checkrun/verify.py index b71ad4d..8d3c58f 100755 --- a/lib/checkrun/verify.py +++ b/lib/checkrun/verify.py @@ -815,17 +815,29 @@ def _run_project_tool( check=False, ) diagnostics = parse_json(project, proc.stdout) - if proc.returncode != 0 and not diagnostics and proc.stderr.strip(): - # govulncheck's JSON stream is structured when scanning reaches - # findings; cargo-audit behaves similarly for normal advisory - # reports. Transport/setup failures can still arrive only on - # stderr, so expose one synthetic diagnostic rather than an - # empty non-zero JSON run. + if proc.returncode != 0 and not diagnostics: + # A non-zero verifier with no parsable diagnostics must never + # fail silently. govulncheck/cargo-audit stream structured JSON + # once scanning reaches findings, but transport/setup failures + # (and outright crashes) can leave nothing parsable. Surface one + # synthetic diagnostic: prefer the last stderr line, then the + # last stdout line, then a generic exit-status message, so a + # failing run always carries a reason. + stderr = proc.stderr.strip() + stdout = proc.stdout.strip() + if stderr: + message = stderr.splitlines()[-1] + elif stdout: + message = stdout.splitlines()[-1] + else: + message = ( + f"{source} exited with status {proc.returncode} and produced no diagnostics" + ) diagnostics = [ _json_error( error_path(project), source, - proc.stderr.strip().splitlines()[-1], + message, ) ] for diagnostic in diagnostics: diff --git a/test/suites/verify-test b/test/suites/verify-test index 195f2f9..9ce21ef 100755 --- a/test/suites/verify-test +++ b/test/suites/verify-test @@ -727,6 +727,45 @@ _assert_contains "govulncheck json error: emits synthetic diagnostic" \ _assert_contains "govulncheck json error: points at go.mod" \ '/module-a/go.mod' "$output" +# Non-zero exit with UNPARSABLE stdout and EMPTY stderr must still surface a +# diagnostic (falls back to the last stdout line) instead of failing silently. +_stdout_only_mock=$(_mock_bin) +cat >"$_stdout_only_mock/govulncheck" <<'EOF' +#!/usr/bin/env bash +if [ "${1:-}" = "-json" ]; then + printf 'panic: unexpected internal error\n' +fi +exit 2 +EOF +chmod +x "$_stdout_only_mock/govulncheck" + +set +e +output=$(PATH="$_stdout_only_mock:$PATH" "$CHECKRUN" verify --json --tool govulncheck \ + "$_go_root/module-a/pkg/a.go" 2>/dev/null) +rc=$? +set -e +_assert_exit "govulncheck json error (stdout only): preserves exit" 2 "$rc" +_assert_contains "govulncheck json error (stdout only): diagnostic from stdout" \ + '"message":"panic: unexpected internal error"' "$output" + +# The core silent-failure case: non-zero exit with NO stdout and NO stderr must +# emit a generic exit-status diagnostic rather than an empty failing run. +_silent_mock=$(_mock_bin) +cat >"$_silent_mock/govulncheck" <<'EOF' +#!/usr/bin/env bash +exit 2 +EOF +chmod +x "$_silent_mock/govulncheck" + +set +e +output=$(PATH="$_silent_mock:$PATH" "$CHECKRUN" verify --json --tool govulncheck \ + "$_go_root/module-a/pkg/a.go" 2>/dev/null) +rc=$? +set -e +_assert_exit "govulncheck json error (silent): preserves exit" 2 "$rc" +_assert_contains "govulncheck json error (silent): emits fallback diagnostic" \ + 'exited with status 2 and produced no diagnostics' "$output" + _warning_mock=$(_mock_bin) cat >"$_warning_mock/govulncheck" <<'EOF' #!/usr/bin/env bash