From b8de469f3711fcc3e30cba901f01dd78f1f3c0fe Mon Sep 17 00:00:00 2001
From: Paul Rastoin <45004772+prastoin@users.noreply.github.com>
Date: Tue, 26 May 2026 17:10:52 +0200
Subject: [PATCH] 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
---
.../application-development.resolver.ts | 5 +-
.../application-install.service.ts | 11 +-
.../application-tarball.service.ts | 1 -
.../application/application.service.ts | 2 -
.../file-storage-exception-filter.spec.ts | 36 +++
.../__tests__/file-storage.service.spec.ts | 137 +++++++++++-
.../file-storage-exception-filter.ts | 29 +++
.../file-storage/file-storage.module.ts | 11 +-
.../file-storage/file-storage.service.ts | 43 ++--
...are-file-for-storage-or-throw.util.spec.ts | 207 ++++++++++++++++++
.../prepare-file-for-storage-or-throw.util.ts | 33 +++
.../constants/twenty-mime-policy.constant.ts | 19 ++
.../services/file-ai-chat.service.ts | 11 +-
.../services/file-core-picture.service.ts | 9 +-
.../services/file-email-attachment.service.ts | 10 +-
.../services/file-workflow.service.ts | 10 +-
.../services/files-field.service.ts | 10 +-
... extract-file-info-or-throw.utils.spec.ts} | 30 ++-
...ts => extract-file-info-or-throw.utils.ts} | 29 ++-
.../logic-function-resource.service.ts | 4 -
.../sdk-client-generation.service.ts | 1 -
.../code-interpreter-tool.ts | 1 -
.../data/services/dev-seeder-data.service.ts | 1 -
.../prefill-front-component.service.ts | 2 -
...ile-mime-mismatch.integration-spec.ts.snap | 37 ++++
...ts-controller-download.integration-spec.ts | 9 +-
...ion-file-mime-mismatch.integration-spec.ts | 136 ++++++++++++
...ts-controller-download.integration-spec.ts | 4 +-
...-file-svg-sanitization.integration-spec.ts | 126 +++++++++++
...pload-application-file.integration-spec.ts | 136 ++++++++----
30 files changed, 958 insertions(+), 142 deletions(-)
create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage-exception-filter.spec.ts
create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/file-storage-exception-filter.ts
create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/prepare-file-for-storage-or-throw.util.spec.ts
create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/prepare-file-for-storage-or-throw.util.ts
create mode 100644 packages/twenty-server/src/engine/core-modules/file/constants/twenty-mime-policy.constant.ts
rename packages/twenty-server/src/engine/core-modules/file/utils/__tests__/{extract-file-info.utils.spec.ts => extract-file-info-or-throw.utils.spec.ts} (73%)
rename packages/twenty-server/src/engine/core-modules/file/utils/{extract-file-info.utils.ts => extract-file-info-or-throw.utils.ts} (62%)
create mode 100644 packages/twenty-server/test/integration/metadata/suites/application/__snapshots__/failing-upload-application-file-mime-mismatch.integration-spec.ts.snap
create mode 100644 packages/twenty-server/test/integration/metadata/suites/application/failing-upload-application-file-mime-mismatch.integration-spec.ts
create mode 100644 packages/twenty-server/test/integration/metadata/suites/application/successful-upload-application-file-svg-sanitization.integration-spec.ts
diff --git a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts
index 6495b0ae34..8d4110a1e1 100644
--- a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts
+++ b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts
@@ -20,7 +20,6 @@ import { DevelopmentApplicationDTO } from 'src/engine/core-modules/application/a
import { GenerateApplicationTokenInput } from 'src/engine/core-modules/application/application-development/dtos/generate-application-token.input';
import { UploadApplicationFileInput } from 'src/engine/core-modules/application/application-development/dtos/upload-application-file.input';
import { WorkspaceMigrationDTO } from 'src/engine/core-modules/application/application-development/dtos/workspace-migration.dto';
-import { validateFilePath } from 'src/engine/core-modules/file-storage/utils/validate-file-path.util';
import { ApplicationExceptionFilter } from 'src/engine/core-modules/application/application-exception-filter';
import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service';
import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util';
@@ -35,6 +34,7 @@ import {
import { ApplicationService } from 'src/engine/core-modules/application/application.service';
import { ApplicationTokenService } from 'src/engine/core-modules/auth/token/services/application-token.service';
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
+import { validateFilePath } from 'src/engine/core-modules/file-storage/utils/validate-file-path.util';
import { FileDTO } from 'src/engine/core-modules/file/dtos/file.dto';
import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe';
import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service';
@@ -192,7 +192,7 @@ export class ApplicationDevelopmentResolver {
async uploadApplicationFile(
@AuthWorkspace() { id: workspaceId }: WorkspaceEntity,
@Args({ name: 'file', type: () => GraphQLUpload })
- { createReadStream, mimetype }: FileUpload,
+ { createReadStream }: FileUpload,
@Args()
{
applicationUniversalIdentifier,
@@ -250,7 +250,6 @@ export class ApplicationDevelopmentResolver {
return await this.fileStorageService.writeFile({
sourceFile: buffer,
- mimeType: mimetype,
fileFolder,
applicationUniversalIdentifier,
workspaceId,
diff --git a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts
index 3c3d17094c..4117963051 100644
--- a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts
+++ b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts
@@ -5,8 +5,6 @@ import { promises as fs } from 'fs';
import { resolve } from 'path';
import semver from 'semver';
-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 { Manifest } from 'twenty-shared/application';
import { FileFolder } from 'twenty-shared/types';
import { isDefined } from 'twenty-shared/utils';
@@ -493,15 +491,8 @@ export class ApplicationInstallService {
);
}
- const { mimeType, ext } = await extractFileInfo({
- file: content,
- filename: relativePath,
- });
- const sanitizedContent = sanitizeFile({ file: content, ext, mimeType });
-
await this.fileStorageService.writeFile({
- sourceFile: sanitizedContent,
- mimeType,
+ sourceFile: content,
fileFolder,
applicationUniversalIdentifier,
workspaceId,
diff --git a/packages/twenty-server/src/engine/core-modules/application/application-registration/application-tarball.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-registration/application-tarball.service.ts
index f2ab940d41..718ba0c583 100644
--- a/packages/twenty-server/src/engine/core-modules/application/application-registration/application-tarball.service.ts
+++ b/packages/twenty-server/src/engine/core-modules/application/application-registration/application-tarball.service.ts
@@ -190,7 +190,6 @@ export class ApplicationTarballService {
const savedFile = await this.fileStorageService.writeFile({
sourceFile: params.tarballBuffer,
resourcePath: `${appRegistration.id}/app.tar.gz`,
- mimeType: 'application/gzip',
fileFolder: FileFolder.AppTarball,
applicationUniversalIdentifier:
workspaceCustomFlatApplication.universalIdentifier,
diff --git a/packages/twenty-server/src/engine/core-modules/application/application.service.ts b/packages/twenty-server/src/engine/core-modules/application/application.service.ts
index aa0399f24c..335935d2e4 100644
--- a/packages/twenty-server/src/engine/core-modules/application/application.service.ts
+++ b/packages/twenty-server/src/engine/core-modules/application/application.service.ts
@@ -411,7 +411,6 @@ export class ApplicationService {
const packageJsonFile = await this.fileStorageService.writeFile({
sourceFile: defaultPackageFields.packageJsonContent,
- mimeType: undefined,
fileFolder: FileFolder.Dependencies,
applicationUniversalIdentifier: application.universalIdentifier,
workspaceId: application.workspaceId,
@@ -422,7 +421,6 @@ export class ApplicationService {
const yarnLockFile = await this.fileStorageService.writeFile({
sourceFile: defaultPackageFields.yarnLockContent,
- mimeType: undefined,
fileFolder: FileFolder.Dependencies,
applicationUniversalIdentifier: application.universalIdentifier,
workspaceId: application.workspaceId,
diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage-exception-filter.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage-exception-filter.spec.ts
new file mode 100644
index 0000000000..f8d31cee6a
--- /dev/null
+++ b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage-exception-filter.spec.ts
@@ -0,0 +1,36 @@
+import { FileStorageExceptionFilter } from 'src/engine/core-modules/file-storage/file-storage-exception-filter';
+import {
+ FileStorageException,
+ FileStorageExceptionCode,
+} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
+import {
+ ForbiddenError,
+ NotFoundError,
+ UserInputError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
+
+describe('FileStorageExceptionFilter', () => {
+ const filter = new FileStorageExceptionFilter();
+
+ it.each([
+ {
+ code: FileStorageExceptionCode.INVALID_EXTENSION,
+ expectedError: UserInputError,
+ },
+ {
+ code: FileStorageExceptionCode.ACCESS_DENIED,
+ expectedError: ForbiddenError,
+ },
+ {
+ code: FileStorageExceptionCode.FILE_NOT_FOUND,
+ expectedError: NotFoundError,
+ },
+ ])(
+ 'should map $code to the expected GraphQL error',
+ ({ code, expectedError }) => {
+ const exception = new FileStorageException('test message', code);
+
+ expect(() => filter.catch(exception)).toThrow(expectedError);
+ },
+ );
+});
diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts
index 39fcbd04f4..851ecbc90c 100644
--- a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts
+++ b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts
@@ -381,7 +381,6 @@ describe('FileStorageService', () => {
...validResourceIdentifier,
resourcePath: context.resourcePath,
sourceFile: Buffer.from('malicious'),
- mimeType: 'application/javascript',
settings: { isTemporaryFile: false, toDelete: false },
}),
).rejects.toMatchObject({
@@ -396,7 +395,6 @@ describe('FileStorageService', () => {
await service.writeFile({
...validResourceIdentifier,
sourceFile: Buffer.from('valid content'),
- mimeType: 'application/javascript',
settings: { isTemporaryFile: false, toDelete: false },
});
@@ -406,6 +404,141 @@ describe('FileStorageService', () => {
}),
);
});
+
+ describe('magic-byte backstop', () => {
+ const pngBuffer = Buffer.from([
+ 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00,
+ 0x0d, 0x49, 0x48, 0x44, 0x52,
+ ]);
+ const textBuffer = Buffer.from('Hello, world!', 'utf-8');
+
+ it('should reject buffer whose magic bytes do not match the path extension', async () => {
+ await expect(
+ service.writeFile({
+ workspaceId: 'workspace-123',
+ applicationUniversalIdentifier: 'app-456',
+ fileFolder: FileFolder.PublicAsset,
+ resourcePath: 'assets/fake-image.png',
+ sourceFile: textBuffer,
+ settings: { isTemporaryFile: false, toDelete: false },
+ }),
+ ).rejects.toMatchObject({
+ code: FileStorageExceptionCode.INVALID_EXTENSION,
+ });
+
+ expect(mockDriver.writeFile).not.toHaveBeenCalled();
+ });
+
+ it('should accept buffer whose magic bytes match the path extension and persist the bytes-derived mime', async () => {
+ await service.writeFile({
+ workspaceId: 'workspace-123',
+ applicationUniversalIdentifier: 'app-456',
+ fileFolder: FileFolder.PublicAsset,
+ resourcePath: 'assets/photo.png',
+ sourceFile: pngBuffer,
+ settings: { isTemporaryFile: false, toDelete: false },
+ });
+
+ expect(mockDriver.writeFile).toHaveBeenCalledWith(
+ expect.objectContaining({ mimeType: 'image/png' }),
+ );
+ expect(mockFileRepository.upsert).toHaveBeenCalledWith(
+ expect.objectContaining({ mimeType: 'image/png' }),
+ expect.anything(),
+ );
+ });
+
+ it('should persist application/typescript for a TypeScript source string (TWENTY_MIME_POLICY)', async () => {
+ await service.writeFile({
+ workspaceId: 'workspace-123',
+ applicationUniversalIdentifier: 'app-456',
+ fileFolder: FileFolder.Source,
+ resourcePath: 'src/index.tsx',
+ sourceFile: 'export const App = () => null;',
+ settings: { isTemporaryFile: false, toDelete: false },
+ });
+
+ expect(mockDriver.writeFile).toHaveBeenCalledWith(
+ expect.objectContaining({ mimeType: 'application/typescript' }),
+ );
+ });
+
+ it('should persist application/typescript for a TypeScript source buffer (policy beats mrmime collision)', async () => {
+ await service.writeFile({
+ workspaceId: 'workspace-123',
+ applicationUniversalIdentifier: 'app-456',
+ fileFolder: FileFolder.Source,
+ resourcePath: 'src/handler.ts',
+ sourceFile: Buffer.from(
+ 'export const handler = () => null;',
+ 'utf-8',
+ ),
+ settings: { isTemporaryFile: false, toDelete: false },
+ });
+
+ expect(mockDriver.writeFile).toHaveBeenCalledWith(
+ expect.objectContaining({ mimeType: 'application/typescript' }),
+ );
+ });
+ });
+
+ describe('SVG sanitization (centralized invariant)', () => {
+ const maliciousSvg =
+ '';
+
+ it('should strip scripts and event handlers from an SVG string upload', async () => {
+ await service.writeFile({
+ workspaceId: 'workspace-123',
+ applicationUniversalIdentifier: 'app-456',
+ fileFolder: FileFolder.PublicAsset,
+ resourcePath: 'assets/icon.svg',
+ sourceFile: maliciousSvg,
+ settings: { isTemporaryFile: false, toDelete: false },
+ });
+
+ const driverCall = mockDriver.writeFile.mock.calls[0][0];
+
+ expect(driverCall.mimeType).toBe('image/svg+xml');
+ expect(typeof driverCall.sourceFile).toBe('string');
+ expect(driverCall.sourceFile).not.toContain('`;
+
+describe('prepareFileForStorageOrThrow', () => {
+ describe('mime resolution', () => {
+ it.each([
+ {
+ name: 'TypeScript .ts string (policy)',
+ sourceFile: 'export const x = 1;',
+ resourcePath: 'src/index.ts',
+ expectedMime: 'application/typescript',
+ },
+ {
+ name: 'TypeScript .tsx string (policy)',
+ sourceFile: 'export const App = () => null;',
+ resourcePath: 'src/App.tsx',
+ expectedMime: 'application/typescript',
+ },
+ {
+ name: '.mjs string (mrmime)',
+ sourceFile: 'export default 1;',
+ resourcePath: 'src/handler.mjs',
+ expectedMime: 'text/javascript',
+ },
+ {
+ name: 'JSON string (mrmime)',
+ sourceFile: '{"foo": "bar"}',
+ resourcePath: 'package.json',
+ expectedMime: 'application/json',
+ },
+ {
+ name: 'unknown-extension string (octet-stream)',
+ sourceFile: 'lock content',
+ resourcePath: 'yarn.lock',
+ expectedMime: 'application/octet-stream',
+ },
+ {
+ name: 'no-extension string (octet-stream)',
+ sourceFile: 'content',
+ resourcePath: 'somefile',
+ expectedMime: 'application/octet-stream',
+ },
+ {
+ name: 'dot-file string (octet-stream — extname semantics)',
+ sourceFile: 'content',
+ resourcePath: '.gitignore',
+ expectedMime: 'application/octet-stream',
+ },
+ ])(
+ 'returns $expectedMime for $name',
+ async ({ sourceFile, resourcePath, expectedMime }) => {
+ const { mimeType } = await prepareFileForStorageOrThrow({
+ sourceFile,
+ resourcePath,
+ });
+
+ expect(mimeType).toBe(expectedMime);
+ },
+ );
+
+ it.each([
+ {
+ name: 'PNG buffer',
+ sourceFile: pngBuffer,
+ resourcePath: 'images/photo.png',
+ expectedMime: 'image/png',
+ },
+ {
+ name: 'PDF buffer',
+ sourceFile: pdfBuffer,
+ resourcePath: 'docs/contract.pdf',
+ expectedMime: 'application/pdf',
+ },
+ {
+ name: 'ZIP buffer',
+ sourceFile: zipBuffer,
+ resourcePath: 'archives/data.zip',
+ expectedMime: 'application/zip',
+ },
+ ])(
+ 'returns the bytes-derived mime for $name',
+ async ({ sourceFile, resourcePath, expectedMime }) => {
+ const { mimeType } = await prepareFileForStorageOrThrow({
+ sourceFile,
+ resourcePath,
+ });
+
+ expect(mimeType).toBe(expectedMime);
+ },
+ );
+
+ it('returns the bytes-derived mime for a Uint8Array source', async () => {
+ const { mimeType } = await prepareFileForStorageOrThrow({
+ sourceFile: new Uint8Array(pngBuffer),
+ resourcePath: 'images/photo.png',
+ });
+
+ expect(mimeType).toBe('image/png');
+ });
+ });
+
+ describe('binary-extension backstop', () => {
+ it('throws when a buffer with a binary extension does not match the magic bytes', async () => {
+ await expect(
+ prepareFileForStorageOrThrow({
+ sourceFile: textBuffer,
+ resourcePath: 'fake-image.png',
+ }),
+ ).rejects.toMatchObject({
+ code: FileStorageExceptionCode.INVALID_EXTENSION,
+ });
+ });
+
+ it('throws when a string with a binary extension does not match the magic bytes', async () => {
+ await expect(
+ prepareFileForStorageOrThrow({
+ sourceFile: 'this is not a png',
+ resourcePath: 'fake-image.png',
+ }),
+ ).rejects.toMatchObject({
+ code: FileStorageExceptionCode.INVALID_EXTENSION,
+ });
+ });
+
+ it('throws on Uint8Array byte/extension mismatch', async () => {
+ await expect(
+ prepareFileForStorageOrThrow({
+ sourceFile: new Uint8Array(textBuffer),
+ resourcePath: 'fake-image.pdf',
+ }),
+ ).rejects.toMatchObject({
+ code: FileStorageExceptionCode.INVALID_EXTENSION,
+ });
+ });
+ });
+
+ describe('SVG sanitization', () => {
+ it('sanitizes an SVG string by removing scripts and event handlers', async () => {
+ const { sourceFile, mimeType } = await prepareFileForStorageOrThrow({
+ sourceFile: maliciousSvg,
+ resourcePath: 'assets/icon.svg',
+ });
+
+ expect(mimeType).toBe('image/svg+xml');
+ expect(typeof sourceFile).toBe('string');
+ expect(sourceFile).not.toContain('
+
+
+
+`;
+
+// Deterministic DOMPurify output for MALICIOUS_SVG. Shape:
+// - stripped (the two leading spaces on that line are
+// preserved — DOMPurify removes the element, not surrounding whitespace).
+// - onload="..." attribute stripped from