Make a missing CHECK_EPOCH fail the build instead of faking it (closes #91)
All checks were successful
check / check (pull_request) Successful in 2m13s
All checks were successful
check / check (pull_request) Successful in 2m13s
Adopt the four remaining upstream CHECK_EPOCH hardening items from sneak/prompts #26, closing the gap #85 left open deliberately. Fail closed on a missing value. Each check stage now asserts `[ -n "$CHECK_EPOCH" ] || exit 1` before running anything. 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, executed nothing, 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. Expand the epoch into each check command rather than leaving it a bare declaration, so the cache miss does not depend on BuildKit's unreferenced-ARG handling staying as it is, and so the value appears in the build log where a reader can see the layer was keyed fresh. Make the epoch unique per invocation rather than per second: `epoch="$(date +%s%N)$$"`. `%N` alone is not enough, since busybox drops it silently and exits 0, handing back second granularity with no warning; `$$` differs between concurrent invocations regardless. The bare-assignment form is kept on purpose -- inlined into an argument, a failing substitution does not abort under `set -eu` and would yield an empty constant epoch, restoring the exact false green this prevents. Pass the same fresh value from script/docker. It is not the CI gate, but local builds are almost always warm, so it was the likelier fooling in practice, and two entrypoints disagreeing about whether the tree is green is worse than either being wrong alone. 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; measurements are recorded once, in the PR verification comment. .golangci.yml, the lint-stage FROM line and its digest, script/lint, REPO_POLICIES.md and .gitea/workflows/check.yml are untouched.
This commit is contained in:
49
Dockerfile
49
Dockerfile
@@ -23,32 +23,32 @@ COPY . .
|
|||||||
#
|
#
|
||||||
# CHECK_EPOCH must stay immediately above these RUNs. These layers are
|
# CHECK_EPOCH must stay immediately above these RUNs. These layers are
|
||||||
# keyed on its value, so they are cache-eligible only for a value
|
# keyed on its value, so they are cache-eligible only for a value
|
||||||
# already built against this same tree. script/cibuild passes a fresh
|
# already built against this same tree. script/cibuild and script/docker
|
||||||
# value on every invocation, which is what makes its green mean the
|
# each pass a fresh value on every invocation, which is what makes their
|
||||||
# checks really executed.
|
# green mean the checks really executed.
|
||||||
#
|
#
|
||||||
# The guarantee is conditional on that fresh value, not absolute. A
|
# The value is expanded into each check command rather than left to a
|
||||||
# build that omits --build-arg -- a bare `docker build .` -- gets an
|
# bare declaration, so the cache miss does not depend on BuildKit's
|
||||||
# empty CHECK_EPOCH, and an empty string is a constant: the first such
|
# unreferenced-ARG handling staying as it is. It also puts the epoch in
|
||||||
# build runs the checks, and every one after it on an unchanged tree
|
# the build log, where a reader can see the layer was keyed fresh.
|
||||||
# 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.
|
|
||||||
#
|
#
|
||||||
# CHECK_EPOCH is deliberately not referenced by the commands below: a
|
# The guard is what makes a build that omits --build-arg fail instead of
|
||||||
# declared-but-unreferenced ARG does enter BuildKit's cache key, which
|
# lie. An unset ARG is an empty string, and an empty string is a
|
||||||
# is measured on this host rather than assumed (PR #89). Upstream
|
# perfectly stable cache key: without the guard the first such build
|
||||||
# sneak/prompts #26 prefers expanding the value into the command so
|
# runs the checks and every one after it on an unchanged tree replays
|
||||||
# that the miss is contractual rather than dependent on that behavior
|
# these layers from cache, executes nothing, and still exits 0. Failed
|
||||||
# staying as it is; adopting that here is tracked in #91. Do not delete
|
# steps are never cached, so the guard fails on EVERY invocation rather
|
||||||
# this ARG as dead code -- the gate depends on it.
|
# 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.
|
# ARG scope is per-stage, so the builder stage declares its own.
|
||||||
# Everything above this line (apk, go.mod, `go mod download`) is
|
# Everything above this line (apk, go.mod, `go mod download`) is
|
||||||
# deliberately outside the busted range and keeps caching.
|
# deliberately outside the busted range and keeps caching.
|
||||||
ARG CHECK_EPOCH
|
ARG CHECK_EPOCH
|
||||||
RUN make fmt-check
|
RUN [ -n "$CHECK_EPOCH" ] || exit 1
|
||||||
RUN make lint
|
RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check
|
||||||
|
RUN echo "check epoch: ${CHECK_EPOCH}" && make lint
|
||||||
|
|
||||||
# Build stage
|
# Build stage
|
||||||
# golang:1.26.1-alpine, 2026-03-17
|
# golang:1.26.1-alpine, 2026-03-17
|
||||||
@@ -71,12 +71,13 @@ RUN go mod download
|
|||||||
# Copy source code
|
# Copy source code
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
# Run tests. See the CHECK_EPOCH comment in the lint stage, including
|
# Run tests. See the CHECK_EPOCH comment in the lint stage for the
|
||||||
# the conditions the guarantee depends on; ARG scope is per-stage, so
|
# mechanism; ARG scope is per-stage, so this stage needs its own
|
||||||
# this stage needs its own declaration, and it must stay immediately
|
# declaration, its own guard, and its own expansion, and they must stay
|
||||||
# above the check RUN.
|
# immediately above the check RUN.
|
||||||
ARG CHECK_EPOCH
|
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)
|
# 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
|
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
|
the pinned linter: a local `make check` and CI cannot disagree about
|
||||||
lint findings.
|
lint findings.
|
||||||
* `script/docker` — build the Docker image tagged via
|
* `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`
|
* `script/cibuild` — CI entrypoint: `docker build` (the `Dockerfile`
|
||||||
runs `make fmt-check` and `make lint` in its lint stage and `make
|
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
|
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
|
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
|
the tree, so it also catches anything that depends on host state. It
|
||||||
passes a fresh `--build-arg CHECK_EPOCH`, which the `Dockerfile`
|
passes a fresh `--build-arg CHECK_EPOCH`, unique per invocation, which
|
||||||
declares immediately above the check `RUN`s in both stages. Those
|
the `Dockerfile` declares immediately above the check `RUN`s in both
|
||||||
layers are keyed on that value, so a new value re-runs them even on a
|
stages and expands into each check command. Those layers are keyed on
|
||||||
byte-identical tree, and a green from this script means the checks
|
that value, so a new value re-runs them even on a byte-identical tree,
|
||||||
executed. Dependency and module layers sit above the `ARG` and still
|
and a green from this script means the checks executed. Dependency and
|
||||||
cache, so a build is not cold.
|
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
|
A build that supplies no `CHECK_EPOCH` — a bare `docker build .` —
|
||||||
through `script/cibuild`, not by invoking `docker build` yourself**. A
|
fails rather than lying. An unset `ARG` is an empty string and an
|
||||||
bare `docker build .` supplies no `CHECK_EPOCH`; the empty default is
|
empty string is a stable cache key, so without a guard such a build
|
||||||
a constant, so the second and every later build on an unchanged tree
|
would serve all three check layers from cache, execute nothing, and
|
||||||
serves all three check layers from cache, executes nothing, and still
|
still exit 0. Each check stage therefore asserts the value is
|
||||||
exits 0. Issue #91 tracks making that case fail loudly instead.
|
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
|
* `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must
|
||||||
not change files), then `script/check`
|
not change files), then `script/check`
|
||||||
* `script/install-precommit` — install the git pre-commit hook that
|
* `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
|
# 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
|
- 2026-08-09: Stopped `script/cibuild` from reporting a green it did
|
||||||
not earn (issue #85). A bare `docker build .` let Docker serve the
|
not earn (issue #85). A bare `docker build .` let Docker serve the
|
||||||
check layers from the layer cache whenever the tree had not changed:
|
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
|
# fresh value here is what forces them to re-run: without it an
|
||||||
# unchanged tree replays them from cache, the checks never execute,
|
# unchanged tree replays them from cache, the checks never execute,
|
||||||
# and the build still exits 0. The ARG sits immediately above the
|
# 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
|
# Assign the epoch on its own line rather than inline in the
|
||||||
# argument. Under `set -eu` a command substitution that fails
|
# 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
|
# script exists to prevent -- so the guard would disarm itself and
|
||||||
# still exit 0. As a bare assignment, `set -e` catches a failing
|
# still exit 0. As a bare assignment, `set -e` catches a failing
|
||||||
# `date` and no build starts.
|
# `date` and no build starts.
|
||||||
epoch="$(date +%s)"
|
epoch="$(date +%s%N)$$"
|
||||||
docker build --build-arg CHECK_EPOCH="$epoch" .
|
docker build --build-arg CHECK_EPOCH="$epoch" .
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -9,7 +9,16 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
|
|||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
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 "$@"
|
main "$@"
|
||||||
|
|||||||
Reference in New Issue
Block a user