feat(uploads): headersSent guard on stream errors + text/html MIME for GridFS (#3744)

Add res.headersSent check in get() and getSharp() stream error handlers so
a GridFS failure mid-transfer (after headers are already flushed) destroys
the socket via res.destroy(err) + logger.error instead of attempting a
second status+body write that throws ERR_HTTP_HEADERS_SENT. Pre-header
errors still return 422 as before.

Add 'text/html': 'html' to MIME_TO_EXT so downstream features storing
rendered HTML snapshots in GridFS get correct .html filenames without
per-deployment config overrides. text/html is intentionally absent from
SAFE_MIME, so the Content-Type allowlist + Content-Disposition: attachment
XSS defenses remain intact.

Port stream-error tests for the headersSent guard and add a MIME map test
for text/html. Full unit suite: 1622/1622 pass.

Adopted from trawl prod hardening; every devkit downstream gets it via
/update-stack.

Author: PierreBrisorgueil

modules/uploads/controllers/uploads.controller.js

@@ -6,6 +6,7 @@ import _ from 'lodash';
 
 import config from '../../../config/index.js';
 import errors from '../../../lib/helpers/errors.js';
+import logger from '../../../lib/services/logger.js';
 import responses from '../../../lib/helpers/responses.js';
 import UploadsService from '../services/uploads.service.js';
 
@@ -40,7 +41,19 @@ const get = async (req, res) => {
     const stream = await UploadsService.getStream({ _id: req.upload._id });
     if (!stream) responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
     stream.on('error', (err) => {
-      responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      // Guard against ERR_HTTP_HEADERS_SENT when GridFS fails mid-transfer:
+      // `res.set('Content-Type', ...)` below flushes response headers to the
+      // client before the stream starts. If the GridFS read then errors, a
+      // second status+body write would throw ERR_HTTP_HEADERS_SENT and crash
+      // the worker process. Pre-header errors still reach the client as 422;
+      // post-header errors destroy the socket so Express surfaces them via
+      // its default error handler instead of double-sending.
+      if (!res.headersSent) {
+        responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      } else {
+        logger.error('uploads.get - stream error after headers sent', err);
+        res.destroy(err);
+      }
     });
     const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
     const norm = normalizeMime(raw);
@@ -65,7 +78,17 @@ const getSharp = async (req, res) => {
     const stream = await UploadsService.getStream({ _id: req.upload._id });
     if (!stream) responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
     stream.on('error', (err) => {
-      responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      // Same headersSent guard as `get` above. A sharp pipeline error after
+      // `res.set('Content-Type', ...)` has flushed the response head cannot
+      // write a new status or body — attempting to do so throws
+      // ERR_HTTP_HEADERS_SENT. Destroy the socket instead so Express surfaces
+      // the error via its default handler without double-sending.
+      if (!res.headersSent) {
+        responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      } else {
+        logger.error('uploads.getSharp - stream error after headers sent', err);
+        res.destroy(err);
+      }
     });
     const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
     const norm = normalizeMime(raw);

modules/uploads/services/uploads.service.js

@@ -15,6 +15,9 @@ const MIME_TO_EXT = {
   'image/png': 'png',
   'image/gif': 'gif',
   'application/pdf': 'pdf',
+  // Allows downstream features that store rendered HTML snapshots in GridFS
+  // alongside binary uploads (e.g. error pages, scrap previews).
+  'text/html': 'html',
 };
 
 /**

modules/uploads/tests/uploads.controller.contenttype.unit.tests.js

@@ -35,6 +35,8 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
     };
   };
 
+  let mockLogger;
+
   beforeEach(async () => {
     jest.resetModules();
 
@@ -44,6 +46,8 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       remove: jest.fn(),
     };
 
+    mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn() };
+
     jest.unstable_mockModule('../services/uploads.service.js', () => ({
       default: mockUploadsService,
     }));
@@ -76,6 +80,10 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       },
     }));
 
+    jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
+      default: mockLogger,
+    }));
+
     const mod = await import('../controllers/uploads.controller.js');
     UploadsController = mod.default;
   });
@@ -360,4 +368,144 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       expect(res.set).toHaveBeenCalledWith('Content-Type', 'image/jpeg');
     });
   });
+
+  // ── get — headersSent guard ───────────────────────────────────────────────
+
+  describe('get — headersSent guard on stream error', () => {
+    /**
+     * Install a fake stream that captures the `error` listener so tests can
+     * fire it manually — simulating either a pre-flush or mid-flush GridFS
+     * failure depending on `res.headersSent`.
+     */
+    const installStream = () => {
+      const listeners = {};
+      const stream = {
+        pipe: jest.fn().mockReturnThis(),
+        on: jest.fn((event, handler) => {
+          listeners[event] = handler;
+          return stream;
+        }),
+      };
+      mockUploadsService.getStream.mockResolvedValueOnce(stream);
+      return { stream, listeners };
+    };
+
+    const buildRes = () => {
+      const res = {};
+      res.status = jest.fn().mockReturnValue(res);
+      res.json = jest.fn().mockReturnValue(res);
+      res.set = jest.fn();
+      res.destroy = jest.fn();
+      res.headersSent = false;
+      return res;
+    };
+
+    const req = {
+      upload: {
+        _id: 'u1',
+        contentType: 'image/jpeg',
+        length: 1234,
+        metadata: { contentType: 'image/jpeg' },
+      },
+    };
+
+    test('responds via responses.error when stream errors before headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.get(req, res);
+
+      res.headersSent = false;
+      listeners.error(new Error('gridfs chunk missing'));
+
+      expect(responses.error).toHaveBeenCalled();
+      expect(res.destroy).not.toHaveBeenCalled();
+    });
+
+    test('calls res.destroy(err) and logger.error when stream errors after headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.get(req, res);
+
+      res.headersSent = true;
+      const err = new Error('gridfs mid-stream abort');
+      listeners.error(err);
+
+      // Must NOT attempt a new response body on an already-flushed response.
+      expect(responses.error).not.toHaveBeenCalled();
+      expect(res.destroy).toHaveBeenCalledWith(err);
+      // Error must reach the logger so mid-stream GridFS failures are visible.
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.get'), err);
+    });
+  });
+
+  // ── getSharp — headersSent guard ──────────────────────────────────────────
+
+  describe('getSharp — headersSent guard on stream error', () => {
+    const installStream = () => {
+      const listeners = {};
+      const stream = {
+        pipe: jest.fn().mockReturnThis(),
+        on: jest.fn((event, handler) => {
+          listeners[event] = handler;
+          return stream;
+        }),
+      };
+      mockUploadsService.getStream.mockResolvedValueOnce(stream);
+      return { stream, listeners };
+    };
+
+    const buildRes = () => {
+      const res = {};
+      res.status = jest.fn().mockReturnValue(res);
+      res.json = jest.fn().mockReturnValue(res);
+      res.set = jest.fn();
+      res.destroy = jest.fn();
+      res.headersSent = false;
+      return res;
+    };
+
+    const req = {
+      upload: {
+        _id: 'u1',
+        contentType: 'image/jpeg',
+        metadata: { contentType: 'image/jpeg' },
+      },
+      sharpSize: null,
+      sharpOption: null,
+    };
+
+    test('responds via responses.error when stream errors before headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.getSharp(req, res);
+
+      res.headersSent = false;
+      listeners.error(new Error('early gridfs failure'));
+
+      expect(responses.error).toHaveBeenCalled();
+      expect(res.destroy).not.toHaveBeenCalled();
+    });
+
+    test('calls res.destroy(err) and logger.error when stream errors after headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.getSharp(req, res);
+
+      res.headersSent = true;
+      const err = new Error('sharp pipeline mid-stream abort');
+      listeners.error(err);
+
+      expect(responses.error).not.toHaveBeenCalled();
+      expect(res.destroy).toHaveBeenCalledWith(err);
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.getSharp'), err);
+    });
+  });
 });

