paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(backup): inline DB dump + fail-loud guard so "Run Backup Now" can't ship files-only

Browse Source 
The previous file-backup workflow only LOOKED UP an existing database
dump via getDatabaseBackupInfo() and silently shipped a files-only
manifest when none was found. Admins clicking "Run Backup Now" (or
relying on the schedule) got an apparent success that omitted every
customer / quote / invoice / contract / payment-log row. The
data-loss footgun was discovered 2026-05-29 when an admin who'd been
"backing up" for weeks via the UI lost the entire CRM after a routine
docker compose down -v — every produced manifest had database:
{ backup_file: null, size: 0, tables: {} }.

Changes to runBackupInternal:

  1. Inline pg_dump (or SQLite copy) before the file scan, via
     databaseBackupService.backup(). Result lands in
     database_backup_runs and is picked up by the existing
     getDatabaseBackupInfo lookup that writes the manifest.

  2. Fail-loud guard after the dump step: if no usable dump file is
     reachable (path missing, 0 bytes, or never existed), throw —
     the existing catch block marks the backup_runs row failed with
     the error_message and emails the admin if configured. No more
     silent files-only manifests.

  3. Opt-out: `backup_database_inline_dump = false` skips the inline
     dump for admins who already run their own scheduled
     `backup_database_schedule`. The fail-loud guard still applies,
     so an opted-out install with no recent dump still aborts loudly
     instead of producing a partial backup. Default ON is encoded
     as "skip only when explicitly false" — undefined (existing
     installs upgrading) falls through to the safe-default ON path.

Test suite covers: default-on happy path, dump-throws-aborts-run,
opt-out + recent dump + proceeds, opt-out + no-dump + fail-loud,
opt-out + 0-byte dump + fail-loud. Mocks
databaseBackupService.backup so the tests don't depend on pg_dump
or sqlite3 CLI binaries being installed.

Stage A of three-stage backup hardening plan. Stage B (config-driven
walker) and Stage C (audit + diagnostic UI) follow in separate
commits.
This commit is contained in:
Luca
2026-05-29 21:57:12 +02:00
parent 5b3bfed144
commit 7c230bdc24
3 changed files with 249 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/services/backupService.js
+60
View File
@@ -814,6 +814,66 @@ async function runBackupInternal(isManual = false) {
}).returning('id');
runId = insertResult[0]?.id || insertResult[0];
// Inline database dump (default ON). Previously, runBackup only LOOKED UP
// an existing database dump via getDatabaseBackupInfo and silently shipped
// a files-only manifest when none was found — admins clicking "Run Backup
// Now" got an apparent success that omitted every customer / quote /
// invoice / contract row. Triggering pg_dump (or the SQLite copy) here
// makes "file backup" always include a fresh database snapshot. Admins
// who run their own scheduled dumps via backup_database_schedule can opt
// out with backup_database_inline_dump = false; the fail-loud guard
// below still catches the case where no recent dump exists.
//
// Default ON is encoded as "skip only when explicitly false". `undefined`
// (setting not yet inserted on existing installs) falls through to the
// ON path, which is the data-loss-safe default. normalizeBoolean(undefined)
// returns false, so checking inequality against false would inadvertently
// disable on unset — guard with `!== undefined` first.
const inlineDumpExplicitlyOff = config.backup_database_inline_dump !== undefined
&& config.backup_database_inline_dump !== null
&& normalizeBoolean(config.backup_database_inline_dump) === false;
if (!inlineDumpExplicitlyOff) {
logger.info('Running inline database dump before file backup...');
const { databaseBackupService } = require('./databaseBackup');
const dumpResult = await databaseBackupService.backup({});
logger.info(`Inline database dump completed: ${dumpResult.path} ` +
`(${(dumpResult.size / 1024 / 1024).toFixed(2)} MB)`);
}
// Fail-loud guard: a "file backup" without a DB component is a data-loss
// trap. Whether the dump came from the inline step above or from a
// separately-scheduled database backup, we require a usable dump file
// before proceeding. Throws — the catch block marks the backup_runs row
// failed with this error_message and emails the admin if configured.
const dbInfoCheck = await service.getDatabaseBackupInfo();
if (!dbInfoCheck.backupFile) {
throw new Error(
'No database backup available to include in this file backup. ' +
'Either keep backup_database_inline_dump enabled (default) or configure ' +
'backup_database_schedule and let it run at least once first.'
);
}
try {
const dumpStat = await fs.stat(dbInfoCheck.backupFile);
if (!dumpStat.size || dumpStat.size === 0) {
throw new Error(
`Database backup file at ${dbInfoCheck.backupFile} is empty (0 bytes). ` +
'Refusing to proceed with file backup to avoid shipping a manifest with no DB content.'
);
}
} catch (statErr) {
// fs.stat throws if file doesn't exist; preserve the more specific
// empty-file error from the inner block.
if (statErr.code === 'ENOENT') {
throw new Error(
`Database backup file at ${dbInfoCheck.backupFile} is missing from disk. ` +
'Refusing to proceed with file backup; configure backup_database_schedule or ' +
'keep backup_database_inline_dump enabled.'
);
}
throw statErr;
}
const files = await service.getFilesToBackup(config.backup_include_archived);
logger.info(`Found ${files.length} files to check for backup`);