Reject a decrypted snapshot database that is not the requested one (closes #156)

Restore and deep verify downloaded and decrypted metadata/<key>/db.zst.age by object name alone. age decryption proves the database is readable, not that it is the snapshot that was asked for: an attacker who swaps in another valid db.zst.age could redirect the operation, and deep verify with a swapped database plus an empty manifest reported success with zero blobs verified.

After the database is opened, both paths now confirm its identity: an exported per-snapshot database holds one snapshot row, and a snapshot remote key derives from that row ID, so the database is the requested one exactly when its sole snapshot hashes back to the remote key fetched. The shared check lives in verifySnapshotDBIdentity, backed by a new SnapshotRepository.GetOnlySnapshot.

Model: opus-4-8

internal/database/snapshots.go

@@ -11,6 +11,18 @@ import (
 	"sneak.berlin/go/vaultik/internal/types"
 )
 
+// Sentinel errors for the single-snapshot invariant that an exported
+// per-snapshot metadata database must satisfy.
+var (
+	// ErrNoSnapshotInDatabase means the metadata database has no snapshot
+	// row at all.
+	ErrNoSnapshotInDatabase = errors.New("database contains no snapshot")
+	// ErrMultipleSnapshotsInDatabase means the metadata database holds
+	// more than the single snapshot an export is supposed to contain.
+	ErrMultipleSnapshotsInDatabase = errors.New(
+		"database contains more than one snapshot")
+)
+
 // SnapshotRepository provides access to the snapshots table and its
 // snapshot_files / snapshot_blobs association tables.
 type SnapshotRepository struct {
@@ -206,6 +218,48 @@ func (r *SnapshotRepository) GetByID(
 	return &snapshot, nil
 }
 
+// GetOnlySnapshot returns the sole snapshot in an exported per-snapshot
+// metadata database. The backup path writes each snapshot's database with
+// exactly one snapshot row (see cleanSnapshotDB), so restore and deep
+// verify expect exactly one. Zero rows return ErrNoSnapshotInDatabase and
+// more than one returns ErrMultipleSnapshotsInDatabase; callers treat
+// either as a failed identity check on the downloaded database.
+func (r *SnapshotRepository) GetOnlySnapshot(ctx context.Context) (*Snapshot, error) {
+	query := `
+		SELECT id, hostname, vaultik_version, vaultik_git_revision,
+			started_at, completed_at, file_count, chunk_count, blob_count,
+			total_size, blob_size, compression_ratio
+		FROM snapshots
+		LIMIT 2
+	`
+
+	rows, err := r.db.conn.QueryContext(ctx, query)
+	if err != nil {
+		return nil, fmt.Errorf("querying snapshots: %w", err)
+	}
+
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
+
+	snapshots, err := r.scanSnapshotRows(rows)
+	if err != nil {
+		return nil, err
+	}
+
+	switch len(snapshots) {
+	case 1:
+		return snapshots[0], nil
+	case 0:
+		return nil, ErrNoSnapshotInDatabase
+	default:
+		return nil, ErrMultipleSnapshotsInDatabase
+	}
+}
+
 // ListRecent returns up to limit snapshots, most recently started first.
 func (r *SnapshotRepository) ListRecent(
 	ctx context.Context, limit int,

internal/vaultik/restore.go

@@ -19,6 +19,7 @@ import (
 	"sneak.berlin/go/vaultik/internal/blobgen"
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/snapshot"
 	"sneak.berlin/go/vaultik/internal/types"
 )
 
@@ -41,6 +42,8 @@ var (
 		"restored file has trailing data after its last chunk")
 	errRestoreIncomplete = errors.New(
 		"restore loop ended with files still pending")
+	errSnapshotDBMismatch = errors.New(
+		"decrypted database is not the requested snapshot")
 )
 
 // snapshotDBFilename is the name the decrypted snapshot database is
@@ -655,7 +658,53 @@ func (v *Vaultik) downloadSnapshotDB(
 
 	log.Debug("Decrypted database", "size", ubytes(int64(len(dbData))))
 
-	return v.materializeSnapshotDB(dbData)
+	db, tempDir, err := v.materializeSnapshotDB(dbData)
+	if err != nil {
+		return nil, "", err
+	}
+
+	// Confirm the decrypted database really is the snapshot named by
+	// remoteKey before any files are read from it. On mismatch, close the
+	// database and remove its private directory so nothing is left behind.
+	err = v.verifySnapshotDBIdentity(db, snapshotID, remoteKey)
+	if err != nil {
+		_ = db.Close()
+		_ = v.Fs.RemoveAll(tempDir)
+
+		return nil, "", err
+	}
+
+	return db, tempDir, nil
+}
+
+// verifySnapshotDBIdentity confirms the decrypted metadata database really
+// is the snapshot named by remoteKey. age decryption proves the database
+// is readable, not that the object served at
+// metadata/<remoteKey>/db.zst.age is the snapshot that was requested: an
+// attacker who swaps in another valid db.zst.age (which needs no key
+// material) would otherwise redirect restore and deep verify to a
+// different snapshot's contents. The exported per-snapshot database holds
+// exactly one snapshot row, and a snapshot's remote key is derived from
+// that row's ID, so the database is the requested one exactly when its
+// sole snapshot hashes back to remoteKey. Comparing the requested
+// identifier directly would not do: it may be a remote-key prefix a
+// recovery host uses in place of a human snapshot ID it cannot know.
+func (v *Vaultik) verifySnapshotDBIdentity(
+	db *database.DB, requested, remoteKey string,
+) error {
+	repos := database.NewRepositories(db)
+
+	snap, err := repos.Snapshots.GetOnlySnapshot(v.ctx)
+	if err != nil {
+		return fmt.Errorf("checking identity of database for %s: %w", requested, err)
+	}
+
+	if snapshot.RemoteSnapshotKey(snap.ID.String()) != remoteKey {
+		return fmt.Errorf("%w: requested %s but the database is snapshot %s",
+			errSnapshotDBMismatch, requested, snap.ID)
+	}
+
+	return nil
 }
 
 // materializeSnapshotDB writes the decrypted snapshot database bytes into

internal/vaultik/restore_snapshot_identity_test.go

@@ -0,0 +1,251 @@
+package vaultik_test
+
+import (
+	"context"
+	"io"
+	"path/filepath"
+	"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"
+	"sneak.berlin/go/vaultik/internal/vaultik"
+)
+
+// TestRestoreAndDeepVerifyRejectSwappedDatabase proves that swapping two
+// snapshots' encrypted databases on the store is caught. age decryption
+// alone proves only that a database is readable; without an identity check
+// restore would happily write the wrong snapshot's files and deep verify
+// would report success. After the swap, restore and deep verify of A both
+// fail, and the error names the snapshot the database actually holds (B).
+func TestRestoreAndDeepVerifyRejectSwappedDatabase(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	storeDir := filepath.Join(tempDir, "remote")
+
+	chunkSize := int64(64 * 1024)
+
+	storer, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+
+	// Two snapshots with different content, backed up into one shared
+	// store. Different names give them different remote keys, so their
+	// metadata directories are distinct and can be tampered with alone.
+	dataA := filepath.Join(tempDir, "srcA")
+	require.NoError(t, fs.MkdirAll(dataA, 0o755))
+	require.NoError(t, afero.WriteFile(fs, filepath.Join(dataA, "a.bin"),
+		bytesPattern("alpha-", int(chunkSize*2)), 0o644))
+
+	dataB := filepath.Join(tempDir, "srcB")
+	require.NoError(t, fs.MkdirAll(dataB, 0o755))
+	require.NoError(t, afero.WriteFile(fs, filepath.Join(dataB, "b.bin"),
+		bytesPattern("beta-", int(chunkSize*2)), 0o644))
+
+	idA := backupNamedSnapshotToStore(ctx, t, fs, dataA, storer,
+		filepath.Join(tempDir, "idxA.sqlite"), "alpha")
+	idB := backupNamedSnapshotToStore(ctx, t, fs, dataB, storer,
+		filepath.Join(tempDir, "idxB.sqlite"), "beta")
+	require.NotEqual(t, idA, idB)
+
+	keyA := snapshot.RemoteSnapshotKey(idA)
+	keyB := snapshot.RemoteSnapshotKey(idB)
+	require.NotEqual(t, keyA, keyB)
+
+	// Baseline: each snapshot verifies against its own intact metadata.
+	require.NoError(t, newStoreClient(ctx, t, fs, storer).RunDeepVerify(
+		idA, &vaultik.VerifyOptions{Deep: true}))
+	require.NoError(t, newStoreClient(ctx, t, fs, storer).RunDeepVerify(
+		idB, &vaultik.VerifyOptions{Deep: true}))
+
+	// Swap the two snapshots' encrypted databases on the store.
+	swapStoreFiles(t, fs,
+		filepath.Join(storeDir, "metadata", keyA, "db.zst.age"),
+		filepath.Join(storeDir, "metadata", keyB, "db.zst.age"))
+
+	// Restore of A now decrypts B's database; the identity check must
+	// reject it and name the snapshot it actually found.
+	restoreErr := newStoreClient(ctx, t, fs, storer).Restore(&vaultik.RestoreOptions{
+		SnapshotID: idA,
+		TargetDir:  filepath.Join(tempDir, "restoreA"),
+	})
+	require.Error(t, restoreErr)
+	require.ErrorContains(t, restoreErr, idB)
+
+	// Deep verify of A must reject the swapped database for the same reason.
+	verifyErr := newStoreClient(ctx, t, fs, storer).RunDeepVerify(
+		idA, &vaultik.VerifyOptions{Deep: true})
+	require.Error(t, verifyErr)
+	require.ErrorContains(t, verifyErr, idB)
+}
+
+// TestDeepVerifyRejectsSwappedDatabaseWithEmptyManifest covers the case the
+// issue calls out: swapping in a database whose blob set is empty and
+// pairing it with an equally empty manifest. The manifest then agrees with
+// the database, so every blob-level check passes and deep verify used to
+// report success with zero blobs verified. The identity check rejects it
+// before any blob check runs.
+func TestDeepVerifyRejectsSwappedDatabaseWithEmptyManifest(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	storeDir := filepath.Join(tempDir, "remote")
+
+	chunkSize := int64(64 * 1024)
+
+	storer, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+
+	// Snapshot A: real content, so its manifest lists blobs.
+	dataA := filepath.Join(tempDir, "srcA")
+	require.NoError(t, fs.MkdirAll(dataA, 0o755))
+	require.NoError(t, afero.WriteFile(fs, filepath.Join(dataA, "a.bin"),
+		bytesPattern("alpha-", int(chunkSize*2)), 0o644))
+	idA := backupNamedSnapshotToStore(ctx, t, fs, dataA, storer,
+		filepath.Join(tempDir, "idxA.sqlite"), "alpha")
+
+	// Snapshot C: a single empty file, so it references no blobs and its
+	// manifest is empty. This is the database/manifest pair an attacker
+	// would swap in to make the blob checks vacuously pass.
+	dataC := filepath.Join(tempDir, "srcC")
+	require.NoError(t, fs.MkdirAll(dataC, 0o755))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(dataC, "empty.bin"), []byte{}, 0o644))
+	idC := backupNamedSnapshotToStore(ctx, t, fs, dataC, storer,
+		filepath.Join(tempDir, "idxC.sqlite"), "charlie")
+
+	keyA := snapshot.RemoteSnapshotKey(idA)
+	keyC := snapshot.RemoteSnapshotKey(idC)
+
+	// Replace A's database and manifest with C's empty pair.
+	copyStoreFile(t, fs,
+		filepath.Join(storeDir, "metadata", keyC, "db.zst.age"),
+		filepath.Join(storeDir, "metadata", keyA, "db.zst.age"))
+	copyStoreFile(t, fs,
+		filepath.Join(storeDir, "metadata", keyC, "manifest.json.zst"),
+		filepath.Join(storeDir, "metadata", keyA, "manifest.json.zst"))
+
+	verifyErr := newStoreClient(ctx, t, fs, storer).RunDeepVerify(
+		idA, &vaultik.VerifyOptions{Deep: true})
+	require.Error(t, verifyErr)
+	require.ErrorContains(t, verifyErr, idC)
+}
+
+// backupNamedSnapshotToStore backs up dataDir into the shared storer under
+// the given snapshot name and returns the human snapshot ID. Two snapshots
+// backed up under different names get different remote keys, so their
+// metadata directories on the store are distinct.
+func backupNamedSnapshotToStore(
+	ctx context.Context, t *testing.T, fs afero.Fs,
+	dataDir string, storer storage.Storer, dbPath, name string,
+) string {
+	t.Helper()
+
+	const (
+		chunkSize   = int64(64 * 1024)
+		maxBlobSize = int64(512 * 1024)
+	)
+
+	cfg := &config.Config{
+		AgeRecipients:    []string{testAgePublicKey},
+		AgeSecretKey:     testAgeSecretKey,
+		CompressionLevel: 3,
+		Hostname:         testHostname,
+	}
+
+	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:        chunkSize,
+		MaxBlobSize:      maxBlobSize,
+		CompressionLevel: cfg.CompressionLevel,
+		AgeRecipients:    cfg.AgeRecipients,
+		Repositories:     repos,
+	})
+
+	snapshotID, err := sm.CreateSnapshotWithName(
+		ctx, cfg.Hostname, name, "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 snapshotID
+}
+
+// newStoreClient builds a Vaultik that reads only from the store: the
+// secret key, the storer, and a filesystem, with no local index. This is
+// what restore and deep verify need.
+func newStoreClient(
+	ctx context.Context, t *testing.T, fs afero.Fs, storer storage.Storer,
+) *vaultik.Vaultik {
+	t.Helper()
+
+	v := &vaultik.Vaultik{
+		Config: &config.Config{
+			AgeSecretKey: testAgeSecretKey,
+			Hostname:     testHostname,
+		},
+		Storage: storer,
+		Fs:      fs,
+		Stdout:  io.Discard,
+		Stderr:  io.Discard,
+		UI:      ui.NewWithColor(io.Discard, false),
+	}
+	v.SetContext(ctx)
+
+	return v
+}
+
+// swapStoreFiles exchanges the contents of two files on the store.
+func swapStoreFiles(t *testing.T, fs afero.Fs, a, b string) {
+	t.Helper()
+
+	dataA, err := afero.ReadFile(fs, a)
+	require.NoError(t, err)
+
+	dataB, err := afero.ReadFile(fs, b)
+	require.NoError(t, err)
+
+	require.NoError(t, afero.WriteFile(fs, a, dataB, 0o644))
+	require.NoError(t, afero.WriteFile(fs, b, dataA, 0o644))
+}
+
+// copyStoreFile overwrites dst with the contents of src on the store.
+func copyStoreFile(t *testing.T, fs afero.Fs, src, dst string) {
+	t.Helper()
+
+	data, err := afero.ReadFile(fs, src)
+	require.NoError(t, err)
+
+	require.NoError(t, afero.WriteFile(fs, dst, data, 0o644))
+}

