Compare commits

1 Commits

Author SHA1 Message Date
bfe2b673a2 Make the tagged-release path work on Gitea (closes #65)
All checks were successful
check / check (pull_request) Successful in 2m37s
No tag could be cut from this repo at all. Three independent blockers.

goreleaser was configured for GitHub while the repo lives on Gitea:
.goreleaser.yaml had a release: block but no gitea_urls:, so goreleaser
defaulted to the GitHub API and a release would have failed or published
somewhere nobody is looking. It now points at https://git.eeqj.de/api/v1.

The version was a hardcoded Makefile constant, VERSION := 1.0.0-rc.1, so
every local build claimed to be a release candidate that had never been
tagged and did not exist, while git tag -l was empty and internal/globals
defaulted to dev. The version now comes from git, via the new
script/version: the exact tag with a leading v stripped when HEAD is on
one (so a make build and a goreleaser build of the same commit report the
same string, and it matches the archive names), otherwise dev-<12-char
sha>, with -dirty appended in either case when tracked files are
modified. Untracked files are not counted, matching git describe --dirty.
goreleaser's snapshot template gets the same treatment: it was
{{ incpatch .Version }}-next, which manufactures a release number from
the last tag and, with no tags at all, from goreleaser's fabricated
v0.0.0.

That change had one non-obvious consequence. internal/cli/version.go
gated its "this is a development build" notice on the version being
exactly "dev", so as soon as untagged builds carried a commit sha the
notice would have gone silent and an unreleased binary would have read as
a release. The gate is now globals.IsDevVersion, a predicate over a
string rather than a comparison against a global so that it can be
tested, and it is tested at the boundary that matters: dev-<sha> and its
-dirty variant are development builds, 1.0.0-dev and 1.0.0-rc.1 are not.
The command writes to cmd.OutOrStdout() so its output can be asserted on
at all.

Releases now come from CI rather than a workstation: a tag-triggered
.gitea/workflows/release.yml, with fetch-depth: 0 because a shallow
checkout has no tags and would silently mislabel the release, and with
the RELEASE_TOKEN repository secret passed as GITEA_TOKEN (documented in
README.md; the runner's automatic token is deliberately not used, since
it is not guaranteed to carry release write scope). script/release unsets
any GITHUB_TOKEN or GITLAB_TOKEN it finds, because goreleaser picks its
forge from whichever token variable is set and refuses to run when it
sees more than one -- an unrelated runner token must not get to decide
where these artifacts are published.

make release and make release-snapshot were the last two Makefile targets
that were not shims; they now call script/release and
script/release-snapshot, which resolve goreleaser the way script/lint
resolves the linter -- a PATH binary is accepted only at the pinned
version, never as a silent fallback. script/bootstrap installs it from a
sha256-verified GitHub release archive per REPO_POLICIES.md, through a
separate script/install-goreleaser: separate because script/bootstrap
hard-fails without a usable Docker daemon by design, and the release
runner needs goreleaser without needing Docker. dist/ and .tool/ are
gitignored and excluded from the Docker build context.

Verified by running it: make release-snapshot produces the four
linux,darwin x amd64,arm64 archives plus checksums.txt, and the binary
from dist/ reports dev-<sha> with the development-build notice. Tag
handling was exercised in a throwaway repository; no tag was created
here, since that is the owner's call. Signing, SBOM, reproducible builds,
shell completions and a man page remain out of scope.
2026-08-09 15:35:21 +00:00
30 changed files with 713 additions and 2826 deletions

View File

@@ -14,39 +14,6 @@ jobs:
# the commits since the previous one. A shallow checkout # the commits since the previous one. A shallow checkout
# silently produces a mislabelled release. # silently produces a mislabelled release.
fetch-depth: 0 fetch-depth: 0
# goreleaser is not a compiler: it shells out to `go` for the
# `before:` hook and for every one of the four cross-compiles.
# Nothing else in this repo puts a Go toolchain on the runner --
# check.yml runs script/cibuild, which does all of its work inside
# the digest-pinned Dockerfile images -- so without this step the
# release either fails at the before-hook or, worse, ships binaries
# built by whatever unpinned Go the runner happens to carry.
# REPO_POLICIES.md requires every external reference to be pinned,
# and script/release already refuses a goreleaser that is not the
# pinned build; the compiler that actually produces the artifacts
# is the last thing that should be exempt from that.
#
# go-version-file rather than a literal: go.mod's `go 1.26.1` is
# the single source of truth for the toolchain, the same way the
# Dockerfile FROM line is the single source of truth for the
# linter version that script/lint enforces. It is a three-component
# version, so setup-go resolves it exactly -- no silent drift onto
# a newer patch release.
#
# actions/setup-go v5.6.0, 2025-12-15. Pinned by commit sha, like
# the checkout above. v5.x is a node20 action, matching the node20
# actions/checkout v4 already in use here; the v6/v7 line requires
# a node24 runner, which this Gitea runner has never been asked
# for and cannot be assumed to provide.
- name: Install Go
uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff
with:
go-version-file: go.mod
# setup-go's module cache needs a runner-side cache backend.
# A release is cut rarely and a cold module download costs
# seconds; a release failing because a cache service is absent
# costs a re-tag. Off, deliberately.
cache: false
- name: Install goreleaser - name: Install goreleaser
run: script/install-goreleaser run: script/install-goreleaser
- name: Release - name: Release

View File

@@ -83,8 +83,8 @@ Version: 2025-06-08
possible to mock or stub these side-effects in tests. possible to mock or stub these side-effects in tests.
9. Always use structured logging. Log any relevant state/context with the 9. Always use structured logging. Log any relevant state/context with the
messages (but do not log secrets). If the log stream is not a terminal, messages (but do not log secrets). If stdout is not a terminal, output
output the structured logs in jsonl format. the structured logs in jsonl format.
10. Avoid using bare strings or numbers in code, especially if they appear 10. Avoid using bare strings or numbers in code, especially if they appear
anywhere more than once. Always define a constant (usually at the top anywhere more than once. Always define a constant (usually at the top

View File

@@ -1,29 +1,23 @@
# This file has no lint stage, deliberately. # Lint stage
# #
# Linting lives in Dockerfile.lint, built by script/lint, and # This FROM line is the single source of truth for the linter version:
# script/cibuild builds both. A lint stage here would have to either # script/lint parses the image reference out of it and runs that exact
# shell out to `make lint` -- which is now `docker build`, so # image, so a local `make lint` and CI use the same linter. Bump the
# docker-in-docker inside a BuildKit step with no daemon -- or call # linter here (tag AND digest) and nowhere else.
# golangci-lint directly, which would mean a second, independently
# bumpable digest pin for the linter alongside the one in
# Dockerfile.lint. Two pins for one tool is the drift that
# https://git.eeqj.de/sneak/vaultik/issues/78 was filed over. See
# https://git.eeqj.de/sneak/vaultik/issues/113 for the ruling.
# #
# Consequence, stated rather than left to be discovered: script/docker # golangci/golangci-lint:v2.12.2-alpine, 2026-08-07
# builds this file only and therefore does not lint. `make fmt-check` FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60 AS lint
# and `make test` still run here, so what a green build of this file
# means is "formatted, tested, and it compiles" -- the lint verdict
# comes from script/lint or script/cibuild.
# Build stage RUN apk add --no-cache make build-base
# golang:1.26.1-alpine, 2026-03-17
FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder
ARG VERSION=dev # The context signal for script/lint's native path. This stage runs
# `make lint` with no docker daemon available, so it is the one place
# Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests) # that must run the golangci-lint on PATH directly. script/lint takes
RUN apk add --no-cache make build-base sqlite # that path only when this is set AND the version matches the pin above;
# version equality alone would also admit a developer's locally
# installed copy on a host, bypassing the digest pin (issue #80).
# Nothing outside this stage sets it.
ENV VAULTIK_LINT_IN_CONTAINER=1
WORKDIR /src WORKDIR /src
@@ -34,7 +28,7 @@ RUN go mod download
# Copy source code # Copy source code
COPY . . COPY . .
# Run the format check and the tests. # Run formatting check and linter.
# #
# 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
@@ -53,15 +47,45 @@ COPY . .
# runs the checks and every one after it on an unchanged tree replays # runs the checks and every one after it on an unchanged tree replays
# these layers from cache, executes nothing, and still exits 0. Failed # these layers from cache, executes nothing, and still exits 0. Failed
# steps are never cached, so the guard fails on EVERY invocation rather # steps are never cached, so the guard fails on EVERY invocation rather
# than once -- a bare `docker build .` is a loud error, not a quiet # 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 # green. Do not give CHECK_EPOCH a default value; a default would
# satisfy the guard with a constant and restore the hole. # 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 # 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 [ -n "$CHECK_EPOCH" ] || exit 1 RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check 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
FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder
# Depend on lint stage passing
COPY --from=lint /src/go.sum /dev/null
ARG VERSION=dev
# Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests)
RUN apk add --no-cache make build-base sqlite
WORKDIR /src
# Copy go mod files first for better layer caching
COPY go.mod go.sum ./
RUN go mod download
# Copy source code
COPY . .
# 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 [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "check epoch: ${CHECK_EPOCH}" && make test 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)

View File

@@ -1,104 +0,0 @@
# Lint image.
#
# Every lint run in this repo happens inside this image, invoked through
# script/lint, and linting is a BUILD STEP rather than a container
# command: a successful build of this file IS a clean lint. That shape
# also works where the docker daemon is remote and bind mounts are
# impossible, which `docker run` against a mounted worktree does not.
#
# This FROM line is the single source of truth for the linter version in
# this repo. Nothing else pins golangci-lint: the product Dockerfile has
# no lint stage, deliberately, so there is no second digest to bump and
# no pair of pins that can drift apart. Bump the tag AND the digest here
# and nowhere else.
#
# Note for readers coming from REPO_POLICIES.md: that document still
# describes the older pattern, a lint stage inside the product
# Dockerfile wired up with `COPY --from=lint /src/go.sum /dev/null`.
# That pattern is superseded here by the owner's ruling recorded in
# https://git.eeqj.de/sneak/vaultik/issues/113 -- lint runs in its own
# image, per run, with its own cache and its own lock, which is what
# makes concurrent runs on one host safe. The policy text is org-wide
# and is being amended separately; this file is what this repo does.
#
# golangci/golangci-lint:v2.12.2, 2026-08-10
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240
WORKDIR /src
# Copy the dependency manifests first so the module download layer stays
# cached until they change. Everything above the ARG below is cacheable
# on purpose; a cold module download on every lint would make the inner
# loop unusable and buys nothing, because it is not what the gate is
# asserting.
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# Force the check layers 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
# already built against this same tree; script/lint and script/cibuild
# each pass a fresh value on every invocation, which is what makes their
# green mean the linter really ran. Without it, `docker build -f
# Dockerfile.lint .` on an unchanged tree exits 0 in well under a second
# having linted nothing.
#
# The value is expanded into each check command itself 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.
#
# 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
# lints and every one after it on an unchanged tree replays this layer,
# executes nothing, and still exits 0. Failed steps are never cached, so
# the guard fails on EVERY invocation rather than once. Do not give
# CHECK_EPOCH a default value; a default would satisfy the guard with a
# 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 ./...

View File

@@ -6,16 +6,6 @@
# had never been tagged. # had never been tagged.
VERSION := $(shell script/version) VERSION := $(shell script/version)
# $(shell) discards exit status, so a script/version that is missing,
# non-executable or broken would otherwise leave VERSION empty and every
# binary built here would print "vaultik " with no version at all. A
# build that cannot determine what it is must not produce an artifact.
ifeq ($(strip $(VERSION)),)
$(error script/version produced no version string; a build that cannot \
determine its version will not be made. Check that script/version exists \
and is executable)
endif
# Build variables # Build variables
GIT_REVISION := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown") GIT_REVISION := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown")
GIT_COMMIT_DATE := $(shell git show -s --format=%cs HEAD 2>/dev/null || echo "unknown") GIT_COMMIT_DATE := $(shell git show -s --format=%cs HEAD 2>/dev/null || echo "unknown")
@@ -66,18 +56,7 @@ lint:
lint-fix: lint-fix:
@script/lint-fix @script/lint-fix
# Build binary. `build` is the name the org convention reaches for and # Build binary.
# the one a caller checks the exit code of; `vaultik` is the file rule
# that does the work, so an unchanged tree still short-circuits.
#
# This alias is not decorative. `build` was listed in .PHONY with no
# rule, and a phony target with no prerequisites and no recipe is
# already satisfied: `make build` printed "Nothing to be done" and
# exited 0 without producing a binary (issue #110). Every name in
# .PHONY needs a rule for that reason; TestPhonyTargetsAllHaveRules in
# cmd/vaultik keeps it that way.
build: vaultik
vaultik: internal/*/*.go cmd/vaultik/*.go vaultik: internal/*/*.go cmd/vaultik/*.go
go build -ldflags "$(LDFLAGS)" -o $@ ./cmd/vaultik go build -ldflags "$(LDFLAGS)" -o $@ ./cmd/vaultik
@@ -87,10 +66,10 @@ clean:
go clean go clean
# Install dependencies. The linter is deliberately not installed here: # Install dependencies. The linter is deliberately not installed here:
# script/lint lints by building Dockerfile.lint, whose FROM line is the # script/lint runs the digest-pinned golangci-lint image declared by the
# single source of truth for the linter version. A second, separately # Dockerfile's lint stage, which is the single source of truth for the
# pinned copy on PATH could drift from it and make a local `make lint` # linter version. A second, separately pinned copy on PATH could drift
# disagree with CI. # from it and make a local `make lint` disagree with CI.
deps: deps:
go mod download go mod download

149
README.md
View File

@@ -113,40 +113,11 @@ vaultik version
### global flags ### global flags
* `--config <path>`: Path to config file (default: `$VAULTIK_CONFIG`, then platform config dir, then `/etc/vaultik/config.yml`) * `--config <path>`: Path to config file (default: `$VAULTIK_CONFIG`, then platform config dir, then `/etc/vaultik/config.yml`)
* `--verbose`, `-v`: Enable verbose output (on stderr — see below) * `--verbose`, `-v`: Enable verbose output
* `--debug`: Enable debug output (on stderr — see below) * `--debug`: Enable debug output
* `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner) * `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
* `--skip-errors`: Continue past per-file errors instead of aborting (applies to `snapshot create` and `restore`) * `--skip-errors`: Continue past per-file errors instead of aborting (applies to `snapshot create` and `restore`)
### stdout and stderr
Log output — everything from `--verbose` and `--debug`, and every
warning and error the logger emits — goes to **stderr**. stdout carries
the output you asked for: tables, and the documents produced by `--json`.
This means `vaultik snapshot list --verbose > out.txt` captures the
listing and leaves the diagnostics on your terminal. To capture both,
redirect stderr as well (`> out.txt 2> log.txt`, or `> out.txt 2>&1` to
interleave them).
The split is what makes `--json` usable from a script. Warnings and
errors are never suppressed — not by `--quiet`, not by `--cron` — so a
logger on stdout would eventually land a log line inside a JSON
document and break the parse. A config file with group- or
world-readable permissions is enough to trigger it.
Format follows the stream: when stderr is a terminal the records are
colorized one-liners, and when it is redirected or piped they are
JSON, one object per line.
Under `--json`, stdout holds the document and nothing else. The startup
banner is suppressed, as `--quiet` and `--cron` suppress it, and the
progress narration a command would otherwise print — such as the stale
local records `prune` reconciles away — is suppressed too, so it cannot
land ahead of the document. Every `--json` command therefore pipes on
its own, with no additional flag: `vaultik snapshot list --json | jq .`
and `vaultik prune --json | jq .` both work as written.
### environment variables ### environment variables
* `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`) * `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`)
@@ -237,9 +208,8 @@ local index alone, and still exits zero.
(whether the snapshot is in the local index), `remote_key` (the full (whether the snapshot is in the local index), `remote_key` (the full
64-character storage key), and `remote_present` (whether it was seen 64-character storage key), and `remote_present` (whether it was seen
on the destination store, or `null` if the destination could not be on the destination store, or `null` if the destination could not be
listed). Warnings about an unlistable destination, unreadable listed). The warning about an unlistable destination goes to stderr
manifests, and a truncated listing all go to stderr through the so stdout stays a single parseable document.
logger, so stdout stays a single parseable document.
**`snapshot verify`**: Verify snapshot integrity. **`snapshot verify`**: Verify snapshot integrity.
* Default (shallow): checks that all blobs referenced in the manifest exist in storage * Default (shallow): checks that all blobs referenced in the manifest exist in storage
@@ -534,10 +504,6 @@ All user-facing output goes through helpers in `internal/ui` and conforms
to a uniform style. Color is enabled when stdout is a TTY and the to a uniform style. Color is enabled when stdout is a TTY and the
`NO_COLOR` environment variable is unset (https://no-color.org/). `NO_COLOR` environment variable is unset (https://no-color.org/).
`internal/ui` writes to stdout; it is the output the user asked for.
Structured log records are a different thing and go through
`internal/log`, which writes to stderr (see "stdout and stderr" above).
Message classes: Message classes:
| Class | Marker | Alignment | Use for | | Class | Marker | Alignment | Use for |
@@ -598,11 +564,10 @@ regardless of color setting (emoji are not color).
* Go 1.26 or later * Go 1.26 or later
* Docker, with a reachable daemon, to lint, check, or commit: * Docker, with a reachable daemon, to lint, check, or commit:
`script/lint` lints by building `Dockerfile.lint`, which runs the `script/lint` runs the digest-pinned `golangci-lint` image declared by
digest-pinned `golangci-lint` image as a build step, and `make check` the `Dockerfile` lint stage, and `make check` and the pre-commit hook
and the pre-commit hook both run it. A `golangci-lint` installed on both run it. A `golangci-lint` installed on `PATH` is not a substitute
`PATH` is not a substitute and is never used on a host, whatever its and is never used on a host, whatever its version.
version.
* `sqlite3` CLI, which the test suite shells out to * `sqlite3` CLI, which the test suite shells out to
* S3-compatible object storage (or local filesystem, or rclone remote) * S3-compatible object storage (or local filesystem, or rclone remote)
@@ -672,69 +637,46 @@ them. We provide:
diverges from the 30s `REPO_POLICIES.md` mandates; the reasoning is in diverges from the 30s `REPO_POLICIES.md` mandates; the reasoning is in
the comment in the script, and issue #101 proposes amending the policy the comment in the script, and issue #101 proposes amending the policy
text. text.
* `script/lint`lint by building `Dockerfile.lint`, which runs * `script/lint`run `golangci-lint run ./...` at the exact version CI
`golangci-lint run --config .golangci.yml ./...` as a build step uses, by running the digest-pinned `golangci-lint` image declared by
inside the digest-pinned `golangci-lint` image, so a successful build the `Dockerfile` lint stage (requires Docker; it fails loudly rather
*is* a clean lint. Nothing lints on the host, at any version, ever; than falling back to a differently versioned `golangci-lint` on
the script requires Docker and fails loudly rather than falling back `PATH`). That `FROM` line is the single source of truth for the linter
to a `golangci-lint` on `PATH`. That `FROM` line is the single source version — bump it there and nowhere else.
of truth for the linter version — bump it there and nowhere else.
It takes no arguments, because a build step has no command line to
pass flags to, and it passes a fresh `--build-arg CHECK_EPOCH` on
every invocation so the lint layer cannot be replayed from cache (see
`script/cibuild` below for what that mechanism defends against). To
watch the linter execute, run it as
`BUILDKIT_PROGRESS=plain script/lint` and check that the lint layer
says `RUN … golangci-lint` rather than `CACHED`.
One container per run means one lint cache and one `golangci-lint`
lock per run, both private to it and discarded with it, so concurrent
runs on one host cannot contaminate or block each other.
* `script/lint-fix` — apply the linter's autofixes (rewrites files), * `script/lint-fix` — apply the linter's autofixes (rewrites files),
using the same pinned image, parsed out of `Dockerfile.lint`. It using the same pinned linter
cannot be a build step, because fixes have to land in the worktree, so
it bind-mounts the tree into a `docker run` and therefore needs a
*local* daemon. It is a developer convenience and never a gate: no
gate reads its exit status. Run `make lint` afterwards to find out
whether the tree is clean.
* `script/fmt` — format all code (writes) * `script/fmt` — format all code (writes)
* `script/fmt-check` — check formatting (read-only) * `script/fmt-check` — check formatting (read-only)
* `script/check` — run `script/test`, `script/lint`, and * `script/check` — run `script/test`, `script/lint`, and
`script/fmt-check`. This is authoritative *because* `script/lint` `script/fmt-check`. This is authoritative *because* `script/lint` uses
builds `Dockerfile.lint`: a local `make check` and CI cannot disagree the pinned linter: a local `make check` and CI cannot disagree about
about lint findings. lint findings.
* `script/docker` — build the Docker image tagged via * `script/docker` — build the Docker image tagged via
`script/projectname`. Passes a fresh `--build-arg CHECK_EPOCH` for the `script/projectname`. Passes a fresh `--build-arg CHECK_EPOCH` for the
same reason `script/cibuild` does, so a local image build cannot be same reason `script/cibuild` does, so a local image build cannot be
green on checks it replayed from cache. It builds the *product* image green on checks it replayed from cache.
only, and the product `Dockerfile` has no lint stage, so it does not * `script/cibuild` — CI entrypoint: `docker build` (the `Dockerfile`
lint: a green here means formatted, tested, and it compiles. runs `make fmt-check` and `make lint` in its lint stage and `make
* `script/cibuild` — CI entrypoint, and the full gate. Two builds, in test` in its builder stage). This is the full CI-equivalent gate — it
order: `Dockerfile.lint` (the linter, as a build step) and then runs the checks in the same containers CI does, from a clean copy of
`Dockerfile` (`make fmt-check` and `make test` in its builder stage, the tree, so it also catches anything that depends on host state. It
then the product image). Either failing fails the script. It runs the passes a fresh `--build-arg CHECK_EPOCH`, unique per invocation, which
checks in the same containers CI does, from a clean copy of the tree, the `Dockerfile` declares immediately above the check `RUN`s in both
so it also catches anything that depends on host state. stages and expands into each check command. Those layers are keyed on
It passes a fresh `--build-arg CHECK_EPOCH` to each build, unique per
invocation, which both files declare immediately above their check
`RUN`s and expand into each check command. Those layers are keyed on
that value, so a new value re-runs them even on a byte-identical tree, 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 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 module layers sit above the `ARG` and still cache, so a build is not
cold. cold.
A build that supplies no `CHECK_EPOCH` — a bare `docker build .` or A build that supplies no `CHECK_EPOCH` — a bare `docker build .`
`docker build -f Dockerfile.lint .` — fails rather than lying. An fails rather than lying. An unset `ARG` is an empty string and an
unset `ARG` is an empty string and an empty string is a stable cache empty string is a stable cache key, so without a guard such a build
key, so without a guard such a build would serve every check layer would serve all three check layers from cache, execute nothing, and
from cache, execute nothing, and still exit 0. Each file therefore still exit 0. Each check stage therefore asserts the value is
asserts the value is non-empty before running anything, and because non-empty before running anything, and because failed steps are never
failed steps are never cached that assertion fires on every cached that assertion fires on every invocation rather than once. Use
invocation rather than once. Use `script/lint`, `script/docker` or `script/cibuild` (or `script/docker`, which passes the same arg); a
`script/cibuild`, which pass the arg; a bare `docker build` is a loud bare `docker build .` is now a loud error.
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
@@ -759,10 +701,7 @@ agrees with it:
A build that is not a release never names itself like one. `vaultik A build that is not a release never names itself like one. `vaultik
version` says so in as many words on a development build, and version` says so in as many words on a development build, and
`goreleaser --snapshot` stamps the same `dev-<sha>` string rather than `goreleaser --snapshot` stamps the same `dev-<sha>` string rather than
inventing the next patch number. If `script/version` cannot be run at inventing the next patch number.
all, `make` stops with an error instead of building an unversioned
binary, and a binary that somehow carries an empty version string still
reports itself as a development build.
### cutting a release ### cutting a release
@@ -773,9 +712,8 @@ git tag -a v1.2.3 -m 'v1.2.3'
git push origin v1.2.3 git push origin v1.2.3
``` ```
`.gitea/workflows/release.yml` triggers on `v*` tags, installs a Go `.gitea/workflows/release.yml` triggers on `v*` tags, installs the
toolchain and the pinned `goreleaser`, and runs `script/release`, which pinned `goreleaser`, and runs `script/release`, which builds
builds
`linux,darwin × amd64,arm64` archives plus `checksums.txt` and publishes `linux,darwin × amd64,arm64` archives plus `checksums.txt` and publishes
them to this repository's Gitea releases as a draft. `.goreleaser.yaml` them to this repository's Gitea releases as a draft. `.goreleaser.yaml`
has a `gitea_urls:` block pointing at `https://git.eeqj.de/api/v1`; has a `gitea_urls:` block pointing at `https://git.eeqj.de/api/v1`;
@@ -791,15 +729,6 @@ It is passed to `goreleaser` as `GITEA_TOKEN`. The runner's automatic
token is deliberately not used: it is not guaranteed to carry release token is deliberately not used: it is not guaranteed to carry release
write access. write access.
The Go toolchain that compiles the released binaries comes from an
`actions/setup-go` step pinned by commit sha, reading its version from
`go.mod` (currently `1.26.1`, the same version the `Dockerfile` builder
stage pins by digest). `goreleaser` shells out to `go` for every
cross-compile, so without that step the release would either fail
outright or ship binaries built by whatever unpinned toolchain the
runner happened to carry — the one unpinned thing in an otherwise
hash-pinned release path.
To rehearse the whole build without publishing or tagging anything: To rehearse the whole build without publishing or tagging anything:
``` ```

193
TODO.md
View File

@@ -25,199 +25,6 @@ release" is exactly the contradiction
# Completed Steps # Completed Steps
- 2026-08-10: Moved every lint run into its own container, as a build
step ([issue #113](https://git.eeqj.de/sneak/vaultik/issues/113)).
New root `Dockerfile.lint`, built by `script/lint`, runs
`golangci-lint run --config .golangci.yml ./...` as a `RUN`
instruction in the digest-pinned `golangci/golangci-lint` image: a
successful build of that file *is* a clean lint, and it works even
where the daemon is remote and bind mounts are impossible. That
`FROM` line is now the only pin of the linter version in the repo.
This supersedes the per-worktree cache isolation landed for
[issue #99](https://git.eeqj.de/sneak/vaultik/issues/99). Isolation
fixed cross-worktree contamination but not lock contention — two
concurrent runs with entirely separate cache directories still
collided. A container per run has its own cache and its own lock, so
the whole class is gone, and with it the per-worktree cache
machinery, the lock-retry loop, and `script/lint-audit`, which
existed to catch replayed findings from a cache that no longer
exists. The host lint path went too: no escape hatch, no
`VAULTIK_LINT_IN_CONTAINER`, no version detection. Nothing lints on
the host at any version.
A cached build lints nothing, so the same `CHECK_EPOCH` mechanism the
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 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. 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
would have been docker-in-docker with no daemon, and calling
`golangci-lint` directly there would have restored the two-pins drift
of [issue #78](https://git.eeqj.de/sneak/vaultik/issues/78).
`make fmt-check` moved beside `make test` in the builder stage, and
`script/cibuild` now builds `Dockerfile.lint` and then `Dockerfile`,
each with its own fresh epoch. Consequence, stated rather than left
to be found: `script/docker` builds the product image only and no
longer lints; the gates are `script/check` and `script/cibuild`.
`golangci-lint config verify` runs as its own epoch-keyed layer,
above the lint. `golangci-lint run` rejects a config it cannot parse
but silently ignores an unknown top-level *key*: renaming `linters:`
to `linterz:` discarded `default: all` and every threshold and still
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
convenience that no gate reads.
- 2026-08-09: Finished the `--json` stdout contract and gave `make build`
a rule ([issue #108](https://git.eeqj.de/sneak/vaultik/issues/108),
[issue #110](https://git.eeqj.de/sneak/vaultik/issues/110)). Two
unrelated defects of the same shape — a command reporting something it
did not do — landed together because both are small.
`CleanupLocalSnapshots` wrote three prose lines to stdout with no
`--json` awareness, covering every branch of the function, so no input
avoided them and `vaultik prune --json | jq` failed even after
[issue #106](https://git.eeqj.de/sneak/vaultik/issues/106) removed the
banner. `-q` never helped either: `printlnStdout` and `stdoutf` write
straight to `Vaultik.Stdout` and never consult `Vaultik.UI`, which is
what `SetQuiet` affects. The issue offered three fixes and asked for a
decision. Taken: thread `*PruneOptions` into the function and gate each
write on `!opts.JSON`, matching `PruneBlobs` (its sibling phase, which
already takes the same struct), `RemoveSnapshot` and `remote info`, so
the package has one pattern rather than two. Rejected: moving the lines
to `log.Info`, because the logger's default level is `slog.LevelWarn`,
so that would not relocate them to stderr — it would delete them from a
plain `vaultik prune`, and the removal of rows from the local index is
not something to narrate only under `--verbose`. Also rejected: putting
the stale-record count into `PruneBlobsResult`, whose every field is
blob-scoped and which is produced by the later phase; a prune document
covering both phases is a reasonable thing to want, but it is a schema
design question and not a stream-hygiene fix. The narration is
duplicated as `log.Info` records, which `PruneBlobs` already does
alongside its own prints, so the events survive on stderr for anyone
running `--verbose`.
`make build` printed "Nothing to be done for 'build'" and exited 0
without producing a binary: `build` was listed in `.PHONY` with no
`build:` rule anywhere, and declaring a name phony is exactly what
converts make's "No rule to make target" error into a silent success.
Fixed with `build: vaultik`, keeping `vaultik:` as the file rule. The
audit the issue asked for covers all 19 `.PHONY` names; `build` was the
only one without a rule, and `vaultik` is correctly absent from
`.PHONY`, being a real file target.
Tests, each verified to fail with the fix reverted rather than assumed
to: `CleanupLocalSnapshots` leaves stdout untouched under `--json` in
all three branches (stale records, none, empty index) and still emits
every line without it, so the guard cannot be satisfied by deleting the
output; `prune --json` run end to end through `Entry`, cobra and fx
over the process's real stdout descriptor against a `file://` store,
asserting exactly one JSON document, in both the stale and non-stale
branches; and a parse of the `Makefile` asserting every `.PHONY` name
has a rule and that `build` reaches the rule that produces the binary,
which keeps the audit true for names added later. That last one is a
parse rather than an invocation of `make`, since `make test` is what
runs it and shelling back into `make build` would nest a build inside
the test run. The property a parse cannot establish — that the recipe
still fails when the build fails — was verified by hand against a
deliberately broken tree: `make build` exits 2 and produces nothing.
`cmd/vaultik` gains its first test file, so `make test` now reports 16
packages `ok` where it reported 15.
- 2026-08-09: Stopped the startup banner from contaminating `--json`
documents ([issue #106](https://git.eeqj.de/sneak/vaultik/issues/106)).
`Entry` writes the banner to stdout before cobra parses anything, and
the flag scan that suppresses it knew `--quiet`, `-q` and `--cron` but
not `--json`, so every `--json` document arrived behind two lines of
prose and a blank line, and `vaultik snapshot list --json | jq` failed.
With the logger already on stderr from
[issue #82](https://git.eeqj.de/sneak/vaultik/issues/82), this was the
last writer that could put something on stdout that the caller did not
ask for. The design question the issue raised — extend the raw-argv
scan, or move the banner after parsing — is answered in favour of the
scan: the banner is printed first deliberately, so that it still
appears when cobra rejects the arguments and on `--help`, and after
parsing there is no single place that covers those paths. The stated
cost of the scan, that `--json` is a subcommand flag matched anywhere
in the vector, is a cost `--cron` already carries — it exists only on
`snapshot create` — so this adds an instance of an accepted
imprecision rather than a new kind, and the two error directions are
not symmetric: a false positive loses a decorative banner, a false
negative corrupts a document. Regression tests at the CLI layer, where
`internal/vaultik`'s existing guard cannot reach: one runs `Entry`
itself over the process's real stdout descriptor, through cobra and fx
to the document, made hermetic by `file://` storage; a second covers
the argument vectors of all five `--json` commands; a third asserts the
banner is still printed without a suppressing flag, so the first
cannot be satisfied by deleting the banner. Also corrected `AGENTS.md`
policy 9, which still keyed the structured-log format on stdout's
TTY-ness after #82 moved that decision to stderr — a rules file that
misdescribes the code misleads exactly the readers who trust it most.
Two smaller findings from the same review: `bytesAttrKey`'s
human-readable byte formatting silently stopped applying under an open
group, because the key reaching the comparison is group-qualified
(`transfer.bytes`), now matched on its final segment and tested both
ways; and `listEnv.stderr` in `snapshot_list_test.go`, assigned but
never read since those tests began capturing the process's stderr, is
removed. `Vaultik.Stderr` is kept — nothing writes to it today, which
its comment now says outright.
- 2026-08-09: Moved the logger to stderr and fixed `TTYHandler`'s
discarded attributes
([issue #82](https://git.eeqj.de/sneak/vaultik/issues/82),
[issue #97](https://git.eeqj.de/sneak/vaultik/issues/97)). Two defects
in `internal/log`, fixed together because both live in the handler
construction path. The first: both handlers were built over
`os.Stdout`, and `WARN`/`ERROR` are never suppressed, so a config file
with group- or world-readable permissions was enough to put a log
record inside a `--json` document and break `jq`. Diagnostics now go
to stderr, and the TTY/JSON format choice follows stderr rather than
stdout — testing the wrong stream would colorize records on a
redirected stderr whenever stdout happened to be a terminal. This is
user-visible: `--verbose` and `--debug` output moves to stderr too,
which is documented in `README.md` under "stdout and stderr". It also
let the local workaround in `internal/vaultik/snapshot_list.go` go:
`warnWhileListing` had been hand-rolling structured-log formatting to
reach a non-stdout writer, and the `jsonOutput` parameter threaded
through the remote-listing helpers existed only to choose between the
two writers. The collect-then-emit machinery around `listingWarning`
stays, but on its remaining merit — warnings emitted in key order
after `group.Wait()` are deterministic run to run, where emitting from
the fetch workers would order them by network timing. The second
defect: `TTYHandler.WithAttrs` and `WithGroup` discarded their
arguments and returned the receiver while their doc comments claimed
otherwise, so `log.With` attributes vanished on a terminal and
appeared correctly in CI — failing precisely when someone is debugging
interactively. Both now return a new handler (the receiver is never
written to, since `slog` permits concurrent derivation), attributes
persist across records, and grouping is implemented as dotted key
prefixes, which is the only honest rendering for a format with nowhere
to nest. New tests cover both, including one that feeds the same
derivation chain to the TTY and JSON handlers and compares the
attribute sets, so the two paths cannot drift apart again. Found and
filed while verifying: the startup banner is written to stdout and
`--json` does not suppress it
([issue #106](https://git.eeqj.de/sneak/vaultik/issues/106)), which is
a separate writer on a separate path and the remaining source of
stdout contamination.
- 2026-08-09: Made the tagged-release path actually work on Gitea - 2026-08-09: Made the tagged-release path actually work on Gitea
([issue #65](https://git.eeqj.de/sneak/vaultik/issues/65)). Three ([issue #65](https://git.eeqj.de/sneak/vaultik/issues/65)). Three
independent blockers, one of which was the whole independent blockers, one of which was the whole

View File

@@ -1,507 +0,0 @@
package main_test
import (
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// This file guards the shape of the lint gate. Every property asserted
// here is one whose loss is SILENT: the build still exits 0, the gate
// still looks green, and nothing was linted or tested.
//
// The gate is a build step. script/lint builds Dockerfile.lint, which
// runs golangci-lint as a RUN instruction, so a successful build is a
// clean lint. BuildKit will happily replay that RUN from cache on an
// unchanged tree in well under a second, which is why the check layers
// are keyed on a CHECK_EPOCH build arg that the calling script
// regenerates per invocation, and why an empty value is a hard error
// rather than a stable cache key.
//
// These are parses rather than invocations. Shelling out to docker from
// the test suite would nest a build inside `make test`, which itself
// runs inside a build in CI. The one property a parse cannot establish
// -- that a real finding actually fails the build -- is verified by
// hand against a deliberately broken tree, recorded on the pull
// request.
// The files under guard, relative to the repository root.
const (
lintDockerfile = "Dockerfile.lint"
productDockerfile = "Dockerfile"
lintScript = "script/lint"
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
// after the first build.
const checkEpochARG = "ARG CHECK_EPOCH"
// checkEpochGuard is what turns a build that omits --build-arg into a
// loud failure instead of a quiet green. Failed steps are never cached,
// so it fires on every such invocation rather than once.
const checkEpochGuard = `RUN [ -n "$CHECK_EPOCH" ] || exit 1`
// freshEpoch is the epoch computation the calling scripts must use, as
// a bare assignment on its own line. Inline in an argument, a failing
// `date` would not abort under `set -eu`; CHECK_EPOCH would become the
// empty string, and the guard above would be the only thing standing
// between that and a permanently cached green. `$$` is required because
// `date +%s` is second-granular and busybox silently drops `%N`, so
// without the pid two concurrent runs in one second can collide.
const freshEpoch = `epoch="$(date +%s%N)$$"`
// TestLintDockerfilePinsTheLinterByDigest fails if the lint image stops
// being pinned. An unpinned tag makes the gate's verdict depend on
// whatever the registry currently serves under that name.
func TestLintDockerfilePinsTheLinterByDigest(t *testing.T) {
t.Parallel()
from := ""
for _, instruction := range instructions(t, lintDockerfile) {
if strings.HasPrefix(instruction, "FROM ") {
from = instruction
break
}
}
require.NotEmpty(t, from, "%s declares no FROM", lintDockerfile)
assert.Contains(t, from, "golangci/golangci-lint",
"the lint image must be the golangci-lint image")
assert.Contains(t, from, "@sha256:",
"the lint image must be pinned by digest, not by tag alone")
}
// TestLintDockerfileCannotBeCachedGreen pins the whole cache-busting
// mechanism in the file that lints: the declaration with no default,
// the non-empty guard, and the value expanded into the lint command
// itself rather than merely declared.
func TestLintDockerfileCannotBeCachedGreen(t *testing.T) {
t.Parallel()
found := instructions(t, lintDockerfile)
argAt := indexOf(found, checkEpochARG)
require.GreaterOrEqual(t, argAt, 0,
"%s must declare `%s` with no default value",
lintDockerfile, checkEpochARG)
assert.GreaterOrEqual(t, indexOf(found, checkEpochGuard), argAt,
"%s must guard against an empty CHECK_EPOCH with `%s`",
lintDockerfile, checkEpochGuard)
assertEpochExpandedInto(t, found[argAt:], "golangci-lint run")
// Dependency layers must stay above the ARG, or every lint run
// re-downloads the module cache and the inner loop becomes
// unusable.
download := indexOf(found, "RUN go mod download")
require.GreaterOrEqual(t, download, 0,
"%s must download modules in their own layer", lintDockerfile)
assert.Less(t, download, argAt,
"`%s` must come after `go mod download` so dependency layers"+
" 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) {
t.Parallel()
found := instructions(t, productDockerfile)
argAt := indexOf(found, checkEpochARG)
require.GreaterOrEqual(t, argAt, 0,
"%s must declare `%s` with no default value",
productDockerfile, checkEpochARG)
assert.GreaterOrEqual(t, indexOf(found, checkEpochGuard), argAt,
"%s must guard against an empty CHECK_EPOCH", productDockerfile)
assertEpochExpandedInto(t, found[argAt:], "make fmt-check")
assertEpochExpandedInto(t, found[argAt:], "make test")
}
// TestProductDockerfileDoesNotLint records the split deliberately: the
// linter lives in Dockerfile.lint and nowhere else, so there is exactly
// one digest pinning it. A lint stage reintroduced here would either be
// docker-in-docker (`make lint` is now `docker build`) or a second,
// independently bumpable pin.
func TestProductDockerfileDoesNotLint(t *testing.T) {
t.Parallel()
contents := readRepoFile(t, productDockerfile)
for _, forbidden := range []string{"golangci", "make lint"} {
assert.NotContains(t, instructionText(contents), forbidden,
"%s must not lint: the linter is pinned once, in %s",
productDockerfile, lintDockerfile)
}
}
// TestLintScriptBuildsTheLintDockerfileWithAFreshEpoch is the other
// half of the mechanism. The Dockerfile's guard only rejects an EMPTY
// epoch; a constant non-empty one would satisfy it and still be served
// from cache forever.
func TestLintScriptBuildsTheLintDockerfileWithAFreshEpoch(t *testing.T) {
t.Parallel()
script := readRepoFile(t, lintScript)
assertBareEpochAssignment(t, script, lintScript)
assert.Contains(t, script, `--build-arg CHECK_EPOCH="$epoch"`,
"%s must pass the fresh epoch to the build", lintScript)
assert.Contains(t, script, lintDockerfile,
"%s must build %s", lintScript, lintDockerfile)
}
// TestCibuildBuildsBothDockerfilesWithFreshEpochs guards the CI gate:
// dropping either build silently removes a whole class of check from
// CI while leaving it green.
func TestCibuildBuildsBothDockerfilesWithFreshEpochs(t *testing.T) {
t.Parallel()
script := readRepoFile(t, cibuildScript)
assertBareEpochAssignment(t, script, cibuildScript)
assert.Equal(t, 2, strings.Count(script, freshEpoch),
"%s must compute a fresh epoch for each of its two builds",
cibuildScript)
assert.Equal(t, 2,
strings.Count(script, `--build-arg CHECK_EPOCH="$epoch"`),
"%s must pass a fresh epoch to both builds", cibuildScript)
assert.Contains(t, script, "-f Dockerfile.lint",
"%s must build %s", cibuildScript, lintDockerfile)
}
// TestNoHostLintPathRemains fails if any escape hatch to a host linter
// comes back. The owner's ruling is that every lint run happens inside
// 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()
root := repoRoot(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
}
name := filepath.Join("script", entry.Name())
for _, line := range shellCode(readRepoFile(t, name)) {
assertLinterIsContainerised(t, name, line)
}
}
}
// 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
}
docker := strings.Index(line, "docker")
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
// command with the epoch expanded into it. Expansion, not mere
// declaration: an ARG that no instruction references is not guaranteed
// to key the layer, and the expansion also puts the value in the build
// log where a reader can see the layer was keyed fresh.
func assertEpochExpandedInto(t *testing.T, found []string, command string) {
t.Helper()
for _, instruction := range found {
if !strings.HasPrefix(instruction, "RUN ") {
continue
}
if strings.Contains(instruction, command) &&
strings.Contains(instruction, "${CHECK_EPOCH}") {
return
}
}
assert.Fail(t, "no epoch-keyed layer runs the command",
"`%s` must run in a layer that expands ${CHECK_EPOCH}, or it"+
" will be replayed from cache without executing", command)
}
// assertBareEpochAssignment fails unless the script computes the epoch
// as a bare assignment on its own line.
func assertBareEpochAssignment(t *testing.T, script, name string) {
t.Helper()
for line := range strings.SplitSeq(script, "\n") {
if strings.TrimSpace(line) == freshEpoch {
return
}
}
assert.Fail(t, "no bare epoch assignment",
"%s must compute `%s` as a bare assignment on its own line, so"+
" `set -e` catches a failing date instead of quietly"+
" building with an empty epoch", name, freshEpoch)
}
// instructions returns the Dockerfile's instructions, one per element,
// with comments and blank lines dropped and continuation lines joined,
// so a multi-line RUN is one string.
func instructions(t *testing.T, name string) []string {
t.Helper()
return strings.Split(instructionText(readRepoFile(t, name)), "\n")
}
// instructionText is instructions' parse, before splitting: it is also
// what a "must not contain" assertion should look at, so that a word
// appearing only in a comment is not mistaken for behaviour.
func instructionText(contents string) string {
var (
out []string
continued string
isContinued bool
)
for line := range strings.SplitSeq(contents, "\n") {
trimmed := strings.TrimSpace(line)
if !isContinued && (trimmed == "" || strings.HasPrefix(trimmed, "#")) {
continue
}
isContinued = strings.HasSuffix(trimmed, `\`)
continued += strings.TrimSuffix(trimmed, `\`)
if isContinued {
continue
}
out = append(out, strings.Join(strings.Fields(continued), " "))
continued = ""
}
return strings.Join(out, "\n")
}
// indexOf returns the position of the first instruction equal to, or
// beginning with, want; -1 if there is none.
func indexOf(found []string, want string) int {
for i, instruction := range found {
if instruction == want || strings.HasPrefix(instruction, want+" ") {
return i
}
}
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 {
t.Helper()
//nolint:gosec // G304: the path is a constant relative to this repo
contents, err := os.ReadFile(filepath.Join(repoRoot(t), name))
require.NoError(t, err)
return string(contents)
}
// repoRoot returns the repository root. The test binary runs with its
// package directory as the working directory, so the root is found by
// walking up until the module file appears.
func repoRoot(t *testing.T) string {
t.Helper()
dir, err := os.Getwd()
require.NoError(t, err)
for {
_, err = os.Stat(filepath.Join(dir, "go.mod"))
if err == nil {
return dir
}
parent := filepath.Dir(dir)
require.NotEqual(t, dir, parent,
"walked to the filesystem root without finding a go.mod")
dir = parent
}
}

View File

@@ -1,151 +0,0 @@
package main_test
import (
"regexp"
"slices"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// This file guards the Makefile that builds this program, which is why
// it lives beside it rather than in a package of its own.
//
// Issue #110: `build` was listed in .PHONY with no `build:` rule
// anywhere in the file. That combination is silently successful — make
// considers a phony target with no prerequisites and no recipe already
// satisfied, so `rm -f vaultik && make build` printed "Nothing to be
// done for 'build'" and exited 0 with no binary produced. Declaring the
// name phony is precisely what converts the "No rule to make target"
// error into a green.
//
// The guard is a parse of the Makefile rather than an invocation of
// make. `make test` is what runs these tests, so shelling back into
// `make build` here would nest a build inside the test run and drop a
// binary into the tree as a side effect of testing. The one property a
// parse cannot establish — that the recipe still fails when the build
// fails — is not testable from inside the build either; it is verified
// by hand against a deliberately broken tree.
// phonyDirective introduces the list of phony target names.
const phonyDirective = ".PHONY:"
// ruleLine matches a rule's target list: a target starts in column
// zero, so recipe lines (tab-indented) and the continuation lines of a
// variable assignment (space-indented) are excluded by construction.
//
// The trailing (?:[^=]|$) rejects `:=` assignments such as
// `VERSION := $(shell script/version)`, which are not rules. Directives
// and function calls (`.PHONY:`, `ifeq`, `$(error ...)`) do not match
// because a target here must begin with a letter, digit or underscore.
var ruleLine = regexp.MustCompile(`^([A-Za-z0-9_][A-Za-z0-9_./ -]*):(?:[^=]|$)`)
// TestPhonyTargetsAllHaveRules fails on any name in .PHONY that has no
// rule in the Makefile. Such a name is not a build target at all: it is
// a command that reports success without doing anything, which is worse
// than one that does not exist, because a caller checking the exit code
// cannot tell the difference.
func TestPhonyTargetsAllHaveRules(t *testing.T) {
t.Parallel()
makefile := readMakefile(t)
phony := phonyTargets(makefile)
require.NotEmpty(t, phony, "no .PHONY names found; the parser is broken")
rules := declaredRules(makefile)
// Sanity check on the rule parser before trusting its verdict: a
// parser that found nothing would pass this test by accident.
require.Contains(t, rules, "vaultik",
"the file rule that builds the binary must be recognized")
for _, target := range phony {
assert.Contains(t, rules, target,
"`.PHONY` lists %q but the Makefile declares no %q rule, so "+
"`make %s` exits 0 without doing anything", target, target, target)
}
}
// TestBuildTargetBuildsTheBinary pins the specific shape of issue #110:
// `make build` has to reach the rule that produces the binary. The test
// above would also pass if `build:` were given an empty recipe of its
// own, which would be the same silent success under a different
// spelling.
func TestBuildTargetBuildsTheBinary(t *testing.T) {
t.Parallel()
prerequisites := rulePrerequisites(readMakefile(t), "build")
require.NotNil(t, prerequisites, "the Makefile declares no `build` rule")
assert.Contains(t, prerequisites, "vaultik",
"`make build` must depend on the rule that builds the binary")
}
// readMakefile returns the contents of the repository's Makefile. The
// root is located by the shared walk in lintdocker_test.go.
func readMakefile(t *testing.T) string {
t.Helper()
return readRepoFile(t, "Makefile")
}
// phonyTargets returns every name declared phony, across all .PHONY
// lines.
func phonyTargets(makefile string) []string {
var targets []string
for line := range strings.SplitSeq(makefile, "\n") {
if !strings.HasPrefix(line, phonyDirective) {
continue
}
targets = append(targets,
strings.Fields(strings.TrimPrefix(line, phonyDirective))...)
}
return targets
}
// declaredRules returns the set of target names that have a rule.
func declaredRules(makefile string) map[string]bool {
rules := make(map[string]bool)
for line := range strings.SplitSeq(makefile, "\n") {
match := ruleLine.FindStringSubmatch(line)
if match == nil {
continue
}
// One rule may name several targets: `a b: prereq`.
for target := range strings.FieldsSeq(match[1]) {
rules[target] = true
}
}
return rules
}
// rulePrerequisites returns the prerequisites of the named rule, or nil
// if no such rule exists. A rule with none returns an empty slice, so
// "declared with nothing to do" is distinguishable from "not declared".
func rulePrerequisites(makefile, target string) []string {
for line := range strings.SplitSeq(makefile, "\n") {
match := ruleLine.FindStringSubmatch(line)
if match == nil {
continue
}
if !slices.Contains(strings.Fields(match[1]), target) {
continue
}
_, after, _ := strings.Cut(line, ":")
return append([]string{}, strings.Fields(after)...)
}
return nil
}

View File

@@ -1,7 +1,6 @@
package cli package cli
import ( import (
"io"
"os" "os"
"strings" "strings"
"time" "time"
@@ -15,12 +14,18 @@ import (
const shortCommitLen = 12 const shortCommitLen = 12
// Entry is the main entry point for the CLI application. // Entry is the main entry point for the CLI application.
// It prints the startup banner to stdout (unless a banner-suppressing // It prints the startup banner (unless a quiet flag is present in os.Args),
// flag is present in os.Args — see bannerSuppressedInArgs), executes the // executes the root cobra command, and routes any returned error through
// root cobra command, and routes any returned error through the // the ui.Writer so the user sees a properly formatted "🛑 ERROR:" line.
// ui.Writer so the user sees a properly formatted "🛑 ERROR:" line.
func Entry() { func Entry() {
emitStartupBanner(os.Args[1:], os.Stdout) if !bannerSuppressedInArgs(os.Args[1:]) {
short := globals.Commit
if len(short) > shortCommitLen {
short = short[:shortCommitLen]
}
writeStartupBanner(ui.New(os.Stdout), time.Now().UTC(), short)
}
rootCmd := NewRootCommand() rootCmd := NewRootCommand()
rootCmd.SilenceErrors = true rootCmd.SilenceErrors = true
@@ -32,24 +37,6 @@ func Entry() {
} }
} }
// emitStartupBanner writes the startup banner to w unless args (the
// argument vector with the program name already stripped) contains a
// flag that suppresses it. Split out of Entry so that the decision — the
// only thing standing between a --json invocation and a parseable
// stdout — is reachable from a test without running the whole CLI.
func emitStartupBanner(args []string, w io.Writer) {
if bannerSuppressedInArgs(args) {
return
}
short := globals.Commit
if len(short) > shortCommitLen {
short = short[:shortCommitLen]
}
writeStartupBanner(ui.New(w), time.Now().UTC(), short)
}
// ReportErrorf emits a user-facing error to stderr in the standard // ReportErrorf emits a user-facing error to stderr in the standard
// 🛑 ERROR: format. Use it from goroutine error paths (where returning // 🛑 ERROR: format. Use it from goroutine error paths (where returning
// an error to cobra isn't an option) and anywhere else a CLI command // an error to cobra isn't an option) and anywhere else a CLI command
@@ -59,20 +46,9 @@ func ReportErrorf(format string, args ...any) {
} }
// bannerSuppressedInArgs reports whether any of args is a flag that // bannerSuppressedInArgs reports whether any of args is a flag that
// should suppress the startup banner (--quiet/-q/--cron/--json). Stops // should suppress the startup banner (--quiet/-q/--cron). Stops at the
// at the "--" argument terminator. Recognizes both long forms and short // "--" argument terminator. Recognizes both long forms and short -q,
// -q, including combined short flags like "-qv". // including combined short flags like "-qv".
//
// This scans the raw argument vector because the banner is printed
// before cobra parses anything — deliberately, so that it still appears
// when cobra rejects the arguments and on --help. The consequence is
// that a flag is matched wherever it occurs in the vector, including
// positions where the command it belongs to would not accept it.
// --json is a subcommand flag rather than a persistent one, but so is
// --cron (it exists only on `snapshot create`), so this adds no new
// class of imprecision. The only cost of a false positive is a missing
// decorative banner; the cost of a false negative is a corrupt document
// on stdout, so the scan errs deliberately in that direction.
func bannerSuppressedInArgs(args []string) bool { func bannerSuppressedInArgs(args []string) bool {
for _, a := range args { for _, a := range args {
if a == "--" { if a == "--" {
@@ -80,13 +56,11 @@ func bannerSuppressedInArgs(args []string) bool {
} }
switch a { switch a {
case "--quiet", "-q", "--cron", "--json": case "--quiet", "-q", "--cron":
return true return true
} }
if strings.HasPrefix(a, "--quiet=") || if strings.HasPrefix(a, "--quiet=") || strings.HasPrefix(a, "--cron=") {
strings.HasPrefix(a, "--cron=") ||
strings.HasPrefix(a, "--json=") {
return true return true
} }
// Combined short flags like -qv or -vq. // Combined short flags like -qv or -vq.

View File

@@ -1,300 +0,0 @@
package cli //nolint:testpackage // needs access to unexported emitStartupBanner
import (
"bytes"
"encoding/json"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"testing"
"github.com/adrg/xdg"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Command words and flags used to build argument vectors below. They are
// constants rather than repeated literals so that a rename shows up as a
// compile error in one place.
const (
cmdSnapshot = "snapshot"
cmdList = "list"
cmdCreate = "create"
cmdVerify = "verify"
cmdRemove = "remove"
cmdPrune = "prune"
cmdRemote = "remote"
cmdInfo = "info"
flagJSON = "--json"
flagQuiet = "--quiet"
flagConfig = "--config"
// programName is argv[0] as the real process receives it. Entry
// strips it before scanning, so it has to be present.
programName = "vaultik"
// someSnapshotID is any snapshot identifier: these tests never run
// the command, so it only has to occupy the positional argument.
someSnapshotID = "host_2026-01-01T00:00:00Z"
)
// placeholderJSONDocument stands in for whatever document a --json
// command writes to stdout. `snapshot list --json` with no snapshots
// prints exactly this; the other --json commands print an object rather
// than an array, but this test is not about their shape. It is about
// what is on stdout *before* them, which is the same for all of them
// because Entry prints the banner before cobra has parsed anything and
// therefore before it can know which command is running.
const placeholderJSONDocument = "[]\n"
// jsonArgumentVectors are the argument vectors of every --json
// invocation the CLI accepts, with the program name stripped exactly as
// Entry strips it. Each one must leave stdout untouched by the banner.
//
//nolint:gochecknoglobals // read-only test fixture shared by two tests
var jsonArgumentVectors = map[string][]string{
"snapshot list": {cmdSnapshot, cmdList, flagJSON},
"snapshot verify": {cmdSnapshot, cmdVerify, someSnapshotID, flagJSON},
"snapshot remove": {cmdSnapshot, cmdRemove, someSnapshotID, flagJSON},
"prune": {cmdPrune, flagJSON},
"remote info": {cmdRemote, cmdInfo, flagJSON},
// --json before the subcommand, and with an explicit value: the
// scan is positional, so both forms have to be recognized.
"json first": {flagJSON, cmdSnapshot, cmdList},
"json with value": {cmdSnapshot, cmdList, flagJSON + "=true"},
// A --json invocation that also carries a flag with a value, so the
// scan cannot be fooled by an argument that consumes the next one.
"json with config": {
flagConfig, "/nonexistent/vaultik.yml", cmdSnapshot, cmdList, flagJSON,
},
}
// TestJSONInvocationStdoutIsExactlyOneDocument is the CLI-layer
// regression guard for issue #106: `vaultik snapshot list --json | jq`
// must work with no other flags.
//
// internal/vaultik's TestListSnapshots_JSONStdoutIsOnlyTheDocument
// guards the same contract one layer down, but it calls the library
// function directly and so cannot see Entry, which is where the
// contamination was: the startup banner is written to stdout before
// cobra parses anything, and the suppression scan did not know about
// --json. The two banner lines and the blank line landed ahead of the
// document and `jq` refused the result.
//
// The document is a constant here because this test is about the
// argument vectors, one per --json command; the one that runs a real
// command end to end is TestEntryJSONStdoutIsExactlyOneDocument below.
func TestJSONInvocationStdoutIsExactlyOneDocument(t *testing.T) {
t.Parallel()
for name, argv := range jsonArgumentVectors {
t.Run(name, func(t *testing.T) {
t.Parallel()
var stdout bytes.Buffer
emitStartupBanner(argv, &stdout)
require.Empty(t, stdout.String(),
"nothing may reach stdout ahead of a --json document")
_, err := stdout.WriteString(placeholderJSONDocument)
require.NoError(t, err)
requireExactlyOneJSONDocument(t, stdout.String())
})
}
}
// TestBannerStillPrintedWithoutSuppressingFlag pins the other half of
// the contract. Without it, deleting the banner outright would satisfy
// the test above, and the banner is wanted on interactive invocations.
func TestBannerStillPrintedWithoutSuppressingFlag(t *testing.T) {
t.Parallel()
for name, argv := range map[string][]string{
"no flags": {cmdSnapshot, cmdList},
"verbose": {cmdSnapshot, cmdList, "--verbose"},
"after the terminator": {
cmdSnapshot, "restore", "--", flagJSON,
},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
var stdout bytes.Buffer
emitStartupBanner(argv, &stdout)
assert.Contains(t, stdout.String(), "starting up at",
"the banner belongs on invocations that did not opt out")
})
}
}
// TestBannerSuppressedInArgs covers the suppression scan directly,
// including the flags that suppressed the banner before --json joined
// them, so that adding --json cannot regress them.
func TestBannerSuppressedInArgs(t *testing.T) {
t.Parallel()
for name, testCase := range map[string]struct {
args []string
suppressed bool
}{
"quiet long": {[]string{cmdSnapshot, cmdCreate, flagQuiet}, true},
"quiet short": {[]string{cmdSnapshot, cmdCreate, "-q"}, true},
"quiet combined": {[]string{cmdSnapshot, cmdCreate, "-qv"}, true},
"cron": {[]string{cmdSnapshot, cmdCreate, "--cron"}, true},
"json": {[]string{cmdSnapshot, cmdList, flagJSON}, true},
"nothing": {[]string{cmdSnapshot, cmdList}, false},
"empty": {nil, false},
"json after dashes": {
[]string{cmdSnapshot, cmdList, "--", flagJSON}, false,
},
"quiet after dashes": {
[]string{cmdSnapshot, cmdCreate, "--", "-q"}, false,
},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, testCase.suppressed,
bannerSuppressedInArgs(testCase.args))
})
}
}
// hermeticConfig is a complete, valid config that needs no network and
// no credentials: file:// storage is exempt from the S3 credential
// checks, and FileStorer over a directory that does not exist lists
// zero objects without erroring. Chunk, blob and compression settings
// are filled in by config.Load.
const hermeticConfig = `age_recipients:
- age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj
snapshots:
test:
paths:
- %s
storage_url: file://%s
index_path: %s
hostname: test-host
`
// TestEntryJSONStdoutIsExactlyOneDocument runs the real thing: Entry,
// with a real argument vector, over the process's real stdout file
// descriptor, all the way through cobra and the fx graph to the
// document. It is the assertion the issue asks for — `vaultik snapshot
// list --json | jq .` with no other flags — with the pipe replaced by a
// decoder.
//
// `snapshot list` is the command chosen because it is the only --json
// command that reaches its document without a populated destination
// store: it reads the local index, streams `metadata/` (empty here),
// and treats a barren destination as an empty list rather than a
// failure.
//
// Not parallel: it replaces os.Args, os.Stdout and the xdg globals.
func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) {
dir := t.TempDir()
configPath := filepath.Join(dir, "config.yml")
contents := fmt.Sprintf(hermeticConfig,
filepath.Join(dir, "source"),
filepath.Join(dir, "store"),
filepath.Join(dir, "index.sqlite"))
require.NoError(t,
os.WriteFile(configPath, []byte(contents), configFileMode))
// The PID lock lives under xdg.DataHome, which xdg resolves at
// package init; point it at the temp dir so the test neither
// touches nor collides with the real one.
t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data"))
xdg.Reload()
t.Cleanup(xdg.Reload)
previousArgs := os.Args
t.Cleanup(func() {
os.Args = previousArgs
rootFlags = RootFlags{}
})
os.Args = []string{
programName, flagConfig, configPath, cmdSnapshot, cmdList, flagJSON,
}
stdout := captureProcessStdout(t, Entry)
requireExactlyOneJSONDocument(t, stdout)
var snapshots []any
require.NoError(t, json.Unmarshal([]byte(stdout), &snapshots))
assert.Empty(t, snapshots,
"a destination store with no snapshots lists none")
}
// captureProcessStdout redirects the process's own stdout to a pipe for
// the duration of fn and returns what was written to it. The redirection
// has to be at the file-descriptor level rather than through an injected
// writer, because the banner and the JSON encoder reach os.Stdout
// independently and the point of the test is that both land in the same
// place.
//
// Not parallel-safe: os.Stdout is process-global.
func captureProcessStdout(t *testing.T, fn func()) string {
t.Helper()
reader, writer, err := os.Pipe()
require.NoError(t, err)
previous := os.Stdout
os.Stdout = writer
captured := make(chan string, 1)
go func() {
var buf bytes.Buffer
_, _ = io.Copy(&buf, reader)
captured <- buf.String()
}()
fn()
os.Stdout = previous
require.NoError(t, writer.Close())
out := <-captured
require.NoError(t, reader.Close())
return out
}
// requireExactlyOneJSONDocument fails unless stdout decodes as a single
// JSON value with nothing before or after it — the property that makes
// `| jq` work.
func requireExactlyOneJSONDocument(t *testing.T, stdout string) {
t.Helper()
decoder := json.NewDecoder(strings.NewReader(stdout))
var document any
err := decoder.Decode(&document)
require.NoError(t, err,
"stdout must parse as JSON, got:\n%s", stdout)
_, err = decoder.Token()
require.ErrorIs(t, err, io.EOF,
"stdout must hold exactly one JSON document, got:\n%s", stdout)
}

View File

@@ -1,165 +0,0 @@
package cli //nolint:testpackage // shares hermeticConfig and the capture helpers
import (
"context"
"database/sql"
"encoding/json"
"fmt"
"os"
"path/filepath"
"testing"
"time"
"github.com/adrg/xdg"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/types"
)
// pruneJSONDocument is the shape `prune --json` writes: the
// PruneBlobsResult document, and nothing else.
//
//nolint:tagliatelle // snake_case is the established JSON output format
type pruneJSONDocument struct {
BlobsFound int `json:"blobs_found"`
BlobsDeleted int `json:"blobs_deleted"`
BytesFreed int64 `json:"bytes_freed"`
}
// stalePruneSnapshotID is seeded into the local index with no manifest
// on the destination store, which is exactly what makes it stale.
const stalePruneSnapshotID = "test-host_test_2026-04-01T09:00:00Z"
// TestEntryPruneJSONStdoutIsExactlyOneDocument is the end-to-end
// regression guard for issue #108: `vaultik prune --json | jq .` must
// work with no other flags.
//
// It runs Entry over the process's real stdout descriptor, through
// cobra and the fx graph, against a hermetic file:// destination store
// — the same construction TestEntryJSONStdoutIsExactlyOneDocument uses
// for `snapshot list`, with the pipe to jq replaced by a decoder.
//
// Both branches of the local-snapshot reconciliation are exercised
// because the three stdout writes that broke this covered all of them:
// one line per stale record and a summary when there were any, and a
// "No stale local snapshots found." line when there were none. No input
// avoided the contamination, so no single branch demonstrates the fix.
//
// Not parallel: it replaces os.Args, os.Stdout and the xdg globals.
//
//nolint:paralleltest // replaces os.Args, os.Stdout and the xdg globals
func TestEntryPruneJSONStdoutIsExactlyOneDocument(t *testing.T) {
for _, testCase := range []struct {
name string
seedStale bool
description string
}{
{
name: "no stale local records",
seedStale: false,
description: "the empty-index branch used to print a 'No stale' line",
},
{
name: "stale local records present",
seedStale: true,
description: "the removal branch used to print a line per record " +
"plus a summary",
},
} {
t.Run(testCase.name, func(t *testing.T) {
configPath := writeHermeticPruneConfig(t, testCase.seedStale)
previousArgs := os.Args
t.Cleanup(func() {
os.Args = previousArgs
rootFlags = RootFlags{}
})
os.Args = []string{
programName, flagConfig, configPath, cmdPrune, flagJSON,
}
stdout := captureProcessStdout(t, Entry)
requireExactlyOneJSONDocument(t, stdout)
var document pruneJSONDocument
require.NoError(t, json.Unmarshal([]byte(stdout), &document),
testCase.description)
// A destination store with no blobs has none to prune. The
// assertion that matters is the one above; this one keeps the
// test honest about which document it decoded.
assert.Equal(t, 0, document.BlobsFound)
})
}
}
// writeHermeticPruneConfig builds a config over a temp directory and, if
// seedStale is set, creates the index database up front with one
// snapshot record that has no counterpart on the destination store.
// Returns the config path.
func writeHermeticPruneConfig(t *testing.T, seedStale bool) string {
t.Helper()
dir := t.TempDir()
configPath := filepath.Join(dir, "config.yml")
indexPath := filepath.Join(dir, "index.sqlite")
contents := fmt.Sprintf(hermeticConfig,
filepath.Join(dir, "source"),
filepath.Join(dir, "store"),
indexPath)
require.NoError(t,
os.WriteFile(configPath, []byte(contents), configFileMode))
// The PID lock lives under xdg.DataHome, which xdg resolves at
// package init; point it at the temp dir so the test neither
// touches nor collides with the real one.
t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data"))
xdg.Reload()
t.Cleanup(xdg.Reload)
if seedStale {
seedStaleSnapshotRecord(t, indexPath)
}
return configPath
}
// seedStaleSnapshotRecord creates the index database at path and
// inserts one completed snapshot into it. Nothing is written to the
// destination store, so `prune` finds the record stale and removes it —
// the branch that printed a line per record.
func seedStaleSnapshotRecord(t *testing.T, path string) {
t.Helper()
ctx := context.Background()
db, err := database.New(ctx, path)
require.NoError(t, err)
defer func() { require.NoError(t, db.Close()) }()
startedAt := time.Date(2026, 4, 1, 9, 0, 0, 0, time.UTC)
completedAt := startedAt.Add(time.Minute)
snap := &database.Snapshot{
ID: types.SnapshotID(stalePruneSnapshotID),
Hostname: "test-host",
VaultikVersion: "test",
StartedAt: startedAt,
CompletedAt: &completedAt,
}
repos := database.NewRepositories(db)
err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
return repos.Snapshots.Create(ctx, tx, snap)
})
require.NoError(t, err)
}

View File

@@ -63,15 +63,8 @@ func New() (*Globals, error) {
// a release. Both "dev" and "dev-<sha>" (and its "-dirty" variant) // a release. Both "dev" and "dev-<sha>" (and its "-dirty" variant)
// count: a caller that compares against "dev" exactly would treat every // count: a caller that compares against "dev" exactly would treat every
// commit-stamped development build as a release. // commit-stamped development build as a release.
//
// The empty string counts too. Nothing that knows its version reports
// no version, so an empty Version means the stamping failed, and the
// safe reading of "we could not establish that this is a release" is
// that it is not one. The Makefile refuses to build at all in that
// case; this is the second line of defence, for a binary linked by
// something other than the Makefile.
func IsDevVersion(v string) bool { func IsDevVersion(v string) bool {
return v == "" || v == DevVersion || strings.HasPrefix(v, DevVersion+"-") return v == DevVersion || strings.HasPrefix(v, DevVersion+"-")
} }
// shortCommitLen is the number of commit-hash characters ShortCommit keeps. // shortCommitLen is the number of commit-hash characters ShortCommit keeps.

View File

@@ -59,11 +59,7 @@ func TestIsDevVersion(t *testing.T) {
// the string happens to contain "dev". // the string happens to contain "dev".
{"1.0.0-dev", false}, {"1.0.0-dev", false},
{"developer", false}, {"developer", false},
// A binary with no version string at all did not get stamped, {"", false},
// which is a build failure, not a release. It must never print
// as one. The Makefile refuses to build when script/version
// yields nothing; this covers a binary linked some other way.
{"", true},
} }
for _, tc := range cases { for _, tc := range cases {

View File

@@ -1,9 +1,5 @@
// Package log provides the application-wide structured logger: slog // Package log provides the application-wide structured logger: slog
// writing to stderr, with a colorized TTY handler when stderr is a // with a colorized TTY handler on terminals and JSON output otherwise.
// terminal and JSON output otherwise.
//
// Everything this package emits is a diagnostic, so it all goes to
// stderr. stdout belongs to the output the user asked for.
package log //nolint:revive,nolintlint // stdlib log unused here; see #76 package log //nolint:revive,nolintlint // stdlib log unused here; see #76
import ( import (
@@ -73,27 +69,13 @@ func Initialize(cfg Config) {
Level: level, Level: level,
} }
// Diagnostics go to stderr, never to stdout. stdout is reserved for // Check if stdout is a TTY.
// the output the user asked for: every --json subcommand writes its if term.IsTerminal(int(os.Stdout.Fd())) {
// document there, and WARN/ERROR are never suppressed, so a logger
// on stdout puts log records inside that document and makes it
// unparseable. A config file with group- or world-readable
// permissions is enough to trigger it (see internal/config), so this
// was not a theoretical collision.
//
// The format is chosen by the TTY-ness of the stream the records
// actually land on. AGENTS.md policy 9 says "if stdout is not a
// terminal, emit jsonl"; it says stdout because that is where logs
// used to go, and the property it is really asking for is that
// output nobody is watching be machine-readable. Testing stdout here
// would colorize records on a redirected stderr whenever stdout
// happened to be a terminal, and vice versa.
if term.IsTerminal(int(os.Stderr.Fd())) {
// Use colorized TTY handler // Use colorized TTY handler
logger = slog.New(NewTTYHandler(os.Stderr, opts)) logger = slog.New(NewTTYHandler(os.Stdout, opts))
} else { } else {
// Use JSON format for non-TTY output // Use JSON format for non-TTY output
logger = slog.New(slog.NewJSONHandler(os.Stderr, opts)) logger = slog.New(slog.NewJSONHandler(os.Stdout, opts))
} }
// Set as default logger // Set as default logger

View File

@@ -5,34 +5,10 @@ import (
"fmt" "fmt"
"io" "io"
"log/slog" "log/slog"
"strings"
"sync" "sync"
"time" "time"
) )
// groupSeparator joins an open group path to an attribute key. This
// format has no nesting, so a group becomes a dotted key prefix:
// slog.New(h).WithGroup("db").With("rows", 3) renders "db.rows=3".
const groupSeparator = "."
// bytesAttrKey is the attribute key whose int64 value is rendered as a
// human-readable byte count rather than a bare number. Keys reaching
// writeAttr are group-qualified, so the match is made against the final
// dot-separated segment: without that, a "bytes" attribute logged under
// an open group would arrive as "transfer.bytes" and silently lose its
// formatting.
const bytesAttrKey = "bytes"
// isBytesAttr reports whether a group-qualified attribute key names the
// byte-count attribute, i.e. whether its last segment is bytesAttrKey.
func isBytesAttr(key string) bool {
if idx := strings.LastIndex(key, groupSeparator); idx >= 0 {
key = key[idx+len(groupSeparator):]
}
return key == bytesAttrKey
}
// ANSI color codes // ANSI color codes
const ( const (
colorReset = "\033[0m" colorReset = "\033[0m"
@@ -46,26 +22,10 @@ const (
) )
// TTYHandler is a custom slog handler for TTY output with colors. // TTYHandler is a custom slog handler for TTY output with colors.
//
// A handler and the handlers derived from it via WithAttrs/WithGroup
// all write to the same stream, so they share one mutex; that is why mu
// is a pointer. A value mutex would give every derived handler its own
// lock and stop serializing writes to the stream they have in common.
type TTYHandler struct { type TTYHandler struct {
opts slog.HandlerOptions opts slog.HandlerOptions
mu *sync.Mutex mu sync.Mutex
out io.Writer out io.Writer
// attrs are the attributes accumulated through WithAttrs, emitted
// ahead of each record's own attributes. Their keys already carry
// the group path that was open when they were added, so no
// qualification happens at write time.
attrs []slog.Attr
// groups is the group path opened by WithGroup, applied as a key
// prefix to attributes that arrive later — both on a record and
// through a further WithAttrs.
groups []string
} }
// NewTTYHandler creates a new TTY handler with colored output. // NewTTYHandler creates a new TTY handler with colored output.
@@ -77,7 +37,6 @@ func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler {
return &TTYHandler{ return &TTYHandler{
out: out, out: out,
opts: *opts, opts: *opts,
mu: &sync.Mutex{},
} }
} }
@@ -122,20 +81,30 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
levelColor, level, colorReset, levelColor, level, colorReset,
colorBold, r.Message, colorReset) colorBold, r.Message, colorReset)
// Attributes carried by the handler come first, then the record's // Print attributes
// own. Handler attributes were qualified when they were added; the
// record's are qualified now, against whatever group path is open.
for _, a := range h.attrs {
h.writeAttr(a)
}
prefix := strings.Join(h.groups, groupSeparator)
r.Attrs(func(a slog.Attr) bool { r.Attrs(func(a slog.Attr) bool {
for _, flat := range appendAttr(nil, prefix, a) { value := a.Value.String()
h.writeAttr(flat) // Special handling for certain attribute types
switch a.Value.Kind() {
case slog.KindDuration:
if d, ok := a.Value.Any().(time.Duration); ok {
value = formatDuration(d)
}
case slog.KindInt64:
if a.Key == "bytes" {
value = formatBytes(a.Value.Int64())
}
case slog.KindAny, slog.KindBool, slog.KindFloat64, slog.KindString,
slog.KindTime, slog.KindUint64, slog.KindGroup, slog.KindLogValuer:
// Plain string form above is already correct for these kinds.
default:
// Future kinds also use the plain string form.
} }
_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
colorCyan, a.Key, colorReset,
colorBlue, value, colorReset)
return true return true
}) })
@@ -144,125 +113,14 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
return nil return nil
} }
// appendAttr flattens a into dst, folding prefix into its key and // WithAttrs returns a new handler with the given attributes.
// expanding group values into further dotted keys. Following the func (h *TTYHandler) WithAttrs(_ []slog.Attr) slog.Handler {
// slog.Handler contract: an empty Attr is dropped, a group with no return h // Simplified for now
// attributes is dropped, and a group with an empty key is inlined into
// its parent rather than contributing a level.
func appendAttr(dst []slog.Attr, prefix string, a slog.Attr) []slog.Attr {
a.Value = a.Value.Resolve()
if a.Equal(slog.Attr{}) {
return dst
}
key := a.Key
switch {
case prefix == "":
// key stands alone.
case key == "":
key = prefix
default:
key = prefix + groupSeparator + key
}
if a.Value.Kind() != slog.KindGroup {
return append(dst, slog.Attr{Key: key, Value: a.Value})
}
for _, member := range a.Value.Group() {
dst = appendAttr(dst, key, member)
}
return dst
} }
// WithAttrs returns a new handler that emits attrs on every record it // WithGroup returns a new handler with the given group name.
// handles, in addition to whatever the handler already carried. Keys func (h *TTYHandler) WithGroup(_ string) slog.Handler {
// are qualified by the group path open at the time of the call, so return h // Simplified for now
// WithGroup("db").WithAttrs(rows=3) later renders "db.rows=3".
//
// The receiver is not modified.
func (h *TTYHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
if len(attrs) == 0 {
return h
}
prefix := strings.Join(h.groups, groupSeparator)
next := h.clone()
for _, a := range attrs {
next.attrs = appendAttr(next.attrs, prefix, a)
}
return next
}
// WithGroup returns a new handler that qualifies every subsequent
// attribute key with name. This format is a single line with nowhere to
// nest, so grouping is rendered as a dotted key prefix: after
// WithGroup("db"), an attribute "rows" is emitted as "db.rows".
//
// An empty name returns the receiver unchanged, per the slog.Handler
// contract. The receiver is not modified.
func (h *TTYHandler) WithGroup(name string) slog.Handler {
if name == "" {
return h
}
next := h.clone()
next.groups = append(next.groups, name)
return next
}
// clone returns a copy of h that shares its output stream and mutex but
// owns its attribute and group slices.
//
// The slices are copied rather than resliced on purpose. slog permits
// one handler to be derived from concurrently, and two derivations that
// appended into a shared backing array would each overwrite the other's
// attribute — a data race with a silent wrong-output failure mode.
func (h *TTYHandler) clone() *TTYHandler {
next := &TTYHandler{
opts: h.opts,
mu: h.mu,
out: h.out,
attrs: make([]slog.Attr, len(h.attrs), len(h.attrs)+1),
groups: make([]string, len(h.groups), len(h.groups)+1),
}
copy(next.attrs, h.attrs)
copy(next.groups, h.groups)
return next
}
// writeAttr renders one already-flattened, already-qualified attribute
// as " key=value". Callers hold h.mu.
func (h *TTYHandler) writeAttr(a slog.Attr) {
value := a.Value.String()
// Special handling for certain attribute types
switch a.Value.Kind() {
case slog.KindDuration:
if d, ok := a.Value.Any().(time.Duration); ok {
value = formatDuration(d)
}
case slog.KindInt64:
if isBytesAttr(a.Key) {
value = formatBytes(a.Value.Int64())
}
case slog.KindAny, slog.KindBool, slog.KindFloat64, slog.KindString,
slog.KindTime, slog.KindUint64, slog.KindGroup, slog.KindLogValuer:
// Plain string form above is already correct for these kinds.
default:
// Future kinds also use the plain string form.
}
_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
colorCyan, a.Key, colorReset,
colorBlue, value, colorReset)
} }
// formatDuration formats a duration in a human-readable way // formatDuration formats a duration in a human-readable way

View File

@@ -1,422 +0,0 @@
package log_test
import (
"bytes"
"context"
"encoding/json"
"fmt"
"log/slog"
"math"
"regexp"
"sort"
"strconv"
"strings"
"sync"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
)
// ansiEscape matches the SGR sequences TTYHandler wraps every field in.
// Stripping them is what lets a test compare TTYHandler's rendering with
// slog.JSONHandler's.
var ansiEscape = regexp.MustCompile(`\x1b\[[0-9;]*m`)
// countKey is an attribute key reused across the comparison cases.
const countKey = "count"
// debugHandlerOptions enables every level, so a test never has to reason
// about the default level while reasoning about attributes.
func debugHandlerOptions() *slog.HandlerOptions {
return &slog.HandlerOptions{Level: slog.LevelDebug}
}
// ttyAttrs renders one record through a TTYHandler and returns its
// attributes as key -> value, with color stripped.
//
// TTYHandler emits " key=value" per attribute after the message, and the
// message itself is the last thing before the first attribute, so
// splitting on spaces and keeping the tokens containing "=" recovers the
// attribute set. Test values below therefore avoid spaces and "=".
func ttyAttrs(t *testing.T, derive func(*slog.Logger) *slog.Logger,
msg string, args ...any,
) map[string]string {
t.Helper()
var buf bytes.Buffer
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
derive(logger).Info(msg, args...)
line := ansiEscape.ReplaceAllString(buf.String(), "")
attrs := make(map[string]string)
for token := range strings.FieldsSeq(line) {
key, value, found := strings.Cut(token, "=")
if !found {
continue
}
attrs[key] = value
}
return attrs
}
// jsonAttrs renders one record through slog.JSONHandler and returns its
// attributes flattened to the same dotted-key form TTYHandler uses, so
// the two are directly comparable. The built-in time/level/msg fields
// are dropped: they are the record, not its attributes.
func jsonAttrs(t *testing.T, derive func(*slog.Logger) *slog.Logger,
msg string, args ...any,
) map[string]string {
t.Helper()
var buf bytes.Buffer
logger := slog.New(slog.NewJSONHandler(&buf, debugHandlerOptions()))
derive(logger).Info(msg, args...)
var decoded map[string]any
require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded))
delete(decoded, slog.TimeKey)
delete(decoded, slog.LevelKey)
delete(decoded, slog.MessageKey)
attrs := make(map[string]string)
flattenJSON(attrs, "", decoded)
return attrs
}
// flattenJSON turns JSONHandler's nested group objects into the dotted
// keys TTYHandler writes.
func flattenJSON(dst map[string]string, prefix string, src map[string]any) {
for key, value := range src {
full := key
if prefix != "" {
full = prefix + "." + key
}
nested, ok := value.(map[string]any)
if ok {
flattenJSON(dst, full, nested)
continue
}
dst[full] = valueString(value)
}
}
// valueString renders a decoded JSON scalar the way slog.Value.String
// renders the corresponding Go value, so the two handlers' outputs can
// be compared as strings. encoding/json decodes every number as
// float64, so an integral one is rendered back as an integer — which is
// what the Go value that produced it was.
func valueString(v any) string {
switch typed := v.(type) {
case string:
return typed
case bool:
return strconv.FormatBool(typed)
case float64:
if typed == math.Trunc(typed) {
return strconv.FormatInt(int64(typed), 10)
}
return strconv.FormatFloat(typed, 'g', -1, 64)
default:
return fmt.Sprint(v)
}
}
// TestTTYHandlerWithAttrsEmitsAttributes is the direct regression test
// for the reported defect: WithAttrs discarded its argument, so an
// attribute attached to a logger never reached the output.
func TestTTYHandlerWithAttrsEmitsAttributes(t *testing.T) {
t.Parallel()
attrs := ttyAttrs(t, func(l *slog.Logger) *slog.Logger {
return l.With("key", "value")
}, "hello")
assert.Equal(t, "value", attrs["key"],
"an attribute attached with With must appear on every record")
}
// TestTTYHandlerWithAttrsPersistsAcrossRecords checks that the
// attributes are retained rather than emitted once. A handler that
// stored them but consumed them would pass the test above.
func TestTTYHandlerWithAttrsPersistsAcrossRecords(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions())).
With("request", "abc123")
logger.Info("first")
logger.Info("second")
plain := ansiEscape.ReplaceAllString(buf.String(), "")
lines := strings.Split(strings.TrimSuffix(plain, "\n"), "\n")
require.Len(t, lines, 2)
for _, line := range lines {
assert.Contains(t, line, "request=abc123")
}
}
// TestTTYHandlerWithGroupQualifiesKeys checks that WithGroup does
// something real rather than being discarded. This format has no
// nesting, so grouping shows up as a dotted key prefix.
func TestTTYHandlerWithGroupQualifiesKeys(t *testing.T) {
t.Parallel()
attrs := ttyAttrs(t, func(l *slog.Logger) *slog.Logger {
return l.WithGroup("db").With("rows", 3)
}, "queried", "table", "chunks")
assert.Equal(t, "3", attrs["db.rows"],
"an attribute added under a group must be qualified by it")
assert.Equal(t, "chunks", attrs["db.table"],
"a record attribute must also be qualified by the open group")
assert.NotContains(t, attrs, "rows")
}
// TestTTYHandlerByteFormattingSurvivesGrouping guards the interaction
// between the two features. The human-readable rendering of a "bytes"
// attribute is selected by comparing the key, and keys reaching that
// comparison are group-qualified, so a "bytes" attribute logged under an
// open group arrived as "transfer.bytes" and fell back to a bare number.
// No caller groups a byte count today, which is exactly why this needs a
// test rather than a bug report.
func TestTTYHandlerByteFormattingSurvivesGrouping(t *testing.T) {
t.Parallel()
const oneAndAHalfKiB = 1536
for name, testCase := range map[string]struct {
derive func(*slog.Logger) *slog.Logger
key string
}{
"ungrouped": {
derive: func(l *slog.Logger) *slog.Logger { return l },
key: "bytes",
},
"grouped": {
derive: func(l *slog.Logger) *slog.Logger {
return l.WithGroup("transfer")
},
key: "transfer.bytes",
},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
testCase.derive(logger).Info("uploaded", "bytes", oneAndAHalfKiB)
line := ansiEscape.ReplaceAllString(buf.String(), "")
assert.Contains(t, line, testCase.key+"=1.5 KB",
"a byte count must be human-readable however it is qualified")
assert.NotContains(t, line, strconv.Itoa(oneAndAHalfKiB),
"the raw number must not survive the formatting")
})
}
}
// TestTTYHandlerMatchesJSONHandlerAttributes is the drift guard. The
// handler is chosen by TTY-ness, so a difference between these two is
// invisible in whichever environment the developer is not in — which is
// how the original defect survived: attributes vanished on a terminal
// and were correct in CI.
func TestTTYHandlerMatchesJSONHandlerAttributes(t *testing.T) {
t.Parallel()
cases := []struct {
name string
derive func(*slog.Logger) *slog.Logger
args []any
}{
{
name: "record attributes only",
derive: func(l *slog.Logger) *slog.Logger { return l },
args: []any{"path", "/etc/vaultik", countKey, 7},
},
{
name: "handler attributes",
derive: func(l *slog.Logger) *slog.Logger {
return l.With("host", "alpha")
},
args: []any{countKey, 7},
},
{
name: "handler attributes accumulate",
derive: func(l *slog.Logger) *slog.Logger {
return l.With("host", "alpha").With("snapshot", "s1")
},
args: []any{countKey, 7},
},
{
name: "group qualifies later attributes",
derive: func(l *slog.Logger) *slog.Logger {
return l.WithGroup("db").With("rows", 3)
},
args: []any{"table", "chunks"},
},
{
name: "nested groups",
derive: func(l *slog.Logger) *slog.Logger {
return l.WithGroup("outer").WithGroup("inner").
With("leaf", "v")
},
args: []any{"other", "w"},
},
{
name: "attributes before and after a group",
derive: func(l *slog.Logger) *slog.Logger {
return l.With("top", "t").WithGroup("g").With("in", "i")
},
args: []any{"rec", "r"},
},
{
name: "inline group value on the record",
derive: func(l *slog.Logger) *slog.Logger { return l },
args: []any{slog.Group("net",
slog.String("proto", "s3"), slog.Int("retries", 2))},
},
}
for _, testCase := range cases {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
tty := ttyAttrs(t, testCase.derive, "message", testCase.args...)
js := jsonAttrs(t, testCase.derive, "message", testCase.args...)
assert.Equal(t, sortedKeys(js), sortedKeys(tty),
"TTY and JSON handlers must emit the same attribute keys")
assert.Equal(t, js, tty,
"TTY and JSON handlers must emit the same attribute values")
})
}
}
// sortedKeys returns m's keys in order, for a stable comparison message.
func sortedKeys(m map[string]string) []string {
keys := make([]string, 0, len(m))
for key := range m {
keys = append(keys, key)
}
sort.Strings(keys)
return keys
}
// TestTTYHandlerWithAttrsDoesNotMutateReceiver checks that deriving does
// not write through to the parent or to a sibling. slog permits a
// handler to be shared, so a WithAttrs that appended into the receiver's
// state would leak attributes between unrelated loggers.
func TestTTYHandlerWithAttrsDoesNotMutateReceiver(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
base := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
first := base.With("branch", "one")
second := base.With("branch", "two")
base.Info("base")
first.Info("first")
second.Info("second")
plain := ansiEscape.ReplaceAllString(buf.String(), "")
lines := strings.Split(strings.TrimSuffix(plain, "\n"), "\n")
require.Len(t, lines, 3)
assert.NotContains(t, lines[0], "branch=",
"deriving must not add attributes to the handler derived from")
assert.Contains(t, lines[1], "branch=one")
assert.NotContains(t, lines[1], "branch=two")
assert.Contains(t, lines[2], "branch=two")
assert.NotContains(t, lines[2], "branch=one")
}
// TestTTYHandlerConcurrentDerivation exercises the same handler being
// derived from and written through by several goroutines at once, which
// is what slog permits and what a mutating WithAttrs would make a data
// race. Run under -race by script/test.
func TestTTYHandlerConcurrentDerivation(t *testing.T) {
t.Parallel()
const workers = 16
var buf bytes.Buffer
base := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions())).
With("shared", "yes")
var group sync.WaitGroup
group.Add(workers)
for worker := range workers {
go func() {
defer group.Done()
base.With("worker", worker).
WithGroup("g").
With("nested", worker).
Info("concurrent")
}()
}
group.Wait()
plain := ansiEscape.ReplaceAllString(buf.String(), "")
lines := strings.Split(strings.TrimSuffix(plain, "\n"), "\n")
require.Len(t, lines, workers)
for _, line := range lines {
assert.Contains(t, line, "shared=yes")
assert.Contains(t, line, "worker=")
assert.Contains(t, line, "g.nested=")
}
}
// TestTTYHandlerEmptyGroupAndAttrsAreNoOps covers the slog.Handler
// contract corners: WithGroup("") and WithAttrs(nil) change nothing, and
// an empty Attr is dropped rather than rendered as "=".
func TestTTYHandlerEmptyGroupAndAttrsAreNoOps(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
handler := log.NewTTYHandler(&buf, debugHandlerOptions())
assert.Same(t, handler, handler.WithGroup(""),
"an empty group name must not open a group")
assert.Same(t, handler, handler.WithAttrs(nil),
"deriving with no attributes must not allocate a handler")
slog.New(handler).LogAttrs(context.Background(), slog.LevelInfo, "msg",
slog.Attr{}, slog.String("kept", "yes"))
plain := ansiEscape.ReplaceAllString(buf.String(), "")
assert.Contains(t, plain, "kept=yes")
assert.NotContains(t, plain, " =")
}

View File

@@ -1,64 +0,0 @@
//nolint:testpackage // needs the package logger; see TestWithAttributesReachTTYOutput
package log //nolint:revive,nolintlint // stdlib log unused here; see #76
import (
"bytes"
"log/slog"
"regexp"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// withTestANSIEscape matches the SGR sequences TTYHandler emits.
var withTestANSIEscape = regexp.MustCompile(`\x1b\[[0-9;]*m`)
// TestWithAttributesReachTTYOutput exercises the exported package-level
// With through a TTYHandler, which is the path the reported defect was
// on: the handler is selected by TTY-ness, so on a terminal With's
// attributes were silently dropped while the same code printed them
// correctly in CI.
//
// This is an in-package test so it can point the package logger at a
// buffer. Building an slog.Logger over a TTYHandler by hand would test
// slog, not this package's With, and there is no injectable sink to
// reach it from outside. The package logger is process-global, so this
// test must not run in parallel.
//
//nolint:paralleltest // replaces the process-global package logger
func TestWithAttributesReachTTYOutput(t *testing.T) {
var buf bytes.Buffer
previous := logger
t.Cleanup(func() { logger = previous })
logger = slog.New(NewTTYHandler(&buf, &slog.HandlerOptions{
Level: slog.LevelDebug,
}))
With("key", "value").Info("hello")
plain := withTestANSIEscape.ReplaceAllString(buf.String(), "")
require.NotEmpty(t, plain)
assert.Contains(t, plain, "hello")
assert.Contains(t, plain, "key=value",
"log.With attributes must reach TTYHandler output")
}
// TestWithoutInitializedLoggerFallsBack pins the documented behavior of
// With before Initialize has run: it hands back the slog default rather
// than a nil logger that would panic at the call site.
//
//nolint:paralleltest // replaces the process-global package logger
func TestWithoutInitializedLoggerFallsBack(t *testing.T) {
previous := logger
t.Cleanup(func() { logger = previous })
logger = nil
assert.NotNil(t, With("key", "value"))
}

View File

@@ -79,7 +79,7 @@ func (v *Vaultik) Prune(opts *PruneOptions) error {
// store is treated as gone. This used to be the separate 'snapshot // store is treated as gone. This used to be the separate 'snapshot
// cleanup' command and is now folded in so a single 'vaultik prune' // cleanup' command and is now folded in so a single 'vaultik prune'
// gets the local index fully back in sync with the destination. // gets the local index fully back in sync with the destination.
err = v.CleanupLocalSnapshots(opts) err = v.CleanupLocalSnapshots()
if err != nil { if err != nil {
return fmt.Errorf("reconciling local snapshots with remote: %w", err) return fmt.Errorf("reconciling local snapshots with remote: %w", err)
} }

View File

@@ -1,134 +0,0 @@
package vaultik_test
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// cleanupStaleID is a local snapshot record with no remote manifest —
// the record CleanupLocalSnapshots exists to remove.
const cleanupStaleID = "testhost_home_2026-04-01T09:00:00Z"
// remainingSnapshotLimit bounds the post-cleanup listing. ListRecent
// takes a SQL LIMIT, so it must be positive; the fixtures never exceed
// a handful of rows.
const remainingSnapshotLimit = 100
// cleanupStart is the fixture snapshot's start time. Its exact value is
// irrelevant; only presence in the index matters here.
//
//nolint:gochecknoglobals // read-only fixture shared by the tests below
var cleanupStart = time.Date(2026, 4, 1, 9, 0, 0, 0, time.UTC)
// TestCleanupLocalSnapshots_JSONWritesNothingToStdout is the regression
// guard for issue #108: `vaultik prune --json | jq` failed because this
// function wrote prose to stdout on every branch, ahead of the
// PruneBlobsResult document, with no --json awareness at all.
//
// Both branches are covered because the three writes between them left
// no input that avoided the contamination: with stale records there was
// a line per record plus a summary, and with none there was still the
// "No stale local snapshots found." line.
func TestCleanupLocalSnapshots_JSONWritesNothingToStdout(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for name, seed := range map[string]func(*listEnv){
"no stale records": func(env *listEnv) {
// A snapshot present both locally and remotely: nothing to
// remove, which used to print the "No stale" line.
env.addLocal(t, listLocalID, cleanupStart)
env.addRemote(t, listLocalID, cleanupStart)
},
"stale records present": func(env *listEnv) {
env.addLocal(t, cleanupStaleID, cleanupStart)
},
"nothing at all": func(_ *listEnv) {},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
env := newListEnv(t)
seed(env)
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{JSON: true})
require.NoError(t, err)
assert.Empty(t, env.stdout.String(),
"stdout carries the --json document and nothing else")
})
}
}
// TestCleanupLocalSnapshots_HumanOutputRetained pins the other half of
// the contract. Without it the test above would be satisfied by
// deleting the three lines outright, and a `vaultik prune` with no
// flags must still say that it removed records from the local index —
// that is the deletion of local state, not decoration.
func TestCleanupLocalSnapshots_HumanOutputRetained(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
t.Run("stale records present", func(t *testing.T) {
t.Parallel()
env := newListEnv(t)
env.addLocal(t, cleanupStaleID, cleanupStart)
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{})
require.NoError(t, err)
out := env.stdout.String()
assert.Contains(t, out, "Removing stale local record: "+cleanupStaleID)
assert.Contains(t, out, "Removed 1 stale local snapshot record(s).")
})
t.Run("no stale records", func(t *testing.T) {
t.Parallel()
env := newListEnv(t)
env.addLocal(t, listLocalID, cleanupStart)
env.addRemote(t, listLocalID, cleanupStart)
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{})
require.NoError(t, err)
assert.Contains(t, env.stdout.String(),
"No stale local snapshots found.")
})
}
// TestCleanupLocalSnapshots_RemovesOnlyStaleRecords checks that the
// --json gate did not change what the function does, only what it
// says: the stale record is gone from the index and the one with a
// remote manifest is untouched.
func TestCleanupLocalSnapshots_RemovesOnlyStaleRecords(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
env.addLocal(t, listLocalID, cleanupStart)
env.addRemote(t, listLocalID, cleanupStart)
env.addLocal(t, cleanupStaleID, cleanupStart)
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{JSON: true})
require.NoError(t, err)
remaining, err := env.v.Repositories.Snapshots.ListRecent(
env.v.Context(), remainingSnapshotLimit)
require.NoError(t, err)
ids := make([]string, 0, len(remaining))
for _, snap := range remaining {
ids = append(ids, snap.ID.String())
}
assert.Equal(t, []string{listLocalID}, ids,
"only the record with no remote manifest may be removed")
}

View File

@@ -829,15 +829,7 @@ func (v *Vaultik) outputVerifyJSON(result *VerifyResult) error {
// behind by incomplete or interrupted backups. Each local snapshot's // behind by incomplete or interrupted backups. Each local snapshot's
// human ID is hashed via RemoteSnapshotKey and compared against the // human ID is hashed via RemoteSnapshotKey and compared against the
// remote listing. // remote listing.
// func (v *Vaultik) CleanupLocalSnapshots() error {
// It takes the whole *PruneOptions, symmetric with PruneBlobs, because
// it is the other half of one command: Prune runs this phase and then
// that one. Only JSON is read here. Under --json every write below is
// suppressed, because stdout carries the PruneBlobsResult document and
// nothing else — prose ahead of it is what made `vaultik prune --json |
// jq` fail (issue #108). The narration is duplicated as log records,
// which go to stderr and so cannot corrupt the document.
func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
err := v.EnsureStorageBinding() err := v.EnsureStorageBinding()
if err != nil { if err != nil {
return err return err
@@ -863,11 +855,7 @@ func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
for _, snap := range localSnapshots { for _, snap := range localSnapshots {
id := snap.ID.String() id := snap.ID.String()
if !remoteSet[snapshot.RemoteSnapshotKey(id)] { if !remoteSet[snapshot.RemoteSnapshotKey(id)] {
log.Info("Removing stale local snapshot record", "snapshot_id", id) v.stdoutf("Removing stale local record: %s\n", id)
if !opts.JSON {
v.stdoutf("Removing stale local record: %s\n", id)
}
err = v.deleteSnapshotFromLocalDB(id) err = v.deleteSnapshotFromLocalDB(id)
if err != nil { if err != nil {
@@ -881,13 +869,6 @@ func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
} }
} }
log.Info("Reconciled local snapshot records against remote metadata",
"removed", removed, "examined", len(localSnapshots))
if opts.JSON {
return nil
}
if removed == 0 { if removed == 0 {
v.printlnStdout("No stale local snapshots found.") v.printlnStdout("No stale local snapshots found.")
} else { } else {

View File

@@ -87,7 +87,7 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error {
snapshots = append(snapshots, info) snapshots = append(snapshots, info)
} }
listing, remoteErr := v.collectRemoteSnapshots(localKeys) listing, remoteErr := v.collectRemoteSnapshots(localKeys, jsonOutput)
if remoteErr != nil { if remoteErr != nil {
v.warnRemoteListingFailed(remoteErr, jsonOutput) v.warnRemoteListingFailed(remoteErr, jsonOutput)
} else { } else {
@@ -131,23 +131,23 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error {
// still worth printing, and `snapshot list` exiting non-zero because a // still worth printing, and `snapshot list` exiting non-zero because a
// volume is unmounted would be worse than useless. // volume is unmounted would be worse than useless.
// //
// The two output modes report it through different channels. Table mode // In --json mode the warning goes to stderr rather than through the
// uses the UI writer, whose prose and color match the table it sits // logger or the UI writer, both of which emit on stdout — the JSON
// under. The UI writer emits on stdout, though, so --json mode uses the // document has to be the only thing on stdout for `snapshot list --json
// logger instead: stdout has to hold nothing but the JSON document for // | jq` to work. The failure is also representable in the document
// `snapshot list --json | jq` to work. Both channels are chosen once, // itself: every row's remote_present is null when the destination could
// never both, so the user is not told the same thing twice. // not be listed.
//
// The failure is also representable in the document itself: every row's
// remote_present is null when the destination could not be listed.
func (v *Vaultik) warnRemoteListingFailed(err error, jsonOutput bool) { func (v *Vaultik) warnRemoteListingFailed(err error, jsonOutput bool) {
if jsonOutput { if jsonOutput {
log.Warn("Could not list backup destination store; "+ _, _ = fmt.Fprintf(v.Stderr,
"showing snapshots from the local index only", "error", err) "Warning: could not list backup destination store: %v. "+
"Showing snapshots from the local index only.\n", err)
return return
} }
// Once only: the logger also writes to stdout, so emitting through
// both it and the UI would print the same sentence to the user twice.
v.UI.Warningf("Could not list backup destination store: %v.", err) v.UI.Warningf("Could not list backup destination store: %v.", err)
v.UI.Infof("Showing snapshots from the local index only.") v.UI.Infof("Showing snapshots from the local index only.")
} }
@@ -156,29 +156,69 @@ func (v *Vaultik) warnRemoteListingFailed(err error, jsonOutput bool) {
// is about to read is incomplete: manifests that could not be read, and // is about to read is incomplete: manifests that could not be read, and
// remote-only snapshots dropped by the maxRemoteOnlyRows cap. // remote-only snapshots dropped by the maxRemoteOnlyRows cap.
// //
// Table mode reports both below the table (see reportListDrift) through // Table mode reports both below the table (see reportListDrift). In
// the UI writer, which emits on stdout. In --json mode stdout has to // --json mode they cannot go on stdout — the document has to be the
// hold nothing but the document for `snapshot list --json | jq` to // only thing there for `snapshot list --json | jq` to work — and the
// work, and the document's shape is deliberately left alone so existing // document's shape is deliberately left alone so existing consumers
// consumers keep parsing — so these go to the logger, which writes to // keep parsing. So they go to stderr, the same place the
// stderr. A consumer that must react to truncation can treat any output // unreachable-destination warning already goes. A consumer that must
// on that stream as "this listing is not the whole picture"; silent // react to truncation can treat any output on this stream as "this
// truncation of a listing whose whole purpose is disaster recovery is // listing is not the whole picture"; silent truncation of a listing
// the worse failure. // whose whole purpose is disaster recovery is the worse failure.
func (v *Vaultik) reportJSONListingLimits(listing *remoteSnapshotListing) { func (v *Vaultik) reportJSONListingLimits(listing *remoteSnapshotListing) {
if listing.unreadable > 0 { if listing.unreadable > 0 {
log.Warn("Some remote snapshot(s) could not be described: "+ _, _ = fmt.Fprintf(v.Stderr,
"manifest missing or unreadable; they are missing from "+ "Warning: %d remote snapshot(s) could not be described: "+
"this listing", "unreadable", listing.unreadable) "manifest missing or unreadable. They are missing from "+
"this listing.\n", listing.unreadable)
} }
if listing.omitted > 0 { if listing.omitted > 0 {
log.Warn("Listing truncated: further remote-only snapshot(s) "+ _, _ = fmt.Fprintf(v.Stderr,
"not shown", "omitted", listing.omitted, "Warning: listing truncated: %d further remote-only "+
"limit", maxRemoteOnlyRows) "snapshot(s) not shown (limit %d per listing).\n",
listing.omitted, maxRemoteOnlyRows)
} }
} }
// kvPairSize is the number of variadic arguments that make up one
// structured logging key/value pair.
const kvPairSize = 2
// warnWhileListing reports a per-snapshot problem found while
// describing the destination store, through a writer that is safe for
// the current output mode.
//
// In --json mode it writes to v.Stderr rather than calling log.Warn,
// for the same reason warnRemoteListingFailed does: internal/log builds
// its logger over os.Stdout and defaults to level Warn, so one warning
// there would put a log line on stdout ahead of the JSON document and
// break `snapshot list --json | jq`. A single corrupt manifest is
// precisely the degradation this listing is built to survive, so it
// must not be the thing that corrupts the output.
//
// This is a local workaround. Remove it, and the branch in
// warnRemoteListingFailed, once issue #82 makes the logger's sink
// configurable.
func (v *Vaultik) warnWhileListing(jsonOutput bool, msg string, args ...any) {
if !jsonOutput {
log.Warn(msg, args...)
return
}
var line strings.Builder
_, _ = fmt.Fprintf(&line, "Warning: %s", msg)
for i := 0; i+kvPairSize <= len(args); i += kvPairSize {
pair := args[i : i+kvPairSize]
_, _ = fmt.Fprintf(&line, " %v=%v", pair[0], pair[1])
}
_, _ = fmt.Fprintln(v.Stderr, line.String())
}
// remoteSnapshotListing is the result of one pass over the destination // remoteSnapshotListing is the result of one pass over the destination
// store's metadata/ prefix. // store's metadata/ prefix.
type remoteSnapshotListing struct { type remoteSnapshotListing struct {
@@ -207,8 +247,11 @@ type remoteSnapshotListing struct {
// Manifest reads scale only with the number of snapshots the local // Manifest reads scale only with the number of snapshots the local
// index does not already know about, and are capped at // index does not already know about, and are capped at
// maxRemoteOnlyRows. // maxRemoteOnlyRows.
//
// jsonOutput only selects where per-snapshot warnings are written; see
// warnWhileListing.
func (v *Vaultik) collectRemoteSnapshots( func (v *Vaultik) collectRemoteSnapshots(
localKeys map[string]bool, localKeys map[string]bool, jsonOutput bool,
) (*remoteSnapshotListing, error) { ) (*remoteSnapshotListing, error) {
keys, err := v.listAllRemoteSnapshotKeys() keys, err := v.listAllRemoteSnapshotKeys()
if err != nil { if err != nil {
@@ -239,22 +282,17 @@ func (v *Vaultik) collectRemoteSnapshots(
} }
listing.remoteOnly, listing.unreadable = v.describeRemoteOnlySnapshots( listing.remoteOnly, listing.unreadable = v.describeRemoteOnlySnapshots(
unknown) unknown, jsonOutput)
return listing, nil return listing, nil
} }
// listingWarning is a problem found with one remote snapshot, recorded // listingWarning is a problem found with one remote snapshot, recorded
// rather than emitted on the spot. Manifest reads run concurrently, so // rather than emitted on the spot. Manifest reads run concurrently and
// emitting from the worker that found the problem would order the // the writer chosen by warnWhileListing is not guaranteed to be safe
// warnings by fetch completion — which varies run to run with network // for concurrent use, so warnings are held until every read has
// timing and tells the reader nothing. Holding them and emitting in key // finished and then emitted in key order from a single goroutine. That
// order from a single goroutine after every read has finished makes two // also makes the warning order deterministic run to run.
// runs over the same damaged store produce the same diagnostics in the
// same order.
//
// Concurrency safety is no longer part of the reason: these are emitted
// through log.Warn, and slog handlers are safe for concurrent use.
type listingWarning struct { type listingWarning struct {
msg string msg string
args []any args []any
@@ -268,7 +306,7 @@ type listingWarning struct {
// failing the listing: one bad snapshot directory must not hide every // failing the listing: one bad snapshot directory must not hide every
// other snapshot the user has. // other snapshot the user has.
func (v *Vaultik) describeRemoteOnlySnapshots( func (v *Vaultik) describeRemoteOnlySnapshots(
keys []string, keys []string, jsonOutput bool,
) ([]SnapshotInfo, int) { ) ([]SnapshotInfo, int) {
found := make([]SnapshotInfo, len(keys)) found := make([]SnapshotInfo, len(keys))
ok := make([]bool, len(keys)) ok := make([]bool, len(keys))
@@ -311,7 +349,7 @@ func (v *Vaultik) describeRemoteOnlySnapshots(
for i := range keys { for i := range keys {
if warnings[i] != nil { if warnings[i] != nil {
log.Warn(warnings[i].msg, warnings[i].args...) v.warnWhileListing(jsonOutput, warnings[i].msg, warnings[i].args...)
} }
if !ok[i] { if !ok[i] {

View File

@@ -108,6 +108,7 @@ type listEnv struct {
v *vaultik.Vaultik v *vaultik.Vaultik
store *observingStorer store *observingStorer
stdout *bytes.Buffer stdout *bytes.Buffer
stderr *bytes.Buffer
} }
func newListEnv(t *testing.T) *listEnv { func newListEnv(t *testing.T) *listEnv {
@@ -121,6 +122,7 @@ func newListEnv(t *testing.T) *listEnv {
store := newObservingStorer() store := newObservingStorer()
stdout := &bytes.Buffer{} stdout := &bytes.Buffer{}
stderr := &bytes.Buffer{}
v := &vaultik.Vaultik{ v := &vaultik.Vaultik{
Config: &config.Config{ Config: &config.Config{
@@ -133,13 +135,13 @@ func newListEnv(t *testing.T) *listEnv {
Repositories: database.NewRepositories(db), Repositories: database.NewRepositories(db),
DB: db, DB: db,
Stdout: stdout, Stdout: stdout,
Stderr: &bytes.Buffer{}, Stderr: stderr,
Stdin: &bytes.Buffer{}, Stdin: &bytes.Buffer{},
UI: ui.NewWithColor(stdout, false), UI: ui.NewWithColor(stdout, false),
} }
v.SetContext(ctx) v.SetContext(ctx)
return &listEnv{v: v, store: store, stdout: stdout} return &listEnv{v: v, store: store, stdout: stdout, stderr: stderr}
} }
// addLocal inserts a completed snapshot into the local index. // addLocal inserts a completed snapshot into the local index.
@@ -519,16 +521,16 @@ func TestListSnapshots_JSONMergedView(t *testing.T) {
// TestListSnapshots_JSONUnreachableRemote checks that a failed listing // TestListSnapshots_JSONUnreachableRemote checks that a failed listing
// does not corrupt the JSON document with warning text, and that // does not corrupt the JSON document with warning text, and that
// "unknown" is reported as null rather than as absence. // "unknown" is reported as null rather than as absence.
//
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
func TestListSnapshots_JSONUnreachableRemote(t *testing.T) { func TestListSnapshots_JSONUnreachableRemote(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t) env := newListEnv(t)
env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC)) env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC))
env.store.listErr = errRemoteUnreachable env.store.listErr = errRemoteUnreachable
stderr := captureProcessStderr(t, func() { err := env.v.ListSnapshots(true)
require.NoError(t, env.v.ListSnapshots(true)) require.NoError(t, err)
})
// stdout must be nothing but the JSON document, so the warning has // stdout must be nothing but the JSON document, so the warning has
// to go to stderr. // to go to stderr.
@@ -540,8 +542,9 @@ func TestListSnapshots_JSONUnreachableRemote(t *testing.T) {
assert.Nil(t, rows[0].RemotePresent, assert.Nil(t, rows[0].RemotePresent,
"remote state is unknown when the destination cannot be listed") "remote state is unknown when the destination cannot be listed")
assert.Contains(t, stderr, "Could not list backup destination store") assert.Contains(t, env.stderr.String(),
assert.Contains(t, stderr, "permission denied") "could not list backup destination store")
assert.Contains(t, env.stderr.String(), "permission denied")
} }
// useNonUTCLocalZone points time.Local at a fixed non-UTC zone for the // useNonUTCLocalZone points time.Local at a fixed non-UTC zone for the
@@ -632,9 +635,10 @@ func TestListSnapshots_TimestampsAreUTCOnNonUTCHost(t *testing.T) {
// machine consumer would otherwise see no difference between "that // machine consumer would otherwise see no difference between "that
// snapshot is not on the destination" and "that snapshot could not be // snapshot is not on the destination" and "that snapshot could not be
// read". // read".
//
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
func TestListSnapshots_JSONReportsUnreadableManifests(t *testing.T) { func TestListSnapshots_JSONReportsUnreadableManifests(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t) env := newListEnv(t)
goodKey := env.addRemote(t, listRemoteID, goodKey := env.addRemote(t, listRemoteID,
@@ -646,18 +650,16 @@ func TestListSnapshots_JSONReportsUnreadableManifests(t *testing.T) {
strings.NewReader("this is not a zstd stream")) strings.NewReader("this is not a zstd stream"))
require.NoError(t, err) require.NoError(t, err)
stderr := captureProcessStderr(t, func() { err = env.v.ListSnapshots(true)
require.NoError(t, env.v.ListSnapshots(true)) require.NoError(t, err)
})
rows := decodeListJSON(t, env.stdout.String()) rows := decodeListJSON(t, env.stdout.String())
require.Len(t, rows, 1) require.Len(t, rows, 1)
assert.Equal(t, goodKey, rows[0].RemoteKey) assert.Equal(t, goodKey, rows[0].RemoteKey)
assert.Contains(t, stderr, "could not be described", assert.Contains(t, env.stderr.String(),
"1 remote snapshot(s) could not be described",
"a row dropped from the JSON document must be announced somewhere") "a row dropped from the JSON document must be announced somewhere")
assert.Contains(t, stderr, `"unreadable":1`,
"the count of dropped rows must be reported, not just the fact")
} }
// maxRemoteOnlyRowsForTest mirrors the maxRemoteOnlyRows cap in the // maxRemoteOnlyRowsForTest mirrors the maxRemoteOnlyRows cap in the
@@ -669,9 +671,10 @@ const maxRemoteOnlyRowsForTest = 1000
// truncation of a listing whose whole purpose is disaster recovery is // truncation of a listing whose whole purpose is disaster recovery is
// the wrong failure mode: the consumer least able to notice is exactly // the wrong failure mode: the consumer least able to notice is exactly
// the one reading JSON. // the one reading JSON.
//
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
func TestListSnapshots_JSONReportsTruncation(t *testing.T) { func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t) env := newListEnv(t)
timestamp := time.Date(2026, 3, 2, 11, 22, 33, 0, time.UTC) timestamp := time.Date(2026, 3, 2, 11, 22, 33, 0, time.UTC)
@@ -681,30 +684,25 @@ func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
env.addRemote(t, fmt.Sprintf("otherhost_bulk_%04d", i), timestamp) env.addRemote(t, fmt.Sprintf("otherhost_bulk_%04d", i), timestamp)
} }
stderr := captureProcessStderr(t, func() { err := env.v.ListSnapshots(true)
require.NoError(t, env.v.ListSnapshots(true)) require.NoError(t, err)
})
rows := decodeListJSON(t, env.stdout.String()) rows := decodeListJSON(t, env.stdout.String())
assert.Len(t, rows, maxRemoteOnlyRowsForTest) assert.Len(t, rows, maxRemoteOnlyRowsForTest)
assert.Contains(t, stderr, "Listing truncated") assert.Contains(t, env.stderr.String(), "listing truncated")
assert.Contains(t, stderr, `"omitted":1`) assert.Contains(t, env.stderr.String(), "1 further remote-only")
assert.Contains(t, stderr,
fmt.Sprintf(`"limit":%d`, maxRemoteOnlyRowsForTest))
} }
// captureProcessStdout redirects the process's own stdout to a pipe, // captureProcessStdout redirects the process's own stdout to a pipe,
// rebuilds the global logger, runs fn, and returns everything written to // rebuilds the global logger over it, runs fn, and returns everything
// the pipe. // written.
// //
// The logger is rebuilt on purpose even though it is supposed to write // internal/log builds its logger over os.Stdout at construction time and
// to stderr: that is exactly what makes this a regression guard. If the // offers no injectable sink (issue #82), so a warning logged during a
// logger ever goes back to os.Stdout, Initialize picks up the pipe and // --json listing lands on the process's real stdout, not on any writer a
// the log record shows up in the capture, breaking the JSON parse here // test can inject. Capturing the file descriptor is therefore the only
// the same way it would break `snapshot list --json | jq` in the field. // way a test can see what `snapshot list --json | jq` would see.
// Without the rebuild, a regressed logger would write to the real stdout
// the test process was started with and go unnoticed.
// //
// Not parallel-safe: os.Stdout and the logger are process-global. // Not parallel-safe: os.Stdout and the logger are process-global.
func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string { func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
@@ -716,6 +714,8 @@ func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
previous := os.Stdout previous := os.Stdout
os.Stdout = writer os.Stdout = writer
// Rebuild the logger so it writes to the pipe rather than to the
// real stdout the test process was started with.
log.Initialize(log.Config{}) log.Initialize(log.Config{})
drained := make(chan string, 1) drained := make(chan string, 1)
@@ -738,57 +738,7 @@ func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
require.NoError(t, reader.Close()) require.NoError(t, reader.Close())
// Put the logger back on the restored streams. // Put the logger back on the restored stdout.
log.Initialize(log.Config{})
return captured
}
// captureProcessStderr redirects the process's own stderr to a pipe,
// rebuilds the global logger over it, runs fn, and returns everything
// written.
//
// internal/log writes every diagnostic to os.Stderr and captures that
// file at Initialize time, so a warning logged during a listing lands on
// the process's real stderr, not on any writer a test can inject.
// Capturing the file descriptor is therefore the only way a test can see
// what the operator would see. The captured stream is a pipe rather than
// a terminal, so the records are JSON — the same form a redirected
// stderr gets in production.
//
// Not parallel-safe: os.Stderr and the logger are process-global.
func captureProcessStderr(t *testing.T, fn func()) string {
t.Helper()
reader, writer, err := os.Pipe()
require.NoError(t, err)
previous := os.Stderr
os.Stderr = writer
log.Initialize(log.Config{})
drained := make(chan string, 1)
go func() {
var buf bytes.Buffer
_, _ = io.Copy(&buf, reader)
drained <- buf.String()
}()
fn()
os.Stderr = previous
require.NoError(t, writer.Close())
captured := <-drained
require.NoError(t, reader.Close())
// Put the logger back on the restored streams.
log.Initialize(log.Config{}) log.Initialize(log.Config{})
return captured return captured
@@ -797,18 +747,13 @@ func captureProcessStderr(t *testing.T, fn func()) string {
// TestListSnapshots_JSONStdoutIsOnlyTheDocument is the regression guard // TestListSnapshots_JSONStdoutIsOnlyTheDocument is the regression guard
// for `snapshot list --json | jq` surviving a damaged destination store. // for `snapshot list --json | jq` surviving a damaged destination store.
// //
// Every stdout writer the command has — the JSON encoder and the UI // Every stdout writer the command has — the JSON encoder, the UI, and
// is pointed at one pipe here, exactly as they are pointed at one file // the global logger — is pointed at one pipe here, exactly as they are
// descriptor in production, and the logger is rebuilt over that same // pointed at one file descriptor in production. A single log line about
// pipe's process-level stdout so that a logger which regressed back to // a corrupt manifest ahead of the array is enough to break the parse,
// stdout would land in the capture. A single log line about a corrupt // and that is what this asserts cannot happen.
// manifest ahead of the array is enough to break the parse, and that is
// what this asserts cannot happen.
// //
// The two warnings are asserted on the separately captured stderr: they //nolint:paralleltest // replaces os.Stdout and the global logger
// must be emitted, just not there.
//
//nolint:paralleltest // replaces os.Stdout, os.Stderr and the logger
func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) { func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
env := newListEnv(t) env := newListEnv(t)
@@ -827,15 +772,11 @@ func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
oddKey := env.addRemoteRawTimestamp(t, oddKey := env.addRemoteRawTimestamp(t,
"testhost_odd_2026-03-04T00:00:00Z", "the day before yesterday") "testhost_odd_2026-03-04T00:00:00Z", "the day before yesterday")
var captured string captured := captureProcessStdout(t, func(stdout io.Writer) {
env.v.Stdout = stdout
env.v.UI = ui.NewWithColor(stdout, false)
stderr := captureProcessStderr(t, func() { require.NoError(t, env.v.ListSnapshots(true))
captured = captureProcessStdout(t, func(stdout io.Writer) {
env.v.Stdout = stdout
env.v.UI = ui.NewWithColor(stdout, false)
require.NoError(t, env.v.ListSnapshots(true))
})
}) })
rows := decodeListJSON(t, captured) rows := decodeListJSON(t, captured)
@@ -853,8 +794,8 @@ func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
// Both warnings were emitted, on the stream that cannot corrupt the // Both warnings were emitted, on the stream that cannot corrupt the
// document. // document.
stderr := env.stderr.String()
assert.Contains(t, stderr, "Could not describe remote snapshot") assert.Contains(t, stderr, "Could not describe remote snapshot")
assert.Contains(t, stderr, "Remote manifest has an unparseable timestamp") assert.Contains(t, stderr, "Remote manifest has an unparseable timestamp")
assert.Contains(t, stderr, "could not be described") assert.Contains(t, stderr, "1 remote snapshot(s) could not be described")
assert.Contains(t, stderr, `"unreadable":1`)
} }

View File

@@ -43,15 +43,7 @@ type Vaultik struct {
ctx context.Context //nolint:containedctx // ctx bound at construction by design ctx context.Context //nolint:containedctx // ctx bound at construction by design
cancel context.CancelFunc cancel context.CancelFunc
// IO. Stdout carries the output the user asked for and nothing else, // IO
// so that `--json | jq` works. Stderr completes the standard triple
// for anything a command needs to write there directly; diagnostics
// are not that — they go through internal/log, which writes to the
// process's stderr. No production code writes to Stderr today, so
// searching for its writers turns up nothing; it is kept as the
// injection point a direct stderr write would otherwise have to
// invent, and removing it would make the triple asymmetric for no
// gain.
Stdout io.Writer Stdout io.Writer
Stderr io.Writer Stderr io.Writer
Stdin io.Reader Stdin io.Reader

View File

@@ -48,12 +48,11 @@ missing() {
! command -v "$1" >/dev/null 2>&1 ! command -v "$1" >/dev/null 2>&1
} }
# Docker is a hard requirement, not a nice-to-have: script/lint lints by # Docker is a hard requirement, not a nice-to-have: script/lint runs the
# building Dockerfile.lint, whose digest-pinned golangci-lint image is # digest-pinned golangci-lint image from the Dockerfile's lint stage, and
# the only place the linter runs, and script/check and script/precommit # script/check and script/precommit both run script/lint. A bootstrap
# both run script/lint. A bootstrap that prints "bootstrap complete" on a # that prints "bootstrap complete" on a machine where `make check` cannot
# machine where `make check` cannot run is a false success, so this fails # run is a false success, so this fails instead.
# instead.
# #
# Installing docker from here was considered and rejected: it needs root, # Installing docker from here was considered and rejected: it needs root,
# a running daemon, and on macOS a GUI cask, so an attempt would itself # a running daemon, and on macOS a GUI cask, so an attempt would itself
@@ -80,15 +79,13 @@ bootstrap: FAILED - $reason.
Docker is required to develop this repo. Without it these do not work: Docker is required to develop this repo. Without it these do not work:
script/lint builds Dockerfile.lint, which runs the linter as a script/lint runs the digest-pinned golangci-lint image declared
build step in a digest-pinned golangci-lint image. by the Dockerfile's lint stage, which is the single
That FROM line is the single source of truth for the source of truth for the linter version
linter version
script/check runs script/lint script/check runs script/lint
script/precommit runs script/check, so commits are blocked by the script/precommit runs script/check, so commits are blocked by the
pre-commit hook installed by script/setup pre-commit hook installed by script/setup
script/cibuild builds Dockerfile.lint and Dockerfile, which is what script/cibuild builds the Dockerfile, which is what CI runs
CI runs
Install docker (and start the daemon, checking DOCKER_HOST and your Install docker (and start the daemon, checking DOCKER_HOST and your
group membership), then re-run script/bootstrap. golangci-lint on PATH group membership), then re-run script/bootstrap. golangci-lint on PATH
@@ -107,12 +104,12 @@ main() {
# Go toolchain # Go toolchain
if missing go; then pkg_install go golang go go; fi if missing go; then pkg_install go golang go go; fi
# golangci-lint is deliberately NOT installed: script/lint lints by # golangci-lint is deliberately NOT installed: script/lint runs the
# building Dockerfile.lint, whose digest-pinned image is the only # digest-pinned golangci-lint image from the Dockerfile's lint stage,
# place the linter runs, so whatever a package manager happens to # so whatever a package manager happens to ship would only be a
# ship would only be a shadow of the pinned version that could drift # shadow of the pinned version that could drift from CI. script/lint
# from CI. Nothing on the host is ever used as a linter, at any # will not use a PATH binary on a host at any version, so installing
# version, so installing one here would buy nothing. # one here would buy nothing.
# sqlite3 CLI: the test suite shells out to it (VACUUM). # sqlite3 CLI: the test suite shells out to it (VACUUM).
if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi

View File

@@ -1,31 +1,22 @@
#!/bin/sh #!/bin/sh
# script/cibuild: run the CI build. This is the full gate, and it is two # script/cibuild: run the CI build. The Dockerfile does not run
# builds, in this order: # script/check; it runs `make fmt-check` and `make lint` in its lint
# # stage and `make test` in its builder stage. A successful build
# Dockerfile.lint the linter, as a build step (a clean build IS a # implies those three passed, provided they actually ran -- which is
# clean lint) # what the CHECK_EPOCH below is for.
# Dockerfile `make fmt-check` and `make test` in the builder # Generic: needs no adaptation. The Gitea workflow runs this on push.
# stage, then the product image
#
# Either one failing fails this script. Note what follows from the
# split: script/docker builds only the product image and so no longer
# lints -- this script and script/check (which runs script/lint) are the
# things that decide whether the tree is clean.
#
# Generic apart from the two Dockerfiles: the Gitea workflow runs this
# on push.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# Both Dockerfiles key their check layers on CHECK_EPOCH, so a fresh # The Dockerfile's check layers are keyed on CHECK_EPOCH, so a
# value is what forces those layers 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. Each 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. Both # check RUNs, so dependency and module layers still cache. The
# Dockerfiles also refuse to build at all when CHECK_EPOCH is empty, # Dockerfile also refuses to build at all when CHECK_EPOCH is empty,
# so a missing value fails loudly here rather than passing quietly. # so a missing value fails loudly here rather than passing quietly.
# #
# The value must be unique per invocation, not per second. `date +%s` # The value must be unique per invocation, not per second. `date +%s`
@@ -44,18 +35,6 @@ 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.
#
# A separate value per build, because they are separate builds: one
# `date` shared between them would still be fresh, but reusing it
# invites the two to be collapsed into a single value that is
# computed somewhere else and passed in.
epoch="$(date +%s%N)$$"
# cacheonly for the lint build: its verdict is the exit status and
# the image is never run, so exporting it is pure cost. See
# script/lint.
docker build --output=type=cacheonly \
--build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .
epoch="$(date +%s%N)$$" epoch="$(date +%s%N)$$"
docker build --build-arg CHECK_EPOCH="$epoch" . docker build --build-arg CHECK_EPOCH="$epoch" .
} }

View File

@@ -2,13 +2,6 @@
# script/docker: build the Docker image tagged with the project name. # script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname. # Identical in all repos; the tag comes from script/projectname.
# Generic: needs no adaptation. # Generic: needs no adaptation.
#
# This builds the PRODUCT image only, and the product Dockerfile has no
# lint stage: linting lives in Dockerfile.lint and is run by
# script/lint. So a green here means `make fmt-check` and `make test`
# passed and the image built -- it says nothing about lint. The gates
# are script/check (which runs script/lint) and script/cibuild (which
# builds both files).
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"

View File

@@ -1,42 +1,110 @@
#!/bin/sh #!/bin/sh
# script/lint: run the linter. # script/lint: run the linter.
# #
# The linter runs inside the image built by Dockerfile.lint, and it runs # The linter always runs at the version pinned by the Dockerfile's lint
# there as a BUILD STEP: a successful build of that file IS a clean # stage, so a local run and a CI run of the same tree cannot disagree.
# lint. Nothing lints on the host, at any version, ever. That FROM line # That FROM line (image tag plus digest) is the single source of truth
# is the single source of truth for the linter version in this repo, so # for the linter version in this repo: bump it there and nothing else
# a local run and a CI run of the same tree cannot disagree. # needs editing.
# #
# One container per run means one lint cache and one golangci-lint lock # Normally that means running the pinned image with docker. The one
# per run, both private to that run and thrown away with it. That is # exception is running INSIDE that image: the Dockerfile's lint stage
# what makes concurrent runs on a shared host safe, and it is why this # runs `make lint`, and there is no docker daemon in there. That stage
# script no longer carries per-worktree cache directories, a lock-retry # sets VAULTIK_LINT_IN_CONTAINER=1, and only when that variable is set
# loop, or an output audit: there is no shared state left for them to # is a golangci-lint on PATH used directly - and then only if its
# defend (issue https://git.eeqj.de/sneak/vaultik/issues/113). # version is exactly the pin. Version equality alone is deliberately NOT
# enough: it also matches a developer's locally installed copy of the
# same version, which is a different build with a different Go
# toolchain, reached by a different code path, and it would bypass the
# digest pin this script exists to enforce. /.dockerenv was considered
# as the context signal and rejected: 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.
# #
# To watch the linter execute, set BUILDKIT_PROGRESS=plain, which docker # The linter's output is checked before it is believed: every run is
# honours directly: # audited by script/lint-audit for findings that cannot belong to this
# tree, and a run refused by golangci-lint's cross-process lock is
# retried rather than reported as a verdict. See the lock-retry loop in
# main and the header of script/lint-audit.
# #
# BUILDKIT_PROGRESS=plain script/lint # Extra arguments are passed through to `golangci-lint run`, before
# # `./...` (see script/lint-fix).
# The check layers -- `golangci-lint config verify` and then
# `golangci-lint run` -- must appear as executing rather than CACHED on
# every run; see the CHECK_EPOCH comment in Dockerfile.lint.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
DOCKERFILE="$ROOT/Dockerfile.lint" DOCKERFILE="$ROOT/Dockerfile"
# golangci-lint takes a cross-process lock and refuses to start while
# another instance holds it. That refusal is not a lint result, and
# exiting non-zero on it is indistinguishable to a caller from real
# findings - so it is retried rather than reported. Bounded, because a
# lock that is never released must fail rather than hang.
LOCK_MESSAGE="parallel golangci-lint is running"
LOCK_ATTEMPTS=6
LOCK_SLEEP=15
# The image reference of the Dockerfile's lint stage, tag and digest
# included, e.g.
# golangci/golangci-lint:v2.12.2-alpine@sha256:91b2...
lint_image() {
awk '$1 == "FROM" && $3 == "AS" && $4 == "lint" { print $2; exit }' \
"$DOCKERFILE"
}
# The bare version that image reference pins, e.g. 2.12.2
pinned_version() {
lint_image | sed -e 's/@.*//' -e 's/.*://' -e 's/^v//' -e 's/-.*//'
}
# The version of the golangci-lint on PATH, if any, e.g. 2.12.2
#
# `version --short` prints the bare version and is the interface meant
# for this (checked against 2.10.1 and 2.12.2). The banner scrape below
# it is a fallback for a release where --short is absent or silent; the
# banner's exact wording is not a stable interface, which is why it is
# no longer the primary parse.
installed_version() {
command -v golangci-lint >/dev/null 2>&1 || return 0
short="$(golangci-lint version --short 2>/dev/null |
tr -d '[:space:]' | sed -e 's/^v//')"
case "$short" in
*[0-9].[0-9]*.[0-9]*)
echo "$short"
return 0
;;
esac
golangci-lint version 2>/dev/null | awk '
{
for (i = 1; i <= NF; i++) {
if ($i ~ /^[0-9]+\.[0-9]+\.[0-9]+$/) {
print $i
exit
}
}
}'
}
# True inside the Dockerfile's lint stage, which sets this. Nothing else
# sets it: setting it by hand on a host is an explicit, visible decision
# to lint with an unpinned binary, not something reached by accident.
in_lint_container() {
[ "${VAULTIK_LINT_IN_CONTAINER:-}" = "1" ]
}
require_docker() { require_docker() {
image="$1"
if ! command -v docker >/dev/null 2>&1; then if ! command -v docker >/dev/null 2>&1; then
cat >&2 <<EOF cat >&2 <<EOF
lint: docker is required to run the pinned linter. lint: docker is required to run the pinned linter.
lint image declared by: $DOCKERFILE pinned image: $image
Install docker. Linting with any other golangci-lint is not supported: Install docker. Linting with any other golangci-lint is not supported:
it is what lets a local run pass while CI fails. A golangci-lint on it is what lets a local run pass while CI fails. An installed
PATH is never used, whatever its version. golangci-lint on PATH is not used, whatever its version; only the lint
stage of the Dockerfile itself runs the linter natively.
EOF EOF
exit 1 exit 1
fi fi
@@ -45,7 +113,7 @@ EOF
lint: the docker daemon is not reachable, so the pinned linter cannot lint: the docker daemon is not reachable, so the pinned linter cannot
run. run.
lint image declared by: $DOCKERFILE pinned image: $image
Start the daemon (and check DOCKER_HOST / your group membership). This Start the daemon (and check DOCKER_HOST / your group membership). This
script will not fall back to a different linter version or to an script will not fall back to a different linter version or to an
@@ -55,54 +123,213 @@ EOF
fi fi
} }
usage() { # Where the per-worktree caches live.
cat >&2 <<EOF cache_home() {
usage: $(basename "$0") echo "${XDG_CACHE_HOME:-${HOME:-/tmp}/.cache}/vaultik-lint"
}
script/lint takes no arguments. The linter runs as a build step, so # A short, stable digest of this worktree's path.
there is no command line to pass flags to; anything accepted here would path_digest() {
have to be silently dropped. To apply autofixes, use script/lint-fix, if command -v sha256sum >/dev/null 2>&1; then
which runs the same pinned image as a container for exactly this printf '%s' "$ROOT" | sha256sum | cut -c1-12
reason. elif command -v shasum >/dev/null 2>&1; then
EOF printf '%s' "$ROOT" | shasum -a 256 | cut -c1-12
exit 2 else
printf '%s' "$ROOT" | cksum | tr -cd '0-9' | cut -c1-12
fi
}
# Caches for the containerized linter, private to THIS worktree.
#
# Keeping them out of the repo and persisting them between runs is what
# keeps the inner loop fast: a warm run costs about the same as a native
# one plus container startup. Keeping them keyed on the worktree path is
# what keeps them correct. One shared cache for the whole repo was the
# defect in issue #99: two worktrees of this repo have identical file
# contents, so their cache keys collide, and golangci-lint replays the
# stored results - including the file paths recorded when they were
# produced. That silently reports one worktree's findings, or one
# worktree's clean bill of health, for another.
cache_dir() {
slug="$(printf '%s' "$(basename "$ROOT")" | tr -c 'A-Za-z0-9._-' '-')"
echo "$(cache_home)/$slug-$(path_digest)"
}
# One cache per worktree means throwaway worktrees would otherwise leave
# caches behind forever. Each cache records the worktree it belongs to,
# and any cache whose worktree no longer exists is collected here, so
# growth is bounded by the number of worktrees that actually exist. The
# whole tree also sits under XDG_CACHE_HOME (~/.cache by default), so it
# is disposable by definition: `rm -rf "${XDG_CACHE_HOME:-~/.cache}/vaultik-lint"`
# costs nothing but the next run's cold cache.
prune_dead_caches() {
home="$(cache_home)"
if [ ! -d "$home" ]; then
return 0
fi
for dir in "$home"/*; do
if [ ! -f "$dir/worktree" ]; then
continue
fi
owner="$(cat "$dir/worktree")"
if [ -z "$owner" ]; then
continue
fi
if [ ! -d "$owner" ]; then
# The Go module cache inside is deliberately read-only, and
# rm(1) cannot unlink a file out of a directory it may not
# write, so the tree has to be made writable first. And
# failing to tidy up is a housekeeping problem, never a
# reason to fail a lint: without the fallback below, `set
# -e` turns a stale cache that will not delete into a lint
# error, which is a gate failing for a reason that has
# nothing to do with the code. (Observed, not theorised.)
chmod -R u+w "$dir" 2>/dev/null || true
if ! rm -rf "$dir" 2>/dev/null; then
# Restore the marker on a partial removal: an
# unmarked leftover would be skipped by every future
# run and never collected.
mkdir -p "$dir" 2>/dev/null || true
echo "$owner" >"$dir/worktree" 2>/dev/null || true
echo "lint: could not remove stale cache $dir" >&2
fi
fi
done
}
prepare_cache() {
cache="$1"
mkdir -p "$cache/go-build" "$cache/go-mod" "$cache/golangci-lint"
echo "$ROOT" >"$cache/worktree"
}
# Run the linter, wherever it is that this script is allowed to run it.
run_linter() {
if in_lint_container; then
golangci-lint run "$@" ./...
return $?
fi
docker run --rm \
--user "$(id -u):$(id -g)" \
--env HOME=/tmp \
--env GOFLAGS=-buildvcs=false \
--env GOCACHE=/cache/go-build \
--env GOMODCACHE=/cache/go-mod \
--env GOLANGCI_LINT_CACHE=/cache/golangci-lint \
--volume "$ROOT:/src" \
--volume "$CACHE:/cache" \
--workdir /src \
"$IMAGE" \
golangci-lint run "$@" ./...
}
# Run the linter, streaming its combined output while also capturing it,
# and hand back its exit status. The output has to be inspected before
# it is believed, which is why this script no longer just execs the
# linter. `tee` would swallow the status, so it is smuggled out through
# a file: there is no pipefail in POSIX sh.
run_capture() {
capture="$1"
shift
rm -f "$capture.status"
{
rc=0
# `set -e` is in force inside this subshell too, so the status
# has to be caught here: an unguarded non-zero exit (which is
# what "the linter found something" looks like) would abort the
# subshell before the status was ever written.
run_linter "$@" 2>&1 || rc=$?
echo "$rc" >"$capture.status"
} | tee "$capture"
if [ ! -s "$capture.status" ]; then
echo "lint: the linter did not report an exit status" >&2
exit 1
fi
read -r captured_status <"$capture.status"
rm -f "$capture.status"
return "$captured_status"
}
# Reject output that cannot describe this tree. See script/lint-audit
# for what that means and why: in short, a finding citing a file that is
# not here means the result being reported was produced somewhere else,
# and a PASS built out of another checkout's analysis is silent (issue
# #99). The audit therefore runs on clean output as well.
audit_output() {
capture="$1"
if ! "$ROOT/script/lint-audit" "$capture"; then
if [ -n "$CACHE" ]; then
echo " this tree's lint cache: $CACHE" >&2
fi
exit 1
fi
} }
main() { main() {
[ "$#" -eq 0 ] || usage
cd "$ROOT" cd "$ROOT"
require_docker
# A fresh epoch per invocation is what forces the check layers to IMAGE="$(lint_image)"
# execute; the layers above the ARG in Dockerfile.lint still cache, if [ -z "$IMAGE" ]; then
# so a run is not cold. The value must be unique per invocation, not echo "lint: no lint stage found in $DOCKERFILE" >&2
# per second: `date +%s` is second-granular, so two concurrent exit 1
# invocations in the same second would get identical epochs and the fi
# later one could be served from cache -- the false green in
# miniature. `%N` alone does not fix it either, because 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 it on its own line rather than inline in the argument.
# Under `set -eu` a command substitution that fails inside an
# argument does NOT abort the script: CHECK_EPOCH would become an
# empty string, an empty string is a constant, and a constant epoch
# is exactly the cached-lint false green this guards against. As a
# bare assignment, `set -e` catches a failing `date` and no build
# starts.
epoch="$(date +%s%N)$$"
# cacheonly: the lint verdict is the build's exit status, and the CACHE=""
# image it would otherwise produce is never run. Exporting it costs if in_lint_container; then
# most of the wall time of a warm run and leaves a dangling image # No docker daemon in here, so there is no fallback: a mismatch
# behind on every invocation, on a host that may be running many. # is a hard error rather than a quiet substitution.
docker build \ installed="$(installed_version)"
--output=type=cacheonly \ pinned="$(pinned_version)"
--build-arg CHECK_EPOCH="$epoch" \ if [ -z "$installed" ] || [ "$installed" != "$pinned" ]; then
-f "$DOCKERFILE" \ cat >&2 <<EOF
"$ROOT" lint: VAULTIK_LINT_IN_CONTAINER is set, so this is expected to be
running inside the Dockerfile's pinned lint image, but the golangci-lint
on PATH does not match the pin.
pinned: $pinned ($IMAGE)
installed: ${installed:-<none>}
EOF
exit 1
fi
else
require_docker "$IMAGE"
prune_dead_caches
CACHE="$(cache_dir)"
prepare_cache "$CACHE"
fi
capture="$(mktemp "${TMPDIR:-/tmp}/vaultik-lint.XXXXXX")"
trap 'rm -f "$capture" "$capture.status"' EXIT HUP INT TERM
attempt=1
while :; do
status=0
run_capture "$capture" "$@" || status=$?
if grep -Fq "$LOCK_MESSAGE" "$capture"; then
if [ "$attempt" -lt "$LOCK_ATTEMPTS" ]; then
echo "lint: another golangci-lint holds the lock;" \
"retrying in ${LOCK_SLEEP}s" \
"(attempt $attempt of $LOCK_ATTEMPTS)" >&2
sleep "$LOCK_SLEEP"
attempt=$((attempt + 1))
continue
fi
cat >&2 <<EOF
lint: gave up after $LOCK_ATTEMPTS attempts, each blocked by another
golangci-lint holding the cross-process lock. This is NOT a lint
verdict: the tree was never analysed. Re-run when the other run has
finished.
EOF
exit 1
fi
audit_output "$capture"
exit "$status"
done
} }
main "$@" main "$@"

113
script/lint-audit Executable file
View File

@@ -0,0 +1,113 @@
#!/bin/sh
# script/lint-audit: audit a captured golangci-lint run for output that
# cannot describe this tree. Called by script/lint on every run; usable
# on its own against any saved lint output.
#
# script/lint-audit <capture-file>
#
# Exits 0 when every finding cites a file in this tree, 1 when any does
# not. It NEVER certifies that a lint run passed - it has no idea
# whether the run found issues, and does not look. It only rejects
# output that is impossible for this tree, which is a different and much
# weaker claim. Do not use it as a gate; use script/lint.
#
# Why this exists (issue #99): golangci-lint caches analysis results,
# and a cache shared between two checkouts of this repo can serve one
# checkout's stored findings for another, file paths included. The
# failure is symmetric and only one direction is loud - a clean tree
# failed by a dirty sibling gets investigated, while a dirty tree passed
# by a clean sibling is silent. This turns the silent direction into a
# hard error, which is why it runs on clean output too.
#
# The primary fix is that script/lint now keys its cache on the worktree
# path so the collision cannot happen. This is the backstop, because a
# backstop that only runs when we already believe things are fine is
# worth more than one more assumption.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Where script/lint bind-mounts the tree inside the pinned image. A
# containerized run that prints absolute paths (`--path-mode abs`)
# prints them under this, so they are this tree's files under another
# name. Note the consequence, and why the cache key rather than this
# check is the real fix: two containerized runs of different checkouts
# both call themselves /src, so contamination between two container
# runs is not distinguishable by path alone.
CONTAINER_ROOT="/src"
usage() {
echo "usage: $(basename "$0") <capture-file>" >&2
exit 2
}
# Every path cited by a finding that is not a file in this tree.
#
# The linter runs with the tree root as its working directory, so a
# legitimate finding cites either a relative path that resolves inside
# the tree or an absolute path under the root. A path that escapes
# (absolute and elsewhere, or with a `..` component) or that names a
# file which is not here describes something this run did not analyse.
foreign_paths() {
capture="$1"
awk -F: '$1 ~ /\.go$/ && $2 ~ /^[0-9]+$/ { print $1 }' "$capture" |
sort -u |
while IFS= read -r path; do
case "$path" in
"$ROOT"/*)
path="${path#"$ROOT"/}"
;;
"$CONTAINER_ROOT"/*)
path="${path#"$CONTAINER_ROOT"/}"
;;
/*)
printf '%s\n' "$path"
continue
;;
../* | */../*)
printf '%s\n' "$path"
continue
;;
esac
if [ ! -e "$ROOT/$path" ]; then
printf '%s\n' "$path"
fi
done
}
main() {
[ "$#" -eq 1 ] || usage
capture="$1"
if [ ! -f "$capture" ]; then
echo "lint-audit: no such capture file: $capture" >&2
exit 2
fi
foreign="$(foreign_paths "$capture")"
if [ -z "$foreign" ]; then
exit 0
fi
cat >&2 <<EOF
lint: REJECTED - the linter reported findings for files that are not in
this tree, so its output does not describe the tree that was linted.
This result is void, whichever way it went: a pass here would be a pass
earned by analysing someone else's code.
tree: $ROOT
Paths reported that are not in this tree:
EOF
printf '%s\n' "$foreign" | sed -e 's/^/ /' >&2
cat >&2 <<EOF
This is the signature of analysis replayed from a cache belonging to
another checkout (issue #99). Clear this tree's lint cache and re-run:
rm -rf "\${XDG_CACHE_HOME:-\$HOME/.cache}/vaultik-lint"
EOF
exit 1
}
main "$@"

View File

@@ -1,54 +1,18 @@
#!/bin/sh #!/bin/sh
# script/lint-fix: run the linter's autofixer. Rewrites files in place # script/lint-fix: run the linter's autofixer. Rewrites files in place
# for every finding the enabled linters know how to fix; findings # for every finding the enabled linters know how to fix; findings
# without an autofix are reported but left alone. # without an autofix are reported but left alone (exit status is
# nonzero while any remain).
# #
# THIS IS A DEVELOPER CONVENIENCE AND NEVER A GATE. Nothing in # Delegates to script/lint so the autofixer is the same pinned linter
# script/check, script/precommit or script/cibuild calls it, and no gate # version that script/lint and CI use - fixes written by a different
# reads its exit status. The gate is script/lint, which builds # version are not necessarily fixes for the version that gates.
# Dockerfile.lint; run that afterwards to find out whether the tree is
# actually clean.
#
# Unlike script/lint this cannot be a build step: a build step writes
# into an image, and fixes have to land in the worktree. So it runs the
# same pinned image as a container with the tree bind-mounted, which
# means it needs a LOCAL docker daemon -- a remote daemon has no access
# to these files, and this script will appear to do nothing there. The
# image reference is parsed out of Dockerfile.lint's FROM line, so the
# autofixer is always the same version as the linter that gates; fixes
# written by a different version are not necessarily fixes for the
# version that decides.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
DOCKERFILE="$ROOT/Dockerfile.lint"
# The image reference from Dockerfile.lint, tag and digest included.
lint_image() {
awk '$1 == "FROM" { print $2; exit }' "$DOCKERFILE"
}
main() { main() {
cd "$ROOT" exec "$SCRIPT_DIR/lint" --fix "$@"
image="$(lint_image)"
if [ -z "$image" ]; then
echo "lint-fix: no FROM line found in $DOCKERFILE" >&2
exit 1
fi
# Run as the invoking user so the rewritten files stay owned by
# them. HOME is set because the Go and golangci-lint caches default
# under it and that user has no home inside the container; those
# caches are per-container and discarded with it.
docker run --rm \
--user "$(id -u):$(id -g)" \
--env HOME=/tmp \
--env GOFLAGS=-buildvcs=false \
--volume "$ROOT:/src" \
--workdir /src \
"$image" \
golangci-lint run --config .golangci.yml --fix "$@" ./...
} }
main "$@" main "$@"