Application file storage service (#20793)

# Introduction Fix unsafe resource path join with expected prefix at file storage directly Add early paths transversal detections in metadata validators
2026-05-21 11:21:44 +02:00
parent 1ed347bc17
commit a3c92311e3
20 changed files with 1661 additions and 5 deletions
@@ -1,9 +1,12 @@
 import { Test, type TestingModule } from '@nestjs/testing';
 import { getRepositoryToken } from '@nestjs/typeorm';

+import { FileFolder } from 'twenty-shared/types';
+
 import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity';
 import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory';
 import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
+import { FileStorageExceptionCode } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
 import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';

 describe('FileStorageService', () => {
@@ -16,6 +19,9 @@ describe('FileStorageService', () => {

  const mockFileRepository = {
    save: jest.fn(),
+    upsert: jest.fn(),
+    findOneOrFail: jest.fn(),
+    delete: jest.fn(),
  };

  const mockApplicationRepository = {
@@ -63,6 +69,7 @@ describe('FileStorageService', () => {
        delete: jest.fn(),
        move: jest.fn(),
        copy: jest.fn(),
+        downloadFile: jest.fn(),
        downloadFolder: jest.fn(),
        uploadFolder: jest.fn(),
        checkFileExists: jest.fn(),
@@ -147,4 +154,306 @@ describe('FileStorageService', () => {
      });
    });
  });
+
+  describe('path traversal protection', () => {
+    let mockDriver: any;
+
+    beforeEach(() => {
+      mockDriver = {
+        writeFile: jest.fn().mockResolvedValue(undefined),
+        readFile: jest.fn().mockResolvedValue('stream'),
+        delete: jest.fn().mockResolvedValue(undefined),
+        copy: jest.fn().mockResolvedValue(undefined),
+        downloadFile: jest.fn().mockResolvedValue(undefined),
+        checkFileExists: jest.fn().mockResolvedValue(true),
+        checkFolderExists: jest.fn().mockResolvedValue(true),
+        getPresignedUrl: jest.fn().mockResolvedValue('https://signed.url'),
+      };
+
+      mockFileStorageDriverFactory.getCurrentDriver.mockReturnValue(mockDriver);
+      mockApplicationRepository.findOneOrFail.mockResolvedValue({
+        id: 'app-id',
+        universalIdentifier: 'app-uid',
+      });
+      mockFileRepository.upsert.mockResolvedValue(undefined);
+      mockFileRepository.findOneOrFail.mockResolvedValue({
+        id: 'file-id',
+        path: 'BuiltFrontComponent/file.mjs',
+        mimeType: 'application/javascript',
+      });
+    });
+
+    const validResourceIdentifier = {
+      workspaceId: 'workspace-123',
+      applicationUniversalIdentifier: 'app-456',
+      fileFolder: FileFolder.BuiltFrontComponent,
+      resourcePath: 'src/components/my-component.mjs',
+    };
+
+    const expectedValidPath =
+      'workspace-123/app-456/built-front-component/src/components/my-component.mjs';
+
+    describe('readFile', () => {
+      it('should allow valid relative paths', async () => {
+        await service.readFile(validResourceIdentifier);
+
+        expect(mockDriver.readFile).toHaveBeenCalledWith({
+          filePath: expectedValidPath,
+        });
+      });
+
+      it('should reject path traversal with ../', () => {
+        expect(() =>
+          service.readFile({
+            ...validResourceIdentifier,
+            resourcePath:
+              '../../../victim-ws/victim-app/built-front-component/secret.mjs',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+
+        expect(mockDriver.readFile).not.toHaveBeenCalled();
+      });
+
+      it('should reject absolute paths', () => {
+        expect(() =>
+          service.readFile({
+            ...validResourceIdentifier,
+            resourcePath: '/etc/passwd',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+
+        expect(mockDriver.readFile).not.toHaveBeenCalled();
+      });
+
+      it('should reject single-level traversal escaping fileFolder', () => {
+        expect(() =>
+          service.readFile({
+            ...validResourceIdentifier,
+            resourcePath: '../source/handler.ts',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+
+        expect(mockDriver.readFile).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('checkFileExists', () => {
+      it('should allow valid relative paths', async () => {
+        await service.checkFileExists(validResourceIdentifier);
+
+        expect(mockDriver.checkFileExists).toHaveBeenCalledWith({
+          filePath: expectedValidPath,
+        });
+      });
+
+      it('should reject path traversal with ../', () => {
+        expect(() =>
+          service.checkFileExists({
+            ...validResourceIdentifier,
+            resourcePath:
+              '../../../other-ws/other-app/built-front-component/file.js',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+
+        expect(mockDriver.checkFileExists).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('getPresignedUrl', () => {
+      it('should reject path traversal', async () => {
+        await expect(
+          service.getPresignedUrl({
+            ...validResourceIdentifier,
+            resourcePath: '../../../other-ws/file.js',
+          }),
+        ).rejects.toMatchObject({
+          code: FileStorageExceptionCode.ACCESS_DENIED,
+        });
+
+        expect(mockDriver.getPresignedUrl).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('downloadFile', () => {
+      it('should reject path traversal', () => {
+        expect(() =>
+          service.downloadFile({
+            ...validResourceIdentifier,
+            resourcePath: '../../../other-ws/file.js',
+            localPath: '/tmp/download.js',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+
+        expect(mockDriver.downloadFile).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('writeFile', () => {
+      it('should reject path traversal on write', async () => {
+        await expect(
+          service.writeFile({
+            ...validResourceIdentifier,
+            resourcePath:
+              '../../../victim-ws/victim-app/built-front-component/overwrite.mjs',
+            sourceFile: Buffer.from('malicious'),
+            mimeType: 'application/javascript',
+            settings: { isTemporaryFile: false, toDelete: false },
+          }),
+        ).rejects.toMatchObject({
+          code: FileStorageExceptionCode.ACCESS_DENIED,
+        });
+
+        expect(mockDriver.writeFile).not.toHaveBeenCalled();
+      });
+
+      it('should allow valid writes', async () => {
+        await service.writeFile({
+          ...validResourceIdentifier,
+          sourceFile: Buffer.from('valid content'),
+          mimeType: 'application/javascript',
+          settings: { isTemporaryFile: false, toDelete: false },
+        });
+
+        expect(mockDriver.writeFile).toHaveBeenCalledWith(
+          expect.objectContaining({
+            filePath: expectedValidPath,
+          }),
+        );
+      });
+    });
+
+    describe('delete', () => {
+      it('should reject path traversal on delete', async () => {
+        await expect(
+          service.delete({
+            ...validResourceIdentifier,
+            resourcePath: '../../../other-ws/other-app/folder',
+          }),
+        ).rejects.toMatchObject({
+          code: FileStorageExceptionCode.ACCESS_DENIED,
+        });
+
+        expect(mockDriver.delete).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('copy', () => {
+      it('should reject path traversal in source', async () => {
+        await expect(
+          service.copy({
+            from: {
+              ...validResourceIdentifier,
+              resourcePath: '../../../other-ws/secret.mjs',
+            },
+            to: validResourceIdentifier,
+          }),
+        ).rejects.toMatchObject({
+          code: FileStorageExceptionCode.ACCESS_DENIED,
+        });
+
+        expect(mockDriver.copy).not.toHaveBeenCalled();
+      });
+
+      it('should reject path traversal in destination', async () => {
+        mockDriver.checkFileExists.mockResolvedValue(true);
+
+        await expect(
+          service.copy({
+            from: validResourceIdentifier,
+            to: {
+              ...validResourceIdentifier,
+              resourcePath: '../../../other-ws/overwrite.mjs',
+            },
+          }),
+        ).rejects.toMatchObject({
+          code: FileStorageExceptionCode.ACCESS_DENIED,
+        });
+      });
+    });
+
+    describe('edge cases', () => {
+      it('should reject traversal with excess .. segments', () => {
+        expect(() =>
+          service.readFile({
+            ...validResourceIdentifier,
+            resourcePath: 'foo/../../../../../../etc/passwd',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+      });
+
+      it('should accept deeply nested valid paths', async () => {
+        await service.readFile({
+          ...validResourceIdentifier,
+          resourcePath: 'a/b/c/d/e/f/deep-file.mjs',
+        });
+
+        expect(mockDriver.readFile).toHaveBeenCalledWith({
+          filePath:
+            'workspace-123/app-456/built-front-component/a/b/c/d/e/f/deep-file.mjs',
+        });
+      });
+
+      it('should reject exact 3-level traversal to another tenant', () => {
+        expect(() =>
+          service.readFile({
+            ...validResourceIdentifier,
+            resourcePath:
+              '../../../target-ws/target-app/built-front-component/file.js',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+      });
+
+      it('should accept paths with dots that are not traversal', async () => {
+        await service.readFile({
+          ...validResourceIdentifier,
+          resourcePath: '.hidden/file.name.ext',
+        });
+
+        expect(mockDriver.readFile).toHaveBeenCalled();
+      });
+
+      it('should reject empty resource path', () => {
+        expect(() =>
+          service.readFile({
+            ...validResourceIdentifier,
+            resourcePath: '',
+          }),
+        ).toThrow(
+          expect.objectContaining({
+            code: FileStorageExceptionCode.ACCESS_DENIED,
+          }),
+        );
+
+        expect(mockDriver.readFile).not.toHaveBeenCalled();
+      });
+    });
+  });
 });
@@ -9,6 +9,8 @@ import { Like, Repository, type QueryRunner } from 'typeorm';

 import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity';
 import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory';
+import { assertStoragePathIsWithinWorkspace } from 'src/engine/core-modules/file-storage/utils/assert-storage-path-is-within-workspace.util';
+import { assertResourcePathIsSafe } from 'src/engine/core-modules/file-storage/utils/assert-resource-path-is-safe.util';
 import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
 import { FileSettings } from 'src/engine/core-modules/file/types/file-settings.types';

@@ -35,12 +37,23 @@ export class FileStorageService {
    fileFolder,
    resourcePath,
  }: ResourceIdentifier): string {
-    return join(
+    assertResourcePathIsSafe(resourcePath);
+
+    const onStoragePath = join(
      workspaceId,
      applicationUniversalIdentifier,
      fileFolder,
      resourcePath,
    ).replace(/\/+/g, '/');
+
+    assertStoragePathIsWithinWorkspace({
+      onStoragePath,
+      workspaceId,
+      applicationUniversalIdentifier,
+      fileFolder,
+    });
+
+    return onStoragePath;
  }

  async writeFile({
@@ -0,0 +1,53 @@
+import { FileStorageExceptionCode } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
+
+import { assertResourcePathIsSafe } from 'src/engine/core-modules/file-storage/utils/assert-resource-path-is-safe.util';
+
+describe('assertResourcePathIsSafe', () => {
+  it('should accept valid relative paths', () => {
+    expect(() =>
+      assertResourcePathIsSafe('src/components/test.mjs'),
+    ).not.toThrow();
+    expect(() => assertResourcePathIsSafe('file.mjs')).not.toThrow();
+    expect(() => assertResourcePathIsSafe('a/b/c/d.txt')).not.toThrow();
+  });
+
+  it('should reject paths with .. traversal', () => {
+    expect(() => assertResourcePathIsSafe('../../../other-ws/file.js')).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject absolute paths', () => {
+    expect(() => assertResourcePathIsSafe('/etc/passwd')).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject paths with null bytes', () => {
+    expect(() => assertResourcePathIsSafe('file\0.txt')).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject paths with backslashes', () => {
+    expect(() => assertResourcePathIsSafe('..\\..\\etc\\passwd')).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject empty strings', () => {
+    expect(() => assertResourcePathIsSafe('')).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+});
@@ -0,0 +1,86 @@
+import { FileFolder } from 'twenty-shared/types';
+
+import { FileStorageExceptionCode } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
+
+import { assertStoragePathIsWithinWorkspace } from 'src/engine/core-modules/file-storage/utils/assert-storage-path-is-within-workspace.util';
+
+const primitives = {
+  workspaceId: 'workspace-id',
+  applicationUniversalIdentifier: 'app-uid',
+  fileFolder: FileFolder.BuiltFrontComponent,
+};
+
+describe('assertStoragePathIsWithinWorkspace', () => {
+  it('should accept paths within the expected prefix', () => {
+    expect(() =>
+      assertStoragePathIsWithinWorkspace({
+        onStoragePath:
+          'workspace-id/app-uid/built-front-component/src/component.mjs',
+        ...primitives,
+      }),
+    ).not.toThrow();
+  });
+
+  it('should accept paths directly under the prefix', () => {
+    expect(() =>
+      assertStoragePathIsWithinWorkspace({
+        onStoragePath: 'workspace-id/app-uid/built-front-component/file.mjs',
+        ...primitives,
+      }),
+    ).not.toThrow();
+  });
+
+  it('should reject paths that escape via .. traversal', () => {
+    expect(() =>
+      assertStoragePathIsWithinWorkspace({
+        onStoragePath:
+          'other-workspace/other-app/built-front-component/stolen.mjs',
+        ...primitives,
+      }),
+    ).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject paths that escape by one level', () => {
+    expect(() =>
+      assertStoragePathIsWithinWorkspace({
+        onStoragePath: 'workspace-id/app-uid/other-folder/file.mjs',
+        ...primitives,
+      }),
+    ).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject the prefix itself without a trailing file', () => {
+    expect(() =>
+      assertStoragePathIsWithinWorkspace({
+        onStoragePath: 'workspace-id/app-uid/built-front-component',
+        ...primitives,
+      }),
+    ).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+
+  it('should reject paths where prefix is a partial match', () => {
+    expect(() =>
+      assertStoragePathIsWithinWorkspace({
+        onStoragePath:
+          'workspace-id/app-uid/built-front-componentMalicious/file.mjs',
+        ...primitives,
+      }),
+    ).toThrow(
+      expect.objectContaining({
+        code: FileStorageExceptionCode.ACCESS_DENIED,
+      }),
+    );
+  });
+});
@@ -0,0 +1,42 @@
+import { isSafeRelativePath } from 'src/engine/core-modules/file-storage/utils/is-safe-relative-path.util';
+
+describe('isSafeRelativePath', () => {
+  it('should accept valid relative paths', () => {
+    expect(isSafeRelativePath('src/components/my-component.mjs')).toBe(true);
+    expect(isSafeRelativePath('file.mjs')).toBe(true);
+    expect(isSafeRelativePath('a/b/c/d.txt')).toBe(true);
+    expect(isSafeRelativePath('.hidden-file')).toBe(true);
+    expect(isSafeRelativePath('folder/.gitignore')).toBe(true);
+    expect(isSafeRelativePath('file.name.ext')).toBe(true);
+  });
+
+  it('should reject paths with .. traversal segments', () => {
+    expect(isSafeRelativePath('../etc/passwd')).toBe(false);
+    expect(isSafeRelativePath('folder/../../etc/passwd')).toBe(false);
+    expect(isSafeRelativePath('..')).toBe(false);
+    expect(
+      isSafeRelativePath(
+        '../../../other-ws/other-app/BuiltFrontComponent/file.js',
+      ),
+    ).toBe(false);
+  });
+
+  it('should reject paths with null bytes', () => {
+    expect(isSafeRelativePath('file\0.txt')).toBe(false);
+    expect(isSafeRelativePath('folder/\0/file.txt')).toBe(false);
+  });
+
+  it('should reject absolute paths', () => {
+    expect(isSafeRelativePath('/etc/passwd')).toBe(false);
+    expect(isSafeRelativePath('/tmp/file.txt')).toBe(false);
+  });
+
+  it('should reject paths with backslashes', () => {
+    expect(isSafeRelativePath('folder\\file.txt')).toBe(false);
+    expect(isSafeRelativePath('..\\..\\etc\\passwd')).toBe(false);
+  });
+
+  it('should reject empty strings', () => {
+    expect(isSafeRelativePath('')).toBe(false);
+  });
+});
@@ -0,0 +1,14 @@
+import {
+  FileStorageException,
+  FileStorageExceptionCode,
+} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
+import { isSafeRelativePath } from 'src/engine/core-modules/file-storage/utils/is-safe-relative-path.util';
+
+export const assertResourcePathIsSafe = (resourcePath: string): void => {
+  if (!isSafeRelativePath(resourcePath)) {
+    throw new FileStorageException(
+      'Invalid resource path: contains unsafe characters or path traversal',
+      FileStorageExceptionCode.ACCESS_DENIED,
+    );
+  }
+};
@@ -0,0 +1,36 @@
+import { join, normalize } from 'path';
+
+import { type FileFolder } from 'twenty-shared/types';
+
+import {
+  FileStorageException,
+  FileStorageExceptionCode,
+} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
+
+export const assertStoragePathIsWithinWorkspace = ({
+  onStoragePath,
+  workspaceId,
+  applicationUniversalIdentifier,
+  fileFolder,
+}: {
+  onStoragePath: string;
+  workspaceId: string;
+  applicationUniversalIdentifier: string;
+  fileFolder: FileFolder;
+}): void => {
+  const expectedPrefix = join(
+    workspaceId,
+    applicationUniversalIdentifier,
+    fileFolder,
+  );
+
+  const normalizedPath = normalize(onStoragePath);
+  const normalizedPrefix = normalize(expectedPrefix + '/');
+
+  if (!normalizedPath.startsWith(normalizedPrefix)) {
+    throw new FileStorageException(
+      'Invalid storage path: resolved path escapes the expected directory',
+      FileStorageExceptionCode.ACCESS_DENIED,
+    );
+  }
+};
@@ -0,0 +1,27 @@
+import { isAbsolute, normalize, sep } from 'path';
+
+export const isSafeRelativePath = (filePath: string): boolean => {
+  if (filePath.length === 0) {
+    return false;
+  }
+
+  if (filePath.includes('\0')) {
+    return false;
+  }
+
+  if (isAbsolute(filePath)) {
+    return false;
+  }
+
+  if (filePath.includes('\\')) {
+    return false;
+  }
+
+  const normalized = normalize(filePath);
+
+  if (normalized.split(sep).includes('..')) {
+    return false;
+  }
+
+  return true;
+};