internal/vaultik/verify.go

@@ -191,24 +191,9 @@ func (v *Vaultik) loadVerificationData(
 		v.stdoutf("Downloading and decrypting database...\n")
 	}
 
-	// Download and decrypt database
+	tdb, err := v.downloadVerifiedSnapshotDB(snapshotID, remoteKey, opts, result, identity)
-	dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey)
-	log.Info("Downloading encrypted database", "path", dbPath)
-
-	dbReader, err := v.Storage.Get(v.ctx, dbPath)
 	if err != nil {
-		return nil, nil, nil, v.deepVerifyFailure(result, opts,
+		return nil, nil, nil, err
-			fmt.Sprintf("failed to download database: %v", err),
-			fmt.Errorf("failed to download database: %w", err))
-	}
-
-	defer func() { _ = dbReader.Close() }()
-
-	tdb, err := v.decryptAndLoadDatabase(dbReader, identity)
-	if err != nil {
-		return nil, nil, nil, v.deepVerifyFailure(result, opts,
-			fmt.Sprintf("failed to decrypt database: %v", err),
-			fmt.Errorf("failed to decrypt database: %w", err))
 	}
 
 	dbBlobs, err := v.getBlobsFromDatabase(tdb.db.Conn())
@@ -237,6 +222,45 @@ func (v *Vaultik) loadVerificationData(
 	return manifest, tdb, dbBlobs, nil
 }
 
