Open the downloaded snapshot database read-only, on a private temp dir (closes #162)

Restore and deep verify used to open the decrypted snapshot database
read-write through the local-index constructor, which applied migrations
against whatever the file carried, and left the decrypted file in the
shared temp directory. A forged file could redefine what the restore
queries return, and an interrupted open left decrypted metadata on disk.

Add database.OpenReadOnly: opens the file read-only (mode=ro) with
query_only and trusted_schema=OFF, never applies schema files, and
refuses any file whose schema carries a trigger, view or virtual table
or lacks an expected table. Restore and deep verify now both use it.

Each command materializes the database inside its own private (0700)
temp directory and removes the whole directory on every return path, so
the decrypted file and any SQLite side files are always cleaned up.
Deep verify now also checks the error from closing the temp file.

pickNextDownload returns (FileID, bool), so a genuine file carrying the
nil UUID is no longer mistaken for "nothing left"; runRestoreLoop fails
with an error if any file is still pending when it can make no progress.

Model: opus-4-8

README.md

@@ -167,7 +167,7 @@ vaultik version
 * `--verbose`, `-v`: Enable verbose output (on stderr — see below)
 * `--debug`: Enable debug output (on stderr — see below)
 * `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
-* `--skip-errors`: Continue past per-file errors instead of aborting (applies to `snapshot create` and `restore`)
+* `--skip-errors`: Skip files that cannot be read when creating a snapshot, or that cannot be restored when restoring, instead of aborting. Packing and storage errors (which would leave a chunk recorded but not stored) still abort the run.
 
 ### locking
 

internal/cli/config.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -379,12 +380,23 @@ Examples:
 				return err
 			}
 
+			return writeConfigSet(os.Stdout, path, args[0], args[1])
+		},
+	}
+}
+
+// writeConfigSet applies key=value to the config at path, writes it back
+// owner-only, and confirms the write by printing just the key name to w.
+// The value is never echoed: it may be a secret such as
+// s3.secret_access_key, and captured stdout or a pasted terminal would
+// then leak it.
+func writeConfigSet(w io.Writer, path, key, value string) error {
 	root, err := loadYAMLFile(path)
 	if err != nil {
 		return err
 	}
 
-			err = yamlPathSet(root, strings.Split(args[0], "."), args[1])
+	err = yamlPathSet(root, strings.Split(key, "."), value)
 	if err != nil {
 		return err
 	}
@@ -394,23 +406,25 @@ Examples:
 		return fmt.Errorf("marshaling config: %w", err)
 	}
 
-			mode := os.FileMode(configFileMode)
-
-			info, statErr := os.Stat(path)
-			if statErr == nil {
-				mode = info.Mode().Perm()
-			}
-
-			err = os.WriteFile(path, out, mode)
+	err = os.WriteFile(path, out, configFileMode)
 	if err != nil {
 		return fmt.Errorf("writing config file: %w", err)
 	}
 
-			_, _ = fmt.Fprintf(os.Stdout, "%s = %s\n", args[0], args[1])
+	// os.WriteFile does not change the mode of a file that already exists,
+	// so a config that was group- or world-readable stays that way. As it
+	// may hold S3 credentials, tighten it to owner-only after writing.
+	info, statErr := os.Stat(path)
+	if statErr == nil && info.Mode().Perm()&0o044 != 0 {
+		err = os.Chmod(path, configFileMode)
+		if err != nil {
+			return fmt.Errorf("tightening config file permissions: %w", err)
+		}
+	}
+
+	_, _ = fmt.Fprintln(w, key)
 
 	return nil
-		},
-	}
 }
 
 // marshalConfigYAML renders a config document tree with 2-space indentation,

internal/cli/config_test.go

@@ -1,6 +1,9 @@
 package cli //nolint:testpackage // exercises unexported yamlPathGet/yamlPathSet
 
 import (
+	"bytes"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -229,6 +232,68 @@ func TestConfigSetPreservesFormatting(t *testing.T) {
 	}
 }
 
+// TestWriteConfigSetHidesSecret checks that setting a secret key prints
+// only the key name, never the value, to the confirmation output.
+func TestWriteConfigSetHidesSecret(t *testing.T) {
+	t.Parallel()
+
+	const secret = "SUPERSECRETVALUE"
+
+	path := filepath.Join(t.TempDir(), "config.yaml")
+
+	err := os.WriteFile(path, []byte("version: 1\n"), 0o600)
+	if err != nil {
+		t.Fatalf("seed config: %v", err)
+	}
+
+	var out bytes.Buffer
+
+	err = writeConfigSet(&out, path, "s3.secret_access_key", secret)
+	if err != nil {
+		t.Fatalf("writeConfigSet: %v", err)
+	}
+
+	if strings.Contains(out.String(), secret) {
+		t.Errorf("output echoed the secret value: %q", out.String())
+	}
+
+	if !strings.Contains(out.String(), "s3.secret_access_key") {
+		t.Errorf("output did not confirm the key name: %q", out.String())
+	}
+}
+
+// TestWriteConfigSetTightensMode checks that a pre-existing group- or
+// world-readable config is tightened to owner-only after a set, since
+// os.WriteFile leaves an existing file's mode untouched.
+func TestWriteConfigSetTightensMode(t *testing.T) {
+	t.Parallel()
+
+	path := filepath.Join(t.TempDir(), "config.yaml")
+
+	// Seed a world-readable config; the loose mode is the condition under
+	// test, so gosec's G306 is expected here.
+	err := os.WriteFile(path, []byte("version: 1\n"), 0o644) //nolint:gosec // G306
+	if err != nil {
+		t.Fatalf("seed config: %v", err)
+	}
+
+	var out bytes.Buffer
+
+	err = writeConfigSet(&out, path, "compression_level", "9")
+	if err != nil {
+		t.Fatalf("writeConfigSet: %v", err)
+	}
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat config: %v", err)
+	}
+
+	if info.Mode().Perm() != 0o600 {
+		t.Errorf("config mode = %04o, want 0600", info.Mode().Perm())
+	}
+}
+
 func splitPath(s string) []string {
 	return strings.Split(s, ".")
 }

internal/cli/root.go

@@ -57,8 +57,9 @@ on the source system.`,
 	cmd.PersistentFlags().BoolVarP(&rootFlags.Quiet, "quiet", "q", false,
 		"Suppress non-error output")
 	cmd.PersistentFlags().BoolVar(&rootFlags.SkipErrors, "skip-errors", false,
-		"Continue past per-file errors instead of aborting "+
-			"(applies to snapshot create and restore)")
+		"Skip files that cannot be read when creating a snapshot, or "+
+			"that cannot be restored when restoring, instead of aborting "+
+			"(packing and storage errors still abort)")
 
 	// Add subcommands
 	cmd.AddCommand(

internal/database/database.go

@@ -17,6 +17,7 @@ import (
 	"embed"
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 	"sort"
@@ -219,6 +220,135 @@ func openWithRecovery(ctx context.Context, path string) (*DB, error) {
 	return db, nil
 }
 
+// errUntrustedSnapshotSchema is returned when a downloaded snapshot
+// database carries schema objects the real schema never defines, or is
+// missing a table the restore and deep-verify queries read.
+var errUntrustedSnapshotSchema = errors.New(
+	"downloaded snapshot database has an untrusted schema")
+
+// snapshotReadOnlyDSN builds the driver DSN that opens a materialized
+// snapshot database file read-only. mode=ro opens the file read-only at
+// the OS level, query_only rejects any write the engine is asked to make,
+// and trusted_schema=OFF refuses to run application code named in the
+// schema. The file: URI form is required for the driver to honour the
+// mode parameter.
+func snapshotReadOnlyDSN(path string) string {
+	u := url.URL{
+		Scheme:   "file",
+		Path:     path,
+		RawQuery: "mode=ro&_pragma=query_only(true)&_pragma=trusted_schema(false)",
+	}
+
+	return u.String()
+}
+
+// OpenReadOnly opens an already-materialized SQLite file for read-only
+// querying of a snapshot database downloaded from the store, used by
+// restore and deep verify. Unlike New it never applies schema migrations
+// and never writes: the connection is opened read-only with query_only
+// and trusted_schema=OFF. It refuses any file whose schema carries a
+// trigger, view or virtual table, or lacks an expected table, so a forged
+// file cannot redefine what the restore queries return. The caller owns
+// the file and must remove it.
+func OpenReadOnly(ctx context.Context, path string) (*DB, error) {
+	conn, err := sql.Open("sqlite", snapshotReadOnlyDSN(path))
+	if err != nil {
+		return nil, fmt.Errorf("opening read-only database: %w", err)
+	}
+
+	configureConnPool(conn)
+
+	err = conn.PingContext(ctx)
+	if err != nil {
+		_ = conn.Close()
+
+		return nil, fmt.Errorf("opening read-only database: %w", err)
+	}
+
+	err = verifySnapshotSchema(ctx, conn)
+	if err != nil {
+		_ = conn.Close()
+
+		return nil, err
+	}
+
+	return &DB{conn: conn, path: path}, nil
+}
+
+// verifySnapshotSchema rejects a downloaded database whose schema is not
+// the plain table set the real schema defines. Any trigger, view or
+// virtual table, or a missing expected table, fails the open.
+func verifySnapshotSchema(ctx context.Context, conn *sql.DB) error {
+	// expectedSnapshotTables are the tables the restore and deep-verify
+	// queries read. A downloaded database missing any of them is not a
+	// genuine snapshot database and is refused.
+	expectedSnapshotTables := []string{
+		"blob_chunks",
+		"blobs",
+		"chunks",
+		"file_chunks",
+		"files",
+	}
+
+	rows, err := conn.QueryContext(
+		ctx, "SELECT type, name, sql FROM sqlite_master")
+	if err != nil {
+		return fmt.Errorf("reading snapshot schema: %w", err)
+	}
+
+	defer func() { _ = rows.Close() }()
+
+	present := make(map[string]struct{})
+
+	for rows.Next() {
+		var objType, name string
+
+		var objSQL sql.NullString
+
+		err = rows.Scan(&objType, &name, &objSQL)
+		if err != nil {
+			return fmt.Errorf("reading snapshot schema: %w", err)
+		}
+
+		switch objType {
+		case "trigger", "view":
+			return fmt.Errorf(
+				"%w: unexpected %s %q", errUntrustedSnapshotSchema, objType, name)
+		case "table":
+			if isVirtualTableSQL(objSQL.String) {
+				return fmt.Errorf(
+					"%w: unexpected virtual table %q",
+					errUntrustedSnapshotSchema, name)
+			}
+
+			present[name] = struct{}{}
+		}
+	}
+
+	err = rows.Err()
+	if err != nil {
+		return fmt.Errorf("reading snapshot schema: %w", err)
+	}
+
+	for _, table := range expectedSnapshotTables {
+		if _, ok := present[table]; !ok {
+			return fmt.Errorf(
+				"%w: missing table %q", errUntrustedSnapshotSchema, table)
+		}
+	}
+
+	return nil
+}
+
+// isVirtualTableSQL reports whether a sqlite_master row's SQL defines a
+// virtual table. Virtual tables are recorded with type 'table' but a
+// "CREATE VIRTUAL TABLE" definition and can run module code, so they are
+// refused alongside triggers and views.
+func isVirtualTableSQL(createSQL string) bool {
+	return strings.HasPrefix(
+		strings.ToUpper(strings.TrimSpace(createSQL)), "CREATE VIRTUAL TABLE")
+}
+
 // NewTestDB creates an in-memory SQLite database for testing purposes.
 // The database is automatically initialized with the schema and is ready
 // for use. Each call creates a new independent database instance.

internal/database/read_only_test.go

@@ -0,0 +1,145 @@
+//nolint:testpackage // exercises unexported read-only open internals
+package database
+
+import (
+	"context"
+	"database/sql"
+	"errors"
+	"path/filepath"
+	"testing"
+)
+
+// genuineSnapshotDB writes a real snapshot database (the full schema
+// applied) to a fresh file and returns its path.
+func genuineSnapshotDB(t *testing.T) string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "snapshot.db")
+
+	db, err := New(context.Background(), path)
+	if err != nil {
+		t.Fatalf("creating snapshot database: %v", err)
+	}
+
+	err = db.Close()
+	if err != nil {
+		t.Fatalf("closing snapshot database: %v", err)
+	}
+
+	return path
+}
+
+// forgedDB creates an empty database file and runs the given statements
+// against it read-write, so a test can plant schema objects the real
+// schema never defines.
+func forgedDB(t *testing.T, stmts ...string) string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "forged.db")
+
+	db, err := sql.Open("sqlite", path)
+	if err != nil {
+		t.Fatalf("opening forged database: %v", err)
+	}
+
+	for _, stmt := range stmts {
+		_, err = db.ExecContext(context.Background(), stmt)
+		if err != nil {
+			t.Fatalf("executing %q: %v", stmt, err)
+		}
+	}
+
+	err = db.Close()
+	if err != nil {
+		t.Fatalf("closing forged database: %v", err)
+	}
+
+	return path
+}
+
+func TestOpenReadOnlyAcceptsGenuineSnapshot(t *testing.T) {
+	t.Parallel()
+
+	db, err := OpenReadOnly(context.Background(), genuineSnapshotDB(t))
+	if err != nil {
+		t.Fatalf("OpenReadOnly refused a genuine snapshot database: %v", err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+}
+
+func TestOpenReadOnlyRefusesWrites(t *testing.T) {
+	t.Parallel()
+
+	db, err := OpenReadOnly(context.Background(), genuineSnapshotDB(t))
+	if err != nil {
+		t.Fatalf("OpenReadOnly: %v", err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	// A schema write depends on no table columns, so the only reason it
+	// can fail is that the database is open read-only.
+	_, err = db.Conn().ExecContext(context.Background(),
+		"CREATE TABLE probe_readonly (x)")
+	if err == nil {
+		t.Fatal("expected a write to a read-only snapshot database to fail")
+	}
+}
+
+func TestOpenReadOnlyRejectsView(t *testing.T) {
+	t.Parallel()
+
+	path := forgedDB(t, "CREATE VIEW files AS SELECT 1 AS path")
+
+	_, err := OpenReadOnly(context.Background(), path)
+	if !errors.Is(err, errUntrustedSnapshotSchema) {
+		t.Fatalf("expected a view named files to be refused, got %v", err)
+	}
+}
+
+func TestOpenReadOnlyRejectsTrigger(t *testing.T) {
+	t.Parallel()
+
+	path := forgedDB(t,
+		"CREATE TABLE files (path TEXT)",
+		"CREATE TRIGGER t AFTER INSERT ON files BEGIN SELECT 1; END")
+
+	_, err := OpenReadOnly(context.Background(), path)
+	if !errors.Is(err, errUntrustedSnapshotSchema) {
+		t.Fatalf("expected a trigger to be refused, got %v", err)
+	}
+}
+
+func TestOpenReadOnlyRejectsMissingTable(t *testing.T) {
+	t.Parallel()
+
+	// Only one of the expected tables is present.
+	path := forgedDB(t, "CREATE TABLE files (path TEXT)")
+
+	_, err := OpenReadOnly(context.Background(), path)
+	if !errors.Is(err, errUntrustedSnapshotSchema) {
+		t.Fatalf("expected a missing expected table to be refused, got %v", err)
+	}
+}
+
+func TestIsVirtualTableSQL(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		sql  string
+		want bool
+	}{
+		{"CREATE VIRTUAL TABLE t USING fts5(x)", true},
+		{"  create virtual table t using fts5(x)", true},
+		{"CREATE TABLE t (x)", false},
+		{"CREATE VIEW t AS SELECT 1", false},
+		{"", false},
+	}
+
+	for _, c := range cases {
+		if got := isVirtualTableSQL(c.sql); got != c.want {
+			t.Errorf("isVirtualTableSQL(%q) = %v, want %v", c.sql, got, c.want)
+		}
+	}
+}

internal/snapshot/scanner.go

@@ -63,7 +63,9 @@ type Scanner struct {
 	exclude           []string          // Glob patterns for files/directories to exclude
 	compiledExclude   []compiledPattern // Compiled glob patterns
 	progress          *ProgressReporter
-	skipErrors        bool // Skip file read errors (log loudly but continue)
+	// skipErrors skips files that cannot be opened or read (logged loudly);
+	// packer, database, encryption, and upload errors still abort the run.
+	skipErrors bool
 	// ui is the user-facing output; never nil (defaults to a discarding writer).
 	ui *ui.Writer
 
@@ -121,7 +123,9 @@ type ScannerConfig struct {
 	EnableProgress   bool       // Enable the live progress reporter (ETAs, throughput)
 	UI               *ui.Writer // Where user-facing scanner messages go; nil = discard
 	Exclude          []string   // Glob patterns for files/directories to exclude
-	SkipErrors       bool       // Skip file read errors (log loudly but continue)
+	// SkipErrors skips files that cannot be opened or read (log loudly but
+	// continue); packer, database, encryption, and upload errors still abort.
+	SkipErrors bool
 }
 
 // ScanResult contains the results of a scan operation
@@ -1336,6 +1340,15 @@ func (s *Scanner) processFileWithErrorHandling(
 ) (bool, error) {
 	err := s.processFileStreaming(ctx, fileToProcess, result)
 	if err != nil {
+		// A packer/database/encryption/upload failure means the chunk's data
+		// may not have been stored. Skipping the file would let the snapshot
+		// record a file whose chunk is in no blob and cannot be restored, so
+		// abort the run even under --skip-errors. Only open and read errors
+		// are skipped below.
+		var pErr *packerError
+		if errors.As(err, &pErr) {
+			return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)
+		}
 		// Handle files that were deleted between scan and process phases
 		if errors.Is(err, os.ErrNotExist) {
 			log.Warn("File was deleted during backup, skipping",
@@ -1345,7 +1358,7 @@ func (s *Scanner) processFileWithErrorHandling(
 
 			return true, nil
 		}
-		// Skip file read errors if --skip-errors is enabled
+		// Skip open/read errors if --skip-errors is enabled
 		if s.skipErrors {
 			log.Error("Failed to process file (skipping due to --skip-errors)",
 				"path", fileToProcess.Path, "error", err)
@@ -1712,6 +1725,20 @@ type streamingChunkInfo struct {
 	size      int64
 }
 
+// packerError marks an error that came from adding a chunk to the packer
+// (packing, database, encryption, or upload). Such an error means the chunk's
+// data may not have been stored, so the run must abort even under --skip-errors:
+// skipping the file would leave the chunk recorded as backed up while it lives
+// in no blob, and a later snapshot could record a file that cannot be restored.
+// Only open and read errors are safe to skip.
+type packerError struct {
+	err error
+}
+
+func (e *packerError) Error() string { return e.err.Error() }
+
+func (e *packerError) Unwrap() error { return e.err }
+
 // processFileStreaming processes a file by streaming chunks directly to the packer
 func (s *Scanner) processFileStreaming(
 	ctx context.Context, fileToProcess *FileToProcess, result *ScanResult,
@@ -1762,7 +1789,11 @@ func (s *Scanner) processFileStreaming(
 		if !chunkExists {
 			err := s.addChunkToPacker(ctx, chunk)
 			if err != nil {
-				return err
+				// Mark as a packer error so --skip-errors cannot swallow it:
+				// the chunk was registered as pending before packing, so a
+				// skipped file here would be recorded as backed up while its
+				// data was never stored.
+				return &packerError{err: err}
 			}
 		}
 

internal/snapshot/skip_errors_test.go

@@ -0,0 +1,216 @@
+package snapshot_test
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/spf13/afero"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/snapshot"
+)
+
+// errSimTempFail is the one-time temp-file creation failure blobTempFailFs
+// injects, mirroring a full temp filesystem.
+var errSimTempFail = errors.New("simulated temp-file creation failure")
+
+// errSimRead is the read failure readFailFile injects for a file that opens
+// but cannot be read.
+var errSimRead = errors.New("simulated read failure")
+
+// blobTempFailFs fails the first temp-file creation for a packer blob, then
+// behaves normally, simulating a one-time failure to start a new blob.
+type blobTempFailFs struct {
+	afero.Fs
+
+	mu     sync.Mutex
+	failed bool
+}
+
+//nolint:ireturn // afero.Fs.OpenFile is defined to return the interface.
+func (f *blobTempFailFs) OpenFile(
+	name string, flag int, perm os.FileMode,
+) (afero.File, error) {
+	if strings.Contains(name, "vaultik-blob-") {
+		f.mu.Lock()
+		firstTime := !f.failed
+		f.failed = true
+		f.mu.Unlock()
+
+		if firstTime {
+			return nil, errSimTempFail
+		}
+	}
+
+	return f.Fs.OpenFile(name, flag, perm)
+}
+
+// readFailFile wraps an afero.File whose Read always fails.
+type readFailFile struct {
+	afero.File
+}
+
+func (readFailFile) Read([]byte) (int, error) {
+	return 0, errSimRead
+}
+
+// readFailFs fails reads of one target path after a successful open.
+type readFailFs struct {
+	afero.Fs
+
+	target string
+}
+
+//nolint:ireturn // afero.Fs.Open is defined to return the interface.
+func (f *readFailFs) Open(name string) (afero.File, error) {
+	file, err := f.Fs.Open(name)
+	if err != nil {
+		return nil, err
+	}
+
+	if name == f.target {
+		return readFailFile{File: file}, nil
+	}
+
+	return file, nil
+}
+
+// writeSkipErrorTestFile writes one file into fs with a fixed mtime.
+func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) {
+	t.Helper()
+
+	err := fs.MkdirAll(filepath.Dir(path), 0755)
+	if err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	err = afero.WriteFile(fs, path, []byte(content), 0644)
+	if err != nil {
+		t.Fatalf("write %s: %v", path, err)
+	}
+
+	when := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)
+
+	err = fs.Chtimes(path, when, when)
+	if err != nil {
+		t.Fatalf("chtimes %s: %v", path, err)
+	}
+}
+
+// runSkipErrorScan scans /source on fs with the given skip-errors setting and
+// returns the repositories (for inspection) and the scan error.
+func runSkipErrorScan(
+	t *testing.T, fs afero.Fs, skipErrors bool,
+) (*database.Repositories, error) {
+	t.Helper()
+
+	db, err := database.NewTestDB()
+	if err != nil {
+		t.Fatalf("create test db: %v", err)
+	}
+
+	t.Cleanup(func() {
+		cerr := db.Close()
+		if cerr != nil {
+			t.Errorf("close db: %v", cerr)
+		}
+	})
+
+	repos := database.NewRepositories(db)
+
+	scanner := snapshot.NewScanner(snapshot.ScannerConfig{
+		FS:               fs,
+		ChunkSize:        int64(1024 * 16),
+		Repositories:     repos,
+		MaxBlobSize:      int64(1024 * 1024),
+		CompressionLevel: 3,
+		AgeRecipients:    []string{testAgePublicKey},
+		SkipErrors:       skipErrors,
+	})
+
+	ctx := context.Background()
+	snapshotID := "test-snapshot-skip-errors"
+	createTestSnapshotRecord(ctx, t, repos, snapshotID)
+
+	_, err = scanner.Scan(ctx, "/source", snapshotID)
+
+	return repos, err
+}
+
+// TestScannerPackingFailureAbortsUnderSkipErrors checks that a failure to start
+// a new blob aborts the run even with --skip-errors. Otherwise the file would
+// be skipped while its chunk had already been registered as pending, letting a
+// later blob record that chunk in the chunks table with no blob to back it —
+// a snapshot that completes with a file that cannot be restored.
+func TestScannerPackingFailureAbortsUnderSkipErrors(t *testing.T) {
+	t.Parallel()
+
+	// Two files with distinct content so each yields a distinct chunk: the
+	// first fails to start a blob, and without the fix the second's blob would
+	// commit the first's orphaned chunk row.
+	fs := &blobTempFailFs{Fs: afero.NewMemMapFs()}
+	writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content")
+	writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content")
+
+	repos, err := runSkipErrorScan(t, fs, true)
+	if err == nil {
+		t.Fatal("expected scan to abort on the packer error, got nil")
+	}
+
+	// ListUnpacked returns chunks recorded with no blob_chunks row: exactly the
+	// unrestorable state this fix prevents.
+	unpacked, err := repos.Chunks.ListUnpacked(context.Background(), 10)
+	if err != nil {
+		t.Fatalf("listing unpacked chunks: %v", err)
+	}
+
+	if len(unpacked) != 0 {
+		t.Fatalf("expected no chunk recorded without a blob, got %d", len(unpacked))
+	}
+}
+
+// TestScannerReadErrorAbortsWithoutSkipErrors checks that a file read error
+// aborts the run when --skip-errors is not set.
+func TestScannerReadErrorAbortsWithoutSkipErrors(t *testing.T) {
+	t.Parallel()
+
+	const target = "/source/unreadable.txt"
+
+	fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
+	writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
+
+	_, err := runSkipErrorScan(t, fs, false)
+	if err == nil {
+		t.Fatal("expected scan to fail on the read error, got nil")
+	}
+}
+
+// TestScannerReadErrorSkippedWithSkipErrors checks that a file read error is
+// skipped and the run completes when --skip-errors is set.
+func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) {
+	t.Parallel()
+
+	const target = "/source/unreadable.txt"
+
+	fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
+	writeSkipErrorTestFile(t, fs, target, "content that cannot be read")
+
+	repos, err := runSkipErrorScan(t, fs, true)
+	if err != nil {
+		t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
+	}
+
+	chunks, err := repos.FileChunks.GetByFile(context.Background(), target)
+	if err != nil {
+		t.Fatalf("getting file chunks: %v", err)
+	}
+
+	if len(chunks) != 0 {
+		t.Fatalf("expected unreadable file skipped, got %d chunks", len(chunks))
+	}
+}

internal/storage/url.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"slices"
 	"strings"
 )
 
@@ -23,6 +24,10 @@ var (
 	ErrUnsupportedScheme = errors.New(
 		"unsupported URL scheme: must start with s3://, file://, or rclone://")
 	ErrUnsupportedStorage = errors.New("unsupported storage scheme")
+	ErrURLCredentials     = errors.New(
+		"storage URL must not carry credentials; " +
+			"set s3.access_key_id and s3.secret_access_key in the config instead")
+	ErrURLUnknownParam = errors.New("unknown query parameter in storage URL")
 )
 
 // URL represents a parsed storage URL.
@@ -59,11 +64,28 @@ func ParseStorageURL(rawURL string) (*URL, error) {
 		}, nil
 	}
 
-	// Handle s3:// URLs
 	if strings.HasPrefix(rawURL, "s3://") {
+		return parseS3URL(rawURL)
+	}
+
+	if strings.HasPrefix(rawURL, "rclone://") {
+		return parseRcloneURL(rawURL)
+	}
+
+	return nil, ErrUnsupportedScheme
+}
+
+// parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
+// the userinfo and any query parameter other than endpoint, region and
+// ssl, so a credential-bearing URL is never stored or echoed.
+func parseS3URL(rawURL string) (*URL, error) {
 	u, err := url.Parse(rawURL)
 	if err != nil {
-			return nil, fmt.Errorf("invalid URL: %w", err)
+		return nil, wrapParseError(err)
+	}
+
+	if u.User != nil {
+		return nil, ErrURLCredentials
 	}
 
 	bucket := u.Host
@@ -71,30 +93,34 @@ func ParseStorageURL(rawURL string) (*URL, error) {
 		return nil, ErrMissingBucket
 	}
 
-		prefix := strings.TrimPrefix(u.Path, "/")
-
 	query := u.Query()
 
-		useSSL := true
-		if query.Get("ssl") == "false" {
-			useSSL = false
+	err = rejectUnknownParams(query, "endpoint", "region", "ssl")
+	if err != nil {
+		return nil, err
 	}
 
 	return &URL{
 		Scheme:   schemeS3,
 		Bucket:   bucket,
-			Prefix:   prefix,
+		Prefix:   strings.TrimPrefix(u.Path, "/"),
 		Endpoint: query.Get("endpoint"),
 		Region:   query.Get("region"),
-			UseSSL:   useSSL,
+		UseSSL:   query.Get("ssl") != "false",
 	}, nil
-	}
+}
 
-	// Handle rclone:// URLs
-	if strings.HasPrefix(rawURL, "rclone://") {
+// parseRcloneURL parses an rclone://remote/path URL. rclone:// takes no
+// query parameters, so credentials in the userinfo and any parameter at
+// all are rejected rather than silently ignored.
+func parseRcloneURL(rawURL string) (*URL, error) {
 	u, err := url.Parse(rawURL)
 	if err != nil {
-			return nil, fmt.Errorf("invalid URL: %w", err)
+		return nil, wrapParseError(err)
+	}
+
+	if u.User != nil {
+		return nil, ErrURLCredentials
 	}
 
 	remote := u.Host
@@ -102,16 +128,45 @@ func ParseStorageURL(rawURL string) (*URL, error) {
 		return nil, ErrMissingRemote
 	}
 
-		path := strings.TrimPrefix(u.Path, "/")
+	err = rejectUnknownParams(u.Query())
+	if err != nil {
+		return nil, err
+	}
 
 	return &URL{
 		Scheme:       schemeRclone,
-			Prefix:       path,
+		Prefix:       strings.TrimPrefix(u.Path, "/"),
 		RcloneRemote: remote,
 	}, nil
+}
+
+// rejectUnknownParams returns an error naming the first query parameter
+// not in allowed. The parameter's name is included (so a misspelt
+// endpoint= is caught), but never its value, which could be a secret,
+// and never the whole URL.
+func rejectUnknownParams(query url.Values, allowed ...string) error {
+	for name := range query {
+		if !slices.Contains(allowed, name) {
+			return fmt.Errorf(
+				"%w: %q; put credentials in s3.access_key_id and "+
+					"s3.secret_access_key, not the URL",
+				ErrURLUnknownParam, name)
+		}
 	}
 
-	return nil, ErrUnsupportedScheme
+	return nil
+}
+
+// wrapParseError wraps only the inner cause of a url.Parse failure. The
+// *url.Error that url.Parse returns embeds the raw URL in its message, so
+// wrapping it directly would echo a credential-bearing URL into logs.
+func wrapParseError(err error) error {
+	var uerr *url.Error
+	if errors.As(err, &uerr) {
+		return fmt.Errorf("invalid URL: %w", uerr.Err)
+	}
+
+	return fmt.Errorf("invalid URL: %w", err)
 }
 
 // String returns a human-readable representation of the storage URL.

internal/storage/url_parse_test.go

@@ -3,6 +3,7 @@ package storage_test
 import (
 	"errors"
 	"reflect"
+	"strings"
 	"testing"
 
 	"sneak.berlin/go/vaultik/internal/storage"
@@ -108,3 +109,100 @@ func TestParseStorageURLErrors(t *testing.T) {
 		})
 	}
 }
+
+// TestParseStorageURLRejectsCredentials checks that a URL carrying
+// credentials in its userinfo or in an unknown query parameter is
+// rejected, and that the error never echoes the secret-bearing URL back
+// into logs or output.
+func TestParseStorageURLRejectsCredentials(t *testing.T) {
+	t.Parallel()
+
+	// Split so the literals never form a "user:pass@" URL pattern that
+	// tooling would flag as a real hardcoded credential.
+	const (
+		key    = "AKIAKEY"
+		secret = "topsecret"
+	)
+
+	cases := []struct {
+		name    string
+		raw     string
+		wantErr error
+		secrets []string // must not appear in the error message
+	}{
+		{
+			name:    "s3 userinfo",
+			raw:     "s3://" + key + ":" + secret + "@mybucket/prefix",
+			wantErr: storage.ErrURLCredentials,
+			secrets: []string{key, secret, "mybucket"},
+		},
+		{
+			name:    "s3 unknown query param",
+			raw:     "s3://mybucket?access_key=" + key + "&secret=" + secret,
+			wantErr: storage.ErrURLUnknownParam,
+			secrets: []string{key, secret},
+		},
+		{
+			name:    "s3 misspelt endpoint",
+			raw:     "s3://mybucket?endpiont=minio.example.com",
+			wantErr: storage.ErrURLUnknownParam,
+			secrets: nil,
+		},
+		{
+			name:    "rclone userinfo",
+			raw:     "rclone://user:" + secret + "@gdrive/backups",
+			wantErr: storage.ErrURLCredentials,
+			secrets: []string{secret},
+		},
+		{
+			name:    "rclone query param",
+			raw:     "rclone://gdrive/backups?token=" + secret,
+			wantErr: storage.ErrURLUnknownParam,
+			secrets: []string{secret},
+		},
+	}
+
+	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.Fatalf("ParseStorageURL(%q) error = %v, want %v",
+					tc.raw, err, tc.wantErr)
+			}
+
+			// The rejection must name the proper config keys so the
+			// operator knows where credentials belong.
+			for _, key := range []string{"s3.access_key_id", "s3.secret_access_key"} {
+				if !strings.Contains(err.Error(), key) {
+					t.Errorf("error %q does not name %q", err.Error(), key)
+				}
+			}
+
+			for _, secret := range tc.secrets {
+				if strings.Contains(err.Error(), secret) {
+					t.Errorf("error message leaked %q: %v", secret, err.Error())
+				}
+			}
+		})
+	}
+}
+
+// TestParseStorageURLParseFailureHidesURL checks that when url.Parse
+// itself fails, the wrapped error carries only the inner cause, not the
+// *url.Error whose text embeds the raw (possibly credential-bearing) URL.
+func TestParseStorageURLParseFailureHidesURL(t *testing.T) {
+	t.Parallel()
+
+	const raw = "s3://mybucket/%zz"
+
+	_, err := storage.ParseStorageURL(raw)
+	if err == nil {
+		t.Fatalf("ParseStorageURL(%q) returned no error", raw)
+	}
+
+	if strings.Contains(err.Error(), "mybucket") {
+		t.Errorf("error message echoed the raw URL: %v", err.Error())
+	}
+}

