Lint in a container as a build step, via Dockerfile.lint (closes #113)

Every lint run now happens inside its own container, invoked through
script/lint, and linting is a build step rather than a container
command: a successful build of the new root Dockerfile.lint IS a clean
lint. That shape also works where the docker daemon is remote and bind
mounts are impossible.

Its FROM line -- golangci/golangci-lint:v2.12.2, pinned by digest -- is
now the only pin of the linter version in this repo.

A container per run has its own lint cache and its own golangci-lint
lock, both discarded with it, so neither cross-worktree contamination
nor lock contention exists any more. The machinery that defended
against them is therefore gone: the per-worktree cache directories, the
lock-retry loop, and script/lint-audit, which existed to catch findings
replayed from a cache that no longer exists. So is the host lint path
in its entirety -- the native escape hatch, its version detection, and
VAULTIK_LINT_IN_CONTAINER in both script/lint and the Dockerfile.
Nothing lints on the host, at any version.

A cached build lints nothing, so the CHECK_EPOCH mechanism the product
Dockerfile already used is what makes a green mean something:
ARG CHECK_EPOCH with no default, placed below the module layers so
dependency caching survives, a `RUN [ -n "$CHECK_EPOCH" ] || exit 1`
guard so a build that withholds the arg fails instead of replaying, and
the value expanded into the lint command itself. script/lint computes
`epoch="$(date +%s%N)$$"` as a bare assignment on its own line, because
inline in the argument a failing substitution does not abort under
`set -eu` and yields a constant empty epoch -- which is exactly the
false green being prevented.

The product Dockerfile loses its lint stage rather than gaining a
second linter pin. That stage ran `make lint`, which is now
`docker build`: docker-in-docker inside a BuildKit step with no daemon.
Calling golangci-lint directly there instead would have meant two
independently bumpable digests for one tool. `make fmt-check` moves
beside `make test` in the builder stage, and script/cibuild now builds
Dockerfile.lint and then Dockerfile, each with its own fresh epoch,
failing on either. Consequence, stated in comments rather than left to
be discovered: script/docker builds the product image only and no
longer lints; script/check and script/cibuild are the gates.

`golangci-lint config verify` runs as its own epoch-keyed layer, above
the lint. It is not belt-and-braces: `golangci-lint run` rejects a
config it cannot PARSE but silently IGNORES an unknown top-level KEY.
Renaming .golangci.yml's `linters:` to `linterz:` -- one character --
discards `default: all`, the disable list and every threshold, leaves
only the small default linter set running, and exits 0 reporting
`0 issues.` on a tree the real config fails with an lll finding, in a
run whose lint layer demonstrably executed. That is a set-but-
ineffective config falling back to defaults instead of failing loudly,
sitting in the gate's own configuration. `config verify` catches it and
does so with the network genuinely off at this pin: under
`docker run --network none` against the pinned digest it exits 0 on
this repo's config and exits 3 on the `linterz:` variant. It is keyed
on CHECK_EPOCH like the lint itself, because a cached validation
validates nothing.

script/lint-fix is kept, reimplemented as a bind-mounted
docker run against the image parsed out of Dockerfile.lint -- a build
step cannot write fixes back to the worktree -- and its header states
outright that it is a developer convenience, never a gate, and needs a
local daemon.

cmd/vaultik/lintdocker_test.go parses both Dockerfiles and both scripts
and fails if any part of the mechanism is dropped: the digest pin, the
defaultless ARG below `go mod download`, the emptiness guard, the
expansion of the epoch into each check command, the bare per-invocation
epoch assignment in both scripts, cibuild building both files, the
config verification running before the lint, and -- structurally, not
by searching for one retired variable name -- that no script invokes
golangci-lint except through docker. Every one of those losses is
silent: the build still exits 0 and nothing is checked, which is why
they are asserted rather than trusted. The scanner behind the last of
those has its own test, because a structural check that goes blind
passes on every tree, including a broken one.

script/lint takes no arguments now, and says so instead of dropping
them: a build step has no command line to pass linter flags to.

Dockerfile.lint

@@ -36,23 +36,7 @@ RUN go mod download
 
 COPY . .
 
-# `golangci-lint config verify` is deliberately NOT run here.
+# Force the check layers to execute on every invocation.
-#
-# It validates .golangci.yml against a JSON schema that it fetches over
-# live HTTPS from an unpinned URL at run time. Running it would make the
-# lint gate depend on a remote resource that is outside this repo's
-# hash-pinning discipline, and would turn an upstream outage or an
-# egress-less runner into a red that is not a lint verdict -- the exact
-# false-red class this repo has spent several issues eliminating. The
-# config is not unvalidated in practice either: `golangci-lint run`
-# below rejects an unparseable or unknown-key config itself, at the
-# version that is actually gating.
-#
-# If it is ever added, it must first be demonstrated to work with the
-# network genuinely off (`docker run --network none`) at the pinned
-# version, with that output recorded.
-
-# Force the lint layer to execute on every invocation.
 #
 # CHECK_EPOCH must stay immediately above the RUNs below. Those layers
 # are keyed on its value, so they are cache-eligible only for a value
@@ -62,8 +46,8 @@ COPY . .
 # Dockerfile.lint .` on an unchanged tree exits 0 in well under a second
 # having linted nothing.
 #
-# The value is expanded into the lint command itself rather than left to
+# The value is expanded into each check command itself rather than left
-# a bare declaration, so the cache miss does not depend on BuildKit's
+# to a bare declaration, so the cache miss does not depend on BuildKit's
 # unreferenced-ARG handling staying as it is. It also puts the epoch in
 # the build log, where a reader can see the layer was keyed fresh.
 #
@@ -77,5 +61,44 @@ COPY . .
 # constant and restore the hole.
 ARG CHECK_EPOCH
 RUN [ -n "$CHECK_EPOCH" ] || exit 1
+
+# Validate .golangci.yml before linting with it.
+#
+# This is not belt-and-braces; it closes a hole that `golangci-lint run`
+# leaves wide open. `run` rejects YAML it cannot PARSE, but it silently
+# IGNORES an unknown top-level KEY. Renaming `linters:` to `linterz:` --
+# one character -- discards `default: all`, the whole disable list and
+# every threshold, leaves only golangci-lint's small default linter set
+# running, and exits 0 reporting `0 issues.` on a tree the real config
+# fails. Demonstrated on this repo at this pin, recorded on
+# https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted
+# over-length line, `script/lint` exits 1 naming the `lll` finding with
+# `linters:` and exits 0 with `linterz:`. A set-but-ineffective config
+# quietly falling back to defaults is precisely the false-green class
+# this gate exists to eliminate, so it must not sit in the gate's own
+# configuration.
+#
+# `config verify` catches it, and it does so OFFLINE at this pinned
+# version -- verified, not assumed. Under `docker run --network none`
+# against the pinned digest it exits 0 on this repo's config and exits 3
+# on the `linterz:` variant with `additional properties 'linterz' not
+# allowed`. An earlier revision of this file asserted the opposite, that
+# the schema is fetched over live HTTPS from an unpinned URL, and used
+# that to justify omitting this line. That claim was false at v2.12.2;
+# the schema is embedded. If a future bump reintroduces a network fetch
+# the failure is loud and this comment is where to record it.
+#
+# It is keyed on CHECK_EPOCH, like the lint run below, so it executes on
+# every invocation. Content-addressing alone would arguably be enough --
+# .golangci.yml arrives through `COPY . .`, so a cache hit here implies
+# a byte-identical config was validated when the layer really ran. That
+# argument is exactly the one that would also excuse caching the lint
+# layer, and this repo has ruled it insufficient: a cached check layer
+# checks nothing, and the cost of being wrong is silent. Forcing it costs
+# milliseconds and puts the epoch in the log, where a reader can see that
+# this validation ran rather than being replayed.
+RUN echo "check epoch: ${CHECK_EPOCH}" && \
+    golangci-lint config verify --config .golangci.yml
+
 RUN echo "check epoch: ${CHECK_EPOCH}" && \
     golangci-lint run --config .golangci.yml ./...

TODO.md

@@ -50,10 +50,13 @@ release" is exactly the contradiction
   product `Dockerfile` already used is what makes the green mean
   something: `ARG CHECK_EPOCH` with no default below the module layers,
   a `RUN [ -n "$CHECK_EPOCH" ] || exit 1` guard, the value expanded
-  into the lint command, and a fresh `$(date +%s%N)$$` per invocation
+  into each check command, and a fresh `$(date +%s%N)$$` per invocation
   computed as a bare assignment. `cmd/vaultik/lintdocker_test.go`
   parses both Dockerfiles and both scripts and fails if any part of
-  that is dropped, because every way of losing it is silent.
+  that is dropped, because every way of losing it is silent. Its
+  host-lint assertion is structural — no script runs `golangci-lint`
+  except through `docker` — rather than a search for the one retired
+  variable name, which nothing could ever reintroduce.
 
   The product `Dockerfile` lost its lint stage rather than gaining a
   second linter pin: `make lint` is now `docker build`, so the stage
@@ -66,11 +69,17 @@ release" is exactly the contradiction
   to be found: `script/docker` builds the product image only and no
   longer lints; the gates are `script/check` and `script/cibuild`.
 
-  Two deliberate decisions, both documented where they apply.
+  `golangci-lint config verify` runs as its own epoch-keyed layer,
-  `golangci-lint config verify` is omitted: it fetches its JSON schema
+  above the lint. `golangci-lint run` rejects a config it cannot parse
-  over an unpinned live HTTPS call, which would make the gate
+  but silently ignores an unknown top-level *key*: renaming `linters:`
-  network-dependent and turn an upstream outage into a red that is not
+  to `linterz:` discarded `default: all` and every threshold and still
-  a lint verdict. `script/lint-fix` is kept, reimplemented as a
+  exited 0 on a tree the real config fails. `config verify` catches
+  that, and it does so with the network off at this pin — checked under
+  `docker run --network none`, not assumed. An earlier revision omitted
+  it on the claim that it fetches its schema over live HTTPS; that
+  claim was false at v2.12.2.
+
+  `script/lint-fix` is kept, reimplemented as a
   bind-mounted `docker run` against the image parsed out of
   `Dockerfile.lint` — it cannot be a build step, because fixes have to
   land in the worktree — and marked in its header as a developer

cmd/vaultik/lintdocker_test.go

@@ -37,6 +37,11 @@ const (
 	cibuildScript     = "script/cibuild"
 )
 
+// linterBinary is the linter's command name. Every occurrence of it in
+// executable shell in this repo must be inside a docker invocation; see
+// TestNoHostLintPathRemains.
+const linterBinary = "golangci-lint"
+
 // checkEpochARG is the declaration, with no default value. A default
 // would satisfy the non-empty guard with a constant, and a constant is
 // a stable cache key: the checks would be replayed from cache forever
@@ -111,6 +116,38 @@ func TestLintDockerfileCannotBeCachedGreen(t *testing.T) {
 			" still cache", checkEpochARG)
 }
 
