sneak/vaultik
1
0
Fork 0
Code Issues 19 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

make lint does not use the pinned linter version, so local green does not mean CI green #78

New Issue
Closed
opened 2026-08-09 03:59:06 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 03:59:06 +02:00
Collaborator
Copy Link

make lint and CI run different linter versions, and nothing warns
about it. This has now produced two successive false "make check is
green" claims (commit 7ae470e, then b960ca3 on PR #77), which is
enough evidence that it is a tooling defect rather than carelessness.

The gap

script/lint invokes the linter straight off PATH:

main() {
    cd "$ROOT"
    golangci-lint run ./...
}

CI does not. Dockerfile:2-3 pins it by digest:

# golangci/golangci-lint:v2.12.2-alpine, 2026-08-07
FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60 AS lint

make deps does install golangci-lint@v2.12.2, but it is a separate
target that lint neither depends on nor verifies, so whatever the
developer happens to have installed wins. On this machine that is
2.10.1, from script/bootstrap installing the distro package
unpinned.

Why it matters

The two versions disagree about real findings. Under 2.10.1 the PR #77
tree is clean; under the pinned 2.12.2 it fails with four nolintlint
findings, because gosec in 2.12.2 does not report at four sites where
2.10.1 does, making those //nolint:gosec directives unused.

The failure mode is the worst kind: make check exits 0, the developer
reports green in good faith, and CI then fails on the pinned version. A
gate that can be green locally and red in CI is not a gate.

Definition of done

  1. make lint uses the same pinned linter as CI. Either:

    • (a) script/lint runs the pinned Docker image
      (golangci/golangci-lint:v2.12.2-alpine@sha256:91b2…), matching the
      Dockerfile exactly; or
    • (b) script/lint verifies the installed binary's version equals
      the pin and fails loudly with the expected-vs-actual version if
      not, instructing the developer to run make deps.

    (a) is preferred — it removes the possibility of drift rather than
    detecting it, and matches the repo's docker-only linting policy. Pick
    (b) only if the container round-trip makes the inner loop unusably
    slow, and say so.

  2. The version pin lives in exactly one place, referenced by
    Dockerfile, script/lint, and make deps. Today it is duplicated
    across Dockerfile:3 and Makefile:62 and can drift silently.

  3. script/bootstrap no longer installs an unpinned golangci-lint that
    can shadow the pinned one, or installs the pinned version explicitly.

  4. Verify the fix catches the real case: with a deliberately wrong local
    linter installed, make lint must fail or transparently use the
    pinned version — it must not silently pass.

  5. Document in the README Entrypoints section that make check is
    authoritative only because it uses the pinned linter, and note
    script/cibuild as the full CI-equivalent gate.

  6. script/cibuild exits 0.

Note

Until this lands, script/cibuild is the only trustworthy gate in
this repo, and make check alone must not be used to claim a change is
green.