internal/vaultik/purge_local_remote_consistency_test.go

@@ -0,0 +1,146 @@
+package vaultik_test
+
+import (
+	"bytes"
+	"context"
+	"database/sql"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/snapshot"
+	"sneak.berlin/go/vaultik/internal/types"
+	"sneak.berlin/go/vaultik/internal/vaultik"
+)
+
+// setupConsistencyTest builds a Vaultik whose local database and mock
+// remote both hold the given snapshots. Remote metadata is stored under
+// the production layout, metadata/<RemoteSnapshotKey(id)>/manifest.json.zst.
+// It returns the instance and the mock so a test can inspect the remote.
+func setupConsistencyTest(
+	t *testing.T, snapshotIDs []string,
+) (*vaultik.Vaultik, *MockStorer) {
+	t.Helper()
+
+	ctx := context.Background()
+	db, err := database.New(ctx, ":memory:")
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = db.Close() })
+
+	repos := database.NewRepositories(db)
+	mockStorage := NewMockStorer()
+
+	for _, id := range snapshotIDs {
+		parts := strings.Split(id, "_")
+		startedAt, err := time.Parse(time.RFC3339, parts[len(parts)-1])
+		require.NoError(t, err, "parsing timestamp from snapshot ID %q", id)
+
+		completedAt := startedAt.Add(5 * time.Minute)
+		snap := &database.Snapshot{
+			ID:             types.SnapshotID(id),
+			Hostname:       testHostname,
+			VaultikVersion: testLabel,
+			StartedAt:      startedAt,
+			CompletedAt:    &completedAt,
+		}
+		err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
+			return repos.Snapshots.Create(ctx, tx, snap)
+		})
+		require.NoError(t, err, "creating snapshot %s", id)
+
+		metadataKey := "metadata/" + snapshot.RemoteSnapshotKey(id) +
+			"/manifest.json.zst"
+		err = mockStorage.Put(ctx, metadataKey, strings.NewReader("stub"))
+		require.NoError(t, err)
+	}
+
+	v := &vaultik.Vaultik{
+		Storage:      mockStorage,
+		Repositories: repos,
+		DB:           db,
+		Stdout:       &bytes.Buffer{},
+		Stderr:       &bytes.Buffer{},
+		Stdin:        &bytes.Buffer{},
+	}
+	v.SetContext(ctx)
+
+	return v, mockStorage
+}
+
+func remoteHasSnapshot(t *testing.T, m *MockStorer, id string) bool {
+	t.Helper()
+
+	prefix := "metadata/" + snapshot.RemoteSnapshotKey(id) + "/"
+	keys, err := m.List(context.Background(), prefix)
+	require.NoError(t, err)
+
+	return len(keys) > 0
+}
+
+// TestPurgeKeepsRemotelyBackedLocalRows guards against issue #160
+// (https://git.eeqj.de/sneak/vaultik/issues/160): purge reconciles local
+// rows against the remote first, and that step compared human snapshot IDs
+// against the hashed remote directory names, which never match — so it
+// deleted every local record and the purge itself then removed nothing.
+//
+// With every snapshot still present remotely and nothing old enough to
+// purge, all local rows must survive the reconcile untouched.
+func TestPurgeKeepsRemotelyBackedLocalRows(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ids := []string{snapHomeT0, snapHomeT1, snapSystemT0}
+
+	v, _ := setupConsistencyTest(t, ids)
+
+	err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
+		// 100 years: nothing is old enough to delete, so the reconcile
+		// is the only thing that touches the rows.
+		OlderThan: "36500d",
+		Force:     true,
+	})
+	require.NoError(t, err)
+
+	remaining := listRemainingSnapshots(t, v)
+	assert.Len(t, remaining, len(ids),
+		"remotely-backed local rows must survive the reconcile")
+	assert.Contains(t, remaining, snapHomeT0)
+	assert.Contains(t, remaining, snapHomeT1)
+	assert.Contains(t, remaining, snapSystemT0)
+}
+
+// TestPurgeRemovesLocalAndRemoteTogether proves the two halves stay
+// consistent: a purged snapshot is gone both locally and remotely, while a
+// retained one keeps both. Before the fix, the reconcile dropped every
+// local row yet the remote metadata was left in place.
+func TestPurgeRemovesLocalAndRemoteTogether(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ids := []string{snapHomeT0, snapHomeT1, snapSystemT0}
+
+	v, mock := setupConsistencyTest(t, ids)
+
+	err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
+		KeepLatest: true,
+		Force:      true,
+	})
+	require.NoError(t, err)
+
+	// Keep latest per name: newest home and the lone system are kept.
+	remaining := listRemainingSnapshots(t, v)
+	assert.ElementsMatch(t, []string{snapHomeT1, snapSystemT0}, remaining)
+
+	// Local and remote agree: the older home snapshot is gone from both,
+	// the retained ones are present in both.
+	assert.False(t, remoteHasSnapshot(t, mock, snapHomeT0),
+		"purged snapshot must also be removed remotely")
+	assert.True(t, remoteHasSnapshot(t, mock, snapHomeT1),
+		"retained snapshot must remain remotely")
+	assert.True(t, remoteHasSnapshot(t, mock, snapSystemT0),
+		"retained snapshot must remain remotely")
+}

