From 5614980edcd1b096bc1122eafb5632560df70102 Mon Sep 17 00:00:00 2001 From: Matt Faltyn <76570855+mattfaltyn@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:56:53 +0200 Subject: [PATCH] Make database restore preserve live data on failure (#1703) Stage and normalize SQLite backups before atomic publication, checkpoint committed live journals safely, and stop PostgreSQL restores on SQL errors. Cover recovery failures and compatibility with real-engine regressions. --- cmd/pad/cmd_db.go | 58 +-- cmd/pad/cmd_db_restore_test.go | 548 +++++++++++++++++++++- cmd/pad/db_restore_ownership_other.go | 9 + cmd/pad/db_restore_ownership_unix.go | 20 + cmd/pad/db_restore_ownership_unix_test.go | 36 ++ cmd/pad/db_restore_sqlite.go | 265 +++++++++++ cmd/pad/postgres_client_test.go | 30 ++ docs/backup.md | 34 +- 8 files changed, 942 insertions(+), 58 deletions(-) create mode 100644 cmd/pad/db_restore_ownership_other.go create mode 100644 cmd/pad/db_restore_ownership_unix.go create mode 100644 cmd/pad/db_restore_ownership_unix_test.go create mode 100644 cmd/pad/db_restore_sqlite.go diff --git a/cmd/pad/cmd_db.go b/cmd/pad/cmd_db.go index 23a193074..a5319d2e2 100644 --- a/cmd/pad/cmd_db.go +++ b/cmd/pad/cmd_db.go @@ -3,7 +3,6 @@ package main import ( "database/sql" "fmt" - "io" "log/slog" "net/url" "os" @@ -209,8 +208,10 @@ func dbRestoreCmd() *cobra.Command { Short: "Restore a database from a backup", Long: `Restores a Pad database from a backup created by 'pad db backup'. -For PostgreSQL: restores from a SQL dump using psql. Requires PAD_DATABASE_URL. -For SQLite (default): copies the backup file over the live database, whose path +For PostgreSQL: restores in one transaction using psql, stopping on SQL errors +and ignoring psql startup files. Requires PAD_DATABASE_URL. +For SQLite (default): stages and checks the backup before atomically replacing +the live database after checkpointing existing WAL data. The database path is resolved the same way the server resolves it (PAD_DB_PATH > PAD_DATA_DIR/pad.db > ~/.pad/pad.db). Stop the server first — restore refuses to run while it detects a live server (a running WAL checkpoint could clobber the restored file); use @@ -253,6 +254,8 @@ WARNING: This will overwrite the current database contents.`, psqlArgs := []string{ "--file", inputFile, "--single-transaction", + "--no-psqlrc", + "--set=ON_ERROR_STOP=on", } psqlCmd, err := postgresClient("psql", dbURL, psqlArgs...) @@ -272,7 +275,7 @@ WARNING: This will overwrite the current database contents.`, return nil } - // SQLite restore via file copy + // SQLite restore via staged, self-contained publication cfg, err := config.Load() if err != nil { return fmt.Errorf("load config: %w", err) @@ -311,51 +314,8 @@ WARNING: This will overwrite the current database contents.`, fmt.Fprintf(os.Stderr, "Restoring SQLite database %s from %s...\n", dstPath, inputFile) - src, err := os.Open(inputFile) - if err != nil { - return fmt.Errorf("open backup file: %w", err) - } - defer src.Close() - - dst, err := os.Create(dstPath) - if err != nil { - return fmt.Errorf("open database for writing: %w", err) - } - defer dst.Close() - - if _, err := io.Copy(dst, src); err != nil { - return fmt.Errorf("copy backup: %w", err) - } - - // Also restore WAL and SHM files if they exist alongside the backup - for _, suffix := range []string{"-wal", "-shm"} { - walPath := inputFile + suffix - if _, err := os.Stat(walPath); err == nil { - walSrc, err := os.Open(walPath) - if err != nil { - return fmt.Errorf("open %s: %w", suffix, err) - } - walDst, err := os.Create(dstPath + suffix) - if err != nil { - walSrc.Close() - return fmt.Errorf("create %s: %w", suffix, err) - } - _, copyErr := io.Copy(walDst, walSrc) - walSrc.Close() - walDst.Close() - if copyErr != nil { - return fmt.Errorf("copy %s: %w", suffix, copyErr) - } - } else { - // No WAL/SHM in backup (the VACUUM INTO path produces none): - // remove any stale sidecar at the target. A leftover -wal/-shm - // would let SQLite replay old WAL state over the freshly - // restored main DB on next open, so a remove failure is fatal - // rather than a silent success. - if err := os.Remove(dstPath + suffix); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("remove stale %s: %w", suffix, err) - } - } + if err := restoreSQLite(inputFile, dstPath); err != nil { + return err } fmt.Fprintln(os.Stderr, "Restore complete. Restart the Pad server to pick up the restored database.") diff --git a/cmd/pad/cmd_db_restore_test.go b/cmd/pad/cmd_db_restore_test.go index fadb8f875..11acde217 100644 --- a/cmd/pad/cmd_db_restore_test.go +++ b/cmd/pad/cmd_db_restore_test.go @@ -1,8 +1,14 @@ package main import ( + "bytes" + "context" + "database/sql" + "fmt" "os" + "os/exec" "path/filepath" + "runtime" "strings" "testing" ) @@ -56,9 +62,7 @@ func TestDBRestoreRejectsSameFile(t *testing.T) { func TestDBRestoreCopiesDistinctFile(t *testing.T) { dataDir := t.TempDir() backup := filepath.Join(dataDir, "backup.db") - if err := os.WriteFile(backup, []byte("backup contents"), 0o600); err != nil { - t.Fatal(err) - } + writeRestoreSQLite(t, backup, "backup contents", 4096, false) setSQLiteRestoreEnv(t, dataDir) cmd := dbRestoreCmd() @@ -66,9 +70,543 @@ func TestDBRestoreCopiesDistinctFile(t *testing.T) { if err := cmd.Execute(); err != nil { t.Fatal(err) } - if got, err := os.ReadFile(filepath.Join(dataDir, "pad.db")); err != nil || string(got) != "backup contents" { - t.Fatalf("restored database = %q, %v", got, err) + assertRestoreSQLiteValue(t, filepath.Join(dataDir, "pad.db"), "backup contents") +} + +// Copy a quiescent committed WAL tuple while its scratch connection is open, +// then close the scratch DB. The fixture itself has no live connection. +func writeRestoreSQLite(t *testing.T, path, value string, pageSize int, wal bool) { + t.Helper() + scratch := filepath.Join(t.TempDir(), "scratch.db") + db, err := sql.Open("sqlite", scratch) + if err != nil { + t.Fatal(err) } + defer db.Close() + db.SetMaxOpenConns(1) + statements := []string{fmt.Sprintf("PRAGMA page_size=%d", pageSize)} + if wal { + statements = append(statements, "PRAGMA journal_mode=WAL", "PRAGMA wal_autocheckpoint=0") + } + statements = append(statements, "CREATE TABLE recovery_probe(value TEXT)") + for _, statement := range statements { + if _, err := db.Exec(statement); err != nil { + t.Fatal(err) + } + } + if _, err := db.Exec("INSERT INTO recovery_probe VALUES(?)", "checkpointed "+value); err != nil { + t.Fatal(err) + } + if wal { + if _, err := db.Exec("PRAGMA wal_checkpoint(TRUNCATE)"); err != nil { + t.Fatal(err) + } + } + if _, err := db.Exec("UPDATE recovery_probe SET value=?", value); err != nil { + t.Fatal(err) + } + for suffix, data := range restoreSQLiteTuple(t, scratch) { + if err := os.WriteFile(path+suffix, data, 0600); err != nil { + t.Fatal(err) + } + } +} + +func restoreSQLiteTuple(t *testing.T, path string) map[string][]byte { + t.Helper() + result := make(map[string][]byte) + for _, suffix := range []string{"", "-wal", "-shm"} { + data, err := os.ReadFile(path + suffix) + if os.IsNotExist(err) { + continue + } + if err != nil { + t.Fatal(err) + } + result[suffix] = data + } + return result +} + +func assertRestoreSQLiteTuple(t *testing.T, path string, want map[string][]byte) { + t.Helper() + got := restoreSQLiteTuple(t, path) + for _, suffix := range []string{"", "-wal", "-shm"} { + wantData, wantPresent := want[suffix] + gotData, gotPresent := got[suffix] + if wantPresent != gotPresent || !bytes.Equal(wantData, gotData) { + t.Errorf("database%s changed: before=%d bytes, after=%d bytes; presence %v -> %v", suffix, len(wantData), len(gotData), wantPresent, gotPresent) + } + } +} + +func openRestoreSQLiteCopy(t *testing.T, path string) *sql.DB { + t.Helper() + // Inspect a copy so opening/closing SQLite cannot checkpoint the fixture. + copyPath := filepath.Join(t.TempDir(), "inspection.db") + for suffix, data := range restoreSQLiteTuple(t, path) { + if err := os.WriteFile(copyPath+suffix, data, 0600); err != nil { + t.Fatal(err) + } + } + db, err := sql.Open("sqlite", copyPath) + if err != nil { + t.Fatal(err) + } + return db +} + +func assertRestoreSQLiteValue(t *testing.T, path, want string) { + t.Helper() + db := openRestoreSQLiteCopy(t, path) + defer db.Close() + var got string + if err := db.QueryRow("SELECT value FROM recovery_probe").Scan(&got); err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("committed value=%q, want %q", got, want) + } +} + +func executeSQLiteRestore(t *testing.T, backup string) error { + t.Helper() + cmd := dbRestoreCmd() + cmd.SetArgs([]string{"--force", backup}) + return cmd.Execute() +} + +func TestDBRestoreInputFailurePreservesCommittedWAL(t *testing.T) { + for _, tc := range []struct{ name, suffix, failure string }{ + {"main directory", "", "directory"}, + {"invalid main", "", "corrupt"}, + {"empty main", "", "empty"}, + {"WAL directory", "-wal", "directory"}, + {"SHM directory", "-shm", "directory"}, + {"unreadable WAL", "-wal", "unreadable"}, + {"unreadable SHM", "-shm", "unreadable"}, + {"WAL stat error", "-wal", "symlink loop"}, + {"SHM stat error", "-shm", "symlink loop"}, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.failure == "unreadable" && (runtime.GOOS == "windows" || os.Geteuid() == 0) { + t.Skip("Unix permission denial requires a non-root process") + } + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + writeRestoreSQLite(t, backup, "restored committed WAL row", 4096, true) + before := restoreSQLiteTuple(t, live) + source := restoreSQLiteTuple(t, backup) + assertRestoreSQLiteValue(t, live, "original committed WAL row") + input := backup + tc.suffix + switch tc.failure { + case "directory": + delete(source, tc.suffix) + if err := os.Remove(input); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(input, 0700); err != nil { + t.Fatal(err) + } + case "corrupt": + source[""] = []byte("not a SQLite database") + if err := os.WriteFile(input, []byte("not a SQLite database"), 0600); err != nil { + t.Fatal(err) + } + case "empty": + source[""] = nil + if err := os.WriteFile(input, nil, 0600); err != nil { + t.Fatal(err) + } + case "unreadable": + if err := os.Chmod(input, 0); err != nil { + t.Fatal(err) + } + defer os.Chmod(input, 0600) + case "symlink loop": + delete(source, tc.suffix) + if err := os.Remove(input); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Base(input), input); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + } + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err == nil { + t.Error("invalid backup restored successfully") + } + if tc.failure == "unreadable" { + if err := os.Chmod(input, 0600); err != nil { + t.Fatal(err) + } + } + for suffix, want := range source { + got, err := os.ReadFile(backup + suffix) + if err != nil || !bytes.Equal(got, want) { + t.Errorf("source backup%s changed: %v", suffix, err) + } + } + assertRestoreSQLiteTuple(t, live, before) + assertRestoreSQLiteValue(t, live, "original committed WAL row") + }) + } +} + +func TestDBRestoreStandaloneAndLegacyWAL(t *testing.T) { + for _, tc := range []struct { + name string + wal bool + pageSize int + }{ + {"standalone", false, 4096}, {"legacy WAL", true, 4096}, + {"different standalone page size", false, 8192}, {"different WAL page size", true, 8192}, + } { + t.Run(tc.name, func(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + writeRestoreSQLite(t, backup, "restored row", tc.pageSize, tc.wal) + source := restoreSQLiteTuple(t, backup) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + assertRestoreSQLiteValue(t, live, "restored row") + assertRestoreSQLiteTuple(t, backup, source) + }) + } +} + +func TestDBRestoreNonWritableTargetPreservesCommittedWAL(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("Unix directory permission denial requires a non-root process") + } + dataDir := t.TempDir() + if err := os.Mkdir(filepath.Join(dataDir, "logs"), 0700); err != nil { + t.Fatal(err) + } + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + writeRestoreSQLite(t, backup, "restored row", 4096, false) + before := restoreSQLiteTuple(t, live) + source := restoreSQLiteTuple(t, backup) + if err := os.Chmod(dataDir, 0555); err != nil { + t.Fatal(err) + } + defer os.Chmod(dataDir, 0755) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err == nil { + t.Error("non-writable target directory accepted") + } + assertRestoreSQLiteTuple(t, live, before) + assertRestoreSQLiteValue(t, live, "original committed WAL row") + assertRestoreSQLiteTuple(t, backup, source) +} + +func TestDBRestoreTargetPreparationFailurePreservesCommittedWAL(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + writeRestoreSQLite(t, backup, "restored row", 4096, false) + // A stale SHM path obstructs SQLite's target preparation. Keep the + // committed WAL intact; SHM is a rebuildable index, not the data. + if err := os.Remove(live + "-shm"); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(live+"-shm", 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(live+"-shm", "obstruction"), nil, 0600); err != nil { + t.Fatal(err) + } + source := restoreSQLiteTuple(t, backup) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err == nil { + t.Error("obstructed target restored successfully") + } + // Remove the obstructing cache path to inspect recoverable committed data. + if err := os.RemoveAll(live + "-shm"); err != nil { + t.Fatal(err) + } + assertRestoreSQLiteValue(t, live, "original committed WAL row") + assertRestoreSQLiteTuple(t, backup, source) +} + +func TestDBRestoreCorruptLiveDatabase(t *testing.T) { + for _, hasWAL := range []bool{false, true} { + t.Run(fmt.Sprintf("WAL=%v", hasWAL), func(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original row", 4096, hasWAL) + if err := os.WriteFile(live, []byte("corrupt live database"), 0600); err != nil { + t.Fatal(err) + } + writeRestoreSQLite(t, backup, "restored row", 4096, false) + before, source := restoreSQLiteTuple(t, live), restoreSQLiteTuple(t, backup) + setSQLiteRestoreEnv(t, dataDir) + err := executeSQLiteRestore(t, backup) + if hasWAL { + if err == nil { + t.Error("corrupt target with potentially committed WAL accepted") + } + assertRestoreSQLiteTuple(t, live, before) + } else { + if err != nil { + t.Fatal(err) + } + assertRestoreSQLiteValue(t, live, "restored row") + } + assertRestoreSQLiteTuple(t, backup, source) + }) + } +} + +func TestDBRestorePreservesDestinationSymlinkAndMode(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + referent := filepath.Join(t.TempDir(), "referent.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, referent, "original row", 4096, true) + if err := os.Chmod(referent, 0640); err != nil { + t.Fatal(err) + } + if err := os.Symlink(referent, live); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + writeRestoreSQLite(t, backup, "restored row", 4096, false) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + info, err := os.Lstat(live) + if err != nil || info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("destination symlink replaced: %v, %v", info, err) + } + info, err = os.Stat(referent) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0640 { + t.Errorf("restored mode=%o, want 640", info.Mode().Perm()) + } + assertRestoreSQLiteValue(t, referent, "restored row") +} + +func TestDBRestoreAtomicReplacementLeavesOtherHardLinkUnchanged(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + other := filepath.Join(dataDir, "other.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original row", 4096, false) + if err := os.Link(live, other); err != nil { + t.Skipf("hard links unavailable: %v", err) + } + writeRestoreSQLite(t, backup, "restored row", 4096, false) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + assertRestoreSQLiteValue(t, live, "restored row") + assertRestoreSQLiteValue(t, other, "original row") +} + +func TestDBRestoreReservedCharactersInTargetDirectory(t *testing.T) { + characters := "?#%" + if runtime.GOOS == "windows" { + characters = "#%" + } + dataDir := filepath.Join(t.TempDir(), "data "+characters) + if err := os.Mkdir(dataDir, 0700); err != nil { + t.Fatal(err) + } + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + writeRestoreSQLite(t, backup, "restored row", 4096, false) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + assertRestoreSQLiteValue(t, live, "restored row") +} + +func TestDBRestoreHeldWriteLockPreservesCommittedWAL(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + writeRestoreSQLite(t, backup, "restored row", 4096, false) + source := restoreSQLiteTuple(t, backup) + db, err := sql.Open("sqlite", live) + if err != nil { + t.Fatal(err) + } + defer db.Close() + conn, err := db.Conn(context.Background()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if _, err := conn.ExecContext(context.Background(), "BEGIN IMMEDIATE"); err != nil { + t.Fatal(err) + } + locked := true + defer func() { + if locked { + _, _ = conn.ExecContext(context.Background(), "ROLLBACK") + } + }() + if _, err := conn.ExecContext(context.Background(), "UPDATE recovery_probe SET value='uncommitted row'"); err != nil { + t.Fatal(err) + } + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err == nil { + t.Error("restore accepted a target with a held write lock") + } + if _, err := conn.ExecContext(context.Background(), "ROLLBACK"); err != nil { + t.Fatal(err) + } + locked = false + if err := conn.Close(); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + assertRestoreSQLiteValue(t, live, "original committed WAL row") + assertRestoreSQLiteTuple(t, backup, source) +} + +func TestDBRestorePreservesLegacyWALRowIDsAndExternalFTS(t *testing.T) { + for _, idDefinition := range []string{"id TEXT PRIMARY KEY", "id TEXT"} { + t.Run(idDefinition, func(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original committed WAL row", 4096, true) + scratch := filepath.Join(t.TempDir(), "fts.db") + db, err := sql.Open("sqlite", scratch) + if err != nil { + t.Fatal(err) + } + defer db.Close() + db.SetMaxOpenConns(1) + for _, statement := range []string{ + "PRAGMA journal_mode=WAL", + "PRAGMA wal_autocheckpoint=0", + fmt.Sprintf("CREATE TABLE recovery_probe(%s, value TEXT NOT NULL)", idDefinition), + "CREATE VIRTUAL TABLE recovery_fts USING fts5(value, content='recovery_probe', content_rowid='rowid')", + "CREATE TRIGGER recovery_ai AFTER INSERT ON recovery_probe BEGIN INSERT INTO recovery_fts(rowid,value) VALUES(new.rowid,new.value); END", + "CREATE TRIGGER recovery_ad AFTER DELETE ON recovery_probe BEGIN INSERT INTO recovery_fts(recovery_fts,rowid,value) VALUES('delete',old.rowid,old.value); END", + "CREATE TRIGGER recovery_au AFTER UPDATE ON recovery_probe BEGIN INSERT INTO recovery_fts(recovery_fts,rowid,value) VALUES('delete',old.rowid,old.value); INSERT INTO recovery_fts(rowid,value) VALUES(new.rowid,new.value); END", + "INSERT INTO recovery_probe VALUES('removed','discarded'),('retained','searchable retained row')", + "DELETE FROM recovery_probe WHERE id='removed'", + } { + if _, err := db.Exec(statement); err != nil { + t.Fatal(err) + } + } + for suffix, data := range restoreSQLiteTuple(t, scratch) { + if err := os.WriteFile(backup+suffix, data, 0600); err != nil { + t.Fatal(err) + } + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + source := restoreSQLiteTuple(t, backup) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + restored := openRestoreSQLiteCopy(t, live) + defer restored.Close() + var rowID int + if err := restored.QueryRow("SELECT rowid FROM recovery_probe WHERE id='retained'").Scan(&rowID); err != nil { + t.Fatal(err) + } + if rowID != 2 { + t.Errorf("restored hidden rowid=%d, want 2", rowID) + } + var match string + if err := restored.QueryRow("SELECT p.value FROM recovery_probe p JOIN recovery_fts f ON f.rowid=p.rowid WHERE recovery_fts MATCH 'searchable'").Scan(&match); err != nil || match != "searchable retained row" { + t.Errorf("external-content FTS lookup=%q, err=%v", match, err) + } + if _, err := restored.Exec("INSERT INTO recovery_fts(recovery_fts,rank) VALUES('integrity-check',1)"); err != nil { + t.Errorf("external-content FTS integrity: %v", err) + } + assertRestoreSQLiteTuple(t, backup, source) + }) + } +} + +func TestDBRestorePreservesDanglingDestinationSymlink(t *testing.T) { + for _, chain := range []bool{false, true} { + t.Run(fmt.Sprintf("chain=%v", chain), func(t *testing.T) { + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + referent := filepath.Join(t.TempDir(), "missing.db") + backup := filepath.Join(t.TempDir(), "backup.db") + target, err := filepath.Rel(dataDir, referent) + if err != nil { + t.Fatal(err) + } + if chain { + link := filepath.Join(dataDir, "intermediate.db") + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + target = filepath.Base(link) + } + if err := os.Symlink(target, live); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + writeRestoreSQLite(t, backup, "restored row", 4096, false) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + got, err := os.Readlink(live) + if err != nil || got != target { + t.Fatalf("dangling destination link replaced: target=%q err=%v", got, err) + } + assertRestoreSQLiteValue(t, referent, "restored row") + }) + } +} + +func TestDBRestorePublicationFailurePreservesDatabase(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("requires macOS user immutable file flags") + } + dataDir := t.TempDir() + live := filepath.Join(dataDir, "pad.db") + backup := filepath.Join(t.TempDir(), "backup.db") + writeRestoreSQLite(t, live, "original row", 4096, false) + writeRestoreSQLite(t, backup, "restored row", 4096, false) + before, source := restoreSQLiteTuple(t, live), restoreSQLiteTuple(t, backup) + if out, err := exec.Command("chflags", "uchg", live).CombinedOutput(); err != nil { + t.Skipf("immutable flags unavailable: %v: %s", err, out) + } + t.Cleanup(func() { + if out, err := exec.Command("chflags", "nouchg", live).CombinedOutput(); err != nil { + t.Errorf("clear immutable flag: %v: %s", err, out) + } + }) + setSQLiteRestoreEnv(t, dataDir) + if err := executeSQLiteRestore(t, backup); err == nil { + t.Error("restore succeeded despite immutable destination") + } + assertRestoreSQLiteTuple(t, live, before) + assertRestoreSQLiteValue(t, live, "original row") + assertRestoreSQLiteTuple(t, backup, source) } func setSQLiteRestoreEnv(t *testing.T, dataDir string) { diff --git a/cmd/pad/db_restore_ownership_other.go b/cmd/pad/db_restore_ownership_other.go new file mode 100644 index 000000000..1416473dd --- /dev/null +++ b/cmd/pad/db_restore_ownership_other.go @@ -0,0 +1,9 @@ +//go:build !unix + +package main + +import "os" + +func preserveSQLiteRestoreOwnership(_ *os.File, _ os.FileInfo) error { + return nil +} diff --git a/cmd/pad/db_restore_ownership_unix.go b/cmd/pad/db_restore_ownership_unix.go new file mode 100644 index 000000000..d18560d81 --- /dev/null +++ b/cmd/pad/db_restore_ownership_unix.go @@ -0,0 +1,20 @@ +//go:build unix + +package main + +import ( + "fmt" + "os" + "syscall" +) + +func preserveSQLiteRestoreOwnership(file *os.File, original os.FileInfo) error { + if original == nil { + return nil + } + stat, ok := original.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot determine database ownership") + } + return file.Chown(int(stat.Uid), int(stat.Gid)) +} diff --git a/cmd/pad/db_restore_ownership_unix_test.go b/cmd/pad/db_restore_ownership_unix_test.go new file mode 100644 index 000000000..d5b1de00c --- /dev/null +++ b/cmd/pad/db_restore_ownership_unix_test.go @@ -0,0 +1,36 @@ +//go:build unix + +package main + +import ( + "os" + "path/filepath" + "syscall" + "testing" +) + +func TestDBRestorePreservesServiceOwnership(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("changing the database to a different service owner requires root") + } + dir := t.TempDir() + live, backup := filepath.Join(dir, "pad.db"), filepath.Join(dir, "backup.db") + writeRestoreSQLite(t, live, "original row", 4096, false) + writeRestoreSQLite(t, backup, "restored row", 4096, false) + if err := os.Chown(live, 99, 99); err != nil { + t.Fatal(err) + } + setSQLiteRestoreEnv(t, dir) + if err := executeSQLiteRestore(t, backup); err != nil { + t.Fatal(err) + } + info, err := os.Stat(live) + if err != nil { + t.Fatal(err) + } + stat := info.Sys().(*syscall.Stat_t) + if stat.Uid != 99 || stat.Gid != 99 || info.Mode().Perm() != 0o600 { + t.Fatalf("restored owner/group/mode = %d/%d/%o", stat.Uid, stat.Gid, info.Mode().Perm()) + } + assertRestoreSQLiteValue(t, live, "restored row") +} diff --git a/cmd/pad/db_restore_sqlite.go b/cmd/pad/db_restore_sqlite.go new file mode 100644 index 000000000..241f6e006 --- /dev/null +++ b/cmd/pad/db_restore_sqlite.go @@ -0,0 +1,265 @@ +package main + +import ( + "context" + "database/sql" + "errors" + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "strings" + + "modernc.org/sqlite" +) + +// restoreSQLite publishes one self-contained database, never a partially copied +// main/WAL tuple. The server must be stopped throughout this operation. +func restoreSQLite(inputPath, dstPath string) error { + mode := os.FileMode(0o600) + var dstInfo os.FileInfo + if info, err := os.Stat(dstPath); err == nil { + if !info.Mode().IsRegular() { + return fmt.Errorf("database is not a regular file: %s", dstPath) + } + mode = info.Mode().Perm() + dstInfo = info + } else if !os.IsNotExist(err) { + return fmt.Errorf("stat database: %w", err) + } + // Keep configured symlinks intact, including links whose database was lost. + var err error + dstPath, err = resolveSQLiteRestorePath(dstPath) + if err != nil { + return fmt.Errorf("resolve database path: %w", err) + } + + stage, err := os.MkdirTemp(filepath.Dir(dstPath), ".pad-restore-*") + if err != nil { + return fmt.Errorf("stage restore: %w", err) + } + defer os.RemoveAll(stage) + staged := filepath.Join(stage, "backup.db") + for _, suffix := range []string{"", "-wal", "-shm"} { + if err := stageSQLiteRestoreFile(inputPath+suffix, staged+suffix, suffix != ""); err != nil { + return fmt.Errorf("stage backup%s: %w", suffix, err) + } + } + + // Read the private copy through SQLite, incorporating committed source WAL + // pages without modifying the original backup or publishing its SHM cache. + if err := checkSQLiteRestoreDatabase(staged); err != nil { + return fmt.Errorf("check backup database: %w", err) + } + publish := filepath.Join(stage, "restored.db") + if err := normalizeSQLiteRestore(staged, publish); err != nil { + return fmt.Errorf("prepare restored database: %w", err) + } + f, err := os.OpenFile(publish, os.O_RDWR, 0) + if err != nil { + return fmt.Errorf("open staged database: %w", err) + } + err = errors.Join(preserveSQLiteRestoreOwnership(f, dstInfo), f.Chmod(mode), f.Sync(), f.Close()) + if err != nil { + return fmt.Errorf("sync staged database: %w", err) + } + + // A live WAL is durable data, not disposable cache. Checkpoint it before + // removing sidecars so any later preparation/rename failure leaves the old + // committed contents readable from the main file. Never discard a WAL that + // SQLite cannot checkpoint (including a corrupt database or a held lock). + hasSidecars := false + for _, suffix := range []string{"-wal", "-shm", "-journal"} { + info, err := os.Stat(dstPath + suffix) + if os.IsNotExist(err) { + continue + } + if err != nil { + return fmt.Errorf("stat database%s: %w", suffix, err) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("database%s is not a regular file", suffix) + } + hasSidecars = true + } + if hasSidecars { + // Even closing a failed live SQLite connection can checkpoint its WAL. + // Validate a private copy first so a corrupt tuple is left untouched. + oldCopy := filepath.Join(stage, "current.db") + for _, suffix := range []string{"", "-wal", "-shm", "-journal"} { + if err := stageSQLiteRestoreFile(dstPath+suffix, oldCopy+suffix, suffix != ""); err != nil { + return fmt.Errorf("stage current database%s: %w", suffix, err) + } + } + if err := checkSQLiteRestoreDatabase(oldCopy); err != nil { + return fmt.Errorf("check current database before checkpoint: %w", err) + } + dsn, err := sqliteFileURI(dstPath) + if err != nil { + return err + } + db, err := sql.Open("sqlite", dsn) + if err != nil { + return fmt.Errorf("open database for checkpoint: %w", err) + } + var journalMode string + err = db.QueryRow("PRAGMA journal_mode=DELETE").Scan(&journalMode) + err = errors.Join(err, db.Close()) + if err != nil { + return fmt.Errorf("checkpoint database before restore: %w", err) + } + if journalMode != "delete" { + return fmt.Errorf("checkpoint database before restore: journal mode remains %s", journalMode) + } + } + for _, suffix := range []string{"-wal", "-shm", "-journal"} { + if err := os.Remove(dstPath + suffix); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove checkpointed %s: %w", suffix, err) + } + } + if err := os.Rename(publish, dstPath); err != nil { + return fmt.Errorf("publish restored database: %w", err) + } + return nil +} + +func resolveSQLiteRestorePath(path string) (string, error) { + for range 40 { + resolved, err := filepath.EvalSymlinks(path) + if err == nil || !os.IsNotExist(err) { + return resolved, err + } + info, err := os.Lstat(path) + if err == nil && info.Mode()&os.ModeSymlink != 0 { + target, err := os.Readlink(path) + if err != nil { + return "", err + } + if !filepath.IsAbs(target) { + target = filepath.Join(filepath.Dir(path), target) + } + path = target + continue + } + if err != nil && !os.IsNotExist(err) { + return "", err + } + parent, err := filepath.EvalSymlinks(filepath.Dir(path)) + return filepath.Join(parent, filepath.Base(path)), err + } + return "", errors.New("too many destination symlinks") +} + +// SQLite's backup API preserves hidden row IDs (unlike VACUUM INTO), including +// those used by external-content FTS indexes. Its destination is a fresh staged +// file, so live WAL mode, page size and corruption cannot impede the copy. +func normalizeSQLiteRestore(srcPath, dstPath string) (retErr error) { + dsn, err := sqliteFileURI(srcPath) + if err != nil { + return err + } + db, err := sql.Open("sqlite", dsn) + if err != nil { + return err + } + defer func() { retErr = errors.Join(retErr, db.Close()) }() + conn, err := db.Conn(context.Background()) + if err != nil { + return err + } + defer func() { retErr = errors.Join(retErr, conn.Close()) }() + dstURI, err := sqliteFileURI(dstPath) + if err != nil { + return err + } + return conn.Raw(func(raw any) error { + backup, err := raw.(interface { + NewBackup(string) (*sqlite.Backup, error) + }).NewBackup(dstURI) + if err != nil { + return err + } + more, stepErr := backup.Step(-1) + if more && stepErr == nil { + stepErr = errors.New("SQLite backup did not finish") + } + // Finalize even on failure to roll back the private destination, and + // close it explicitly: Backup.Finish discards its Close error. + dst, finishErr := backup.Commit() + if dst != nil { + finishErr = errors.Join(finishErr, dst.Close()) + } + return errors.Join(stepErr, finishErr) + }) +} + +func checkSQLiteRestoreDatabase(path string) error { + dsn, err := sqliteFileURI(path) + if err != nil { + return err + } + db, err := sql.Open("sqlite", dsn) + if err != nil { + return err + } + var result string + err = db.QueryRow("PRAGMA quick_check").Scan(&result) + err = errors.Join(err, db.Close()) + if err != nil { + return err + } + if result != "ok" { + return fmt.Errorf("SQLite quick_check: %s", result) + } + return nil +} + +// Escape filesystem names rather than letting SQLite interpret '?' or '#' as +// connection options. Windows drive paths need a leading slash in file URIs. +func sqliteFileURI(path string) (string, error) { + abs, err := filepath.Abs(path) + if err != nil { + return "", err + } + path = filepath.ToSlash(abs) + if filepath.VolumeName(abs) != "" && !strings.HasPrefix(path, "/") { + path = "/" + path + } + return (&url.URL{Scheme: "file", Path: path}).String(), nil +} + +func stageSQLiteRestoreFile(srcPath, dstPath string, optional bool) (retErr error) { + info, err := os.Stat(srcPath) + if optional && os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + if !info.Mode().IsRegular() || (!optional && info.Size() == 0) { + return fmt.Errorf("not a non-empty regular database file: %s", srcPath) + } + src, err := os.Open(srcPath) + if err != nil { + return err + } + defer func() { retErr = errors.Join(retErr, src.Close()) }() + // Recheck the open file before copying, rather than relying on path stat. + info, err = src.Stat() + if err != nil { + return err + } + if !info.Mode().IsRegular() { + return fmt.Errorf("not a regular file: %s", srcPath) + } + dst, err := os.OpenFile(dstPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) + if err != nil { + return err + } + defer func() { retErr = errors.Join(retErr, dst.Close()) }() + if _, err := io.Copy(dst, src); err != nil { + return err + } + return dst.Sync() +} diff --git a/cmd/pad/postgres_client_test.go b/cmd/pad/postgres_client_test.go index a9134dd79..50938ae02 100644 --- a/cmd/pad/postgres_client_test.go +++ b/cmd/pad/postgres_client_test.go @@ -188,6 +188,36 @@ func TestPostgresBackupRestoreConnection(t *testing.T) { t.Fatalf("round trip: value=%q err=%v", value, err) } } + t.Run("SQL failure rolls back and overrides psqlrc", func(t *testing.T) { + psqlrc := filepath.Join(t.TempDir(), "psqlrc") + if err := os.WriteFile(psqlrc, []byte("\\set ON_ERROR_STOP off\n"), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("PSQLRC", psqlrc) + t.Setenv("PAD_DATABASE_URL", targetURL) + failedArchive := filepath.Join(t.TempDir(), "sql-error.sql") + if err := os.WriteFile(failedArchive, []byte("UPDATE recovery_probe SET value='uncommitted restore';\nSELECT 1/0;\n"), 0600); err != nil { + t.Fatal(err) + } + restore := dbRestoreCmd() + restore.SetArgs([]string{"--force", failedArchive}) + if err := restore.Execute(); err == nil { + t.Error("restore returned success after a SQL error rolled back its transaction") + } + var value string + if err := target.QueryRow("SELECT value FROM recovery_probe").Scan(&value); err != nil || value != "restored row" { + t.Fatalf("failed restore changed target: value=%q err=%v", value, err) + } + // The same ambient psqlrc must not break a subsequent valid restore. + validRestore := dbRestoreCmd() + validRestore.SetArgs([]string{"--force", archive}) + if err := validRestore.Execute(); err != nil { + t.Fatal(err) + } + if err := target.QueryRow("SELECT value FROM recovery_probe").Scan(&value); err != nil || value != "restored row" { + t.Fatalf("valid restore after SQL error: value=%q err=%v", value, err) + } + }) missing := *u missing.Path, missing.RawPath = "/pad_missing_"+strings.ReplaceAll(uuid.NewString(), "-", ""), "" t.Setenv("PAD_DATABASE_URL", missing.String()) diff --git a/docs/backup.md b/docs/backup.md index ff2ab99f2..f9702c1f3 100644 --- a/docs/backup.md +++ b/docs/backup.md @@ -7,7 +7,7 @@ Pad provides built-in tooling for database backup, restore, and migration betwee | Command | Description | |---------|-------------| | `pad db backup` | Database backup — SQLite (`VACUUM INTO`, default) or PostgreSQL (`pg_dump`) | -| `pad db restore ` | Database restore — SQLite (file copy) or PostgreSQL (`psql`) | +| `pad db restore ` | Database restore — SQLite (staged atomic replacement) or PostgreSQL (`psql`) | | `pad db migrate-to-pg` | One-time SQLite → PostgreSQL migration | | `pad workspace export` | Application-level JSON export (portable) | | `pad workspace import` | Application-level JSON import | @@ -61,9 +61,29 @@ pad db restore ~/backups/pad-20250101.db pad server start ``` -Restore writes the backup over the resolved database path and clears any stale -`-wal`/`-shm` sidecars. Use `--force` to skip the confirmation prompt and -override the live-server guard (not recommended while the server is running). +Restore first copies the complete backup (including legacy `-wal`/`-shm` +sidecars when present) into a private staging directory beside the destination. +SQLite then creates a checked, self-contained database from that copy. The +original backup is not changed. Empty, malformed, unreadable, or non-file inputs +fail before the live database is opened. Allow disk space for both the staged +backup and its self-contained copy, plus a temporary copy of the current live +database when it has journal sidecars. + +Before atomically replacing the main database, restore checkpoints existing +live WAL data through SQLite and clears the checkpointed sidecars. Preparation +or publication failures retain the original committed contents, although a +successful checkpoint can change the physical files. If SQLite cannot +checkpoint an existing WAL (for example, a locked or corrupt live database), +restore refuses to discard it. A damaged main file without sidecars can still +be replaced. Destination symlinks are preserved; atomic replacement creates a +new inode, so other hard links retain the old database. Existing database file +mode bits are retained, along with its owner and group on Unix. Custom ACLs and +other filesystem metadata are not copied; reapply them before restarting if +your deployment requires them. The destination directory must be writable. + +Use `--force` to skip the confirmation prompt and override the live-server +guard. Keep the server stopped throughout restore, including when using +`--force`; it does not make concurrent database access safe. ## PostgreSQL Backups @@ -108,6 +128,12 @@ pad db restore /backups/pad-backup.sql pad db restore --force /backups/pad-backup.sql ``` +Restore uses `psql --single-transaction --set=ON_ERROR_STOP=on --no-psqlrc`. +A SQL error returns a command failure and rolls back the transaction instead +of reporting completion. User startup files cannot change these settings. +These guarantees apply to Pad/`pg_dump` backups; custom scripts that explicitly +commit or change transaction/error handling can override them. + ### Cloud Database Snapshots For managed PostgreSQL (AWS RDS, Google Cloud SQL, Azure Database):