script/bootstrap installs golangci-lint unpinned, so local lint diverges from CI #45

Closed
opened 2026-08-09 03:42:51 +02:00 by clawbot · 2 comments
Collaborator

From the repo-standards audit. Present on main and the pending lint branch.

Divergence

Policy, described in its own text as the single most important rule in the document:

> ALL external references must be pinned by cryptographic hash. ... Version tags are server-mutable and therefore remote code execution vulnerabilities. ... never curl | bash to install tools ... download a specific release archive from GitHub, verify its hash ... There are zero exceptions to this rule.

script/bootstrap:142-144:

if missing golangci-lint; then
    pkg_install golangci-lint golangci-lint golangci-lint golangci-lint
fi

No version, no hash. On apt this becomes apt-get install -y golangci-lint. The comment directly above these lines already states the correct approach — download a specific release archive and verify with verify_sha256, never curl | sh — and the code does not do it. The verify_sha256 helper already exists in the script and is unused for this path.

Beyond the pinning rule this is a correctness problem, and a live one given PR #29: a developer's make lint runs whatever golangci-lint version their distro packages against a config written for v2.12.2. Older versions will not recognize the newer linters and will either error or silently skip them, so local runs and CI disagree about whether the tree is clean. The Dockerfile correctly pins v2.12.2 by digest; script/bootstrap does not match it.

In fairness: the canonical org script/bootstrap has the same unpinned pkg_install shape, so this is inherited from the template rather than introduced here. Worth raising upstream against the prompts repo as well.

Definition of done

  • script/bootstrap installs golangci-lint at a pinned version matching the Dockerfile (v2.12.2), from a release archive verified against a hardcoded sha256 via the existing verify_sha256 helper.
  • The version constant appears exactly once in the script, with the version-and-date comment policy requires above the hash.
  • Platform handling is preserved: the script currently detects nix, apt, brew, and apk in that order. The pinned download path must work on Linux x86_64 and arm64 and on macOS arm64 at minimum, or explicitly fall back with a clear message on anything else.
  • Idempotent, per the existing contract: re-running script/bootstrap with the correct version already installed does not re-download.
  • If golangci-lint is already on PATH at a different version, the script says so rather than silently accepting it — a mismatched linter is precisely the failure this is fixing.
  • The script stays POSIX sh, keeps set -eu, no bashisms.
  • make check green after a fresh script/bootstrap run. TODO.md updated in the same commit.

Implementation requirements

  • The sha256 must be for the specific platform archive being downloaded; golangci-lint publishes a different archive per OS/arch, so this is a small table, not one constant. Pin every platform the script claims to support.
  • Do not curl | sh the upstream install script, and do not use the --version convenience installer. Download the archive, verify, extract, install.
  • Verify the hash before extracting, not after.
  • Sequence after PR #29 so the pinned version matches what actually landed.
  • Consider whether the same treatment is needed for other tools this script installs. If any others are unpinned, note them on this issue rather than expanding this commit.
From the repo-standards audit. Present on `main` and the pending lint branch. ## Divergence Policy, described in its own text as the single most important rule in the document: > **ALL external references must be pinned by cryptographic hash.** ... Version tags are server-mutable and therefore remote code execution vulnerabilities. ... never `curl | bash` to install tools ... download a specific release archive from GitHub, verify its hash ... There are zero exceptions to this rule. `script/bootstrap:142-144`: ```sh if missing golangci-lint; then pkg_install golangci-lint golangci-lint golangci-lint golangci-lint fi ``` No version, no hash. On apt this becomes `apt-get install -y golangci-lint`. The comment directly above these lines already states the correct approach — download a specific release archive and verify with `verify_sha256`, never `curl | sh` — and the code does not do it. The `verify_sha256` helper already exists in the script and is unused for this path. Beyond the pinning rule this is a **correctness** problem, and a live one given PR #29: a developer's `make lint` runs whatever golangci-lint version their distro packages against a config written for v2.12.2. Older versions will not recognize the newer linters and will either error or silently skip them, so local runs and CI disagree about whether the tree is clean. The `Dockerfile` correctly pins v2.12.2 by digest; `script/bootstrap` does not match it. **In fairness:** the canonical org `script/bootstrap` has the same unpinned `pkg_install` shape, so this is inherited from the template rather than introduced here. Worth raising upstream against the `prompts` repo as well. ## Definition of done - `script/bootstrap` installs golangci-lint at a pinned version matching the `Dockerfile` (v2.12.2), from a release archive verified against a hardcoded sha256 via the existing `verify_sha256` helper. - The version constant appears exactly once in the script, with the version-and-date comment policy requires above the hash. - Platform handling is preserved: the script currently detects nix, apt, brew, and apk in that order. The pinned download path must work on Linux x86_64 and arm64 and on macOS arm64 at minimum, or explicitly fall back with a clear message on anything else. - Idempotent, per the existing contract: re-running `script/bootstrap` with the correct version already installed does not re-download. - If golangci-lint is already on `PATH` at a **different** version, the script says so rather than silently accepting it — a mismatched linter is precisely the failure this is fixing. - The script stays POSIX sh, keeps `set -eu`, no bashisms. - `make check` green after a fresh `script/bootstrap` run. `TODO.md` updated in the same commit. ## Implementation requirements - The sha256 must be for the specific platform archive being downloaded; golangci-lint publishes a different archive per OS/arch, so this is a small table, not one constant. Pin every platform the script claims to support. - Do not `curl | sh` the upstream install script, and do not use the `--version` convenience installer. Download the archive, verify, extract, install. - Verify the hash **before** extracting, not after. - Sequence after PR #29 so the pinned version matches what actually landed. - Consider whether the same treatment is needed for other tools this script installs. If any others are unpinned, note them on this issue rather than expanding this commit.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:42:51 +02:00
Author
Collaborator