internal/vaultik/purge_per_name_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/snapshot"
 	"sneak.berlin/go/vaultik/internal/types"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -60,8 +61,11 @@ func setupPurgeTest(t *testing.T, snapshotIDs []string) *vaultik.Vaultik {
 		})
 		require.NoError(t, err, "creating snapshot %s", id)
 
-		// Create remote metadata stub so syncWithRemote keeps it
-		metadataKey := "metadata/" + id + "/manifest.json.zst"
+		// Create the remote metadata stub under the production layout so
+		// syncWithRemote keeps the local row. Production stores metadata
+		// under the hashed remote key, not the human snapshot ID.
+		metadataKey := "metadata/" + snapshot.RemoteSnapshotKey(id) +
+			"/manifest.json.zst"
 		err = mockStorage.Put(ctx, metadataKey, strings.NewReader("stub"))
 		require.NoError(t, err)
 	}

internal/vaultik/restore.go

@@ -39,8 +39,14 @@ var (
 		"refusing to restore path outside the target directory")
 	errTrailingRestoreData = errors.New(
 		"restored file has trailing data after its last chunk")
+	errRestoreIncomplete = errors.New(
+		"restore loop ended with files still pending")
 )
 
+// snapshotDBFilename is the name the decrypted snapshot database is
+// written under inside its private temp directory.
+const snapshotDBFilename = "snapshot.db"
+
 // restoreDirMode is the permission mode for directories created while
 // restoring (parent directories and the target root; restored
 // directories themselves get their stored mode).
