Accept a remote key for restore and verify, and document it (closes #124)

A host restoring after the original is gone has no local index and cannot
know a snapshot's human ID; `snapshot list` shows such snapshots only by
their remote key. Restore and verify now resolve an identifier to that
remote key: a human ID is hashed as before, and a remote key (or an
unambiguous leading part of it, as the table prints) is used directly,
resolved against the store's metadata listing. Deep verify reads the one
snapshot in the exported database rather than filtering by the human ID.

Adds an integration test that backs up with one index and hostname, then
lists, restores, and deep-verifies from the store with a fresh empty
index, a different hostname, and no age_recipients — comparing restored
bytes to the source. The empty index is what makes it fail if restore
ever needed the original one.

Adds a "Restoring on another machine" README section walking the flow end
to end, and drops the now-done roadmap item.

Model: claude-opus-4-8

ARCHITECTURE.md

@@ -63,7 +63,7 @@ A content-addressed unit of data. Files are split into variable-size chunks usin
 - `ChunkHash`: SHA256 hash of chunk content (primary key)
 - `Size`: Chunk size in bytes
 
-Chunk sizes vary between `avgChunkSize/4` and `avgChunkSize*4` (2.5MB-40MB for the 10MB default average).
+Chunk sizes vary between `avgChunkSize/4` and `avgChunkSize*4` (typically 16KB-256KB for 64KB average).
 
 #### FileChunk (`database.FileChunk`)
 Maps files to their constituent chunks:
@@ -120,7 +120,7 @@ The CLI uses fx for dependency injection. Here's the instantiation order:
 ```go
 // cli/app.go: NewApp()
 fx.New(
-    fx.Supply(config.Path(opts.ConfigPath)),         // 1. Config path
+    fx.Supply(config.ConfigPath(opts.ConfigPath)),  // 1. Config path
     fx.Supply(opts.LogOptions),                      // 2. Log options
     fx.Provide(globals.New),                         // 3. Globals
     fx.Provide(log.New),                             // 4. Logger config
@@ -193,7 +193,7 @@ scanner := v.ScannerFactory(snapshot.ScannerParams{
 - **Created by**: `chunker.NewChunker(avgChunkSize)`
 - **When**: Inside `snapshot.NewScanner()`
 - **Configuration**:
-  - `avgChunkSize`: From config (default 10MB)
+  - `avgChunkSize`: From config (typically 64KB)
   - `minChunkSize`: avgChunkSize / 4
   - `maxChunkSize`: avgChunkSize * 4
 

Dockerfile

@@ -20,6 +20,8 @@
 # golang:1.26.1-alpine, 2026-03-17
 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.
 # The sqlite driver is pure Go (modernc.org/sqlite), so no sqlite library or
 # CLI is required.
@@ -64,31 +66,8 @@ RUN [ -n "$CHECK_EPOCH" ] || exit 1
 RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check
 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)
-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
+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
 
 # Runtime stage
 # alpine:3.21, 2026-02-25

README.md

@@ -147,10 +147,10 @@ vaultik [--config <path>] config edit
 vaultik [--config <path>] config get <key>
 vaultik [--config <path>] config set <key> <value>
 vaultik [--config <path>] snapshot create [snapshot-names...] [--cron] [--prune] [--keep-newer-than <duration>]
-vaultik [--config <path>] snapshot list [--json]                  # alias: ls
+vaultik [--config <path>] snapshot list [--json]
 vaultik [--config <path>] snapshot verify <snapshot-id> [--deep] [--json]
 vaultik [--config <path>] snapshot purge [--keep-latest | --older-than <duration>] [--snapshot <name>...] [--force]
-vaultik [--config <path>] snapshot remove <snapshot-id> [--dry-run] [--force] [--local-only] [--json]  # alias: rm
+vaultik [--config <path>] snapshot remove <snapshot-id> [--dry-run] [--force] [--local-only] [--json]
 vaultik [--config <path>] snapshot restore <snapshot-id> <target-dir> [paths...] [--verify]
 vaultik [--config <path>] prune [--force] [--json]
 vaultik [--config <path>] info
@@ -169,23 +169,6 @@ vaultik version
 * `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
 * `--skip-errors`: Continue past per-file errors instead of aborting (applies to `snapshot create` and `restore`)
 
-### locking
-
-Commands that write persistent state — `snapshot create`, `snapshot
-purge`, `snapshot remove`, `prune`, and `remote nuke` — take a
-process-wide lock at `$XDG_DATA_HOME/vaultik/vaultik.pid`
-(`~/.local/share/vaultik/vaultik.pid` on Linux) for the whole run. Only
-one of them runs at a time: a second one exits immediately with an
-"already running" error rather than waiting, so two writers can never
-corrupt the local index or the destination store.
-
-Read-only commands — `info`, `snapshot list`, `snapshot verify`, and
-`remote info` — do not take the lock and are never blocked, so they run
-even while a backup is in progress. `snapshot restore` does not take the
-lock either: it writes only to the target directory you name, not the
-local index or the destination store. `config`, `database delete`,
-`completion`, and `version` do not take the lock.
-
 ### stdout and stderr
 
 Log output — everything from `--verbose` and `--debug`, and every
@@ -220,8 +203,6 @@ and `vaultik prune --json | jq .` both work as written.
 * `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`)
 * `VAULTIK_CONFIG`: Path to config file (overridden by `--config`)
 * `VAULTIK_INDEX_PATH`: Override local SQLite index path
-* `VAULTIK_CPUPROFILE`: Write a CPU profile to this path for the duration of the run (development/debugging)
-* `VAULTIK_MEMPROFILE`: Write a heap profile to this path when the run exits (development/debugging)
 
 ### shell completion
 
@@ -414,10 +395,6 @@ both are set.
 
 ## architecture
 
-For an implementation-level view of the internals — the data model, the
-`fx` dependency-injection wiring, and the scanner — see
-[`ARCHITECTURE.md`](ARCHITECTURE.md).
-
 ### remote storage layout
 
 ```
@@ -495,30 +472,25 @@ derivation.
 
 ### compression
 
-* zstd compression at configurable level (1-19, default 3). The level is
-  accepted as 1-19 but maps onto zstd's four internal speed presets:
-  1-2 fastest, 3-5 default, 6-9 better, 10-19 best. Levels within the
-  same band compress identically.
+* zstd compression at configurable level (1-19, default 3)
 * Applied before encryption at the blob level
 
 ---
 
 ## configuration reference
 
-Run `vaultik config init` to generate a fully commented config file; a
-complete annotated example also lives in
-[`config.example.yml`](config.example.yml). Key fields:
+Run `vaultik config init` to generate a fully commented config file.
+Key fields:
 
 | Field | Default | Description |
 |-------|---------|-------------|
 | `age_recipients` | (required) | Age public keys for encryption |
-| `age_secret_key` | (unset) | Age private key for decryption (`snapshot restore`, `snapshot verify --deep`). Setting it in the config file places the private key on the backed-up host, defeating the public-key-only design (see "why" above). Prefer the `VAULTIK_AGE_SECRET_KEY` environment variable, supplied only on the machine you restore from. |
 | `snapshots` | (required) | Named snapshot definitions with paths and excludes |
 | `storage_url` | | Storage backend URL (`s3://`, `file://`, `rclone://`) |
 | `s3.*` | | Legacy S3 configuration (endpoint, bucket, credentials) |
 | `exclude` | | Global exclude patterns (applied to all snapshots) |
 | `chunk_size` | `10MB` | Average chunk size for content-defined chunking |
-| `blob_size_limit` | `10GB` | Maximum blob size before splitting. Must be at least four times `chunk_size` (the largest chunk the chunker can emit), otherwise a single-chunk blob could exceed the limit |
+| `blob_size_limit` | `10GB` | Maximum blob size before splitting |
 | `compression_level` | `3` | zstd compression level (1-19) |
 | `hostname` | system hostname | Hostname used in snapshot IDs |
 | `index_path` | platform data dir | Local SQLite index path |
@@ -628,17 +600,9 @@ priority.
 
 ## output style
 