modules/uploads/tests/uploads.createFromBuffer.unit.tests.js

@@ -175,6 +175,24 @@ describe('Uploads createFromBuffer unit tests:', () => {
     expect(filename).toMatch(/^[a-f0-9]{64}\.html$/);
   });
 
+  test('should resolve text/html to .html extension via built-in MIME_TO_EXT (no config.uploads.mimeTypes needed)', async () => {
+    // text/html is in the built-in MIME_TO_EXT map so downstream features that
+    // store rendered HTML snapshots in GridFS get a correct .html filename
+    // without requiring per-deployment mimeTypes config.
+    mockConfig.uploads.snapshot = {
+      kind: 'snapshot',
+      formats: ['text/html'],
+      limits: { fileSize: 1 * 1024 * 1024 },
+    };
+    mockGridfs.createFromBuffer.mockResolvedValue({ ...fakeFile, contentType: 'text/html' });
+
+    const buffer = Buffer.alloc(512);
+    await UploadsService.createFromBuffer(buffer, 'text/html', 'snapshot');
+
+    const [, filename] = mockGridfs.createFromBuffer.mock.calls[0];
+    expect(filename).toMatch(/^[a-f0-9]{64}\.html$/);
+  });
+
   test('should throw error when kind has no formats configured', async () => {
     // Adding 'broken' kind at runtime — service reads config dynamically via module reference
     mockConfig.uploads.broken = { kind: 'broken', limits: { fileSize: 1024 } };