From 11108b31e1668ee108074273d12b7165b36f93ee Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 08:51:07 -0700 Subject: [PATCH 01/12] Run govulncheck on a schedule This repo had no vulnerability scanning, so four (six for kamal-proxy) Go stdlib advisories sat against it unnoticed until a sibling repo that does scan went red and prompted a fleet check. Nothing reported them here because nothing was looking. Scheduled rather than PR-gating on purpose. A toolchain or dependency advisory is published against code that has not changed, so gating pull requests on it turns every new CVE into a red build on unrelated work -- which is exactly what happened to basecamp/cli today. The clock is the right trigger; the diff is not. Failures open an issue rather than only reddening the Actions tab, since a scheduled job nobody watches is the same silence this is meant to end. One issue at a time. --- .github/workflows/security.yml | 126 +++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 .github/workflows/security.yml diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 0000000..270d18d --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,126 @@ +name: Security + +# Toolchain and dependency advisories are published against code that has not +# changed, so this runs on a clock rather than on a diff. Gating pull requests +# on it would turn every newly published CVE into a red build on unrelated work. +on: + schedule: + - cron: '31 6 * * *' + workflow_dispatch: + +permissions: {} + +jobs: + govulncheck: + name: Govulncheck + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + env: + # Findings we have looked at and are choosing to carry, so the run stays + # quiet until something NEW appears. Everything else fails the job. + # + # GO-2026-4887 Moby AuthZ plugin bypass on oversized request bodies + # GO-2026-4883 Moby off-by-one in plugin privilege validation + # + # Both are in github.com/docker/docker, which reports "Fixed in: N/A" -- + # the fix exists only in github.com/moby/moby/v2 >= 2.0.0-beta.8, a + # module-path migration onto a beta. + # + # govulncheck reports these as reachable across most of the docker client + # surface we use, so treat that as "we link the module", not as a specific + # exploitable path: both bugs are in the *daemon's* plugin authorization, + # and we are a client of the daemon, not a host of it. + # + # Revisit when moby/moby/v2 is stable. + ACCEPTED: "GO-2026-4887 GO-2026-4883" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + + - name: Install govulncheck + run: go install golang.org/x/vuln/cmd/govulncheck@latest + + - name: Run govulncheck + id: scan + run: | + govulncheck -format json ./... > "${RUNNER_TEMP}/govulncheck.json" + govulncheck ./... > "${RUNNER_TEMP}/govulncheck.txt" 2>&1 || true + cat "${RUNNER_TEMP}/govulncheck.txt" + + # Fail only on findings govulncheck considers reachable (a symbol-level + # trace) that are not in ACCEPTED. This is the same set its own + # "Your code is affected by N vulnerabilities" line counts. + new=$(python3 - "$ACCEPTED" <<'PY' + import json, os, sys + accepted = {x for x in sys.argv[1].split() if x} + raw = open(os.environ["RUNNER_TEMP"] + "/govulncheck.json").read() + dec, i, reachable = json.JSONDecoder(), 0, set() + while i < len(raw): + while i < len(raw) and raw[i].isspace(): i += 1 + if i >= len(raw): break + obj, i = dec.raw_decode(raw, i) + f = obj.get("finding") + if f and (f.get("trace") or [{}])[0].get("function"): + reachable.add(f["osv"]) + print("\n".join(sorted(reachable - accepted))) + PY + ) + if [ -n "$new" ]; then + echo "New reachable vulnerabilities:" + echo "$new" + echo "$new" > "${RUNNER_TEMP}/new.txt" + exit 1 + fi + echo "No new reachable vulnerabilities (accepted: ${ACCEPTED:-none})." + + - name: Summarize + if: always() + run: | + { + echo '## govulncheck' + echo + echo '```' + cat "${RUNNER_TEMP}/govulncheck.txt" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + # A scheduled job that only goes red in the Actions tab is the same silence + # this workflow exists to end, so failures open an issue. One at a time: + # if a govulncheck issue is already open, the finding is already visible. + - name: Report + if: failure() && steps.scan.outcome == 'failure' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + if [ -n "$(gh issue list --label govulncheck --state open --limit 1 --json number --jq '.[].number')" ]; then + echo "A govulncheck issue is already open; not filing another." + exit 0 + fi + gh label create govulncheck --description "Reported by the scheduled govulncheck run" --color d93f0b --force + { + echo "The scheduled \`govulncheck\` run reported vulnerabilities that are not in this workflow's accepted list." + echo + echo "Run: ${RUN_URL}" + echo + echo '### New' + echo '```' + cat "${RUNNER_TEMP}/new.txt" 2>/dev/null || echo '(see full output below)' + echo '```' + echo + echo '### Full output' + echo '```' + cat "${RUNNER_TEMP}/govulncheck.txt" + echo '```' + echo + echo "Standard library findings are usually cleared by bumping the \`go\` directive in \`go.mod\` to the version named under \"Fixed in\"." + } > "${RUNNER_TEMP}/issue.md" + gh issue create --title "govulncheck: new vulnerabilities reported" --label govulncheck --body-file "${RUNNER_TEMP}/issue.md" From d7af5c74ba7573fbfb8be85f5bc8a2170aa8f165 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 09:33:14 -0700 Subject: [PATCH 02/12] Address review: exit codes, stale issues, races, module coverage govulncheck exits 3 for findings and other nonzero codes for failing to run. Conflating them meant a transient module-proxy error would file an issue saying vulnerabilities were found -- and that wrong issue would then suppress the next real one. Demuxed: 3 is a finding, anything else is a scanner error, reported as such. Bailing out whenever any govulncheck issue was open meant an issue filed for advisory A silenced advisory B entirely. Now every run writes itself into the tracking issue: comment if one is open, create if not. Nothing gets swallowed. Reporting was also skipped for failures before the scan -- a broken checkout or a failed govulncheck install reported nothing at all. It now runs on any job failure, and tolerates a missing output file instead of aborting on an unguarded cat. Added a concurrency group: read-then-write on the issue is not atomic, so a manual dispatch overlapping the cron could file two. ./... does not cross module boundaries, so modules are listed explicitly rather than assumed to be one. --- .github/workflows/security.yml | 160 ++++++++++++++++++++++++--------- 1 file changed, 116 insertions(+), 44 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 270d18d..c2c3ed4 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -10,6 +10,12 @@ on: permissions: {} +# The reporting step reads then writes the tracking issue, which is not atomic. +# A manual dispatch overlapping the scheduled run could otherwise file two. +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + jobs: govulncheck: name: Govulncheck @@ -18,6 +24,8 @@ jobs: contents: read issues: write env: + # ./... does not cross module boundaries, so every module is listed. + MODULES: ". installer integration/testapp" # Findings we have looked at and are choosing to carry, so the run stays # quiet until something NEW appears. Everything else fails the job. # @@ -29,11 +37,14 @@ jobs: # module-path migration onto a beta. # # govulncheck reports these as reachable across most of the docker client - # surface we use, so treat that as "we link the module", not as a specific + # surface, so treat that as "we link the module", not as a specific # exploitable path: both bugs are in the *daemon's* plugin authorization, # and we are a client of the daemon, not a host of it. # - # Revisit when moby/moby/v2 is stable. + # An ID stays accepted only while it has no fix. Re-check when bumping + # docker, and drop the ID once a fixed version is reachable for us -- + # otherwise this list keeps the job green exactly when it could go green + # honestly. Revisit when moby/moby/v2 is stable. ACCEPTED: "GO-2026-4887 GO-2026-4883" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -50,34 +61,65 @@ jobs: - name: Run govulncheck id: scan run: | - govulncheck -format json ./... > "${RUNNER_TEMP}/govulncheck.json" - govulncheck ./... > "${RUNNER_TEMP}/govulncheck.txt" 2>&1 || true - cat "${RUNNER_TEMP}/govulncheck.txt" - - # Fail only on findings govulncheck considers reachable (a symbol-level - # trace) that are not in ACCEPTED. This is the same set its own - # "Your code is affected by N vulnerabilities" line counts. - new=$(python3 - "$ACCEPTED" <<'PY' - import json, os, sys + set +e + : > "${RUNNER_TEMP}/report.txt" + rm -f "${RUNNER_TEMP}"/gv-*.json + n=0 + scan_error=0 + for module in ${MODULES}; do + n=$((n + 1)) + echo "===== ${module}" >> "${RUNNER_TEMP}/report.txt" + ( cd "${module}" && govulncheck -format json ./... ) > "${RUNNER_TEMP}/gv-${n}.json" 2> "${RUNNER_TEMP}/gv-${n}.err" + rc=$? + # govulncheck: 0 = clean, 3 = vulnerabilities found, anything else = it failed to run. + if [ "${rc}" -ne 0 ] && [ "${rc}" -ne 3 ]; then + scan_error=1 + echo "govulncheck exited ${rc} (scanner error, not a finding):" >> "${RUNNER_TEMP}/report.txt" + cat "${RUNNER_TEMP}/gv-${n}.err" >> "${RUNNER_TEMP}/report.txt" + rm -f "${RUNNER_TEMP}/gv-${n}.json" + continue + fi + ( cd "${module}" && govulncheck ./... ) >> "${RUNNER_TEMP}/report.txt" 2>&1 + done + set -e + + cat "${RUNNER_TEMP}/report.txt" + + if [ "${scan_error}" -eq 1 ]; then + echo "outcome=error" >> "$GITHUB_OUTPUT" + echo "govulncheck failed to run; see above." >&2 + exit 1 + fi + + # Fail only on findings govulncheck considers reachable (a symbol-level trace) + # that are not in ACCEPTED -- the same set its "Your code is affected by N + # vulnerabilities" line counts. + python3 - "$ACCEPTED" > "${RUNNER_TEMP}/new.txt" <<'PY' + import glob, json, os, sys accepted = {x for x in sys.argv[1].split() if x} - raw = open(os.environ["RUNNER_TEMP"] + "/govulncheck.json").read() - dec, i, reachable = json.JSONDecoder(), 0, set() - while i < len(raw): - while i < len(raw) and raw[i].isspace(): i += 1 - if i >= len(raw): break - obj, i = dec.raw_decode(raw, i) - f = obj.get("finding") - if f and (f.get("trace") or [{}])[0].get("function"): - reachable.add(f["osv"]) - print("\n".join(sorted(reachable - accepted))) + reachable = set() + for path in glob.glob(os.environ["RUNNER_TEMP"] + "/gv-*.json"): + raw = open(path).read() + dec, i = json.JSONDecoder(), 0 + while i < len(raw): + while i < len(raw) and raw[i].isspace(): i += 1 + if i >= len(raw): break + obj, i = dec.raw_decode(raw, i) + f = obj.get("finding") + if f and (f.get("trace") or [{}])[0].get("function"): + reachable.add(f["osv"]) + new = sorted(reachable - accepted) + if new: + print("\n".join(new)) PY - ) - if [ -n "$new" ]; then + + if [ -s "${RUNNER_TEMP}/new.txt" ]; then + echo "outcome=vulnerable" >> "$GITHUB_OUTPUT" echo "New reachable vulnerabilities:" - echo "$new" - echo "$new" > "${RUNNER_TEMP}/new.txt" + cat "${RUNNER_TEMP}/new.txt" exit 1 fi + echo "outcome=clean" >> "$GITHUB_OUTPUT" echo "No new reachable vulnerabilities (accepted: ${ACCEPTED:-none})." - name: Summarize @@ -87,40 +129,70 @@ jobs: echo '## govulncheck' echo echo '```' - cat "${RUNNER_TEMP}/govulncheck.txt" + cat "${RUNNER_TEMP}/report.txt" 2>/dev/null || echo '(no output -- the job failed before scanning)' echo '```' } >> "$GITHUB_STEP_SUMMARY" - # A scheduled job that only goes red in the Actions tab is the same silence - # this workflow exists to end, so failures open an issue. One at a time: - # if a govulncheck issue is already open, the finding is already visible. + # Reports any failure, not just a failing scan: a broken checkout or a + # failed govulncheck install is also something nobody would otherwise see. - name: Report - if: failure() && steps.scan.outcome == 'failure' + if: failure() env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + OUTCOME: ${{ steps.scan.outputs.outcome }} run: | - if [ -n "$(gh issue list --label govulncheck --state open --limit 1 --json number --jq '.[].number')" ]; then - echo "A govulncheck issue is already open; not filing another." - exit 0 + # Always reflect THIS run into the tracking issue. An issue opened for an + # earlier advisory says nothing about a new one, so bailing out when any issue + # is open would recreate the silence this workflow exists to end. One issue, + # kept current, with each run appended. + title="govulncheck: findings on the default branch" + if [ -s "${RUNNER_TEMP}/report.txt" ]; then + body_output=$(cat "${RUNNER_TEMP}/report.txt") + else + body_output="(govulncheck produced no output -- the job failed before or during the scan; see the run log.)" + fi + + if [ "${OUTCOME}" = "error" ]; then + headline="The scheduled \`govulncheck\` run **could not complete**. This is a scanner or environment failure, not a vulnerability report." + elif [ "${OUTCOME}" = "vulnerable" ]; then + headline="The scheduled \`govulncheck\` run found vulnerabilities that are not in this workflow's accepted list." + else + headline="The scheduled \`govulncheck\` job failed before it could scan. This is an environment or setup failure, not a vulnerability report." fi - gh label create govulncheck --description "Reported by the scheduled govulncheck run" --color d93f0b --force + { - echo "The scheduled \`govulncheck\` run reported vulnerabilities that are not in this workflow's accepted list." + echo "${headline}" echo echo "Run: ${RUN_URL}" + if [ -s "${RUNNER_TEMP}/new.txt" ]; then + echo + echo '### Not in the accepted list' + echo '```' + cat "${RUNNER_TEMP}/new.txt" + echo '```' + fi echo - echo '### New' - echo '```' - cat "${RUNNER_TEMP}/new.txt" 2>/dev/null || echo '(see full output below)' - echo '```' + echo '
govulncheck output' echo - echo '### Full output' echo '```' - cat "${RUNNER_TEMP}/govulncheck.txt" + echo "${body_output}" echo '```' echo - echo "Standard library findings are usually cleared by bumping the \`go\` directive in \`go.mod\` to the version named under \"Fixed in\"." - } > "${RUNNER_TEMP}/issue.md" - gh issue create --title "govulncheck: new vulnerabilities reported" --label govulncheck --body-file "${RUNNER_TEMP}/issue.md" + echo '
' + echo + echo "Standard library findings are usually cleared by bumping the \`go\` directive in \`go.mod\` (and any \`FROM golang:\` base image) to the version named under \"Fixed in\"." + } > "${RUNNER_TEMP}/body.md" + + # Let a failure of the lookup itself stop the job rather than read as "no issue + # open", which would file a duplicate. + existing=$(gh issue list --label govulncheck --state open --limit 1 --json number --jq '.[].number') + + if [ -n "${existing}" ]; then + gh issue comment "${existing}" --body-file "${RUNNER_TEMP}/body.md" + echo "Updated issue #${existing}." + else + gh label create govulncheck --description "Reported by the scheduled govulncheck run" --color d93f0b --force + gh issue create --title "${title}" --label govulncheck --body-file "${RUNNER_TEMP}/body.md" + fi From 1c230eb1f11d1c82b3a9723a3e7392d05ef209eb Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 09:44:00 -0700 Subject: [PATCH 03/12] Address review: scan released build configs, expire accepted IDs, name the ref govulncheck's source mode only analyses the files the current build configuration selects, so a single native run covered one of the configurations we actually ship. It now scans each GOOS/GOARCH in TARGETS with CGO_ENABLED=0, matching the Makefile's release matrix. An accepted ID was suppressed unconditionally, which kept the job green at exactly the moment a fix became available -- advisory metadata changes without any repository change, so nothing would have prompted anyone. Accepted IDs are now re-checked against the advisory's fixed versions and reported as actionable once a fix lands. The check is scoped to the module path we actually import: these advisories list a renamed module (moby/moby/v2) carrying a fix while the path we depend on (docker/docker) has none, so an unscoped check would fire wrongly. workflow_dispatch can target any ref, so a branch-only finding was being appended to an issue titled for the default branch. The report now names the ref and SHA it scanned and keeps non-default refs in their own issue. --- .github/workflows/security.yml | 112 +++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 27 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index c2c3ed4..15cbfc0 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -26,6 +26,9 @@ jobs: env: # ./... does not cross module boundaries, so every module is listed. MODULES: ". installer integration/testapp" + # govulncheck's source mode only analyses the files the current build + # configuration selects, so scan each GOOS/GOARCH we actually release. + TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" # Findings we have looked at and are choosing to carry, so the run stays # quiet until something NEW appears. Everything else fails the job. # @@ -66,20 +69,29 @@ jobs: rm -f "${RUNNER_TEMP}"/gv-*.json n=0 scan_error=0 - for module in ${MODULES}; do - n=$((n + 1)) - echo "===== ${module}" >> "${RUNNER_TEMP}/report.txt" - ( cd "${module}" && govulncheck -format json ./... ) > "${RUNNER_TEMP}/gv-${n}.json" 2> "${RUNNER_TEMP}/gv-${n}.err" - rc=$? - # govulncheck: 0 = clean, 3 = vulnerabilities found, anything else = it failed to run. - if [ "${rc}" -ne 0 ] && [ "${rc}" -ne 3 ]; then - scan_error=1 - echo "govulncheck exited ${rc} (scanner error, not a finding):" >> "${RUNNER_TEMP}/report.txt" - cat "${RUNNER_TEMP}/gv-${n}.err" >> "${RUNNER_TEMP}/report.txt" - rm -f "${RUNNER_TEMP}/gv-${n}.json" - continue - fi - ( cd "${module}" && govulncheck ./... ) >> "${RUNNER_TEMP}/report.txt" 2>&1 + # Releases are built CGO_ENABLED=0 for each GOOS/GOARCH in TARGETS. govulncheck's + # source mode only sees the files the current build configuration selects, so a + # single native run would miss a path reachable only on another target. + export CGO_ENABLED=0 + for target in ${TARGETS}; do + GOOS="${target%%/*}" + GOARCH="${target##*/}" + export GOOS GOARCH + for module in ${MODULES}; do + n=$((n + 1)) + echo "===== ${target} ${module}" >> "${RUNNER_TEMP}/report.txt" + ( cd "${module}" && govulncheck -format json ./... ) > "${RUNNER_TEMP}/gv-${n}.json" 2> "${RUNNER_TEMP}/gv-${n}.err" + rc=$? + # govulncheck: 0 = clean, 3 = vulnerabilities found, anything else = it failed to run. + if [ "${rc}" -ne 0 ] && [ "${rc}" -ne 3 ]; then + scan_error=1 + echo "govulncheck exited ${rc} (scanner error, not a finding):" >> "${RUNNER_TEMP}/report.txt" + cat "${RUNNER_TEMP}/gv-${n}.err" >> "${RUNNER_TEMP}/report.txt" + rm -f "${RUNNER_TEMP}/gv-${n}.json" + continue + fi + ( cd "${module}" && govulncheck ./... ) >> "${RUNNER_TEMP}/report.txt" 2>&1 + done done set -e @@ -91,13 +103,17 @@ jobs: exit 1 fi - # Fail only on findings govulncheck considers reachable (a symbol-level trace) - # that are not in ACCEPTED -- the same set its "Your code is affected by N - # vulnerabilities" line counts. + # Fail on findings govulncheck considers reachable (a symbol-level trace) that + # are not in ACCEPTED -- the same set its "Your code is affected by N + # vulnerabilities" line counts -- and on accepted IDs that have since gained a + # fix for the module path we actually depend on. python3 - "$ACCEPTED" > "${RUNNER_TEMP}/new.txt" <<'PY' import glob, json, os, sys + accepted = {x for x in sys.argv[1].split() if x} - reachable = set() + reachable = {} # osv id -> set of module paths it was reached through + advisories = {} + for path in glob.glob(os.environ["RUNNER_TEMP"] + "/gv-*.json"): raw = open(path).read() dec, i = json.JSONDecoder(), 0 @@ -105,22 +121,49 @@ jobs: while i < len(raw) and raw[i].isspace(): i += 1 if i >= len(raw): break obj, i = dec.raw_decode(raw, i) + if "osv" in obj: + advisories[obj["osv"]["id"]] = obj["osv"] f = obj.get("finding") - if f and (f.get("trace") or [{}])[0].get("function"): - reachable.add(f["osv"]) - new = sorted(reachable - accepted) - if new: - print("\n".join(new)) + if f: + frame = (f.get("trace") or [{}])[0] + if frame.get("function"): + reachable.setdefault(f["osv"], set()).add(frame.get("module")) + + def fixed_versions(osv_id, modules): + # Only a fix on a module path we actually import counts. These advisories + # routinely list a renamed module (moby/moby/v2) that carries the fix while + # the path we depend on (docker/docker) never gets one. + out = [] + for affected in (advisories.get(osv_id) or {}).get("affected", []): + if affected.get("package", {}).get("name") not in modules: + continue + for rng in affected.get("ranges", []): + for event in rng.get("events", []): + if event.get("fixed"): + out.append(f"{affected['package']['name']}@{event['fixed']}") + return out + + problems = [] + for osv_id in sorted(reachable): + if osv_id not in accepted: + problems.append(osv_id) + continue + fixes = fixed_versions(osv_id, reachable[osv_id]) + if fixes: + problems.append(f"{osv_id} (accepted, but now fixed in {', '.join(sorted(set(fixes)))} -- drop it from ACCEPTED and upgrade)") + + if problems: + print("\n".join(problems)) PY if [ -s "${RUNNER_TEMP}/new.txt" ]; then echo "outcome=vulnerable" >> "$GITHUB_OUTPUT" - echo "New reachable vulnerabilities:" + echo "Actionable findings:" cat "${RUNNER_TEMP}/new.txt" exit 1 fi echo "outcome=clean" >> "$GITHUB_OUTPUT" - echo "No new reachable vulnerabilities (accepted: ${ACCEPTED:-none})." + echo "No actionable findings (accepted: ${ACCEPTED:-none})." - name: Summarize if: always() @@ -142,12 +185,22 @@ jobs: GH_REPO: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} OUTCOME: ${{ steps.scan.outputs.outcome }} + SCANNED_REF: ${{ github.ref_name }} + SCANNED_SHA: ${{ github.sha }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | # Always reflect THIS run into the tracking issue. An issue opened for an # earlier advisory says nothing about a new one, so bailing out when any issue # is open would recreate the silence this workflow exists to end. One issue, # kept current, with each run appended. - title="govulncheck: findings on the default branch" + # A workflow_dispatch can target any ref, so never describe a run as being the + # default branch without checking. Non-default refs get their own issue rather + # than appending a branch-only finding to the canonical one. + if [ "${SCANNED_REF}" = "${DEFAULT_BRANCH}" ]; then + title="govulncheck: findings on ${DEFAULT_BRANCH}" + else + title="govulncheck: findings on ${SCANNED_REF}" + fi if [ -s "${RUNNER_TEMP}/report.txt" ]; then body_output=$(cat "${RUNNER_TEMP}/report.txt") else @@ -165,6 +218,8 @@ jobs: { echo "${headline}" echo + echo "Ref: \`${SCANNED_REF}\` @ \`${SCANNED_SHA}\`" + echo "Targets: \`${TARGETS}\`" echo "Run: ${RUN_URL}" if [ -s "${RUNNER_TEMP}/new.txt" ]; then echo @@ -183,11 +238,14 @@ jobs: echo '' echo echo "Standard library findings are usually cleared by bumping the \`go\` directive in \`go.mod\` (and any \`FROM golang:\` base image) to the version named under \"Fixed in\"." + echo + echo "An entry marked \"accepted, but now fixed\" means an ID in this workflow's ACCEPTED list has gained a fix for a module path we import: upgrade and drop the ID, rather than re-accepting it." } > "${RUNNER_TEMP}/body.md" # Let a failure of the lookup itself stop the job rather than read as "no issue # open", which would file a duplicate. - existing=$(gh issue list --label govulncheck --state open --limit 1 --json number --jq '.[].number') + existing=$(gh issue list --label govulncheck --state open --limit 30 --json number,title \ + --jq "[.[] | select(.title == \"${title}\")] | .[0].number // empty") if [ -n "${existing}" ]; then gh issue comment "${existing}" --body-file "${RUNNER_TEMP}/body.md" From 39f947f47b2eb9e4fd6463effca39a1cf8439e99 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 09:53:37 -0700 Subject: [PATCH 04/12] Address review: arm64 target, jq title as data, interval-aware fix check script/release publishes linux/amd64 and linux/arm64 via buildx, so scanning only amd64 missed half of what ships. My earlier check looked at the Makefile and the workflows and not at script/, which is where the release actually happens. Added linux/arm64. The issue lookup interpolated the title -- which carries a ref name, and git permits quotes in those -- straight into the jq program. A ref like foo"bar turned it into a syntax error, so the lookup failed and no tracking issue was filed at all. Passed as data via $ENV.TITLE instead; verified the old form errors and the new one matches. The fixed-version check collected every fixed event for the module, so an advisory that was fixed and later reintroduced would report the historical fix as an available upgrade and fail forever with advice that does not apply. It now walks each range's events in order and only reports a fix for the interval our own version actually falls in. --- .github/workflows/security.yml | 66 ++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 15cbfc0..835d8d6 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -111,7 +111,7 @@ jobs: import glob, json, os, sys accepted = {x for x in sys.argv[1].split() if x} - reachable = {} # osv id -> set of module paths it was reached through + reachable = {} # osv id -> {module path: set of versions} it was reached through advisories = {} for path in glob.glob(os.environ["RUNNER_TEMP"] + "/gv-*.json"): @@ -127,20 +127,53 @@ jobs: if f: frame = (f.get("trace") or [{}])[0] if frame.get("function"): - reachable.setdefault(f["osv"], set()).add(frame.get("module")) + mods = reachable.setdefault(f["osv"], {}) + mods.setdefault(frame.get("module"), set()).add(frame.get("version") or "v0.0.0") - def fixed_versions(osv_id, modules): - # Only a fix on a module path we actually import counts. These advisories - # routinely list a renamed module (moby/moby/v2) that carries the fix while - # the path we depend on (docker/docker) never gets one. + def parse(version): + # Enough of semver to order Go module versions: drop the leading v and any + # +incompatible/build metadata, split off a prerelease, compare numerically. + version = version.lstrip("v").split("+", 1)[0] + main, _, pre = version.partition("-") + parts = [] + for chunk in main.split("."): + parts.append(int(chunk) if chunk.isdigit() else 0) + while len(parts) < 3: + parts.append(0) + # No prerelease sorts above a prerelease of the same version. + return (parts, 0 if pre else 1, pre) + + def fix_for(osv_id, module_versions): + # Only a fix on a module path we actually import counts: these advisories + # routinely list a renamed module (moby/moby/v2) carrying the fix while the + # path we depend on (docker/docker) never gets one. + # + # And only a fix for the interval OUR version falls in counts. An advisory + # can be fixed, then reintroduced and left unfixed; treating the historical + # fixed event as an upgrade would fail the job forever with advice that does + # not apply. out = [] for affected in (advisories.get(osv_id) or {}).get("affected", []): - if affected.get("package", {}).get("name") not in modules: + name = affected.get("package", {}).get("name") + if name not in module_versions: continue - for rng in affected.get("ranges", []): - for event in rng.get("events", []): - if event.get("fixed"): - out.append(f"{affected['package']['name']}@{event['fixed']}") + for ours in module_versions[name]: + try: + target = parse(ours) + except Exception: + continue + for rng in affected.get("ranges", []): + introduced, fixed = None, None + for event in rng.get("events", []): + if event.get("introduced") is not None: + # A new interval starts; the previous one did not contain us. + introduced, fixed = event["introduced"], None + elif event.get("fixed") is not None: + fixed = event["fixed"] + if introduced is not None and parse(introduced) <= target < parse(fixed): + out.append(f"{name}@{fixed}") + introduced = None + # A trailing introduced with no fixed is an open, unfixed interval. return out problems = [] @@ -148,7 +181,7 @@ jobs: if osv_id not in accepted: problems.append(osv_id) continue - fixes = fixed_versions(osv_id, reachable[osv_id]) + fixes = fix_for(osv_id, reachable[osv_id]) if fixes: problems.append(f"{osv_id} (accepted, but now fixed in {', '.join(sorted(set(fixes)))} -- drop it from ACCEPTED and upgrade)") @@ -244,8 +277,13 @@ jobs: # Let a failure of the lookup itself stop the job rather than read as "no issue # open", which would file a duplicate. - existing=$(gh issue list --label govulncheck --state open --limit 30 --json number,title \ - --jq "[.[] | select(.title == \"${title}\")] | .[0].number // empty") + # The title carries a ref name, which git allows to contain quotes -- pass it to + # jq as data via $ENV rather than interpolating it into the jq program. + # $ENV.TITLE is expanded by jq, not by the shell -- that is the point of passing + # the title as data instead of interpolating it, so SC2016 is expected here. + # shellcheck disable=SC2016 + existing=$(TITLE="${title}" gh issue list --label govulncheck --state open --limit 30 --json number,title \ + --jq '[.[] | select(.title == $ENV.TITLE)] | .[0].number // empty') if [ -n "${existing}" ]; then gh issue comment "${existing}" --body-file "${RUNNER_TEMP}/body.md" From 51d61a199b31b2d2c960a3e0e130b1826e0f9941 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 09:57:42 -0700 Subject: [PATCH 05/12] Address review: a failed target must not bury findings from one that worked The early exit on scan_error ran before the filter, so if one target or module failed to analyse while another found a real reachable vulnerability, the run reported only a scanner error -- explicitly telling the team it was not a vulnerability report -- and never wrote new.txt at all. A persistent target-specific failure could hide confirmed findings indefinitely. The filter now runs over whatever JSON was produced regardless, and the two conditions are reported independently: vulnerable, error, or vulnerable-and-error, which says both need attention and that the coverage gap may be hiding more. Verified with one good module and one unresolvable module on an old toolchain: outcome=vulnerable-and-error, all four stdlib findings still listed, scanner error still reported. --- .github/workflows/security.yml | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 835d8d6..e208632 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -97,11 +97,8 @@ jobs: cat "${RUNNER_TEMP}/report.txt" - if [ "${scan_error}" -eq 1 ]; then - echo "outcome=error" >> "$GITHUB_OUTPUT" - echo "govulncheck failed to run; see above." >&2 - exit 1 - fi + # A failed target must not bury a real finding from a target that succeeded, so + # always filter whatever JSON was produced before deciding what to report. # Fail on findings govulncheck considers reachable (a symbol-level trace) that # are not in ACCEPTED -- the same set its "Your code is affected by N @@ -189,10 +186,23 @@ jobs: print("\n".join(problems)) PY + findings=0 if [ -s "${RUNNER_TEMP}/new.txt" ]; then - echo "outcome=vulnerable" >> "$GITHUB_OUTPUT" + findings=1 echo "Actionable findings:" cat "${RUNNER_TEMP}/new.txt" + fi + + if [ "${scan_error}" -eq 1 ] && [ "${findings}" -eq 1 ]; then + echo "outcome=vulnerable-and-error" >> "$GITHUB_OUTPUT" + echo "govulncheck also failed to run for at least one target/module; see above." >&2 + exit 1 + elif [ "${scan_error}" -eq 1 ]; then + echo "outcome=error" >> "$GITHUB_OUTPUT" + echo "govulncheck failed to run for at least one target/module; see above." >&2 + exit 1 + elif [ "${findings}" -eq 1 ]; then + echo "outcome=vulnerable" >> "$GITHUB_OUTPUT" exit 1 fi echo "outcome=clean" >> "$GITHUB_OUTPUT" @@ -240,7 +250,9 @@ jobs: body_output="(govulncheck produced no output -- the job failed before or during the scan; see the run log.)" fi - if [ "${OUTCOME}" = "error" ]; then + if [ "${OUTCOME}" = "vulnerable-and-error" ]; then + headline="The scheduled \`govulncheck\` run found vulnerabilities that are not in this workflow's accepted list, **and** failed to scan at least one target or module. Both need attention: the findings below are real, and the coverage gap means there may be more." + elif [ "${OUTCOME}" = "error" ]; then headline="The scheduled \`govulncheck\` run **could not complete**. This is a scanner or environment failure, not a vulnerability report." elif [ "${OUTCOME}" = "vulnerable" ]; then headline="The scheduled \`govulncheck\` run found vulnerabilities that are not in this workflow's accepted list." From bc7280e6ed5a54e6f2fdeed6f133c6f3e3924529 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 14:45:03 -0700 Subject: [PATCH 06/12] Address review: make the version comparator actually correct Three separate bugs in the same small function, so it is rewritten to SemVer rules once rather than patched per symptom. Stdlib findings carry a go prefix (go1.26.5) while the advisory's own ranges use a bare 1.26.5. Stripping only v made the former parse as [0,26,5], which matches a historical fixed interval and would fail the run forever with an inapplicable instruction. Prerelease identifiers were compared as one string, so beta.2 sorted above beta.10 and a real available fix went unreported. They are now split on dots and compared per SemVer 11.4, with numeric identifiers ranking below alphanumeric ones and any prerelease ranking below its release. Build metadata is dropped before comparison rather than left to influence precedence. Also: an accepted ID that no longer appears in the scan is dead config that would silently swallow the advisory if a later version reintroduced it. It is now listed in the job summary as removable, without failing an otherwise clean run. And the tracking-issue lookup fetched a bounded page, so with enough per-ref issues open the one for this ref could fall outside it and a duplicate would be created. Comparator verified against 11 cases including both reported bugs. --- .github/workflows/security.yml | 63 ++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index e208632..8e51946 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -128,17 +128,36 @@ jobs: mods.setdefault(frame.get("module"), set()).add(frame.get("version") or "v0.0.0") def parse(version): - # Enough of semver to order Go module versions: drop the leading v and any - # +incompatible/build metadata, split off a prerelease, compare numerically. - version = version.lstrip("v").split("+", 1)[0] + # Order Go module versions by SemVer rules. Two prefixes to shed: module + # versions carry "v" (v28.5.2), and stdlib findings carry "go" (go1.26.5) + # while the advisory's own ranges use a bare 1.26.5 -- comparing those + # unnormalized would match the wrong interval entirely. Build metadata + # (+incompatible) does not participate in precedence. + version = version.split("+", 1)[0] + if version.startswith("go"): + version = version[2:] + elif version.startswith("v"): + version = version[1:] main, _, pre = version.partition("-") - parts = [] + + release = [] for chunk in main.split("."): - parts.append(int(chunk) if chunk.isdigit() else 0) - while len(parts) < 3: - parts.append(0) - # No prerelease sorts above a prerelease of the same version. - return (parts, 0 if pre else 1, pre) + release.append(int(chunk) if chunk.isdigit() else 0) + while len(release) < 3: + release.append(0) + + # SemVer 11.4: dot-separated prerelease identifiers compare field by field; + # numeric ones compare numerically and rank below alphanumeric ones. String + # comparison would put beta.10 before beta.2. + identifiers = [] + for chunk in pre.split(".") if pre else []: + if chunk.isdigit(): + identifiers.append((0, int(chunk), "")) + else: + identifiers.append((1, 0, chunk)) + + # A version with no prerelease outranks the same release with one. + return (release, 0 if pre else 1, identifiers) def fix_for(osv_id, module_versions): # Only a fix on a module path we actually import counts: these advisories @@ -173,6 +192,14 @@ jobs: # A trailing introduced with no fixed is an open, unfixed interval. return out + # An accepted ID that no longer shows up is a suppression with nothing left to + # suppress -- dead config that would silently swallow the advisory if a future + # version reintroduced it. Worth saying, not worth failing a clean run over. + stale = sorted(accepted - set(reachable)) + if stale: + with open(os.environ["RUNNER_TEMP"] + "/stale-accepted.txt", "w") as handle: + handle.write("\n".join(stale) + "\n") + problems = [] for osv_id in sorted(reachable): if osv_id not in accepted: @@ -214,6 +241,18 @@ jobs: { echo '## govulncheck' echo + if [ -s "${RUNNER_TEMP}/stale-accepted.txt" ]; then + echo '### Accepted IDs no longer reported' + echo + echo 'These are in ACCEPTED but the scan no longer finds them -- most likely' + echo 'the dependency was upgraded. Drop them, so the list cannot silently' + echo 'swallow the advisory if a later version reintroduces it.' + echo + echo '```' + cat "${RUNNER_TEMP}/stale-accepted.txt" + echo '```' + echo + fi echo '```' cat "${RUNNER_TEMP}/report.txt" 2>/dev/null || echo '(no output -- the job failed before scanning)' echo '```' @@ -294,7 +333,11 @@ jobs: # $ENV.TITLE is expanded by jq, not by the shell -- that is the point of passing # the title as data instead of interpolating it, so SC2016 is expected here. # shellcheck disable=SC2016 - existing=$(TITLE="${title}" gh issue list --label govulncheck --state open --limit 30 --json number,title \ + # --limit bounds how many issues are fetched, and the jq filter only sees those. + # Failed dispatches on different refs deliberately open separate issues, so a + # small page could miss this ref's issue and open a duplicate. The label already + # narrows this to our own issues; the limit is just a sane ceiling above it. + existing=$(TITLE="${title}" gh issue list --label govulncheck --state open --limit 1000 --json number,title \ --jq '[.[] | select(.title == $ENV.TITLE)] | .[0].number // empty') if [ -n "${existing}" ]; then From 5f7f1099898f73e5ecbad307c3fc4e0742d7b8c3 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 14:53:28 -0700 Subject: [PATCH 07/12] Address review: close the tracking issue when the scan comes back clean Reporting only ran on failure, so the tracking state was stale in the other direction: once a vulnerability was fixed, or a transient setup failure cleared, the next run passed silently and the issue it had filed stayed open forever with nothing to say the scan was green again. The step now runs unless the job was cancelled. A clean run comments the result on the open issue for that ref and closes it; if none is open it does nothing. Failing runs behave as before. Verified all five paths against a mocked gh: clean with an issue closes it, clean without one is a no-op, vulnerable and error paths still comment on an existing issue or create one. --- .github/workflows/security.yml | 44 ++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 8e51946..38e8163 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -258,19 +258,30 @@ jobs: echo '```' } >> "$GITHUB_STEP_SUMMARY" - # Reports any failure, not just a failing scan: a broken checkout or a - # failed govulncheck install is also something nobody would otherwise see. + # Runs on success too, not just failure: a failing run leaves an issue open, + # and only a later clean run can close it. Skipped only on cancellation. - name: Report - if: failure() + if: ${{ !cancelled() }} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} OUTCOME: ${{ steps.scan.outputs.outcome }} + TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" SCANNED_REF: ${{ github.ref_name }} SCANNED_SHA: ${{ github.sha }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | + # --limit bounds how many issues are fetched, and the jq filter only sees those. + # Failed dispatches on different refs deliberately open separate issues, so a + # small page could miss this ref's issue and open a duplicate. The label already + # narrows this to our own issues; the limit is just a sane ceiling above it. + lookup() { + # shellcheck disable=SC2016 + TITLE="$1" gh issue list --label govulncheck --state open --limit 1000 --json number,title \ + --jq '[.[] | select(.title == $ENV.TITLE)] | .[0].number // empty' + } + # Always reflect THIS run into the tracking issue. An issue opened for an # earlier advisory says nothing about a new one, so bailing out when any issue # is open would recreate the silence this workflow exists to end. One issue, @@ -283,6 +294,26 @@ jobs: else title="govulncheck: findings on ${SCANNED_REF}" fi + existing=$(lookup "${title}") + + # A clean run has to close the loop as well. Otherwise the issue from a run that + # failed -- or from a vulnerability since fixed -- stays open forever and the + # tracking state is stale in the other direction. + if [ "${OUTCOME}" = "clean" ]; then + if [ -n "${existing}" ]; then + gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again on \`${SCANNED_REF}\` @ \`${SCANNED_SHA}\` (targets \`${TARGETS}\`). + + Run: ${RUN_URL} + + Closing; a later failure reopens tracking by filing a fresh issue." + gh issue close "${existing}" --reason completed + echo "Closed issue #${existing} -- scan is clean." + else + echo "Scan is clean and no tracking issue is open; nothing to do." + fi + exit 0 + fi + if [ -s "${RUNNER_TEMP}/report.txt" ]; then body_output=$(cat "${RUNNER_TEMP}/report.txt") else @@ -333,13 +364,6 @@ jobs: # $ENV.TITLE is expanded by jq, not by the shell -- that is the point of passing # the title as data instead of interpolating it, so SC2016 is expected here. # shellcheck disable=SC2016 - # --limit bounds how many issues are fetched, and the jq filter only sees those. - # Failed dispatches on different refs deliberately open separate issues, so a - # small page could miss this ref's issue and open a duplicate. The label already - # narrows this to our own issues; the limit is just a sane ceiling above it. - existing=$(TITLE="${title}" gh issue list --label govulncheck --state open --limit 1000 --json number,title \ - --jq '[.[] | select(.title == $ENV.TITLE)] | .[0].number // empty') - if [ -n "${existing}" ]; then gh issue comment "${existing}" --body-file "${RUNNER_TEMP}/body.md" echo "Updated issue #${existing}." From 38524a79fab840972688333dbe146c6f01e615b8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 14:57:07 -0700 Subject: [PATCH 08/12] Address review: per-ref concurrency, ref type in the key, bounded body The concurrency group was workflow-wide, but GitHub keeps only one run pending per group, so a third dispatch silently cancelled the second. Keyed by ref now, which is the actual unit of contention: each ref has its own tracking issue, so different refs never touch the same one and only same-ref runs need to queue. github.ref_name reduces a branch and a tag with the same short name to the same value, so their findings would have shared an issue. The full ref decides the key and tags are labelled as such. An issue body or comment is capped at 65536 characters. Twelve scans of verbose output can exceed that, and gh would fail the report entirely -- turning a large finding into no notification, the exact failure this workflow exists to prevent. Long output keeps its head and tail and says what was dropped. Verified with a mocked gh: branch main and tag main get separate issue titles, and a 90000-character report is truncated to well under the limit. --- .github/workflows/security.yml | 38 ++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 38e8163..310c1a9 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -11,9 +11,11 @@ on: permissions: {} # The reporting step reads then writes the tracking issue, which is not atomic. -# A manual dispatch overlapping the scheduled run could otherwise file two. +# Keyed by ref because that is the unit of contention: each ref has its own +# tracking issue, so runs on different refs never touch the same one, and only +# same-ref runs -- which scan the same tree -- need to queue behind each other. concurrency: - group: ${{ github.workflow }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: false jobs: @@ -269,6 +271,9 @@ jobs: OUTCOME: ${{ steps.scan.outputs.outcome }} TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" SCANNED_REF: ${{ github.ref_name }} + # A branch and a tag can share a short name; the full ref keeps their + # tracking issues apart. + SCANNED_FULL_REF: ${{ github.ref }} SCANNED_SHA: ${{ github.sha }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | @@ -289,10 +294,14 @@ jobs: # A workflow_dispatch can target any ref, so never describe a run as being the # default branch without checking. Non-default refs get their own issue rather # than appending a branch-only finding to the canonical one. - if [ "${SCANNED_REF}" = "${DEFAULT_BRANCH}" ]; then + case "${SCANNED_FULL_REF}" in + refs/tags/*) ref_label="tag ${SCANNED_REF}" ;; + *) ref_label="${SCANNED_REF}" ;; + esac + if [ "${SCANNED_FULL_REF}" = "refs/heads/${DEFAULT_BRANCH}" ]; then title="govulncheck: findings on ${DEFAULT_BRANCH}" else - title="govulncheck: findings on ${SCANNED_REF}" + title="govulncheck: findings on ${ref_label}" fi existing=$(lookup "${title}") @@ -301,7 +310,7 @@ jobs: # tracking state is stale in the other direction. if [ "${OUTCOME}" = "clean" ]; then if [ -n "${existing}" ]; then - gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again on \`${SCANNED_REF}\` @ \`${SCANNED_SHA}\` (targets \`${TARGETS}\`). + gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again on \`${SCANNED_FULL_REF}\` @ \`${SCANNED_SHA}\` (targets \`${TARGETS}\`). Run: ${RUN_URL} @@ -314,10 +323,21 @@ jobs: exit 0 fi - if [ -s "${RUNNER_TEMP}/report.txt" ]; then - body_output=$(cat "${RUNNER_TEMP}/report.txt") - else + # GitHub caps an issue body or comment at 65536 characters. A dozen scans of + # verbose output can pass that, and gh would fail the whole report -- turning a + # large finding into no notification at all, which is the failure this workflow + # exists to prevent. Keep the head and tail and say what was dropped. + limit=45000 + if [ ! -s "${RUNNER_TEMP}/report.txt" ]; then body_output="(govulncheck produced no output -- the job failed before or during the scan; see the run log.)" + elif [ "$(wc -c < "${RUNNER_TEMP}/report.txt")" -gt "${limit}" ]; then + body_output="$(head -c 22000 "${RUNNER_TEMP}/report.txt") + + [... truncated to fit GitHub's 65536-character limit -- full output in the run log ...] + + $(tail -c 22000 "${RUNNER_TEMP}/report.txt")" + else + body_output=$(cat "${RUNNER_TEMP}/report.txt") fi if [ "${OUTCOME}" = "vulnerable-and-error" ]; then @@ -333,7 +353,7 @@ jobs: { echo "${headline}" echo - echo "Ref: \`${SCANNED_REF}\` @ \`${SCANNED_SHA}\`" + echo "Ref: \`${SCANNED_FULL_REF}\` @ \`${SCANNED_SHA}\`" echo "Targets: \`${TARGETS}\`" echo "Run: ${RUN_URL}" if [ -s "${RUNNER_TEMP}/new.txt" ]; then From 2cf9de353cf24c19113f19704831ecda22cac4d1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 16:19:47 -0700 Subject: [PATCH 09/12] Scan the default branch always, and delete the per-ref machinery The last two review rounds were all about the same thing: a branch and a tag colliding in the tracking key, a dispatch on a feature branch appending to the canonical issue, per-ref concurrency, queued dispatches being dropped. Every one of those existed to support scanning a non-default ref -- a capability nobody asked for. Scheduled runs only ever run on the default branch, and the default branch is what we ship. So checkout now pins to it and workflow_dispatch means run it now, not run it here. That removes the ref-type case statement, the per-ref titles, the per-ref concurrency key and the full-ref plumbing, and makes the workflow-level concurrency group correct rather than a compromise: every run scans the same tree, so a superseded queued run was redundant. One repo, one tracking issue, one fixed title. The commit actually scanned is recorded and reported, so pinning the ref does not cost traceability. Verified all seven report paths against a mocked gh -- clean/vulnerable/ error/vulnerable-and-error/pre-scan-failure, with and without an open issue -- plus truncation of a 90000-character report, and an end-to-end scan of 2 targets x 3 modules. --- .github/workflows/security.yml | 79 +++++++++++++--------------------- 1 file changed, 29 insertions(+), 50 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 310c1a9..6621f65 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -10,12 +10,11 @@ on: permissions: {} -# The reporting step reads then writes the tracking issue, which is not atomic. -# Keyed by ref because that is the unit of contention: each ref has its own -# tracking issue, so runs on different refs never touch the same one, and only -# same-ref runs -- which scan the same tree -- need to queue behind each other. +# The reporting step reads then writes the one tracking issue, which is not +# atomic, so runs must not overlap. Every run scans the same tree, so a queued +# run that gets superseded was redundant anyway. concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }} cancel-in-progress: false jobs: @@ -52,10 +51,18 @@ jobs: # honestly. Revisit when moby/moby/v2 is stable. ACCEPTED: "GO-2026-4887 GO-2026-4883" steps: + # Always scan the default branch, including on workflow_dispatch. What we + # ship is what needs scanning, and pinning it here is what keeps this a + # single tracking issue rather than one per dispatched ref. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.event.repository.default_branch }} persist-credentials: false + - name: Record the scanned commit + id: commit + run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: go-version-file: go.mod @@ -270,51 +277,31 @@ jobs: RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} OUTCOME: ${{ steps.scan.outputs.outcome }} TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" - SCANNED_REF: ${{ github.ref_name }} - # A branch and a tag can share a short name; the full ref keeps their - # tracking issues apart. - SCANNED_FULL_REF: ${{ github.ref }} - SCANNED_SHA: ${{ github.sha }} + SCANNED_SHA: ${{ steps.commit.outputs.sha }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | - # --limit bounds how many issues are fetched, and the jq filter only sees those. - # Failed dispatches on different refs deliberately open separate issues, so a - # small page could miss this ref's issue and open a duplicate. The label already - # narrows this to our own issues; the limit is just a sane ceiling above it. + # One tracking issue per repo. The scan always runs against the default branch, + # so there is exactly one thing to track and the title is fixed. + title="govulncheck: findings on ${DEFAULT_BRANCH}" + + # shellcheck disable=SC2016 lookup() { - # shellcheck disable=SC2016 - TITLE="$1" gh issue list --label govulncheck --state open --limit 1000 --json number,title \ + TITLE="${title}" gh issue list --label govulncheck --state open --limit 1000 --json number,title \ --jq '[.[] | select(.title == $ENV.TITLE)] | .[0].number // empty' } - # Always reflect THIS run into the tracking issue. An issue opened for an - # earlier advisory says nothing about a new one, so bailing out when any issue - # is open would recreate the silence this workflow exists to end. One issue, - # kept current, with each run appended. - # A workflow_dispatch can target any ref, so never describe a run as being the - # default branch without checking. Non-default refs get their own issue rather - # than appending a branch-only finding to the canonical one. - case "${SCANNED_FULL_REF}" in - refs/tags/*) ref_label="tag ${SCANNED_REF}" ;; - *) ref_label="${SCANNED_REF}" ;; - esac - if [ "${SCANNED_FULL_REF}" = "refs/heads/${DEFAULT_BRANCH}" ]; then - title="govulncheck: findings on ${DEFAULT_BRANCH}" - else - title="govulncheck: findings on ${ref_label}" - fi - existing=$(lookup "${title}") + existing=$(lookup) - # A clean run has to close the loop as well. Otherwise the issue from a run that - # failed -- or from a vulnerability since fixed -- stays open forever and the + # A clean run has to close the loop too. Otherwise the issue from a run that + # failed -- or from a vulnerability since fixed -- stays open forever, and the # tracking state is stale in the other direction. if [ "${OUTCOME}" = "clean" ]; then if [ -n "${existing}" ]; then - gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again on \`${SCANNED_FULL_REF}\` @ \`${SCANNED_SHA}\` (targets \`${TARGETS}\`). + gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again at \`${SCANNED_SHA}\` (targets \`${TARGETS}\`). Run: ${RUN_URL} - Closing; a later failure reopens tracking by filing a fresh issue." + Closing; a later failure files a fresh issue." gh issue close "${existing}" --reason completed echo "Closed issue #${existing} -- scan is clean." else @@ -324,13 +311,12 @@ jobs: fi # GitHub caps an issue body or comment at 65536 characters. A dozen scans of - # verbose output can pass that, and gh would fail the whole report -- turning a - # large finding into no notification at all, which is the failure this workflow - # exists to prevent. Keep the head and tail and say what was dropped. - limit=45000 + # verbose output can pass that, and gh would fail the report entirely -- turning + # a large finding into no notification, the exact failure this exists to + # prevent. Keep the head and tail and say what was dropped. if [ ! -s "${RUNNER_TEMP}/report.txt" ]; then body_output="(govulncheck produced no output -- the job failed before or during the scan; see the run log.)" - elif [ "$(wc -c < "${RUNNER_TEMP}/report.txt")" -gt "${limit}" ]; then + elif [ "$(wc -c < "${RUNNER_TEMP}/report.txt")" -gt 45000 ]; then body_output="$(head -c 22000 "${RUNNER_TEMP}/report.txt") [... truncated to fit GitHub's 65536-character limit -- full output in the run log ...] @@ -353,7 +339,7 @@ jobs: { echo "${headline}" echo - echo "Ref: \`${SCANNED_FULL_REF}\` @ \`${SCANNED_SHA}\`" + echo "Commit: \`${SCANNED_SHA}\`" echo "Targets: \`${TARGETS}\`" echo "Run: ${RUN_URL}" if [ -s "${RUNNER_TEMP}/new.txt" ]; then @@ -377,13 +363,6 @@ jobs: echo "An entry marked \"accepted, but now fixed\" means an ID in this workflow's ACCEPTED list has gained a fix for a module path we import: upgrade and drop the ID, rather than re-accepting it." } > "${RUNNER_TEMP}/body.md" - # Let a failure of the lookup itself stop the job rather than read as "no issue - # open", which would file a duplicate. - # The title carries a ref name, which git allows to contain quotes -- pass it to - # jq as data via $ENV rather than interpolating it into the jq program. - # $ENV.TITLE is expanded by jq, not by the shell -- that is the point of passing - # the title as data instead of interpolating it, so SC2016 is expected here. - # shellcheck disable=SC2016 if [ -n "${existing}" ]; then gh issue comment "${existing}" --body-file "${RUNNER_TEMP}/body.md" echo "Updated issue #${existing}." From 36d8eeb466c342215966a5c267f4e8ac3e8e7c77 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 16:29:05 -0700 Subject: [PATCH 10/12] Find the tracking issue by its label, and let only the canonical run own it Matching the issue by an exact title meant a default-branch rename, or anyone editing the title, orphaned it: the next failure opened a second issue and a clean run closed neither. Since there is exactly one issue, its own label identifies it. That also deletes the $ENV.TITLE jq machinery and its shellcheck suppression -- both of which existed only to compare a title safely. Pinning checkout to the default branch pins the source tree, not the workflow definition: GitHub loads that from the dispatched ref. An experimental branch could therefore scan default-branch code with its own ACCEPTED list and write to, or close, the real tracking issue. The report step now checks it is the canonical run first. Dispatching from a branch still works and still scans -- it reports to the run summary rather than the issue, which keeps this workflow testable without letting a draft of it speak for the repository. Verified six paths against a mocked gh, including dispatch from a branch and from a tag. --- .github/workflows/security.yml | 39 +++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 6621f65..5ab0b4d 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -51,9 +51,12 @@ jobs: # honestly. Revisit when moby/moby/v2 is stable. ACCEPTED: "GO-2026-4887 GO-2026-4883" steps: - # Always scan the default branch, including on workflow_dispatch. What we - # ship is what needs scanning, and pinning it here is what keeps this a - # single tracking issue rather than one per dispatched ref. + # Always scan the default branch, including on workflow_dispatch: what we + # ship is what needs scanning, and pinning it keeps this to one tracking + # issue rather than one per dispatched ref. Note this pins the source tree + # only -- GitHub loads the workflow definition itself from the dispatched + # ref, which is why the reporting step checks it is the canonical run + # before touching the issue. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ github.event.repository.default_branch }} @@ -279,18 +282,24 @@ jobs: TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" SCANNED_SHA: ${{ steps.commit.outputs.sha }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + GITHUB_REF: ${{ github.ref }} run: | - # One tracking issue per repo. The scan always runs against the default branch, - # so there is exactly one thing to track and the title is fixed. - title="govulncheck: findings on ${DEFAULT_BRANCH}" - - # shellcheck disable=SC2016 - lookup() { - TITLE="${title}" gh issue list --label govulncheck --state open --limit 1000 --json number,title \ - --jq '[.[] | select(.title == $ENV.TITLE)] | .[0].number // empty' - } - - existing=$(lookup) + # The scan always runs against the default branch, so there is exactly one thing + # to track: one issue, found by its own label. Matching on the title instead + # would break the moment the default branch is renamed or someone edits the + # title, and would silently open a second issue. + existing=$(gh issue list --label govulncheck --state open --limit 1 --json number --jq '.[0].number // empty') + + # Only the canonical run owns that issue. workflow_dispatch pins the *source + # tree* to the default branch but not the workflow definition, which GitHub + # loads from the dispatched ref -- so a branch carrying an experimental ACCEPTED + # list or reporting change would otherwise write to, or close, the real issue. + # Dispatching from a branch stays useful for trying this workflow out; it just + # reports to the run summary instead. + if [ "${GITHUB_REF}" != "refs/heads/${DEFAULT_BRANCH}" ]; then + echo "Running from ${GITHUB_REF}, not the default branch -- results are in the run summary; the tracking issue is left alone." + exit 0 + fi # A clean run has to close the loop too. Otherwise the issue from a run that # failed -- or from a vulnerability since fixed -- stays open forever, and the @@ -368,5 +377,5 @@ jobs: echo "Updated issue #${existing}." else gh label create govulncheck --description "Reported by the scheduled govulncheck run" --color d93f0b --force - gh issue create --title "${title}" --label govulncheck --body-file "${RUNNER_TEMP}/body.md" + gh issue create --title "govulncheck: findings on the default branch" --label govulncheck --body-file "${RUNNER_TEMP}/body.md" fi From a36c1c816b93db96649d9a5751fdfa6527dd716a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 16:35:48 -0700 Subject: [PATCH 11/12] Scan each module for the platforms that module actually ships on A single target list crossed with every module was wrong in both directions. once/installer ships as a container -- linux only -- so darwin findings against it were unactionable noise, while integration/testapp is a net/http fixture used by the integration tests and is not shipped at all. Scanning it could only produce findings for code no user runs. SCAN_MATRIX now carries one line per module: the directory, then the targets that module ships on. once drops the test fixture and scans the root on four platforms and installer on two, so 12 scans become 6 and every one of them corresponds to something real. One setup-go installs one toolchain, from the root go.mod, so a nested module declaring a different go directive would have been scanned against the wrong standard library and reported a confidently wrong answer. Rather than build per-module toolchains for a divergence that does not exist today, the scan asserts they agree and fails loudly if they ever stop agreeing. Dropped the stale-accepted-ID note. It only ever wrote to the summary of a run that was otherwise green, and nobody reads those -- the same objection I raised in favour of the expiry check, which applies here too. The security-relevant case, an accepted ID gaining a fix, still fails the run. --- .github/workflows/security.yml | 73 +++++++++++++++++----------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 5ab0b4d..e2e11b3 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -25,11 +25,13 @@ jobs: contents: read issues: write env: - # ./... does not cross module boundaries, so every module is listed. - MODULES: ". installer integration/testapp" - # govulncheck's source mode only analyses the files the current build - # configuration selects, so scan each GOOS/GOARCH we actually release. - TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" + # One line per module: the module directory, then the GOOS/GOARCH targets + # that module actually ships on. ./... does not cross module boundaries, + # so every shipped module is listed; the targets differ per module because + # what each one ships on differs. + SCAN_MATRIX: | + . linux/amd64 linux/arm64 darwin/amd64 darwin/arm64 + installer linux/amd64 linux/arm64 # Findings we have looked at and are choosing to carry, so the run stays # quiet until something NEW appears. Everything else fails the job. # @@ -81,16 +83,33 @@ jobs: rm -f "${RUNNER_TEMP}"/gv-*.json n=0 scan_error=0 - # Releases are built CGO_ENABLED=0 for each GOOS/GOARCH in TARGETS. govulncheck's - # source mode only sees the files the current build configuration selects, so a - # single native run would miss a path reachable only on another target. + toolchain=$(go env GOVERSION) + + # Releases are built CGO_ENABLED=0, and govulncheck's source mode only sees the + # files the current build configuration selects -- so each module is scanned for + # the platforms that module actually ships on. They are not the same list: a + # container-only artifact never runs on darwin, and reporting a darwin-only path + # against it would be a finding nobody can act on. export CGO_ENABLED=0 - for target in ${TARGETS}; do - GOOS="${target%%/*}" - GOARCH="${target##*/}" - export GOOS GOARCH - for module in ${MODULES}; do + while read -r module targets; do + [ -z "${module}" ] && continue + + # One setup-go installs one toolchain, from the root go.mod. If a nested + # module asks for a different one, this would silently scan it against the + # wrong standard library -- so say so rather than report a wrong answer. + want=$(awk '/^go /{print "go" $2; exit}' "${module}/go.mod") + if [ "${want}" != "${toolchain}" ]; then + scan_error=1 + echo "===== ${module}" >> "${RUNNER_TEMP}/report.txt" + echo "module declares ${want} but the scan is running ${toolchain}; align the go directives or give this module its own scan job." >> "${RUNNER_TEMP}/report.txt" + continue + fi + + for target in ${targets}; do n=$((n + 1)) + GOOS="${target%%/*}" + GOARCH="${target##*/}" + export GOOS GOARCH echo "===== ${target} ${module}" >> "${RUNNER_TEMP}/report.txt" ( cd "${module}" && govulncheck -format json ./... ) > "${RUNNER_TEMP}/gv-${n}.json" 2> "${RUNNER_TEMP}/gv-${n}.err" rc=$? @@ -104,7 +123,9 @@ jobs: fi ( cd "${module}" && govulncheck ./... ) >> "${RUNNER_TEMP}/report.txt" 2>&1 done - done + done </dev/null || echo '(no output -- the job failed before scanning)' echo '```' @@ -279,7 +280,6 @@ jobs: GH_REPO: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} OUTCOME: ${{ steps.scan.outputs.outcome }} - TARGETS: "linux/amd64 linux/arm64 darwin/amd64 darwin/arm64" SCANNED_SHA: ${{ steps.commit.outputs.sha }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} GITHUB_REF: ${{ github.ref }} @@ -306,7 +306,7 @@ jobs: # tracking state is stale in the other direction. if [ "${OUTCOME}" = "clean" ]; then if [ -n "${existing}" ]; then - gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again at \`${SCANNED_SHA}\` (targets \`${TARGETS}\`). + gh issue comment "${existing}" --body "The scheduled \`govulncheck\` run is clean again at \`${SCANNED_SHA}\`. Run: ${RUN_URL} @@ -349,7 +349,6 @@ jobs: echo "${headline}" echo echo "Commit: \`${SCANNED_SHA}\`" - echo "Targets: \`${TARGETS}\`" echo "Run: ${RUN_URL}" if [ -s "${RUNNER_TEMP}/new.txt" ]; then echo From 7b1636f5105c2ecf13991ae48c376882ef183f0a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 20 Aug 2026 16:39:47 -0700 Subject: [PATCH 12/12] Let checkout pin the triggering SHA instead of naming a branch Naming the default branch checks out whatever the tip is when the job runs, while GitHub executes the workflow definition from the commit that triggered it. A push that adds a module together with its SCAN_MATRIX entry could therefore be scanned with the previous matrix, report clean, and close the tracking issue without ever covering the new artifact. Given no ref, checkout uses github.context.sha -- the triggering commit -- so the tree and the definition are always the same commit. The fix is deleting the line I added, not adding another. Scheduled runs still only trigger on the default branch, which is what we ship. A dispatch from elsewhere now scans its own ref, which is what you want when testing a change to this workflow, and the reporting step already declines to touch the tracking issue for non-canonical runs. --- .github/workflows/security.yml | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index e2e11b3..b068e87 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -53,15 +53,17 @@ jobs: # honestly. Revisit when moby/moby/v2 is stable. ACCEPTED: "GO-2026-4887 GO-2026-4883" steps: - # Always scan the default branch, including on workflow_dispatch: what we - # ship is what needs scanning, and pinning it keeps this to one tracking - # issue rather than one per dispatched ref. Note this pins the source tree - # only -- GitHub loads the workflow definition itself from the dispatched - # ref, which is why the reporting step checks it is the canonical run - # before touching the issue. + # No ref: on purpose. With none given, checkout pins to the SHA that + # triggered the run, so the tree and the workflow definition are the same + # commit -- a push that adds a module and its SCAN_MATRIX entry together + # can never be scanned with the previous matrix. Naming the branch instead + # would follow the moving tip and reintroduce exactly that gap. + # + # Scheduled runs only ever trigger on the default branch, which is what we + # ship. A workflow_dispatch from elsewhere scans its own ref, and the + # reporting step declines to touch the tracking issue for those. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ github.event.repository.default_branch }} persist-credentials: false - name: Record the scanned commit