fix(backupRoutes): async backup existence check + headersSent guards (#3524) (#3529)

Harden the system-backup download handler (carried verbatim from server.ts in
#3523, now isolated and easy to fix):

- Replace the synchronous fs.existsSync (which blocked the event loop in the
  request path) with an async fsp.access check via a static fs/promises import.
- Guard both the archiver 'error' handler and the catch block with
  res.headersSent — the archive can error / finalize() can throw after
  archive.pipe(res) has begun streaming, which previously threw 'Cannot set
  headers after they are sent'. When already streaming, terminate via res.end().

Tests: add a 404 case (missing backup → async access rejects) and a happy-path
streaming case (real temp dir → real archiver → 200 tar.gz + audit log).

Closes #3524.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Yeraze

src/server/routes/backupRoutes.test.ts

@@ -1,6 +1,9 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import request from 'supertest';
 import express from 'express';
+import { mkdtemp, writeFile, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
 
 const mockBackupFileService = vi.hoisted(() => ({
   listBackups: vi.fn(),
@@ -171,6 +174,33 @@ describe('backupRoutes - system backups', () => {
     expect(res.status).toBe(400);
   });
 
+  it('GET /system/backup/download returns 404 when the backup is missing (async fs check, #3524)', async () => {
+    // Points at a path that doesn't exist → the new async fsp.access rejects → 404.
+    mockSystemBackupService.getBackupPath.mockReturnValue(
+      join(tmpdir(), 'mm-3524-absent', '2099-01-01_000000')
+    );
+    const res = await request(app).get('/system/backup/download/2099-01-01_000000');
+    expect(res.status).toBe(404);
+    expect(res.body.error).toBe('Backup not found');
+  });
+
+  it('GET /system/backup/download streams a tar.gz for an existing backup (#3524 happy path)', async () => {
+    const dir = await mkdtemp(join(tmpdir(), 'mm-3524-backup-'));
+    await writeFile(join(dir, 'meshmonitor.db'), 'fake-sqlite-bytes');
+    mockSystemBackupService.getBackupPath.mockReturnValue(dir);
+    try {
+      const res = await request(app).get('/system/backup/download/2099-01-01_000000');
+      expect(res.status).toBe(200);
+      expect(res.headers['content-type']).toContain('gzip');
+      expect(res.headers['content-disposition']).toContain('2099-01-01_000000.tar.gz');
+      expect(databaseService.auditLogAsync).toHaveBeenCalledWith(
+        1, 'system_backup_downloaded', 'system_backup', expect.any(String), expect.anything()
+      );
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+
   it('DELETE /system/backup/delete rejects invalid dirname', async () => {
     const res = await request(app).delete('/system/backup/delete/not-a-date');
     expect(res.status).toBe(400);

src/server/routes/backupRoutes.ts

@@ -1,4 +1,5 @@
 import { Router, Request, Response } from 'express';
+import { promises as fsp } from 'fs';
 import { requirePermission } from '../auth/authMiddleware.js';
 import databaseService from '../../services/database.js';
 import { backupFileService } from '../services/backupFileService.js';
@@ -198,10 +199,11 @@ systemBackupRouter.get('/download/:dirname', requirePermission('configuration',
     const { TarArchive } = (await import('archiver')) as unknown as {
       TarArchive: new (opts: import('archiver').ArchiverOptions) => import('archiver').Archiver;
     };
-    const fs = await import('fs');
-
-    // Check if backup exists
-    if (!fs.existsSync(backupPath)) {
+    // Check the backup exists — async so we don't block the event loop with
+    // synchronous fs in the request path (#3524).
+    try {
+      await fsp.access(backupPath);
+    } catch {
       return res.status(404).json({ error: 'Backup not found' });
     }
 
@@ -216,7 +218,14 @@ systemBackupRouter.get('/download/:dirname', requirePermission('configuration',
 
     archive.on('error', err => {
       logger.error('❌ Error creating archive:', err);
-      res.status(500).json({ error: 'Failed to create archive' });
+      // The archive can error mid-stream — after archive.pipe(res) has already
+      // sent headers — so guard against "Cannot set headers after they are
+      // sent" (#3524). If we're already streaming, just terminate the response.
+      if (!res.headersSent) {
+        res.status(500).json({ error: 'Failed to create archive' });
+      } else {
+        res.end();
+      }
     });
 
     // Audit log before streaming
@@ -235,10 +244,15 @@ systemBackupRouter.get('/download/:dirname', requirePermission('configuration',
     logger.info(`📥 System backup downloaded: ${dirname}`);
   } catch (error) {
     logger.error('❌ Error downloading system backup:', error);
-    res.status(500).json({
-      error: 'Failed to download system backup',
-      details: error instanceof Error ? error.message : 'Unknown error',
-    });
+    // finalize() can throw after streaming began (headers sent) — same guard.
+    if (!res.headersSent) {
+      res.status(500).json({
+        error: 'Failed to download system backup',
+        details: error instanceof Error ? error.message : 'Unknown error',
+      });
+    } else {
+      res.end();
+    }
   }
 });