Skip to content

Use VACUUM INTO for consistent database snapshots in backups - #454

Merged
tomcardoso merged 2 commits into
mainfrom
fix/issue-437-backup-snapshot
Jul 20, 2026
Merged

Use VACUUM INTO for consistent database snapshots in backups#454
tomcardoso merged 2 commits into
mainfrom
fix/issue-437-backup-snapshot

Conversation

@tomcardoso

Copy link
Copy Markdown
Owner

Closes #437

Three paths copied the live, open database with plain filesystem calls while writes could be in flight, risking a torn copy: hourly auto-backup, backup:export, and settings:move-vault. A corrupt backup is discovered exactly when it's needed most.

Approach

Adds snapshotDatabase(destPath) in src/main/database/index.ts, which runs VACUUM INTO through the open connection, and routes all three call sites through it.

Note on the issue's fix sketch: it suggested db.backup(destPath) or VACUUM INTO. db.backup() does not work here — it throws backup is not supported with incompatible source and target databases on a SQLCipher database. Only VACUUM INTO is viable.

Verified empirically against the repo's native module before implementing:

  • the snapshot remains fully encrypted — no plaintext SQLite header, no plaintext row data, still requires the key
  • user_version is preserved (migrations depend on it)
  • VACUUM INTO refuses to write to an existing path, so every call site targets a fresh temp path
  • the destination is passed as a bound parameter, not interpolated — a vault folder named Tom's Backups would otherwise break or inject

Changes

  • src/main/database/index.ts — new snapshotDatabase() helper
  • src/main/ipc/backup.tsfs.copyFilesnapshotDatabase (existing temp-dir plumbing reused)
  • src/main/ipc/auto-backup.ts — snapshots to a temp dir, cleaned up in finally
  • src/main/ipc/settings.tsfs.cpsnapshotDatabase; corrected the comment claiming DELETE journal mode makes a live copy safe at all times (it's only safe between transactions, which is the bug)
  • src/test/database-snapshot.test.ts — new; pins readability with the key, user_version preservation, the not-plaintext property, and the apostrophe-path regression

The not-plaintext assertion is the important one long-term: it guards against a future change silently producing an unencrypted backup.

Tradeoff worth knowing

VACUUM INTO is synchronous and briefly blocks the main process while it runs, where the old fs.readFile/copyFile was async. This is consistent with how the rest of the codebase does DB work on the main process, and the DB is small relative to screenshots (stored separately), but it does mean an hourly pause that scales with database size. Flagging rather than pre-optimising.

Testing

npm run typecheck clean; npm test 523 passed across 30 files.

tomcardoso and others added 2 commits July 20, 2026 13:10
Auto-backup, manual backup export, and vault move all copied the live,
open SQLite file with plain filesystem calls, which can capture a torn
database if a write is in flight. Add snapshotDatabase() (VACUUM INTO,
via a bound parameter so paths with apostrophes work) and route all
three call sites through it instead of fs.readFile/copyFile/cp.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Creating the snapshot temp dir outside the try meant an mkdtemp failure
escaped as a rejected promise rather than the documented
{ success: false, error } result, which the backup:run-auto IPC handler
returns directly to the renderer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tomcardoso
tomcardoso merged commit 399affc into main Jul 20, 2026
1 check passed
@tomcardoso
tomcardoso deleted the fix/issue-437-backup-snapshot branch July 20, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backups copy the live DB file without a consistent snapshot

1 participant