refactor(backup): download through storage sendToResponse

server/src/nest/backup/backup.controller.ts

@@ -28,6 +28,7 @@ import { AutoBackupSettingsDto } from './backup.dto';
 import { getClientIp } from '../audit/client-ip';
 import { AuditService } from '../audit/audit.service';
 import { getUploadTmpDir, MAX_BACKUP_UPLOAD_SIZE } from './backup.impl';
+import { StorageNotFoundError } from '../storage/storage.types';
 import { ManagedForbidden, isManagedBlocked, MANAGED_FORBIDDEN_ERROR } from '../common/managed';
 import { RuntimeEnvService } from '../app-config/runtime-env.service';
 
@@ -91,7 +92,16 @@ export class BackupController {
     if (!(await this.backup.backupFileExists(filename))) {
       throw new HttpException({ error: 'Backup not found' }, 404);
     }
-    res.download(this.backup.backupFilePath(filename), filename);
+    try {
+      await this.backup.sendBackupToResponse(filename, res);
+    } catch (err) {
+      // The pre-check above owns the normal miss; this only covers a delete
+      // racing between the check and the send.
+      if (err instanceof StorageNotFoundError) {
+        throw new HttpException({ error: 'Backup not found' }, 404);
+      }
+      throw err;
+    }
   }
 
   @ManagedForbidden('a restore replaces database and uploads, and the operator owns the recovery point')

server/src/nest/backup/backup.impl.ts

@@ -10,6 +10,7 @@ import { invalidatePermissionsCache } from '../permissions/permissions-cache';
 import { pluginsCodeRoot, pluginsDataRoot } from '../plugins/paths';
 import { stageExtractedPluginTrees, applyStagedRestoreNow } from '../plugins/plugin-backup';
 import { snapshotAllPluginDataDbs } from '../plugins/host/plugin-data.service';
+import type { Response } from 'express';
 import type { StorageService } from '../storage/storage.service';
 
 // ---------------------------------------------------------------------------
@@ -89,6 +90,17 @@ export function backupFileExists(storage: StorageService, filename: string): Pro
   return storage.exists('backups', filename);
 }
 
+/**
+ * The codebase's only res.download becomes the storage equivalent: root-relative
+ * sendFile via sendToResponse, with res.download's attachment header rebuilt by
+ * hand (filenames are regex-gated ASCII — no encoding cases).
+ */
+export function sendBackupToResponse(storage: StorageService, filename: string, res: Response): Promise<void> {
+  return storage.sendToResponse('backups', filename, res, {
+    disposition: `attachment; filename="${filename}"`,
+  });
+}
+
 // ---------------------------------------------------------------------------
 // Rate limiter state (shared across requests)
 // ---------------------------------------------------------------------------

server/src/nest/backup/backup.service.ts

@@ -1,4 +1,5 @@
 import { Injectable } from '@nestjs/common';
+import type { Response } from 'express';
 import { StorageService } from '../storage/storage.service';
 import * as svc from './backup.impl';
 