-The operational narration of the long-running commands — the Begin,
-Complete, Progress, and status lines of `snapshot create`, `prune`,
-`snapshot restore`, and the like — goes through helpers in `internal/ui`
-and conforms to the uniform style below. Some commands instead write
-plain text straight to stdout (`version`, `info`, `config`, the
-`database delete` prompt, and the `snapshot list` table); that output is
-unstyled and does not honor `--quiet`. Routing it through `internal/ui`
-is tracked in
-[issue #149](https://git.eeqj.de/sneak/vaultik/issues/149). Color is
-enabled when stdout is a TTY and the `NO_COLOR` environment variable is
-unset (https://no-color.org/).
+All user-facing output goes through helpers in `internal/ui` and conforms
+to a uniform style. Color is enabled when stdout is a TTY and the
+`NO_COLOR` environment variable is unset (https://no-color.org/).
 
 `internal/ui` writes to stdout; it is the output the user asked for.
 Structured log records are a different thing and go through

TODO.md

@@ -25,43 +25,6 @@ release" is exactly the contradiction
 
 # Completed Steps
 
-- 2026-09-21: Stopped an interrupted blob upload from making a later
-  backup deduplicate against data that was never stored
-  ([issue #148](https://git.eeqj.de/sneak/vaultik/issues/148)). The
-  packer commits a blob's `chunks`, `blob_chunks`, and `blobs` rows
-  before the upload is attempted, so a failed upload left chunk rows
-  behind and the next run skipped re-uploading them, producing a
-  snapshot that reported success but could not be restored. A run now
-  deduplicates only against chunks held by a blob whose `uploaded_ts` is
-  set, and at startup drops any un-uploaded blob rows (and the chunks
-  they orphan) so the affected data is re-chunked and re-uploaded. Blobs
-  recorded with no remote backend are marked uploaded so this invariant
-  holds uniformly.
-
-- 2026-09-22: Made restore refuse any snapshot path that would write
-  outside the target directory
-  ([issue #154](https://git.eeqj.de/sneak/vaultik/issues/154)).
-  `restoreFile` and `verifyRestoredFiles` joined the stored path onto the
-  target with no containment check, so a `..` segment or an absolute path
-  escaped the target and a restored symlink could redirect a later child
-  write anywhere on disk. Every stored path is now rejected unless
-  `filepath.IsLocal` accepts it with the leading separator removed, and
-  each existing ancestor directory below the target is `Lstat`ed to refuse
-  descending through a symlink; honest symlinks pointing outside the tree
-  are still written verbatim. age decryption proves a snapshot is
-  readable, not honest, and restore usually runs as root.
-
-- 2026-09-21: Stopped `--json` from silencing stderr diagnostics
-  ([issue #112](https://git.eeqj.de/sneak/vaultik/issues/112)). `--json`
-  used to be folded into `Quiet`, which pinned the log level to `WARN`,
-  so `prune --json` gave a machine consumer no record of the local index
-  rows it deleted even under `--verbose`. `--json` now quiets only the
-  stdout UI (the JSON document must stay clean, per
-  [issue #108](https://git.eeqj.de/sneak/vaultik/issues/108)); the stderr
-  log level follows `--verbose`/`--debug` again. The coupling was
-  removed the same way for `snapshot verify`, `snapshot remove`, and
-  `remote info`, which carried it for the same outdated reason.
-
 - 2026-09-21: Stopped `prune` from reporting a failed row count as 0
   ([issue #96](https://git.eeqj.de/sneak/vaultik/issues/96)). The seven
   `getTableCount` reads in `PruneDatabase` discarded their error, so a

cmd/vaultik/dockerversion_test.go

@@ -1,102 +0,0 @@
-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)
-}

cmd/vaultik/lintdocker_test.go

@@ -304,14 +304,10 @@ func instructionText(contents string) string {
 }
 
 // indexOf returns the position of the first instruction equal to, or
-// 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.
+// beginning with, want; -1 if there is none.
 func indexOf(found []string, want string) int {
 	for i, instruction := range found {
-		if instruction == want ||
-			strings.HasPrefix(instruction, want+" ") ||
-			strings.HasPrefix(instruction, want+"=") {
+		if instruction == want || strings.HasPrefix(instruction, want+" ") {
 			return i
 		}
 	}

cmd/vaultik/main.go

@@ -10,16 +10,6 @@ import (
 )
 
 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
 	if cpuProfile := os.Getenv("VAULTIK_CPUPROFILE"); cpuProfile != "" {
 		f, err := os.Create(cpuProfile) //nolint:gosec // G304: operator-set path
@@ -56,5 +46,5 @@ func run() int {
 		}()
 	}
 
-	return cli.Entry()
+	cli.Entry()
 }

config.example.yml

@@ -304,8 +304,6 @@ storage_url: "rclone://las1stor1//srv/pool.2024.04/backups/heraklion"
 
 # Maximum blob size
 # Multiple chunks are packed into blobs up to this size
-# Must be at least four times chunk_size (the largest chunk the chunker can
-# emit); a smaller limit would let a single-chunk blob exceed it.
 # Supports: 1GB, 10G, 500MB, 1GiB, etc.
 # Default: 10GB
 #blob_size_limit: 10GB

docs/REPOSTRUCTURE.md

@@ -145,10 +145,10 @@ An observer cannot determine:
 ## Pruning Safety
 
 The prune operation is safe because:
-1. It keeps every blob listed in any snapshot's manifest and deletes only blobs that no manifest references
+1. It only deletes blobs not referenced in any manifest
 2. Manifests are unencrypted and can be read without keys
-3. If any manifest cannot be downloaded or decoded, prune deletes nothing and exits with an error, rather than treating that snapshot's blobs as unreferenced
-4. Prune requires exclusive access to the destination: running it during a concurrent backup can race a snapshot whose manifest is not yet written, so do not prune while a backup is in progress
+3. The operation compares the latest local DB snapshot with the latest S3 snapshot to ensure consistency
+4. Pruning will fail if these don't match, preventing accidental deletion of needed blobs
 
 ## Restoration Requirements
 

internal/chunker/chunker.go

@@ -33,10 +33,9 @@ type Chunker struct {
 	maxChunkSize int
 }
 
-// ChunkSizeSpread is the FastCDC-recommended factor between the average
+// chunkSizeSpread is the FastCDC-recommended factor between the average
 // chunk size and the minimum (avg/spread) and maximum (avg*spread) sizes.
-// The largest chunk the chunker can emit is therefore avg*ChunkSizeSpread.
-const ChunkSizeSpread = 4
+const chunkSizeSpread = 4
 
 // NewChunker creates a new chunker with the specified average chunk size.
 // The actual chunk sizes will vary between avgChunkSize/4 and avgChunkSize*4
@@ -46,8 +45,8 @@ func NewChunker(avgChunkSize int64) *Chunker {
 	// FastCDC recommends min = avg/4 and max = avg*4
 	return &Chunker{
 		avgChunkSize: int(avgChunkSize),
-		minChunkSize: int(avgChunkSize / ChunkSizeSpread),
-		maxChunkSize: int(avgChunkSize * ChunkSizeSpread),
+		minChunkSize: int(avgChunkSize / chunkSizeSpread),
+		maxChunkSize: int(avgChunkSize * chunkSizeSpread),
 	}
 }
 

internal/cli/app.go

@@ -11,7 +11,6 @@ import (
 	"os/signal"
 	"path/filepath"
 	"strings"
-	"sync"
 	"syscall"
 	"time"
 
@@ -33,33 +32,14 @@ import (
 // may take before we give up.
 const shutdownTimeout = 30 * time.Second
 
-// lockMode says whether a command mutates persistent state — the local
-// index database or the remote store — and so must hold the process-wide
-// PID lock, or only reads that state and may run alongside a mutator.
-type lockMode int
-
-const (
-	// mutating commands (snapshot create, snapshot purge, snapshot remove,
-	// prune, remote nuke) write the local index or the remote store. They
-	// hold the PID lock so that at most one runs at a time.
-	mutating lockMode = iota
-	// readOnly commands (info, snapshot list, snapshot verify, remote info,
-	// snapshot restore) do not write the local index or the remote store,
-	// so they run without the lock and are never blocked by a running
-	// mutator. restore writes only to the target directory it is given.
-	readOnly
-)
-
-// AppOptions contains common options for creating and running the fx
-// application: the configuration file path, logging options, additional fx
-// modules and invocations, and whether the command mutates persistent
-// state (which decides whether it takes the PID lock).
+// AppOptions contains common options for creating the fx application.
+// It includes the configuration file path, logging options, and additional
+// fx modules and invocations that should be included in the application.
 type AppOptions struct {
 	ConfigPath string
 	LogOptions log.Options
 	Modules    []fx.Option
 	Invokes    []fx.Option
-	Mode       lockMode
 }
 
 // setupGlobals records the startup time and, when an output-suppression
@@ -68,11 +48,6 @@ type AppOptions struct {
 // silenced — per the documented convention that --quiet suppresses
 // non-error output only. The startup banner is printed by Entry
 // before cobra parses arguments, gated by the same arg-level check.
-//
-// --json quiets the UI here too, because stdout then carries a JSON
-// document and human narration would corrupt it. Unlike Quiet it does
-// not lower the stderr log level (issue #112), so --verbose/--debug
-// still surface diagnostics alongside the document.
 func setupGlobals(
 	lc fx.Lifecycle, g *globals.Globals, v *vaultik.Vaultik, opts log.Options,
 ) {
@@ -80,7 +55,7 @@ func setupGlobals(
 		OnStart: func(_ context.Context) error {
 			g.StartTime = time.Now().UTC()
 
-			if opts.Cron || opts.Quiet || opts.JSON {
+			if opts.Cron || opts.Quiet {
 				v.UI.SetQuiet(true)
 			}
 
@@ -221,54 +196,52 @@ 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),
+// runVaultikApp runs the standard single-operation command lifecycle
+// shared by the list/purge/verify/remove/remote-info subcommands:
+// resolve the config, start the fx app, run op against the Vaultik
+// instance in a goroutine, report a failure prefixed with failMsg
+// (suppressed while suppressErrors is true, e.g. under --json), then
+// trigger shutdown. The operation is cancelled when the app stops.
+// extraQuiet is OR-ed into LogOptions.Quiet (e.g. --json output modes).
+func runVaultikApp(
+	cmd *cobra.Command, extraQuiet, suppressErrors bool,
+	failMsg string, op func(v *vaultik.Vaultik) error,
 ) error {
-	var (
-		mu     sync.Mutex
-		failed bool
-	)
+	configPath, err := ResolveConfigPath()
+	if err != nil {
+		return err
+	}
 
-	opts.Invokes = append(opts.Invokes,
+	rootFlags := GetRootFlags()
+
+	return RunWithApp(cmd.Context(), AppOptions{
+		ConfigPath: configPath,
+		LogOptions: log.Options{
+			Verbose: rootFlags.Verbose,
+			Debug:   rootFlags.Debug,
+			Quiet:   rootFlags.Quiet || extraQuiet,
+		},
+		Modules: []fx.Option{},
+		Invokes: []fx.Option{
 			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()
+							if err != nil {
+								if !errors.Is(err, context.Canceled) {
+									if !suppressErrors {
+										log.Error(failMsg, "error", err)
+										ReportErrorf("%s: %v", failMsg, err)
 									}
 
-						stopErr := v.Shutdowner.Shutdown()
-						if stopErr != nil {
-							log.Error("Failed to shutdown", "error", stopErr)
+									os.Exit(1)
+								}
+							}
+
+							err = v.Shutdowner.Shutdown()
+							if err != nil {
+								log.Error("Failed to shutdown", "error", err)
 							}
 						}()
 
@@ -280,106 +253,36 @@ func RunOperation(
 						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
-// shared by the snapshot list/purge/remove and remote nuke subcommands:
-// resolve the config, then run op against the Vaultik instance through
-// RunOperation, reporting a failure prefixed with failMsg (suppressed
-// while suppressErrors is true, e.g. under --json). mode says whether the
-// command takes the PID lock. jsonOutput marks a command whose stdout is a
-// JSON document: it quiets the UI but, unlike Quiet, leaves the stderr log
-// level alone.
-func runVaultikApp(
-	cmd *cobra.Command, mode lockMode, jsonOutput, suppressErrors bool,
-	failMsg string, op func(v *vaultik.Vaultik) error,
-) error {
-	configPath, err := ResolveConfigPath()
-	if err != nil {
-		return err
-	}
-
-	rootFlags := GetRootFlags()
-
-	return RunOperation(cmd.Context(), AppOptions{
-		ConfigPath: configPath,
-		LogOptions: log.Options{
-			Verbose: rootFlags.Verbose,
-			Debug:   rootFlags.Debug,
-			Quiet:   rootFlags.Quiet,
-			JSON:    jsonOutput,
+			}),
 		},
-		Mode: mode,
-	}, op, func(err error) {
-		if suppressErrors {
-			return
-		}
-
-		log.Error(failMsg, "error", err)
-		ReportErrorf("%s: %v", failMsg, err)
 	})
 }
 
 // RunWithApp is a helper that creates and runs an fx app with the given options.
 // It combines NewApp and RunApp into a single convenient function. This is the
 // preferred way to run CLI commands that need the full application context.
-// A mutating command takes the process-wide PID lock before starting so that
-// only one runs at a time; a read-only command runs without it and is not
-// blocked while a mutator holds the lock (opts.Mode).
+// It acquires a PID lock before starting to prevent concurrent instances.
 func RunWithApp(ctx context.Context, opts AppOptions) error {
-	release, err := acquireLockIfMutating(opts.Mode,
-		filepath.Join(xdg.DataHome, "vaultik"))
+	// Acquire PID lock to prevent concurrent instances
+	lockDir := filepath.Join(xdg.DataHome, "vaultik")
+
+	lock, err := pidlock.Acquire(lockDir)
 	if err != nil {
-		return err
+		if errors.Is(err, pidlock.ErrAlreadyRunning) {
+			return fmt.Errorf("cannot start: %w", err)
 		}
 
-	defer release()
+		return fmt.Errorf("failed to acquire lock: %w", err)
+	}
+
+	defer func() {
+		err := lock.Release()
+		if err != nil {
+			log.Warn("Failed to release PID lock", "error", err)
+		}
+	}()
 
 	app := NewApp(opts)
 
 	return RunApp(ctx, app)
 }
-
-// acquireLockIfMutating takes the process-wide PID lock in lockDir for a
-// mutating command and returns a function that releases it. A read-only
-// command takes no lock, so it returns a no-op release and is never blocked
-// while a mutator holds the lock. ErrAlreadyRunning (another mutator holds
-// the lock) is surfaced as a "cannot start" error.
-func acquireLockIfMutating(mode lockMode, lockDir string) (func(), error) {
-	if mode != mutating {
-		return func() {}, nil
-	}
-
-	lock, err := pidlock.Acquire(lockDir)
-	if err != nil {
-		if errors.Is(err, pidlock.ErrAlreadyRunning) {
-			return nil, fmt.Errorf("cannot start: %w", err)
-		}
-
-		return nil, fmt.Errorf("failed to acquire lock: %w", err)
-	}
-
-	return func() {
-		err := lock.Release()
-		if err != nil {
-			log.Warn("Failed to release PID lock", "error", err)
-		}
-	}, nil
-}

internal/cli/app_test.go

@@ -2,10 +2,7 @@ package cli //nolint:testpackage // needs access to unexported cleanStartupError
 
 import (
 	"errors"
-	"path/filepath"
 	"testing"
-
-	"sneak.berlin/go/vaultik/internal/pidlock"
 )
 
 func TestCleanStartupError(t *testing.T) {
@@ -56,42 +53,3 @@ func TestCleanStartupError(t *testing.T) {
 		})
 	}
 }
-
-// TestLockScopedToMutatingCommands proves the partition the PID lock now
-// enforces: a read-only command runs while a mutator holds the lock, and
-// two mutating commands still mutually exclude.
-func TestLockScopedToMutatingCommands(t *testing.T) {
-	t.Parallel()
-
-	lockDir := filepath.Join(t.TempDir(), "vaultik")
-
-	// A mutating command takes the process-wide lock.
-	releaseMutator, err := acquireLockIfMutating(mutating, lockDir)
-	if err != nil {
-		t.Fatalf("mutating command could not acquire lock: %v", err)
-	}
-
-	// A read-only command runs to completion even while the lock is held.
-	releaseReader, err := acquireLockIfMutating(readOnly, lockDir)
-	if err != nil {
-		t.Fatalf("read-only command was blocked by held lock: %v", err)
-	}
-
-	releaseReader()
-
-	// A second mutating command is refused while the first holds the lock.
-	_, err = acquireLockIfMutating(mutating, lockDir)
-	if !errors.Is(err, pidlock.ErrAlreadyRunning) {
-		t.Fatalf("second mutating command was not excluded, got: %v", err)
-	}
-
-	// Once the first mutator releases, another mutating command may run.
-	releaseMutator()
-
-	release, err := acquireLockIfMutating(mutating, lockDir)
-	if err != nil {
-		t.Fatalf("mutating command could not acquire released lock: %v", err)
-	}
-
-	release()
-}

internal/cli/config.go

@@ -192,8 +192,8 @@ storage_url: ""
 #   access_key_id: YOUR_ACCESS_KEY
 #   secret_access_key: YOUR_SECRET_KEY
 #   # region: us-east-1        # Default: us-east-1
+#   # use_ssl: true            # Default: true
 #   # part_size: 5MB           # Multipart upload part size. Default: 5MB
-#   # For the s3:// form, disable TLS with ?ssl=false in the URL, not use_ssl.
 
 # ─── OPTIONAL ────────────────────────────────────────────────────────────────
 
@@ -212,8 +212,6 @@ storage_url: ""
 # chunk_size: 10MB
 
 # Maximum blob size before splitting into a new blob.
-# Must be at least four times chunk_size (the largest chunk the chunker can
-# emit); a smaller limit would let a single-chunk blob exceed it.
 # Accepts: 1GB, 10G, 500MB, etc.
 # Default: 10GB
 # blob_size_limit: 10GB

internal/cli/entry.go

@@ -1,7 +1,6 @@
 package cli
 
 import (
-	"errors"
 	"io"
 	"os"
 	"strings"
@@ -20,11 +19,7 @@ const shortCommitLen = 12
 // flag is present in os.Args — see bannerSuppressedInArgs), executes the
 // root cobra command, and routes any returned error through the
 // ui.Writer so the user sees a properly formatted "🛑 ERROR:" line.
-//
-// 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 {
+func Entry() {
 	emitStartupBanner(os.Args[1:], os.Stdout)
 
 	rootCmd := NewRootCommand()
@@ -32,19 +27,9 @@ func Entry() int {
 
 	err := rootCmd.Execute()
 	if err != nil {
-		// An operation that ran inside the fx app has already reported
-		// 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())
+		os.Exit(1)
 	}
-
-		return 1
-	}
-
-	return 0
 }
 
 // emitStartupBanner writes the startup banner to w unless args (the

internal/cli/entry_banner_test.go

@@ -230,7 +230,7 @@ func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) {
 		programName, flagConfig, configPath, cmdSnapshot, cmdList, flagJSON,
 	}
 
-	stdout := captureProcessStdout(t, func() { _ = Entry() })
+	stdout := captureProcessStdout(t, Entry)
 
 	requireExactlyOneJSONDocument(t, stdout)
 

internal/cli/entry_prune_json_stderr_test.go

@@ -1,140 +0,0 @@
-package cli //nolint:testpackage // shares the prune fixtures and capture helpers
-
-import (
-	"bytes"
-	"io"
-	"os"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// staleRecordLogMessage is the local-cleanup audit line CleanupLocalSnapshots
-// logs for each stale record. It is exactly the signal issue #112 says a
-// machine consumer lost under --json: gated off stdout, and pinned below
-// the log level on stderr because --json used to force Quiet.
-const staleRecordLogMessage = "Removing stale local snapshot record"
-
-// TestEntryPruneJSONStderrHonoursVerbosity is the end-to-end regression
-// guard for issue #112. Under --json the log level must still follow
-// --verbose/--debug rather than being pinned to WARN, so the
-// local-cleanup records reach stderr under --verbose while stdout stays
-// exactly one JSON document; without --verbose they stay below the
-// level, as they do without --json.
-//
-// Both halves are asserted together on the same run, because the fix has
-// to keep the document clean (issue #108) while freeing stderr.
-//
-// Not parallel: it replaces os.Args, os.Stdout, os.Stderr and the xdg
-// globals.
-//
-//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
-func TestEntryPruneJSONStderrHonoursVerbosity(t *testing.T) {
-	for _, testCase := range []struct {
-		name         string
-		verbose      bool
-		wantOnStderr bool
-	}{
-		{
-			name:         "verbose json surfaces the cleanup record on stderr",
-			verbose:      true,
-			wantOnStderr: true,
-		},
-		{
-			name:         "json alone keeps the cleanup record below the level",
-			verbose:      false,
-			wantOnStderr: false,
-		},
-	} {
-		t.Run(testCase.name, func(t *testing.T) {
-			configPath := writeHermeticPruneConfig(t, true)
-
-			previousArgs := os.Args
-
-			t.Cleanup(func() {
-				os.Args = previousArgs
-				rootFlags = RootFlags{}
-			})
-
-			args := []string{
-				programName, flagConfig, configPath, cmdPrune, flagJSON,
-			}
-			if testCase.verbose {
-				args = append(args, "--verbose")
-			}
-
-			os.Args = args
-
-			stdout, stderr := captureProcessStdoutAndStderr(t,
-				func() { _ = Entry() })
-
-			// The document stays clean in both cases: freeing stderr must
-			// not regress issue #108.
-			requireExactlyOneJSONDocument(t, stdout)
-
-			if testCase.wantOnStderr {
-				assert.Contains(t, stderr, staleRecordLogMessage,
-					"--verbose --json must emit the cleanup record on stderr")
-				assert.Contains(t, stderr, stalePruneSnapshotID,
-					"the record must name the snapshot it removed")
-			} else {
-				assert.NotContains(t, stderr, staleRecordLogMessage,
-					"without --verbose the record stays below the log level")
-			}
-		})
-	}
-}
-
-// captureProcessStdoutAndStderr redirects both of the process's own
-// standard streams to pipes for the duration of fn and returns what was
-// written to each. The redirection is at the file-descriptor level
-// because the logger binds os.Stderr when it initializes inside fn, and
-// the JSON document reaches os.Stdout independently; the point is to see
-// where each actually lands.
-//
-// Not parallel-safe: os.Stdout and os.Stderr are process-global.
-func captureProcessStdoutAndStderr(t *testing.T, fn func()) (string, string) {
-	t.Helper()
-
-	outReader, outWriter, err := os.Pipe()
-	require.NoError(t, err)
-
-	errReader, errWriter, err := os.Pipe()
-	require.NoError(t, err)
-
-	previousOut, previousErr := os.Stdout, os.Stderr
-	os.Stdout, os.Stderr = outWriter, errWriter
-
-	capturedOut := drain(outReader)
-	capturedErr := drain(errReader)
-
-	fn()
-
-	os.Stdout, os.Stderr = previousOut, previousErr
-
-	require.NoError(t, outWriter.Close())
-	require.NoError(t, errWriter.Close())
-
-	out, errOut := <-capturedOut, <-capturedErr
-
-	require.NoError(t, outReader.Close())
-	require.NoError(t, errReader.Close())
-
-	return out, errOut
-}
-
-// drain copies a reader to a string on a goroutine and delivers the
-// result once the writer end is closed.
-func drain(reader io.Reader) <-chan string {
-	captured := make(chan string, 1)
-
-	go func() {
-		var buf bytes.Buffer
-
-		_, _ = io.Copy(&buf, reader)
-		captured <- buf.String()
-	}()
-
-	return captured
-}

internal/cli/entry_prune_json_test.go

@@ -81,7 +81,7 @@ func TestEntryPruneJSONStdoutIsExactlyOneDocument(t *testing.T) {
 				programName, flagConfig, configPath, cmdPrune, flagJSON,
 			}
 
-			stdout := captureProcessStdout(t, func() { _ = Entry() })
+			stdout := captureProcessStdout(t, Entry)
 
 			requireExactlyOneJSONDocument(t, stdout)
 

internal/cli/entry_status_test.go

@@ -1,58 +0,0 @@
-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)
-		})
-	}
-}

internal/cli/info.go

@@ -1,7 +1,12 @@
 package cli
 
 import (
+	"context"
+	"errors"
+	"os"
+
 	"github.com/spf13/cobra"
+	"go.uber.org/fx"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -28,19 +33,44 @@ func NewInfoCommand() *cobra.Command {
 			// Use the app framework
 			rootFlags := GetRootFlags()
 
-			return RunOperation(cmd.Context(), AppOptions{
+			return RunWithApp(cmd.Context(), AppOptions{
 				ConfigPath: configPath,
 				LogOptions: log.Options{
 					Verbose: rootFlags.Verbose,
 					Debug:   rootFlags.Debug,
 					Quiet:   rootFlags.Quiet,
 				},
-				Mode: readOnly,
-			}, func(v *vaultik.Vaultik) error {
-				return v.ShowInfo()
-			}, func(err error) {
+				Modules: []fx.Option{},
+				Invokes: []fx.Option{
+					fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) {
+						lc.Append(fx.Hook{
+							OnStart: func(_ context.Context) error {
+								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
+							},
+						})
+					}),
+				},
 			})
 		},
 	}

internal/cli/prune.go

@@ -1,7 +1,12 @@
 package cli
 
 import (
+	"context"
+	"errors"
+	"os"
+
 	"github.com/spf13/cobra"
+	"go.uber.org/fx"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -36,24 +41,51 @@ work (e.g. after a crashed backup or to reclaim storage).`,
 			// Use the app framework like other commands
 			rootFlags := GetRootFlags()
 
-			return RunOperation(cmd.Context(), AppOptions{
+			return RunWithApp(cmd.Context(), AppOptions{
 				ConfigPath: configPath,
 				LogOptions: log.Options{
 					Verbose: rootFlags.Verbose,
 					Debug:   rootFlags.Debug,
-					Quiet:   rootFlags.Quiet,
-					JSON:    opts.JSON,
+					Quiet:   rootFlags.Quiet || opts.JSON,
 				},
-				Mode: mutating,
-			}, func(v *vaultik.Vaultik) error {
-				return v.Prune(opts)
-			}, func(err error) {
-				if opts.JSON {
-					return
-				}
-
+				Modules: []fx.Option{},
+				Invokes: []fx.Option{
+					fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) {
+						lc.Append(fx.Hook{
+							OnStart: func(_ context.Context) error {
+								// 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)
+										}
+									}
+
+									// 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
+							},
+						})
+					}),
+				},
 			})
 		},
 	}

internal/cli/remote.go

@@ -1,9 +1,12 @@
 package cli
 
 import (
+	"context"
 	"errors"
+	"os"
 
 	"github.com/spf13/cobra"
+	"go.uber.org/fx"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -45,7 +48,7 @@ This is destructive and irreversible. Requires --force.`,
 				return errNukeNeedsForce
 			}
 
-			return runVaultikApp(cmd, mutating, false, false, "Remote nuke failed",
+			return runVaultikApp(cmd, false, false, "Remote nuke failed",
 				func(v *vaultik.Vaultik) error {
 					return v.NukeRemote(true)
 				})
@@ -80,24 +83,47 @@ func newRemoteInfoCommand() *cobra.Command {
 
 			rootFlags := GetRootFlags()
 
-			return RunOperation(cmd.Context(), AppOptions{
+			return RunWithApp(cmd.Context(), AppOptions{
 				ConfigPath: configPath,
 				LogOptions: log.Options{
 					Verbose: rootFlags.Verbose,
 					Debug:   rootFlags.Debug,
-					Quiet:   rootFlags.Quiet,
-					JSON:    jsonOutput,
+					Quiet:   rootFlags.Quiet || jsonOutput,
 				},
-				Mode: readOnly,
-			}, func(v *vaultik.Vaultik) error {
-				return v.RemoteInfo(jsonOutput)
-			}, func(err error) {
-				if jsonOutput {
-					return
-				}
-
+				Modules: []fx.Option{},
+				Invokes: []fx.Option{
+					fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) {
+						lc.Append(fx.Hook{
+							OnStart: func(_ context.Context) error {
+								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)
+										}
+									}
+
+									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
+							},
+						})
+					}),
+				},
 			})
 		},
 	}

internal/cli/snapshot.go

@@ -1,10 +1,13 @@
 package cli
 
 import (
+	"context"
 	"errors"
 	"fmt"
+	"os"
 
 	"github.com/spf13/cobra"
+	"go.uber.org/fx"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -83,8 +86,7 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
 			// Use the backup functionality from cli package
 			rootFlags := GetRootFlags()
 
-			// --cron suppression is wired through v.UI by setupGlobals.
-			return RunOperation(cmd.Context(), AppOptions{
+			return RunWithApp(cmd.Context(), AppOptions{
 				ConfigPath: configPath,
 				LogOptions: log.Options{
 					Verbose: rootFlags.Verbose,
@@ -92,12 +94,42 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
 					Cron:    opts.Cron,
 					Quiet:   rootFlags.Quiet,
 				},
-				Mode: mutating,
-			}, func(v *vaultik.Vaultik) error {
-				return v.CreateSnapshot(opts)
-			}, func(err error) {
+				Modules: []fx.Option{},
+				Invokes: []fx.Option{
+					fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) {
+						lc.Append(fx.Hook{
+							OnStart: func(_ context.Context) error {
+								// 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
+							},
+						})
+					}),
+				},
 			})
 		},
 	}
@@ -126,7 +158,7 @@ func newSnapshotListCommand() *cobra.Command {
 		Long:    "Lists all snapshots with their ID, timestamp, and compressed size",
 		Args:    cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			return runVaultikApp(cmd, readOnly, false, false,
+			return runVaultikApp(cmd, false, false,
 				"Failed to list snapshots",
 				func(v *vaultik.Vaultik) error {
 					return v.ListSnapshots(jsonOutput)
@@ -162,7 +194,7 @@ restrict the operation to specific snapshot names.`,
 				return errPurgeCriteriaBoth
 			}
 
-			return runVaultikApp(cmd, mutating, false, false,
+			return runVaultikApp(cmd, false, false,
 				"Failed to purge snapshots",
 				func(v *vaultik.Vaultik) error {
 					return v.PurgeSnapshotsWithOptions(opts)
@@ -205,24 +237,47 @@ func newSnapshotVerifyCommand() *cobra.Command {
 
 			rootFlags := GetRootFlags()
 
-			return RunOperation(cmd.Context(), AppOptions{
+			return RunWithApp(cmd.Context(), AppOptions{
 				ConfigPath: configPath,
 				LogOptions: log.Options{
 					Verbose: rootFlags.Verbose,
 					Debug:   rootFlags.Debug,
-					Quiet:   rootFlags.Quiet,
-					JSON:    opts.JSON,
+					Quiet:   rootFlags.Quiet || opts.JSON,
 				},
-				Mode: readOnly,
-			}, func(v *vaultik.Vaultik) error {
-				return v.VerifySnapshotWithOptions(snapshotID, opts)
-			}, func(err error) {
-				if opts.JSON {
-					return
-				}
-
+				Modules: []fx.Option{},
+				Invokes: []fx.Option{
+					fx.Invoke(func(v *vaultik.Vaultik, lc fx.Lifecycle) {
+						lc.Append(fx.Hook{
+							OnStart: func(_ context.Context) error {
+								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)
+										}
+									}
+
+									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
+							},
+						})
+					}),
+				},
 			})
 		},
 	}
@@ -261,7 +316,7 @@ To wipe the entire destination store and start over, use 'vaultik remote
 nuke --force' — it is the single supported entry point for that.`,
 		Args: requireSnapshotIDArg,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runVaultikApp(cmd, mutating, opts.JSON, opts.JSON,
+			return runVaultikApp(cmd, opts.JSON, opts.JSON,
 				"Failed to remove snapshot",
 				func(v *vaultik.Vaultik) error {
 					_, err := v.RemoveSnapshot(args[0], opts)

internal/cli/snapshot_restore.go

@@ -1,8 +1,16 @@
 package cli
 
 import (
+	"context"
+	"errors"
+	"os"
+
 	"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/storage"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
@@ -17,6 +25,15 @@ type RestoreOptions struct {
 	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
 func newSnapshotRestoreCommand() *cobra.Command {
 	opts := &RestoreOptions{}
@@ -64,8 +81,7 @@ Examples:
 	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 {
 	snapshotID := args[0]
 
@@ -74,31 +90,87 @@ func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error {
 		opts.Paths = args[restoreMinArgs:]
 	}
 
+	// Use unified config resolution
 	configPath, err := ResolveConfigPath()
 	if err != nil {
 		return err
 	}
 
+	// Use the app framework like other commands
 	rootFlags := GetRootFlags()
 
-	return RunOperation(cmd.Context(), AppOptions{
+	return RunWithApp(cmd.Context(), AppOptions{
 		ConfigPath: configPath,
 		LogOptions: log.Options{
 			Verbose: rootFlags.Verbose,
 			Debug:   rootFlags.Debug,
 			Quiet:   rootFlags.Quiet,
 		},
-		Mode: readOnly,
-	}, func(v *vaultik.Vaultik) error {
-		return v.Restore(&vaultik.RestoreOptions{
+		Modules: buildRestoreModules(),
+		Invokes: buildRestoreInvokes(snapshotID, opts),
+	})
+}
+
+// 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: rootFlags.SkipErrors,
-		})
-	}, func(err error) {
+							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
+				},
 			})
+		}),
+	}
 }

internal/config/config.go

@@ -16,7 +16,6 @@ import (
 	"github.com/adrg/xdg"
 	"go.uber.org/fx"
 	"gopkg.in/yaml.v3"
-	"sneak.berlin/go/vaultik/internal/chunker"
 	"sneak.berlin/go/vaultik/internal/log"
 )
 
@@ -42,9 +41,7 @@ var (
 		"at least one snapshot must be configured (see config.example.yml)")
 	errSnapshotNoPaths   = errors.New("snapshot must have at least one path")
 	errChunkSizeTooSmall = errors.New("chunk_size must be at least 1MB")
-	errBlobSizeTooSmall  = errors.New(
-		"blob_size_limit must be at least the largest chunk the chunker can " +
-			"emit (chunk_size times the FastCDC size spread)")
+	errBlobSizeTooSmall  = errors.New("blob_size_limit must be at least chunk_size")
 	errBadCompression    = errors.New("compression_level must be between 1 and 19")
 	errBadStorageScheme  = errors.New(
 		"storage_url must start with s3://, file://, or rclone://")
@@ -165,9 +162,7 @@ type S3Config struct {
 	AccessKeyID     string `yaml:"access_key_id"`
 	SecretAccessKey string `yaml:"secret_access_key"`
 	Region          string `yaml:"region"`
-	// UseSSL selects HTTPS for a scheme-less endpoint. Omitted (nil) means
-	// the default, TLS; set it to false only to force plain HTTP.
-	UseSSL   *bool `yaml:"use_ssl"`
+	UseSSL          bool   `yaml:"use_ssl"`
 	PartSize        Size   `yaml:"part_size"`
 }
 
@@ -294,11 +289,8 @@ func Load(path string) (*Config, error) {
 // - At least one snapshot must be configured with at least one path
 // - Storage must be configured (either storage_url or s3.* fields)
 // - Chunk size must be at least 1MB
-//   - Blob size limit must be at least the largest chunk the chunker can emit
-//     (chunk_size times chunker.ChunkSizeSpread), so a single-chunk blob never
-//     exceeds the configured limit
+// - Blob size limit must be at least the chunk size
 // - Compression level must be between 1 and 19
-//
 // Returns an error describing the first validation failure encountered.
 func (c *Config) Validate() error {
 	if len(c.AgeRecipients) == 0 {
@@ -325,13 +317,8 @@ func (c *Config) Validate() error {
 		return errChunkSizeTooSmall
 	}
 
-	// The chunker can emit chunks up to chunk_size * ChunkSizeSpread, and the
-	// packer places a single such chunk into an otherwise empty blob. A limit
-	// below that bound would let a blob exceed it, so reject it.
-	largestChunk := c.ChunkSize.Int64() * chunker.ChunkSizeSpread
-	if c.BlobSizeLimit.Int64() < largestChunk {
-		return fmt.Errorf("%w: need at least %d bytes",
-			errBlobSizeTooSmall, largestChunk)
+	if c.BlobSizeLimit.Int64() < c.ChunkSize.Int64() {
+		return errBlobSizeTooSmall
 	}
 
 	if c.CompressionLevel < minCompressionLevel ||

internal/config/config_test.go

@@ -1,12 +1,9 @@
 package config //nolint:testpackage // exercises unexported extractAgeSecretKey
 
 import (
-	"errors"
 	"os"
 	"path/filepath"
 	"testing"
-
-	"sneak.berlin/go/vaultik/internal/chunker"
 )
 
 const (
@@ -104,80 +101,6 @@ func TestConfigFromEnv(t *testing.T) {
 	}
 }
 
-// TestValidateBlobSizeLimit checks the blob_size_limit boundary: it must be at
-// least the largest chunk the chunker can emit (chunk_size times
-// chunker.ChunkSizeSpread), because the packer places a single such chunk into
-// an otherwise empty blob. A limit between chunk_size and that bound is rejected.
-func TestValidateBlobSizeLimit(t *testing.T) {
-	t.Parallel()
-
-	const chunkSize = Size(10 * 1024 * 1024) // 10MB
-
-	largestChunk := chunkSize.Int64() * chunker.ChunkSizeSpread
-
-	newConfig := func(blobLimit Size) *Config {
-		return &Config{
-			AgeRecipients:    []string{testSneakAgePublicKey},
-			Snapshots:        map[string]SnapshotConfig{"test": {Paths: []string{"/tmp/src"}}},
-			StorageURL:       "file:///tmp/vaultik-test-store",
-			ChunkSize:        chunkSize,
-			BlobSizeLimit:    blobLimit,
-			CompressionLevel: 3,
-		}
-	}
-
-	tests := []struct {
-		name      string
-		blobLimit Size
-		wantErr   bool
-	}{
-		{
-			name:      "at chunk_size but below largest chunk is rejected",
-			blobLimit: chunkSize,
-			wantErr:   true,
-		},
-		{
-			name:      "between chunk_size and largest chunk is rejected",
-			blobLimit: Size(chunkSize.Int64() * 2),
-			wantErr:   true,
-		},
-		{
-			name:      "one byte below largest chunk is rejected",
-			blobLimit: Size(largestChunk - 1),
-			wantErr:   true,
-		},
-		{
-			name:      "exactly at largest chunk is accepted",
-			blobLimit: Size(largestChunk),
-			wantErr:   false,
-		},
-		{
-			name:      "above largest chunk is accepted",
-			blobLimit: Size(largestChunk * 100),
-			wantErr:   false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			err := newConfig(tt.blobLimit).Validate()
-			if tt.wantErr {
-				if !errors.Is(err, errBlobSizeTooSmall) {
-					t.Fatalf("Validate() error = %v, want errBlobSizeTooSmall", err)
-				}
-
-				return
-			}
-
-			if err != nil {
-				t.Fatalf("Validate() unexpected error: %v", err)
-			}
-		})
-	}
-}
-
 // TestExtractAgeSecretKey tests extraction of AGE-SECRET-KEY from various inputs
 func TestExtractAgeSecretKey(t *testing.T) {
 	t.Parallel()

internal/database/blobs.go

@@ -208,30 +208,6 @@ func (r *BlobRepository) DeleteOrphaned(ctx context.Context) error {
 	return nil
 }
 
-// DeleteUnuploaded deletes blob rows whose upload never completed
-// (uploaded_ts IS NULL) and returns how many were removed. Their
-// blob_chunks rows are removed by the ON DELETE CASCADE foreign key.
-// A blob is only ever attached to a snapshot once its upload has been
-// recorded, so an un-uploaded blob is never referenced by a completed
-// snapshot: dropping it discards chunk rows that point at data which
-// was never stored remotely, so the affected content is re-chunked and
-// re-uploaded on the next run.
-func (r *BlobRepository) DeleteUnuploaded(ctx context.Context) (int64, error) {
-	query := `DELETE FROM blobs WHERE uploaded_ts IS NULL`
-
-	result, err := r.db.ExecWithLog(ctx, query)
-	if err != nil {
-		return 0, fmt.Errorf("deleting un-uploaded blobs: %w", err)
-	}
-
-	rowsAffected, _ := result.RowsAffected()
-	if rowsAffected > 0 {
-		log.Debug("Deleted un-uploaded blobs", "count", rowsAffected)
-	}
-
-	return rowsAffected, nil
-}
-
 // getOne fetches a single blob row matched on the given column, or
 // (nil, nil) when no row matches.
 func (r *BlobRepository) getOne(

internal/database/chunks_ext.go

@@ -7,32 +7,12 @@ import (
 
 // List returns every chunk in the index, ordered by chunk hash.
 func (r *ChunkRepository) List(ctx context.Context) ([]*Chunk, error) {
-	return r.list(ctx, `
+	query := `
 		SELECT chunk_hash, size
 		FROM chunks
 		ORDER BY chunk_hash
-	`)
-}
+	`
 
-// ListInUploadedBlobs returns the chunks that are stored in a blob whose
-// upload has completed (uploaded_ts set), ordered by chunk hash. These
-// are the only chunks a backup may safely deduplicate against: a chunk
-// recorded solely in a blob that was never uploaded refers to data that
-// is not in remote storage, so trusting it would silently drop that data
-// from later snapshots.
-func (r *ChunkRepository) ListInUploadedBlobs(ctx context.Context) ([]*Chunk, error) {
-	return r.list(ctx, `
-		SELECT DISTINCT c.chunk_hash, c.size
-		FROM chunks c
-		JOIN blob_chunks bc ON c.chunk_hash = bc.chunk_hash
-		JOIN blobs b ON bc.blob_id = b.id
-		WHERE b.uploaded_ts IS NOT NULL
-		ORDER BY c.chunk_hash
-	`)
-}
-
-// list runs a chunk-selecting query and scans the (chunk_hash, size) rows.
-func (r *ChunkRepository) list(ctx context.Context, query string) ([]*Chunk, error) {
 	rows, err := r.db.conn.QueryContext(ctx, query)
 	if err != nil {
 		return nil, fmt.Errorf("querying chunks: %w", err)

internal/log/module.go

@@ -14,18 +14,8 @@ var Module = fx.Module("log",
 )
 
 // New creates a new logger configuration from provided options.
-//
-// JSON is intentionally not carried into Config: a command emitting a
-// JSON document on stdout must keep its stderr log level under
-// --verbose/--debug, so --json must not lower it (issue #112). JSON
-// silences the stdout UI in setupGlobals instead.
 func New(opts Options) Config {
-	return Config{
-		Verbose: opts.Verbose,
-		Debug:   opts.Debug,
-		Cron:    opts.Cron,
-		Quiet:   opts.Quiet,
-	}
+	return Config(opts)
 }
 
 // Options are provided by the CLI.
@@ -34,8 +24,4 @@ type Options struct {
 	Debug   bool
 	Cron    bool
 	Quiet   bool
-	// JSON marks a command whose stdout carries a machine-readable
-	// document. It silences the human UI on stdout (see setupGlobals),
-	// but unlike Quiet it leaves the stderr log level alone.
-	JSON bool
 }

internal/snapshot/manifest_gen_test.go

@@ -1,64 +0,0 @@
-//nolint:testpackage // exercises the unexported generateBlobManifest
-package snapshot
-
-import (
-	"context"
-	"path/filepath"
-	"testing"
-	"time"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/types"
-)
-
-// TestGenerateBlobManifest_MissingBlobFails is the regression guard for
-// issue #157: a blob the snapshot references but that is absent from the
-// blobs table used to be logged and skipped, yielding a manifest with
-// fewer blobs than the snapshot needs. Since prune trusts the manifest
-// alone, that omitted blob would be deleted at the next prune. Manifest
-// generation must fail instead.
-func TestGenerateBlobManifest_MissingBlobFails(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	dbPath := filepath.Join(t.TempDir(), "snapshot.db")
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	// A real blob row satisfies the snapshot_blobs foreign key on
-	// blob_id; the snapshot then references a different, absent hash.
-	presentBlob := &database.Blob{
-		ID:        types.NewBlobID(),
-		Hash:      types.BlobHash("present-blob-hash"),
-		CreatedTS: time.Now().Truncate(time.Second),
-	}
-	require.NoError(t, repos.Blobs.Create(ctx, nil, presentBlob))
-
-	snap := &database.Snapshot{
-		ID:       "testhost_home_2026-05-01T00:00:00Z",
-		Hostname: "testhost",
-	}
-	require.NoError(t, repos.Snapshots.Create(ctx, nil, snap))
-	require.NoError(t, repos.Snapshots.AddBlob(ctx, nil,
-		snap.ID.String(), presentBlob.ID, types.BlobHash("absent-blob-hash")))
-
-	require.NoError(t, db.Close())
-
-	sm := &SnapshotManager{
-		config: &config.Config{CompressionLevel: 3},
-		fs:     afero.NewOsFs(),
-	}
-
-	_, err = sm.generateBlobManifest(ctx, dbPath, snap.ID.String())
-	require.Error(t, err, "manifest generation must fail on a missing blob")
-	assert.Contains(t, err.Error(), "absent-blob-hash")
-}

internal/snapshot/scanner.go

@@ -220,14 +220,7 @@ func (s *Scanner) Scan(
 		defer s.progress.Stop()
 	}
 
-	// Phase 0: Repair any state left by an interrupted previous run, then
-	// load known files and chunks from the database into memory for fast
-	// lookup.
-	err := s.repairInterruptedBlobs(ctx)
-	if err != nil {
-		return nil, err
-	}
-
+	// Phase 0: Load known files and chunks from database into memory for fast lookup
 	knownFiles, err := s.loadDatabaseState(ctx, path)
 	if err != nil {
 		return nil, err
@@ -324,38 +317,6 @@ func (s *Scanner) loadDatabaseState(
 	return knownFiles, nil
 }
 
-// repairInterruptedBlobs discards blob rows left by a previous run whose
-// upload never completed. Such a blob has its chunks, blob_chunks, and
-// blobs rows committed to the local index before the upload is attempted,
-// so a crash or dropped connection mid-upload leaves them behind while the
-// data never reaches remote storage. Deduplicating against those chunks on
-// a later run would produce a snapshot that reports success but cannot be
-// restored. Dropping the un-uploaded blobs (their blob_chunks cascade) and
-// then any chunks left unreferenced forces the affected data to be
-// re-chunked and re-uploaded this run. A blob is attached to a snapshot
-// only once its upload is recorded, so this never touches a completed
-// snapshot's data.
-func (s *Scanner) repairInterruptedBlobs(ctx context.Context) error {
-	removed, err := s.repos.Blobs.DeleteUnuploaded(ctx)
-	if err != nil {
-		return fmt.Errorf("removing un-uploaded blob records: %w", err)
-	}
-
-	if removed == 0 {
-		return nil
-	}
-
-	log.Warn("Discarded blob records from an interrupted previous run; "+
-		"their data will be re-uploaded", "blobs", removed)
-
-	err = s.repos.Chunks.DeleteOrphaned(ctx)
-	if err != nil {
-		return fmt.Errorf("removing orphaned chunks: %w", err)
-	}
-
-	return nil
-}
-
 // summarizeScanPhase calculates total size to process, updates progress tracking,
 // and prints the scan phase summary with file counts and sizes
 func (s *Scanner) summarizeScanPhase(
@@ -431,14 +392,11 @@ func (s *Scanner) loadKnownFiles(
 	return result, nil
 }
 
-// loadKnownChunks loads the chunk hashes safe to deduplicate against into
-// an in-memory map for fast lookup, avoiding per-chunk database queries
-// during file processing. Only chunks held by a blob whose upload
-// completed are loaded: a chunk left behind by an interrupted upload
-// refers to data that never reached remote storage, and deduplicating
-// against it would silently produce an unrestorable snapshot.
+// loadKnownChunks loads all known chunk hashes from the database into a
+// map for fast lookup. This avoids per-chunk database queries during file
+// processing.
 func (s *Scanner) loadKnownChunks(ctx context.Context) error {
-	chunks, err := s.repos.Chunks.ListInUploadedBlobs(ctx)
+	chunks, err := s.repos.Chunks.List(ctx)
 	if err != nil {
 		return fmt.Errorf("listing chunks: %w", err)
 	}
@@ -1443,17 +1401,7 @@ func (s *Scanner) finalizeProcessPhase(ctx context.Context, result *ScanResult)
 				return fmt.Errorf("parsing blob ID: %w", err)
 			}
 
-			// With no remote backend the blob's lifecycle ends here, so
-			// mark it uploaded in the same transaction that attaches it to
-			// the snapshot. This keeps the invariant that any blob a
-			// snapshot references has uploaded_ts set, so deduplication and
-			// interrupted-run repair treat these blobs as trustworthy.
 			err = s.repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
-				err := s.repos.Blobs.UpdateUploaded(ctx, tx, b.ID)
-				if err != nil {
-					return fmt.Errorf("marking blob uploaded: %w", err)
-				}
-
 				return s.repos.Snapshots.AddBlob(ctx, tx, s.snapshotID, blobID,
 					types.BlobHash(b.Hash))
 			})

internal/snapshot/snapshot.go

@@ -809,11 +809,6 @@ func (sm *SnapshotManager) copyFile(src, dst string) error {
 	return nil
 }
 
-// errBlobMissingFromDatabase means a snapshot references a blob that is
-// absent from the blobs table, so a complete manifest cannot be built.
-var errBlobMissingFromDatabase = errors.New(
-	"blob referenced by snapshot is not in the database")
-
 // generateBlobManifest creates a compressed JSON list of all blobs in the snapshot
 func (sm *SnapshotManager) generateBlobManifest(
 	ctx context.Context, dbPath string, snapshotID string,
@@ -844,27 +839,21 @@ func (sm *SnapshotManager) generateBlobManifest(
 	totalCompressedSize := int64(0)
 
 	for _, hash := range blobHashes {
-		// Every blob the snapshot references must appear in the manifest.
-		// Prune consults only the manifest to decide what is still in use,
-		// so silently dropping a blob here would let a later prune delete
-		// it while this snapshot still needs it. A lookup failure or a
-		// missing blob row therefore fails manifest generation.
 		blob, err := repos.Blobs.GetByHash(ctx, hash)
 		if err != nil {
-			return nil, fmt.Errorf("getting blob details for %s: %w", hash, err)
-		}
+			log.Warn("Failed to get blob details", "hash", hash, "error", err)
 
-		if blob == nil {
-			return nil, fmt.Errorf("%w: blob %s, snapshot %s",
-				errBlobMissingFromDatabase, hash, snapshotID)
+			continue
 		}
 
+		if blob != nil {
 			blobs = append(blobs, BlobInfo{
 				Hash:           hash,
 				CompressedSize: blob.CompressedSize,
 			})
 			totalCompressedSize += blob.CompressedSize
 		}
+	}
 
 	// Create manifest. SnapshotID in the unencrypted manifest is the
 	// double-SHA256 remote key (see RemoteSnapshotKey), not the human ID,

internal/storage/conformance_test.go

@@ -1,198 +0,0 @@
-package storage_test
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"io"
-	"reflect"
-	"sort"
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/storage"
-)
-
-// runStorerConformance is the shared Storer contract. Every backend that
-// can run in-process is expected to pass it: TestFileStorer runs it against
-// file://, TestS3Storer against s3://. A new backend inherits this coverage
-// by passing its own constructor, so the contract is defined once.
-//
-// It exercises the public Storer interface: round-trip, stat, list with
-// prefix filtering, overwrite, delete, delete-of-missing, and not-found on
-// Get and Stat. Each section takes its own fresh backend instance, so the
-// order of sections never matters and no section sees another's objects.
-func runStorerConformance(t *testing.T, newStorer func(*testing.T) storage.Storer) {
-	t.Helper()
-
-	conformanceRoundTrip(t, newStorer(t))
-	conformanceOverwrite(t, newStorer(t))
-	conformanceList(t, newStorer(t))
-	conformanceDelete(t, newStorer(t))
-	conformanceNotFound(t, newStorer(t))
-}
-
-// conformanceRoundTrip stores a nested key, then reads it back and stats it.
-func conformanceRoundTrip(t *testing.T, s storage.Storer) {
-	t.Helper()
-
-	ctx := context.Background()
-	key := "blobs/aa/bb/object.bin"
-	want := []byte("round-trip payload")
-
-	err := s.Put(ctx, key, bytes.NewReader(want))
-	if err != nil {
-		t.Fatalf("Put: %v", err)
-	}
-
-	got := getBytes(t, s, key)
-	if !bytes.Equal(got, want) {
-		t.Errorf("Get returned %q, want %q", got, want)
-	}
-
-	info, err := s.Stat(ctx, key)
-	if err != nil {
-		t.Fatalf("Stat: %v", err)
-	}
-
-	if info.Key != key {
-		t.Errorf("Stat key = %q, want %q", info.Key, key)
-	}
-
-	if info.Size != int64(len(want)) {
-		t.Errorf("Stat size = %d, want %d", info.Size, len(want))
-	}
-}
-
-// conformanceOverwrite checks that a second Put replaces the first.
-func conformanceOverwrite(t *testing.T, s storage.Storer) {
-	t.Helper()
-
-	ctx := context.Background()
-	key := "meta/snapshot.json"
-
-	err := s.Put(ctx, key, bytes.NewReader([]byte("first")))
-	if err != nil {
-		t.Fatalf("first Put: %v", err)
-	}
-
-	want := []byte("second and longer payload")
-
-	err = s.Put(ctx, key, bytes.NewReader(want))
-	if err != nil {
-		t.Fatalf("second Put: %v", err)
-	}
-
-	got := getBytes(t, s, key)
-	if !bytes.Equal(got, want) {
-		t.Errorf("after overwrite Get returned %q, want %q", got, want)
-	}
-}
-
-// conformanceList checks prefix filtering and the empty result for a
-// prefix that matches nothing.
-func conformanceList(t *testing.T, s storage.Storer) {
-	t.Helper()
-
-	ctx := context.Background()
-	keys := []string{"blobs/aa/one", "blobs/bb/two", "meta/three"}
-
-	for _, k := range keys {
-		err := s.Put(ctx, k, bytes.NewReader([]byte("data")))
-		if err != nil {
-			t.Fatalf("Put %q: %v", k, err)
-		}
-	}
-
-	if got := listSorted(t, s, ""); !reflect.DeepEqual(got, keys) {
-		t.Errorf("List(\"\") = %v, want %v", got, keys)
-	}
-
-	wantBlobs := []string{"blobs/aa/one", "blobs/bb/two"}
-	if got := listSorted(t, s, "blobs/"); !reflect.DeepEqual(got, wantBlobs) {
-		t.Errorf("List(\"blobs/\") = %v, want %v", got, wantBlobs)
-	}
-
-	if got := listSorted(t, s, "absent/"); len(got) != 0 {
-		t.Errorf("List(\"absent/\") = %v, want empty", got)
-	}
-}
-
-// conformanceDelete checks that Delete removes an object and that deleting
-// a missing key is not an error.
-func conformanceDelete(t *testing.T, s storage.Storer) {
-	t.Helper()
-
-	ctx := context.Background()
-	key := "blobs/cc/gone.bin"
-
-	err := s.Put(ctx, key, bytes.NewReader([]byte("temporary")))
-	if err != nil {
-		t.Fatalf("Put: %v", err)
-	}
-
-	err = s.Delete(ctx, key)
-	if err != nil {
-		t.Fatalf("Delete: %v", err)
-	}
-
-	_, err = s.Get(ctx, key)
-	if !errors.Is(err, storage.ErrNotFound) {
-		t.Errorf("Get after Delete error = %v, want ErrNotFound", err)
-	}
-
-	err = s.Delete(ctx, key)
-	if err != nil {
-		t.Errorf("Delete of missing key = %v, want nil", err)
-	}
-}
-
-// conformanceNotFound checks Get and Stat on an absent key.
-func conformanceNotFound(t *testing.T, s storage.Storer) {
-	t.Helper()
-
-	ctx := context.Background()
-	key := "never/written"
-
-	_, err := s.Get(ctx, key)
-	if !errors.Is(err, storage.ErrNotFound) {
-		t.Errorf("Get error = %v, want ErrNotFound", err)
-	}
-
-	_, err = s.Stat(ctx, key)
-	if !errors.Is(err, storage.ErrNotFound) {
-		t.Errorf("Stat error = %v, want ErrNotFound", err)
-	}
-}
-
-// getBytes reads a key fully and closes the reader.
-func getBytes(t *testing.T, s storage.Storer, key string) []byte {
-	t.Helper()
-
-	rc, err := s.Get(context.Background(), key)
-	if err != nil {
-		t.Fatalf("Get %q: %v", key, err)
-	}
-
-	defer func() { _ = rc.Close() }()
-
-	data, err := io.ReadAll(rc)
-	if err != nil {
-		t.Fatalf("read %q: %v", key, err)
-	}
-
-	return data
-}
-
-// listSorted returns the keys under a prefix in a stable order.
-func listSorted(t *testing.T, s storage.Storer, prefix string) []string {
-	t.Helper()
-
-	keys, err := s.List(context.Background(), prefix)
-	if err != nil {
-		t.Fatalf("List %q: %v", prefix, err)
-	}
-
-	sort.Strings(keys)
-
-	return keys
-}

internal/storage/faultstore/faultstore.go

@@ -1,209 +0,0 @@
-// Package faultstore provides a storage.Storer wrapper that injects
-// faults on demand, so tests can reproduce the failure modes a real
-// backend exhibits: an upload that fails partway, a backend that reports
-// success while storing nothing, and reads that return corrupt or
-// truncated bytes. It is the seam called for by the fault-injection
-// tests (sneak/vaultik issue 72) and is meant to be reused by future
-// tests rather than re-implemented per case.
-//
-// The wrapper delegates every method to the inner Storer. Two hooks
-// change that: OnPut decides the fate of each write, and OnGet decides
-// how each read's bytes are returned. Both are keyed by the object key,
-// so a test can fault only blobs, only metadata, or a single object.
-package faultstore
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"fmt"
-	"io"
-
-	"sneak.berlin/go/vaultik/internal/storage"
-)
-
-// ErrInjectedUpload is returned by a Put the OnPut hook chose to fail.
-var ErrInjectedUpload = errors.New("faultstore: injected upload failure")
-
-// PutAction is the disposition OnPut assigns to a write.
-type PutAction int
-
-const (
-	// PutNormal writes through to the inner Storer.
-	PutNormal PutAction = iota
-	// PutFail reads part of the stream, then fails without storing the
-	// object — a network upload that dies partway through.
-	PutFail
-	// PutSwallow reports success but stores nothing — a backend that
-	// lies about durability.
-	PutSwallow
-)
-
-// GetFault is how OnGet chooses to damage a read.
-type GetFault int
-
-const (
-	// GetNormal returns the stored bytes unchanged.
-	GetNormal GetFault = iota
-	// GetCorrupt flips a byte so the returned object no longer matches
-	// what was stored.
-	GetCorrupt
-	// GetTruncate returns a short read: the object's bytes cut off
-	// before the end.
-	GetTruncate
-)
-
-// Storer wraps an inner storage.Storer with fault-injection hooks. A
-// zero-valued hook means "no fault": construct with New and set only the
-// hook a test needs.
-type Storer struct {
-	inner storage.Storer
-
-	// OnPut, when set, is consulted before every Put and
-	// PutWithProgress with the object key.
-	OnPut func(key string) PutAction
-
-	// OnGet, when set, is consulted for every Get with the object key
-	// and damages the returned bytes accordingly.
-	OnGet func(key string) GetFault
-}
-
-// New wraps inner. inner must be non-nil.
-func New(inner storage.Storer) *Storer {
-	return &Storer{inner: inner}
-}
-
-// midStreamBytes is how far a PutFail reads before failing, enough to be
-// past the start of any real blob without depending on the blob's size.
-const midStreamBytes = 512
-
-// Put stores data unless OnPut faults the write.
-func (f *Storer) Put(ctx context.Context, key string, data io.Reader) error {
-	handled, err := f.injectPut(key, data)
-	if handled {
-		return err
-	}
-
-	return f.inner.Put(ctx, key, data)
-}
-
-// PutWithProgress stores data unless OnPut faults the write.
-func (f *Storer) PutWithProgress(
-	ctx context.Context, key string, data io.Reader,
-	size int64, progress storage.ProgressCallback,
-) error {
-	handled, err := f.injectPut(key, data)
-	if handled {
-		return err
-	}
-
-	return f.inner.PutWithProgress(ctx, key, data, size, progress)
-}
-
-// Get retrieves data, damaging it if OnGet faults the read.
-func (f *Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) {
-	rc, err := f.inner.Get(ctx, key)
-	if err != nil {
-		return nil, err
-	}
-
-	fault := GetNormal
-	if f.OnGet != nil {
-		fault = f.OnGet(key)
-	}
-
-	if fault == GetNormal {
-		return rc, nil
-	}
-
-	data, err := io.ReadAll(rc)
-	_ = rc.Close()
-
-	if err != nil {
-		return nil, err
-	}
-
-	return io.NopCloser(bytes.NewReader(damage(fault, data))), nil
-}
-
-// damage returns a faulted copy of the stored bytes. GetCorrupt flips a
-// byte in the middle so decryption authentication fails; GetTruncate
-// drops the final byte so the read ends short. Both are no-ops on empty
-// input, which cannot be damaged into something distinguishable.
-func damage(fault GetFault, data []byte) []byte {
-	out := make([]byte, len(data))
-	copy(out, data)
-
-	if len(out) == 0 {
-		return out
-	}
-
-	switch fault {
-	case GetCorrupt:
-		out[len(out)/2] ^= 0xff
-	case GetTruncate:
-		out = out[:len(out)-1]
-	case GetNormal:
-	}
-
-	return out
-}
-
-// Stat delegates unchanged.
-func (f *Storer) Stat(ctx context.Context, key string) (*storage.ObjectInfo, error) {
-	return f.inner.Stat(ctx, key)
-}
-
-// Delete delegates unchanged.
-func (f *Storer) Delete(ctx context.Context, key string) error {
-	return f.inner.Delete(ctx, key)
-}
-
-// List delegates unchanged.
-func (f *Storer) List(ctx context.Context, prefix string) ([]string, error) {
-	return f.inner.List(ctx, prefix)
-}
-
-// ListStream delegates unchanged.
-func (f *Storer) ListStream(
-	ctx context.Context, prefix string,
-) <-chan storage.ObjectInfo {
-	return f.inner.ListStream(ctx, prefix)
-}
-
-// Info delegates unchanged.
-func (f *Storer) Info() storage.Info {
-	return f.inner.Info()
-}
-
-func (f *Storer) putAction(key string) PutAction {
-	if f.OnPut == nil {
-		return PutNormal
-	}
-
-	return f.OnPut(key)
-}
-
-// injectPut handles the non-normal write dispositions. It reports
-// whether it handled the write and, if so, with what error.
-func (f *Storer) injectPut(key string, data io.Reader) (bool, error) {
-	switch f.putAction(key) {
-	case PutFail:
-		// Consume part of the stream so the failure lands mid-transfer,
-		// the way a dropped connection would, then error without
-		// storing anything.
-		_, _ = io.CopyN(io.Discard, data, midStreamBytes)
-
-		return true, fmt.Errorf("%w for %q", ErrInjectedUpload, key)
-	case PutSwallow:
-		// A lying backend still drains the request body, then keeps
-		// nothing.
-		_, _ = io.Copy(io.Discard, data)
-
-		return true, nil
-	case PutNormal:
-		return false, nil
-	default:
-		return false, nil
-	}
-}

internal/storage/file_backend_test.go

@@ -1,27 +0,0 @@
-package storage_test
-
-import (
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/storage"
-)
-
-// newFileStorer builds a file:// backend rooted at a fresh temp directory.
-//
-//nolint:ireturn // conformance runs against the Storer interface by design
-func newFileStorer(t *testing.T) storage.Storer {
-	t.Helper()
-
-	s, err := storage.NewFileStorer(t.TempDir())
-	if err != nil {
-		t.Fatalf("NewFileStorer: %v", err)
-	}
-
-	return s
-}
-
-// TestFileStorer runs the shared Storer contract against the file:// backend.
-func TestFileStorer(t *testing.T) {
-	t.Parallel()
-	runStorerConformance(t, newFileStorer)
-}

internal/storage/module.go

@@ -111,11 +111,10 @@ func storerFromParsedS3URL(parsed *URL, cfg *config.Config) (Storer, error) {
 func storerFromLegacyS3Config(cfg *config.Config) (Storer, error) {
 	endpoint := cfg.S3.Endpoint
 
-	// Ensure protocol is present. Absent an explicit use_ssl, default to TLS;
-	// plain HTTP only when use_ssl is written as false.
+	// Ensure protocol is present
 	if !strings.HasPrefix(endpoint, "http://") &&
 		!strings.HasPrefix(endpoint, "https://") {
-		if cfg.S3.UseSSL == nil || *cfg.S3.UseSSL {
+		if cfg.S3.UseSSL {
 			endpoint = "https://" + endpoint
 		} else {
 			endpoint = "http://" + endpoint

internal/storage/module_test.go

@@ -1,61 +0,0 @@
-package storage_test
-
-import (
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/storage"
-)
-
-// legacyS3Config returns a minimal s3.* (no storage_url) configuration with a
-// scheme-less endpoint. useSSL mirrors the config file: nil means the key is
-// omitted, a pointer means it was written explicitly.
-func legacyS3Config(useSSL *bool) *config.Config {
-	return &config.Config{
-		S3: config.S3Config{
-			Endpoint:        "s3.example.com",
-			Bucket:          "bucket",
-			AccessKeyID:     "key",
-			SecretAccessKey: "secret",
-			Region:          "us-east-1",
-			UseSSL:          useSSL,
-		},
-	}
-}
-
-// endpointScheme builds the storer from cfg and returns the scheme its
-// resolved endpoint carries (Info().Location is "endpoint/bucket").
-func endpointScheme(t *testing.T, cfg *config.Config) string {
-	t.Helper()
-
-	storer, err := storage.NewStorer(cfg)
-	if err != nil {
-		t.Fatalf("NewStorer: %v", err)
-	}
-
-	location := storer.Info().Location
-	switch {
-	case strings.HasPrefix(location, "https://"):
-		return "https"
-	case strings.HasPrefix(location, "http://"):
-		return "http"
-	default:
-		t.Fatalf("endpoint has no http(s) scheme: %q", location)
-
-		return ""
-	}
-}
-
-func TestLegacyS3SchemelessEndpointDefaultsToTLS(t *testing.T) {
-	t.Parallel()
-
-	if got := endpointScheme(t, legacyS3Config(nil)); got != "https" {
-		t.Errorf("use_ssl omitted: got %q scheme, want https", got)
-	}
-
-	no := false
-	if got := endpointScheme(t, legacyS3Config(&no)); got != "http" {
-		t.Errorf("use_ssl: false: got %q scheme, want http", got)
-	}
-}

internal/storage/rclone_test.go

@@ -1,58 +0,0 @@
-package storage_test
-
-import (
-	"context"
-	"errors"
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/storage"
-)
-
-// The rclone backend is a thin adapter over the rclone library: it turns a
-// (remote, path) pair into rclone's "remote:path" string, hands it to
-// rclone, and maps rclone's own results back to the Storer interface. What
-// can be tested in-process, without a configured remote or network, is that
-// adapter layer — how the arguments are shaped and how construction errors
-// are reported. The data-plane operations (Put/Get/List/Delete) are rclone's
-// own, exercised against a real provider (drive, s3-via-rclone, ...), which
-// needs a configured remote with credentials and network access and so is
-// out of reach of a unit test. The shared Storer conformance suite therefore
-// runs against the in-process file and s3 backends; the rclone backend
-// inherits that contract once a remote is configured.
-//
-// These tests use rclone's ":local:" on-the-fly backend, which addresses the
-// local filesystem directly without any configured remote, so construction
-// runs entirely in-process.
-
-// TestNewRcloneStorerConstruction checks that a valid remote constructs a
-// backend and that Info() reports the shaped "remote:path" location.
-//
-//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
-func TestNewRcloneStorerConstruction(t *testing.T) {
-	dir := t.TempDir()
-
-	s, err := storage.NewRcloneStorer(context.Background(), ":local", dir)
-	if err != nil {
-		t.Fatalf("NewRcloneStorer: %v", err)
-	}
-
-	// Info().Location is the "remote:path" string the adapter builds from
-	// its two arguments, so asserting it confirms the argument shaping.
-	want := ":local:" + dir
-	if got := s.Info().Location; got != want {
-		t.Errorf("Info().Location = %q, want %q", got, want)
-	}
-}
-
-// TestNewRcloneStorerUnknownRemote checks that a remote that is not in the
-// rclone config fails construction with the ErrRemoteNotFound sentinel,
-// rather than silently returning a backend pointed nowhere.
-//
-//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
-func TestNewRcloneStorerUnknownRemote(t *testing.T) {
-	_, err := storage.NewRcloneStorer(
-		context.Background(), "vaultik-no-such-remote", "path")
-	if !errors.Is(err, storage.ErrRemoteNotFound) {
-		t.Errorf("NewRcloneStorer error = %v, want ErrRemoteNotFound", err)
-	}
-}

internal/storage/s3_test.go

@@ -13,23 +13,18 @@ import (
 	"sneak.berlin/go/vaultik/internal/storage"
 )
 
-// s3TestBucket is the bucket created for each in-process S3 server.
-const s3TestBucket = "test-bucket"
-
-// newS3Storer builds an s3:// backend backed by a fresh in-process
-// S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem
-// over httptest) that internal/s3 and the not-found regression test use,
-// so no new mock or dependency is introduced. Each call gets its own
-// server, bucket, and client, so the conformance suite's per-section
-// instances stay isolated.
+// 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:ireturn // conformance runs against the Storer interface by design
-func newS3Storer(t *testing.T) storage.Storer {
-	t.Helper()
+//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(s3TestBucket)
+	err := backend.CreateBucket(bucket)
 	if err != nil {
 		t.Fatalf("create bucket: %v", err)
 	}
@@ -37,9 +32,11 @@ func newS3Storer(t *testing.T) storage.Storer {
 	srv := httptest.NewServer(gofakes3.New(backend).Server())
 	t.Cleanup(srv.Close)
 
-	client, err := s3.NewClient(context.Background(), s3.Config{
+	ctx := context.Background()
+
+	client, err := s3.NewClient(ctx, s3.Config{
 		Endpoint:        srv.URL,
-		Bucket:          s3TestBucket,
+		Bucket:          bucket,
 		AccessKeyID:     "test",
 		SecretAccessKey: "test",
 		Region:          "us-east-1",
@@ -48,28 +45,9 @@ func newS3Storer(t *testing.T) storage.Storer {
 		t.Fatalf("new client: %v", err)
 	}
 
-	return storage.NewS3Storer(client)
-}
+	storer := storage.NewS3Storer(client)
 
-// TestS3Storer runs the shared Storer contract against the s3:// backend,
-// so it is held to the same round-trip, list, delete, and not-found
-// behaviour as the file:// backend.
-func TestS3Storer(t *testing.T) {
-	t.Parallel()
-	runStorerConformance(t, newS3Storer)
-}
-
-// TestS3StorerMissingKeyMapsToErrNotFound pins the specific contract that a
-// missing object surfaces as storage.ErrNotFound rather than the raw AWS SDK
-// error. Without the mapping, errors.Is(err, storage.ErrNotFound) is false on
-// s3 and callers would branch differently per backend.
-func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
-	t.Parallel()
-
-	storer := newS3Storer(t)
-	ctx := context.Background()
-
-	_, err := storer.Get(ctx, "does-not-exist")
+	_, err = storer.Get(ctx, "does-not-exist")
 	if !errors.Is(err, storage.ErrNotFound) {
 		t.Errorf("Get on missing key: got %v, want ErrNotFound", err)
 	}

internal/storage/url_parse_test.go

@@ -1,110 +0,0 @@
-package storage_test
-
-import (
-	"errors"
-	"reflect"
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/storage"
-)
-
-// TestParseStorageURLValid checks that each supported scheme parses into
-// the expected fields, since those fields decide which backend is built.
-func TestParseStorageURLValid(t *testing.T) {
-	t.Parallel()
-
-	const bucket = "mybucket"
-
-	cases := []struct {
-		name string
-		raw  string
-		want *storage.URL
-	}{
-		{
-			name: "file absolute path",
-			raw:  "file:///var/backups/vaultik",
-			want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
-		},
-		{
-			name: "s3 bucket and prefix, ssl defaults on",
-			raw:  "s3://mybucket/backups/host",
-			want: &storage.URL{
-				Scheme: "s3", Bucket: bucket,
-				Prefix: "backups/host", UseSSL: true,
-			},
-		},
-		{
-			name: "s3 bucket only",
-			raw:  "s3://mybucket",
-			want: &storage.URL{Scheme: "s3", Bucket: bucket, UseSSL: true},
-		},
-		{
-			name: "s3 with endpoint, region, ssl off",
-			raw:  "s3://mybucket?endpoint=minio.example.com&region=us-west-2&ssl=false",
-			want: &storage.URL{
-				Scheme: "s3", Bucket: bucket,
-				Endpoint: "minio.example.com", Region: "us-west-2", UseSSL: false,
-			},
-		},
-		{
-			name: "rclone remote and path",
-			raw:  "rclone://gdrive/backups/host",
-			want: &storage.URL{
-				Scheme: "rclone", RcloneRemote: "gdrive", Prefix: "backups/host",
-			},
-		},
-		{
-			name: "rclone remote only",
-			raw:  "rclone://gdrive",
-			want: &storage.URL{Scheme: "rclone", RcloneRemote: "gdrive"},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			got, err := storage.ParseStorageURL(tc.raw)
-			if err != nil {
-				t.Fatalf("ParseStorageURL(%q) returned error: %v", tc.raw, err)
-			}
-
-			if !reflect.DeepEqual(got, tc.want) {
-				t.Errorf("ParseStorageURL(%q) = %+v, want %+v", tc.raw, got, tc.want)
-			}
-		})
-	}
-}
-
-// TestParseStorageURLErrors checks that empty, missing, and unknown-scheme
-// inputs fail with the documented sentinel errors instead of parsing to a
-// wrong destination.
-func TestParseStorageURLErrors(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name    string
-		raw     string
-		wantErr error
-	}{
-		{"empty url", "", storage.ErrEmptyStorageURL},
-		{"file empty path", "file://", storage.ErrEmptyFilePath},
-		{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
-		{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
-		{"rclone missing remote", "rclone://", storage.ErrMissingRemote},
-		{"unknown scheme", "gs://bucket/x", storage.ErrUnsupportedScheme},
-		{"no scheme", "/local/path", storage.ErrUnsupportedScheme},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := storage.ParseStorageURL(tc.raw)
-			if !errors.Is(err, tc.wantErr) {
-				t.Errorf("ParseStorageURL(%q) error = %v, want %v",
-					tc.raw, err, tc.wantErr)
-			}
-		})
-	}
-}

internal/vaultik/blob_fetch.go

@@ -18,13 +18,6 @@ import (
 // not match the expected double-SHA-256 hash.
 var errBlobHashMismatch = errors.New("blob hash mismatch")
 
-// errBlobNotFullyRead is returned when the verifying reader is closed
-// before its plaintext reached EOF. The hash can only be checked once
-// the whole stream has been read, so an early or short-read close must
-// fail rather than silently skip verification.
-var errBlobNotFullyRead = errors.New(
-	"blob closed before fully read; hash not verified")
-
 // hashVerifyReader wraps a blobgen.Reader and verifies the double-SHA-256 hash
 // of decrypted plaintext when Close is called. It reuses the hash that
 // blobgen.Reader already computes internally via its TeeReader, avoiding
@@ -45,18 +38,12 @@ func (h *hashVerifyReader) Read(p []byte) (int, error) {
 	return n, err
 }
 
-// Close closes the underlying readers and verifies the blob hash. The
-// hash check cannot be skipped: closing before the plaintext reached
-// EOF (a short read or an early close) is an error, so a caller can
-// never obtain unverified blob bytes.
+// Close verifies the hash (if the stream was fully read) and closes underlying readers.
 func (h *hashVerifyReader) Close() error {
 	readerErr := h.reader.Close()
 	fetcherErr := h.fetcher.Close()
 
-	if !h.done {
-		return errBlobNotFullyRead
-	}
-
+	if h.done {
 		firstHash := h.reader.Sum256()
 		secondHasher := sha256.New()
 		secondHasher.Write(firstHash)
@@ -66,6 +53,7 @@ func (h *hashVerifyReader) Close() error {
 			return fmt.Errorf("%w: expected %s, got %s",
 				errBlobHashMismatch, h.blobHash[:16], actualHashHex[:16])
 		}
+	}
 
 	if readerErr != nil {
 		return readerErr

internal/vaultik/blob_fetch_hash_test.go

@@ -133,51 +133,3 @@ func TestFetchAndDecryptBlobVerifiesHash(t *testing.T) {
 		}
 	})
 }
-
-// TestFetchAndDecryptBlobCloseBeforeEOFFails verifies the hash check
-// cannot be skipped: a caller that reads only part of the blob and then
-// closes gets an error rather than silently unverified bytes.
-func TestFetchAndDecryptBlobCloseBeforeEOFFails(t *testing.T) {
-	t.Parallel()
-
-	identity, err := age.GenerateX25519Identity()
-	if err != nil {
-		t.Fatalf("generating identity: %v", err)
-	}
-
-	plaintext := []byte("hello world test data for blob hash verification")
-	encryptedData, correctHash := buildHashTestBlob(t, identity, plaintext)
-
-	mockStorage := NewMockStorer()
-	blobPath := "blobs/" + correctHash[:2] + "/" +
-		correctHash[2:4] + "/" + correctHash
-
-	mockStorage.mu.Lock()
-	mockStorage.data[blobPath] = encryptedData
-	mockStorage.mu.Unlock()
-
-	tv := vaultik.NewForTesting(mockStorage)
-
-	rc, err := tv.FetchAndDecryptBlob(
-		context.Background(), correctHash, int64(len(encryptedData)), identity)
-	if err != nil {
-		t.Fatalf("unexpected error opening stream: %v", err)
-	}
-
-	// Read one byte, far short of the plaintext length, then close.
-	buf := make([]byte, 1)
-
-	_, err = rc.Read(buf)
-	if err != nil {
-		t.Fatalf("reading first byte: %v", err)
-	}
-
-	err = rc.Close()
-	if err == nil {
-		t.Fatal("expected error closing before EOF, got nil")
-	}
-
-	if !strings.Contains(err.Error(), "hash not verified") {
-		t.Fatalf("expected not-verified error, got: %v", err)
-	}
-}

internal/vaultik/fault_injection_test.go

@@ -1,647 +0,0 @@
-package vaultik_test
-
-import (
-	"context"
-	"errors"
-	"io"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/snapshot"
-	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/storage/faultstore"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// These tests cover the failure modes a backup tool must survive:
-// interrupted uploads, an interrupted metadata export, corrupt and
-// truncated reads, a full restore disk, and a backend that reports
-// success while storing nothing. Faults are injected through the
-// storage.Storer seam (internal/storage/faultstore), never by patching
-// production code. Each test asserts on the observable end state — what
-// is in the index, what is at the destination, what the user is told —
-// not merely that an error was returned. See
-// https://git.eeqj.de/sneak/vaultik/issues/72.
-//
-// Object-level write atomicity (no partial blob object left behind) is
-// covered by the file:// backend's atomic-write work
-// (https://git.eeqj.de/sneak/vaultik/issues/130) and is not re-tested
-// here; these tests target the layers above the backend.
-//
-// The tests run serially, not with t.Parallel: each calls
-// log.Initialize, which replaces the package-global logger, and a
-// backup or restore running concurrently reads that same logger. Under
-// -race the two collide. Running one at a time is the same choice
-// prune_count_test.go already makes for the same reason.
-
-const (
-	faultChunkSize   = int64(64 * 1024)
-	faultMaxBlobSize = int64(256 * 1024)
-)
-
-// faultTestConfig returns the config shared by the fault-injection
-// tests: a real recipient/secret keypair so blobs are genuinely
-// encrypted, and a blob size limit the restore sweeper can divide.
-func faultTestConfig() *config.Config {
-	return &config.Config{
-		AgeRecipients:    []string{testAgePublicKey},
-		AgeSecretKey:     testAgeSecretKey,
-		CompressionLevel: 3,
-		Hostname:         testHostname,
-		BlobSizeLimit:    config.Size(faultMaxBlobSize),
-	}
-}
-
-// writeFaultSourceTree writes a spread of file sizes that forces several
-// chunks across more than one blob, so a fault landing on a single blob
-// still leaves other data intact. Returns the expected content by path.
-func writeFaultSourceTree(
-	t *testing.T, fs afero.Fs, dataDir string,
-) map[string][]byte {
-	t.Helper()
-
-	files := map[string][]byte{
-		filepath.Join(dataDir, "small.txt"):    []byte("hello vaultik"),
-		filepath.Join(dataDir, "a.bin"):        bytesPattern("a-", int(faultChunkSize*3)),
-		filepath.Join(dataDir, "sub", "b.bin"): bytesPattern("b-", int(faultChunkSize*3)),
-		filepath.Join(dataDir, "sub", "c.bin"): bytesPattern("c-", int(faultChunkSize*2)),
-	}
-
-	for path, content := range files {
-		require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o755))
-		require.NoError(t, afero.WriteFile(fs, path, content, 0o644))
-	}
-
-	return files
-}
-
-// newFaultScanner builds a scanner writing through the given storer.
-func newFaultScanner(
-	fs afero.Fs, storer storage.Storer,
-	cfg *config.Config, repos *database.Repositories,
-) *snapshot.Scanner {
-	return snapshot.NewScanner(snapshot.ScannerConfig{
-		FS:               fs,
-		Storage:          storer,
-		ChunkSize:        faultChunkSize,
-		MaxBlobSize:      faultMaxBlobSize,
-		CompressionLevel: cfg.CompressionLevel,
-		AgeRecipients:    cfg.AgeRecipients,
-		Repositories:     repos,
-	})
-}
-
-// newFaultSnapshotManager builds a snapshot manager writing through the
-// given storer.
-func newFaultSnapshotManager(
-	fs afero.Fs, storer storage.Storer,
-	cfg *config.Config, repos *database.Repositories,
-) *snapshot.SnapshotManager {
-	sm := snapshot.NewSnapshotManager(snapshot.SnapshotManagerParams{
-		Repos:   repos,
-		Storage: storer,
-		Config:  cfg,
-	})
-	sm.SetFilesystem(fs)
-
-	return sm
-}
-
-// fullFaultBackup runs a complete backup (create, scan, complete,
-// export) through storer and returns the snapshot ID.
-func fullFaultBackup(
-	ctx context.Context, t *testing.T, fs afero.Fs, storer storage.Storer,
-	cfg *config.Config, repos *database.Repositories,
-	dataDir, dbPath, name string,
-) string {
-	t.Helper()
-
-	sm := newFaultSnapshotManager(fs, storer, cfg, repos)
-	scanner := newFaultScanner(fs, storer, cfg, repos)
-
-	id, err := sm.CreateSnapshotWithName(ctx, cfg.Hostname, name, "v", "g")
-	require.NoError(t, err)
-
-	_, err = scanner.Scan(ctx, dataDir, id)
-	require.NoError(t, err)
-
-	require.NoError(t, sm.CompleteSnapshot(ctx, id))
-	require.NoError(t, sm.ExportSnapshotMetadata(ctx, dbPath, id))
-
-	return id
-}
-
-// newReaderVaultik builds a Vaultik that reads (restore/verify) through
-// storer, with the given repositories (nil is fine for restore/verify,
-// which read metadata from storage).
-func newReaderVaultik(
-	ctx context.Context, cfg *config.Config, storer storage.Storer,
-	repos *database.Repositories, fs afero.Fs,
-) *vaultik.Vaultik {
-	v := &vaultik.Vaultik{
-		Config:       cfg,
-		Storage:      storer,
-		Repositories: repos,
-		Fs:           fs,
-		Stdout:       io.Discard,
-		Stderr:       io.Discard,
-		UI:           ui.NewWithColor(io.Discard, false),
-	}
-	v.SetContext(ctx)
-
-	return v
-}
-
-// Scenario 3: a stored blob's bytes are flipped before restore reads
-// them. Restore must fail loudly, and no file must be left on the
-// restore target holding corrupt content.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestRestoreRejectsCorruptBlob(t *testing.T) {
-	assertRestoreRejectsDamagedBlob(t, faultstore.GetCorrupt, "corrupt")
-}
-
-// Scenario 4: a stored blob is truncated before restore reads it. Same
-// contract as the corrupt case.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestRestoreRejectsTruncatedBlob(t *testing.T) {
-	assertRestoreRejectsDamagedBlob(t, faultstore.GetTruncate, "truncated")
-}
-
-// assertRestoreRejectsDamagedBlob backs up the source tree, then restores
-// through a store that damages every blob read with the given fault, and
-// asserts restore fails naming a blob and leaves no file on the target
-// holding wrong bytes. Metadata reads are returned intact so the failure
-// is isolated to the blob.
-func assertRestoreRejectsDamagedBlob(
-	t *testing.T, fault faultstore.GetFault, name string,
-) {
-	t.Helper()
-	log.Initialize(log.Config{})
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-	testFiles := writeFaultSourceTree(t, fs, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	id := fullFaultBackup(ctx, t, fs, inner, cfg, repos, dataDir, dbPath, name)
-	require.NoError(t, db.Close())
-
-	faultStore := faultstore.New(inner)
-	faultStore.OnGet = func(key string) faultstore.GetFault {
-		if strings.HasPrefix(key, "blobs/") {
-			return fault
-		}
-
-		return faultstore.GetNormal
-	}
-
-	v := newReaderVaultik(ctx, cfg, faultStore, nil, fs)
-	err = v.Restore(&vaultik.RestoreOptions{SnapshotID: id, TargetDir: restoreDir})
-
-	require.Error(t, err, "restore must fail on a damaged blob")
-	assert.Contains(t, err.Error(), "blob",
-		"error should name the blob that failed")
-	assertNoCorruptFiles(t, fs, restoreDir, testFiles)
-}
-
-// Scenario 6: the backend accepts blob uploads and reports success but
-// stores nothing. verify --deep must catch it.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestDeepVerifyCatchesLyingBackend(t *testing.T) {
-	log.Initialize(log.Config{})
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-
-	writeFaultSourceTree(t, fs, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	// Blob uploads are swallowed; metadata uploads land, so verify can
-	// download the manifest and database and then discover the blobs are
-	// absent.
-	lying := faultstore.New(inner)
-	lying.OnPut = func(key string) faultstore.PutAction {
-		if strings.HasPrefix(key, "blobs/") {
-			return faultstore.PutSwallow
-		}
-
-		return faultstore.PutNormal
-	}
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	id := fullFaultBackup(ctx, t, fs, lying, cfg, repos, dataDir, dbPath, "lying")
-	require.NoError(t, db.Close())
-
-	// No blob objects were actually written.
-	blobKeys, err := inner.List(ctx, "blobs/")
-	require.NoError(t, err)
-	assert.Empty(t, blobKeys, "lying backend should have stored no blobs")
-
-	// Read back through the honest underlying store.
-	v := newReaderVaultik(ctx, cfg, inner, nil, fs)
-	err = v.VerifySnapshotWithOptions(id, &vaultik.VerifyOptions{Deep: true})
-	require.Error(t, err, "deep verify must catch a backend that stored nothing")
-}
-
-// Scenario 1a: a blob upload fails partway through. The interrupted run
-// must not record the blob as uploaded, must not reference it from the
-// snapshot, and must leave no blob object at the destination.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestInterruptedBlobUploadRecordsNoUploadedBlob(t *testing.T) {
-	log.Initialize(log.Config{})
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-
-	writeFaultSourceTree(t, fs, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	defer func() { _ = db.Close() }()
-
-	repos := database.NewRepositories(db)
-
-	// Every blob upload fails partway through. The scan must surface it.
-	fault := faultstore.New(inner)
-	fault.OnPut = func(key string) faultstore.PutAction {
-		if strings.HasPrefix(key, "blobs/") {
-			return faultstore.PutFail
-		}
-
-		return faultstore.PutNormal
-	}
-
-	sm := newFaultSnapshotManager(fs, fault, cfg, repos)
-	scanner := newFaultScanner(fs, fault, cfg, repos)
-
-	id, err := sm.CreateSnapshotWithName(ctx, cfg.Hostname, "interrupted", "v", "g")
-	require.NoError(t, err)
-
-	_, err = scanner.Scan(ctx, dataDir, id)
-	require.Error(t, err, "scan must fail when a blob upload fails")
-
-	// No blob may claim to be uploaded.
-	blobs, err := repos.Blobs.GetAll(ctx)
-	require.NoError(t, err)
-
-	for _, b := range blobs {
-		assert.Nilf(t, b.UploadedTS,
-			"blob %s marked uploaded after a failed upload", b.Hash)
-	}
-
-	// The snapshot may reference no blobs, and the destination holds none.
-	hashes, err := repos.Snapshots.GetBlobHashes(ctx, id)
-	require.NoError(t, err)
-	assert.Empty(t, hashes, "interrupted snapshot must reference no blobs")
-
-	blobKeys, err := inner.List(ctx, "blobs/")
-	require.NoError(t, err)
-	assert.Empty(t, blobKeys, "no blob object may survive at the destination")
-}
-
-// Scenario 1b: after an interrupted upload, a retry on the same local
-// index must produce a restorable snapshot. The interrupted run leaves
-// the blob's chunk rows in the index; the fix for
-// https://git.eeqj.de/sneak/vaultik/issues/148 discards those un-uploaded
-// blob rows at the start of the next scan and deduplicates only against
-// chunks in a blob that was actually uploaded, so the retry re-chunks and
-// re-uploads the affected data instead of silently referencing data that
-// never reached storage.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestBackupRetryAfterInterruptedUploadIsRestorable(t *testing.T) {
-	log.Initialize(log.Config{})
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-	testFiles := writeFaultSourceTree(t, fs, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	// Attempt 1: every blob upload fails.
-	fault := faultstore.New(inner)
-	fault.OnPut = func(key string) faultstore.PutAction {
-		if strings.HasPrefix(key, "blobs/") {
-			return faultstore.PutFail
-		}
-
-		return faultstore.PutNormal
-	}
-
-	sm := newFaultSnapshotManager(fs, fault, cfg, repos)
-	scanner := newFaultScanner(fs, fault, cfg, repos)
-
-	id1, err := sm.CreateSnapshotWithName(ctx, cfg.Hostname, "interrupted", "v", "g")
-	require.NoError(t, err)
-
-	_, err = scanner.Scan(ctx, dataDir, id1)
-	require.Error(t, err)
-
-	// Retry on the same local index with a working backend.
-	id2 := fullFaultBackup(ctx, t, fs, inner, cfg, repos, dataDir, dbPath, "retry")
-	require.NoError(t, db.Close())
-
-	v := newReaderVaultik(ctx, cfg, inner, nil, fs)
-	require.NoError(t, v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: id2,
-		TargetDir:  restoreDir,
-		Verify:     true,
-	}), "retry after an interrupted upload must produce a restorable snapshot")
-
-	assertRestoredTree(t, fs, restoreDir, testFiles)
-}
-
-// Scenario 2: the process dies during the metadata export, after the
-// database is uploaded but before the manifest. The destination is left
-// with blobs and a database but no manifest. verify and snapshot list
-// must report the damage honestly rather than crashing or passing.
-// Automatic detection and repair of this partial state on the next run
-// is tracked in https://git.eeqj.de/sneak/vaultik/issues/177 and is not
-// asserted here.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestBackupSurvivesMetadataExportInterruption(t *testing.T) {
-	log.Initialize(log.Config{})
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-
-	writeFaultSourceTree(t, fs, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	// Back up and complete with a working backend.
-	sm := newFaultSnapshotManager(fs, inner, cfg, repos)
-	scanner := newFaultScanner(fs, inner, cfg, repos)
-
-	id, err := sm.CreateSnapshotWithName(ctx, cfg.Hostname, "export", "v", "g")
-	require.NoError(t, err)
-
-	_, err = scanner.Scan(ctx, dataDir, id)
-	require.NoError(t, err)
-	require.NoError(t, sm.CompleteSnapshot(ctx, id))
-
-	// Export through a backend that fails only the manifest upload. The
-	// database uploads first and lands; the manifest does not.
-	fault := faultstore.New(inner)
-	fault.OnPut = func(key string) faultstore.PutAction {
-		if strings.HasSuffix(key, "manifest.json.zst") {
-			return faultstore.PutFail
-		}
-
-		return faultstore.PutNormal
-	}
-
-	smFault := newFaultSnapshotManager(fs, fault, cfg, repos)
-
-	err = smFault.ExportSnapshotMetadata(ctx, dbPath, id)
-	require.Error(t, err, "export must fail when the manifest upload fails")
-
-	// The destination is in the partial state the scenario describes.
-	key := snapshot.RemoteSnapshotKey(id)
-
-	_, err = inner.Stat(ctx, "metadata/"+key+"/db.zst.age")
-	require.NoError(t, err, "database should have been uploaded before the manifest")
-
-	_, err = inner.Stat(ctx, "metadata/"+key+"/manifest.json.zst")
-	require.ErrorIs(t, err, storage.ErrNotFound, "manifest upload should not have landed")
-
-	// verify must fail loudly for this snapshot, in both modes.
-	reader := newReaderVaultik(ctx, cfg, inner, repos, fs)
-
-	deepOpts := &vaultik.VerifyOptions{Deep: true}
-	require.Error(t, reader.VerifySnapshotWithOptions(id, deepOpts),
-		"deep verify must report the missing manifest")
-
-	shallowOpts := &vaultik.VerifyOptions{Deep: false}
-	require.Error(t, reader.VerifySnapshotWithOptions(id, shallowOpts),
-		"shallow verify must report the missing manifest")
-
-	// snapshot list must not crash on the partial snapshot.
-	require.NoError(t, reader.ListSnapshots(false),
-		"snapshot list must tolerate a partially-exported snapshot")
-}
-
-// Scenario 5: the restore target runs out of space mid-file. Restore
-// must fail with an out-of-space error, and must not leave a truncated
-// file at the target path presenting as a complete restore. Restore
-// today writes each file straight to its final path and does not remove
-// it when a write fails, so the truncated file survives; deleting it is
-// tracked by https://git.eeqj.de/sneak/vaultik/issues/163. Skipped until
-// that lands, so the destination assertion below is recorded rather than
-// dropped.
-//
-//nolint:paralleltest // installs the global logger via log.Initialize
-func TestRestoreReportsDiskFull(t *testing.T) {
-	t.Skip("blocked on https://git.eeqj.de/sneak/vaultik/issues/163: " +
-		"a disk-full write leaves a truncated file at the target path " +
-		"instead of removing it")
-	log.Initialize(log.Config{})
-
-	osFS := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-
-	testFiles := writeFaultSourceTree(t, osFS, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	id := fullFaultBackup(ctx, t, osFS, inner, cfg, repos, dataDir, dbPath, "diskfull")
-	require.NoError(t, db.Close())
-
-	// Restore onto a filesystem that allows only a few bytes of file
-	// content: enough to create files, far too little to hold them.
-	budget := int64(8)
-	quota := &quotaFS{Fs: osFS, remaining: &budget}
-
-	v := newReaderVaultik(ctx, cfg, inner, nil, quota)
-	err = v.Restore(&vaultik.RestoreOptions{SnapshotID: id, TargetDir: restoreDir})
-
-	require.Error(t, err, "restore must fail when the target disk is full")
-	assert.Contains(t, err.Error(), errNoSpace.Error(),
-		"restore error should surface the out-of-space cause")
-
-	// The failure must not leave a truncated file behind presenting as a
-	// complete restore: any file at the target must hold the original
-	// bytes, or be absent.
-	assertNoCorruptFiles(t, osFS, restoreDir, testFiles)
-}
-
-// assertRestoredTree byte-compares every restored file against the
-// original.
-func assertRestoredTree(
-	t *testing.T, fs afero.Fs, restoreDir string, testFiles map[string][]byte,
-) {
-	t.Helper()
-
-	for origPath, expected := range testFiles {
-		restoredPath := filepath.Join(restoreDir, origPath)
-		got, err := afero.ReadFile(fs, restoredPath)
-		require.NoErrorf(t, err, "restored file missing: %s", origPath)
-		require.Equalf(t, expected, got, "restored content mismatch for %s", origPath)
-	}
-}
-
-// errNoSpace is the out-of-space error quotaFS returns once its byte
-// budget is exhausted, mirroring a real ENOSPC.
-var errNoSpace = errors.New("no space left on device")
-
-// quotaFS is an afero.Fs whose files may write only a fixed total number
-// of content bytes before failing, simulating a full restore target. It
-// wraps the interface so every method except Create delegates to the
-// real filesystem; only file writes are capped.
-type quotaFS struct {
-	afero.Fs
-
-	remaining *int64
-}
-
-//nolint:ireturn // afero.Fs.Create's signature requires returning afero.File.
-func (q *quotaFS) Create(name string) (afero.File, error) {
-	f, err := q.Fs.Create(name)
-	if err != nil {
-		return nil, err
-	}
-
-	return &quotaFile{File: f, remaining: q.remaining}, nil
-}
-
-// quotaFile fails writes once the shared byte budget is exhausted.
-type quotaFile struct {
-	afero.File
-
-	remaining *int64
-}
-
-func (q *quotaFile) Write(p []byte) (int, error) {
-	if *q.remaining <= 0 {
-		return 0, errNoSpace
-	}
-
-	allowed := min(int64(len(p)), *q.remaining)
-
-	n, err := q.File.Write(p[:allowed])
-	*q.remaining -= int64(n)
-
-	if err != nil {
-		return n, err
-	}
-
-	if int64(n) < int64(len(p)) {
-		return n, errNoSpace
-	}
-
-	return n, nil
-}
-
-// assertNoCorruptFiles fails if any file that made it to the restore
-// target holds content that differs from the original: a failed restore
-// may leave a file absent, but must never leave wrong bytes presenting
-// as the real file.
-func assertNoCorruptFiles(
-	t *testing.T, fs afero.Fs, restoreDir string, testFiles map[string][]byte,
-) {
-	t.Helper()
-
-	for origPath, expected := range testFiles {
-		restoredPath := filepath.Join(restoreDir, origPath)
-
-		got, err := afero.ReadFile(fs, restoredPath)
-		if err != nil {
-			if os.IsNotExist(err) {
-				continue
-			}
-
-			require.NoError(t, err)
-		}
-
-		assert.Equalf(t, expected, got,
-			"restored file %s holds corrupt content", origPath)
-	}
-}

internal/vaultik/prune.go

@@ -167,12 +167,6 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 
 // collectReferencedBlobs downloads all manifests and returns the set of
 // referenced blob hashes.
-//
-// Every manifest must be read successfully. A manifest that cannot be
-// downloaded or decoded means its snapshot's blobs are unknown, so
-// treating them as unreferenced would let prune delete data a snapshot
-// still needs. Rather than risk that silent loss, any failure returns an
-// error naming the remote key and prune deletes nothing.
 func (v *Vaultik) collectReferencedBlobs() (map[string]bool, error) {
 	log.Info("Listing remote snapshots")
 	// IDs returned by listUniqueSnapshotIDs are remote keys (hashed
@@ -185,22 +179,27 @@ func (v *Vaultik) collectReferencedBlobs() (map[string]bool, error) {
 	log.Info("Found manifests in remote storage", "count", len(remoteKeys))
 
 	allBlobsReferenced := make(map[string]bool)
+	manifestCount := 0
 
 	for _, remoteKey := range remoteKeys {
 		log.Debug("Processing manifest", "remote_key", remoteKey)
 
 		manifest, err := v.downloadManifestByKey(remoteKey)
 		if err != nil {
-			return nil, fmt.Errorf("reading manifest %s: %w", remoteKey, err)
+			log.Error("Failed to download manifest", "remote_key", remoteKey, "error", err)
+
+			continue
 		}
 
 		for _, blob := range manifest.Blobs {
 			allBlobsReferenced[blob.Hash] = true
 		}
+
+		manifestCount++
 	}
 
 	log.Info("Processed manifests",
-		"count", len(remoteKeys), "unique_blobs_referenced", len(allBlobsReferenced))
+		"count", manifestCount, "unique_blobs_referenced", len(allBlobsReferenced))
 
 	return allBlobsReferenced, nil
 }

internal/vaultik/prune_manifest_safety_test.go

@@ -1,47 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestPruneBlobs_UnreadableManifestDeletesNothing is the regression guard
-// for issue #157: prune identifies referenced blobs by reading every
-// snapshot's manifest, and a manifest it cannot decode used to be logged
-// and skipped. Blobs referenced only by that snapshot then looked
-// unreferenced and were deleted, with a zero exit — silent backup loss,
-// made worse by `snapshot create --prune` running unattended with force.
-//
-// The single blob here is referenced only by the snapshot whose manifest
-// is corrupt, so the old behaviour would delete it and succeed. Prune
-// must instead delete nothing and return an error.
-func TestPruneBlobs_UnreadableManifestDeletesNothing(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newListEnv(t)
-	ctx := context.Background()
-
-	blobKey := "blobs/" + testBlobHashA[:2] + "/" + testBlobHashA[2:4] +
-		"/" + testBlobHashA
-	require.NoError(t, env.store.Put(ctx, blobKey,
-		bytes.NewReader([]byte("blob-bytes"))))
-
-	// A manifest at the path prune reads, but with contents it cannot
-	// decode.
-	require.NoError(t, env.store.Put(ctx,
-		"metadata/corruptkey/manifest.json.zst",
-		bytes.NewReader([]byte("not a valid manifest"))))
-
-	err := env.v.PruneBlobs(&vaultik.PruneOptions{Force: true})
-
-	require.Error(t, err, "prune must fail when a manifest cannot be read")
-	assert.True(t, env.store.hasKey(blobKey),
-		"no blob may be deleted when a manifest is unreadable")
-}

internal/vaultik/restore.go

@@ -11,7 +11,6 @@ import (
 	"math"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 
 	"filippo.io/age"
@@ -35,10 +34,6 @@ var (
 	errChunkNotInAnyBlob    = errors.New("chunk not found in any blob")
 	errBlobIDNotInHashIndex = errors.New("blob id missing from hash index")
 	errShortChunkRead       = errors.New("short read")
-	errRestorePathEscapesTarget = errors.New(
-		"refusing to restore path outside the target directory")
-	errTrailingRestoreData = errors.New(
-		"restored file has trailing data after its last chunk")
 )
 
 // restoreDirMode is the permission mode for directories created while
@@ -46,13 +41,6 @@ var (
 // directories themselves get their stored mode).
 const restoreDirMode = 0o755
 
-// restoreFileMode is the restrictive mode a regular file is created with
-// during restore. Content is written while the file holds this mode; the
-// stored mode is applied only after the file is fully written and closed,
-// so a file whose stored mode is restrictive is never briefly readable by
-// other local users while its content is being written.
-const restoreFileMode = 0o600
-
 // sweepIntervalDivisor sets the sweeper threshold to one N-th of the
 // configured blob size limit.
 const sweepIntervalDivisor = 100
@@ -772,85 +760,13 @@ type restoreSession struct {
 	runningAsRoot bool
 }
 
-// containedRestorePath resolves rel — a path read from the snapshot
-// database — to its location under targetDir and confirms the write will
-// stay inside the target.
-//
-// age decryption proves a snapshot is readable, not that it is honest, so
-// every stored path is treated as hostile. rel is rejected unless
-// filepath.IsLocal accepts it once the leading separator is stripped:
-// stored paths are absolute and the join to targetDir drops that
-// separator, so "/etc/passwd" is judged as the relative "etc/passwd" it
-// becomes on disk. This bars "..", absolute, and empty paths.
-//
-// A stored symlink whose target points outside the tree is still honest
-// (and restored verbatim), but a later entry must not be written through
-// it. Each existing ancestor directory below the target is therefore
-// Lstat'ed and a symlink among them is refused. The leaf itself is not
-// traversed: honest snapshots restore symlinks at leaf positions, and the
-// unique-path constraint keeps a leaf from being both a symlink and a
-// regular file. The target directory itself may be a symlink; only
-// components below it are checked.
-func containedRestorePath(fs afero.Fs, targetDir, rel string) (string, error) {
-	local := strings.TrimPrefix(rel, string(filepath.Separator))
-	if !filepath.IsLocal(local) {
-		return "", fmt.Errorf("%w: %s", errRestorePathEscapesTarget, rel)
-	}
-
-	local = filepath.Clean(local)
-	targetPath := filepath.Join(targetDir, local)
-
-	relDir := filepath.Dir(local)
-	if relDir == "." {
-		return targetPath, nil
-	}
-
-	current := targetDir
-	for component := range strings.SplitSeq(relDir, string(filepath.Separator)) {
-		current = filepath.Join(current, component)
-
-		info, err := lstatIfPossible(fs, current)
-		if err != nil {
-			if os.IsNotExist(err) {
-				continue
-			}
-
-			return "", fmt.Errorf("checking restore path %s: %w", current, err)
-		}
-
-		if info.Mode()&os.ModeSymlink != 0 {
-			return "", fmt.Errorf("%w: %s descends through symlink %s",
-				errRestorePathEscapesTarget, rel, current)
-		}
-	}
-
-	return targetPath, nil
-}
-
-// lstatIfPossible performs a symlink-aware stat when the filesystem
-// supports it. afero.OsFs does; MemMapFs, which has no symlinks, reports
-// that Lstat was not used and its result never carries ModeSymlink.
-func lstatIfPossible(fs afero.Fs, name string) (os.FileInfo, error) {
-	if lstater, ok := fs.(afero.Lstater); ok {
-		info, _, err := lstater.LstatIfPossible(name)
-
-		return info, err
-	}
-
-	return fs.Stat(name)
-}
-
 // restoreFile dispatches to the right per-kind restorer.
 func (s *restoreSession) restoreFile(file *database.File) error {
-	targetPath, err := containedRestorePath(
-		s.v.Fs, s.opts.TargetDir, file.Path.String())
-	if err != nil {
-		return err
-	}
+	targetPath := filepath.Join(s.opts.TargetDir, file.Path.String())
 
 	parentDir := filepath.Dir(targetPath)
 
-	err = s.v.Fs.MkdirAll(parentDir, restoreDirMode)
+	err := s.v.Fs.MkdirAll(parentDir, restoreDirMode)
 	if err != nil {
 		return fmt.Errorf("creating parent directory: %w", err)
 	}
@@ -898,13 +814,6 @@ func (s *restoreSession) restoreDirectory(
 		return fmt.Errorf("creating directory: %w", err)
 	}
 
-	// MkdirAll applies the process umask, so chmod to the exact stored
-	// mode. A failure here is non-fatal.
-	err = s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
-	if err != nil {
-		log.Debug("Failed to set permissions", "path", targetPath, "error", err)
-	}
-
 	s.applyFileMetadata(file, targetPath)
 
 	s.result.FilesRestored++
@@ -912,22 +821,25 @@ func (s *restoreSession) restoreDirectory(
 	return nil
 }
 
-// applyFileMetadata applies ownership (when running as root on a real
-// filesystem) and mtime to a restored path. Permission mode is applied
-// separately by each caller, with different failure handling, so it is
-// not touched here. Failures are logged at debug level and do not abort
-// the restore.
+// applyFileMetadata applies stored permissions, ownership (when running
+// as root on a real filesystem), and mtime to a restored path. Failures
+// are logged at debug level and do not abort the restore.
 func (s *restoreSession) applyFileMetadata(file *database.File, targetPath string) {
+	err := s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
+	if err != nil {
+		log.Debug("Failed to set permissions", "path", targetPath, "error", err)
+	}
+
 	if s.runningAsRoot {
 		if _, ok := s.v.Fs.(*afero.OsFs); ok {
-			err := os.Chown(targetPath, int(file.UID), int(file.GID))
+			err = os.Chown(targetPath, int(file.UID), int(file.GID))
 			if err != nil {
 				log.Debug("Failed to set ownership", "path", targetPath, "error", err)
 			}
 		}
 	}
 
-	err := s.v.Fs.Chtimes(targetPath, file.MTime, file.MTime)
+	err = s.v.Fs.Chtimes(targetPath, file.MTime, file.MTime)
 	if err != nil {
 		log.Debug("Failed to set mtime", "path", targetPath, "error", err)
 	}
@@ -961,30 +873,17 @@ func (s *restoreSession) restoreRegularFile(
 
 	t0 = time.Now()
 
-	// Remove any existing entry, then create the file with a restrictive
-	// mode via O_EXCL. The stored mode is applied only after the content
-	// is written and the file closed, so a file whose stored mode is
-	// restrictive is never briefly readable by other local users while
-	// its content is written. Removing first (rather than failing on a
-	// leftover file) matches the documented behaviour that re-running
-	// restore overwrites partial output.
-	_ = s.v.Fs.Remove(targetPath)
-
-	outFile, err := s.v.Fs.OpenFile(
-		targetPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, restoreFileMode)
+	outFile, err := s.v.Fs.Create(targetPath)
 	createDur := time.Since(t0)
 
 	if err != nil {
 		return fmt.Errorf("creating output file: %w", err)
 	}
 
+	defer func() { _ = outFile.Close() }()
+
 	bytesWritten, timings, err := s.writeFileChunks(outFile, fileChunks)
 	if err != nil {
-		// Do not leave a partial file behind.
-		_ = outFile.Close()
-
-		s.removePartialRestore(targetPath)
-
 		return err
 	}
 
@@ -1002,12 +901,9 @@ func (s *restoreSession) restoreRegularFile(
 
 	err = outFile.Close()
 	if err != nil {
-		s.removePartialRestore(targetPath)
-
 		return fmt.Errorf("closing output file: %w", err)
 	}
 
-	s.applyRestoredFileMode(file, targetPath)
 	s.applyFileMetadata(file, targetPath)
 
 	s.result.FilesRestored++
@@ -1018,31 +914,6 @@ func (s *restoreSession) restoreRegularFile(
 	return nil
 }
 
-// applyRestoredFileMode applies the stored permission bits to a
-// just-written regular file (created with restoreFileMode). A failure is
-// a user-visible warning, not a fatal error: the file's content is
-// intact and it remains at the restrictive create-time mode, so the
-// restore is not aborted or discarded over it.
-func (s *restoreSession) applyRestoredFileMode(
-	file *database.File, targetPath string,
-) {
-	err := s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
-	if err != nil {
-		s.v.UI.Warningf("Failed to set mode %s on %s: %v",
-			os.FileMode(file.Mode).Perm(), s.v.UI.Path(targetPath), err)
-	}
-}
-
-// removePartialRestore deletes a restore output file whose write did not
-// complete, so a failed restore never leaves a partial file behind.
-func (s *restoreSession) removePartialRestore(targetPath string) {
-	err := s.v.Fs.Remove(targetPath)
-	if err != nil {
-		log.Debug("Failed to remove partial restore file",
-			"path", targetPath, "error", err)
-	}
-}
-
 // writeFileChunks streams each of the file's chunks from the blob disk
 // cache into outFile, crediting restored bytes to the sweeper as it
 // goes. Returns the bytes written plus per-phase timing accumulators.
@@ -1124,19 +995,11 @@ func (s *restoreSession) downloadBlobToCache(
 	streamDur := time.Since(t0)
 	closeErr := rc.Close()
 
-	// closeErr carries the blob's hash-verification result (a mismatch,
-	// or the stream not being fully read). On any failure, drop the
-	// cache entry so a blob that failed verification is never read back
-	// as if it were valid.
 	if copyErr != nil {
-		s.blobCache.Delete(blobHash)
-
 		return copyErr
 	}
 
 	if closeErr != nil {
-		s.blobCache.Delete(blobHash)
-
 		return closeErr
 	}
 
@@ -1199,22 +1062,17 @@ func (v *Vaultik) verifyRestoredFiles(
 			return ctx.Err()
 		}
 
-		targetPath, err := containedRestorePath(v.Fs, targetDir, file.Path.String())
-		if err == nil {
-			var bytesVerified int64
-
-			bytesVerified, err = v.verifyFile(ctx, repos, file, targetPath)
-			if err == nil {
-				result.FilesVerified++
-				result.BytesVerified += bytesVerified
-			}
-		}
+		targetPath := filepath.Join(targetDir, file.Path.String())
 
+		bytesVerified, err := v.verifyFile(ctx, repos, file, targetPath)
 		if err != nil {
 			log.Error("File verification failed", "path", file.Path, "error", err)
 
 			result.FilesFailed++
 			result.FailedFiles = append(result.FailedFiles, file.Path.String())
+		} else {
+			result.FilesVerified++
+			result.BytesVerified += bytesVerified
 		}
 
 		bytesProcessed += file.Size
@@ -1304,17 +1162,6 @@ func (v *Vaultik) verifyFile(
 		bytesVerified += int64(n)
 	}
 
-	// The stored chunks account for the whole file, so the reader must
-	// be at EOF now. Trailing bytes past the last chunk are corruption
-	// the per-chunk loop cannot see.
-	extra := make([]byte, 1)
-
-	n, err := f.Read(extra)
-	if n != 0 || !errors.Is(err, io.EOF) {
-		return bytesVerified, fmt.Errorf("%w: file longer than its %d chunk(s)",
-			errTrailingRestoreData, len(fileChunks))
-	}
-
 	log.Debug("File verified",
 		"path", file.Path, "bytes", bytesVerified, "chunks", len(fileChunks))
 

internal/vaultik/restore_containment_test.go

@@ -1,175 +0,0 @@
-package vaultik //nolint:testpackage // drives unexported restore internals
-
-import (
-	"context"
-	"io"
-	"os"
-	"path/filepath"
-	"testing"
-	"time"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/types"
-	"sneak.berlin/go/vaultik/internal/ui"
-)
-
-// These tests exercise the path-containment guard that keeps restore from
-// writing outside its target directory. age decryption proves only that a
-// snapshot is readable, not that its recorded paths are honest, so restore
-// treats every stored path as hostile: a compromised backed-up host could
-// forge a snapshot that decrypts cleanly, and restore usually runs as root.
-//
-// They drive restoreAllFiles directly (rather than the full Restore, which
-// downloads and decrypts the metadata database from storage) so a snapshot
-// database with adversarial rows can be handed to the restore loop without
-// the surrounding blob/storage machinery. Directory and symlink entries
-// carry no chunks, so no blobs are needed.
-
-// containmentDirMode marks a File row as a directory for the restore loop.
-const containmentDirMode = uint32(os.ModeDir | 0o755)
-
-// newContainmentVaultik builds the minimal Vaultik needed to run
-// restoreAllFiles against fs.
-func newContainmentVaultik(ctx context.Context, fs afero.Fs) *Vaultik {
-	v := &Vaultik{
-		Config: &config.Config{
-			BlobSizeLimit: config.Size(10 * 1024 * 1024),
-		},
-		Fs:     fs,
-		Stdout: io.Discard,
-		Stderr: io.Discard,
-		UI:     ui.NewWithColor(io.Discard, false),
-	}
-	v.SetContext(ctx)
-
-	return v
-}
-
-// makeFiles inserts the given rows into a fresh in-memory snapshot database
-// and returns them (with IDs assigned) plus the repositories.
-func makeFiles(
-	ctx context.Context, t *testing.T, rows []*database.File,
-) ([]*database.File, *database.Repositories) {
-	t.Helper()
-
-	db, err := database.New(ctx, filepath.Join(t.TempDir(), "index.sqlite"))
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = db.Close() })
-
-	repos := database.NewRepositories(db)
-	for _, f := range rows {
-		require.NoError(t, repos.Files.Create(ctx, nil, f))
-	}
-
-	return rows, repos
-}
-
-func TestRestoreRejectsPathTraversal(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	tests := []struct {
-		name string
-		// rows are inserted in order; the escape entry is restored after
-		// any entry it depends on (the symlink case needs its link first).
-		rows func(outsideDir string) []*database.File
-		// escaped is the path, outside the target, that must not appear.
-		escaped func(tempDir, outsideDir string) string
-	}{
-		{
-			name: "relative dotdot",
-			rows: func(_ string) []*database.File {
-				return []*database.File{{
-					Path: "../escaped-relative",
-					Mode: containmentDirMode,
-				}}
-			},
-			escaped: func(tempDir, _ string) string {
-				return filepath.Join(tempDir, "escaped-relative")
-			},
-		},
-		{
-			name: "absolute with dotdot",
-			rows: func(_ string) []*database.File {
-				return []*database.File{{
-					Path: "/a/../../escaped-absolute",
-					Mode: containmentDirMode,
-				}}
-			},
-			escaped: func(tempDir, _ string) string {
-				return filepath.Join(tempDir, "escaped-absolute")
-			},
-		},
-		{
-			name: "child through symlink",
-			rows: func(outsideDir string) []*database.File {
-				return []*database.File{
-					// Restored first: an in-target symlink pointing out.
-					{Path: "linkdir", LinkTarget: types.FilePath(outsideDir)},
-					// Restored second: a child written through that link.
-					{Path: "linkdir/child", Mode: containmentDirMode},
-				}
-			},
-			escaped: func(_, outsideDir string) string {
-				return filepath.Join(outsideDir, "child")
-			},
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			ctx := context.Background()
-			fs := afero.NewOsFs()
-			tempDir := t.TempDir()
-			targetDir := filepath.Join(tempDir, "target")
-			outsideDir := filepath.Join(tempDir, "outside")
-			require.NoError(t, fs.MkdirAll(outsideDir, 0o755))
-
-			rows, repos := makeFiles(ctx, t, tc.rows(outsideDir))
-			v := newContainmentVaultik(ctx, fs)
-
-			_, err := v.restoreAllFiles(rows, repos,
-				&RestoreOptions{TargetDir: targetDir}, nil, nil)
-
-			require.ErrorIs(t, err, errRestorePathEscapesTarget)
-
-			escaped := tc.escaped(tempDir, outsideDir)
-			_, statErr := os.Lstat(escaped)
-			require.Truef(t, os.IsNotExist(statErr),
-				"restore wrote outside the target at %s", escaped)
-		})
-	}
-}
-
-// TestRestoreAllowsSymlinkPointingOutsideTree confirms the guard does not
-// over-block: an honest snapshot may contain a symlink whose target lies
-// outside the restored tree, and it must still be restored verbatim.
-func TestRestoreAllowsSymlinkPointingOutsideTree(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	targetDir := filepath.Join(tempDir, "target")
-	linkTarget := filepath.Join(tempDir, "outside", "data")
-
-	rows, repos := makeFiles(ctx, t, []*database.File{
-		{Path: "goodlink", LinkTarget: types.FilePath(linkTarget), MTime: time.Unix(0, 0)},
-	})
-	v := newContainmentVaultik(ctx, fs)
-
-	_, err := v.restoreAllFiles(rows, repos,
-		&RestoreOptions{TargetDir: targetDir}, nil, nil)
-	require.NoError(t, err)
-
-	got, err := os.Readlink(filepath.Join(targetDir, "goodlink"))
-	require.NoError(t, err)
-	require.Equal(t, linkTarget, got)
-}

internal/vaultik/restore_perms_test.go

@@ -1,306 +0,0 @@
-package vaultik //nolint:testpackage // drives restore through unexported session
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"io"
-	"os"
-	"path/filepath"
-	"strings"
-	"sync"
-	"syscall"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/snapshot"
-	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/ui"
-)
-
-// errSpyWrite is the injected write failure used to exercise the
-// partial-file cleanup path.
-var errSpyWrite = errors.New("injected write failure")
-
-// modeSpyFs wraps a real filesystem so restore tests can observe and
-// perturb the single output file whose path contains watch. It records
-// the on-disk permission bits seen at the moment content is first
-// written (the window during which another local user could read it),
-// and can inject a write failure or append trailing bytes on close.
-type modeSpyFs struct {
-	afero.Fs
-
-	watch string
-
-	mu         sync.Mutex
-	writeModes []os.FileMode
-	failWrite  bool
-	trailing   int
-}
-
-//nolint:ireturn // afero.Fs.OpenFile is defined to return the interface
-func (m *modeSpyFs) OpenFile(
-	name string, flag int, perm os.FileMode,
-) (afero.File, error) {
-	f, err := m.Fs.OpenFile(name, flag, perm)
-	if err != nil || !strings.Contains(name, m.watch) {
-		return f, err
-	}
-
-	return &modeSpyFile{File: f, fs: m, path: name}, nil
-}
-
-type modeSpyFile struct {
-	afero.File
-
-	fs      *modeSpyFs
-	path    string
-	written bool
-}
-
-func (f *modeSpyFile) Write(p []byte) (int, error) {
-	if !f.written {
-		f.written = true
-
-		info, err := f.fs.Stat(f.path)
-		if err == nil {
-			f.fs.mu.Lock()
-			f.fs.writeModes = append(f.fs.writeModes, info.Mode().Perm())
-			f.fs.mu.Unlock()
-		}
-	}
-
-	if f.fs.failWrite {
-		return 0, errSpyWrite
-	}
-
-	return f.File.Write(p)
-}
-
-func (f *modeSpyFile) Close() error {
-	if f.fs.trailing > 0 {
-		_, _ = f.File.Write(bytes.Repeat([]byte{'x'}, f.fs.trailing))
-	}
-
-	return f.File.Close()
-}
-
-// backupOneFile writes a single source file with the given mode and
-// backs it up into a fresh file storer, returning everything a restore
-// needs. The index database is closed before returning so the restore
-// half runs from the exported metadata and remote bytes only.
-func backupOneFile(
-	ctx context.Context, t *testing.T, fs afero.Fs, tempDir, name string,
-	content []byte, mode os.FileMode,
-) (*config.Config, *storage.FileStorer, string, string) {
-	t.Helper()
-
-	dataDir := filepath.Join(tempDir, "src")
-	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
-
-	srcPath := filepath.Join(dataDir, name)
-	require.NoError(t, afero.WriteFile(fs, srcPath, content, mode))
-	require.NoError(t, fs.Chmod(srcPath, mode))
-
-	storeDir := filepath.Join(tempDir, "remote")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	storer, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	cfg := &config.Config{
-		AgeRecipients: []string{
-			"age1ezrjmfpwsc95svdg0y54mums3zevgzu0x0ecq2f7tp8a05gl0sjq9q9wjg",
-		},
-		AgeSecretKey: "AGE-SECRET-KEY-19CR5YSFW59HM4TLD6GXVEDMZFTVVF7PPHKU" +
-			"T68TXSFPK7APHXA2QS2NJA5",
-		CompressionLevel: 3,
-		Hostname:         "test-host",
-		BlobSizeLimit:    config.Size(5 * 1024 * 1024),
-	}
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-
-	sm := snapshot.NewSnapshotManager(snapshot.SnapshotManagerParams{
-		Repos:   repos,
-		Storage: storer,
-		Config:  cfg,
-	})
-	sm.SetFilesystem(fs)
-
-	scanner := snapshot.NewScanner(snapshot.ScannerConfig{
-		FS:               fs,
-		Storage:          storer,
-		ChunkSize:        4 * 1024 * 1024,
-		MaxBlobSize:      5 * 1024 * 1024,
-		CompressionLevel: cfg.CompressionLevel,
-		AgeRecipients:    cfg.AgeRecipients,
-		Repositories:     repos,
-	})
-
-	snapshotID, err := sm.CreateSnapshotWithName(
-		ctx, cfg.Hostname, "perms", "test-version", "test-git")
-	require.NoError(t, err)
-
-	_, err = scanner.Scan(ctx, dataDir, snapshotID)
-	require.NoError(t, err)
-
-	require.NoError(t, sm.CompleteSnapshot(ctx, snapshotID))
-	require.NoError(t, sm.ExportSnapshotMetadata(ctx, dbPath, snapshotID))
-	require.NoError(t, db.Close())
-
-	return cfg, storer, snapshotID, srcPath
-}
-
-// restoredPathFor returns where backupOneFile's source lands under a
-// restore target: restore recreates each file at its original absolute
-// path beneath TargetDir.
-func restoredPathFor(restoreDir, srcPath string) string {
-	return filepath.Join(restoreDir, srcPath)
-}
-
-// withUmask022 forces the process umask to 022 for the duration of a
-// test, so the difference between a 0600 create and a default create is
-// observable. Restored serially (no t.Parallel) so it does not race
-// other tests.
-func withUmask022(t *testing.T) {
-	t.Helper()
-
-	old := syscall.Umask(0o022)
-
-	t.Cleanup(func() { syscall.Umask(old) })
-}
-
-// TestRestoreCreatesFileNeverWiderThanStoredMode checks that a file with
-// a restrictive stored mode (0600) is never observable with a wider mode
-// while its content is being written, and ends at its stored mode.
-//
-//nolint:paralleltest // sets the process umask; must run serially
-func TestRestoreCreatesFileNeverWiderThanStoredMode(t *testing.T) {
-	log.Initialize(log.Config{})
-	withUmask022(t)
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	ctx := context.Background()
-
-	content := randomBytes(t, 4096)
-
-	cfg, storer, snapshotID, srcPath := backupOneFile(
-		ctx, t, fs, tempDir, "secret.bin", content, 0o600)
-
-	restoreDir := filepath.Join(tempDir, "restored")
-	spy := &modeSpyFs{Fs: fs, watch: "secret.bin"}
-
-	v := newRestoreVaultik(ctx, cfg, storer, spy)
-	require.NoError(t, v.Restore(&RestoreOptions{
-		SnapshotID: snapshotID,
-		TargetDir:  restoreDir,
-	}))
-
-	spy.mu.Lock()
-	observed := append([]os.FileMode(nil), spy.writeModes...)
-	spy.mu.Unlock()
-
-	require.NotEmpty(t, observed,
-		"spy never saw the output file being written")
-
-	for _, m := range observed {
-		assert.Equalf(t, os.FileMode(0o600), m,
-			"file was observable at mode %o during write; must be 0600", m)
-	}
-
-	// The stored mode is applied after the content is written.
-	info, err := fs.Stat(restoredPathFor(restoreDir, srcPath))
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0o600), info.Mode().Perm())
-
-	got, err := afero.ReadFile(fs, restoredPathFor(restoreDir, srcPath))
-	require.NoError(t, err)
-	require.True(t, bytes.Equal(got, content))
-}
-
-// TestRestoreRemovesPartialFileOnWriteFailure checks that a file whose
-// content write fails is not left behind.
-//
-//nolint:paralleltest // sets the process umask; must run serially
-func TestRestoreRemovesPartialFileOnWriteFailure(t *testing.T) {
-	log.Initialize(log.Config{})
-	withUmask022(t)
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	ctx := context.Background()
-
-	cfg, storer, snapshotID, srcPath := backupOneFile(
-		ctx, t, fs, tempDir, "doomed.bin", randomBytes(t, 4096), 0o600)
-
-	restoreDir := filepath.Join(tempDir, "restored")
-	spy := &modeSpyFs{Fs: fs, watch: "doomed.bin", failWrite: true}
-
-	v := newRestoreVaultik(ctx, cfg, storer, spy)
-	err := v.Restore(&RestoreOptions{
-		SnapshotID: snapshotID,
-		TargetDir:  restoreDir,
-	})
-	require.Error(t, err, "restore should fail when the write fails")
-
-	exists, err := afero.Exists(fs, restoredPathFor(restoreDir, srcPath))
-	require.NoError(t, err)
-	assert.False(t, exists, "partial file must be removed after a failed write")
-}
-
-// TestVerifyRejectsTrailingBytes checks that --verify fails a restored
-// file that has bytes past its last chunk.
-//
-//nolint:paralleltest // sets the process umask; must run serially
-func TestVerifyRejectsTrailingBytes(t *testing.T) {
-	log.Initialize(log.Config{})
-	withUmask022(t)
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	ctx := context.Background()
-
-	cfg, storer, snapshotID, _ := backupOneFile(
-		ctx, t, fs, tempDir, "padded.bin", randomBytes(t, 4096), 0o600)
-
-	restoreDir := filepath.Join(tempDir, "restored")
-	// Append one byte to the file as it is written, so its content still
-	// matches the stored chunks but it is one byte too long.
-	spy := &modeSpyFs{Fs: fs, watch: "padded.bin", trailing: 1}
-
-	v := newRestoreVaultik(ctx, cfg, storer, spy)
-	err := v.Restore(&RestoreOptions{
-		SnapshotID: snapshotID,
-		TargetDir:  restoreDir,
-		Verify:     true,
-	})
-	require.Error(t, err, "verify should fail on a file with trailing bytes")
-	assert.ErrorIs(t, err, errFilesFailedVerify)
-}
-
-// newRestoreVaultik builds a Vaultik wired for a restore-only test.
-func newRestoreVaultik(
-	ctx context.Context, cfg *config.Config, storer storage.Storer, fs afero.Fs,
-) *Vaultik {
-	v := &Vaultik{
-		Config:  cfg,
-		Storage: storer,
-		Fs:      fs,
-		Stdout:  io.Discard,
-		Stderr:  io.Discard,
-		UI:      ui.NewWithColor(io.Discard, false),
-	}
-	v.SetContext(ctx)
-
-	return v
-}

internal/vaultik/verify.go

@@ -29,8 +29,6 @@ var (
 	errTrailingBlobData  = errors.New(
 		"blob has unexpected trailing bytes not covered by chunk list")
 	errManifestExtraBlob = errors.New("manifest contains blob not in database")
-	errManifestMissingBlob = errors.New(
-		"manifest omits blob present in database")
 	errBlobSizeMismatch  = errors.New("blob size mismatch")
 )
 
@@ -577,11 +575,16 @@ func (v *Vaultik) verifyManifestAgainstDatabase(
 		manifestBlobMap[blob.Hash] = blob.CompressedSize
 	}
 
-	// The manifest is the only blob list prune consults, so it must match
-	// the database exactly. A blob in the manifest but not the database
-	// points at a corrupt manifest; a blob in the database but omitted
-	// from the manifest would be pruned away while this snapshot still
-	// needs it. Either divergence fails verification.
+	// Check counts match
+	if len(dbBlobMap) != len(manifestBlobMap) {
+		log.Warn("Manifest blob count mismatch",
+			"database_blobs", len(dbBlobMap),
+			"manifest_blobs", len(manifestBlobMap),
+		)
+		// This is a warning, not an error - database is authoritative
+	}
+
+	// Check each manifest blob exists in database with correct size
 	for hash, manifestSize := range manifestBlobMap {
 		dbSize, exists := dbBlobMap[hash]
 		if !exists {
@@ -595,12 +598,6 @@ func (v *Vaultik) verifyManifestAgainstDatabase(
 		}
 	}
 
-	for hash := range dbBlobMap {
-		if _, exists := manifestBlobMap[hash]; !exists {
-			return fmt.Errorf("%w: %s", errManifestMissingBlob, hash)
-		}
-	}
-
 	log.Info("✓ Manifest verified against database",
 		"manifest_blobs", len(manifestBlobMap),
 		"database_blobs", len(dbBlobMap),

internal/vaultik/verify_manifest_test.go

@@ -1,61 +0,0 @@
-package vaultik //nolint:testpackage // calls unexported verifyManifestAgainstDatabase
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/snapshot"
-)
-
-// Blob hashes shared by the manifest-verification tests below.
-const (
-	manifestTestBlobA = "blob-a"
-	manifestTestBlobB = "blob-b"
-)
-
-// TestVerifyManifestAgainstDatabase_MissingBlobFails is the regression
-// guard for issue #157: deep verify must fail when the manifest omits a
-// blob the database records. The divergence used to be logged as a
-// warning while verification still returned ok, so an incomplete
-// manifest — the exact defect that lets prune later delete a needed blob
-// — passed unnoticed.
-func TestVerifyManifestAgainstDatabase_MissingBlobFails(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	v := &Vaultik{}
-
-	dbBlobs := []snapshot.BlobInfo{
-		{Hash: manifestTestBlobA, CompressedSize: 10},
-		{Hash: manifestTestBlobB, CompressedSize: 20},
-	}
-	manifest := &snapshot.Manifest{
-		Blobs: []snapshot.BlobInfo{
-			{Hash: manifestTestBlobA, CompressedSize: 10},
-		},
-	}
-
-	err := v.verifyManifestAgainstDatabase(manifest, dbBlobs)
-	require.Error(t, err, "verify must fail when the manifest omits a database blob")
-	assert.Contains(t, err.Error(), manifestTestBlobB)
-}
-
-// TestVerifyManifestAgainstDatabase_MatchingSetsPass keeps the other half
-// honest: identical blob sets still verify, so the check above cannot be
-// satisfied by failing everything.
-func TestVerifyManifestAgainstDatabase_MatchingSetsPass(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	v := &Vaultik{}
-
-	blobs := []snapshot.BlobInfo{
-		{Hash: manifestTestBlobA, CompressedSize: 10},
-		{Hash: manifestTestBlobB, CompressedSize: 20},
-	}
-	manifest := &snapshot.Manifest{Blobs: blobs}
-
-	require.NoError(t, v.verifyManifestAgainstDatabase(manifest, blobs))
-}

script/cibuild

@@ -56,23 +56,8 @@ main() {
     docker build --output=type=cacheonly \
         --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)$$"
-    docker build --build-arg CHECK_EPOCH="$epoch" \
-        --build-arg VERSION="$version" \
-        --build-arg COMMIT="$commit" \
-        --build-arg COMMIT_DATE="$commit_date" \
-        .
+    docker build --build-arg CHECK_EPOCH="$epoch" .
 }
 
 main "$@"

script/docker

@@ -24,24 +24,7 @@ main() {
     # whether the tree is clean. The Dockerfile now refuses to build
     # without a non-empty value, so this is required, not optional.
     epoch="$(date +%s%N)$$"
-
-    # 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" \
-        --build-arg VERSION="$version" \
-        --build-arg COMMIT="$commit" \
-        --build-arg COMMIT_DATE="$commit_date" \
         -t "$("$SCRIPT_DIR/projectname")" .
 }