fix(database-backup): Postgres-safe insert destructure (runs the inline dump)

databaseBackupService.backup() did `const [runId] = await db(...).insert({...})`
without a .returning() — works on SQLite (knex returns [lastInsertId]) but
throws "(intermediate value) is not iterable" on Postgres (knex returns
a non-iterable shape).
Bug was latent until Stage A of the backup-hardening plan wired this
method into the "Run Backup Now" inline-dump path. Before Stage A only
the scheduled cron + the dedicated admin-DB-backup page called it, and
Ralf's install had never exercised either — so the inline-dump default
landing in production was the first time the destructure ran on his PG.
Cure: same explicit .returning('id') + dual-shape coalesce pattern that
backupService.js uses for its own backup_runs insert (line 949).
Two more sibling files have the same anti-pattern (userManagementService,
customerAccountsService — invitation flows) and will bite under the
same conditions; spawned a follow-up task to fix them in a separate PR.
This commit is contained in:
Luca
2026-05-30 02:56:10 +02:00
parent 03e6617f38
commit f741e88acb
+19 -3
View File
@@ -309,8 +309,23 @@ class DatabaseBackupService {
// Get current schema version // Get current schema version
const schemaVersion = await this.getCurrentSchemaVersion(); const schemaVersion = await this.getCurrentSchemaVersion();
// Create backup run record with version info // Create backup run record with version info.
const [runId] = await db('database_backup_runs').insert({ //
// Insert shape divergence between SQLite + Postgres made the old
// `const [runId] = await db(...).insert({...})` form throw
// "(intermediate value) is not iterable" on Postgres installs:
//
// - SQLite-via-knex: insert() returns `[lastInsertId]` (array)
// - Postgres-via-knex: insert() without .returning() returns an
// empty object / row count — not iterable
//
// Bug went undetected until Stage A wired this method into the
// "Run Backup Now" inline-dump path — before that, only the
// scheduled-cron + dedicated-admin-page callers exercised it,
// and Ralf's install had never triggered either. Cure: same
// explicit `.returning('id')` + dual-shape coalesce pattern that
// `backupService.js:949` uses for its own `backup_runs` insert.
const insertResult = await db('database_backup_runs').insert({
started_at: startTime, started_at: startTime,
status: 'running', status: 'running',
backup_type: this.dbType, backup_type: this.dbType,
@@ -324,7 +339,8 @@ class DatabaseBackupService {
node_env: process.env.NODE_ENV || 'production', node_env: process.env.NODE_ENV || 'production',
db_type: this.dbType db_type: this.dbType
}) })
}); }).returning('id');
const runId = insertResult[0]?.id || insertResult[0];
backupRun = { id: runId }; backupRun = { id: runId };