1 Commits

Author SHA1 Message Date
clawbot
be59376522 Make the pinned golangci-lint actually reach the host (closes #28)
All checks were successful
check / check (push) Successful in 11s
REPO_POLICIES.md now carries the canonical script/bootstrap snippet for Go
repos alongside the .golangci.yml bullet, where the pinned linter version
already lives.

The guard it replaces, `if missing golangci-lint; then go install ...; fi`,
tests PATH presence and never version, so on any already-provisioned machine
the pin is inert and a version bump is a no-op. The Dockerfile installs
unconditionally into a clean image, so CI and local then disagree about what
the linter is: a local `make check` green while `make docker` rejects the same
commit, and a container run surfacing findings the host run cannot see.

Comparing versions alone is not enough. `go install` writes to GOBIN (or
GOPATH/bin) while callers resolve through PATH, so a shadowing binary earlier
in PATH lets the install succeed and change nothing a caller ever sees, while
bootstrap prints success. The canonical form therefore compares the installed
version against the pin, re-resolves through PATH after installing and asserts
the pin, and treats any unparseable --version output as a mismatch so the
failure direction is a redundant install rather than a skipped one.

When the assert fails the diagnosis is derived from the resolved path rather
than asserted: a path outside the install directory is shadowing and the
operator is told to remove it or reorder PATH; a path inside it is not, and
saying so would send them after a fault that does not exist; no resolution at
all means the install directory is simply absent from PATH.

The snippet ends with a call site. Two function definitions and no invocation
are a silent no-op with exactly the shape this change exists to close, and the
adoption instructions say to paste the block verbatim.

The version helper ends in `|| true` so a --version that exits non-zero cannot
kill the script through `set -e` under `set -o pipefail` before the diagnostic
is printed, which the styleguide's bash form would otherwise do.

The policy text states each of those as a requirement rather than leaving them
implicit in the code, records why the commit-pinned `go install` ref satisfies
the hash-pinning rule (a commit hash is not a mutable tag, and the checksum
database verifies the fetch; no repo go.sum is consulted, since `go install
pkg@version` ignores the go.mod in the current directory or any parent), and
requires the pin to be a tagged release commit: an untagged pin yields a
pseudo-version that the parser truncates at the first `-`, so no value of
GOLANGCI_LINT_VERSION derived from what --version prints can ever match.

Verification requires two things the previous round lacked: a negative control
against a shadowing binary, and running the controls against the block as a
consuming repo would adopt it, pasted into a script/bootstrap-shaped file and
executed, rather than sourcing it and calling the function directly.

The node and yarn handling described earlier in the document is untouched.
2026-08-09 15:41:09 +00:00

View File

@@ -381,17 +381,21 @@ style conventions are in separate documents:
The canonical form, placed in `script/bootstrap` after Go itself is present: The canonical form, placed in `script/bootstrap` after Go itself is present:
```sh ```sh
# golangci-lint v2.12.2, 2026-05-06 # golangci-lint v2.12.2, 2026-05-06. The ref must resolve to a TAGGED
# release; see the note on untagged pins below.
GOLANGCI_LINT_VERSION="2.12.2" GOLANGCI_LINT_VERSION="2.12.2"
GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5" GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5"
# The version golangci-lint reports, resolved the way callers resolve it. # The version golangci-lint reports, resolved the way callers resolve it.
# Prints nothing when the binary is absent, exits non-zero, or prints # Prints nothing when the binary is absent, exits non-zero, or prints
# something unparseable all of which must read as "does not match". # something unparseable: all of those must read as "does not match".
# The trailing `|| true` is required, not tidiness. Under `set -o pipefail`
# a non-zero --version would otherwise propagate out of the pipeline and
# kill the script through `set -e` before the diagnostic below is printed.
golangci_lint_version() { golangci_lint_version() {
command -v golangci-lint >/dev/null 2>&1 || return 0 command -v golangci-lint >/dev/null 2>&1 || return 0
golangci-lint --version 2>/dev/null | head -n 1 | golangci-lint --version 2>/dev/null | head -n 1 |
sed -n 's/.*has version v\{0,1\}\([0-9][0-9.]*\).*/\1/p' sed -n 's/.*has version v\{0,1\}\([0-9][0-9.]*\).*/\1/p' || true
} }
ensure_golangci_lint() { ensure_golangci_lint() {
@@ -403,57 +407,98 @@ style conventions are in separate documents:
# go install writes to GOBIN (or GOPATH/bin); callers resolve through # go install writes to GOBIN (or GOPATH/bin); callers resolve through
# PATH. Re-resolve through PATH and assert the install took effect. # PATH. Re-resolve through PATH and assert the install took effect.
# `hash -r` is load-bearing: without it a shell that already resolved
# a stale golangci-lint answers from its own lookup cache, and this
# check false-fails with the shadowing message below.
hash -r 2>/dev/null || true hash -r 2>/dev/null || true
got="$(golangci_lint_version)" gcl_got="$(golangci_lint_version)"
if [ "$got" = "$GOLANGCI_LINT_VERSION" ]; then if [ "$gcl_got" = "$GOLANGCI_LINT_VERSION" ]; then
return 0 return 0
fi fi
gobin="$(go env GOBIN)" gcl_bin="$(go env GOBIN)"
[ -n "$gobin" ] || gobin="$(go env GOPATH)/bin" [ -n "$gcl_bin" ] || gcl_bin="$(go env GOPATH)/bin"
found="$(command -v golangci-lint 2>/dev/null || true)" gcl_found="$(command -v golangci-lint 2>/dev/null || true)"
echo "bootstrap: installed golangci-lint $GOLANGCI_LINT_VERSION into" \ echo "bootstrap: installed golangci-lint $GOLANGCI_LINT_VERSION into" \
"$gobin, but PATH resolves golangci-lint to ${found:-nothing}," \ "$gcl_bin, but that is not what callers will get." >&2
"reporting version ${got:-unparseable}." >&2 case "$gcl_found" in
echo "bootstrap: remove that binary or put $gobin earlier in PATH," \ "")
"then re-run bootstrap." >&2 echo "bootstrap: PATH resolves no golangci-lint at all." \
"Add $gcl_bin to PATH, then re-run bootstrap." >&2
;;
"$gcl_bin"/*)
echo "bootstrap: PATH resolves $gcl_found, inside that same" \
"directory, reporting version ${gcl_got:-unparseable}." \
"Nothing is shadowing it, so the install itself did not" \
"produce the pinned version: check that" \
"GOLANGCI_LINT_VERSION matches GOLANGCI_LINT_REF." >&2
;;
*)
echo "bootstrap: PATH resolves $gcl_found instead, reporting" \
"version ${gcl_got:-unparseable}. Remove that binary or" \
"put $gcl_bin earlier in PATH, then re-run bootstrap." >&2
;;
esac
exit 1 exit 1
} }
# The definitions above are inert on their own; the call site is part of
# the canonical form. In a script/bootstrap that follows the "define all
# functions, then call main" convention, this line belongs inside main()
# next to the other ensure_* steps.
ensure_golangci_lint
``` ```
Three properties are load-bearing; each guards a failure mode that otherwise Four properties are load-bearing; each guards a failure mode that otherwise
fails green: fails green:
- **Compare the installed version against the pin**, never test presence. - **Compare the installed version against the pin**, never test presence.
This is what makes a version bump propagate to machines that already have This is what makes a version bump propagate to machines that already have
some golangci-lint. some golangci-lint.
- **After installing, re-resolve the binary the way callers resolve it** — - **After installing, re-resolve the binary the way callers resolve it** —
through `PATH`, not the path `go install` wrote to — and assert through `PATH`, not the path `go install` wrote to — and assert
`--version` reports the pin. When it does not, fail non-zero and **name `--version` reports the pin. When it does not, fail non-zero and name the
the shadowing path** `command -v` actually found, the version it reports, path `command -v` actually found, the version it reports, and the
and the directory the install wrote to. That is a condition a human has to directory the install wrote to. That is a condition a human has to fix by
fix by hand, so bootstrap must not print success in it. Use `hash -r` hand, so bootstrap must not print success in it. Use `hash -r` first so
first so the shell does not answer from its own lookup cache. the shell does not answer from its own lookup cache. Diagnose the cause
from the resolved path rather than asserting one: only a path **outside**
the install directory is shadowing. When the resolved path is inside it,
nothing is shadowing and telling the operator to delete that binary or
reorder `PATH` sends them after a fault that does not exist.
- **A mis-parse must fall through to reinstall, never to a false match.** - **A mis-parse must fall through to reinstall, never to a false match.**
Absent binary, non-zero exit, empty output, and unrecognised output all Absent binary, non-zero exit, empty output, and unrecognised output all
yield an empty string, which compares unequal to the pin. The failure yield an empty string, which compares unequal to the pin. The failure
direction is always a redundant install, never a skipped one. direction is always a redundant install, never a skipped one.
- **Call it.** Two function definitions with no call site are a silent no-op
that reproduces the original defect exactly: exit 0, nothing installed, no
output, stale linter still resolved.
Keep it POSIX sh: no bashisms, no arrays, no `[[`, no `grep -P`. Keep it POSIX sh: no bashisms, no arrays, no `[[`, no `grep -P`.
**On the hash-pinning rule.** `@c0d3ddc9cf3faa61a4e378e879ece580256d76e5` is **On the hash-pinning rule.** `@c0d3ddc9cf3faa61a4e378e879ece580256d76e5` is
a commit hash, not a server-mutable version tag, and the go command verifies a commit hash, not a server-mutable version tag, and the go command verifies
the fetched module against the checksum database and `go.sum` — the the fetched module against the checksum database — the mechanism the
mechanism the hash-pinning rule at the top of this document already names as hash-pinning rule at the top of this document already names as acceptable
acceptable for Go modules. So the ref stays a bare `go install` of a for Go modules. Note that `go install pkg@version` runs in module-aware mode
commit-pinned module rather than a `go.mod` tool dependency; the linter is a ignoring the `go.mod` in the current directory or any parent, so no repo
bootstrap prerequisite rather than part of the module graph, and tracking it `go.sum` is consulted for this install; the checksum database is what
as a tool dependency would pull its whole dependency tree into every verifies it. The linter is a bootstrap prerequisite rather than part of any
consuming repo's `go.mod` and `go.sum`. `GOLANGCI_LINT_VERSION` is a repo's module graph, which is why the canonical form installs it by
separate string because the ref is a hash and carries no readable version; commit-pinned ref instead of declaring it in `go.mod`.
it must be updated with the ref. That commit is the `v2.12.2` tag commit, so
the go command resolves it to `v2.12.2` and the built binary reports **The pin must be a tagged release commit.** `GOLANGCI_LINT_VERSION` is a
`2.12.2`. If a pin is ever moved to a commit that carries no release tag, separate string because the ref is a hash and carries no readable version,
the binary will report a pseudo-version instead and `GOLANGCI_LINT_VERSION` so the two must be updated together. It works out to a clean `2.12.2` only
must be set to whatever `--version` then prints. because that commit is the `v2.12.2` tag commit: the go command resolves a
commit query to a semver tag on that commit when one exists, so the built
binary reports `2.12.2`. Pin to an **untagged** commit and the go command
falls back to a pseudo-version — the binary then reports something like
`2.12.3-0.20260506110758-c0d3ddc9cf3f`, and this comparison cannot express
it: the parser above stops at the first `-`, so the value to match would
have to be the truncated numeric prefix `2.12.3`, which is also the number
of a real future release. Setting `GOLANGCI_LINT_VERSION` to the full string
`--version` prints can never match, and bootstrap would then reinstall on
every run and exit 1 blaming a shadowing binary that does not exist. Keep
pins on tagged releases.
**Verifying a change to this logic requires a negative control run in an **Verifying a change to this logic requires a negative control run in an
environment where a shadowing binary exists earlier in `PATH` than the environment where a shadowing binary exists earlier in `PATH` than the
@@ -462,6 +507,13 @@ style conventions are in separate documents:
the mis-parse direction by feeding it unparseable `--version` output and the mis-parse direction by feeding it unparseable `--version` output and
confirming it reinstalls rather than reporting a match. confirming it reinstalls rather than reporting a match.
**Run those controls against the block as a consuming repo would adopt it**
— pasted into a `script/bootstrap`-shaped file that is then executed — not
by sourcing it and invoking the function yourself. Driving the function
directly tests something the artifact does not do, and it is exactly how a
missing call site passes every control while the adopted snippet does
nothing.
- When pinning images or packages by hash, add a comment above the reference - When pinning images or packages by hash, add a comment above the reference
with the version and date (YYYY-MM-DD). with the version and date (YYYY-MM-DD).