diff --git a/backend/internal/system/backup.go b/backend/internal/system/backup.go index 393d951..7cb56b8 100644 --- a/backend/internal/system/backup.go +++ b/backend/internal/system/backup.go @@ -8,6 +8,7 @@ import ( "io" "os" "path/filepath" + "strings" "time" _ "modernc.org/sqlite" @@ -22,24 +23,63 @@ type BackupPaths struct { // WriteZip creates a zip archive containing the SQLite database and Mihomo config // when present. The caller owns closing the writer. -// flushSQLiteWAL checkpoints the WAL into the main database file so a plain -// file copy of dbPath contains recent writes (listener↔user bindings, etc.). -// Without this, ExportBackup can miss rows that only lived in the -wal file. -func flushSQLiteWAL(dbPath string) { +// snapshotSQLiteDB produces a crash-consistent copy of the live SQLite database +// at dbPath into destPath without stopping the running panel. +// +// The panel's own GORM connection pool keeps the database open and may write to +// it at any moment. A raw file copy of dbPath (the old approach) races with +// those writes and can capture a torn page, and a WAL checkpoint alone is not +// enough — under a busy writer the checkpoint can leave frames in -wal that a +// plain copy never replays. Instead we use SQLite's online backup primitive +// VACUUM INTO, which reads through the WAL and any in-flight transactions to +// produce a self-contained, fully-replayed snapshot. The destination is a +// complete database file, so the backup no longer needs (and must not carry) +// the -wal/-shm sidecars. +func snapshotSQLiteDB(dbPath, destPath string) error { if dbPath == "" { - return + return fmt.Errorf("database path is empty") + } + // VACUUM INTO requires the destination to not already exist. + if err := os.Remove(destPath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("prepare snapshot destination: %w", err) } db, err := sql.Open("sqlite", dbPath) if err != nil { - return + return fmt.Errorf("open live database: %w", err) } defer db.Close() - _, _ = db.Exec(`PRAGMA busy_timeout=5000`) - _, _ = db.Exec(`PRAGMA wal_checkpoint(TRUNCATE)`) + if _, err := db.Exec(`PRAGMA busy_timeout=5000`); err != nil { + return fmt.Errorf("set busy_timeout: %w", err) + } + // VACUUM INTO takes a filename literal, not a bind parameter. + if _, err := db.Exec(fmt.Sprintf("VACUUM INTO %s", sqliteQuotePath(destPath))); err != nil { + return fmt.Errorf("vacuum into %s: %w", destPath, err) + } + return nil +} + +// sqliteQuotePath quotes a filesystem path as a single-quoted SQLite string +// literal, escaping embedded single quotes per SQLite string rules. +func sqliteQuotePath(p string) string { + return "'" + strings.ReplaceAll(p, "'", "''") + "'" } func WriteZip(w io.Writer, paths BackupPaths) error { - flushSQLiteWAL(paths.DatabasePath) + dbPath := paths.DatabasePath + // Snapshot the live database into a temp file so the zip carries a + // crash-consistent copy instead of whatever bytes the open file happened to + // expose mid-write. If the db file is absent we keep dbPath as-is (addFile + // skips a missing file), preserving the previous best-effort behavior. + if dbPath != "" { + if _, err := os.Stat(dbPath); err == nil { + snap := dbPath + ".backup-snapshot" + if serr := snapshotSQLiteDB(dbPath, snap); serr != nil { + return fmt.Errorf("snapshot database: %w", serr) + } + defer os.Remove(snap) + dbPath = snap + } + } zw := zip.NewWriter(w) defer zw.Close() @@ -76,12 +116,13 @@ func WriteZip(w io.Writer, paths BackupPaths) error { return err } - if err := addFile("3m-ui.db", paths.DatabasePath); err != nil { + if err := addFile("3m-ui.db", dbPath); err != nil { return fmt.Errorf("database: %w", err) } - // Best-effort: if checkpoint could not run, keep sidecars so restore can replay. - _ = addFile("3m-ui.db-wal", paths.DatabasePath+"-wal") - _ = addFile("3m-ui.db-shm", paths.DatabasePath+"-shm") + // No longer bundling 3m-ui.db-wal / 3m-ui.db-shm: the VACUUM INTO snapshot is + // already self-contained (WAL replayed into the main file), so the sidecars + // would be dead weight. They were previously bundled but never restored, + // which silently dropped any un-checkpointed writes on restore. if err := addFile("mihomo-config.yaml", paths.MihomoConfig); err != nil { return fmt.Errorf("mihomo config: %w", err) } diff --git a/backend/internal/system/backup_consistency_test.go b/backend/internal/system/backup_consistency_test.go new file mode 100644 index 0000000..084708e --- /dev/null +++ b/backend/internal/system/backup_consistency_test.go @@ -0,0 +1,108 @@ +package system + +import ( + "archive/zip" + "bytes" + "database/sql" + "io" + "os" + "path/filepath" + "testing" + + _ "modernc.org/sqlite" +) + +// TestWriteZipSnapshotIsConsistentAndSelfContained verifies that WriteZip copies +// a live, WAL-mode database into a crash-consistent snapshot and no longer +// bundles the -wal/-shm sidecars. +// +// Regression for two issues: +// - A raw file copy of an open, actively-written database can capture a torn +// page; VACUUM INTO must instead produce a fully-replayed copy. +// - The previous code bundled 3m-ui.db-wal / 3m-ui.db-shm but RestoreDatabase +// never read them, so un-checkpointed writes were silently lost on restore. +func TestWriteZipSnapshotIsConsistentAndSelfContained(t *testing.T) { + dir := t.TempDir() + dbPath := filepath.Join(dir, "3m-ui.db") + + // Open a real database and keep the handle open to simulate the running + // GORM pool writing concurrently while a backup is taken. + src, err := sql.Open("sqlite", dbPath) + if err != nil { + t.Fatalf("open source db: %v", err) + } + defer src.Close() + if _, err := src.Exec(`PRAGMA busy_timeout=5000`); err != nil { + t.Fatalf("busy_timeout: %v", err) + } + if _, err := src.Exec(`PRAGMA journal_mode=WAL`); err != nil { + t.Fatalf("set WAL: %v", err) + } + if _, err := src.Exec(`CREATE TABLE listeners (id INTEGER PRIMARY KEY, name TEXT)`); err != nil { + t.Fatalf("create table: %v", err) + } + // Insert enough rows that some land in the -wal file (not yet checkpointed). + for i := 0; i < 200; i++ { + if _, err := src.Exec(`INSERT INTO listeners (id, name) VALUES (?, ?)`, i, "listener-"+string(rune('a'+i%26))); err != nil { + t.Fatalf("insert row %d: %v", i, err) + } + } + + // Snapshot while the source connection is still open and dirty. + var buf bytes.Buffer + if err := WriteZip(&buf, BackupPaths{DatabasePath: dbPath}); err != nil { + t.Fatalf("WriteZip: %v", err) + } + + // Read the archive back. + zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len())) + if err != nil { + t.Fatalf("open zip: %v", err) + } + var dbBytes []byte + sawWal := false + sawShm := false + for _, f := range zr.File { + switch f.Name { + case "3m-ui.db": + rc, e := f.Open() + if e != nil { + t.Fatalf("open 3m-ui.db in zip: %v", e) + } + dbBytes, e = io.ReadAll(rc) + rc.Close() + if e != nil { + t.Fatalf("read 3m-ui.db in zip: %v", e) + } + case "3m-ui.db-wal": + sawWal = true + case "3m-ui.db-shm": + sawShm = true + } + } + if sawWal || sawShm { + t.Fatalf("backup still bundles sidecars: wal=%v shm=%v", sawWal, sawShm) + } + if len(dbBytes) == 0 { + t.Fatal("zip contains no 3m-ui.db") + } + + // The snapshot must be a complete, replayed database: open it standalone + // (no source connection, no -wal present) and confirm every row survived. + restored := filepath.Join(dir, "restored.db") + if err := os.WriteFile(restored, dbBytes, 0o600); err != nil { + t.Fatalf("write restored db: %v", err) + } + rdb, err := sql.Open("sqlite", restored) + if err != nil { + t.Fatalf("open restored db: %v", err) + } + defer rdb.Close() + var n int + if err := rdb.QueryRow(`SELECT COUNT(*) FROM listeners`).Scan(&n); err != nil { + t.Fatalf("count restored rows: %v", err) + } + if n != 200 { + t.Fatalf("restored db missing rows: got %d want 200", n) + } +}