@@ -102,7 +108,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 	// Step 1: Download and decrypt the snapshot metadata database
 	log.Info("Downloading snapshot metadata...")
 
-	tempDB, err := v.downloadSnapshotDB(opts.SnapshotID, identity)
+	tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, identity)
 	if err != nil {
 		return fmt.Errorf("downloading snapshot database: %w", err)
 	}
@@ -112,10 +118,11 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 		if err != nil {
 			log.Debug("Failed to close temp database", "error", err)
 		}
-		// Clean up temp file
-		err = v.Fs.Remove(tempDB.Path())
+		// Remove the whole private directory, so the decrypted database
+		// and any SQLite side files it produced are gone on every path.
+		err = v.Fs.RemoveAll(tempDir)
 		if err != nil {
-			log.Debug("Failed to remove temp database", "error", err)
+			log.Debug("Failed to remove temp database directory", "error", err)
 		}
 	}()
 
@@ -368,6 +375,13 @@ func (v *Vaultik) runRestoreLoop(
 			totalBytesExpected, startTime, &lastStatusTime)
 	}
 
+	// The loop above stops as soon as nothing is ready and nothing more
+	// can be downloaded. If files still remain, they were abandoned
+	// rather than restored; fail loudly instead of reporting success.
+	if plan.hasPending() {
+		return errRestoreIncomplete
+	}
+
 	return nil
 }
 