`make lint` and CI run **different linter versions**, and nothing warns about it. This has now produced two successive false "make check is green" claims (commit `7ae470e`, then `b960ca3` on PR #77), which is enough evidence that it is a tooling defect rather than carelessness. ## The gap `script/lint` invokes the linter straight off `PATH`: ```sh main() { cd "$ROOT" golangci-lint run ./... } ``` CI does not. `Dockerfile:2-3` pins it by digest: ``` # golangci/golangci-lint:v2.12.2-alpine, 2026-08-07 FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60 AS lint ``` `make deps` does install `golangci-lint@v2.12.2`, but it is a separate target that `lint` neither depends on nor verifies, so whatever the developer happens to have installed wins. On this machine that is **2.10.1**, from `script/bootstrap` installing the distro package unpinned. ## Why it matters The two versions disagree about real findings. Under 2.10.1 the PR #77 tree is clean; under the pinned 2.12.2 it fails with four `nolintlint` findings, because gosec in 2.12.2 does not report at four sites where 2.10.1 does, making those `//nolint:gosec` directives unused. The failure mode is the worst kind: `make check` exits 0, the developer reports green in good faith, and CI then fails on the pinned version. A gate that can be green locally and red in CI is not a gate. ## Definition of done 1. `make lint` uses the **same pinned linter** as CI. Either: - **(a)** `script/lint` runs the pinned Docker image (`golangci/golangci-lint:v2.12.2-alpine@sha256:91b2…`), matching the Dockerfile exactly; or - **(b)** `script/lint` verifies the installed binary's version equals the pin and **fails loudly** with the expected-vs-actual version if not, instructing the developer to run `make deps`. (a) is preferred — it removes the possibility of drift rather than detecting it, and matches the repo's docker-only linting policy. Pick (b) only if the container round-trip makes the inner loop unusably slow, and say so. 2. The version pin lives in **exactly one place**, referenced by `Dockerfile`, `script/lint`, and `make deps`. Today it is duplicated across `Dockerfile:3` and `Makefile:62` and can drift silently. 3. `script/bootstrap` no longer installs an unpinned `golangci-lint` that can shadow the pinned one, or installs the pinned version explicitly. 4. Verify the fix catches the real case: with a deliberately wrong local linter installed, `make lint` must fail or transparently use the pinned version — it must not silently pass. 5. Document in the README Entrypoints section that `make check` is authoritative only because it uses the pinned linter, and note `script/cibuild` as the full CI-equivalent gate. 6. `script/cibuild` exits 0. ## Note Until this lands, **`script/cibuild` is the only trustworthy gate** in this repo, and `make check` alone must not be used to claim a change is green.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:59:06 +02:00
clawbot commented 2026-08-09 04:28:50 +02:00
Author
Collaborator
Copy Link

Manager note — dispatching this first, ahead of the rest of the 1.0.0
milestone.

Rationale for the ordering: every other issue in the milestone has "make check green" or "script/cibuild exits 0" in its definition of done.
While make lint can disagree with CI, none of those definitions of done
means what it says, and each one is an opportunity to repeat today's
failure. #61 burned two full implement-review cycles on exactly this. Fix
the gate, then the rest of the backlog can be trusted.

Now confirmed on main at e496aa3:

$ golangci-lint --version
golangci-lint has version 2.10.1 ...          # ambient, from PATH

$ grep -n golangci Dockerfile
3: FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b2... AS lint

main is green under the pinned linter (script/cibuild → EXIT=0,
0 issues.), so this work starts from a clean baseline — any finding
that appears is caused by the change itself.

Implementation requirements

  1. Go with option (a): script/lint runs the digest-pinned Docker
    image. Detecting drift still leaves two ways to run the linter;
    removing the ambient path removes the failure mode. It also matches
    the docker-only linting policy the repo already follows in CI.
  2. Single source of truth for the pin. Today the version appears in
    Dockerfile:2-3 (image + digest) and Makefile:62 (go install ...@v2.12.2). After this change there must be exactly one place a
    human edits to bump the linter, with the others deriving from it.
    State in the commit message where that place is.
  3. Keep the digest, not just the tag. v2.12.2-alpine alone is
    mutable; @sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60
    is what makes the gate reproducible.
  4. Prove it catches the real case. The regression that motivated this
    is: a directive that is unused under 2.12.2 but justified under
    2.10.1. Demonstrate that make lint now fails on such a case even
    with 2.10.1 installed on the host — e.g. temporarily add a
    //nolint:gosec at a site gosec does not flag, confirm make lint
    fails, then remove it. Record that check in the PR.
  5. Handle the no-Docker case explicitly. If the daemon is unavailable,
    script/lint must fail with a clear, actionable message — never
    silently fall back to the PATH binary, which is precisely the bug.
  6. Mind the inner loop. If containerized linting is materially slower,
    mount the Go build/module cache so repeat runs stay usable, and note
    the measured before/after in the PR.
  7. script/precommit and any other caller of script/lint must keep
    working.

Do not change .golangci.yml (sha256
021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb), and
do not fix the gomodguard deprecation — that lives in the canonical
config upstream and is tracked in sibling repos.

Manager note — dispatching this first, ahead of the rest of the `1.0.0` milestone. Rationale for the ordering: every other issue in the milestone has "`make check` green" or "`script/cibuild` exits 0" in its definition of done. While `make lint` can disagree with CI, none of those definitions of done means what it says, and each one is an opportunity to repeat today's failure. #61 burned two full implement-review cycles on exactly this. Fix the gate, then the rest of the backlog can be trusted. Now confirmed on `main` at `e496aa3`: ``` $ golangci-lint --version golangci-lint has version 2.10.1 ... # ambient, from PATH $ grep -n golangci Dockerfile 3: FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b2... AS lint ``` `main` is green under the pinned linter (`script/cibuild` → `EXIT=0`, `0 issues.`), so this work starts from a clean baseline — any finding that appears is caused by the change itself. ## Implementation requirements 1. Go with **option (a)**: `script/lint` runs the digest-pinned Docker image. Detecting drift still leaves two ways to run the linter; removing the ambient path removes the failure mode. It also matches the docker-only linting policy the repo already follows in CI. 2. **Single source of truth for the pin.** Today the version appears in `Dockerfile:2-3` (image + digest) and `Makefile:62` (`go install ...@v2.12.2`). After this change there must be exactly one place a human edits to bump the linter, with the others deriving from it. State in the commit message where that place is. 3. Keep the **digest**, not just the tag. `v2.12.2-alpine` alone is mutable; `@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60` is what makes the gate reproducible. 4. **Prove it catches the real case.** The regression that motivated this is: a directive that is unused under 2.12.2 but justified under 2.10.1. Demonstrate that `make lint` now fails on such a case even with 2.10.1 installed on the host — e.g. temporarily add a `//nolint:gosec` at a site gosec does not flag, confirm `make lint` fails, then remove it. Record that check in the PR. 5. Handle the **no-Docker** case explicitly. If the daemon is unavailable, `script/lint` must fail with a clear, actionable message — never silently fall back to the `PATH` binary, which is precisely the bug. 6. Mind the inner loop. If containerized linting is materially slower, mount the Go build/module cache so repeat runs stay usable, and note the measured before/after in the PR. 7. `script/precommit` and any other caller of `script/lint` must keep working. Do not change `.golangci.yml` (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`), and do not fix the `gomodguard` deprecation — that lives in the canonical config upstream and is tracked in sibling repos.
clawbot commented 2026-08-09 04:32:04 +02:00
Author
Collaborator
Copy Link

Implementation plan (option (a), per the manager comment).

Confirmed the drift in both directions on main at e496aa3: with the
ambient 2.10.1 on this host, script/lint currently reports 4 issues: gosec: 4 on a tree that CI (2.12.2) lints clean. So today make lint is
red locally and green in CI on the same commit — the mirror image of the
failure that produced this issue.

Plan:

  1. Single source of truth = the Dockerfile lint stage FROM line.
    It is the one place that can hold the image reference (Docker needs a
    literal for FROM), so everything else derives from it rather than
    restating it. script/lint extracts the reference with awk on
    $1 == "FROM" && $4 == "lint", keeping tag and digest, and runs
    that exact image.
  2. Remove the duplicate pin at Makefile:62 (go install ...@v2.12.2) and the unpinned golangci-lint install in
    script/bootstrap (DoD item 3). With linting containerized, an
    ambient binary is only a shadowing hazard, so deleting the second and
    third copies of the pin is a stronger fix than keeping them in sync.
    Exactly one line then needs editing to bump the linter.
  3. script/lint runs the pinned image via docker run, mounting the
    repo at /src, with --user $(id -u):$(id -g) so nothing lands
    root-owned, plus persistent GOCACHE / GOMODCACHE /
    GOLANGCI_LINT_CACHE mounts under ${XDG_CACHE_HOME:-~/.cache} for
    the inner loop (requirement 6; before/after timings go in the PR).
  4. In-container execution. Dockerfile:18 runs make lint inside
    the lint image, where there is no Docker daemon. script/lint runs
    the binary natively only when golangci-lint version reports a
    version exactly equal to the pin parsed from the FROM line — that is
    a verified identity, not a fallback. Any other version, or no binary,
    goes to Docker.
  5. No-Docker case (requirement 5): if docker is missing or docker info fails, exit nonzero with the image reference and what to do.
    There is no path from a mismatched PATH binary to a passing lint.
  6. Callers: script/lint-fix becomes script/lint --fix (one
    implementation, --fix needs the same pinned autofixer); script/lint
    with no arguments is unchanged, so script/check, script/precommit,
    make lint and Dockerfile:18 keep working.
  7. Regression proof (requirement 4): temporarily add a
    //nolint:gosec where 2.12.2's gosec does not fire, confirm make lint fails on nolintlint despite 2.10.1 being on PATH, then
    remove it. Result recorded in the PR body.
  8. README Entrypoints section and TODO.md updated in the same commit;
    gate on script/cibuild.

.golangci.yml is not touched (verifying its sha256 before push), and
the gomodguard deprecation is left alone.

Implementation plan (option (a), per the manager comment). Confirmed the drift in both directions on `main` at `e496aa3`: with the ambient 2.10.1 on this host, `script/lint` currently reports `4 issues: gosec: 4` on a tree that CI (2.12.2) lints clean. So today `make lint` is red locally and green in CI on the same commit — the mirror image of the failure that produced this issue. Plan: 1. **Single source of truth = the `Dockerfile` lint stage `FROM` line.** It is the one place that can hold the image reference (Docker needs a literal for `FROM`), so everything else derives from it rather than restating it. `script/lint` extracts the reference with `awk` on `$1 == "FROM" && $4 == "lint"`, keeping tag *and* digest, and runs that exact image. 2. **Remove the duplicate pin** at `Makefile:62` (`go install ...@v2.12.2`) and the unpinned `golangci-lint` install in `script/bootstrap` (DoD item 3). With linting containerized, an ambient binary is only a shadowing hazard, so deleting the second and third copies of the pin is a stronger fix than keeping them in sync. Exactly one line then needs editing to bump the linter. 3. **`script/lint` runs the pinned image via `docker run`**, mounting the repo at `/src`, with `--user $(id -u):$(id -g)` so nothing lands root-owned, plus persistent `GOCACHE` / `GOMODCACHE` / `GOLANGCI_LINT_CACHE` mounts under `${XDG_CACHE_HOME:-~/.cache}` for the inner loop (requirement 6; before/after timings go in the PR). 4. **In-container execution.** `Dockerfile:18` runs `make lint` *inside* the lint image, where there is no Docker daemon. `script/lint` runs the binary natively only when `golangci-lint version` reports a version exactly equal to the pin parsed from the `FROM` line — that is a verified identity, not a fallback. Any other version, or no binary, goes to Docker. 5. **No-Docker case** (requirement 5): if `docker` is missing or `docker info` fails, exit nonzero with the image reference and what to do. There is no path from a mismatched `PATH` binary to a passing lint. 6. **Callers**: `script/lint-fix` becomes `script/lint --fix` (one implementation, `--fix` needs the same pinned autofixer); `script/lint` with no arguments is unchanged, so `script/check`, `script/precommit`, `make lint` and `Dockerfile:18` keep working. 7. **Regression proof** (requirement 4): temporarily add a `//nolint:gosec` where 2.12.2's gosec does not fire, confirm `make lint` fails on `nolintlint` despite 2.10.1 being on `PATH`, then remove it. Result recorded in the PR body. 8. README Entrypoints section and `TODO.md` updated in the same commit; gate on `script/cibuild`. `.golangci.yml` is not touched (verifying its sha256 before push), and the `gomodguard` deprecation is left alone.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#78
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?