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

Harden the pinned-linter gate: context-gate the native path, and make bootstrap yield a working machine #80

New Issue
Closed
opened 2026-08-09 04:52:49 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 04:52:49 +02:00
Collaborator
Copy Link

Follow-up to #78 / PR #79, from the non-blocking findings in that PR's
review (#79 (comment)).
None of these blocked the merge; all are worth closing out while the
context is fresh.

1. The native-lint escape hatch is version-gated, not context-gated

script/lint runs the PATH binary when its version equals the pin:

if [ "$(installed_version)" = "$(pinned_version)" ]; then
    exec golangci-lint run "$@" ./...
fi

Its purpose is narrow — the Dockerfile runs make lint inside the
pinned image, where there is no Docker daemon. But the condition it
actually tests is version equality, which also admits a developer's
locally installed 2.12.2. That is a different binary from the pinned
image (different build, different Go toolchain), reached by a different
code path, and it silently bypasses the digest pin this whole mechanism
exists to enforce.

The review confirmed the hatch cannot currently produce a silent false
green — the both-empty case fails loudly with exit 127 — so this is
hardening, not a live bug.

Fix: gate on execution context as well as version, e.g. additionally
require /.dockerenv to exist, or have the Dockerfile set an env var
(VAULTIK_LINT_IN_CONTAINER=1) that script/lint checks. A host with
2.12.2 on PATH must still go through the pinned image.

2. script/bootstrap no longer produces a machine that can run make check

script/bootstrap stopped installing golangci-lint (correctly — a
second copy on PATH can only drift). But Docker is now required by
script/lint → script/check → script/precommit, and bootstrap only
warns when Docker is missing, then exits 0 printing bootstrap complete. A contributor follows the documented flow, is told it
succeeded, and then make check fails.

The warning text is also two sentence fragments and never says what will
break.

Fix: bootstrap either installs Docker or fails loudly with an
actionable message naming exactly what will not work without it. "Bootstrap
complete" must mean the machine can run the gate.

3. Hand-rolled version scraping where a real interface exists

script/lint:39-50 scrapes the golangci-lint version banner with awk.
golangci-lint version --short exists and prints the bare version in
both 2.10.1 and 2.12.2 (verified in review). If a future release restores
a leading v to the banner, the parse breaks, the in-container path
silently disappears, and the lint stage fails with a misleading "docker is
required to run the pinned linter" — inside a container.

Fix: use --short, keeping a fallback only if needed for an older
release the repo still supports.

4. TODO.md overstates the guarantee and contradicts README.md

TODO.md says make check is now "as trustworthy as script/cibuild".
Only the lint leg is equivalent — tests and gofmt still run on the
host, against the host toolchain. README.md, changed in the same commit,
states this correctly.

Given that this repo has already produced two false green claims, an
overstated trust claim in the tracking doc is exactly the wrong error to
leave in place. Fix: make TODO.md match README.md.

5. README.md requirements section is now incomplete

## requirements still lists only Go 1.26+ and object storage. Docker is
now needed to lint, check, or commit. Fix: add it.

Definition of done

  1. Items 1-5 addressed.
  2. Item 1 verified concretely: on a host with 2.12.2 installed on PATH,
    make lint demonstrably still runs the pinned image. Record how this
    was verified.
  3. Item 2 verified: on a machine without Docker, script/bootstrap does
    not report success. Record the output.
  4. No change to .golangci.yml (sha256
    021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb).
  5. The Dockerfile FROM line remains the single source of truth for
    the linter version.
  6. script/cibuild exits 0.
Follow-up to #78 / PR #79, from the non-blocking findings in that PR's review (https://git.eeqj.de/sneak/vaultik/pulls/79#issuecomment-46683). None of these blocked the merge; all are worth closing out while the context is fresh. ## 1. The native-lint escape hatch is version-gated, not context-gated `script/lint` runs the `PATH` binary when its version equals the pin: ```sh if [ "$(installed_version)" = "$(pinned_version)" ]; then exec golangci-lint run "$@" ./... fi ``` Its purpose is narrow — the `Dockerfile` runs `make lint` **inside** the pinned image, where there is no Docker daemon. But the condition it actually tests is version equality, which also admits a developer's locally installed 2.12.2. That is a different binary from the pinned image (different build, different Go toolchain), reached by a different code path, and it silently bypasses the digest pin this whole mechanism exists to enforce. The review confirmed the hatch cannot currently produce a *silent* false green — the both-empty case fails loudly with exit 127 — so this is hardening, not a live bug. **Fix:** gate on execution context as well as version, e.g. additionally require `/.dockerenv` to exist, or have the `Dockerfile` set an env var (`VAULTIK_LINT_IN_CONTAINER=1`) that `script/lint` checks. A host with 2.12.2 on `PATH` must still go through the pinned image. ## 2. `script/bootstrap` no longer produces a machine that can run `make check` `script/bootstrap` stopped installing `golangci-lint` (correctly — a second copy on `PATH` can only drift). But Docker is now required by `script/lint` → `script/check` → `script/precommit`, and bootstrap only *warns* when Docker is missing, then exits 0 printing `bootstrap complete`. A contributor follows the documented flow, is told it succeeded, and then `make check` fails. The warning text is also two sentence fragments and never says what will break. **Fix:** bootstrap either installs Docker or fails loudly with an actionable message naming exactly what will not work without it. "Bootstrap complete" must mean the machine can run the gate. ## 3. Hand-rolled version scraping where a real interface exists `script/lint:39-50` scrapes the `golangci-lint version` banner with awk. `golangci-lint version --short` exists and prints the bare version in both 2.10.1 and 2.12.2 (verified in review). If a future release restores a leading `v` to the banner, the parse breaks, the in-container path silently disappears, and the lint stage fails with a misleading "docker is required to run the pinned linter" — inside a container. **Fix:** use `--short`, keeping a fallback only if needed for an older release the repo still supports. ## 4. `TODO.md` overstates the guarantee and contradicts `README.md` `TODO.md` says `make check` is now "as trustworthy as `script/cibuild`". Only the **lint** leg is equivalent — tests and `gofmt` still run on the host, against the host toolchain. `README.md`, changed in the same commit, states this correctly. Given that this repo has already produced two false green claims, an overstated trust claim in the tracking doc is exactly the wrong error to leave in place. **Fix:** make `TODO.md` match `README.md`. ## 5. `README.md` requirements section is now incomplete `## requirements` still lists only Go 1.26+ and object storage. Docker is now needed to lint, check, or commit. **Fix:** add it. ## Definition of done 1. Items 1-5 addressed. 2. Item 1 verified concretely: on a host with 2.12.2 installed on `PATH`, `make lint` demonstrably still runs the pinned image. Record how this was verified. 3. Item 2 verified: on a machine without Docker, `script/bootstrap` does not report success. Record the output. 4. No change to `.golangci.yml` (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`). 5. The `Dockerfile` `FROM` line remains the single source of truth for the linter version. 6. `script/cibuild` exits 0.
clawbot added this to the 1.0.0 milestone 2026-08-09 04:52:49 +02:00
clawbot commented 2026-08-09 16:32:29 +02:00
Author
Collaborator
Copy Link

Implementation plan (fixed together with #99 in one PR, branch
fix-lint-isolation: both are the same defect in script/lint — the
native path is gated on version equality rather than execution context,
and cache isolation is part of that context).

  1. Context gate. The Dockerfile lint stage gets
    ENV VAULTIK_LINT_IN_CONTAINER=1, and script/lint takes the native
    path only when that variable is set and the PATH version equals
    the pin. /.dockerenv was considered and rejected as the signal: it
    is created by dockerd for docker run, and is not reliably present
    during a BuildKit docker build, which is exactly the case the hatch
    exists for. Inside the container a version mismatch becomes a hard,
    explicit error rather than a fall-through to Docker, since there is
    no daemon there. A host with 2.12.2 on PATH and no such variable
    goes through the pinned image.

  2. Bootstrap. Docker becomes a hard requirement: script/bootstrap
    fails, naming script/lint, script/check, script/precommit and
    the pinned image, rather than warning and printing bootstrap complete. It will also reject a present-but-unreachable daemon,
    since a machine with the CLI and no daemon fails make check exactly
    the same way. Installing Docker from bootstrap was rejected: it needs
    root, a daemon and (on macOS) a GUI cask, so an install attempt would
    itself fail in the common case and produce a second false success
    mode.

  3. golangci-lint version --short replaces the awk banner scrape,
    with the scrape kept only as a fallback if --short is unsupported
    or prints nothing.

  4. TODO.md claim that make check is "as trustworthy as
    script/cibuild" is corrected to match README.md: only the lint
    leg is equivalent; tests and gofmt still run on the host toolchain.

  5. README.md ## requirements gains Docker, with what it is for.

Verification will be behavioural, not by inspection: with a matching
golangci-lint on PATH, confirm make lint still goes through the
pinned image; with docker shadowed on PATH, confirm script/bootstrap
does not report success; and script/cibuild (which runs make lint
inside the pinned image with no daemon) exits 0, which is what proves the
in-container path still works. .golangci.yml is untouched (sha256
verified before push) and the lint-stage FROM line remains the single
source of truth for the linter version.

Implementation plan (fixed together with #99 in one PR, branch `fix-lint-isolation`: both are the same defect in `script/lint` — the native path is gated on version equality rather than execution context, and cache isolation is part of that context). 1. **Context gate.** The `Dockerfile` lint stage gets `ENV VAULTIK_LINT_IN_CONTAINER=1`, and `script/lint` takes the native path only when that variable is set **and** the `PATH` version equals the pin. `/.dockerenv` was considered and rejected as the signal: it is created by `dockerd` for `docker run`, and is not reliably present during a BuildKit `docker build`, which is exactly the case the hatch exists for. Inside the container a version mismatch becomes a hard, explicit error rather than a fall-through to Docker, since there is no daemon there. A host with 2.12.2 on `PATH` and no such variable goes through the pinned image. 2. **Bootstrap.** Docker becomes a hard requirement: `script/bootstrap` fails, naming `script/lint`, `script/check`, `script/precommit` and the pinned image, rather than warning and printing `bootstrap complete`. It will also reject a present-but-unreachable daemon, since a machine with the CLI and no daemon fails `make check` exactly the same way. Installing Docker from bootstrap was rejected: it needs root, a daemon and (on macOS) a GUI cask, so an install attempt would itself fail in the common case and produce a second false success mode. 3. **`golangci-lint version --short`** replaces the awk banner scrape, with the scrape kept only as a fallback if `--short` is unsupported or prints nothing. 4. **`TODO.md`** claim that `make check` is "as trustworthy as `script/cibuild`" is corrected to match `README.md`: only the lint leg is equivalent; tests and `gofmt` still run on the host toolchain. 5. **`README.md`** `## requirements` gains Docker, with what it is for. Verification will be behavioural, not by inspection: with a matching `golangci-lint` on `PATH`, confirm `make lint` still goes through the pinned image; with `docker` shadowed on `PATH`, confirm `script/bootstrap` does not report success; and `script/cibuild` (which runs `make lint` inside the pinned image with no daemon) exits 0, which is what proves the in-container path still works. `.golangci.yml` is untouched (sha256 verified before push) and the lint-stage `FROM` line remains the single source of truth for the linter version.
clawbot commented 2026-08-09 16:49:14 +02:00
Author
Collaborator
Copy Link

Implemented in PR #102 (branch fix-lint-isolation, one commit,
together with #99). Point by point against the five items and the
definition of done.

1. Context gate. The Dockerfile lint stage sets
ENV VAULTIK_LINT_IN_CONTAINER=1, and script/lint takes the native
path only when that is set and the PATH version equals the pin.
Inside the container a version mismatch is now a hard error naming both
versions, rather than a fall-through to Docker that cannot work there.
/.dockerenv was rejected as the signal: dockerd creates it for
docker run, but it is not reliably present during a BuildKit
docker build, which is exactly the case the exception exists for.

Verified: a golangci-lint shim on PATH reporting 2.12.2 and
logging every invocation was never invoked by make lint, which ran
the pinned image (its output carries the image's gomodguard
deprecation warnings, which the shim does not emit). Re-running the same
command with VAULTIK_LINT_IN_CONTAINER=1 invoked the shim
(version --short, then run ./...), which confirms that variable is
the only door. With the shim reporting 2.10.1 and the variable set,
exit 1 with pinned: 2.12.2 ... installed: 2.10.1.

2. Bootstrap. Docker is now a hard requirement, checked last so that
everything installable is installed first. Missing docker, or a docker
whose daemon is unreachable, fails with a message naming script/lint,
script/check, script/precommit and script/cibuild and what each
loses. Installing docker from bootstrap was rejected: it needs root, a
running daemon, and on macOS a GUI cask, so the attempt would itself
fail in the common case and trade one false success for a second failure
mode.

Verified: with a PATH containing the toolchain but no docker,
bootstrap: FAILED - docker is not installed., exit 1, and zero
occurrences of bootstrap complete in the output. With a docker shim
whose info fails, bootstrap: FAILED - the docker daemon is not reachable., exit 1. Unchanged happy path on a normal host: exit 0,
bootstrap complete.

3. Version scraping. golangci-lint version --short is now the
parse; the awk banner scrape survives only as a fallback for a release
where --short is absent or silent.

4. TODO.md overstatement. Corrected in place to match
README.md: only the lint leg of make check became equivalent to
script/cibuild, while tests and gofmt still run on the host against
the host toolchain. The correction says so explicitly rather than
quietly deleting the claim.

5. README.md requirements. Gained docker (with a reachable daemon,
and why: script/lint runs the pinned image, and make check and the
pre-commit hook run it) and the sqlite3 CLI the test suite shells out
to.

Definition of done: items 1-5 addressed; items 1 and 2 verified
behaviourally as recorded above; .golangci.yml untouched, sha256
021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb
verified before pushing; the Dockerfile FROM line and digest are
unchanged and remain the single source of truth; script/cibuild exits
0, with the [lint 9/9] ... make lint layer executing for 78.7s inside
the pinned image and printing 0 issues. — which is also the proof that
the in-container native path still works after the gate was tightened.

Implemented in PR #102 (branch `fix-lint-isolation`, one commit, together with #99). Point by point against the five items and the definition of done. **1. Context gate.** The `Dockerfile` lint stage sets `ENV VAULTIK_LINT_IN_CONTAINER=1`, and `script/lint` takes the native path only when that is set **and** the `PATH` version equals the pin. Inside the container a version mismatch is now a hard error naming both versions, rather than a fall-through to Docker that cannot work there. `/.dockerenv` was rejected as the signal: `dockerd` creates it for `docker run`, but it is not reliably present during a BuildKit `docker build`, which is exactly the case the exception exists for. *Verified*: a `golangci-lint` shim on `PATH` reporting `2.12.2` and logging every invocation was **never invoked** by `make lint`, which ran the pinned image (its output carries the image's `gomodguard` deprecation warnings, which the shim does not emit). Re-running the same command with `VAULTIK_LINT_IN_CONTAINER=1` invoked the shim (`version --short`, then `run ./...`), which confirms that variable is the only door. With the shim reporting `2.10.1` and the variable set, exit 1 with `pinned: 2.12.2 ... installed: 2.10.1`. **2. Bootstrap.** Docker is now a hard requirement, checked last so that everything installable is installed first. Missing docker, or a docker whose daemon is unreachable, fails with a message naming `script/lint`, `script/check`, `script/precommit` and `script/cibuild` and what each loses. Installing docker from bootstrap was rejected: it needs root, a running daemon, and on macOS a GUI cask, so the attempt would itself fail in the common case and trade one false success for a second failure mode. *Verified*: with a `PATH` containing the toolchain but no `docker`, `bootstrap: FAILED - docker is not installed.`, exit 1, and zero occurrences of `bootstrap complete` in the output. With a `docker` shim whose `info` fails, `bootstrap: FAILED - the docker daemon is not reachable.`, exit 1. Unchanged happy path on a normal host: exit 0, `bootstrap complete`. **3. Version scraping.** `golangci-lint version --short` is now the parse; the awk banner scrape survives only as a fallback for a release where `--short` is absent or silent. **4. `TODO.md` overstatement.** Corrected in place to match `README.md`: only the **lint** leg of `make check` became equivalent to `script/cibuild`, while tests and `gofmt` still run on the host against the host toolchain. The correction says so explicitly rather than quietly deleting the claim. **5. `README.md` requirements.** Gained docker (with a reachable daemon, and why: `script/lint` runs the pinned image, and `make check` and the pre-commit hook run it) and the `sqlite3` CLI the test suite shells out to. **Definition of done**: items 1-5 addressed; items 1 and 2 verified behaviourally as recorded above; `.golangci.yml` untouched, sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` verified before pushing; the `Dockerfile` `FROM` line and digest are unchanged and remain the single source of truth; `script/cibuild` exits 0, with the `[lint 9/9] ... make lint` layer executing for 78.7s inside the pinned image and printing `0 issues.` — which is also the proof that the in-container native path still works after the gate was tightened.
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#80
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?