@@ -24,6 +25,7 @@ export class BackupService {
   isValidBackupFilename(filename: string) { return svc.isValidBackupFilename(filename); }
   backupFilePath(filename: string) { return svc.backupFilePath(filename); }
   backupFileExists(filename: string) { return svc.backupFileExists(this.storage, filename); }
+  sendBackupToResponse(filename: string, res: Response) { return svc.sendBackupToResponse(this.storage, filename, res); }
   checkRateLimit(key: string, maxAttempts: number, windowMs: number) { return svc.checkRateLimit(key, maxAttempts, windowMs); }
 
   get rateWindow() { return svc.BACKUP_RATE_WINDOW; }

server/tests/integration/backup.test.ts

@@ -85,6 +85,7 @@ import { resetTestDb, resetRateLimits } from '../helpers/test-db';
 import { createAdmin, createUser } from '../helpers/factories';
 import { authCookie } from '../helpers/auth';
 import * as backupService from '../../src/nest/backup/backup.impl';
+import { DEFAULT_BACKUPS_ROOT } from '../../src/nest/storage/storage-paths';
 import fs from 'fs';
 import path from 'path';
 import os from 'os';
@@ -200,31 +201,36 @@ describe('Backup security', () => {
 // ---------------------------------------------------------------------------
 
 describe('Backup download', () => {
-  let tmpFile: string;
+  // Serving goes through the real StorageService now (sendBackupToResponse is
+  // NOT overridden in the impl mock), so the fixture must live in the real
+  // backups root. Only this one file is cleaned up — never the whole dir.
+  const downloadFixture = 'backup-2026-04-06T12-00-00.zip';
+  const downloadFixturePath = path.join(DEFAULT_BACKUPS_ROOT, downloadFixture);
 
   beforeEach(() => {
-    // Create a real temporary file that Express can stream back
-    tmpFile = path.join(os.tmpdir(), `test-backup-${Date.now()}.zip`);
-    fs.writeFileSync(tmpFile, 'fake zip content');
-    vi.mocked(backupService.backupFileExists).mockReturnValue(true);
-    vi.mocked(backupService.backupFilePath).mockReturnValue(tmpFile);
+    vi.mocked(backupService.backupFileExists).mockResolvedValue(true);
   });
 
   afterAll(() => {
-    try { fs.unlinkSync(tmpFile); } catch {}
+    try { fs.unlinkSync(downloadFixturePath); } catch {}
   });
 
   it('BACKUP-INT-001 — GET /backup/download/:filename returns 200 with content-disposition', async () => {
     const { user: admin } = createAdmin(testDb);
-    const filename = 'backup-2026-04-06T12-00-00.zip';
+    // (Re)write the fixture immediately before the request that serves it.
+    fs.mkdirSync(DEFAULT_BACKUPS_ROOT, { recursive: true });
+    fs.writeFileSync(downloadFixturePath, 'fake zip content');
 
     const res = await request(app)
-      .get(`/api/backup/download/${filename}`)
+      .get(`/api/backup/download/${downloadFixture}`)
       .set('Cookie', authCookie(admin.id));
 
     expect(res.status).toBe(200);
     expect(res.headers['content-disposition']).toMatch(/attachment/i);
-    expect(res.headers['content-disposition']).toContain(filename);
+    expect(res.headers['content-disposition']).toContain(downloadFixture);
+    // superagent has no parser for application/zip, so assert the byte count
+    // rather than the (unbuffered) body.
+    expect(res.headers['content-length']).toBe(String('fake zip content'.length));
   });
 
   it('BACKUP-INT-002 — GET /backup/download/:filename returns 400 for invalid filename', async () => {

server/tests/unit/nest/backup.controller.test.ts

@@ -18,10 +18,12 @@ vi.mock('../../../src/nest/backup/backup.impl', () => ({
   isValidBackupFilename: vi.fn().mockReturnValue(true),
   backupFilePath: vi.fn().mockReturnValue('/data/backups/svc.zip'),
   backupFileExists: vi.fn().mockReturnValue(true),
+  sendBackupToResponse: vi.fn().mockResolvedValue(undefined),
   checkRateLimit: vi.fn().mockReturnValue(true),
 }));
 
 import { BackupController } from '../../../src/nest/backup/backup.controller';
+import { StorageNotFoundError } from '../../../src/nest/storage/storage.types';
 import { BackupService as RealBackupService } from '../../../src/nest/backup/backup.service';
 import { AdminGuard } from '../../../src/nest/auth/admin.guard';
 import type { BackupService } from '../../../src/nest/backup/backup.service';
@@ -56,6 +58,7 @@ function svc(o: Partial<BackupService> = {}): BackupService {
     isValidBackupFilename: vi.fn().mockReturnValue(true),
     backupFilePath: vi.fn().mockReturnValue('/b/x.zip'),
     backupFileExists: vi.fn().mockReturnValue(true),
+    sendBackupToResponse: vi.fn().mockResolvedValue(undefined),
     checkRateLimit: vi.fn().mockReturnValue(true),
     rateWindow: 3600000,
     ...o,
@@ -106,12 +109,22 @@ describe('BackupController', () => {
     expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'backup.create', resource: 'b.zip' }));
   });
 
-  it('GET /download 400 invalid / 404 missing, else res.download', async () => {
-    const res = { download: vi.fn() } as unknown as Response;
+  it('GET /download 400 invalid / 404 missing, else serves through storage', async () => {
+    const res = {} as unknown as Response;
     expect(await thrownAsync(() => bc(svc({ isValidBackupFilename: vi.fn().mockReturnValue(false) })).download('x', res))).toEqual({ status: 400, body: { error: 'Invalid filename' } });
     expect(await thrownAsync(() => bc(svc({ backupFileExists: vi.fn().mockResolvedValue(false) })).download('x.zip', res))).toEqual({ status: 404, body: { error: 'Backup not found' } });
-    await bc(svc()).download('x.zip', res);
-    expect(res.download).toHaveBeenCalledWith('/b/x.zip', 'x.zip');
+    const sendBackupToResponse = vi.fn().mockResolvedValue(undefined);
+    await bc(svc({ sendBackupToResponse } as Partial<BackupService>)).download('x.zip', res);
+    expect(sendBackupToResponse).toHaveBeenCalledWith('x.zip', res);
+  });
+
+  it('GET /download maps a StorageNotFoundError raced past the pre-check to the same 404', async () => {
+    const res = {} as unknown as Response;
+    const sendBackupToResponse = vi.fn().mockRejectedValue(new StorageNotFoundError('x.zip'));
+    expect(await thrownAsync(() => bc(svc({ sendBackupToResponse } as Partial<BackupService>)).download('x.zip', res))).toEqual({ status: 404, body: { error: 'Backup not found' } });
+    // A non-miss failure is NOT swallowed into the 404 envelope.
+    const boom = vi.fn().mockRejectedValue(new Error('io'));
+    await expect(bc(svc({ sendBackupToResponse: boom } as Partial<BackupService>)).download('x.zip', res)).rejects.toThrow('io');
   });
 
   it('POST /restore maps the service status, else audits', async () => {
@@ -224,6 +237,10 @@ describe('BackupService (wrapper)', () => {
     expect(wrapper.backupFileExists('svc.zip')).toBe(true);
     expect(backupSvc.backupFileExists).toHaveBeenCalledWith(storage, 'svc.zip');
 
+    const fakeRes = {} as Response;
+    await wrapper.sendBackupToResponse('svc.zip', fakeRes);
+    expect(backupSvc.sendBackupToResponse).toHaveBeenCalledWith(storage, 'svc.zip', fakeRes);
+
     expect(wrapper.checkRateLimit('ip', 3, 1000)).toBe(true);
     expect(backupSvc.checkRateLimit).toHaveBeenCalledWith('ip', 3, 1000);
   });

server/tests/unit/nest/backup.impl.test.ts

@@ -77,6 +77,7 @@ import {
   backupFilePath,
   backupFileExists,
   listBackups,
+  sendBackupToResponse,
 } from '../../../src/nest/backup/backup.impl';
 import type { StorageService } from '../../../src/nest/storage/storage.service';
 
@@ -933,6 +934,38 @@ describe('BACKUP-040 backupFileExists', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// sendBackupToResponse
+// ---------------------------------------------------------------------------
+
+describe('BACKUP-062 sendBackupToResponse', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('BACKUP-062a — serves through storage with the res.download attachment header', async () => {
+    const storage = stubStorage();
+    const res = { setHeader: vi.fn() } as unknown as import('express').Response;
+
+    await sendBackupToResponse(storage, 'backup-2026-01-01T00-00-00.zip', res);
+
+    expect(storage.sendToResponse).toHaveBeenCalledOnce();
+    expect(storage.sendToResponse).toHaveBeenCalledWith(
+      'backups',
+      'backup-2026-01-01T00-00-00.zip',
+      res,
+      { disposition: 'attachment; filename="backup-2026-01-01T00-00-00.zip"' },
+    );
+  });
+
+  it('BACKUP-062b — propagates a storage failure (the controller owns the miss contract)', async () => {
+    const storage = stubStorage({ sendToResponse: vi.fn(async () => { throw new Error('missing'); }) });
+    const res = {} as import('express').Response;
+
+    await expect(sendBackupToResponse(storage, 'backup-x.zip', res)).rejects.toThrow('missing');
+  });
+});
+
 // ---------------------------------------------------------------------------
 // listBackups
 // ---------------------------------------------------------------------------