Trust only uploaded blobs for deduplication (closes #148 )

An interrupted blob upload left the blob's chunks, blob_chunks, and blobs rows committed before the upload was attempted, so a later run deduplicated against data that never reached storage and produced a snapshot that reported success but could not be restored. Chosen fix (issue option b): a chunk counts as known only when a blob holding it has uploaded_ts set, and each run drops un-uploaded blob rows and the chunks they orphan at startup, so the affected data is re-chunked and re-uploaded. Option a (withholding all row writes until the upload succeeds) would entangle the packer's writes with upload ordering; gated reads plus a startup repair is smaller and self-healing. Blobs recorded with no remote backend are marked uploaded so the invariant holds uniformly. model: claude-opus-4-8
2026-09-21 23:22:29 +00:00
7 changed files with 325 additions and 52 deletions
@@ -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

@@ -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,21 +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
-
-Every command that opens the local index — `snapshot create`, `snapshot
-list`, `snapshot verify`, `snapshot purge`, `snapshot remove`, `snapshot
-restore`, `prune`, `info`, and `remote info`/`remote nuke` — takes a
-process-wide lock at `$XDG_DATA_HOME/vaultik/vaultik.pid`
-(`~/.local/share/vaultik/vaultik.pid` on Linux) for the whole run. Only
-one such command runs at a time: a second one exits immediately with an
-"already running" error rather than waiting. The lock is not scoped to
-mutating commands, so read-only commands are affected too — `vaultik
-snapshot list` fails while a backup is in progress; scoping it so
-read-only commands run during a backup is tracked in
-[issue #150](https://git.eeqj.de/sneak/vaultik/issues/150). `config`,
-`database delete`, `completion`, and `version` do not take the lock.
-
 ### stdout and stderr

 Log output — everything from `--verbose` and `--debug`, and every
@@ -218,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

@@ -412,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

 ```
@@ -493,24 +472,19 @@ 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) |
@@ -626,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
@@ -25,6 +25,19 @@ 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-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
@@ -208,6 +208,30 @@ 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(
@@ -7,12 +7,32 @@ import (

 // List returns every chunk in the index, ordered by chunk hash.
 func (r *ChunkRepository) List(ctx context.Context) ([]*Chunk, error) {
-	query := `
+	return r.list(ctx, `
 		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)
@@ -220,7 +220,14 @@ func (s *Scanner) Scan(
 		defer s.progress.Stop()
 	}

-	// Phase 0: Load known files and chunks from database into memory for fast lookup
+	// 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
+	}
+
 	knownFiles, err := s.loadDatabaseState(ctx, path)
 	if err != nil {
 		return nil, err
@@ -317,6 +324,38 @@ 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(
@@ -392,11 +431,14 @@ func (s *Scanner) loadKnownFiles(
 	return result, nil
 }

-// loadKnownChunks loads all known chunk hashes from the database into a
-// map for fast lookup. This avoids per-chunk database queries during file
-// processing.
+// 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.
 func (s *Scanner) loadKnownChunks(ctx context.Context) error {
-	chunks, err := s.repos.Chunks.List(ctx)
+	chunks, err := s.repos.Chunks.ListInUploadedBlobs(ctx)
 	if err != nil {
 		return fmt.Errorf("listing chunks: %w", err)
 	}
@@ -1401,7 +1443,17 @@ 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))
 			})
@@ -0,0 +1,198 @@
+package vaultik //nolint:testpackage // constructs Vaultik with unexported fields
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"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/snapshot"
+	"sneak.berlin/go/vaultik/internal/storage"
+	"sneak.berlin/go/vaultik/internal/ui"
+)
+
+// errUploadInterrupted stands in for a dropped connection or kill -9
+// partway through a blob upload.
+var errUploadInterrupted = errors.New("simulated interrupted blob upload")
+
+// interruptedBlobStorer wraps a real Storer but fails every blob upload,
+// modelling a run that dies mid-blob after the packer has already
+// committed the blob's chunk rows to the local index.
+type interruptedBlobStorer struct {
+	storage.Storer
+}
+
+func (s *interruptedBlobStorer) PutWithProgress(
+	ctx context.Context, key string, reader io.Reader,
+	size int64, cb storage.ProgressCallback,
+) error {
+	if strings.HasPrefix(key, "blobs/") {
+		return errUploadInterrupted
+	}
+
+	return s.Storer.PutWithProgress(ctx, key, reader, size, cb)
+}
+
+func (s *interruptedBlobStorer) Put(
+	ctx context.Context, key string, reader io.Reader,
+) error {
+	if strings.HasPrefix(key, "blobs/") {
+		return errUploadInterrupted
+	}
+
+	return s.Storer.Put(ctx, key, reader)
+}
+
+// TestBackupRetryAfterInterruptedUploadIsRestorable reproduces the
+// silent-data-loss defect in
+// https://git.eeqj.de/sneak/vaultik/issues/148: a blob upload is
+// interrupted, leaving chunk rows in the local index for data that never
+// reached storage. The retry run reuses the same index. Before the fix it
+// deduplicated against those orphaned chunks, uploaded nothing for them,
+// and produced a snapshot that reported success but could not be
+// restored. The retried snapshot must instead restore byte-for-byte.
+func TestBackupRetryAfterInterruptedUploadIsRestorable(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+
+	dataDir := filepath.Join(tempDir, "source")
+	storeDir := filepath.Join(tempDir, "remote")
+	restoreDir := filepath.Join(tempDir, "restored")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
+
+	// Random content forces real chunks; small blobs guarantee at least
+	// one blob is finalized (and its upload attempted) during the run.
+	sources := map[string][]byte{
+		"a.bin": randomBytes(t, 128*1024),
+		"b.bin": randomBytes(t, 128*1024),
+		"c.bin": randomBytes(t, 128*1024),
+	}
+	for name, data := range sources {
+		require.NoError(t, afero.WriteFile(
+			fs, filepath.Join(dataDir, name), data, 0o644))
+	}
+
+	cfg := &config.Config{
+		AgeRecipients: []string{"age1ezrjmfpwsc95svdg0y54mums3zevgzu0x0ecq2" +
+			"f7tp8a05gl0sjq9q9wjg"},
+		AgeSecretKey: "AGE-SECRET-KEY-19CR5YSFW59HM4TLD6GXVEDMZFTVVF7PPHKU" +
+			"T68TXSFPK7APHXA2QS2NJA5",
+		CompressionLevel: 3,
+		Hostname:         "test-host",
+		ChunkSize:        config.Size(16 * 1024),
+		BlobSizeLimit:    config.Size(64 * 1024),
+	}
+
+	working, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+
+	// Run 1: the upload is interrupted, so the scan fails but leaves the
+	// interrupted blob's chunk rows committed in the index.
+	_, err = runBackup(ctx, fs, cfg, &interruptedBlobStorer{Storer: working},
+		dataDir, dbPath, "interrupted")
+	require.Error(t, err, "interrupted upload must fail the run")
+
+	// Run 2: retry on the same index with a working backend. This must
+	// succeed and produce a fully restorable snapshot.
+	snapshotID, err := runBackup(ctx, fs, cfg, working, dataDir, dbPath, "retry")
+	require.NoError(t, err, "retry backup must succeed")
+
+	v := &Vaultik{
+		Config:  cfg,
+		Storage: working,
+		Fs:      fs,
+		Stdout:  io.Discard,
+		Stderr:  io.Discard,
+		UI:      ui.NewWithColor(io.Discard, false),
+	}
+	v.SetContext(ctx)
+
+	require.NoError(t, v.Restore(&RestoreOptions{
+		SnapshotID: snapshotID,
+		TargetDir:  restoreDir,
+	}), "the retried snapshot must be restorable")
+
+	for name, data := range sources {
+		restored := filepath.Join(restoreDir, dataDir, name)
+		got, err := afero.ReadFile(fs, restored)
+		require.NoErrorf(t, err, "restored file missing: %s", name)
+		require.Truef(t, bytes.Equal(got, data),
+			"restored bytes differ from original for %s", name)
+	}
+}
+
+// runBackup performs one backup of dataDir into a fresh snapshot on the
+// index at dbPath, returning the snapshot ID. When the scan succeeds it
+// also completes and exports the snapshot metadata so the result can be
+// restored. The index database is always closed before returning.
+func runBackup(
+	ctx context.Context,
+	fs afero.Fs,
+	cfg *config.Config,
+	storer storage.Storer,
+	dataDir, dbPath, name string,
+) (string, error) {
+	db, err := database.New(ctx, dbPath)
+	if err != nil {
+		return "", err
+	}
+
+	defer func() { _ = db.Close() }()
+
+	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:        cfg.ChunkSize.Int64(),
+		MaxBlobSize:      cfg.BlobSizeLimit.Int64(),
+		CompressionLevel: cfg.CompressionLevel,
+		AgeRecipients:    cfg.AgeRecipients,
+		Repositories:     repos,
+	})
+
+	snapshotID, err := sm.CreateSnapshotWithName(
+		ctx, cfg.Hostname, name, "test-version", "test-git")
+	if err != nil {
+		return "", err
+	}
+
+	_, err = scanner.Scan(ctx, dataDir, snapshotID)
+	if err != nil {
+		return snapshotID, err
+	}
+
+	err = sm.CompleteSnapshot(ctx, snapshotID)
+	if err != nil {
+		return snapshotID, err
+	}
+
+	err = sm.ExportSnapshotMetadata(ctx, dbPath, snapshotID)
+	if err != nil {
+		return snapshotID, err
+	}
+
+	return snapshotID, nil
+}