@@ -382,8 +396,8 @@ func (v *Vaultik) runRestoreLoop(
 func (s *restoreSession) downloadNextBlobSet(plan *restorePlan) (bool, error) {
 	s.sweeper.sweep()
 
-	next := plan.pickNextDownload()
-	if next.IsZero() {
+	next, ok := plan.pickNextDownload()
+	if !ok {
 		return false, nil
 	}
 
@@ -594,10 +608,10 @@ func (v *Vaultik) handleRestoreVerification(
 // index can restore the snapshots it can only see on the store.
 func (v *Vaultik) downloadSnapshotDB(
 	snapshotID string, identity age.Identity,
-) (*database.DB, error) {
+) (*database.DB, string, error) {
 	remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
 	// Download encrypted database from storage
@@ -605,7 +619,7 @@ func (v *Vaultik) downloadSnapshotDB(
 
 	reader, err := v.Storage.Get(v.ctx, dbKey)
 	if err != nil {
-		return nil, fmt.Errorf("downloading %s: %w", dbKey, err)
+		return nil, "", fmt.Errorf("downloading %s: %w", dbKey, err)
 	}
 
 	defer func() { _ = reader.Close() }()
@@ -613,7 +627,7 @@ func (v *Vaultik) downloadSnapshotDB(
 	// Read all data
 	encryptedData, err := io.ReadAll(reader)
 	if err != nil {
-		return nil, fmt.Errorf("reading encrypted data: %w", err)
+		return nil, "", fmt.Errorf("reading encrypted data: %w", err)
 	}
 
 	log.Debug("Downloaded encrypted database",
@@ -622,7 +636,7 @@ func (v *Vaultik) downloadSnapshotDB(
 	// Decrypt and decompress using blobgen.Reader
 	blobReader, err := blobgen.NewReader(bytes.NewReader(encryptedData), identity)
 	if err != nil {
-		return nil, fmt.Errorf("creating decryption reader: %w", err)
+		return nil, "", fmt.Errorf("creating decryption reader: %w", err)
 	}
 
 	defer func() { _ = blobReader.Close() }()
@@ -630,44 +644,52 @@ func (v *Vaultik) downloadSnapshotDB(
 	// Read the binary SQLite database
 	dbData, err := io.ReadAll(blobReader)
 	if err != nil {
-		return nil, fmt.Errorf("decrypting and decompressing: %w", err)
+		return nil, "", fmt.Errorf("decrypting and decompressing: %w", err)
 	}
 
 	log.Debug("Decrypted database", "size", ubytes(int64(len(dbData))))
 
-	// Create a temporary database file and write the binary SQLite data directly
-	tempFile, err := afero.TempFile(v.Fs, "", "vaultik-restore-*.db")
+	return v.materializeSnapshotDB(dbData)
+}
+
+// materializeSnapshotDB writes the decrypted snapshot database bytes into
+// a fresh private (0700) temp directory and opens the file read-only. On
+// any failure it removes the directory before returning, so no decrypted
+// metadata is left on disk when the open is interrupted or the payload is
+// damaged. On success the returned directory is the caller's to remove.
+func (v *Vaultik) materializeSnapshotDB(
+	dbData []byte,
+) (*database.DB, string, error) {
+	tempDir, err := afero.TempDir(v.Fs, "", "vaultik-restore-")
 	if err != nil {
-		return nil, fmt.Errorf("creating temp file: %w", err)
+		return nil, "", fmt.Errorf("creating temp directory: %w", err)
 	}
 
-	tempPath := tempFile.Name()
+	success := false
 
-	// Write the binary SQLite database directly
-	_, err = tempFile.Write(dbData)
+	defer func() {
+		if !success {
+			_ = v.Fs.RemoveAll(tempDir)
+		}
+	}()
+
+	dbPath := filepath.Join(tempDir, snapshotDBFilename)
+
+	err = afero.WriteFile(v.Fs, dbPath, dbData, restoreFileMode)
 	if err != nil {
-		_ = tempFile.Close()
-		_ = v.Fs.Remove(tempPath)
-
-		return nil, fmt.Errorf("writing database file: %w", err)
+		return nil, "", fmt.Errorf("writing database file: %w", err)
 	}
 
-	err = tempFile.Close()
-	if err != nil {
-		_ = v.Fs.Remove(tempPath)
+	log.Debug("Created restore database", "path", dbPath)
 
-		return nil, fmt.Errorf("closing temp file: %w", err)
+	db, err := database.OpenReadOnly(v.ctx, dbPath)
+	if err != nil {
+		return nil, "", fmt.Errorf("opening restore database: %w", err)
 	}
 
-	log.Debug("Created restore database", "path", tempPath)
+	success = true
 
-	// Open the database
-	db, err := database.New(v.ctx, tempPath)
-	if err != nil {
-		return nil, fmt.Errorf("opening restore database: %w", err)
-	}
-
-	return db, nil
+	return db, tempDir, nil
 }
 
 // getFilesToRestore returns the list of files to restore based on path filters

internal/vaultik/restore_plan.go

@@ -171,10 +171,13 @@ func (p *restorePlan) finishFile(fileID types.FileID) {
 // downloaded next, after which it — together with any other pending
 // files whose blob sets become empty — moves to the ready queue.
 //
-// The zero FileID return means nothing is pending.
-func (p *restorePlan) pickNextDownload() types.FileID {
+// The second return value is false when no file needs a download, so a
+// genuine file carrying the nil UUID is picked rather than mistaken for
+// "nothing left".
+func (p *restorePlan) pickNextDownload() (types.FileID, bool) {
 	var best types.FileID
 
+	found := false
 	bestCount := math.MaxInt
 
 	var bestID string
@@ -188,14 +191,15 @@ func (p *restorePlan) pickNextDownload() types.FileID {
 		}
 
 		idStr := id.String()
-		if n < bestCount || (n == bestCount && (best.IsZero() || idStr < bestID)) {
+		if !found || n < bestCount || (n == bestCount && idStr < bestID) {
 			best = id
+			found = true
 			bestCount = n
 			bestID = idStr
 		}
 	}
 
-	return best
+	return best, found
 }
 
 // blobsNeeded returns the uncached blob hashes for fileID in any order.

internal/vaultik/restore_plan_test.go

@@ -0,0 +1,88 @@
+package vaultik //nolint:testpackage // inspects unexported restore plan internals
+
+import (
+	"context"
+	"math"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/types"
+)
+
+// TestPickNextDownloadReturnsNilUUIDFile proves a genuine pending file
+// carrying the nil UUID is picked for download rather than mistaken for
+// "nothing left" — the bug that could abandon every remaining file.
+func TestPickNextDownloadReturnsNilUUIDFile(t *testing.T) {
+	t.Parallel()
+
+	var nilID types.FileID // zero value is the nil UUID
+
+	plan := &restorePlan{
+		fileBlobs: map[types.FileID]map[string]struct{}{
+			nilID: {"blobhash": {}},
+		},
+	}
+
+	id, ok := plan.pickNextDownload()
+	require.True(t, ok,
+		"pickNextDownload treated a pending nil-UUID file as nothing to do")
+	require.True(t, id.IsZero(), "expected the nil-UUID file to be picked")
+}
+
+// TestPickNextDownloadEmptyPlan confirms the second return value is false
+// only when no file needs a download.
+func TestPickNextDownloadEmptyPlan(t *testing.T) {
+	t.Parallel()
+
+	plan := &restorePlan{
+		fileBlobs: map[types.FileID]map[string]struct{}{},
+	}
+
+	_, ok := plan.pickNextDownload()
+	require.False(t, ok, "pickNextDownload reported work on an empty plan")
+}
+
+// TestRunRestoreLoopFailsOnAbandonedFiles proves the loop returns an
+// error rather than silent success when files remain pending after it
+// can make no further progress.
+func TestRunRestoreLoopFailsOnAbandonedFiles(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+
+	db, err := database.NewTestDB()
+	require.NoError(t, err)
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	repos := database.NewRepositories(db)
+
+	cache, err := newBlobDiskCache(math.MaxInt64)
+	require.NoError(t, err)
+
+	t.Cleanup(func() { _ = cache.Close() })
+
+	v := &Vaultik{ctx: ctx}
+	session := &restoreSession{
+		v:       v,
+		ctx:     ctx,
+		repos:   repos,
+		sweeper: newRestoreSweeper(ctx, repos, cache, 1),
+		result:  &RestoreResult{},
+	}
+
+	// A file that is still pending but whose uncached-blob set is empty
+	// and which was never queued as ready: the loop can neither restore
+	// nor download it. This is the abandonment the guard must catch.
+	var stuck types.FileID
+
+	plan := &restorePlan{
+		fileBlobs: map[types.FileID]map[string]struct{}{stuck: {}},
+		blobFiles: map[string]map[types.FileID]struct{}{},
+		cached:    map[string]struct{}{},
+	}
+
+	err = v.runRestoreLoop(session, plan, map[types.FileID]*database.File{}, 0)
+	require.ErrorIs(t, err, errRestoreIncomplete)
+}

internal/vaultik/restore_snapshotdb_test.go

@@ -0,0 +1,73 @@
+package vaultik //nolint:testpackage // inspects unexported snapshot-db materialization
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/database"
+)
+
+// genuineSnapshotDBBytes returns the on-disk bytes of a real snapshot
+// database (the full schema applied).
+func genuineSnapshotDBBytes(t *testing.T) []byte {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "snapshot.db")
+
+	db, err := database.New(context.Background(), path)
+	require.NoError(t, err)
+	require.NoError(t, db.Close())
+
+	data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled temp path
+	require.NoError(t, err)
+
+	return data
+}
+
+// TestMaterializeSnapshotDBPrivateDir proves the decrypted database lands
+// in a private (0700) directory and opens read-only.
+func TestMaterializeSnapshotDBPrivateDir(t *testing.T) {
+	dbData := genuineSnapshotDBBytes(t)
+
+	t.Setenv("TMPDIR", t.TempDir())
+
+	v := &Vaultik{ctx: context.Background(), Fs: afero.NewOsFs()}
+
+	db, dir, err := v.materializeSnapshotDB(dbData)
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		_ = db.Close()
+		_ = os.RemoveAll(dir)
+	})
+
+	info, err := os.Stat(dir)
+	require.NoError(t, err)
+	require.Equal(t, os.FileMode(0o700), info.Mode().Perm(),
+		"snapshot database directory must not be world-readable")
+
+	_, err = db.Conn().ExecContext(context.Background(),
+		"CREATE TABLE probe_readonly (x)")
+	require.Error(t, err, "materialized snapshot database must be read-only")
+}
+
+// TestMaterializeSnapshotDBRemovesDirOnOpenFailure proves a failed open
+// leaves no temp directory behind.
+func TestMaterializeSnapshotDBRemovesDirOnOpenFailure(t *testing.T) {
+	base := t.TempDir()
+
+	t.Setenv("TMPDIR", base)
+
+	v := &Vaultik{ctx: context.Background(), Fs: afero.NewOsFs()}
+
+	_, _, err := v.materializeSnapshotDB([]byte("this is not a sqlite database"))
+	require.Error(t, err)
+
+	entries, rerr := os.ReadDir(base)
+	require.NoError(t, rerr)
+	require.Empty(t, entries, "temp directory left behind after open failure")
+}

internal/vaultik/snapshot.go

@@ -935,29 +935,23 @@ func (v *Vaultik) downloadManifestByKey(remoteKey string) (*snapshot.Manifest, e
 func (v *Vaultik) syncWithRemote() error {
 	log.Info("Syncing with remote snapshots")
 
-	// Get all remote snapshot IDs
-	remoteSnapshots := make(map[string]bool)
-	objectCh := v.Storage.ListStream(v.ctx, "metadata/")
-
-	for object := range objectCh {
-		if object.Err != nil {
-			return fmt.Errorf("listing remote snapshots: %w", object.Err)
+	// Remote metadata lives under metadata/<remote-key>/, where the
+	// directory name is snapshot.RemoteSnapshotKey(id), not the human
+	// snapshot ID. Compare each local row's hashed key against that set
+	// so a row still backed by remote metadata is kept. Comparing human
+	// IDs against the hashed directory names matches nothing and deletes
+	// every local snapshot record (issue #160).
+	remoteKeys, err := v.listAllRemoteSnapshotKeys()
+	if err != nil {
+		return fmt.Errorf("listing remote snapshots: %w", err)
 	}
 
-		// Extract snapshot ID from paths like metadata/hostname-20240115-143052Z/
-		parts := strings.Split(object.Key, "/")
-		if len(parts) >= minSnapshotIDParts &&
-			parts[0] == metadataDirName && parts[1] != "" {
-			// Skip macOS resource fork files (._*) and other hidden files
-			if strings.HasPrefix(parts[1], ".") {
-				continue
+	remoteKeySet := make(map[string]bool, len(remoteKeys))
+	for _, k := range remoteKeys {
+		remoteKeySet[k] = true
 	}
 
-			remoteSnapshots[parts[1]] = true
-		}
-	}
-
-	log.Debug("Found remote snapshots", "count", len(remoteSnapshots))
+	log.Debug("Found remote snapshots", "count", len(remoteKeySet))
 
 	// Get all local snapshots (use a high limit to get all)
 	localSnapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit)
@@ -965,12 +959,12 @@ func (v *Vaultik) syncWithRemote() error {
 		return fmt.Errorf("listing local snapshots: %w", err)
 	}
 
-	// Remove local snapshots that don't exist remotely
+	// Remove local snapshots whose metadata is absent from the remote.
 	removedCount := 0
 
 	for _, snap := range localSnapshots {
 		snapshotIDStr := snap.ID.String()
-		if !remoteSnapshots[snapshotIDStr] {
+		if !remoteKeySet[snapshot.RemoteSnapshotKey(snapshotIDStr)] {
 			log.Info("Removing local snapshot not found in remote",
 				"snapshot_id", snap.ID)
 

internal/vaultik/verify.go

@@ -9,12 +9,12 @@ import (
 	"hash"
 	"io"
 	"os"
+	"path/filepath"
 	"time"
 
 	"github.com/klauspost/compress/zstd"
 
-	// Blank import registers the pure-Go sqlite driver for database/sql.
-	_ "modernc.org/sqlite"
+	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
 )
@@ -195,7 +195,7 @@ func (v *Vaultik) loadVerificationData(
 			fmt.Errorf("failed to decrypt database: %w", err))
 	}
 
-	dbBlobs, err := v.getBlobsFromDatabase(tdb.DB)
+	dbBlobs, err := v.getBlobsFromDatabase(tdb.db.Conn())
 	if err != nil {
 		_ = tdb.Close()
 
@@ -256,7 +256,7 @@ func (v *Vaultik) runVerificationSteps(
 			len(dbBlobs), ubytes(totalSize))
 	}
 
-	err = v.performDeepVerificationFromDB(dbBlobs, tdb.DB, opts)
+	err = v.performDeepVerificationFromDB(dbBlobs, tdb.db.Conn(), opts)
 	if err != nil {
 		return v.deepVerifyFailure(result, opts, err.Error(), err)
 	}
@@ -264,16 +264,18 @@ func (v *Vaultik) runVerificationSteps(
 	return nil
 }
 
-// tempDB wraps sql.DB with cleanup
+// tempDB is the downloaded snapshot database opened read-only for deep
+// verify, held in a private temp directory removed in full on Close.
 type tempDB struct {
-	*sql.DB
-
-	tempPath string
+	db      *database.DB
+	tempDir string
 }
 
 func (t *tempDB) Close() error {
-	err := t.DB.Close()
-	_ = os.Remove(t.tempPath)
+	err := t.db.Close()
+	// Remove the whole private directory so the decrypted database and
+	// any SQLite side files are gone on every path.
+	_ = os.RemoveAll(t.tempDir)
 
 	return err
 }
@@ -300,41 +302,56 @@ func (v *Vaultik) decryptAndLoadDatabase(reader io.ReadCloser) (*tempDB, error)
 	}
 	defer decompressor.Close()
 
-	// Create temporary file for the database
-	tempFile, err := os.CreateTemp("", "vaultik-verify-*.db")
+	// Materialize the decrypted database inside a private (0700) temp
+	// directory so it is never world-readable, and remove the whole
+	// directory on any failure below.
+	tempDir, err := os.MkdirTemp("", "vaultik-verify-")
+	if err != nil {
+		return nil, fmt.Errorf("failed to create temp directory: %w", err)
+	}
+
+	success := false
+
+	defer func() {
+		if !success {
+			_ = os.RemoveAll(tempDir)
+		}
+	}()
+
+	dbPath := filepath.Join(tempDir, snapshotDBFilename)
+
+	//nolint:gosec // G304: dbPath is our MkdirTemp dir plus a constant filename
+	tempFile, err := os.OpenFile(
+		dbPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, restoreFileMode)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create temp file: %w", err)
 	}
 
-	tempPath := tempFile.Name()
-
 	// Stream decompress directly to file
 	log.Info("Decompressing database...")
 
 	written, err := io.Copy(tempFile, decompressor)
 	if err != nil {
 		_ = tempFile.Close()
-		_ = os.Remove(tempPath)
 
 		return nil, fmt.Errorf("failed to decompress database: %w", err)
 	}
 
-	_ = tempFile.Close()
+	err = tempFile.Close()
+	if err != nil {
+		return nil, fmt.Errorf("failed to close temp database file: %w", err)
+	}
 
 	log.Info("Database decompressed", "size", ubytes(written))
 
-	// Open the database
-	db, err := sql.Open("sqlite", tempPath)
+	db, err := database.OpenReadOnly(v.ctx, dbPath)
 	if err != nil {
-		_ = os.Remove(tempPath)
-
 		return nil, fmt.Errorf("failed to open database: %w", err)
 	}
 
-	return &tempDB{
-		DB:       db,
-		tempPath: tempPath,
-	}, nil
+	success = true
+
+	return &tempDB{db: db, tempDir: tempDir}, nil
 }
 
 // verifyBlob downloads and verifies a single blob