Refactor and centralize file mimeType integrity check and sanitization (#20889)

# Introduction closes https://github.com/twentyhq/private-issues/issues/484 This PR refactors the writeFile API to never expect to be passed a mimetype, its extract is done programmatically low level so any callers will pass through Same for the file sanitization ## IANA override Disclaimer for consistency we existing behavior we wanted to always have `application/typescript` - should we rather consider fallbacking to octect-steam instead ? - Any pulbic assets that has .ts will now also fallback to `application/typescript` instead of the official IANA ## Integration Added coverage
2026-05-26 17:10:52 +02:00
parent c74337978b
commit b8de469f37
30 changed files with 958 additions and 142 deletions
@@ -0,0 +1,19 @@
+// Extensions where Twenty intentionally deviates from the IANA-standard mime
+// mapping returned by mrmime. Twenty's storage layer persists these values
+// instead of mrmime's output (or instead of throwing in the case of `.ts`).
+//
+// Why each entry exists:
+//   - ts/tsx:    IANA registers `.ts` as `video/mp2t` (MPEG-2 Transport Stream),
+//                which predates TypeScript. `.tsx` is unregistered. Twenty uses
+//                the developer-tooling convention `application/typescript`.
+//
+// Only add an entry when (1) mrmime returns the wrong mime (collision) or no
+// mime AND (2) Twenty actually writes that extension via FileStorageService AND
+// (3) there is a clear developer-tooling convention for the right mime. For
+// everything else — media, archives, documents, `.js`, `.mjs`, `.cjs`, etc. —
+// trust mrmime. This policy is reactive: extend it the day we observe a real
+// wrong-mime persisted by an exercised code path, not in anticipation.
+export const TWENTY_MIME_POLICY: Record<string, string> = {
+  ts: 'application/typescript',
+  tsx: 'application/typescript',
+};
@@ -8,8 +8,8 @@ import { ApplicationService } from 'src/engine/core-modules/application/applicat
 import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
 import { FileWithSignedUrlDTO } from 'src/engine/core-modules/file/dtos/file-with-sign-url.dto';
 import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service';
-import { extractFileInfo } from 'src/engine/core-modules/file/utils/extract-file-info.utils';
-import { sanitizeFile } from 'src/engine/core-modules/file/utils/sanitize-file.utils';
+import { extractFileInfoOrThrow } from 'src/engine/core-modules/file/utils/extract-file-info-or-throw.utils';
+
@Injectable()
 export class FileAiChatService {
  constructor(
@@ -27,13 +27,11 @@ export class FileAiChatService {
    filename: string;
    workspaceId: string;
  }): Promise<FileWithSignedUrlDTO> {
-    const { mimeType, ext } = await extractFileInfo({
+    const { ext } = await extractFileInfoOrThrow({
      file,
      filename,
    });

-    const sanitizedFile = sanitizeFile({ file, ext, mimeType });
-
    const fileId = v4();
    const name = `${fileId}${isNonEmptyString(ext) ? `.${ext}` : ''}`;

@@ -45,9 +43,8 @@ export class FileAiChatService {
      );

    const savedFile = await this.fileStorageService.writeFile({
-      sourceFile: sanitizedFile,
+      sourceFile: file,
      resourcePath: name,
-      mimeType,
      fileFolder: FileFolder.AgentChat,
      applicationUniversalIdentifier:
        workspaceCustomFlatApplication.universalIdentifier,
@@ -19,9 +19,8 @@ import { FileStorageService } from 'src/engine/core-modules/file-storage/file-st
 import { FileWithSignedUrlDTO } from 'src/engine/core-modules/file/dtos/file-with-sign-url.dto';
 import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
 import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service';
-import { extractFileInfo } from 'src/engine/core-modules/file/utils/extract-file-info.utils';
+import { extractFileInfoOrThrow } from 'src/engine/core-modules/file/utils/extract-file-info-or-throw.utils';
 import { removeFileFolderFromFileEntityPath } from 'src/engine/core-modules/file/utils/remove-file-folder-from-file-entity-path.utils';
-import { sanitizeFile } from 'src/engine/core-modules/file/utils/sanitize-file.utils';
 import { SecureHttpClientService } from 'src/engine/core-modules/secure-http-client/secure-http-client.service';
 import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
 import { getImageBufferFromUrl } from 'src/utils/image';
@@ -72,8 +71,7 @@ export class FileCorePictureService {
    applicationUniversalIdentifier?: string;
    queryRunner?: QueryRunner;
  }): Promise<FileEntity> {
-    const { mimeType, ext } = await extractFileInfo({ file, filename });
-    const sanitizedFile = sanitizeFile({ file, ext, mimeType });
+    const { ext } = await extractFileInfoOrThrow({ file, filename });

    const fileId = v4();
    const finalName = `${fileId}${isNonEmptyString(ext) ? `.${ext}` : ''}`;
@@ -83,9 +81,8 @@ export class FileCorePictureService {
      (await this.findCustomApplicationUniversalIdentifier(workspaceId));

    const savedFile = await this.fileStorageService.writeFile({
-      sourceFile: sanitizedFile,
+      sourceFile: file,
      resourcePath: finalName,
-      mimeType,
      fileFolder: FileFolder.CorePicture,
      applicationUniversalIdentifier: universalIdentifier,
      workspaceId,
@@ -8,8 +8,7 @@ import { ApplicationService } from 'src/engine/core-modules/application/applicat
 import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
 import { FileWithSignedUrlDTO } from 'src/engine/core-modules/file/dtos/file-with-sign-url.dto';
 import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service';
-import { extractFileInfo } from 'src/engine/core-modules/file/utils/extract-file-info.utils';
-import { sanitizeFile } from 'src/engine/core-modules/file/utils/sanitize-file.utils';
+import { extractFileInfoOrThrow } from 'src/engine/core-modules/file/utils/extract-file-info-or-throw.utils';

@Injectable()
 export class FileEmailAttachmentService {
@@ -30,13 +29,11 @@ export class FileEmailAttachmentService {
    filename: string;
    workspaceId: string;
  }): Promise<FileWithSignedUrlDTO> {
-    const { mimeType, ext } = await extractFileInfo({
+    const { ext } = await extractFileInfoOrThrow({
      file,
      filename,
    });

-    const sanitizedFile = sanitizeFile({ file, ext, mimeType });
-
    const fileId = v4();
    const name = `${fileId}${isNonEmptyString(ext) ? `.${ext}` : ''}`;

@@ -48,9 +45,8 @@ export class FileEmailAttachmentService {
      );

    const savedFile = await this.fileStorageService.writeFile({
-      sourceFile: sanitizedFile,
+      sourceFile: file,
      resourcePath: name,
-      mimeType,
      fileFolder: FileFolder.EmailAttachment,
      applicationUniversalIdentifier:
        workspaceCustomFlatApplication.universalIdentifier,
@@ -8,8 +8,7 @@ import { ApplicationService } from 'src/engine/core-modules/application/applicat
 import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
 import { FileWithSignedUrlDTO } from 'src/engine/core-modules/file/dtos/file-with-sign-url.dto';
 import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service';
-import { extractFileInfo } from 'src/engine/core-modules/file/utils/extract-file-info.utils';
-import { sanitizeFile } from 'src/engine/core-modules/file/utils/sanitize-file.utils';
+import { extractFileInfoOrThrow } from 'src/engine/core-modules/file/utils/extract-file-info-or-throw.utils';

@Injectable()
 export class FileWorkflowService {
@@ -28,13 +27,11 @@ export class FileWorkflowService {
    filename: string;
    workspaceId: string;
  }): Promise<FileWithSignedUrlDTO> {
-    const { mimeType, ext } = await extractFileInfo({
+    const { ext } = await extractFileInfoOrThrow({
      file,
      filename,
    });

-    const sanitizedFile = sanitizeFile({ file, ext, mimeType });
-
    const fileId = v4();
    const name = `${fileId}${isNonEmptyString(ext) ? `.${ext}` : ''}`;

@@ -46,9 +43,8 @@ export class FileWorkflowService {
      );

    const savedFile = await this.fileStorageService.writeFile({
-      sourceFile: sanitizedFile,
+      sourceFile: file,
      resourcePath: name,
-      mimeType,
      fileFolder: FileFolder.Workflow,
      applicationUniversalIdentifier:
        workspaceCustomFlatApplication.universalIdentifier,
@@ -15,8 +15,7 @@ import {
  FilesFieldException,
  FilesFieldExceptionCode,
 } from 'src/engine/core-modules/file/files-field/files-field.exception';
-import { extractFileInfo } from 'src/engine/core-modules/file/utils/extract-file-info.utils';
-import { sanitizeFile } from 'src/engine/core-modules/file/utils/sanitize-file.utils';
+import { extractFileInfoOrThrow } from 'src/engine/core-modules/file/utils/extract-file-info-or-throw.utils';
 import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';

@Injectable()
@@ -53,13 +52,11 @@ export class FilesFieldService {
      );
    }

-    const { mimeType, ext } = await extractFileInfo({
+    const { ext } = await extractFileInfoOrThrow({
      file,
      filename,
    });

-    const sanitizedFile = sanitizeFile({ file, ext, mimeType });
-
    const fileId = v4();
    const name = `${fileId}${isNonEmptyString(ext) ? `.${ext}` : ''}`;

@@ -82,9 +79,8 @@ export class FilesFieldService {
    });

    const savedFile = await this.fileStorageService.writeFile({
-      sourceFile: sanitizedFile,
+      sourceFile: file,
      resourcePath: `${fieldMetadata.universalIdentifier}/${name}`,
-      mimeType,
      fileFolder: FileFolder.FilesField,
      applicationUniversalIdentifier: application.universalIdentifier,
      workspaceId,
@@ -1,4 +1,4 @@
-import { extractFileInfo } from '../extract-file-info.utils';
+import { extractFileInfoOrThrow } from '../extract-file-info-or-throw.utils';

 const pngBuffer = Buffer.from([
  0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49,
@@ -8,7 +8,7 @@ const pdfBuffer = Buffer.from('%PDF-1.4\n', 'utf-8');
 const textBuffer = Buffer.from('Hello, world!', 'utf-8');
 const zipBuffer = Buffer.from([0x50, 0x4b, 0x03, 0x04]);

-describe('extractFileInfo', () => {
+describe('extractFileInfoOrThrow', () => {
  it.each([
    {
      name: 'PNG',
@@ -41,7 +41,7 @@ describe('extractFileInfo', () => {
  ])(
    'should detect $name from buffer magic numbers',
    async ({ buffer, filename, ext, mime }) => {
-      const result = await extractFileInfo({ file: buffer, filename });
+      const result = await extractFileInfoOrThrow({ file: buffer, filename });

      expect(result).toEqual({ mimeType: mime, ext });
    },
@@ -78,7 +78,10 @@ describe('extractFileInfo', () => {
  ])(
    'should fall back to extension for $name files',
    async ({ filename, ext, mime }) => {
-      const result = await extractFileInfo({ file: textBuffer, filename });
+      const result = await extractFileInfoOrThrow({
+        file: textBuffer,
+        filename,
+      });

      expect(result).toEqual({ mimeType: mime, ext });
    },
@@ -95,10 +98,27 @@ describe('extractFileInfo', () => {
    'should throw when $ext extension does not match buffer content',
    async ({ filename, ext, expectedMime }) => {
      await expect(
-        extractFileInfo({ file: textBuffer, filename }),
+        extractFileInfoOrThrow({ file: textBuffer, filename }),
      ).rejects.toThrow(
        `File content does not match its extension. The file has extension '${ext}' (expected mime type: ${expectedMime}), but the file content could not be detected as this type. The file may be corrupted, have the wrong extension, or be a security risk.`,
      );
    },
  );
+
+  describe('TWENTY_MIME_POLICY (Twenty deviates from IANA)', () => {
+    it.each([
+      { filename: 'src/index.ts', expectedMime: 'application/typescript' },
+      { filename: 'src/index.tsx', expectedMime: 'application/typescript' },
+    ])(
+      'should return $expectedMime for $filename without throwing on IANA collision',
+      async ({ filename, expectedMime }) => {
+        const result = await extractFileInfoOrThrow({
+          file: textBuffer,
+          filename,
+        });
+
+        expect(result.mimeType).toBe(expectedMime);
+      },
+    );
+  });
 });
@@ -10,9 +10,14 @@ import {
 } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';

 import { detectPdf } from '@file-type/pdf';
+import { TWENTY_MIME_POLICY } from 'src/engine/core-modules/file/constants/twenty-mime-policy.constant';
 import { buildFileInfo } from 'src/engine/core-modules/file/utils/build-file-info.utils';

-export const extractFileInfo = async ({
+const fileTypeParser = new FileTypeParser({
+  customDetectors: [detectPdf],
+});
+
+export const extractFileInfoOrThrow = async ({
  file,
  filename,
 }: {
@@ -21,12 +26,8 @@ export const extractFileInfo = async ({
 }) => {
  const { ext: declaredExt } = buildFileInfo(filename);

-  const fileParser = new FileTypeParser({
-    customDetectors: [detectPdf],
-  });
-
  const { ext: detectedExt, mime: detectedMime } =
-    (await fileParser.fromBuffer(file)) ?? {};
+    (await fileTypeParser.fromBuffer(file)) ?? {};

  if (isDefined(detectedExt) && isDefined(detectedMime)) {
    return {
@@ -40,6 +41,22 @@ export const extractFileInfo = async ({
  let mimeType: string = 'application/octet-stream';

  if (isNonEmptyString(ext)) {
+    // Twenty policy wins over the ext-based fallback for the (small) set of
+    // extensions where mrmime's IANA mapping collides with a developer-tooling
+    // convention. This branch is only reached when file-type's magic-byte
+    // sniff returned nothing — when the bytes actually match (e.g. a real
+    // MPEG-TS video at foo.ts), we already returned above.
+    //
+    // For .ts/.tsx the policy is also load-bearing for correctness: without
+    // it, lookup('ts') → 'video/mp2t' is in file-type's supportedMimeTypes,
+    // so the check below would throw INVALID_EXTENSION on every TypeScript
+    // source upload.
+    const policyMime = TWENTY_MIME_POLICY[ext];
+
+    if (isDefined(policyMime)) {
+      return { mimeType: policyMime, ext };
+    }
+
    const mimeTypeFromExtension = lookup(ext);

    if (