+// TestLintDockerfileVerifiesTheLinterConfig guards the validation of
+// .golangci.yml itself. `golangci-lint run` rejects a config it cannot
+// parse but silently IGNORES an unknown top-level key, so renaming
+// `linters:` to `linterz:` discards `default: all` and every threshold
+// and still exits 0 reporting no issues. `config verify` is what turns
+// that into a failure, and it has to run BEFORE the lint, or the lint
+// spends a minute reporting a verdict from a config already known to be
+// wrong.
+func TestLintDockerfileVerifiesTheLinterConfig(t *testing.T) {
+	t.Parallel()
+
+	found := instructions(t, lintDockerfile)
+	verify := linterBinary + " config verify"
+
+	verifyAt := indexContaining(found, verify)
+	require.GreaterOrEqual(t, verifyAt, 0,
+		"%s must run `%s --config .golangci.yml`: without it a typo'd"+
+			" top-level key in .golangci.yml is silently ignored and the"+
+			" gate passes with only the default linter set", lintDockerfile,
+		verify)
+
+	runAt := indexContaining(found, linterBinary+" run")
+	require.GreaterOrEqual(t, runAt, 0, "%s must lint", lintDockerfile)
+	assert.Less(t, verifyAt, runAt,
+		"%s must verify the config before linting with it", lintDockerfile)
+
+	// Keyed on the epoch like every other check layer, so it executes
+	// per invocation rather than being replayed. A cached validation
+	// validates nothing.
+	assertEpochExpandedInto(t, found, verify)
+}
+
 // TestProductDockerfileCannotBeCachedGreen holds the same line for the
 // checks that remain in the product image build.
 func TestProductDockerfileCannotBeCachedGreen(t *testing.T) {
@@ -187,6 +224,19 @@ func TestCibuildBuildsBothDockerfilesWithFreshEpochs(t *testing.T) {
 // a container; a PATH binary that happens to match the pinned version
 // is a different build reached by a different code path, and admitting
 // it is what lets a local pass disagree with CI.
+//
+// This asserts the PROPERTY -- no script invokes the linter except
+// through docker -- rather than the absence of any particular variable
+// name. An earlier version of this test looked only for the literal
+// VAULTIK_LINT_IN_CONTAINER, the name of the hatch that was removed
+// alongside it, so nothing could ever trip it again: a hatch under any
+// other name left it passing. A structural test that passes on a broken
+// tree is worse than no test, because it is what a later reader trusts
+// instead of re-deriving the invariant.
+//
+// script/lint-fix is not exempted. It is the one script that runs the
+// linter as a container rather than as a build step, but it still runs
+// it in one, so the same property holds of it.
 func TestNoHostLintPathRemains(t *testing.T) {
 	t.Parallel()
 
@@ -194,21 +244,63 @@ func TestNoHostLintPathRemains(t *testing.T) {
 
 	entries, err := os.ReadDir(filepath.Join(root, "script"))
 	require.NoError(t, err)
+	require.NotEmpty(t, entries, "no scripts found to scan")
 
 	for _, entry := range entries {
 		if entry.IsDir() {
 			continue
 		}
 
-		contents := readRepoFile(t, filepath.Join("script", entry.Name()))
+		name := filepath.Join("script", entry.Name())
-		assert.NotContains(t, contents, "VAULTIK_LINT_IN_CONTAINER",
+		for _, line := range shellCode(readRepoFile(t, name)) {
-			"script/%s revives the in-container escape hatch",
+			assertLinterIsContainerised(t, name, line)
-			entry.Name())
+		}
+	}
+}
+
+// assertLinterIsContainerised fails if the line runs the linter without
+// handing it to docker first. Position matters: docker has to come
+// before the binary, or the line is running the host linter and merely
+// mentioning docker afterwards.
+func assertLinterIsContainerised(t *testing.T, name, line string) {
+	t.Helper()
+
+	at := strings.Index(line, linterBinary)
+	if at < 0 {
+		return
 	}
 
-	assert.NotContains(t, readRepoFile(t, lintDockerfile),
+	docker := strings.Index(line, "docker")
-		"VAULTIK_LINT_IN_CONTAINER",
+
-		"%s revives the in-container escape hatch", lintDockerfile)
+	assert.True(t, docker >= 0 && docker < at,
+		"%s runs %s on the host; every lint run happens in a container"+
+			" (line: %s)", name, linterBinary, line)
+}
+
+// TestShellCodeSeesCodeAndNotProse keeps the scanner above honest. It
+// has to ignore comments and here-document bodies, because script/lint
+// and script/bootstrap both NAME golangci-lint in prose -- in comments,
+// and in the error text they print -- precisely to say that the host
+// binary is never used. A scanner that went blind, by over-eager
+// stripping or by failing to join continuation lines, would make
+// TestNoHostLintPathRemains pass on everything.
+func TestShellCodeSeesCodeAndNotProse(t *testing.T) {
+	t.Parallel()
+
+	script := strings.Join([]string{
+		"#!/bin/sh",
+		"# a comment naming golangci-lint",
+		"cat >&2 <<EOF",
+		"prose naming golangci-lint, printed not executed",
+		"EOF",
+		"docker run --rm \\",
+		"    \"$image\" \\",
+		"    golangci-lint run ./...",
+	}, "\n")
+
+	assert.Equal(t,
+		[]string{"cat >&2 <<EOF", `docker run --rm "$image" golangci-lint run ./...`},
+		shellCode(script))
 }
 
 // assertEpochExpandedInto fails unless some instruction runs the named
@@ -303,6 +395,82 @@ func indexOf(found []string, want string) int {
 	return -1
 }
 
+// indexContaining returns the position of the first instruction
+// containing want; -1 if there is none.
+func indexContaining(found []string, want string) int {
+	for i, instruction := range found {
+		if strings.Contains(instruction, want) {
+			return i
+		}
+	}
+
+	return -1
+}
+
+// shellCode returns a POSIX shell script's executable lines: comments
+// dropped, here-document bodies dropped, and backslash continuations
+// joined so a multi-line command is a single string. Whitespace is
+// collapsed, as it is for Dockerfile instructions.
+//
+// Both exclusions are load-bearing rather than tidiness. The scripts
+// name golangci-lint in prose to state that the host binary is never
+// used, and joining continuations is what lets the one legitimate
+// container invocation -- script/lint-fix's `docker run`, whose linter
+// command sits several lines below the word `docker` -- be recognised
+// as containerised.
+func shellCode(contents string) []string {
+	var (
+		out       []string
+		joined    string
+		terminate string
+	)
+
+	for line := range strings.SplitSeq(contents, "\n") {
+		trimmed := strings.TrimSpace(line)
+
+		if terminate != "" {
+			if trimmed == terminate {
+				terminate = ""
+			}
+
+			continue
+		}
+
+		if joined == "" && (trimmed == "" || strings.HasPrefix(trimmed, "#")) {
+			continue
+		}
+
+		joined += strings.TrimSuffix(trimmed, `\`) + " "
+		if strings.HasSuffix(trimmed, `\`) {
+			continue
+		}
+
+		joined = strings.Join(strings.Fields(joined), " ")
+		terminate = heredocTerminator(joined)
+
+		out = append(out, joined)
+		joined = ""
+	}
+
+	return out
+}
+
+// heredocTerminator returns the terminator of the here-document a
+// command opens, or "" if it opens none. Only the first on a line is
+// recognised; nothing in script/ opens two.
+func heredocTerminator(line string) string {
+	_, after, opens := strings.Cut(line, "<<")
+	if !opens {
+		return ""
+	}
+
+	// `<<-` strips leading tabs from the body; the terminator word is
+	// the same either way, and callers compare against trimmed lines.
+	word, _, _ := strings.Cut(strings.TrimPrefix(after, "-"), " ")
+
+	return strings.Trim(word, `'"`)
+}
+
 // readRepoFile reads a file by its path relative to the repository
 // root.
 func readRepoFile(t *testing.T, name string) string {

script/lint

@@ -19,8 +19,9 @@
 #
 #   BUILDKIT_PROGRESS=plain script/lint
 #
-# The lint layer must appear as executing rather than CACHED on every
+# The check layers -- `golangci-lint config verify` and then
-# run; see the CHECK_EPOCH comment in Dockerfile.lint.
+# `golangci-lint run` -- must appear as executing rather than CACHED on
+# every run; see the CHECK_EPOCH comment in Dockerfile.lint.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -73,7 +74,7 @@ main() {
     cd "$ROOT"
     require_docker
 
-    # A fresh epoch per invocation is what forces the lint layer to
+    # A fresh epoch per invocation is what forces the check layers to
     # execute; the layers above the ARG in Dockerfile.lint still cache,
     # so a run is not cold. The value must be unique per invocation, not
     # per second: `date +%s` is second-granular, so two concurrent