+// downloadVerifiedSnapshotDB downloads and decrypts the snapshot metadata
+// database and confirms it really is the snapshot named by remoteKey
+// before any of its rows are trusted (see verifySnapshotDBIdentity). On
+// any failure it records the failure in result and returns the error the
+// caller should propagate; the temp database is closed on a rejected
+// identity so nothing is left on disk.
+func (v *Vaultik) downloadVerifiedSnapshotDB(
+	snapshotID, remoteKey string, opts *VerifyOptions, result *VerifyResult,
+	identity age.Identity,
+) (*tempDB, error) {
+	dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey)
+	log.Info("Downloading encrypted database", "path", dbPath)
+
+	dbReader, err := v.Storage.Get(v.ctx, dbPath)
+	if err != nil {
+		return nil, v.deepVerifyFailure(result, opts,
+			fmt.Sprintf("failed to download database: %v", err),
+			fmt.Errorf("failed to download database: %w", err))
+	}
+
+	defer func() { _ = dbReader.Close() }()
+
+	tdb, err := v.decryptAndLoadDatabase(dbReader, identity)
+	if err != nil {
+		return nil, v.deepVerifyFailure(result, opts,
+			fmt.Sprintf("failed to decrypt database: %v", err),
+			fmt.Errorf("failed to decrypt database: %w", err))
+	}
+
+	err = v.verifySnapshotDBIdentity(tdb.db, snapshotID, remoteKey)
+	if err != nil {
+		_ = tdb.Close()
+
+		return nil, v.deepVerifyFailure(result, opts, err.Error(), err)
+	}
+
+	return tdb, nil
+}
+
 // runVerificationSteps executes manifest verification, blob existence
 // check, and deep content verification.
 func (v *Vaultik) runVerificationSteps(