From f7bce7d01d770d0d636083c64490b98c95adb2a8 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 14 Aug 2026 11:33:25 +0200 Subject: [PATCH 1/2] ci(vulncheck): one declaration for the govulncheck pin, and a check that keeps it (backend#1972) The version lived in THREE places -- build.yml's job, vulncheck.yml's job, and GOVULNCHECK_VERSION in the Makefile -- held in step by a comment reading "keep the job in lockstep". All three happened to read v1.1.4, which is exactly what made it look fine. Two copies kept in sync by a request is not a mechanism. Both workflows now run `make vulncheck`, so the Makefile is the single declaration, and a green `make ci` locally cannot disagree with the PR gate about which govulncheck ran -- the same reasoning already applied to lint-full/GOLANGCI_LINT_VERSION. The "keep in lockstep" comment is replaced by a machine check rather than deleted: scripts/check-tool-pins.sh PARSES GOVULNCHECK_VERSION out of the Makefile and fails the Lint job if any workflow hardcodes that module with an @version. The guard holds no version of its own, so it cannot agree with itself while disagreeing with reality. Mutation-proved, anchors asserted: reintroduce `go install ...govulncheck@v1.1.4` in build.yml -> exit 1, names file and line delete GOVULNCHECK_VERSION from the Makefile -> exit 2, refuses to report clean run from an unrelated cwd -> still correct (cd's to its root) The two jobs still differ, deliberately, and now ONLY in the ref: vulncheck.yml checks out develop on a schedule, build.yml judges the PR head. That difference is stated in the file instead of being buried in duplicated steps. DRAFT until cli#500 lands. `govulncheck` is already red on develop -- go1.26.5 carries 4 reachable stdlib CVEs -- so this branch inherits that failure. Verified locally that the folded target reproduces it exactly: make vulncheck -> exit status 3, "affected by 4 vulnerabilities from the Go standard library", each "Fixed in: @go1.26.6" which also independently confirms backend#1972's counterfactual. This PR does not change WHAT govulncheck finds, only where its version is declared. NOT in this PR, and both belong to the ticket rather than here: adding govulncheck to `main`'s required contexts (needs #500 on main first, plus the break-glass decision -- enforce_admins is true there and skip-fr-gate does not bypass a required check), and the same three-copies shape for GOLANGCI_LINT_VERSION, which TOOLS in the new guard is structured to take as a second row. The two actionlint SC2001 findings on build.yml are pre-existing on develop (verified against the unmodified base) and untouched here. Co-Authored-By: Claude Opus 5 --- .github/workflows/build.yml | 25 +++++++---- .github/workflows/vulncheck.yml | 14 ++++--- Makefile | 6 +++ scripts/check-tool-pins.sh | 73 +++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 12 deletions(-) create mode 100755 scripts/check-tool-pins.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2bdd1cf..1b3381f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -52,6 +52,7 @@ jobs: sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck dash shellcheck --shell=sh --severity=error scripts/install.sh shellcheck --shell=bash --severity=error scripts/check-style.sh + shellcheck --shell=bash --severity=error scripts/check-tool-pins.sh dash -n scripts/install.sh bash -n scripts/tests/install-verify.sh - name: Verification harness (mandatory cosign / fail-closed) @@ -188,16 +189,25 @@ jobs: # checks only — role/wording judgement stays with review. run: bash scripts/check-style.sh + - name: Tool-pin guard (one declaration per pinned tool) + # backend#1972: fails if a workflow restates a tool version the Makefile + # already declares. govulncheck's pin lived in three places, kept in step + # by a comment; this is that comment turned into a check. + run: bash scripts/check-tool-pins.sh + govulncheck: timeout-minutes: 10 name: govulncheck # Reachability-scans the module for known vulnerabilities (stdlib + # deps) on every PR and push. This is a customer-installed binary — # 6 reachable CVEs shipped in v0.8.0 before this gate existed (#276). - # Pinned version, same rationale as the lint tools above; keep in - # lockstep with GOVULNCHECK_VERSION in the Makefile and the copy of - # this job in vulncheck.yml (the weekly cron that catches CVEs - # published between PRs). Bump deliberately. + # + # backend#1972: the version is NOT restated here. It lived in three + # places — this job, vulncheck.yml, and GOVULNCHECK_VERSION in the + # Makefile — held in lockstep by a comment asking people to remember. + # All three now resolve to the Makefile declaration, because `make + # vulncheck` is what runs. scripts/check-tool-pins.sh fails the Lint + # job if a literal pin creeps back in. runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -209,9 +219,10 @@ jobs: cache: true - name: govulncheck ./... - run: | - go install golang.org/x/vuln/cmd/govulncheck@v1.1.4 - govulncheck ./... + # `make vulncheck`, not a restated `go install ...@version`: a green + # `make ci` locally and this gate can then never disagree about which + # govulncheck ran. Same reasoning as lint-full/GOLANGCI_LINT_VERSION. + run: make vulncheck build: timeout-minutes: 20 diff --git a/.github/workflows/vulncheck.yml b/.github/workflows/vulncheck.yml index 0a261b5..668b685 100644 --- a/.github/workflows/vulncheck.yml +++ b/.github/workflows/vulncheck.yml @@ -14,8 +14,14 @@ name: Vulncheck # new CVE needs to surface first — main gets the same signal on its # next promotion via build.yml's copy. # -# Keep the job in lockstep with build.yml's govulncheck job and -# GOVULNCHECK_VERSION in the Makefile. Bump deliberately. +# backend#1972: this job and build.yml's no longer hold their own copy of the +# pinned version. Both run `make vulncheck`, so GOVULNCHECK_VERSION in the +# Makefile is the single declaration and the "keep in lockstep" instruction +# this comment used to carry is now a machine check +# (scripts/check-tool-pins.sh, in the Lint job). +# +# The two jobs still differ, deliberately, and it is only the REF: this one +# checks out develop on a schedule, build.yml judges the PR head. on: schedule: @@ -44,6 +50,4 @@ jobs: cache: true - name: govulncheck ./... - run: | - go install golang.org/x/vuln/cmd/govulncheck@v1.1.4 - govulncheck ./... + run: make vulncheck diff --git a/Makefile b/Makefile index c658197..72b4fb4 100644 --- a/Makefile +++ b/Makefile @@ -237,6 +237,12 @@ deadcode: # reachable vulns before this gate existed (#276). Mirrors the govulncheck # job in build.yml (PR gate) and vulncheck.yml (weekly cron on develop). # Needs network for the vuln DB (https://vuln.go.dev), like schema-check. +# backend#1972: asserts no workflow restates a version this Makefile declares. +# Runs in the Lint job beside check-style.sh. +.PHONY: check-tool-pins +check-tool-pins: + bash scripts/check-tool-pins.sh + .PHONY: vulncheck vulncheck: $(GO) run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./... diff --git a/scripts/check-tool-pins.sh b/scripts/check-tool-pins.sh new file mode 100755 index 0000000..e852b18 --- /dev/null +++ b/scripts/check-tool-pins.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# ============================================================================= +# check-tool-pins.sh — one declaration per pinned tool version (backend#1972) +# +# govulncheck's version used to live in THREE places: build.yml's job, +# vulncheck.yml's job, and GOVULNCHECK_VERSION in the Makefile — kept in step by +# a comment reading "keep the job in lockstep". Two copies held in sync by a +# request is not a mechanism; all three happened to read v1.1.4, which is +# exactly what made it look fine. +# +# Both workflows now run `make vulncheck`, so the Makefile is the declaration. +# This guard exists so that stays true: it PARSES the Makefile for the tools it +# covers and fails if a workflow hardcodes a version for one of them. +# +# DERIVED, NOT RESTATED: the version to look for is read from the Makefile. This +# guard holds no version of its own, so it cannot agree with itself while +# disagreeing with reality. +# +# Runs in CI (the Lint job, beside check-style.sh) and locally: +# make check-tool-pins (or: bash scripts/check-tool-pins.sh) +# Exit 0 = clean, 1 = a restated pin was found, 2 = the guard itself errored. +# ============================================================================= +set -uo pipefail +cd "$(dirname "$0")/.." || exit 2 + +# Fail CLOSED. A guard that cannot find its inputs must not report clean: that is +# the failure this file was written against (backend#1729). +[[ -f Makefile ]] || { echo "check-tool-pins: no Makefile — refusing to report clean" >&2; exit 2; } +[[ -d .github/workflows ]] || { echo "check-tool-pins: no .github/workflows — refusing to report clean" >&2; exit 2; } + +# Tools whose version the Makefile owns, as :. +# Add a row when a tool moves to a `make` target that CI calls. +TOOLS=( + "GOVULNCHECK_VERSION:golang.org/x/vuln/cmd/govulncheck" +) + +fail=0 +checked=0 + +for row in "${TOOLS[@]}"; do + var="${row%%:*}" + module="${row#*:}" + + # Parse the REAL declaration. `?=` or `=`, any surrounding spaces. + version="$(sed -nE "s/^[[:space:]]*${var}[[:space:]]*\\??=[[:space:]]*([^[:space:]#]+).*/\\1/p" Makefile | head -1)" + if [[ -z "$version" ]]; then + echo "check-tool-pins: ${var} is not declared in the Makefile, so this guard cannot" >&2 + echo " verify anything about ${module}. Either restore the declaration or drop the" >&2 + echo " row from TOOLS — an unparseable input is a finding, not a pass." >&2 + exit 2 + fi + + # Any workflow naming the module with an @version is holding its own copy. + # `make ` references carry no version and are therefore invisible here, + # which is the whole point. + offenders="$(grep -rn -- "${module}@" .github/workflows/ 2>/dev/null || true)" + if [[ -n "$offenders" ]]; then + echo "A workflow pins ${module} directly:" >&2 + printf '%s\n' "$offenders" >&2 + echo >&2 + echo " ${var} in the Makefile already declares this (${version}), and CI runs it" >&2 + echo " via a make target. A second copy here is what backend#1972 removed: three" >&2 + echo " copies agreeing today, drifting on the next bump, with nothing to notice." >&2 + echo " Call the make target instead." >&2 + fail=1 + fi + checked=$((checked + 1)) +done + +if (( fail )); then + exit 1 +fi +echo "check-tool-pins: ${checked} tool pin(s) declared once, in the Makefile" From 1144759def52baee20b5f2611c838e16377ab4af Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 14 Aug 2026 12:21:11 +0200 Subject: [PATCH 2/2] ci(vulncheck): run the pin guard in make ci/check + fail it closed on grep errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Bugbot findings on the govulncheck single-declaration change (backend#1972): - check-tool-pins ran only in the CI Lint job, so a green local `make ci` / `make check` could disagree with the PR about a restated pin — the exact local/CI divergence the Makefile-as-entry-point exists to prevent. Add it to both aggregate targets, beside check-style/file-budget. - The workflow scan used `grep ... 2>/dev/null || true`, laundering a real grep error (rc>=2: unreadable tree, bad invocation) into an empty hit list and an unearned clean pass — the fail-open this guard was written against. Capture rc and fail closed on rc>=2, matching scan() in check-style.sh. Co-Authored-By: Claude Opus 4.8 --- Makefile | 4 ++-- scripts/check-tool-pins.sh | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 72b4fb4..1600354 100644 --- a/Makefile +++ b/Makefile @@ -50,7 +50,7 @@ help: # * schema-check — fetches data-ingestors at the pinned ref. # * deadcode — another `go run tool@version` fetch. .PHONY: check -check: vet test-fast fmt-check file-budget check-style +check: vet test-fast fmt-check file-budget check-style check-tool-pins @echo "==> check: green (run 'make check-all' for the full CI set)" # check-all: the full PR gate. `ci` is the original name and stays — @@ -132,7 +132,7 @@ GOIMPORTS_VERSION ?= v0.48.0 # which fails on findings since #430. A green `make ci` must imply a green # PR; lint-full's own guard tells you how to install the tool if missing. .PHONY: ci -ci: vet test lint lint-full fmt-check schema-check vulncheck file-budget deadcode check-style +ci: vet test lint lint-full fmt-check schema-check vulncheck file-budget deadcode check-style check-tool-pins @echo "==> ci: all green" .PHONY: build diff --git a/scripts/check-tool-pins.sh b/scripts/check-tool-pins.sh index e852b18..778b1d2 100755 --- a/scripts/check-tool-pins.sh +++ b/scripts/check-tool-pins.sh @@ -53,7 +53,17 @@ for row in "${TOOLS[@]}"; do # Any workflow naming the module with an @version is holding its own copy. # `make ` references carry no version and are therefore invisible here, # which is the whole point. - offenders="$(grep -rn -- "${module}@" .github/workflows/ 2>/dev/null || true)" + # + # No 2>/dev/null and no `|| true`: grep rc 1 is "no offender" and fine, but rc>=2 + # is a real error (unreadable tree, bad invocation) and must fail CLOSED. Laundering + # it into an empty hit list is the unearned exit 0 this guard was written against + # (backend#1729); scan() in check-style.sh handles the same grep class this way. + offenders="$(grep -rn -- "${module}@" .github/workflows/)" + rc=$? + if (( rc >= 2 )); then + echo "check-tool-pins: grep errored (rc=${rc}) scanning .github/workflows for '${module}@' — refusing to report clean" >&2 + exit 2 + fi if [[ -n "$offenders" ]]; then echo "A workflow pins ${module} directly:" >&2 printf '%s\n' "$offenders" >&2