From 992b3c68f5d213a5ebdc6b8d497780ff4b5dc793 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 18 Aug 2026 01:07:16 +0200 Subject: [PATCH] Run all linting in Docker via Dockerfile.lint (closes #109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit golangci-lint no longer runs on the host. script/lint builds Dockerfile.lint, which copies the repo into the digest-pinned linter image, so the container holds only this repo and the cross-worktree cache contamination of #106 becomes structurally impossible rather than filtered after the fact. Five workers hit that contamination in one evening, in both directions. Three properties are load-bearing. --no-cache-filter=lint forces the lint stage to re-execute while deps keeps its cache, because a cached build lints nothing in 0.27s and exits 0. script/lint does not trust that flag, since Docker silently ignores an unmatched stage name: it asserts golangci-lint's own summary line appears, so no summary means no lint whatever the exit code says. And both lint steps run --network=none, which enforces rather than assumes that config verify does not fetch its schema — verify is kept, because golangci-lint run silently ignores unrecognized config keys and it is the only thing that catches a typo that disables a setting. Independently reviewed four times. Three passed the behaviour; the remaining rounds were a README merge against #151, whose premise was that the file contains no false statements. Six statements this change falsified were found and corrected across those rounds — the last reviewer re-derived every countable claim against the tree rather than reading for plausibility, and found no seventh. Supersedes #106. --- Dockerfile | 9 ++++-- Dockerfile.lint | 37 +++++++++++++++++++++++ README.md | 78 +++++++++++++++++++++++++++++++++++------------- script/bootstrap | 69 +++++++----------------------------------- script/lint | 47 +++++++++++++++++++++++++++-- 5 files changed, 157 insertions(+), 83 deletions(-) create mode 100644 Dockerfile.lint diff --git a/Dockerfile b/Dockerfile index 3d32d01..207b733 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,9 +19,14 @@ RUN go mod download # .dockerignore. COPY . . -# Run formatting check and linter +# Run formatting check and linter. golangci-lint is invoked directly rather +# than through `make lint`: this stage is already the pinned linter image, and +# script/lint is a wrapper that builds Dockerfile.lint, so calling it here +# would need a docker daemon inside the build. Keep these steps in step with +# Dockerfile.lint, including --network=none (see its header for why). RUN make fmt-check -RUN make lint +RUN --network=none golangci-lint config verify --config .golangci.yml +RUN --network=none golangci-lint run --config .golangci.yml ./... # Build stage # golang:1.26.1-bookworm (Debian-based), 2026-03-17 diff --git a/Dockerfile.lint b/Dockerfile.lint new file mode 100644 index 0000000..9a9af51 --- /dev/null +++ b/Dockerfile.lint @@ -0,0 +1,37 @@ +# Lint-only image, built by script/lint. golangci-lint is never installed on +# the host: the repo is COPYed into the pinned image and linted as a build +# step, so a successful build IS a clean lint. This works even when the docker +# daemon is remote and bind mounts are impossible. +# +# script/lint passes --no-cache-filter=lint. Without it an unchanged tree +# replays the lint stage from cache and the build succeeds in under a second +# having run no linter at all. Do not drop that flag. +# +# The lint steps run with --network=none. `golangci-lint config verify` is +# documented as fetching its JSON schema over HTTPS, which would make linting +# depend on an unpinned remote artifact; this pinned image resolves the schema +# without any network, and --network=none enforces that rather than trusting +# it. It also proves no linter reaches out at analysis time. If a future image +# bump makes either step need the network, this build fails loudly instead of +# quietly acquiring an unpinned dependency. + +# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 +# Using Debian-based image because mattn/go-sqlite3 (CGO) does not +# compile on Alpine musl (off64_t is a glibc type). +FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps + +WORKDIR /src + +# Copy go mod files first for better layer caching. This stage is cacheable; +# only the lint stage below is forced to re-execute. +COPY go.mod go.sum ./ +RUN go mod download + +FROM deps AS lint + +COPY . . + +# `run` silently ignores config keys it does not recognize, so a typo would +# disable a setting without a word. `config verify` is what catches that. +RUN --network=none golangci-lint config verify --config .golangci.yml +RUN --network=none golangci-lint run --config .golangci.yml ./... diff --git a/README.md b/README.md index 7bc0010..fcdf1fe 100644 --- a/README.md +++ b/README.md @@ -12,14 +12,16 @@ with retry support, logging, and observability. Category: infrastructure ### Prerequisites - Go 1.26.1+ (the version in `go.mod`) -- golangci-lint v2.12.2 (the version pinned in `script/bootstrap` and - in the `Dockerfile`'s lint stage; `make bootstrap` installs it) -- Docker (for containerized deployment, and for the lint and test - stages of the CI gate) +- Docker (for linting, for the test stage of the CI gate, and for + containerized deployment) - `curl`, used by `script/fetch-assets` to download the third-party browser assets, which are not committed (`make bootstrap` installs it if missing) +golangci-lint is not a prerequisite and must not be installed on the +host: `script/bootstrap` does not install it, and `make lint` runs the +digest-pinned linter image via `Dockerfile.lint`. + ### Quick Start ```bash @@ -27,9 +29,9 @@ with retry support, logging, and observability. Category: infrastructure git clone https://git.eeqj.de/sneak/webhooker.git cd webhooker -# Install Go dependencies, the pinned linter, and the third-party -# browser assets. `make deps` alone is not enough: it only runs -# go mod download/tidy, and the checks below need the fetched assets. +# Install Go dependencies and the third-party browser assets. +# `make deps` alone is not enough: it only runs go mod download/tidy, +# and the checks below need the fetched assets. make bootstrap # Run all checks (test, lint, format check) @@ -52,7 +54,7 @@ make setup # Bootstrap + install git pre-commit hook make assets # Fetch + verify third-party browser assets make fmt # Format code (gofmt + goimports) make fmt-check # Fail if gofmt would change anything (writes nothing) -make lint # Run golangci-lint +make lint # Run golangci-lint in Docker (Dockerfile.lint) make test # Run tests with race detection make check # test + lint + fmt-check (CI gate) make build # Build binary to bin/webhooker @@ -275,7 +277,7 @@ are inline commands with no script behind them. We provide: - `script/fetch-assets` — download the third-party browser assets into `static/`, verifying each against its pinned sha256 - `script/test` — run the test suite -- `script/lint` — run golangci-lint +- `script/lint` — run golangci-lint in Docker (see Linting below) - `script/fmt` — format all code (writes) - `script/fmt-check` — check formatting (read-only) - `script/check` — run test, lint, and fmt-check @@ -1247,6 +1249,7 @@ webhooker/ ├── templates/ # Go HTML templates (base, login, sources, etc.) ├── script/ # Scripts to Rule Them All entrypoints ├── Dockerfile # Three stages: lint, test+build, Alpine runtime +├── Dockerfile.lint # Lint-only image built by script/lint ├── Makefile # 10 of 16 targets shim script/; 6 are inline ├── go.mod / go.sum └── .golangci.yml # Linter configuration @@ -1452,6 +1455,37 @@ Two operational consequences follow from bounding the sequence: no shutdown diagnostics at all. Keep the deployment's grace above the stop timeout. +### Linting + +golangci-lint never runs on the host. `script/lint` builds +`Dockerfile.lint`, which copies the repo into the digest-pinned +golangci-lint image and lints as a build step, so a successful build is +a clean lint. A host binary would share one cache and one lock with +every other checkout on the machine, which has produced both invented +findings attributed to other worktrees and unearned passes. + +Three properties are load-bearing: + +- `script/lint` passes `--no-cache-filter=lint`. Without it an unchanged + tree replays the lint layer from cache and the build exits 0 in under + a second having linted nothing. The `deps` stage stays cacheable, so + module downloads are not repeated. Invalidation is scoped to the one + stage; never prune the shared build cache. +- `script/lint` does not trust that flag. Docker silently ignores + `--no-cache-filter` for a stage name that does not match, so a stage + rename or a one-character typo would restore the cached false green + with no warning and a fast exit 0. The script therefore tees the + build output and treats a run as a pass only if golangci-lint's own + summary line (`N issues.` / `N issues:`) appears in it: no summary, + no lint, whatever the exit code says. +- Both lint steps use `RUN --network=none`. `golangci-lint config + verify` is documented as fetching its JSON schema over HTTPS, which + would be an unpinned remote dependency; the pinned image resolves the + schema without network access, and `--network=none` enforces that + instead of trusting it. Verify is worth keeping because + `golangci-lint run` silently ignores config keys it does not + recognize, so a typo would disable a setting with no warning. + ### Docker The Dockerfile uses a three-stage build. Each stage is pinned by @@ -1460,7 +1494,8 @@ version is fixed independently of the compiler's: 1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) — installs `make`, downloads dependencies, copies the source, and runs - `make fmt-check` then `make lint`. + `make fmt-check`, then `golangci-lint config verify` and + `golangci-lint run`, both with `--network=none`. 2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint stage passing (it copies a file from it), runs `script/fetch-assets` to download and verify the third-party browser assets, then runs @@ -1471,20 +1506,21 @@ version is fixed independently of the compiler's: runs as the non-root `webhooker` user (UID 1000), exposes port 8080, and includes a health check against `/.well-known/healthcheck`. +The lint stage invokes `golangci-lint` directly rather than `make lint`: +it is already the pinned linter image, and `make lint` builds +`Dockerfile.lint`, which would need a docker daemon inside this build. + Both check stages use Debian rather than Alpine because `gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO and does not compile against musl. Only the final binary is statically linked, which is what lets it run on the Alpine runtime image. -`script/cibuild` — `docker build .` — is the CI gate: the four check -targets run inside the image, so a build that succeeds is a repo that -is formatted, linted, tested and compiled. Only `script/cibuild` and -`script/docker` involve Docker. `script/lint`, and therefore -`make lint` and `make check`, run whatever `golangci-lint` is on the -host, which can be a different version from the pinned one — so the -container is the authoritative lint result -([issue #109](https://git.eeqj.de/sneak/webhooker/issues/109) tracks -routing local linting through it as well). +`script/cibuild` — `docker build .` — is the CI gate: the checks run +inside the image, so a build that succeeds is a repo that is formatted, +linted, tested and compiled. `script/lint` also uses Docker +(`Dockerfile.lint`, see Linting above), so `make lint` and `make check` +run the same pinned linter version the gate does; only `script/test` +and `script/fmt-check` run on the host. #### CI gate honesty @@ -1497,8 +1533,8 @@ the hash of the last commit that touched the build context, so: - Any commit that changes code (including a squash merge whose tree matches an already-built branch) gets a new fingerprint, invalidates the `COPY . .` layer of both check stages, and really runs - `make fmt-check`, `make lint`, `make test`, and `make build`. A run - that reports success ran them. + `make fmt-check`, `golangci-lint`, `make test`, and `make build`. A + run that reports success ran them. - A docs-only commit leaves the fingerprint unchanged — `.dockerignore` excludes `*.md`, `LICENSE` and `.editorconfig` from the context anyway — so the image replays from cache and costs seconds. diff --git a/script/bootstrap b/script/bootstrap index 7d2914b..49001f8 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -3,20 +3,14 @@ # this repo. Idempotent: every install is guarded by a check so already # installed tools are skipped. Base tooling comes from nix, apt, brew, # or apk (detected in that order); assumes NOTHING is present (not git, -# make, or go). golangci-lint is packaged in nix, brew, and apk; on apt -# it is installed from a hash-verified GitHub release archive (never -# curl | sh). Finishes by running script/fetch-assets, which installs the -# hash-pinned third-party browser assets the repo does not commit. +# make, or go). golangci-lint is deliberately not installed: linting runs +# only in docker, via script/lint and Dockerfile.lint. Finishes by running +# script/fetch-assets, which installs the hash-pinned third-party browser +# assets the repo does not commit. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" -# Pinned versions, 2026-08-07. Never "latest"; exact versions only. -GOLANGCI_LINT_VERSION="2.12.2" -# sha256 of golangci-lint-2.12.2-linux-.tar.gz release archives -GOLANGCI_LINT_SHA256_AMD64="8df580d2670fed8fa984aac0507099af8df275e665215f5c7a2ae3943893a553" -GOLANGCI_LINT_SHA256_ARM64="44cd40a8c76c86755375adfeea52cfd3533cb43d7bd647771e0ae065e166df3a" - PKGMGR="" SUDO="" @@ -57,52 +51,6 @@ missing() { ! command -v "$1" >/dev/null 2>&1 } -# verify_sha256 -verify_sha256() { - if command -v sha256sum >/dev/null 2>&1; then - actual="$(sha256sum "$1" | cut -d' ' -f1)" - else - actual="$(shasum -a 256 "$1" | cut -d' ' -f1)" - fi - if [ "$actual" != "$2" ]; then - echo "bootstrap: sha256 mismatch for $1" >&2 - echo " expected: $2" >&2 - echo " actual: $actual" >&2 - exit 1 - fi -} - -# apt has no golangci-lint package: install a pinned release archive -# from GitHub, verified by hardcoded sha256 (never curl | sh). -install_golangci_lint_release() { - case "$(uname -m)" in - x86_64) goarch="amd64"; sha="$GOLANGCI_LINT_SHA256_AMD64" ;; - aarch64|arm64) goarch="arm64"; sha="$GOLANGCI_LINT_SHA256_ARM64" ;; - *) - echo "bootstrap: unsupported architecture $(uname -m)" >&2 - exit 1 - ;; - esac - if missing curl; then pkg_install curl curl curl curl; fi - name="golangci-lint-${GOLANGCI_LINT_VERSION}-linux-${goarch}" - tmp="$(mktemp -d)" - curl -fsSL -o "$tmp/$name.tar.gz" \ - "https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/${name}.tar.gz" - verify_sha256 "$tmp/$name.tar.gz" "$sha" - tar -xzf "$tmp/$name.tar.gz" -C "$tmp" - $SUDO install -m 0755 "$tmp/$name/golangci-lint" /usr/local/bin/golangci-lint - rm -rf "$tmp" -} - -ensure_golangci_lint() { - if ! missing golangci-lint; then return 0; fi - detect_pkgmgr - case "$PKGMGR" in - apt) install_golangci_lint_release ;; - *) pkg_install golangci-lint golangci-lint golangci-lint golangci-lint ;; - esac -} - main() { cd "$ROOT" @@ -110,9 +58,14 @@ main() { if missing git; then pkg_install git git git git; fi if missing make; then pkg_install gnumake make make make; fi - # Go toolchain and linter + # Go toolchain if missing go; then pkg_install go golang go go; fi - ensure_golangci_lint + + # Not installed here: docker is platform-specific and out of scope for a + # package-manager bootstrap, but script/lint needs it. + if missing docker; then + echo "bootstrap: docker not found; script/lint requires it" >&2 + fi go mod download diff --git a/script/lint b/script/lint index 8017180..15de890 100755 --- a/script/lint +++ b/script/lint @@ -1,12 +1,55 @@ #!/bin/sh -# script/lint: run the linter. +# script/lint: run the linter. golangci-lint is never installed locally: it +# runs via docker only, one way, everywhere — script/lint builds +# Dockerfile.lint, which COPYs the repo into the pinned golangci-lint image +# and lints as a build step. This works even when the docker daemon is remote +# and bind mounts are impossible, and it removes the host linter's shared +# cache, which has attributed other checkouts' findings to this one. +# +# --no-cache-filter=lint forces the lint stage to re-execute on every run; a +# cached lint stage exits 0 in under a second having linted nothing. The deps +# stage keeps its cache, so module downloads are not repeated. +# --progress=plain keeps the linter's own output visible on success, so a +# passing run shows the issue count rather than nothing. +# --output=type=cacheonly leaves no image behind to clean up. +# +# docker silently ignores --no-cache-filter for a stage name that does not +# match, so a rename or a typo would restore the cached false green with no +# warning and a fast exit 0. The flag is therefore not trusted: the build +# output is teed to a log and a run is only a pass if golangci-lint's own +# summary line ("N issues." / "N issues:") is in it. No summary, no lint, +# whatever the exit code says. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - golangci-lint run --config .golangci.yml ./... + + log="$(mktemp -t webhooker-lint.XXXXXXXX)" + rcfile="$(mktemp -t webhooker-lint-rc.XXXXXXXX)" + trap 'rm -f "$log" "$rcfile"' EXIT INT TERM + + # The pipeline's status is tee's, and POSIX sh has no pipefail, so the + # build's status travels via a file. Output still streams live. + { + docker build \ + -f Dockerfile.lint \ + --no-cache-filter=lint \ + --progress=plain \ + --output=type=cacheonly \ + . 2>&1 && echo 0 >"$rcfile" || echo $? >"$rcfile" + } | tee "$log" >&2 + + rc="$(cat "$rcfile")" + [ "$rc" -eq 0 ] || exit "$rc" + + if ! grep -qE '[0-9]+ issues[.:]' "$log"; then + echo "script/lint: golangci-lint printed no summary line; the linter" >&2 + echo " did not run. Check that the stage named in --no-cache-filter" >&2 + echo " still matches a stage in Dockerfile.lint." >&2 + exit 1 + fi } main "$@"