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..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 @@ -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..778b1d2 --- /dev/null +++ b/scripts/check-tool-pins.sh @@ -0,0 +1,83 @@ +#!/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. + # + # 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 + 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"