fix(api): create DB export temp paths with mkdtemp instead of predictable timestamps (#12579)

Both db-backups/exportAll and db-backups/export built temp paths from a
deterministic timestamp under os.tmpdir() and wrote into them without any
exclusive-creation guard. A local attacker could pre-place a symlink at the
predictable path; mkdirSync({recursive:true})/writeFileSync then silently
followed it (TOCTOU / symlink-following) instead of failing, redirecting the
backup write into an attacker-controlled location.

Replace both with fs.mkdtempSync (unique, exclusive, 0700) matching the
existing convention at src/mitm/systemCommands.ts:236-247. Cleanup now
removes the mkdtemp-created directory recursively on every path (success,
db.backup failure, request abort), instead of unlinking a single file.

changelog.d/fixes/12579-db-export-tempdir-mkdtemp.md

@@ -0,0 +1 @@
+- fix(api): create DB export temp paths with `fs.mkdtempSync` instead of predictable timestamps (#12579)

src/app/api/db-backups/export/route.ts

@@ -27,20 +27,28 @@ export async function GET(request: Request) {
 
     const timestamp = new Date().toISOString().replace(/[:.]/g, "-");
     const exportFilename = `omniroute-backup-${timestamp}.sqlite`;
-    const tmpDir = os.tmpdir();
-    const tmpPath = path.join(tmpDir, exportFilename);
+    // Use mkdtempSync (exclusive creation, random suffix) instead of a
+    // deterministic timestamp path — a predictable path lets a local
+    // attacker pre-place a symlink and redirect the write (TOCTOU).
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-backup-"));
+    const tmpPath = path.join(tmpDir, "backup.sqlite");
 
     // Use native SQLite backup API for a consistent snapshot
     const db = getDbInstance();
-    await db.backup(tmpPath);
+    try {
+      await db.backup(tmpPath);
+    } catch (backupError) {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+      throw backupError;
+    }
 
     const { size: fileSize } = fs.statSync(tmpPath);
     const readStream = fs.createReadStream(tmpPath);
 
-    // Cleanup temp file on completion, error, or client abort
+    // Cleanup temp dir (and everything in it) on completion, error, or client abort
     const cleanup = () => {
       readStream.destroy();
-      fs.unlink(tmpPath, () => {});
+      fs.rm(tmpDir, { recursive: true, force: true }, () => {});
     };
     request.signal.addEventListener("abort", cleanup, { once: true });
 

src/app/api/db-backups/exportAll/route.ts

@@ -28,13 +28,13 @@ export async function GET(request: NextRequest) {
 
     const db = getDbInstance();
     const timestamp = new Date().toISOString().replace(/[:.]/g, "-").slice(0, 19);
-    const tempDir = path.join(os.tmpdir(), `omniroute-export-${timestamp}`);
+    // Use mkdtempSync (exclusive creation, random suffix) instead of a
+    // deterministic timestamp path — a predictable path lets a local
+    // attacker pre-place a symlink and redirect the write (TOCTOU).
+    const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-export-"));
     const zipPath = path.join(os.tmpdir(), `omniroute-full-backup-${timestamp}.zip`);
 
     try {
-      // Create temp directory
-      fs.mkdirSync(tempDir, { recursive: true });
-
       // 1. Export database using native backup API
       const dbBackupPath = path.join(tempDir, "storage.sqlite");
       await db.backup(dbBackupPath);

tests/unit/db-backup-export-streaming-9045.test.ts

@@ -61,14 +61,16 @@ test("temp file cleanup on stream completion, error, and abort (#9045)", () => {
     "utf-8"
   );
 
-  // The fix must clean up the temp file on stream completion and client abort
+  // The fix must clean up the temp dir on stream completion and client abort
+  // (#12579: the temp path moved from a single unlink-able file to an
+  // fs.mkdtempSync-created directory, so cleanup now recursively removes it)
   assert.ok(
     source.includes("cleanup"),
     "route must have a cleanup function for temp file removal"
   );
   assert.ok(
-    source.includes("unlink("),
-    "route must call unlink on the temp file during cleanup"
+    source.includes("rm(") || source.includes("unlink("),
+    "route must remove the temp file/dir during cleanup"
   );
   assert.ok(
     source.includes("abort"),

tests/unit/db-backup-tempdir-mkdtemp.test.ts

@@ -0,0 +1,61 @@
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { test } from "node:test";
+
+// Regression guard for #12579: DB export temp paths must be created via
+// fs.mkdtempSync (unique + exclusive) rather than a predictable, deterministic
+// timestamp-derived path passed to mkdirSync({ recursive: true }) or a raw
+// write target. A deterministic path lets a local attacker pre-place a
+// symlink at the predicted location; mkdirSync/writeFileSync then silently
+// follow it (TOCTOU / symlink-following) instead of failing.
+
+const exportAllSource = fs.readFileSync(
+  path.join(process.cwd(), "src/app/api/db-backups/exportAll/route.ts"),
+  "utf8"
+);
+const exportSource = fs.readFileSync(
+  path.join(process.cwd(), "src/app/api/db-backups/export/route.ts"),
+  "utf8"
+);
+
+test("exportAll/route.ts: uses fs.mkdtempSync to create the temp export directory", () => {
+  assert.match(exportAllSource, /fs\.mkdtempSync\(/);
+});
+
+test("exportAll/route.ts: never passes a manually-built timestamp path to mkdirSync", () => {
+  assert.doesNotMatch(exportAllSource, /fs\.mkdirSync\(\s*tempDir/);
+});
+
+test("export/route.ts: uses fs.mkdtempSync to create the temp export directory", () => {
+  assert.match(exportSource, /fs\.mkdtempSync\(/);
+});
+
+test("export/route.ts: the sqlite backup write target lives inside an mkdtemp-created directory, not a bare tmpdir path", () => {
+  assert.doesNotMatch(exportSource, /path\.join\(tmpDir,\s*exportFilename\)/);
+});
+
+test("mkdtempSync-based paths are unique across two calls made within the same millisecond (no timestamp collision)", () => {
+  const a = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-export-"));
+  const b = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-export-"));
+  try {
+    assert.notEqual(a, b);
+  } finally {
+    fs.rmSync(a, { recursive: true, force: true });
+    fs.rmSync(b, { recursive: true, force: true });
+  }
+});
+
+test("mkdtempSync rejects a pre-placed symlink at the target prefix path (exclusive creation, no TOCTOU)", () => {
+  // mkdtempSync always appends 6 random characters, so an attacker cannot
+  // predict (and therefore cannot pre-place a symlink at) the final path —
+  // unlike the old `mkdirSync(deterministicPath, { recursive: true })`.
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-export-"));
+  try {
+    assert.ok(fs.lstatSync(dir).isDirectory());
+    assert.ok(!fs.lstatSync(dir).isSymbolicLink());
+  } finally {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});