test(files): upload filters fail closed on an excluded or unresolvable type

server/src/nest/files/files.controller.ts

@@ -56,14 +56,21 @@ export function filesUploadFileFilter(allowedTypes: AllowedFileTypesService): Op
     // allowed-extension list is now an async read. The decision runs in a
     // detached async function that always answers through `cb` — multer holds
     // the file stream until it does (make-middleware.js says as much). A
-    // rejection refuses the file, so the filter still fails closed.
+    // rejection refuses the file, so the filter still fails closed. Only the
+    // await is wrapped in try/catch, so a throw from cb() itself is never
+    // re-routed into a second cb() call.
     void (async () => {
-      const allowed = (await allowedTypes.get()).split(',').map((e) => e.trim().toLowerCase());
+      let allowed: string[];
+      try {
+        allowed = (await allowedTypes.get()).split(',').map((e) => e.trim().toLowerCase());
+      } catch {
+        return reject();
+      }
       const fileExt = ext.replace('.', '');
       // Video is accepted as media regardless of the admin doc-types allowlist (#823).
       if (allowed.includes(fileExt) || isVideoExtension(fileExt) || (allowed.includes('*') && !BLOCKED_EXTENSIONS.includes(ext))) return cb(null, true);
       reject();
-    })().catch(() => reject());
+    })();
   };
 }
 

server/src/nest/journey/journey.controller.ts

@@ -78,23 +78,27 @@ export function journeyImageFileFilter(allowedTypes: AllowedFileTypesService): O
       return cb(err);
     }
     const ext = path.extname(file.originalname).toLowerCase().replace('.', '');
-    // R1.5: multer's fileFilter is a callback API that cannot await, and the
-    // allowed-extension list is now an async read. The decision runs in a
-    // detached async function that always answers through `cb`; a rejection
-    // refuses the file, so the filter still fails closed.
-    void (async () => {
-      const allowed = (await allowedTypes.get()).split(',').map((e) => e.trim().toLowerCase());
-      if (!allowed.includes('*') && !allowed.includes(ext)) {
-        const err: Error & { statusCode?: number } = new Error(`File type .${ext} is not allowed`);
-        err.statusCode = 400;
-        return cb(err);
-      }
-      cb(null, true);
-    })().catch(() => {
+    const reject = () => {
       const err: Error & { statusCode?: number } = new Error(`File type .${ext} is not allowed`);
       err.statusCode = 400;
       cb(err);
-    });
+    };
+    // R1.5: multer's fileFilter is a callback API that cannot await, and the
+    // allowed-extension list is now an async read. The decision runs in a
+    // detached async function that always answers through `cb`; a rejection
+    // refuses the file, so the filter still fails closed. Only the await is
+    // wrapped in try/catch, so a throw from cb() itself is never re-routed
+    // into a second cb() call.
+    void (async () => {
+      let allowed: string[];
+      try {
+        allowed = (await allowedTypes.get()).split(',').map((e) => e.trim().toLowerCase());
+      } catch {
+        return reject();
+      }
+      if (!allowed.includes('*') && !allowed.includes(ext)) return reject();
+      cb(null, true);
+    })();
   };
 }
 

server/tests/unit/nest/files.service.test.ts

