From f4fbf6a12d63fba76e31f43134a39c808ca3d0ea Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Wed, 12 Aug 2026 12:39:08 +0200 Subject: [PATCH 1/2] Name the concrete removal trigger for the openpgp exclusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Remove when sigstore drops the dependency" is not something anyone can check, so the entry would outlive its cause. Rekor already made the change — sigstore/rekor#2883, merged after v1.5.3 — so the trigger is a release containing it. Also record the import chain and that every govulncheck trace is package-init reachability rather than a call, which is the part that makes the exclusion defensible. --- .github/workflows/security-scan.yml | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 4c0f08d506..12c165e7a0 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -123,10 +123,20 @@ jobs: # # GO-2026-5932: golang.org/x/crypto/openpgp is deprecated-by-design # ("unsafe, not maintained, should not be used"). No fixed version - # exists and none is planned. ToolHive does not import openpgp - # directly; it is pulled transitively via sigstore (rekor/sigstore-go) - # for backward-compatible OpenPGP verification. Remove when sigstore - # drops the openpgp dependency. + # exists and none is planned. + # + # ToolHive does not import openpgp. It arrives four levels up: + # pkg/skills/signer -> sigstore-go/pkg/sign -> rekor/pkg/pki + # -> rekor/pkg/pki/pgp -> golang.org/x/crypto/openpgp + # rekor/pkg/pki is a pluggable signature-format registry, so + # importing it links every format, PGP included. Every govulncheck + # trace is package-init reachability, not a call: the code is + # linked but never parses PGP data on any ToolHive path. + # + # REMOVAL TRIGGER: rekor migrated to ProtonMail/go-crypto/openpgp + # in sigstore/rekor#2883 (merged 2026-07-15), which landed after + # v1.5.3 (2026-07-02) and so is not in any release yet. Bump rekor + # past v1.5.3 once a release contains it, then drop this entry. IGNORED_VULNS="GO-2026-5037 GO-2026-5038 GO-2026-5039 GO-2026-5932" # Show the raw output for debugging From dff06fa44fc01f522d644aba5b19440d40811fac Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Wed, 12 Aug 2026 15:14:48 +0200 Subject: [PATCH 2/2] Fail the vulnerability gate when the scan itself fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trailing `|| true` covered the whole pipeline, so a jq parse error, an empty output file, or a missing one all produced an empty finding list — indistinguishable from a clean scan, and the gate reported success. Verified against malformed, empty, and missing output: all three passed before, all three now fail. A genuinely clean scan and a real unexcluded finding both keep their previous verdicts. Found in review of stacklok/toolhive-core#230, whose equivalent step was modelled on this one. --- .github/workflows/security-scan.yml | 30 +++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 12c165e7a0..e5eaa16a89 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -111,6 +111,8 @@ jobs: - name: Check for vulnerabilities (with exclusions) run: | + set -euo pipefail + # Ignored vulnerabilities with justification: # Go stdlib advisories published 2026-06-02, all fixed in go1.26.4 / # go1.25.11 (DoS / log-injection class, no RCE): @@ -140,14 +142,34 @@ jobs: IGNORED_VULNS="GO-2026-5037 GO-2026-5038 GO-2026-5039 GO-2026-5932" # Show the raw output for debugging + # A scan that produced nothing is a failed scan, not a clean one. + # Without this, a crashed or truncated run yields an empty finding + # list indistinguishable from "no vulnerabilities" and the gate + # goes green. + if [ ! -s govulncheck-output.json ]; then + echo "::error::govulncheck produced no output; refusing to report success" + exit 1 + fi + echo "::group::govulncheck raw output" cat govulncheck-output.json echo "::endgroup::" - # Extract vulnerability IDs that have actual findings (called symbols) - # The JSON has "finding" objects with "osv" field only for vulnerabilities - # where vulnerable code paths are actually called - FOUND_VULNS=$(jq -r 'select(.finding != null) | .finding.osv' govulncheck-output.json | sort -u | grep -E '^GO-' || true) + # Extract vulnerability IDs that have actual findings (called symbols). + # The JSON has "finding" objects with an "osv" field only for + # vulnerabilities whose vulnerable code paths are actually called. + # + # jq failures must be fatal. A trailing `|| true` on the pipeline + # would swallow a parse error and hand us an empty list, which is + # how malformed output could pass the gate. + if ! jq -r 'select(.finding != null) | .finding.osv' govulncheck-output.json > found-osv.txt; then + echo "::error::could not parse govulncheck output; treating as a failed scan" + exit 1 + fi + + # grep exiting 1 means "no matches", which is legitimately clean — + # that is the only non-zero status tolerated here. + FOUND_VULNS=$(sort -u found-osv.txt | { grep -E '^GO-' || true; }) if [ -z "$FOUND_VULNS" ]; then echo "✅ No vulnerabilities found"