From d34036c4efd7769ef4896107cc53ea1a4cd1fa84 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sat, 30 May 2026 03:26:10 +0200 Subject: [PATCH] fix(safe-exec): Node 22-compatible stdio + error-bridge for spawnTo/FromFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spawnToFile and spawnFromFile passed an unopened WriteStream/ReadStream directly as a stdio entry to child_process.spawn. Older Node versions auto-extracted .fd; Node 22 throws synchronously: The argument 'stdio' is invalid. Received WriteStream { fd: null, path: '/backup/database/...sql', ... } Bug bit Ralf's install once today's `bugfix/crm-backup` image landed — Node 22 came with that image, and Stage A's inline-dump path is the first caller of spawnToFile on this install. Latent on the previous image (Node 20); fatal on this one. restoreService's pre-restore safety snapshot uses the same helper and would have hit it next time a restore ran. Cure: stdio: ['ignore', 'pipe', 'pipe'] (and ['pipe', 'pipe', 'pipe'] for spawnFromFile) + manual pipe of child.stdout/stdin through the file stream. Works on every Node version. Also wires the WriteStream's 'error' event to the promise via settleReject so a future EACCES / ENOSPC reaches the caller's try/catch instead of becoming a process- fatal unhandled error event — closing the same "Stage A guard bypassed" hole noted in the spawned follow-up task. Side benefit: outStream.end() now awaits flush before resolving, so fast pg_dump runs can no longer produce a truncated dump. --- backend/src/utils/safeExec.js | 110 +++++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/backend/src/utils/safeExec.js b/backend/src/utils/safeExec.js index efba1ae7..076b20eb 100644 --- a/backend/src/utils/safeExec.js +++ b/backend/src/utils/safeExec.js @@ -41,60 +41,128 @@ function spawnAsync(cmd, args = [], options = {}) { /** * Run a command and redirect stdout to a file (replaces shell `> file`). + * + * Historically this passed `fs.createWriteStream(outputPath)` directly as + * `stdio[1]` to `child_process.spawn`. That relied on Node auto-extracting + * the WriteStream's `.fd` — but the stream opens async, so on a fast call + * `fd` is still `null` when `spawn()` reads it. Older Node releases would + * tolerate this; Node 22 throws synchronously with + * `The argument 'stdio' is invalid. Received WriteStream { fd: null, ... }`. + * + * Cure: use `stdio: ['ignore', 'pipe', 'pipe']` and wire the WriteStream + * up via the streams API (`stdout.pipe(outStream)`). Works on every Node + * version; also gives us a clean error bridge from both the WriteStream + * AND the child process to the promise, instead of the previous code's + * blind `outStream.destroy()` / `outStream.end()` calls that left stream + * errors uncaught (Node 22 process-fatal — separate footgun this fixes + * by the same change). + * + * Used by: + * - databaseBackup.createPostgreSQLBackup (inline-dump path, the + * thing that just bit Ralf's install) + * - restoreService pre-restore safety snapshot (would have hit the + * same on next restore attempt) */ function spawnToFile(cmd, args, outputPath, options = {}) { const fs = require('fs'); return new Promise((resolve, reject) => { const outStream = fs.createWriteStream(outputPath); + let settled = false; + const settleReject = (err) => { + if (settled) return; + settled = true; + try { outStream.destroy(); } catch (_) { /* best effort */ } + reject(err); + }; + const settleResolve = (value) => { + if (settled) return; + settled = true; + resolve(value); + }; + + // Bridge WriteStream errors (EACCES, ENOSPC, etc.) to the promise. + // Without this, an unhandled 'error' event on the stream is process- + // fatal on Node 22 and bypasses the caller's try/catch entirely — + // which is exactly the failure mode that crashed the picpeak + // backend container on its first inline-dump attempt. + outStream.on('error', settleReject); + const child = spawn(cmd, args, { shell: false, ...options, - stdio: ['ignore', outStream, 'pipe'] + stdio: ['ignore', 'pipe', 'pipe'] }); + // Pipe stdout → file. The pipe call attaches its own 'error' + // handlers on both ends so a child-stdout failure also reaches us. + child.stdout.pipe(outStream); + const stderrChunks = []; child.stderr.on('data', chunk => stderrChunks.push(chunk)); + child.stderr.on('error', settleReject); - child.on('error', (err) => { - outStream.destroy(); - reject(err); - }); + child.on('error', settleReject); child.on('close', (code) => { - outStream.end(); - const stderr = Buffer.concat(stderrChunks).toString(); - if (code !== 0) { - const err = new Error(`${cmd} exited with code ${code}: ${stderr}`); - err.code = code; - err.stderr = stderr; - return reject(err); - } - resolve({ stderr }); + // Wait for the file write to flush before resolving — otherwise + // a fast 'close' could resolve while the WriteStream still has + // buffered bytes, producing a truncated dump. + outStream.end(() => { + const stderr = Buffer.concat(stderrChunks).toString(); + if (code !== 0) { + const err = new Error(`${cmd} exited with code ${code}: ${stderr}`); + err.code = code; + err.stderr = stderr; + return settleReject(err); + } + settleResolve({ stderr }); + }); }); }); } /** * Run a command and pipe a file into stdin (replaces shell `< file`). + * + * Same Node 22 stdio strictness applies as for `spawnToFile` above — the + * ReadStream `fd` is null at spawn time. Use `stdio[0] = 'pipe'` and pipe + * the file stream into `child.stdin` via the streams API instead. */ function spawnFromFile(cmd, args, inputPath, options = {}) { const fs = require('fs'); return new Promise((resolve, reject) => { const inStream = fs.createReadStream(inputPath); + let settled = false; + const settleReject = (err) => { + if (settled) return; + settled = true; + try { inStream.destroy(); } catch (_) { /* best effort */ } + reject(err); + }; + const settleResolve = (value) => { + if (settled) return; + settled = true; + resolve(value); + }; + + inStream.on('error', settleReject); + const child = spawn(cmd, args, { shell: false, ...options, - stdio: [inStream, 'pipe', 'pipe'] + stdio: ['pipe', 'pipe', 'pipe'] }); + inStream.pipe(child.stdin); + const stdoutChunks = []; const stderrChunks = []; child.stdout.on('data', chunk => stdoutChunks.push(chunk)); + child.stdout.on('error', settleReject); child.stderr.on('data', chunk => stderrChunks.push(chunk)); + child.stderr.on('error', settleReject); + child.stdin.on('error', settleReject); - child.on('error', (err) => { - inStream.destroy(); - reject(err); - }); + child.on('error', settleReject); child.on('close', (code) => { const stdout = Buffer.concat(stdoutChunks).toString(); const stderr = Buffer.concat(stderrChunks).toString(); @@ -103,9 +171,9 @@ function spawnFromFile(cmd, args, inputPath, options = {}) { err.code = code; err.stdout = stdout; err.stderr = stderr; - return reject(err); + return settleReject(err); } - resolve({ stdout, stderr }); + settleResolve({ stdout, stderr }); }); }); }