Implementation guidance, from a cross-repo clarification on what "pinned by hash" means for each install mechanism. This narrows the approach and removes a wrong turn, so read it before starting.

The two mechanisms are pinned differently, and this repo needs both.

  • The Dockerfile lint stage installs golangci-lint by pulling a container image. Its content hash is the Docker digest, and it is already correct: FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240, with the version-and-date comment above it. Nothing to do there. If anyone suggests adding a git commit SHA to that line, it is redundant — a Docker digest already pins the exact bytes, and the commit SHA does not participate in what Docker resolves.
  • script/bootstrap installs a Go tool onto a developer's machine, which is a different mechanism with a different correct answer. Where the install goes through go install, the pin is the upstream git commit, and for golangci-lint v2.12.2 that is c0d3ddc9cf3faa61a4e378e879ece580256d76e5. That form is what this issue needs.

So the fix has two viable shapes, and the choice is worth making deliberately rather than by whichever is easier:

  1. go install pinned to the commitgo install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5. Go verifies the module against go.sum/GONOSUMDB and the module proxy's transparency log, so this is genuinely content-pinned. It needs a Go toolchain present, which script/bootstrap already guarantees since it installs Go first. It is also platform-agnostic, which removes the per-OS/arch hash table the issue body anticipated.
  2. Release-archive download plus verify_sha256 — as the issue body originally described. Still valid, but it needs a hash per platform and more script surface.

Option 1 is the better fit here and I would take it unless something blocks it: it is fewer moving parts, it reuses machinery the script already has, it sidesteps maintaining a platform hash table, and it removes the pkg_install path entirely rather than working around it. Report on this issue if you hit a reason it does not work.

Whichever is chosen, the rest of the definition of done in the issue body stands unchanged — in particular that the version appears exactly once, that a pre-existing golangci-lint at a different version is reported rather than silently accepted, and that the installed version matches the Dockerfile's v2.12.2. The mismatch between a developer's distro-packaged linter and the pinned CI one is the actual bug being fixed here; the pinning-policy compliance is the secondary benefit.

Worth restating the sequencing from the issue body: this lands after PR #29, so the pinned version matches what actually shipped.

Implementation guidance, from a cross-repo clarification on what "pinned by hash" means for each install mechanism. This narrows the approach and removes a wrong turn, so read it before starting. **The two mechanisms are pinned differently, and this repo needs both.** - The **Dockerfile lint stage** installs golangci-lint by pulling a container image. Its content hash is the Docker digest, and it is already correct: `FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`, with the version-and-date comment above it. **Nothing to do there.** If anyone suggests adding a git commit SHA to that line, it is redundant — a Docker digest already pins the exact bytes, and the commit SHA does not participate in what Docker resolves. - **`script/bootstrap`** installs a Go tool onto a developer's machine, which is a different mechanism with a different correct answer. Where the install goes through `go install`, the pin is the **upstream git commit**, and for golangci-lint v2.12.2 that is `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. That form is what this issue needs. So the fix has two viable shapes, and the choice is worth making deliberately rather than by whichever is easier: 1. **`go install` pinned to the commit** — `go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. Go verifies the module against `go.sum`/GONOSUMDB and the module proxy's transparency log, so this is genuinely content-pinned. It needs a Go toolchain present, which `script/bootstrap` already guarantees since it installs Go first. It is also platform-agnostic, which removes the per-OS/arch hash table the issue body anticipated. 2. **Release-archive download plus `verify_sha256`** — as the issue body originally described. Still valid, but it needs a hash per platform and more script surface. **Option 1 is the better fit here** and I would take it unless something blocks it: it is fewer moving parts, it reuses machinery the script already has, it sidesteps maintaining a platform hash table, and it removes the `pkg_install` path entirely rather than working around it. Report on this issue if you hit a reason it does not work. Whichever is chosen, the rest of the definition of done in the issue body stands unchanged — in particular that the version appears exactly once, that a pre-existing golangci-lint at a *different* version is reported rather than silently accepted, and that the installed version matches the Dockerfile's v2.12.2. The mismatch between a developer's distro-packaged linter and the pinned CI one is the actual bug being fixed here; the pinning-policy compliance is the secondary benefit. Worth restating the sequencing from the issue body: this lands after PR #29, so the pinned version matches what actually shipped.
Author
Collaborator

Closing as superseded. The owner ruling captured in #55 moves every lint run into a Docker container invoked through script/lint, and explicitly removes the golangci-lint install from script/bootstrap — so there is no longer a host linter that can diverge from CI, which was this issue's entire subject. Nothing here is dropped: the divergence is eliminated rather than pinned.

The Dockerfile lint-stage digest pin noted above is already correct and stays.

Closing as superseded. The owner ruling captured in https://git.eeqj.de/sneak/secret/issues/55 moves every lint run into a Docker container invoked through `script/lint`, and explicitly removes the golangci-lint install from `script/bootstrap` — so there is no longer a host linter that can diverge from CI, which was this issue's entire subject. Nothing here is dropped: the divergence is eliminated rather than pinned. The Dockerfile lint-stage digest pin noted above is already correct and stays.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/secret#45