Skip to content

Commit 3d4e235

Browse files
jeremyjeremy
authored
Lint the release the same way we lint everything else (#625)
* Lint the release the same way we lint everything else release.yml pinned golangci-lint at v2.9.0 while test.yml and security.yml had moved to v2.11.1. Nothing noticed, because the release gate is the one job no pull request runs. It surfaced as v0.9.0-rc.1 failing, on this: internal/commands/assignments.go:142:85: G115: integer overflow conversion int -> int32 (gosec) That conversion is guarded two lines above it — `position < 1 || position > math.MaxInt32` returns before the cast. gosec at v2.11.1 follows the bound and stays quiet; at v2.9.0 it does not. So the finding was a false positive that every PR check had already passed, and the only thing it blocked was a release tag. The pin is now in lockstep, and scripts/check-lint-lockstep.sh enforces that rather than trusting the "keep in lockstep" comments that were already there and had already failed. It reads the version off every golangci-lint-action step and fails when they disagree, naming the files: FAIL: golangci-lint pins disagree across workflows: .github/workflows/release.yml:v2.9.0 .github/workflows/security.yml:v2.11.1 .github/workflows/test.yml:v2.11.1 Wired into `make check`, the release gate, and test.yml ahead of the linter itself, so drift is reported as drift instead of as a version-specific finding in whichever job happens to be the odd one out. No change to assignments.go: the code was already correct. * Fail on a golangci-lint step that pins nothing The first version of this check parsed pins and compared them, so a step that declared no version was skipped rather than caught: delete release.yml's version line and the check reported success across the remaining two workflows. That is the same hole in a different shape. An unpinned step resolves to whatever golangci-lint-action defaults to, which moves on its own schedule and puts the release gate back on a version nothing else runs. Now every golangci-lint-action step must carry a version. Verified in all four states: agreeing pins pass, a drifted pin fails naming the files, a deleted version line fails as UNPINNED, and restoring it passes again.
1 parent 7aac5fd commit 3d4e235

4 files changed

Lines changed: 84 additions & 3 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ jobs:
5757
- name: Install golangci-lint
5858
uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0
5959
with:
60-
version: v2.9.0
60+
version: v2.11.1 # keep in lockstep with test.yml Lint
6161
install-only: true
6262

6363
- name: Cache BATS
@@ -79,7 +79,7 @@ jobs:
7979

8080
- name: Run release quality gate
8181
run: |
82-
make fmt-check vet lint test test-e2e provenance-check check-naming check-surface
82+
make check-lint-lockstep fmt-check vet lint test test-e2e provenance-check check-naming check-surface
8383
go test -tags dev -race -count=1 ./...
8484
8585
- name: Run govulncheck

‎.github/workflows/test.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ jobs:
6767
with:
6868
go-version-file: 'go.mod'
6969

70+
# Runs before the linter: if the pins have drifted, say so plainly rather
71+
# than letting the odd job out fail later on a version-specific finding.
72+
- name: Check golangci-lint pins are in lockstep
73+
run: make check-lint-lockstep
74+
7075
- name: Run golangci-lint
7176
uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0
7277
with:

‎Makefile‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ check-smoke-coverage: build
333333

334334
# Run all checks (local CI gate)
335335
.PHONY: check
336-
check: fmt-check vet lint lint-actions test test-e2e check-naming check-surface check-skill-drift check-bare-groups check-smoke-coverage provenance-check tidy-check
336+
check: fmt-check vet lint lint-actions test test-e2e check-naming check-surface check-skill-drift check-bare-groups check-lint-lockstep check-smoke-coverage provenance-check tidy-check
337337

338338
# Lint GitHub Actions workflows (requires actionlint + zizmor)
339339
.PHONY: lint-actions
@@ -413,6 +413,11 @@ check-skill-drift:
413413
check-bare-groups:
414414
@scripts/check-bare-groups.sh
415415

416+
# Verify every workflow lints with the same golangci-lint version
417+
.PHONY: check-lint-lockstep
418+
check-lint-lockstep:
419+
@scripts/check-lint-lockstep.sh
420+
416421
# Guard against bcq/BCQ creeping back (allowlist in .naming-allowlist)
417422
.PHONY: check-naming
418423
check-naming:

‎scripts/check-lint-lockstep.sh‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
#!/usr/bin/env bash
2+
# Verify every workflow lints with the same golangci-lint version.
3+
#
4+
# A stale pin here does not fail a PR — it fails whichever job is the odd one
5+
# out, at the moment that job runs. release.yml sat on v2.9.0 while test.yml and
6+
# security.yml moved to v2.11.1, and the drift surfaced as a gosec G115 false
7+
# positive that blocked a release tag after every PR check had gone green. The
8+
# "keep in lockstep" comments were already there; comments do not enforce.
9+
#
10+
# Every golangci-lint-action step must carry a version, not just agree with the
11+
# others. An unpinned step resolves to whatever the action defaults to, which
12+
# drifts on its own schedule and would rebuild exactly the release-only mismatch
13+
# this exists to prevent — so a missing pin is a failure, not a skip.
14+
set -euo pipefail
15+
16+
WORKFLOW_DIR=".github/workflows"
17+
18+
# One line per golangci-lint-action step: "<file>:<version>", or
19+
# "<file>:UNPINNED" when the step declares no version.
20+
mapfile -t pins < <(
21+
for f in "$WORKFLOW_DIR"/*.yml; do
22+
awk -v file="$f" '
23+
function close_step() {
24+
if (in_step) { print file ":UNPINNED"; in_step = 0 }
25+
}
26+
# A new list item ends the previous step, pinned or not.
27+
/^[[:space:]]*-[[:space:]]/ { close_step() }
28+
/uses:.*golangci-lint-action/ { in_step = 1; next }
29+
in_step && /^[[:space:]]*version:[[:space:]]*v[0-9]/ {
30+
match($0, /v[0-9]+\.[0-9]+\.[0-9]+/)
31+
print file ":" substr($0, RSTART, RLENGTH)
32+
in_step = 0
33+
next
34+
}
35+
# Dedenting to a new job or top-level key also ends the step.
36+
in_step && /^[[:space:]]{0,4}[A-Za-z_-]+:/ { close_step() }
37+
END { close_step() }
38+
' "$f"
39+
done
40+
)
41+
42+
if [[ "${#pins[@]}" -eq 0 ]]; then
43+
echo "FAIL: found no golangci-lint-action steps under $WORKFLOW_DIR"
44+
echo "If the linter moved to a different action, update $0 to match."
45+
exit 1
46+
fi
47+
48+
unpinned=$(printf '%s\n' "${pins[@]}" | grep ':UNPINNED$' || true)
49+
if [[ -n "$unpinned" ]]; then
50+
echo "FAIL: golangci-lint-action step with no version pin:"
51+
printf '%s\n' "$unpinned" | sed 's/:UNPINNED$//; s/^/ /'
52+
echo ""
53+
echo "An unpinned step takes whatever the action defaults to and drifts on"
54+
echo "its own schedule, which is the mismatch this check exists to prevent."
55+
exit 1
56+
fi
57+
58+
versions=$(printf '%s\n' "${pins[@]}" | sed 's/.*://' | sort -u)
59+
count=$(printf '%s\n' "$versions" | wc -l | tr -d ' ')
60+
61+
if [[ "$count" -ne 1 ]]; then
62+
echo "FAIL: golangci-lint pins disagree across workflows:"
63+
printf ' %s\n' "${pins[@]}"
64+
echo ""
65+
echo "Every workflow that lints must pin the same version. Otherwise a job"
66+
echo "that is not run on PRs — the release gate especially — can fail on a"
67+
echo "finding no PR check would ever have surfaced."
68+
exit 1
69+
fi
70+
71+
echo "golangci-lint lockstep check passed (${#pins[@]} steps pinned at $versions)"

0 commit comments

Comments
 (0)