Make a missing CHECK_EPOCH fail the build instead of faking it (closes #91) #92
49
Dockerfile
49
Dockerfile
@@ -23,32 +23,32 @@ COPY . .
|
||||
#
|
||||
# CHECK_EPOCH must stay immediately above these RUNs. These layers are
|
||||
# keyed on its value, so they are cache-eligible only for a value
|
||||
# already built against this same tree. script/cibuild passes a fresh
|
||||
# value on every invocation, which is what makes its green mean the
|
||||
# checks really executed.
|
||||
# already built against this same tree. script/cibuild and script/docker
|
||||
# each pass a fresh value on every invocation, which is what makes their
|
||||
# green mean the checks really executed.
|
||||
#
|
||||
# The guarantee is conditional on that fresh value, not absolute. A
|
||||
# build that omits --build-arg -- a bare `docker build .` -- gets an
|
||||
# empty CHECK_EPOCH, and an empty string is a constant: the first such
|
||||
# build runs the checks, and every one after it on an unchanged tree
|
||||
# replays these layers from cache, never executing a check and still
|
||||
# exiting 0, a green nothing earned. Gate through script/cibuild.
|
||||
# Making the missing-arg case fail loudly instead is tracked in #91.
|
||||
# The value is expanded into each check command rather than left 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.
|
||||
#
|
||||
# CHECK_EPOCH is deliberately not referenced by the commands below: a
|
||||
# declared-but-unreferenced ARG does enter BuildKit's cache key, which
|
||||
# is measured on this host rather than assumed (PR #89). Upstream
|
||||
# sneak/prompts #26 prefers expanding the value into the command so
|
||||
# that the miss is contractual rather than dependent on that behavior
|
||||
# staying as it is; adopting that here is tracked in #91. Do not delete
|
||||
# this ARG as dead code -- the gate depends on it.
|
||||
# The guard is what makes a build that omits --build-arg fail instead of
|
||||
# lie. An unset ARG is an empty string, and an empty string is a
|
||||
# perfectly stable cache key: without the guard the first such build
|
||||
# runs the checks and every one after it on an unchanged tree replays
|
||||
# these layers from cache, executes nothing, and still exits 0. Failed
|
||||
# steps are never cached, so the guard fails on EVERY invocation rather
|
||||
# than once -- a bare `docker build .` is now a loud error, not a quiet
|
||||
# green. Do not give CHECK_EPOCH a default value; a default would
|
||||
# satisfy the guard with a constant and restore the hole.
|
||||
#
|
||||
# ARG scope is per-stage, so the builder stage declares its own.
|
||||
# Everything above this line (apk, go.mod, `go mod download`) is
|
||||
# deliberately outside the busted range and keeps caching.
|
||||
ARG CHECK_EPOCH
|
||||
RUN make fmt-check
|
||||
RUN make lint
|
||||
RUN [ -n "$CHECK_EPOCH" ] || exit 1
|
||||
RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check
|
||||
RUN echo "check epoch: ${CHECK_EPOCH}" && make lint
|
||||
|
||||
# Build stage
|
||||
# golang:1.26.1-alpine, 2026-03-17
|
||||
@@ -71,12 +71,13 @@ RUN go mod download
|
||||
# Copy source code
|
||||
COPY . .
|
||||
|
||||
# Run tests. See the CHECK_EPOCH comment in the lint stage, including
|
||||
# the conditions the guarantee depends on; ARG scope is per-stage, so
|
||||
# this stage needs its own declaration, and it must stay immediately
|
||||
# above the check RUN.
|
||||
# Run tests. See the CHECK_EPOCH comment in the lint stage for the
|
||||
# mechanism; ARG scope is per-stage, so this stage needs its own
|
||||
# declaration, its own guard, and its own expansion, and they must stay
|
||||
# immediately above the check RUN.
|
||||
ARG CHECK_EPOCH
|
||||
RUN make test
|
||||
RUN [ -n "$CHECK_EPOCH" ] || exit 1
|
||||
RUN echo "check epoch: ${CHECK_EPOCH}" && make test
|
||||
|
||||
# Build (pure Go, no CGO required since we use modernc.org/sqlite)
|
||||
RUN CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/globals.Version=${VERSION}' -X 'sneak.berlin/go/vaultik/internal/globals.Commit=$(git rev-parse HEAD 2>/dev/null || echo unknown)' -X 'sneak.berlin/go/vaultik/internal/globals.CommitDate=$(git show -s --format=%cs HEAD 2>/dev/null || echo unknown)'" -o /vaultik ./cmd/vaultik
|
||||
|
||||
32
README.md
32
README.md
@@ -616,25 +616,31 @@ them. We provide:
|
||||
the pinned linter: a local `make check` and CI cannot disagree about
|
||||
lint findings.
|
||||
* `script/docker` — build the Docker image tagged via
|
||||
`script/projectname`
|
||||
`script/projectname`. Passes a fresh `--build-arg CHECK_EPOCH` for the
|
||||
same reason `script/cibuild` does, so a local image build cannot be
|
||||
green on checks it replayed from cache.
|
||||
* `script/cibuild` — CI entrypoint: `docker build` (the `Dockerfile`
|
||||
runs `make fmt-check` and `make lint` in its lint stage and `make
|
||||
test` in its builder stage). This is the full CI-equivalent gate — it
|
||||
runs the checks in the same containers CI does, from a clean copy of
|
||||
the tree, so it also catches anything that depends on host state. It
|
||||
passes a fresh `--build-arg CHECK_EPOCH`, which the `Dockerfile`
|
||||
declares immediately above the check `RUN`s in both stages. Those
|
||||
layers are keyed on that value, so a new value re-runs them even on a
|
||||
byte-identical tree, and a green from this script means the checks
|
||||
executed. Dependency and module layers sit above the `ARG` and still
|
||||
cache, so a build is not cold.
|
||||
passes a fresh `--build-arg CHECK_EPOCH`, unique per invocation, which
|
||||
the `Dockerfile` declares immediately above the check `RUN`s in both
|
||||
stages and expands into each check command. Those layers are keyed on
|
||||
that value, so a new value re-runs them even on a byte-identical tree,
|
||||
and a green from this script means the checks executed. Dependency and
|
||||
module layers sit above the `ARG` and still cache, so a build is not
|
||||
cold.
|
||||
|
||||
That guarantee is conditional on the fresh value, so **run the gate
|
||||
through `script/cibuild`, not by invoking `docker build` yourself**. A
|
||||
bare `docker build .` supplies no `CHECK_EPOCH`; the empty default is
|
||||
a constant, so the second and every later build on an unchanged tree
|
||||
serves all three check layers from cache, executes nothing, and still
|
||||
exits 0. Issue #91 tracks making that case fail loudly instead.
|
||||
A build that supplies no `CHECK_EPOCH` — a bare `docker build .` —
|
||||
fails rather than lying. An unset `ARG` is an empty string and an
|
||||
empty string is a stable cache key, so without a guard such a build
|
||||
would serve all three check layers from cache, execute nothing, and
|
||||
still exit 0. Each check stage therefore asserts the value is
|
||||
non-empty before running anything, and because failed steps are never
|
||||
cached that assertion fires on every invocation rather than once. Use
|
||||
`script/cibuild` (or `script/docker`, which passes the same arg); a
|
||||
bare `docker build .` is now a loud error.
|
||||
* `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must
|
||||
not change files), then `script/check`
|
||||
* `script/install-precommit` — install the git pre-commit hook that
|
||||
|
||||
36
TODO.md
36
TODO.md
@@ -19,6 +19,42 @@ or delete the branch.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-09: Adopted the remaining upstream `CHECK_EPOCH` hardening
|
||||
(issue #91), closing the gap #85 knowingly left open. Four changes,
|
||||
all four decided as adopt upstream in `sneak/prompts` #26. (1) Each
|
||||
check stage now asserts `[ -n "$CHECK_EPOCH" ] || exit 1` before
|
||||
running anything, so a build that supplies no `--build-arg` fails
|
||||
instead of lying. This is the item that mattered: an unset `ARG` is
|
||||
an empty string and an empty string is a stable cache key, so the
|
||||
second and every later bare `docker build .` on an unchanged tree
|
||||
replayed all three check layers and still exited 0 — and `docker
|
||||
build .` is the command `REPO_POLICIES.md` names verbatim as a thing
|
||||
that must be green, so the documented command was precisely the one
|
||||
that lied. Failed steps are never cached, which is what makes the
|
||||
guard fire on every invocation rather than once. (2) The epoch is now
|
||||
expanded into each check command rather than left as a bare
|
||||
declaration, so the cache miss no longer depends on BuildKit's
|
||||
unreferenced-`ARG` handling staying as it is, and the value appears
|
||||
in the build log. (3) `script/cibuild` uses
|
||||
`epoch="$(date +%s%N)$$"`, unique per invocation rather than per
|
||||
second; `%N` alone is insufficient because busybox drops it silently
|
||||
and exits 0, and `$$` is what makes the guarantee hold regardless.
|
||||
The bare-assignment form is kept deliberately — inlined in an
|
||||
argument, a failing substitution does not abort under `set -eu` and
|
||||
would yield an empty constant epoch, restoring the exact false green
|
||||
being fixed. (4) `script/docker` passes the same fresh arg, so the
|
||||
two entrypoints cannot disagree about whether the tree is green;
|
||||
local builds are almost always warm, which made it the likelier
|
||||
fooling in practice. The `ARG` placement from #85 is unchanged, below
|
||||
`apk add`, `COPY go.mod go.sum` and `go mod download`, so dependency
|
||||
layers still cache and the build is not cold. Verified by negative
|
||||
control rather than inspection — a bare `docker build .` run twice
|
||||
back to back, plus back-to-back pairs of both scripts and a host-side
|
||||
`make check`; the measurements are recorded once, in the PR
|
||||
verification comment, rather than restated here. `.golangci.yml`, the
|
||||
lint-stage `FROM` line and its digest, `script/lint`,
|
||||
`REPO_POLICIES.md` and `.gitea/workflows/check.yml` are all
|
||||
untouched.
|
||||
- 2026-08-09: Stopped `script/cibuild` from reporting a green it did
|
||||
not earn (issue #85). A bare `docker build .` let Docker serve the
|
||||
check layers from the layer cache whenever the tree had not changed:
|
||||
|
||||
@@ -15,7 +15,17 @@ main() {
|
||||
# fresh value here is what forces them to re-run: without it an
|
||||
# unchanged tree replays them from cache, the checks never execute,
|
||||
# and the build still exits 0. The ARG sits immediately above the
|
||||
# check RUNs, so dependency and module layers still cache.
|
||||
# check RUNs, so dependency and module layers still cache. The
|
||||
# Dockerfile also refuses to build at all when CHECK_EPOCH is empty,
|
||||
# so a missing value fails loudly here rather than passing quietly.
|
||||
#
|
||||
# The value must be unique per invocation, not per second. `date +%s`
|
||||
# is second-granular, so two concurrent invocations in the same
|
||||
# second get identical epochs and the later one can be served from
|
||||
# cache -- the original defect in miniature. `%N` alone does not fix
|
||||
# it: busybox silently drops %N, exits 0, and hands back second
|
||||
# granularity with no warning. `$$` is what makes this correct
|
||||
# regardless, since concurrent invocations have different pids.
|
||||
#
|
||||
# Assign the epoch on its own line rather than inline in the
|
||||
# argument. Under `set -eu` a command substitution that fails
|
||||
@@ -25,7 +35,7 @@ main() {
|
||||
# script exists to prevent -- so the guard would disarm itself and
|
||||
# still exit 0. As a bare assignment, `set -e` catches a failing
|
||||
# `date` and no build starts.
|
||||
epoch="$(date +%s)"
|
||||
epoch="$(date +%s%N)$$"
|
||||
docker build --build-arg CHECK_EPOCH="$epoch" .
|
||||
}
|
||||
|
||||
|
||||
@@ -9,7 +9,16 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
|
||||
|
||||
main() {
|
||||
cd "$ROOT"
|
||||
docker build -t "$("$SCRIPT_DIR/projectname")" .
|
||||
# Same CHECK_EPOCH contract as script/cibuild, for the same reason
|
||||
# and with the same bare-assignment and `$$` requirements -- see the
|
||||
# comments there. This script is not the CI gate, but a local build
|
||||
# is almost always warm, so without this it would report a green the
|
||||
# tree had not earned and the two entrypoints would disagree about
|
||||
# whether the tree is clean. The Dockerfile now refuses to build
|
||||
# without a non-empty value, so this is required, not optional.
|
||||
epoch="$(date +%s%N)$$"
|
||||
docker build --build-arg CHECK_EPOCH="$epoch" \
|
||||
-t "$("$SCRIPT_DIR/projectname")" .
|
||||
}
|
||||
|
||||
main "$@"
|
||||
|
||||
Reference in New Issue
Block a user