8 Commits
Author SHA1 Message Date
clawbot 6fcd8e1668 Stamp Docker image version from the host; flush profiles on error exit (closes #75)
check / check (push) Failing after 1s
check / check (pull_request) Failing after 1s
Docker images reported commit unknown because the build ran git inside the container while .dockerignore excludes .git, and VERSION was never overridden. script/docker and script/cibuild now compute version, commit and date on the host and pass them as build args; the Dockerfile runs no git and falls back to dev and unknown, never empty, on a bare docker build.

Profiling a failing command gave a truncated or missing profile: Entry and each command goroutine called os.Exit(1), skipping the deferred profile writers in main. Entry now returns a status that main exits with after its defers run, and command goroutines report failure through one RunOperation helper, which also restores PID-lock release and graceful shutdown on failure.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)

Co-authored-by: clawbot <clawbot@noreply.example.org>
2026-09-21 22:01:05 +02:00
clawbot aab6a87f8c Reconcile the schema/migration docs with the code (closes #68)
check / check (pull_request) Failing after 1s
check / check (push) Successful in 2m46s
Four documents told different stories about the database schema. docs/DATAMODEL.md now owns the explanation and separates two things: the policy, which is unchanged (no supported upgrade path between versions; delete the local index with vaultik database delete and back up again), and the schema bootstrap that does exist (numbered files in internal/database/schema applied to a fresh database and recorded in schema_migrations).

README.md and AGENTS.md are reworded to match and link there. AGENTS.md names the real file to edit, internal/database/schema/001.sql, and notes that the pre-1.0 disposability clause expires on tagging. No code changed.

Judgement call: REPO_POLICIES.md still names a different schema file; it is cross-project policy and was left alone.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
2026-09-21 21:58:37 +02:00
clawbot c355ef4d25 Report a prune count that could not be read as unknown, not 0 (closes #96)
check / check (pull_request) Failing after 1s
check / check (push) Successful in 2m46s
Prune read table row counts before and after to report how many orphaned files, chunks and blobs it removed, and discarded the error from every read. A failed query therefore reported as a count of 0, and the summary showed plausible wrong numbers.

A count that cannot be read is now logged as a warning (on stderr, also under --json) and shown as "unknown"; a difference computed from an unknown count is itself unknown. 0 still means the table was empty. No --json document carries these counts, so none can show a false 0.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
2026-09-21 21:41:57 +02:00
clawbot 5927e1aa3d Write file:// blobs atomically via temp file and rename (closes #130)
check / check (push) Failing after 0s
check / check (pull_request) Failing after 0s
The file:// backend streamed each object straight to its final key, so an upload cut off mid-stream left a truncated object there. The next backup saw that Stat succeeded, recorded the blob as complete, and produced a snapshot that reported success but could not be restored.

Writes now go to a temporary file with a .partial suffix in the destination directory, are synced, then renamed onto the key. List and ListStream skip .partial files, so a leftover is never trusted as a blob and is overwritten when the key is written again. S3 PutObject is already atomic.

Disclosure: the containing directory is not synced after the rename, so a host crash right after it could still lose the object on some filesystems.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
2026-09-21 21:24:42 +02:00
clawbot 9ca962969a Map s3 not-found to storage.ErrNotFound in Get and Stat (closes #129)
check / check (push) Failing after 1s
check / check (pull_request) Successful in 3m50s
The Storer interface documents that Get and Stat return storage.ErrNotFound for a missing object. The file and rclone backends did; the s3 backend returned the raw SDK error, so callers testing for ErrNotFound behaved differently on s3.

S3Storer.Get and Stat now wrap ErrNotFound when the SDK reports a missing object and leave every other error untouched. The SDK reports a missing key two ways (NoSuchKey from Get, NotFound from Head); both are recognised in one helper, s3.IsNotFound, which HeadObject now also uses. The mapping lives in the storage package because internal/s3 cannot import it.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
2026-09-21 21:07:35 +02:00
clawbot 89ebfc78e2 Use one duration parser and fix the --older-than months example (closes #123)
check / check (push) Failing after 1s
check / check (pull_request) Failing after 1s
Two parseDuration functions existed with different grammars; only the one in internal/vaultik/helpers.go was reachable from a flag. The unused copy in internal/cli/duration.go is deleted, so no flag accepts anything it did not before.

The README gave 6m as the six-months example for snapshot purge --older-than, but m is minutes: that command removed every snapshot older than six minutes. The example is now 6mo, and the help for --older-than and --keep-newer-than states that m is minutes and mo is months.

The parser now rejects negative durations, which it used to accept or silently make positive.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)

Co-authored-by: clawbot <clawbot@noreply.example.org>
2026-09-21 20:58:30 +02:00
clawbot 07ef3a1c78 Drop the lint-guard shell scanner, keep the Dockerfile.lint checks (closes #121)
check / check (push) Failing after 0s
check / check (pull_request) Successful in 2m37s
The guard test in cmd/vaultik/lintdocker_test.go tried to prove that no script runs the linter outside the container by parsing shell scripts with a hand-written scanner. Four reviews each found another spelling it missed; such a parser cannot be complete, and nobody could follow it in one reading.

The scanner, its helpers and their tests are deleted. The plain Dockerfile.lint assertions stay: the linter image is pinned by digest, config verify runs before run, and the per-run value reaches both steps. TODO.md no longer claims a test proves the property; script/lint is the only lint entry point, and keeping it so is a review matter.

Judgement call: this drops a guard two reviewers asked to harden.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (decision, merge)
2026-09-21 20:41:33 +02:00
clawbot c423d13191 Hash the plaintext, not the encrypted bytes, in verify --deep (closes #131)
check / check (push) Failing after 0s
check / check (pull_request) Failing after 0s
The last step of verify --deep hashed the encrypted bytes it downloaded once with SHA256 and compared the result to the blob name. The name is the double SHA256 of the blob plaintext, so the two could never match and deep verification failed on every healthy blob with "blob hash mismatch".

It now hashes the decompressed plaintext as chunk verification streams it and compares the double SHA256 of that to the blob name, the same derivation the writer uses. A new test backs up a real snapshot, runs deep verify on it, then flips one byte in a stored blob and expects failure.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
2026-09-21 20:24:37 +02:00
32 changed files with 949 additions and 1015 deletions
+8 -3
View File
@@ -104,7 +104,12 @@ Version: 2025-06-08
13. Pre-1.0: NEVER write database migrations. There are no live databases 13. Pre-1.0: NEVER write database migrations. There are no live databases
anywhere — every user's local index can be rebuilt from a fresh full anywhere — every user's local index can be rebuilt from a fresh full
backup. When the schema changes, just change `schema.sql` (and any code backup. To change the schema, edit `internal/database/schema/001.sql`
that touches the affected tables). The local index is disposable until (and any code that touches the affected tables) directly; do not add new
1.0 ships and is tagged. numbered schema files. Those numbered files and the `schema_migrations`
table they populate only bootstrap a fresh database — they are not an
upgrade path. The local index is disposable until 1.0 ships and is
tagged; once 1.0 is tagged that clause expires and the question of
upgrading existing indexes returns. See [`docs/DATAMODEL.md`](docs/DATAMODEL.md)
for the full explanation.
+24 -3
View File
@@ -20,8 +20,6 @@
# golang:1.26.1-alpine, 2026-03-17 # golang:1.26.1-alpine, 2026-03-17
FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder
ARG VERSION=dev
# Build tooling: make, plus a C toolchain because `go test -race` needs cgo. # Build tooling: make, plus a C toolchain because `go test -race` needs cgo.
# The sqlite driver is pure Go (modernc.org/sqlite), so no sqlite library or # The sqlite driver is pure Go (modernc.org/sqlite), so no sqlite library or
# CLI is required. # CLI is required.
@@ -66,8 +64,31 @@ 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 test RUN echo "check epoch: ${CHECK_EPOCH}" && make test
# Version, commit and build date are computed on the host by
# script/docker and script/cibuild (where .git exists) and passed in as
# build args. The build context excludes .git (see .dockerignore), so
# the build cannot derive them itself: it used to try, with `git
# rev-parse` inside this stage, and always got "unknown". VERSION comes
# from script/version, the source of truth shared with the Makefile, so
# it carries the same tag / dev-<sha> / -dirty rules and a Docker image
# reports the same string a local build of the same tree would.
#
# The defaults are the fallback for a bare `docker build .` that passes
# none of them: an unset arg would otherwise stamp an empty string and
# produce an image that cannot report its own version, commit or date.
# They match what an out-of-git build reports elsewhere.
#
# These ARGs sit here, after the checks, rather than at the top of the
# stage: every commit changes their values, and a value change
# invalidates all layers below the ARG. Declared up top they would bust
# `go mod download`; here they only rekey this build layer, which the
# COPY of the sources above already rebuilds on any change anyway.
ARG VERSION=dev
ARG COMMIT=unknown
ARG COMMIT_DATE=unknown
# Build (pure Go, no CGO required since we use modernc.org/sqlite) # Build (pure Go, no CGO required since we use modernc.org/sqlite)
RUN CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/globals.Version=${VERSION}' -X 'sneak.berlin/go/vaultik/internal/globals.Commit=$(git rev-parse HEAD 2>/dev/null || echo unknown)' -X 'sneak.berlin/go/vaultik/internal/globals.CommitDate=$(git show -s --format=%cs HEAD 2>/dev/null || echo unknown)'" -o /vaultik ./cmd/vaultik RUN CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/globals.Version=${VERSION}' -X 'sneak.berlin/go/vaultik/internal/globals.Commit=${COMMIT}' -X 'sneak.berlin/go/vaultik/internal/globals.CommitDate=${COMMIT_DATE}'" -o /vaultik ./cmd/vaultik
# Runtime stage # Runtime stage
# alpine:3.21, 2026-02-25 # alpine:3.21, 2026-02-25
+5 -5
View File
@@ -72,11 +72,11 @@ RUN [ -n "$CHECK_EPOCH" ] || exit 1
# running, and exits 0 reporting `0 issues.` on a tree the real config # running, and exits 0 reporting `0 issues.` on a tree the real config
# fails. Demonstrated on this repo at this pin, recorded on # fails. Demonstrated on this repo at this pin, recorded on
# https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted # https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted
# over-length line, `script/lint` exits 1 naming the `lll` finding with # over-length line, `script/lint` exits 1 naming the `revive` finding
# `linters:` and exits 0 with `linterz:`. A set-but-ineffective config # with `linters:` and exits 0 with `linterz:`. A set-but-ineffective
# quietly falling back to defaults is precisely the false-green class # config quietly falling back to defaults is precisely the false-green
# this gate exists to eliminate, so it must not sit in the gate's own # class this gate exists to eliminate, so it must not sit in the gate's
# configuration. # own configuration.
# #
# `config verify` catches it, and it does so OFFLINE at this pinned # `config verify` catches it, and it does so OFFLINE at this pinned
# version -- verified, not assumed. Under `docker run --network none` # version -- verified, not assumed. Under `docker run --network none`
+15 -8
View File
@@ -251,7 +251,8 @@ local index alone, and still exits zero.
per-snapshot-name (`--keep-latest` keeps the latest of each name, not the per-snapshot-name (`--keep-latest` keeps the latest of each name, not the
latest globally). latest globally).
* `--keep-latest`: Keep only the most recent snapshot of each name * `--keep-latest`: Keep only the most recent snapshot of each name
* `--older-than <duration>`: Remove snapshots older than duration (e.g. `30d`, `6m`, `1y`) * `--older-than <duration>`: Remove snapshots older than duration (e.g. `30d`,
`4w`, `6mo`, `1y`; `m` is minutes, `mo` is months)
* `--snapshot <name>`: Restrict to specific snapshot names (repeat for multiple) * `--snapshot <name>`: Restrict to specific snapshot names (repeat for multiple)
* `--force`: Skip confirmation prompt * `--force`: Skip confirmation prompt
@@ -456,9 +457,13 @@ Key fields:
sequentially. Restore speed is bound by single-stream throughput. sequentially. Restore speed is bound by single-stream throughput.
* **Device nodes, named pipes, and sockets are silently skipped.** Only * **Device nodes, named pipes, and sockets are silently skipped.** Only
regular files, directories, and symlinks are backed up. regular files, directories, and symlinks are backed up.
* **No database migrations.** If the local SQLite schema changes between * **No upgrade path between versions.** There is no supported way to carry
versions, delete the local database (`vaultik database delete`) and run an existing local index across a schema change; if the local SQLite
a full backup. Remote storage is unaffected. schema changes between versions, delete the local database (`vaultik
database delete`) and run a full backup. Remote storage is unaffected.
(The binary does embed numbered schema files and a `schema_migrations`
table to bootstrap a fresh database — see [`docs/DATAMODEL.md`](docs/DATAMODEL.md)
— but that is not an upgrade path.)
* **Files that change during backup may be inconsistent.** There is no * **Files that change during backup may be inconsistent.** There is no
filesystem snapshot or freeze. If a file is modified between the scan filesystem snapshot or freeze. If a file is modified between the scan
and chunk phases, the backed-up copy may reflect a partial write. and chunk phases, the backed-up copy may reflect a partial write.
@@ -528,10 +533,12 @@ priority.
another host" workflow works but isn't documented as a another host" workflow works but isn't documented as a
first-class operation in this README. Worth a dedicated section first-class operation in this README. Worth a dedicated section
once it's settled. once it's settled.
* **Schema migrations.** Currently nonexistent — pre-1.0 schema * **Cross-version schema upgrades.** There is no upgrade path between
changes are handled by `vaultik database delete` plus a full released versions — pre-1.0 schema changes are handled by `vaultik
re-scan. Post-1.0 we'll need a migration story to keep existing database delete` plus a full re-scan (see
index databases usable across upgrades. [`docs/DATAMODEL.md`](docs/DATAMODEL.md)). Post-1.0 we'll need a
migration story to keep existing index databases usable across
upgrades.
* **Storage backend coverage tests.** S3, file://, and rclone:// * **Storage backend coverage tests.** S3, file://, and rclone://
all share the Storer interface but the rclone path is the least all share the Storer interface but the rclone path is the least
exercised in CI. exercised in CI.
+41 -4
View File
@@ -35,6 +35,24 @@ release" is exactly the contradiction
no `--json` representation — under `--json` the summary is suppressed no `--json` representation — under `--json` the summary is suppressed
entirely — so nothing there can show a false `0`. entirely — so nothing there can show a false `0`.
- 2026-09-21: Made the s3 storage backend report a missing object as
`storage.ErrNotFound`, like the `file` and `rclone` backends and as the
`Storer` interface documents. `S3Storer.Get` and `Stat` returned the raw
AWS SDK error, so `errors.Is(err, storage.ErrNotFound)` was false on s3
and callers branched differently per backend. Added a small `s3.IsNotFound`
helper (reused by `HeadObject`) and a test that a missing key maps to
`ErrNotFound`
([issue #129](https://git.eeqj.de/sneak/vaultik/issues/129)).
- 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as
corrupt. Its final blob-integrity check hashed the encrypted
downloaded bytes with a single SHA256 and compared that to the blob
ID, which is the double SHA256 of the plaintext, so the two could
never match. It now hashes the decompressed plaintext and compares the
double SHA256. Added a test that backs up a real snapshot, deep-verifies
it, then flips a byte in one stored blob and confirms deep verification
then fails
([issue #131](https://git.eeqj.de/sneak/vaultik/issues/131)).
- 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata - 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata
database through the `modernc.org/sqlite` driver instead of shelling database through the `modernc.org/sqlite` driver instead of shelling
out to the external `sqlite` command-line binary (issue #120). A out to the external `sqlite` command-line binary (issue #120). A
@@ -60,6 +78,24 @@ release" is exactly the contradiction
keeps that exact compiler from auto-switching. Bumping Go now touches keeps that exact compiler from auto-switching. Bumping Go now touches
`go.mod`, the checksum, and the `Dockerfile` `golang` digest together. `go.mod`, the checksum, and the `Dockerfile` `golang` digest together.
- 2026-09-21: Collapsed the two duration parsers into one and fixed the
`--older-than` months example
([issue #123](https://git.eeqj.de/sneak/vaultik/issues/123)). Two
functions named `parseDuration` existed with different grammars;
`snapshot purge --older-than` and `--keep-newer-than` both already went
through the one in `internal/vaultik`, while the richer copy in
`internal/cli/duration.go` was reachable only from its own test. Kept
the live-path parser and deleted the unused one, so no flag's accepted
grammar changes. The trap the issue was filed over: `README.md`
documented `6m` as the months example for `--older-than`, but `m` is
minutes, so the documented command deleted every snapshot older than
six minutes on a destructive flag. Corrected the doc to `6mo` and put
both flags' help text on one example list that states `m` is minutes
and `mo` is months. The surviving parser now rejects negatives, which
it previously accepted (`-5h`) or silently made positive (`-5d`).
Table-driven tests cover every unit, `6m` as six minutes, `6mo` as 180
days, and rejection of a bare number, an unknown unit, and a negative.
- 2026-08-10: Moved every lint run into its own container, as a build - 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)). step ([issue #113](https://git.eeqj.de/sneak/vaultik/issues/113)).
New root `Dockerfile.lint`, built by `script/lint`, runs New root `Dockerfile.lint`, built by `script/lint`, runs
@@ -88,10 +124,11 @@ release" is exactly the contradiction
into each check command, and a fresh `$(date +%s%N)$$` per invocation into each check command, and a fresh `$(date +%s%N)$$` per invocation
computed as a bare assignment. `cmd/vaultik/lintdocker_test.go` computed as a bare assignment. `cmd/vaultik/lintdocker_test.go`
parses both Dockerfiles and both scripts and fails if any part of parses both Dockerfiles and both scripts and fails if any part of
that is dropped, because every way of losing it is silent. Its that is dropped, because every way of losing it is silent. No test
host-lint assertion is structural — no script runs `golangci-lint` asserts that no script runs the host linter: `script/lint` is the one
except through `docker` — rather than a search for the one retired lint entry point and runs `golangci-lint` only inside the container,
variable name, which nothing could ever reintroduce. and keeping it that way is a review matter, not something a test
proves.
The product `Dockerfile` lost its lint stage rather than gaining a The product `Dockerfile` lost its lint stage rather than gaining a
second linter pin: `make lint` is now `docker build`, so the stage second linter pin: `make lint` is now `docker build`, so the stage
+102
View File
@@ -0,0 +1,102 @@
package main_test
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// This file guards the version stamping of the product image (issue
// #75). The failure it protects against is silent: the image still
// builds and runs, but `vaultik version` inside it reports "commit:
// unknown", so an operator cannot tell which source produced a given
// backup. .dockerignore excludes .git, so the build cannot derive the
// commit itself; the values must be computed on the host and passed in.
//
// These are parses of the committed files, for the same reason the lint
// guards next door are: shelling out to docker would nest a build
// inside `make test`. That `vaultik version` in the built image really
// prints the host's version is verified by hand and recorded on the
// pull request.
// dockerScript is script/docker, relative to the repository root.
const dockerScript = "script/docker"
// versionArgs are the ldflag targets the build stamps and, matching
// them, the build args the host must supply. The names line up so the
// same list checks both files.
func versionArgs() []string {
return []string{"VERSION", "COMMIT", "COMMIT_DATE"}
}
// TestProductDockerfileTakesVersionAsBuildArgs fails unless the build
// declares each version arg and stamps it into the binary by ldflag
// reference, rather than computing it in the container.
func TestProductDockerfileTakesVersionAsBuildArgs(t *testing.T) {
t.Parallel()
found := instructions(t, productDockerfile)
for _, arg := range versionArgs() {
require.GreaterOrEqual(t, indexOf(found, "ARG "+arg), 0,
"%s must declare `ARG %s` so the host can pass it in",
productDockerfile, arg)
assertLdflagReferences(t, found, arg)
}
}
// TestProductDockerfileDoesNotDeriveVersionItself is the anti-regression
// for the original defect: the container ran `git rev-parse`, but .git
// is not in the build context, so it always resolved to "unknown". No
// git command may reach into a build that cannot see the history.
func TestProductDockerfileDoesNotDeriveVersionItself(t *testing.T) {
t.Parallel()
text := instructionText(readRepoFile(t, productDockerfile))
assert.NotContains(t, text, "git ",
"%s must not run git: .git is excluded from the build context, so"+
" any value it derives is wrong. Pass version, commit and date"+
" in as build args instead.", productDockerfile)
}
// TestDockerScriptComputesVersionOnTheHost fails unless script/docker
// derives each value where .git exists and passes it as a build arg,
// with VERSION coming from script/version so a Docker build reports the
// same string a local build of the same tree would.
func TestDockerScriptComputesVersionOnTheHost(t *testing.T) {
t.Parallel()
script := readRepoFile(t, dockerScript)
for _, arg := range versionArgs() {
assert.Contains(t, script, "--build-arg "+arg+"=",
"%s must pass --build-arg %s to the build", dockerScript, arg)
}
assert.Contains(t, script, "/version",
"%s must take VERSION from script/version, the source of truth"+
" shared with the Makefile", dockerScript)
}
// assertLdflagReferences fails unless some build instruction stamps the
// named variable from the ARG (a ${arg} reference), not from a value
// computed inside the container.
func assertLdflagReferences(t *testing.T, found []string, arg string) {
t.Helper()
for _, instruction := range found {
if strings.HasPrefix(instruction, "RUN ") &&
strings.Contains(instruction, "go build") &&
strings.Contains(instruction, "${"+arg+"}") {
return
}
}
assert.Fail(t, "version arg is declared but never stamped",
"the go build in %s must reference ${%s} in its ldflags, or the"+
" arg is passed and discarded", productDockerfile, arg)
}
+13 -153
View File
@@ -28,6 +28,11 @@ import (
// -- that a real finding actually fails the build -- is verified by // -- that a real finding actually fails the build -- is verified by
// hand against a deliberately broken tree, recorded on the pull // hand against a deliberately broken tree, recorded on the pull
// request. // request.
//
// One property is deliberately NOT tested here: that no script runs the
// linter on the host. script/lint is the only lint entry point, and it
// runs golangci-lint only inside the container; keeping it that way is a
// review matter, not something a test in this file establishes.
// The files under guard, relative to the repository root. // The files under guard, relative to the repository root.
const ( const (
@@ -37,9 +42,8 @@ const (
cibuildScript = "script/cibuild" cibuildScript = "script/cibuild"
) )
// linterBinary is the linter's command name. Every occurrence of it in // linterBinary is the linter's command name, used to locate the
// executable shell in this repo must be inside a docker invocation; see // config-verify and lint steps in Dockerfile.lint.
// TestNoHostLintPathRemains.
const linterBinary = "golangci-lint" const linterBinary = "golangci-lint"
// checkEpochARG is the declaration, with no default value. A default // checkEpochARG is the declaration, with no default value. A default
@@ -219,90 +223,6 @@ func TestCibuildBuildsBothDockerfilesWithFreshEpochs(t *testing.T) {
"%s must build %s", cibuildScript, lintDockerfile) "%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 // assertEpochExpandedInto fails unless some instruction runs the named
// command with the epoch expanded into it. Expansion, not mere // command with the epoch expanded into it. Expansion, not mere
// declaration: an ARG that no instruction references is not guaranteed // declaration: an ARG that no instruction references is not guaranteed
@@ -384,10 +304,14 @@ func instructionText(contents string) string {
} }
// indexOf returns the position of the first instruction equal to, or // indexOf returns the position of the first instruction equal to, or
// beginning with, want; -1 if there is none. // beginning with, want; -1 if there is none. An `ARG NAME=default`
// counts as beginning with `ARG NAME`, so a declared arg is found
// whether or not it carries a default.
func indexOf(found []string, want string) int { func indexOf(found []string, want string) int {
for i, instruction := range found { for i, instruction := range found {
if instruction == want || strings.HasPrefix(instruction, want+" ") { if instruction == want ||
strings.HasPrefix(instruction, want+" ") ||
strings.HasPrefix(instruction, want+"=") {
return i return i
} }
} }
@@ -407,70 +331,6 @@ func indexContaining(found []string, want string) int {
return -1 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 // readRepoFile reads a file by its path relative to the repository
// root. // root.
func readRepoFile(t *testing.T, name string) string { func readRepoFile(t *testing.T, name string) string {
+11 -1
View File
@@ -10,6 +10,16 @@ import (
) )
func main() { func main() {
os.Exit(run())
}
// run sets up optional profiling, runs the CLI, and returns the process
// exit code. os.Exit lives in main so it fires only after run's deferred
// profile writers have flushed. cli.Entry returns a status code rather
// than calling os.Exit itself: an os.Exit from inside it would skip
// these defers and truncate the profile of a failing command -- exactly
// the command one most often wants to profile.
func run() int {
// CPU profiling: set VAULTIK_CPUPROFILE=/path/to/cpu.prof // CPU profiling: set VAULTIK_CPUPROFILE=/path/to/cpu.prof
if cpuProfile := os.Getenv("VAULTIK_CPUPROFILE"); cpuProfile != "" { if cpuProfile := os.Getenv("VAULTIK_CPUPROFILE"); cpuProfile != "" {
f, err := os.Create(cpuProfile) //nolint:gosec // G304: operator-set path f, err := os.Create(cpuProfile) //nolint:gosec // G304: operator-set path
@@ -46,5 +56,5 @@ func main() {
}() }()
} }
cli.Entry() return cli.Entry()
} }
+24 -5
View File
@@ -5,11 +5,30 @@
Vaultik uses a local SQLite database to track file metadata, chunk mappings, and blob associations during the backup process. This database serves as an index for incremental backups and enables efficient deduplication. Vaultik uses a local SQLite database to track file metadata, chunk mappings, and blob associations during the backup process. This database serves as an index for incremental backups and enables efficient deduplication.
**Important Notes:** **Important Notes:**
- **No Migration Support (pre-1.0)**: Vaultik does not support database schema
migrations. The local index is treated as disposable — if the schema changes, This section is the authoritative explanation of the schema/migration story;
delete the local SQLite database (`vaultik database delete`) and run a full other documents (the README and `AGENTS.md`) link here.
backup. The remote storage is unaffected; the new index will re-deduplicate
against existing remote blobs. - **No upgrade path between versions (pre-1.0)**: Vaultik has no supported way to
carry an existing local index across a schema change. The index is disposable
— if the on-disk schema changes between versions, delete the local SQLite
database (`vaultik database delete`) and run a full backup. Remote storage is
unaffected; the new index re-deduplicates against existing remote blobs. This
is the standing project policy, and it is separate from the schema bootstrap
described next.
- **Schema bootstrap**: a fresh database is populated from numbered SQL files
embedded in the binary under `internal/database/schema/`. `000.sql` creates the
`schema_migrations` table; `001.sql` creates the application tables. On opening
a database the code applies each numbered file that has not yet run and records
its version in `schema_migrations`. This bootstraps a new database; it does not
upgrade an existing one between released versions.
- **Changing the schema (pre-1.0)**: edit `internal/database/schema/001.sql` (and
the code that touches the affected tables) directly. Do not add new numbered
files — there is no installed base to migrate.
- **Disposability expires at 1.0**: the index is treated as disposable only until
1.0 ships and is tagged. Once 1.0 is tagged that clause expires and the
question of upgrading existing indexes returns. It is deliberately left open
here.
- **Version Compatibility**: In rare cases, you may need to use the same version - **Version Compatibility**: In rare cases, you may need to use the same version
of Vaultik to restore a backup as was used to create it. This ensures of Vaultik to restore a backup as was used to create it. This ensures
compatibility with the metadata format stored in S3. compatibility with the metadata format stored in S3.
+90 -39
View File
@@ -11,6 +11,7 @@ import (
"os/signal" "os/signal"
"path/filepath" "path/filepath"
"strings" "strings"
"sync"
"syscall" "syscall"
"time" "time"
@@ -196,13 +197,90 @@ func RunApp(ctx context.Context, app *fx.App) error {
} }
} }
// errReported marks a failure the operation has already shown the user
// (and deliberately withheld under --json). Entry turns it into a
// non-zero exit status without printing anything further, so the error
// line is not doubled. It flows up from RunOperation through cobra to
// Entry.
var errReported = errors.New("operation failed")
// RunOperation runs op against the Vaultik instance inside the fx app
// and turns a failure into a returned error rather than an os.Exit from
// within the goroutine. An os.Exit there skipped main's deferred
// profile writers -- so profiling a failing command yielded a truncated
// profile (issue #75) -- and RunWithApp's PID-lock release, and denied
// the app any graceful shutdown; returning the error to the top runs
// all three.
//
// op runs in a goroutine so OnStart returns promptly and an interrupt
// can still cancel through OnStop; when it finishes, success or failure,
// it triggers shutdown, which is what lets RunWithApp return. report is
// called with a non-canceled failure so the caller can log it (and
// suppress it under --json) before it becomes errReported. A context
// cancellation is the interrupt path, not a failure: it is neither
// reported nor counted as one.
func RunOperation(
ctx context.Context, opts AppOptions,
op func(v *vaultik.Vaultik) error, report func(err error),
) error {
var (
mu sync.Mutex
failed bool
)
opts.Invokes = append(opts.Invokes,
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) {
lc.Append(fx.Hook{
OnStart: func(_ context.Context) error {
go func() {
err := op(v)
if err != nil && !errors.Is(err, context.Canceled) {
report(err)
mu.Lock()
failed = true
mu.Unlock()
}
stopErr := v.Shutdowner.Shutdown()
if stopErr != nil {
log.Error("Failed to shutdown", "error", stopErr)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
v.Cancel()
return nil
},
})
}))
err := RunWithApp(ctx, opts)
if err != nil {
return err
}
// The goroutine sets failed before triggering the shutdown that lets
// RunWithApp return, so the write is in place by the time we read it.
mu.Lock()
defer mu.Unlock()
if failed {
return errReported
}
return nil
}
// runVaultikApp runs the standard single-operation command lifecycle // runVaultikApp runs the standard single-operation command lifecycle
// shared by the list/purge/verify/remove/remote-info subcommands: // shared by the list/purge/verify/remove/remote-info subcommands:
// resolve the config, start the fx app, run op against the Vaultik // resolve the config, then run op against the Vaultik instance through
// instance in a goroutine, report a failure prefixed with failMsg // RunOperation, reporting a failure prefixed with failMsg (suppressed
// (suppressed while suppressErrors is true, e.g. under --json), then // while suppressErrors is true, e.g. under --json). extraQuiet is OR-ed
// trigger shutdown. The operation is cancelled when the app stops. // into LogOptions.Quiet (e.g. --json output modes).
// extraQuiet is OR-ed into LogOptions.Quiet (e.g. --json output modes).
func runVaultikApp( func runVaultikApp(
cmd *cobra.Command, extraQuiet, suppressErrors bool, cmd *cobra.Command, extraQuiet, suppressErrors bool,
failMsg string, op func(v *vaultik.Vaultik) error, failMsg string, op func(v *vaultik.Vaultik) error,
@@ -214,47 +292,20 @@ func runVaultikApp(
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet || extraQuiet, Quiet: rootFlags.Quiet || extraQuiet,
}, },
Modules: []fx.Option{}, }, op, func(err error) {
Invokes: []fx.Option{ if suppressErrors {
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) { return
lc.Append(fx.Hook{ }
OnStart: func(_ context.Context) error {
go func() {
err := op(v)
if err != nil {
if !errors.Is(err, context.Canceled) {
if !suppressErrors {
log.Error(failMsg, "error", err)
ReportErrorf("%s: %v", failMsg, err)
}
os.Exit(1) log.Error(failMsg, "error", err)
} ReportErrorf("%s: %v", failMsg, err)
}
err = v.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
v.Cancel()
return nil
},
})
}),
},
}) })
} }
-126
View File
@@ -1,126 +0,0 @@
package cli
import (
"errors"
"fmt"
"regexp"
"strconv"
"strings"
"time"
)
// Approximate lengths of the extended calendar units accepted by
// parseDuration.
const (
durationDay = 24 * time.Hour
durationWeek = 7 * durationDay
durationMonth = 30 * durationDay
durationYear = 365 * durationDay
)
var (
errNegativeDuration = errors.New("negative durations are not supported")
errInvalidDuration = errors.New("invalid duration format")
errUnknownTimeUnit = errors.New("unknown time unit")
)
// parseDuration parses duration strings. Supports standard Go duration format
// (e.g., "3h30m", "1h45m30s") as well as extended units:
// - d: days (e.g., "30d", "7d")
// - w: weeks (e.g., "2w", "4w")
// - mo: months (30 days) (e.g., "6mo", "1mo")
// - y: years (365 days) (e.g., "1y", "2y")
//
// Can combine units: "1y6mo", "2w3d", "1d12h30m"
func parseDuration(s string) (time.Duration, error) {
// First try standard Go duration parsing
d, err := time.ParseDuration(s)
if err == nil {
return d, nil
}
// Extended duration parsing
// Check for negative values
if strings.HasPrefix(strings.TrimSpace(s), "-") {
return 0, errNegativeDuration
}
// Pattern matches: number + unit, repeated
re := regexp.MustCompile(`(\d+(?:\.\d+)?)\s*([a-zA-Z]+)`)
matches := re.FindAllStringSubmatch(s, -1)
if len(matches) == 0 {
return 0, fmt.Errorf("%w: %q", errInvalidDuration, s)
}
var total time.Duration
for _, match := range matches {
valueStr := match[1]
unit := strings.ToLower(match[2])
value, err := strconv.ParseFloat(valueStr, 64)
if err != nil {
return 0, fmt.Errorf("invalid number %q: %w", valueStr, err)
}
d, err := durationForUnit(value, unit)
if err != nil {
return 0, err
}
total += d
}
return total, nil
}
// durationForUnit converts a value with a (case-normalized) unit suffix
// into a time.Duration, accepting Go's standard units plus the extended
// calendar units.
func durationForUnit(value float64, unit string) (time.Duration, error) {
switch unit {
// Standard time units
case "ns", "nanosecond", "nanoseconds":
return time.Duration(value), nil
case "us", "µs", "microsecond", "microseconds":
return time.Duration(value * float64(time.Microsecond)), nil
case "ms", "millisecond", "milliseconds":
return time.Duration(value * float64(time.Millisecond)), nil
case "s", "sec", "second", "seconds":
return time.Duration(value * float64(time.Second)), nil
case "m", "min", "minute", "minutes":
return time.Duration(value * float64(time.Minute)), nil
case "h", "hr", "hour", "hours":
return time.Duration(value * float64(time.Hour)), nil
// Extended units
case "d", "day", "days":
return time.Duration(value * float64(durationDay)), nil
case "w", "week", "weeks":
return time.Duration(value * float64(durationWeek)), nil
case "mo", "month", "months":
// Using 30 days as approximation
return time.Duration(value * float64(durationMonth)), nil
case "y", "year", "years":
// Using 365 days as approximation
return time.Duration(value * float64(durationYear)), nil
default:
// Try parsing as standard Go duration unit
testStr := "1" + unit
_, err := time.ParseDuration(testStr)
if err != nil {
return 0, fmt.Errorf("%w: %q", errUnknownTimeUnit, unit)
}
// It's a valid Go duration unit, parse the full value
fullStr := fmt.Sprintf("%g%s", value, unit)
d, err := time.ParseDuration(fullStr)
if err != nil {
return 0, fmt.Errorf("invalid duration %q: %w", fullStr, err)
}
return d, nil
}
}
-299
View File
@@ -1,299 +0,0 @@
package cli //nolint:testpackage // needs access to unexported parseDuration
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type parseDurationCase struct {
name string
input string
expected time.Duration
wantErr bool
}
// runParseDurationCases executes a table of parseDuration cases as
// parallel subtests.
func runParseDurationCases(t *testing.T, tests []parseDurationCase) {
t.Helper()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := parseDuration(tt.input)
if tt.wantErr {
require.Error(t, err, "expected error for input %q", tt.input)
return
}
require.NoError(t, err, "unexpected error for input %q", tt.input)
assert.Equal(t, tt.expected, got, "duration mismatch for input %q", tt.input)
})
}
}
func TestParseDurationStandard(t *testing.T) {
t.Parallel()
runParseDurationCases(t, []parseDurationCase{
{
name: "standard seconds",
input: "30s",
expected: 30 * time.Second,
},
{
name: "standard minutes",
input: "45m",
expected: 45 * time.Minute,
},
{
name: "standard hours",
input: "2h",
expected: 2 * time.Hour,
},
{
name: "standard combined",
input: "3h30m",
expected: 3*time.Hour + 30*time.Minute,
},
{
name: "standard complex",
input: "1h45m30s",
expected: 1*time.Hour + 45*time.Minute + 30*time.Second,
},
{
name: "standard with milliseconds",
input: "1s500ms",
expected: 1*time.Second + 500*time.Millisecond,
},
})
}
func TestParseDurationExtendedUnits(t *testing.T) {
t.Parallel()
runParseDurationCases(t, []parseDurationCase{
// Extended units - days
{
name: "single day",
input: "1d",
expected: 24 * time.Hour,
},
{
name: "multiple days",
input: "7d",
expected: 7 * 24 * time.Hour,
},
{
name: "fractional days",
input: "1.5d",
expected: 36 * time.Hour,
},
{
name: "days spelled out",
input: "3days",
expected: 3 * 24 * time.Hour,
},
// Extended units - weeks
{
name: "single week",
input: "1w",
expected: 7 * 24 * time.Hour,
},
{
name: "multiple weeks",
input: "4w",
expected: 4 * 7 * 24 * time.Hour,
},
{
name: "weeks spelled out",
input: "2weeks",
expected: 2 * 7 * 24 * time.Hour,
},
// Extended units - months
{
name: "single month",
input: "1mo",
expected: 30 * 24 * time.Hour,
},
{
name: "multiple months",
input: "6mo",
expected: 6 * 30 * 24 * time.Hour,
},
{
name: "months spelled out",
input: "3months",
expected: 3 * 30 * 24 * time.Hour,
},
// Extended units - years
{
name: "single year",
input: "1y",
expected: 365 * 24 * time.Hour,
},
{
name: "multiple years",
input: "2y",
expected: 2 * 365 * 24 * time.Hour,
},
{
name: "years spelled out",
input: "1year",
expected: 365 * 24 * time.Hour,
},
})
}
func TestParseDurationCombinedAndErrors(t *testing.T) {
t.Parallel()
runParseDurationCases(t, []parseDurationCase{
// Combined extended units
{
name: "weeks and days",
input: "2w3d",
expected: 2*7*24*time.Hour + 3*24*time.Hour,
},
{
name: "years and months",
input: "1y6mo",
expected: 365*24*time.Hour + 6*30*24*time.Hour,
},
{
name: "days and hours",
input: "1d12h",
expected: 24*time.Hour + 12*time.Hour,
},
{
name: "complex combination",
input: "1y2mo3w4d5h6m7s",
expected: 365*24*time.Hour + 2*30*24*time.Hour +
3*7*24*time.Hour + 4*24*time.Hour +
5*time.Hour + 6*time.Minute + 7*time.Second,
},
{
name: "with spaces",
input: "1d 12h 30m",
expected: 24*time.Hour + 12*time.Hour + 30*time.Minute,
},
// Edge cases
{
name: "zero duration",
input: "0s",
expected: 0,
},
{
name: "large duration",
input: "10y",
expected: 10 * 365 * 24 * time.Hour,
},
// Error cases
{
name: "empty string",
input: "",
wantErr: true,
},
{
name: "invalid format",
input: "abc",
wantErr: true,
},
{
name: "unknown unit",
input: "5x",
wantErr: true,
},
{
name: "invalid number",
input: "xyzd",
wantErr: true,
},
{
name: "negative not supported",
input: "-5d",
wantErr: true,
},
})
}
func TestParseDurationSpecialCases(t *testing.T) {
t.Parallel()
// Test that standard Go durations work exactly as expected
standardDurations := []string{
"300ms",
"1.5h",
"2h45m",
"72h",
"1us",
"1µs",
"1ns",
}
for _, d := range standardDurations {
expected, err := time.ParseDuration(d)
require.NoError(t, err)
got, err := parseDuration(d)
require.NoError(t, err)
assert.Equal(t, expected, got, "standard duration %q should parse identically", d)
}
}
func TestParseDurationRealWorldExamples(t *testing.T) {
t.Parallel()
// Test real-world snapshot purge scenarios
tests := []struct {
description string
input string
olderThan time.Duration
}{
{
description: "keep snapshots from last 30 days",
input: "30d",
olderThan: 30 * 24 * time.Hour,
},
{
description: "keep snapshots from last 6 months",
input: "6mo",
olderThan: 6 * 30 * 24 * time.Hour,
},
{
description: "keep snapshots from last year",
input: "1y",
olderThan: 365 * 24 * time.Hour,
},
{
description: "keep snapshots from last week and a half",
input: "1w3d",
olderThan: 10 * 24 * time.Hour,
},
{
description: "keep snapshots from last 90 days",
input: "90d",
olderThan: 90 * 24 * time.Hour,
},
}
for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
t.Parallel()
got, err := parseDuration(tt.input)
require.NoError(t, err)
assert.Equal(t, tt.olderThan, got)
// Verify the duration makes sense for snapshot purging
assert.Greater(t, got, time.Hour,
"snapshot purge duration should be at least an hour")
})
}
}
+18 -3
View File
@@ -1,6 +1,7 @@
package cli package cli
import ( import (
"errors"
"io" "io"
"os" "os"
"strings" "strings"
@@ -19,7 +20,11 @@ const shortCommitLen = 12
// flag is present in os.Args — see bannerSuppressedInArgs), executes the // flag is present in os.Args — see bannerSuppressedInArgs), executes the
// root cobra command, and routes any returned error through the // root cobra command, and routes any returned error through 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() { //
// It returns the process exit code (0 on success, 1 on error) rather
// than calling os.Exit, so that main's deferred profile writers run
// before the process ends. See run in cmd/vaultik/main.go.
func Entry() int {
emitStartupBanner(os.Args[1:], os.Stdout) emitStartupBanner(os.Args[1:], os.Stdout)
rootCmd := NewRootCommand() rootCmd := NewRootCommand()
@@ -27,9 +32,19 @@ func Entry() {
err := rootCmd.Execute() err := rootCmd.Execute()
if err != nil { if err != nil {
ReportErrorf("%s", err.Error()) // An operation that ran inside the fx app has already reported
os.Exit(1) // its own failure (and suppressed it under --json); errReported
// says so. Printing it again here would double the error line.
// Every other error — bad arguments, a config that would not
// load — reaches Entry unreported, so it is shown here.
if !errors.Is(err, errReported) {
ReportErrorf("%s", err.Error())
}
return 1
} }
return 0
} }
// emitStartupBanner writes the startup banner to w unless args (the // emitStartupBanner writes the startup banner to w unless args (the
+1 -1
View File
@@ -230,7 +230,7 @@ func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) {
programName, flagConfig, configPath, cmdSnapshot, cmdList, flagJSON, programName, flagConfig, configPath, cmdSnapshot, cmdList, flagJSON,
} }
stdout := captureProcessStdout(t, Entry) stdout := captureProcessStdout(t, func() { _ = Entry() })
requireExactlyOneJSONDocument(t, stdout) requireExactlyOneJSONDocument(t, stdout)
+1 -1
View File
@@ -81,7 +81,7 @@ func TestEntryPruneJSONStdoutIsExactlyOneDocument(t *testing.T) {
programName, flagConfig, configPath, cmdPrune, flagJSON, programName, flagConfig, configPath, cmdPrune, flagJSON,
} }
stdout := captureProcessStdout(t, Entry) stdout := captureProcessStdout(t, func() { _ = Entry() })
requireExactlyOneJSONDocument(t, stdout) requireExactlyOneJSONDocument(t, stdout)
+58
View File
@@ -0,0 +1,58 @@
package cli //nolint:testpackage // shares programName and the capture helpers
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
)
// TestEntryReturnsStatusCode pins the contract main() relies on for
// issue #75: Entry reports success or failure through its return value
// and never calls os.Exit. An os.Exit from inside Entry would skip
// main's deferred profile writers and truncate the profile of a failing
// command. main turns this code into os.Exit only after those defers
// run, so a failing command must come back with a non-zero code rather
// than ending the process here.
//
// Stdout is captured only to keep the banner and command output off the
// test log; the assertion is on the returned code.
//
//nolint:paralleltest // replaces os.Args and rootFlags
func TestEntryReturnsStatusCode(t *testing.T) {
for _, testCase := range []struct {
name string
args []string
want int
}{
{
// version is self-contained: it needs no config and no
// destination store, so it exercises the success path.
name: "successful command returns zero",
args: []string{programName, "version"},
want: 0,
},
{
name: "unknown command returns one",
args: []string{programName, "no-such-command"},
want: 1,
},
} {
t.Run(testCase.name, func(t *testing.T) {
previousArgs := os.Args
t.Cleanup(func() {
os.Args = previousArgs
rootFlags = RootFlags{}
})
os.Args = testCase.args
var code int
_ = captureProcessStdout(t, func() { code = Entry() })
assert.Equal(t, testCase.want, code)
})
}
}
+6 -37
View File
@@ -1,12 +1,7 @@
package cli package cli
import ( import (
"context"
"errors"
"os"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -33,44 +28,18 @@ func NewInfoCommand() *cobra.Command {
// Use the app framework // Use the app framework
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
}, },
Modules: []fx.Option{}, }, func(v *vaultik.Vaultik) error {
Invokes: []fx.Option{ return v.ShowInfo()
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) { }, func(err error) {
lc.Append(fx.Hook{ log.Error("Failed to show info", "error", err)
OnStart: func(_ context.Context) error { ReportErrorf("Failed to show info: %v", err)
go func() {
err := v.ShowInfo()
if err != nil {
if !errors.Is(err, context.Canceled) {
log.Error("Failed to show info", "error", err)
ReportErrorf("Failed to show info: %v", err)
os.Exit(1)
}
}
err = v.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
v.Cancel()
return nil
},
})
}),
},
}) })
}, },
} }
+9 -43
View File
@@ -1,12 +1,7 @@
package cli package cli
import ( import (
"context"
"errors"
"os"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -41,51 +36,22 @@ work (e.g. after a crashed backup or to reclaim storage).`,
// Use the app framework like other commands // Use the app framework like other commands
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet || opts.JSON, Quiet: rootFlags.Quiet || opts.JSON,
}, },
Modules: []fx.Option{}, }, func(v *vaultik.Vaultik) error {
Invokes: []fx.Option{ return v.Prune(opts)
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) { }, func(err error) {
lc.Append(fx.Hook{ if opts.JSON {
OnStart: func(_ context.Context) error { return
// Start the prune operation in a goroutine }
go func() {
// Run the prune operation
err := v.Prune(opts)
if err != nil {
if !errors.Is(err, context.Canceled) {
if !opts.JSON {
log.Error("Prune operation failed", "error", err)
ReportErrorf("Prune failed: %v", err)
}
os.Exit(1) log.Error("Prune operation failed", "error", err)
} ReportErrorf("Prune failed: %v", err)
}
// Shutdown the app when prune completes
err = v.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
log.Debug("Stopping prune operation")
v.Cancel()
return nil
},
})
}),
},
}) })
}, },
} }
+9 -37
View File
@@ -1,12 +1,9 @@
package cli package cli
import ( import (
"context"
"errors" "errors"
"os"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -83,47 +80,22 @@ func newRemoteInfoCommand() *cobra.Command {
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet || jsonOutput, Quiet: rootFlags.Quiet || jsonOutput,
}, },
Modules: []fx.Option{}, }, func(v *vaultik.Vaultik) error {
Invokes: []fx.Option{ return v.RemoteInfo(jsonOutput)
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) { }, func(err error) {
lc.Append(fx.Hook{ if jsonOutput {
OnStart: func(_ context.Context) error { return
go func() { }
err := v.RemoteInfo(jsonOutput)
if err != nil {
if !errors.Is(err, context.Canceled) {
if !jsonOutput {
log.Error("Failed to get remote info", "error", err)
ReportErrorf("Failed to get remote info: %v", err)
}
os.Exit(1) log.Error("Failed to get remote info", "error", err)
} ReportErrorf("Failed to get remote info: %v", err)
}
err = v.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
v.Cancel()
return nil
},
})
}),
},
}) })
}, },
} }
+20 -76
View File
@@ -1,13 +1,10 @@
package cli package cli
import ( import (
"context"
"errors" "errors"
"fmt" "fmt"
"os"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -86,7 +83,8 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
// Use the backup functionality from cli package // Use the backup functionality from cli package
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ // --cron suppression is wired through v.UI by setupGlobals.
return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
@@ -94,42 +92,11 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
Cron: opts.Cron, Cron: opts.Cron,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
}, },
Modules: []fx.Option{}, }, func(v *vaultik.Vaultik) error {
Invokes: []fx.Option{ return v.CreateSnapshot(opts)
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) { }, func(err error) {
lc.Append(fx.Hook{ log.Error("Snapshot creation failed", "error", err)
OnStart: func(_ context.Context) error { ReportErrorf("Snapshot creation failed: %v", err)
// Start the snapshot creation in a goroutine
go func() {
// --cron suppression is wired through v.UI by setupGlobals.
err := v.CreateSnapshot(opts)
if err != nil {
if !errors.Is(err, context.Canceled) {
log.Error("Snapshot creation failed", "error", err)
ReportErrorf("Snapshot creation failed: %v", err)
os.Exit(1)
}
}
// Shutdown the app when snapshot completes
err = v.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
log.Debug("Stopping snapshot creation")
// Cancel the Vaultik context
v.Cancel()
return nil
},
})
}),
},
}) })
}, },
} }
@@ -141,7 +108,8 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
"orphaned blobs") "orphaned blobs")
cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "", cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "",
"With --prune: keep snapshots newer than this duration "+ "With --prune: keep snapshots newer than this duration "+
"(e.g. 4w, 30d, 6mo) instead of only the latest") "(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months) "+
"instead of only the latest")
return cmd return cmd
} }
@@ -204,7 +172,8 @@ restrict the operation to specific snapshot names.`,
cmd.Flags().BoolVar(&opts.KeepLatest, "keep-latest", false, cmd.Flags().BoolVar(&opts.KeepLatest, "keep-latest", false,
"Keep only the latest snapshot of each name") "Keep only the latest snapshot of each name")
cmd.Flags().StringVar(&opts.OlderThan, "older-than", "", cmd.Flags().StringVar(&opts.OlderThan, "older-than", "",
"Remove snapshots older than duration (e.g., 30d, 6m, 1y)") "Remove snapshots older than duration "+
"(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months)")
cmd.Flags().BoolVar(&opts.Force, "force", false, "Skip confirmation prompt") cmd.Flags().BoolVar(&opts.Force, "force", false, "Skip confirmation prompt")
cmd.Flags().StringArrayVar(&opts.Names, "snapshot", nil, cmd.Flags().StringArrayVar(&opts.Names, "snapshot", nil,
"Restrict to snapshots with these names (repeat for multiple)") "Restrict to snapshots with these names (repeat for multiple)")
@@ -232,47 +201,22 @@ func newSnapshotVerifyCommand() *cobra.Command {
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet || opts.JSON, Quiet: rootFlags.Quiet || opts.JSON,
}, },
Modules: []fx.Option{}, }, func(v *vaultik.Vaultik) error {
Invokes: []fx.Option{ return v.VerifySnapshotWithOptions(snapshotID, opts)
fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) { }, func(err error) {
lc.Append(fx.Hook{ if opts.JSON {
OnStart: func(_ context.Context) error { return
go func() { }
err := v.VerifySnapshotWithOptions(snapshotID, opts)
if err != nil {
if !errors.Is(err, context.Canceled) {
if !opts.JSON {
log.Error("Verification failed", "error", err)
ReportErrorf("Verification failed: %v", err)
}
os.Exit(1) log.Error("Verification failed", "error", err)
} ReportErrorf("Verification failed: %v", err)
}
err = v.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
v.Cancel()
return nil
},
})
}),
},
}) })
}, },
} }
+14 -87
View File
@@ -1,16 +1,8 @@
package cli package cli
import ( import (
"context"
"errors"
"os"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/globals"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -25,15 +17,6 @@ type RestoreOptions struct {
Verify bool // Verify restored files after restore Verify bool // Verify restored files after restore
} }
// RestoreApp contains all dependencies needed for restore
type RestoreApp struct {
Globals *globals.Globals
Config *config.Config
Storage storage.Storer
Vaultik *vaultik.Vaultik
Shutdowner fx.Shutdowner
}
// newSnapshotRestoreCommand creates the 'snapshot restore' subcommand // newSnapshotRestoreCommand creates the 'snapshot restore' subcommand
func newSnapshotRestoreCommand() *cobra.Command { func newSnapshotRestoreCommand() *cobra.Command {
opts := &RestoreOptions{} opts := &RestoreOptions{}
@@ -77,7 +60,8 @@ Examples:
return cmd return cmd
} }
// runRestore parses arguments and runs the restore operation through the app framework // runRestore parses arguments and runs the restore operation through the
// app framework.
func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error { func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error {
snapshotID := args[0] snapshotID := args[0]
@@ -86,87 +70,30 @@ func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error {
opts.Paths = args[restoreMinArgs:] opts.Paths = args[restoreMinArgs:]
} }
// Use unified config resolution
configPath, err := ResolveConfigPath() configPath, err := ResolveConfigPath()
if err != nil { if err != nil {
return err return err
} }
// Use the app framework like other commands
rootFlags := GetRootFlags() rootFlags := GetRootFlags()
return RunWithApp(cmd.Context(), AppOptions{ return RunOperation(cmd.Context(), AppOptions{
ConfigPath: configPath, ConfigPath: configPath,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
}, },
Modules: buildRestoreModules(), }, func(v *vaultik.Vaultik) error {
Invokes: buildRestoreInvokes(snapshotID, opts), return v.Restore(&vaultik.RestoreOptions{
SnapshotID: snapshotID,
TargetDir: opts.TargetDir,
Paths: opts.Paths,
Verify: opts.Verify,
SkipErrors: rootFlags.SkipErrors,
})
}, func(err error) {
log.Error("Restore operation failed", "error", err)
ReportErrorf("Restore failed: %v", err)
}) })
} }
// buildRestoreModules returns the fx.Options for dependency injection in restore
func buildRestoreModules() []fx.Option {
return []fx.Option{
fx.Provide(fx.Annotate(
func(g *globals.Globals, cfg *config.Config,
storer storage.Storer, v *vaultik.Vaultik, shutdowner fx.Shutdowner) *RestoreApp {
return &RestoreApp{
Globals: g,
Config: cfg,
Storage: storer,
Vaultik: v,
Shutdowner: shutdowner,
}
},
)),
}
}
// buildRestoreInvokes returns the fx.Options that wire up the restore lifecycle
func buildRestoreInvokes(snapshotID string, opts *RestoreOptions) []fx.Option {
return []fx.Option{
fx.Invoke(func(app *RestoreApp, lc fx.Lifecycle) {
lc.Append(fx.Hook{
OnStart: func(_ context.Context) error {
// Start the restore operation in a goroutine
go func() {
// Run the restore operation
restoreOpts := &vaultik.RestoreOptions{
SnapshotID: snapshotID,
TargetDir: opts.TargetDir,
Paths: opts.Paths,
Verify: opts.Verify,
SkipErrors: GetRootFlags().SkipErrors,
}
err := app.Vaultik.Restore(restoreOpts)
if err != nil {
if !errors.Is(err, context.Canceled) {
log.Error("Restore operation failed", "error", err)
ReportErrorf("Restore failed: %v", err)
os.Exit(1)
}
}
// Shutdown the app when restore completes
err = app.Shutdowner.Shutdown()
if err != nil {
log.Error("Failed to shutdown", "error", err)
}
}()
return nil
},
OnStop: func(_ context.Context) error {
log.Debug("Stopping restore operation")
app.Vaultik.Cancel()
return nil
},
})
}),
}
}
+13 -5
View File
@@ -219,11 +219,7 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
Key: aws.String(fullKey), Key: aws.String(fullKey),
}) })
if err != nil { if err != nil {
var ( if IsNotFound(err) {
notFound *s3types.NotFound
noSuchKey *s3types.NoSuchKey
)
if errors.As(err, &notFound) || errors.As(err, &noSuchKey) {
return false, nil return false, nil
} }
@@ -233,6 +229,18 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
return true, nil return true, nil
} }
// IsNotFound reports whether err indicates that an object does not exist.
// Head and Get requests surface a missing object as different SDK types,
// so both are checked here.
func IsNotFound(err error) bool {
var (
notFound *s3types.NotFound
noSuchKey *s3types.NoSuchKey
)
return errors.As(err, &notFound) || errors.As(err, &noSuchKey)
}
// ObjectInfo contains information about an S3 object. // ObjectInfo contains information about an S3 object.
// It is used by ListObjectsStream to return object metadata // It is used by ListObjectsStream to return object metadata
// along with any errors encountered during listing. // along with any errors encountered during listing.
+79 -54
View File
@@ -46,31 +46,18 @@ func (f *FileStorer) SetFilesystem(fs afero.Fs) {
// storage base path. // storage base path.
const storageDirPerm = 0o755 const storageDirPerm = 0o755
// tempSuffix marks a partially written object. writeAtomic streams into a
// temp file carrying this suffix and only renames it onto the real key once
// the whole object is on disk, so an interrupted write can never leave a
// truncated object at the key a later run would Stat and trust as a complete
// blob. List and ListStream skip these files, so a leftover from an
// interrupted write is never listed or trusted as a blob; it is otherwise
// harmless and is overwritten when the same key is written again.
const tempSuffix = ".partial"
// Put stores data at the specified key. // Put stores data at the specified key.
func (f *FileStorer) Put(_ context.Context, key string, data io.Reader) error { func (f *FileStorer) Put(_ context.Context, key string, data io.Reader) error {
path := f.fullPath(key) return f.writeAtomic(key, data, nil)
// Create parent directories
dir := filepath.Dir(path)
err := f.fs.MkdirAll(dir, storageDirPerm)
if err != nil {
return fmt.Errorf("creating directories: %w", err)
}
file, err := f.fs.Create(path)
if err != nil {
return fmt.Errorf("creating file: %w", err)
}
defer func() { _ = file.Close() }()
_, err = io.Copy(file, data)
if err != nil {
return fmt.Errorf("writing file: %w", err)
}
return nil
} }
// PutWithProgress stores data with progress reporting. // PutWithProgress stores data with progress reporting.
@@ -78,35 +65,7 @@ func (f *FileStorer) PutWithProgress(
_ context.Context, key string, data io.Reader, _ context.Context, key string, data io.Reader,
_ int64, progress ProgressCallback, _ int64, progress ProgressCallback,
) error { ) error {
path := f.fullPath(key) return f.writeAtomic(key, data, progress)
// Create parent directories
dir := filepath.Dir(path)
err := f.fs.MkdirAll(dir, storageDirPerm)
if err != nil {
return fmt.Errorf("creating directories: %w", err)
}
file, err := f.fs.Create(path)
if err != nil {
return fmt.Errorf("creating file: %w", err)
}
defer func() { _ = file.Close() }()
// Wrap with progress tracking
pw := &progressWriter{
writer: file,
callback: progress,
}
_, err = io.Copy(pw, data)
if err != nil {
return fmt.Errorf("writing file: %w", err)
}
return nil
} }
// Get retrieves data from the specified key. // Get retrieves data from the specified key.
@@ -188,7 +147,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
default: default:
} }
if !info.IsDir() { if !info.IsDir() && !strings.HasSuffix(info.Name(), tempSuffix) {
// Convert back to key (relative path from basePath) // Convert back to key (relative path from basePath)
relPath, err := filepath.Rel(f.basePath, path) relPath, err := filepath.Rel(f.basePath, path)
if err != nil { if err != nil {
@@ -245,7 +204,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
return nil //nolint:nilerr // continue walking despite errors return nil //nolint:nilerr // continue walking despite errors
} }
if !info.IsDir() { if !info.IsDir() && !strings.HasSuffix(info.Name(), tempSuffix) {
relPath, err := filepath.Rel(f.basePath, path) relPath, err := filepath.Rel(f.basePath, path)
if err != nil { if err != nil {
ch <- ObjectInfo{Err: fmt.Errorf("computing relative path: %w", err)} ch <- ObjectInfo{Err: fmt.Errorf("computing relative path: %w", err)}
@@ -275,6 +234,72 @@ func (f *FileStorer) Info() Info {
} }
} }
// writeAtomic streams data into a temp file in the destination directory,
// fsyncs it, and renames it onto the final key. The key therefore appears
// only once the whole object has been durably written; a failure part-way
// leaves a temp file (removed here on the failing path) rather than a
// truncated object at the key.
func (f *FileStorer) writeAtomic(
key string, data io.Reader, progress ProgressCallback,
) error {
path := f.fullPath(key)
dir := filepath.Dir(path)
err := f.fs.MkdirAll(dir, storageDirPerm)
if err != nil {
return fmt.Errorf("creating directories: %w", err)
}
tmp, err := afero.TempFile(f.fs, dir, filepath.Base(path)+"-*"+tempSuffix)
if err != nil {
return fmt.Errorf("creating temp file: %w", err)
}
tmpPath := tmp.Name()
// Remove the temp file unless the rename below claims it. On the success
// path renamed is true, so the deferred Close and Remove are harmless
// no-ops on a name that no longer exists.
renamed := false
defer func() {
_ = tmp.Close()
if !renamed {
_ = f.fs.Remove(tmpPath)
}
}()
var w io.Writer = tmp
if progress != nil {
w = &progressWriter{writer: tmp, callback: progress}
}
_, err = io.Copy(w, data)
if err != nil {
return fmt.Errorf("writing file: %w", err)
}
err = tmp.Sync()
if err != nil {
return fmt.Errorf("syncing temp file: %w", err)
}
err = tmp.Close()
if err != nil {
return fmt.Errorf("closing temp file: %w", err)
}
err = f.fs.Rename(tmpPath, path)
if err != nil {
return fmt.Errorf("renaming temp file: %w", err)
}
renamed = true
return nil
}
// fullPath returns the full filesystem path for a key. // fullPath returns the full filesystem path for a key.
func (f *FileStorer) fullPath(key string) string { func (f *FileStorer) fullPath(key string) string {
return filepath.Join(f.basePath, key) return filepath.Join(f.basePath, key)
+119
View File
@@ -0,0 +1,119 @@
package storage_test
import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"sneak.berlin/go/vaultik/internal/storage"
)
// errStreamInterrupted stands in for an upload cut off mid-stream.
var errStreamInterrupted = errors.New("connection reset mid-upload")
// failingReader yields its data once, then fails.
type failingReader struct {
data []byte
done bool
}
func (r *failingReader) Read(p []byte) (int, error) {
if r.done {
return 0, errStreamInterrupted
}
n := copy(p, r.data)
r.done = true
return n, nil
}
// TestFileStorer_InterruptedWriteLeavesNoTrustedObject checks that a write
// cut off mid-stream leaves nothing at the destination key, so a later run
// cannot Stat a truncated object and trust it as a complete blob.
func TestFileStorer_InterruptedWriteLeavesNoTrustedObject(t *testing.T) {
t.Parallel()
f, err := storage.NewFileStorer(t.TempDir())
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
ctx := context.Background()
key := "blobs/aa/bb/aabbccddeeff"
err = f.PutWithProgress(ctx, key, &failingReader{data: []byte("partial")}, 4096, nil)
if err == nil {
t.Fatal("expected the interrupted write to fail, got nil")
}
_, err = f.Stat(ctx, key)
if !errors.Is(err, storage.ErrNotFound) {
t.Fatalf("expected key absent after interrupted write, got Stat err %v", err)
}
keys, err := f.List(ctx, "blobs/")
if err != nil {
t.Fatalf("List: %v", err)
}
if len(keys) != 0 {
t.Fatalf("expected no keys listed after interrupted write, got %v", keys)
}
}
// TestFileStorer_ListSkipsPartialFiles checks that a leftover temp file (the
// storage layer names them with a ".partial" suffix) is never surfaced as a
// key by List or ListStream.
func TestFileStorer_ListSkipsPartialFiles(t *testing.T) {
t.Parallel()
base := t.TempDir()
f, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
ctx := context.Background()
realKey := "blobs/aa/bb/aabbccddeeff"
err = f.Put(ctx, realKey, strings.NewReader("blob-bytes"))
if err != nil {
t.Fatalf("Put: %v", err)
}
// A stray temp file, as an interrupted write would leave behind.
leftover := filepath.Join(base, "blobs/aa/bb/aabbccddeeff-123456.partial")
err = os.WriteFile(leftover, []byte("half"), 0o600)
if err != nil {
t.Fatalf("writing leftover temp file: %v", err)
}
keys, err := f.List(ctx, "blobs/")
if err != nil {
t.Fatalf("List: %v", err)
}
if len(keys) != 1 || keys[0] != realKey {
t.Fatalf("List should return only the real key, got %v", keys)
}
var streamed []string
for obj := range f.ListStream(ctx, "blobs/") {
if obj.Err != nil {
t.Fatalf("ListStream: %v", obj.Err)
}
streamed = append(streamed, obj.Key)
}
if len(streamed) != 1 || streamed[0] != realKey {
t.Fatalf("ListStream should return only the real key, got %v", streamed)
}
}
+16 -1
View File
@@ -38,14 +38,29 @@ func (s *S3Storer) PutWithProgress(
} }
// Get retrieves data from the specified key. // Get retrieves data from the specified key.
// Returns ErrNotFound if the object does not exist.
func (s *S3Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) { func (s *S3Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) {
return s.client.GetObject(ctx, key) rc, err := s.client.GetObject(ctx, key)
if err != nil {
if s3.IsNotFound(err) {
return nil, fmt.Errorf("get %q: %w", key, ErrNotFound)
}
return nil, err
}
return rc, nil
} }
// Stat returns metadata about an object without retrieving its contents. // Stat returns metadata about an object without retrieving its contents.
// Returns ErrNotFound if the object does not exist.
func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) { func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) {
info, err := s.client.StatObject(ctx, key) info, err := s.client.StatObject(ctx, key)
if err != nil { if err != nil {
if s3.IsNotFound(err) {
return nil, fmt.Errorf("stat %q: %w", key, ErrNotFound)
}
return nil, err return nil, err
} }
+59
View File
@@ -0,0 +1,59 @@
package storage_test
import (
"context"
"errors"
"net/http/httptest"
"testing"
"github.com/johannesboyne/gofakes3"
"github.com/johannesboyne/gofakes3/backend/s3mem"
"sneak.berlin/go/vaultik/internal/s3"
"sneak.berlin/go/vaultik/internal/storage"
)
// TestS3StorerMissingKeyMapsToErrNotFound verifies that the s3 backend reports
// a missing object as storage.ErrNotFound, matching the file and rclone
// backends and the Storer contract. Without the mapping, Get and Stat leak the
// raw SDK error and errors.Is(err, storage.ErrNotFound) is false.
//
//nolint:paralleltest // shares an in-process S3 server via t.Cleanup
func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
const bucket = "test-bucket"
backend := s3mem.New()
err := backend.CreateBucket(bucket)
if err != nil {
t.Fatalf("create bucket: %v", err)
}
srv := httptest.NewServer(gofakes3.New(backend).Server())
t.Cleanup(srv.Close)
ctx := context.Background()
client, err := s3.NewClient(ctx, s3.Config{
Endpoint: srv.URL,
Bucket: bucket,
AccessKeyID: "test",
SecretAccessKey: "test",
Region: "us-east-1",
})
if err != nil {
t.Fatalf("new client: %v", err)
}
storer := storage.NewS3Storer(client)
_, err = storer.Get(ctx, "does-not-exist")
if !errors.Is(err, storage.ErrNotFound) {
t.Errorf("Get on missing key: got %v, want ErrNotFound", err)
}
_, err = storer.Stat(ctx, "does-not-exist")
if !errors.Is(err, storage.ErrNotFound) {
t.Errorf("Stat on missing key: got %v, want ErrNotFound", err)
}
}
+108
View File
@@ -0,0 +1,108 @@
package vaultik_test
import (
"context"
"io"
"os"
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob backs up a real
// snapshot with the on-disk storage backend, runs deep verification on
// it, then flips a byte inside one stored blob and runs deep
// verification again. A healthy snapshot must pass; a corrupted blob
// must fail. The healthy case is the regression guard: deep
// verification used to hash the encrypted blob bytes and compare them
// to the blob's ID (the double SHA256 of the plaintext), so it reported
// every healthy blob as corrupt.
func TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "source")
storeDir := filepath.Join(tempDir, "remote")
dbPath := filepath.Join(tempDir, "index.sqlite")
chunkSize := int64(64 * 1024)
maxBlobSize := int64(512 * 1024)
// One file large enough to span several chunks within a single blob.
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
require.NoError(t, afero.WriteFile(fs,
filepath.Join(dataDir, "data.bin"),
bytesPattern("deep-", int(chunkSize*3)), 0o644))
ctx := context.Background()
// runFileStorageBackup writes a real snapshot to storeDir and closes
// the source index, so verification runs from remote bytes only.
cfg, storer, snapshotID := runFileStorageBackup(
ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
newVerifier := func() *vaultik.Vaultik {
v := &vaultik.Vaultik{
Config: cfg,
Storage: storer,
Fs: fs,
Stdout: io.Discard,
Stderr: io.Discard,
UI: ui.NewWithColor(io.Discard, false),
}
v.SetContext(ctx)
return v
}
require.NoError(t,
newVerifier().RunDeepVerify(snapshotID, &vaultik.VerifyOptions{Deep: true}),
"deep verify should pass on a healthy snapshot")
// Flip a byte inside one blob without changing its length, so the
// blob-existence and size checks still pass and verification reaches
// the blob-content stage.
corruptOneBlob(t, fs, filepath.Join(storeDir, "blobs"))
require.Error(t,
newVerifier().RunDeepVerify(snapshotID, &vaultik.VerifyOptions{Deep: true}),
"deep verify should fail on a corrupted blob")
}
// corruptOneBlob flips a middle byte of the first blob file found under
// blobsDir, leaving the file length unchanged.
func corruptOneBlob(t *testing.T, fs afero.Fs, blobsDir string) {
t.Helper()
var blobPath string
err := afero.Walk(fs, blobsDir,
func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if blobPath == "" && !info.IsDir() {
blobPath = path
}
return nil
})
require.NoError(t, err)
require.NotEmpty(t, blobPath, "expected at least one blob on disk")
data, err := afero.ReadFile(fs, blobPath)
require.NoError(t, err)
require.NotEmpty(t, data)
data[len(data)/2] ^= 0xff
require.NoError(t, afero.WriteFile(fs, blobPath, data, 0o644))
}
+9 -3
View File
@@ -33,8 +33,9 @@ func ubytes(n int64) string {
var ( var (
errMalformedSnapshotID = errors.New( errMalformedSnapshotID = errors.New(
"invalid snapshot ID format: expected hostname_snapshotname_timestamp") "invalid snapshot ID format: expected hostname_snapshotname_timestamp")
errInvalidDuration = errors.New("invalid duration") errInvalidDuration = errors.New("invalid duration")
errUnknownTimeUnit = errors.New("unknown time unit") errUnknownTimeUnit = errors.New("unknown time unit")
errNegativeDuration = errors.New("negative durations are not supported")
) )
// Time-unit lengths used by parseDuration. // Time-unit lengths used by parseDuration.
@@ -138,8 +139,13 @@ func parseSnapshotName(snapshotID string) string {
// parseDuration parses a duration string with support for human-friendly units: // parseDuration parses a duration string with support for human-friendly units:
// d/day/days, w/week/weeks, mo/month/months, y/year/years, plus standard Go // d/day/days, w/week/weeks, mo/month/months, y/year/years, plus standard Go
// duration units (h, m, s). // duration units. Following Go, m is minutes and mo is months. A bare number,
// an unknown unit, and a negative value are all rejected.
func parseDuration(s string) (time.Duration, error) { func parseDuration(s string) (time.Duration, error) {
if strings.HasPrefix(strings.TrimSpace(s), "-") {
return 0, errNegativeDuration
}
d, err := time.ParseDuration(s) d, err := time.ParseDuration(s)
if err == nil { if err == nil {
return d, nil return d, nil
+25 -6
View File
@@ -51,13 +51,32 @@ func TestParseDuration(t *testing.T) {
want time.Duration want time.Duration
err bool err bool
}{ }{
{"30d", 30 * 24 * time.Hour, false}, // Go units, including the m-is-minutes / mo-is-months distinction
{"4w", 4 * 7 * 24 * time.Hour, false}, // that this parser exists to keep straight.
{"6mo", 6 * 30 * 24 * time.Hour, false}, {"10ns", 10 * time.Nanosecond, false},
{"1y", 365 * 24 * time.Hour, false}, {"10us", 10 * time.Microsecond, false},
{"2w3d", 2*7*24*time.Hour + 3*24*time.Hour, false}, {"500ms", 500 * time.Millisecond, false},
{"1h", time.Hour, false},
{"30s", 30 * time.Second, false}, {"30s", 30 * time.Second, false},
{"6m", 6 * time.Minute, false},
{"1h", time.Hour, false},
// Extended calendar units.
{"30d", 30 * 24 * time.Hour, false},
{"3days", 3 * 24 * time.Hour, false},
{"4w", 4 * 7 * 24 * time.Hour, false},
{"2weeks", 2 * 7 * 24 * time.Hour, false},
{"6mo", 180 * 24 * time.Hour, false},
{"1month", 30 * 24 * time.Hour, false},
{"1y", 365 * 24 * time.Hour, false},
{"2years", 2 * 365 * 24 * time.Hour, false},
// Combined units.
{"2w3d", 2*7*24*time.Hour + 3*24*time.Hour, false},
{"1y6mo", 365*24*time.Hour + 180*24*time.Hour, false},
// Rejected inputs.
{"6", 0, true}, // bare number, no unit
{"5x", 0, true}, // unknown unit
{"-5d", 0, true}, // negative, extended unit
{"-5h", 0, true}, // negative, Go unit
{"", 0, true}, // empty
{"garbage", 0, true}, {"garbage", 0, true},
} }
+19 -14
View File
@@ -344,12 +344,8 @@ func (v *Vaultik) verifyBlob(blobInfo snapshot.BlobInfo, db *sql.DB) error {
return fmt.Errorf("failed to get decryptor: %w", err) return fmt.Errorf("failed to get decryptor: %w", err)
} }
// Hash the encrypted blob data as it streams through to decryption // Decrypt blob
blobHasher := sha256.New() decryptedReader, err := decryptor.DecryptStream(reader)
teeReader := io.TeeReader(reader, blobHasher)
// Decrypt blob (reading through teeReader to hash encrypted data)
decryptedReader, err := decryptor.DecryptStream(teeReader)
if err != nil { if err != nil {
return fmt.Errorf("failed to decrypt: %w", err) return fmt.Errorf("failed to decrypt: %w", err)
} }
@@ -361,12 +357,19 @@ func (v *Vaultik) verifyBlob(blobInfo snapshot.BlobInfo, db *sql.DB) error {
} }
defer decompressor.Close() defer decompressor.Close()
chunkCount, err := v.verifyBlobChunks(db, blobInfo.Hash, decompressor) // A blob's hash — its remote name — is the double SHA256 of its
// decompressed plaintext (see blobgen.Writer.Sum256), not of the
// encrypted bytes. Hash the plaintext as chunk verification streams
// it, then compare on completion.
plaintextHasher := sha256.New()
hashedStream := io.TeeReader(decompressor, plaintextHasher)
chunkCount, err := v.verifyBlobChunks(db, blobInfo.Hash, hashedStream)
if err != nil { if err != nil {
return err return err
} }
err = v.verifyBlobFinalIntegrity(decompressor, blobHasher, blobInfo.Hash) err = v.verifyBlobFinalIntegrity(hashedStream, plaintextHasher, blobInfo.Hash)
if err != nil { if err != nil {
return err return err
} }
@@ -470,14 +473,13 @@ func (v *Vaultik) verifyBlobChunks(
} }
// verifyBlobFinalIntegrity checks that no trailing data exists in the // verifyBlobFinalIntegrity checks that no trailing data exists in the
// decompressed stream and that the encrypted blob hash matches the // decompressed stream and that the blob hash matches the expected value.
// expected value.
func (v *Vaultik) verifyBlobFinalIntegrity( func (v *Vaultik) verifyBlobFinalIntegrity(
decompressor io.Reader, blobHasher hash.Hash, expectedHash string, plaintext io.Reader, plaintextHasher hash.Hash, expectedHash string,
) error { ) error {
// Verify no remaining data in blob - if the chunk list is accurate, // Verify no remaining data in blob - if the chunk list is accurate,
// the blob should be fully consumed. // the blob should be fully consumed.
remaining, err := io.Copy(io.Discard, decompressor) remaining, err := io.Copy(io.Discard, plaintext)
if err != nil { if err != nil {
return fmt.Errorf("failed to check for remaining blob data: %w", err) return fmt.Errorf("failed to check for remaining blob data: %w", err)
} }
@@ -486,8 +488,11 @@ func (v *Vaultik) verifyBlobFinalIntegrity(
return fmt.Errorf("%w: %d bytes", errTrailingBlobData, remaining) return fmt.Errorf("%w: %d bytes", errTrailingBlobData, remaining)
} }
// Verify blob hash matches the encrypted data we downloaded // The blob hash is the double SHA256 of its plaintext content.
calculatedBlobHash := hex.EncodeToString(blobHasher.Sum(nil)) firstHash := plaintextHasher.Sum(nil)
secondHash := sha256.Sum256(firstHash)
calculatedBlobHash := hex.EncodeToString(secondHash[:])
if calculatedBlobHash != expectedHash { if calculatedBlobHash != expectedHash {
return fmt.Errorf("%w: calculated %s, expected %s", return fmt.Errorf("%w: calculated %s, expected %s",
errBlobHashMismatch, calculatedBlobHash, expectedHash) errBlobHashMismatch, calculatedBlobHash, expectedHash)
+16 -1
View File
@@ -56,8 +56,23 @@ main() {
docker build --output=type=cacheonly \ docker build --output=type=cacheonly \
--build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint . --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .
# Version, commit and build date are computed here on the host, the
# same way script/docker does, and passed into the product build so
# the CI-built image reports its real source. The build context
# excludes .git (see .dockerignore), so the build cannot derive them
# itself; without these it would stamp the Dockerfile's dev/unknown
# fallbacks. VERSION comes from script/version, the source of truth
# shared with the Makefile.
version="$("$ROOT/script/version")"
commit="$(git rev-parse HEAD 2>/dev/null || echo unknown)"
commit_date="$(git show -s --format=%cs HEAD 2>/dev/null || echo unknown)"
epoch="$(date +%s%N)$$" epoch="$(date +%s%N)$$"
docker build --build-arg CHECK_EPOCH="$epoch" . docker build --build-arg CHECK_EPOCH="$epoch" \
--build-arg VERSION="$version" \
--build-arg COMMIT="$commit" \
--build-arg COMMIT_DATE="$commit_date" \
.
} }
main "$@" main "$@"
+17
View File
@@ -24,7 +24,24 @@ main() {
# whether the tree is clean. The Dockerfile now refuses to build # whether the tree is clean. The Dockerfile now refuses to build
# without a non-empty value, so this is required, not optional. # without a non-empty value, so this is required, not optional.
epoch="$(date +%s%N)$$" epoch="$(date +%s%N)$$"
# Version, commit and build date are computed here on the host,
# where .git exists, and passed into the build. The build context
# excludes .git (see .dockerignore), so the container cannot derive
# them itself -- it used to try and always got "unknown", giving
# every image a "commit: unknown" it could not be traced from.
# VERSION comes from script/version, the source of truth shared with
# the Makefile, so a Docker build reports the same string (tag,
# dev-<sha>, or a -dirty variant) that a local build of the same
# tree would.
version="$("$SCRIPT_DIR/version")"
commit="$(git rev-parse HEAD 2>/dev/null || echo unknown)"
commit_date="$(git show -s --format=%cs HEAD 2>/dev/null || echo unknown)"
docker build --build-arg CHECK_EPOCH="$epoch" \ docker build --build-arg CHECK_EPOCH="$epoch" \
--build-arg VERSION="$version" \
--build-arg COMMIT="$commit" \
--build-arg COMMIT_DATE="$commit_date" \
-t "$("$SCRIPT_DIR/projectname")" . -t "$("$SCRIPT_DIR/projectname")" .
} }