@@ -343,7 +343,7 @@ describe('updateFile', () => {
     const { user, trip } = seedTrip();
     const place = createPlace(testDb, trip.id);
     const file = await makeFile(trip.id, user.id, {}, { description: 'keep me', place_id: String(place.id) });
-    const current = await svc.getFileById(file.id, trip.id)!;
+    const current = (await svc.getFileById(file.id, trip.id))!;
     const updated = await svc.updateFile(file.id, current, {}) as Record<string, unknown>;
     expect(updated.description).toBe('keep me');
     expect(updated.place_id).toBe(place.id);
@@ -354,7 +354,7 @@ describe('updateFile', () => {
     const place = createPlace(testDb, trip.id);
     const reservation = createReservation(testDb, trip.id);
     const file = await makeFile(trip.id, user.id, {}, { description: 'old', place_id: String(place.id), reservation_id: String(reservation.id) });
-    const current = await svc.getFileById(file.id, trip.id)!;
+    const current = (await svc.getFileById(file.id, trip.id))!;
     const updated = await svc.updateFile(file.id, current, { description: '', place_id: '', reservation_id: null }) as Record<string, unknown>;
     expect(updated.description).toBeNull(); // '' → NULL on update too (post-migration fix: symmetric with createFile)
     expect(updated.place_id).toBeNull();

server/tests/unit/nest/files/upload-filters.test.ts

@@ -0,0 +1,120 @@
+import { describe, it, expect, vi } from 'vitest';
+import type { Request } from 'express';
+import { filesUploadFileFilter } from '../../../../src/nest/files/files.controller';
+import { journeyImageFileFilter } from '../../../../src/nest/journey/journey.controller';
+import type { AllowedFileTypesService } from '../../../../src/nest/files/allowed-file-types.service';
+
+/**
+ * Unit coverage for the two upload fileFilters that read the operator's
+ * allowed-extension list asynchronously (files.controller.ts,
+ * journey.controller.ts). Both filters run a synchronous pre-check first (a
+ * blocked extension / non-image mimetype), then a detached async IIFE that
+ * always answers through `cb` exactly once — even when the allow-list lookup
+ * itself rejects (fail closed, never `cb(null, true)`).
+ */
+
+const req = {} as Request;
+
+function fakeFile(overrides: Partial<Express.Multer.File> = {}): Express.Multer.File {
+  return {
+    fieldname: 'file',
+    originalname: 'photo.jpg',
+    encoding: '7bit',
+    mimetype: 'image/jpeg',
+    size: 100,
+    stream: undefined as never,
+    destination: '',
+    filename: '',
+    path: '',
+    buffer: undefined as never,
+    ...overrides,
+  };
+}
+
+/** A stub whose get() can either resolve to a comma-separated list or reject. */
+function allowedTypesStub(result: string | Error): AllowedFileTypesService {
+  return {
+    get: vi.fn(() => (result instanceof Error ? Promise.reject(result) : Promise.resolve(result))),
+  } as unknown as AllowedFileTypesService;
+}
+
+/** Wait a couple of microtask turns so the filter's detached async IIFE settles. */
+async function flush(): Promise<void> {
+  await Promise.resolve();
+  await Promise.resolve();
+  await Promise.resolve();
+}
+
+describe('filesUploadFileFilter', () => {
+  it('UPLOAD-FILTER-001: a blocked extension is rejected synchronously, before the allow-list is read', async () => {
+    const allowedTypes = allowedTypesStub('jpg,png');
+    const filter = filesUploadFileFilter(allowedTypes)!;
+    const cb = vi.fn();
+    filter(req, fakeFile({ originalname: 'malware.exe', mimetype: 'application/octet-stream' }), cb);
+    await flush();
+    expect(cb).toHaveBeenCalledTimes(1);
+    const err = cb.mock.calls[0][0] as (Error & { statusCode?: number }) | null;
+    expect(err).toBeInstanceOf(Error);
+    expect(err?.statusCode).toBe(400);
+    expect(allowedTypes.get).not.toHaveBeenCalled();
+  });
+
+  it('UPLOAD-FILTER-002: an unresolvable allow-list rejects the file, never cb(null, true)', async () => {
+    const allowedTypes = allowedTypesStub(new Error('db down'));
+    const filter = filesUploadFileFilter(allowedTypes)!;
+    const cb = vi.fn();
+    filter(req, fakeFile({ originalname: 'photo.jpg', mimetype: 'image/jpeg' }), cb);
+    await flush();
+    expect(cb).toHaveBeenCalledTimes(1);
+    const [err, accept] = cb.mock.calls[0] as [(Error & { statusCode?: number }) | null, boolean | undefined];
+    expect(err).toBeInstanceOf(Error);
+    expect(accept).toBeUndefined();
+  });
+
+  it('UPLOAD-FILTER-003: an allowed extension resolves cb(null, true) exactly once', async () => {
+    const allowedTypes = allowedTypesStub('jpg,png');
+    const filter = filesUploadFileFilter(allowedTypes)!;
+    const cb = vi.fn();
+    filter(req, fakeFile({ originalname: 'photo.jpg', mimetype: 'image/jpeg' }), cb);
+    await flush();
+    expect(cb).toHaveBeenCalledTimes(1);
+    expect(cb).toHaveBeenCalledWith(null, true);
+  });
+});
+
+describe('journeyImageFileFilter', () => {
+  it('UPLOAD-FILTER-004: a non-image mimetype is rejected synchronously, before the allow-list is read', async () => {
+    const allowedTypes = allowedTypesStub('jpg,png');
+    const filter = journeyImageFileFilter(allowedTypes)!;
+    const cb = vi.fn();
+    filter(req, fakeFile({ originalname: 'notes.pdf', mimetype: 'application/pdf' }), cb);
+    await flush();
+    expect(cb).toHaveBeenCalledTimes(1);
+    const err = cb.mock.calls[0][0] as (Error & { statusCode?: number }) | null;
+    expect(err).toBeInstanceOf(Error);
+    expect(err?.statusCode).toBe(400);
+    expect(allowedTypes.get).not.toHaveBeenCalled();
+  });
+
+  it('UPLOAD-FILTER-005: an unresolvable allow-list rejects the file, never cb(null, true)', async () => {
+    const allowedTypes = allowedTypesStub(new Error('db down'));
+    const filter = journeyImageFileFilter(allowedTypes)!;
+    const cb = vi.fn();
+    filter(req, fakeFile({ originalname: 'photo.jpg', mimetype: 'image/jpeg' }), cb);
+    await flush();
+    expect(cb).toHaveBeenCalledTimes(1);
+    const [err, accept] = cb.mock.calls[0] as [(Error & { statusCode?: number }) | null, boolean | undefined];
+    expect(err).toBeInstanceOf(Error);
+    expect(accept).toBeUndefined();
+  });
+
+  it('UPLOAD-FILTER-006: an allowed image extension resolves cb(null, true) exactly once', async () => {
+    const allowedTypes = allowedTypesStub('jpg,png');
+    const filter = journeyImageFileFilter(allowedTypes)!;
+    const cb = vi.fn();
+    filter(req, fakeFile({ originalname: 'photo.jpg', mimetype: 'image/jpeg' }), cb);
+    await flush();
+    expect(cb).toHaveBeenCalledTimes(1);
+    expect(cb).toHaveBeenCalledWith(null, true);
+  });
+});