From e891e13e64acaa00d5ec6f8436fe9afd2af25d72 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Thu, 21 May 2026 11:21:44 +0200 Subject: [PATCH] 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 --- .../__tests__/file-storage.service.spec.ts | 309 ++++++++++++++++++ .../file-storage/file-storage.service.ts | 15 +- .../assert-resource-path-is-safe.util.spec.ts | 53 +++ ...rage-path-is-within-workspace.util.spec.ts | 86 +++++ .../is-safe-relative-path.util.spec.ts | 42 +++ .../assert-resource-path-is-safe.util.ts | 14 + ...t-storage-path-is-within-workspace.util.ts | 36 ++ .../utils/is-safe-relative-path.util.ts | 27 ++ .../logic-function.exception.ts | 3 + ...ion-graphql-api-exception-handler.utils.ts | 1 + .../flat-front-component-validator.service.ts | 46 +++ .../flat-logic-function-validator.service.ts | 54 ++- ...omponent-creation.integration-spec.ts.snap | 110 ++++++- ...te-path-traversal.integration-spec.ts.snap | 141 ++++++++ ...ont-component-creation.integration-spec.ts | 37 +++ ...-update-path-traversal.integration-spec.ts | 141 ++++++++ ...function-creation.integration-spec.ts.snap | 141 ++++++++ ...te-path-traversal.integration-spec.ts.snap | 141 ++++++++ ...ogic-function-creation.integration-spec.ts | 128 ++++++++ ...-update-path-traversal.integration-spec.ts | 141 ++++++++ 20 files changed, 1661 insertions(+), 5 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-resource-path-is-safe.util.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-storage-path-is-within-workspace.util.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/is-safe-relative-path.util.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-resource-path-is-safe.util.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-storage-path-is-within-workspace.util.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/is-safe-relative-path.util.ts create mode 100644 packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-update-path-traversal.integration-spec.ts.snap create mode 100644 packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-update-path-traversal.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-creation.integration-spec.ts.snap create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-update-path-traversal.integration-spec.ts.snap create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-creation.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-update-path-traversal.integration-spec.ts 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 1cbe772e04f..a955b8bf472 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 @@ -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(); + }); + }); + }); }); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts b/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts index 95d64004dbb..77c07786b14 100644 --- a/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts @@ -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({ diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-resource-path-is-safe.util.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-resource-path-is-safe.util.spec.ts new file mode 100644 index 00000000000..ba1b727cf21 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-resource-path-is-safe.util.spec.ts @@ -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, + }), + ); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-storage-path-is-within-workspace.util.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-storage-path-is-within-workspace.util.spec.ts new file mode 100644 index 00000000000..416879a6f22 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/assert-storage-path-is-within-workspace.util.spec.ts @@ -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, + }), + ); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/is-safe-relative-path.util.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/is-safe-relative-path.util.spec.ts new file mode 100644 index 00000000000..12c9da69133 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/is-safe-relative-path.util.spec.ts @@ -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); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-resource-path-is-safe.util.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-resource-path-is-safe.util.ts new file mode 100644 index 00000000000..f891fe12b29 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-resource-path-is-safe.util.ts @@ -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, + ); + } +}; diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-storage-path-is-within-workspace.util.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-storage-path-is-within-workspace.util.ts new file mode 100644 index 00000000000..55161b996c0 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/assert-storage-path-is-within-workspace.util.ts @@ -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, + ); + } +}; diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/is-safe-relative-path.util.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/is-safe-relative-path.util.ts new file mode 100644 index 00000000000..bc04d48dde0 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/is-safe-relative-path.util.ts @@ -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; +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/logic-function/logic-function.exception.ts b/packages/twenty-server/src/engine/metadata-modules/logic-function/logic-function.exception.ts index 31a40b64cba..ab16047553f 100644 --- a/packages/twenty-server/src/engine/metadata-modules/logic-function/logic-function.exception.ts +++ b/packages/twenty-server/src/engine/metadata-modules/logic-function/logic-function.exception.ts @@ -18,6 +18,7 @@ export enum LogicFunctionExceptionCode { LOGIC_FUNCTION_LAYER_BUILD_FAILED = 'LOGIC_FUNCTION_LAYER_BUILD_FAILED', LOGIC_FUNCTION_DISABLED = 'LOGIC_FUNCTION_DISABLED', LOGIC_FUNCTION_INVALID_SEED_PROJECT = 'LOGIC_FUNCTION_INVALID_SEED_PROJECT', + INVALID_LOGIC_FUNCTION_INPUT = 'INVALID_LOGIC_FUNCTION_INPUT', } const getLogicFunctionExceptionUserFriendlyMessage = ( @@ -50,6 +51,8 @@ const getLogicFunctionExceptionUserFriendlyMessage = ( return msg`Logic function execution is disabled.`; case LogicFunctionExceptionCode.LOGIC_FUNCTION_INVALID_SEED_PROJECT: return msg`Invalid seed project configuration.`; + case LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT: + return msg`Invalid logic function input.`; default: assertUnreachable(code); } diff --git a/packages/twenty-server/src/engine/metadata-modules/logic-function/utils/logic-function-graphql-api-exception-handler.utils.ts b/packages/twenty-server/src/engine/metadata-modules/logic-function/utils/logic-function-graphql-api-exception-handler.utils.ts index f234ef791f7..15a8209b453 100644 --- a/packages/twenty-server/src/engine/metadata-modules/logic-function/utils/logic-function-graphql-api-exception-handler.utils.ts +++ b/packages/twenty-server/src/engine/metadata-modules/logic-function/utils/logic-function-graphql-api-exception-handler.utils.ts @@ -33,6 +33,7 @@ export const logicFunctionGraphQLApiExceptionHandler = (error: any) => { case LogicFunctionExceptionCode.LOGIC_FUNCTION_LAYER_BUILD_FAILED: throw error; case LogicFunctionExceptionCode.LOGIC_FUNCTION_COMPILATION_FAILED: + case LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT: throw new UserInputError(error); case LogicFunctionExceptionCode.LOGIC_FUNCTION_DISABLED: throw new ForbiddenError(error); diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-front-component-validator.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-front-component-validator.service.ts index 3d7bdd19c68..062992dd515 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-front-component-validator.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-front-component-validator.service.ts @@ -5,6 +5,7 @@ import { isNonEmptyString } from '@sniptt/guards'; import { ALL_METADATA_NAME } from 'twenty-shared/metadata'; import { isDefined } from 'twenty-shared/utils'; +import { isSafeRelativePath } from 'src/engine/core-modules/file-storage/utils/is-safe-relative-path.util'; import { findFlatEntityByUniversalIdentifier } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-universal-identifier.util'; import { FrontComponentExceptionCode } from 'src/engine/metadata-modules/front-component/front-component.exception'; import { type FailedFlatEntityValidation } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/types/failed-flat-entity-validation.type'; @@ -36,6 +37,28 @@ export class FlatFrontComponentValidatorService { }); } + if ( + isDefined(flatFrontComponent.builtComponentPath) && + !isSafeRelativePath(flatFrontComponent.builtComponentPath) + ) { + validationResult.errors.push({ + code: FrontComponentExceptionCode.INVALID_FRONT_COMPONENT_INPUT, + message: t`Built component path contains unsafe characters`, + userFriendlyMessage: msg`Built component path contains unsafe characters`, + }); + } + + if ( + isDefined(flatFrontComponent.sourceComponentPath) && + !isSafeRelativePath(flatFrontComponent.sourceComponentPath) + ) { + validationResult.errors.push({ + code: FrontComponentExceptionCode.INVALID_FRONT_COMPONENT_INPUT, + message: t`Source component path contains unsafe characters`, + userFriendlyMessage: msg`Source component path contains unsafe characters`, + }); + } + return validationResult; } @@ -76,6 +99,7 @@ export class FlatFrontComponentValidatorService { public validateFlatFrontComponentUpdate({ universalIdentifier, + flatEntityUpdate, optimisticFlatEntityMapsAndRelatedFlatEntityMaps: { flatFrontComponentMaps: optimisticFlatFrontComponentMaps, }, @@ -105,6 +129,28 @@ export class FlatFrontComponentValidatorService { return validationResult; } + if ( + isDefined(flatEntityUpdate.builtComponentPath) && + !isSafeRelativePath(flatEntityUpdate.builtComponentPath) + ) { + validationResult.errors.push({ + code: FrontComponentExceptionCode.INVALID_FRONT_COMPONENT_INPUT, + message: t`Built component path contains unsafe characters`, + userFriendlyMessage: msg`Built component path contains unsafe characters`, + }); + } + + if ( + isDefined(flatEntityUpdate.sourceComponentPath) && + !isSafeRelativePath(flatEntityUpdate.sourceComponentPath) + ) { + validationResult.errors.push({ + code: FrontComponentExceptionCode.INVALID_FRONT_COMPONENT_INPUT, + message: t`Source component path contains unsafe characters`, + userFriendlyMessage: msg`Source component path contains unsafe characters`, + }); + } + return validationResult; } } diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts index bf0ebc305a8..da5d99bad6a 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-logic-function-validator.service.ts @@ -4,12 +4,13 @@ import { msg, t } from '@lingui/core/macro'; import { ALL_METADATA_NAME } from 'twenty-shared/metadata'; import { isDefined } from 'twenty-shared/utils'; +import { isSafeRelativePath } from 'src/engine/core-modules/file-storage/utils/is-safe-relative-path.util'; import { findFlatEntityByUniversalIdentifier } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-universal-identifier.util'; import { LogicFunctionExceptionCode } from 'src/engine/metadata-modules/logic-function/logic-function.exception'; -import { FailedFlatEntityValidation } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/types/failed-flat-entity-validation.type'; +import { type FailedFlatEntityValidation } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/types/failed-flat-entity-validation.type'; import { getEmptyFlatEntityValidationError } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/utils/get-flat-entity-validation-error.util'; -import { FlatEntityUpdateValidationArgs } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type'; -import { UniversalFlatEntityValidationArgs } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-validation-args.type'; +import { type FlatEntityUpdateValidationArgs } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type'; +import { type UniversalFlatEntityValidationArgs } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-validation-args.type'; @Injectable() export class FlatLogicFunctionValidatorService { @@ -17,6 +18,7 @@ export class FlatLogicFunctionValidatorService { public validateFlatLogicFunctionUpdate({ universalIdentifier, + flatEntityUpdate, optimisticFlatEntityMapsAndRelatedFlatEntityMaps: { flatLogicFunctionMaps: optimisticFlatLogicFunctionMaps, }, @@ -42,6 +44,30 @@ export class FlatLogicFunctionValidatorService { message: t`Logic function not found`, userFriendlyMessage: msg`Logic function not found`, }); + + return validationResult; + } + + if ( + isDefined(flatEntityUpdate.builtHandlerPath) && + !isSafeRelativePath(flatEntityUpdate.builtHandlerPath) + ) { + validationResult.errors.push({ + code: LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + message: t`Built handler path contains unsafe characters`, + userFriendlyMessage: msg`Built handler path contains unsafe characters`, + }); + } + + if ( + isDefined(flatEntityUpdate.sourceHandlerPath) && + !isSafeRelativePath(flatEntityUpdate.sourceHandlerPath) + ) { + validationResult.errors.push({ + code: LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + message: t`Source handler path contains unsafe characters`, + userFriendlyMessage: msg`Source handler path contains unsafe characters`, + }); } return validationResult; @@ -110,6 +136,28 @@ export class FlatLogicFunctionValidatorService { }); } + if ( + isDefined(flatLogicFunctionToValidate.builtHandlerPath) && + !isSafeRelativePath(flatLogicFunctionToValidate.builtHandlerPath) + ) { + validationResult.errors.push({ + code: LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + message: t`Built handler path contains unsafe characters`, + userFriendlyMessage: msg`Built handler path contains unsafe characters`, + }); + } + + if ( + isDefined(flatLogicFunctionToValidate.sourceHandlerPath) && + !isSafeRelativePath(flatLogicFunctionToValidate.sourceHandlerPath) + ) { + validationResult.errors.push({ + code: LogicFunctionExceptionCode.INVALID_LOGIC_FUNCTION_INPUT, + message: t`Source handler path contains unsafe characters`, + userFriendlyMessage: msg`Source handler path contains unsafe characters`, + }); + } + return validationResult; } } diff --git a/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-creation.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-creation.integration-spec.ts.snap index 4e8ac3f094e..f737c6b5c87 100644 --- a/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-creation.integration-spec.ts.snap +++ b/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-creation.integration-spec.ts.snap @@ -1,4 +1,76 @@ -// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Front component creation should fail when builtComponentPath contains path traversal 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Built component path contains unsafe characters", + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "name": "TraversalTest", + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + "message": "Multiple validation errors occurred while creating front component", + "name": "GraphQLError", +} +`; + +exports[`Front component creation should fail when builtComponentPath is an absolute path 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Built component path contains unsafe characters", + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "name": "AbsolutePathTest", + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + "message": "Multiple validation errors occurred while creating front component", + "name": "GraphQLError", +} +`; exports[`Front component creation should fail when name is empty 1`] = ` { @@ -71,3 +143,39 @@ exports[`Front component creation should fail when name is whitespace-only 1`] = "name": "GraphQLError", } `; + +exports[`Front component creation should fail when sourceComponentPath contains path traversal 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Source component path contains unsafe characters", + "userFriendlyMessage": "Source component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "name": "TraversalTest", + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Source component path contains unsafe characters", + }, + "message": "Multiple validation errors occurred while creating front component", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-update-path-traversal.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-update-path-traversal.integration-spec.ts.snap new file mode 100644 index 00000000000..b4c9d8e5453 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/front-component/__snapshots__/failing-front-component-update-path-traversal.integration-spec.ts.snap @@ -0,0 +1,141 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Front component update via manifest sync should fail for path traversal when builtComponentPath is updated with absolute path via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Built component path contains unsafe characters", + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Front component update via manifest sync should fail for path traversal when builtComponentPath is updated with backslash path via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Built component path contains unsafe characters", + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Front component update via manifest sync should fail for path traversal when builtComponentPath is updated with path traversal via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Built component path contains unsafe characters", + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built component path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Front component update via manifest sync should fail for path traversal when sourceComponentPath is updated with path traversal via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "frontComponent": [ + { + "errors": [ + { + "code": "INVALID_FRONT_COMPONENT_INPUT", + "message": "Source component path contains unsafe characters", + "userFriendlyMessage": "Source component path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "frontComponent", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 frontComponent", + "summary": { + "frontComponent": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Source component path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-creation.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-creation.integration-spec.ts index e1fcc8c5df6..7c4e401e327 100644 --- a/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-creation.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-creation.integration-spec.ts @@ -37,6 +37,43 @@ const FAILING_TEST_CASES: EachTestingContext[] = [ }, }, }, + { + title: 'when builtComponentPath contains path traversal', + context: { + input: { + name: 'TraversalTest', + componentName: 'TraversalTest', + sourceComponentPath: 'src/front-components/index.tsx', + builtComponentPath: + '../../../other-workspace/other-app/BuiltFrontComponent/stolen.mjs', + builtComponentChecksum: 'abc123', + }, + }, + }, + { + title: 'when sourceComponentPath contains path traversal', + context: { + input: { + name: 'TraversalTest', + componentName: 'TraversalTest', + sourceComponentPath: '../../etc/passwd', + builtComponentPath: 'src/front-components/index.mjs', + builtComponentChecksum: 'abc123', + }, + }, + }, + { + title: 'when builtComponentPath is an absolute path', + context: { + input: { + name: 'AbsolutePathTest', + componentName: 'AbsolutePathTest', + sourceComponentPath: 'src/front-components/index.tsx', + builtComponentPath: '/etc/passwd', + builtComponentChecksum: 'abc123', + }, + }, + }, ]; describe('Front component creation should fail', () => { diff --git a/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-update-path-traversal.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-update-path-traversal.integration-spec.ts new file mode 100644 index 00000000000..6f332129ebc --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/front-component/failing-front-component-update-path-traversal.integration-spec.ts @@ -0,0 +1,141 @@ +import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { + type FrontComponentManifest, + type Manifest, +} from 'twenty-shared/application'; +import { + type EachTestingContext, + eachTestingContextFilter, +} from 'twenty-shared/testing'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); +const FRONT_COMPONENT_ID = uuidv4(); + +const VALID_BUILT_PATH = 'src/front-components/test.mjs'; + +const buildValidFrontComponent = (): FrontComponentManifest => ({ + universalIdentifier: FRONT_COMPONENT_ID, + name: 'TestComponent', + description: 'A test front component', + sourceComponentPath: 'src/front-components/test.tsx', + builtComponentPath: VALID_BUILT_PATH, + builtComponentChecksum: 'valid-checksum', + componentName: 'TestComponent', + isHeadless: false, +}); + +const buildManifest = ( + frontComponentOverrides: Partial = {}, +): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + frontComponents: [ + { + ...buildValidFrontComponent(), + ...frontComponentOverrides, + }, + ], + }, + }); + +type TestContext = { + manifest: Manifest; +}; + +const FAILING_UPDATE_TEST_CASES: EachTestingContext[] = [ + { + title: + 'when builtComponentPath is updated with path traversal via manifest sync', + context: { + manifest: buildManifest({ + builtComponentPath: + '../../../other-workspace/other-app/BuiltFrontComponent/stolen.mjs', + }), + }, + }, + { + title: + 'when sourceComponentPath is updated with path traversal via manifest sync', + context: { + manifest: buildManifest({ + sourceComponentPath: '../../etc/passwd', + }), + }, + }, + { + title: + 'when builtComponentPath is updated with absolute path via manifest sync', + context: { + manifest: buildManifest({ + builtComponentPath: '/etc/passwd', + }), + }, + }, + { + title: + 'when builtComponentPath is updated with backslash path via manifest sync', + context: { + manifest: buildManifest({ + builtComponentPath: '..\\..\\..\\etc\\passwd', + }), + }, + }, +]; + +describe('Front component update via manifest sync should fail for path traversal', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Path Traversal App', + description: 'App for testing path traversal validation on update', + sourcePath: 'test-path-traversal', + }); + + jest.useRealTimers(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: TEST_APP_ID, + fileFolder: 'BuiltFrontComponent', + filePath: VALID_BUILT_PATH, + fileBuffer: Buffer.from('dummy built component content'), + filename: 'test.mjs', + contentType: 'application/javascript', + expectToFail: false, + }); + + jest.useFakeTimers(); + + await syncApplication({ + manifest: buildManifest(), + expectToFail: false, + }); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it.each(eachTestingContextFilter(FAILING_UPDATE_TEST_CASES))( + '$title', + async ({ context }) => { + const { errors } = await syncApplication({ + manifest: context.manifest, + expectToFail: true, + }); + + expectOneNotInternalServerErrorSnapshot({ errors }); + }, + 60000, + ); +}); diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-creation.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-creation.integration-spec.ts.snap new file mode 100644 index 00000000000..760929e8ee7 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-creation.integration-spec.ts.snap @@ -0,0 +1,141 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Logic function creation via manifest sync should fail for path traversal when builtHandlerPath contains backslash path traversal 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Built handler path contains unsafe characters", + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function creation via manifest sync should fail for path traversal when builtHandlerPath contains path traversal 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Built handler path contains unsafe characters", + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function creation via manifest sync should fail for path traversal when builtHandlerPath is an absolute path 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Built handler path contains unsafe characters", + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function creation via manifest sync should fail for path traversal when sourceHandlerPath contains path traversal 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Source handler path contains unsafe characters", + "userFriendlyMessage": "Source handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "create", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Source handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-update-path-traversal.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-update-path-traversal.integration-spec.ts.snap new file mode 100644 index 00000000000..64ab0a14d91 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/__snapshots__/failing-logic-function-update-path-traversal.integration-spec.ts.snap @@ -0,0 +1,141 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Logic function update via manifest sync should fail for path traversal when builtHandlerPath is updated with absolute path via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Built handler path contains unsafe characters", + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function update via manifest sync should fail for path traversal when builtHandlerPath is updated with backslash path via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Built handler path contains unsafe characters", + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function update via manifest sync should fail for path traversal when builtHandlerPath is updated with path traversal via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Built handler path contains unsafe characters", + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Built handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; + +exports[`Logic function update via manifest sync should fail for path traversal when sourceHandlerPath is updated with path traversal via manifest sync 1`] = ` +{ + "extensions": { + "code": "METADATA_VALIDATION_FAILED", + "errors": { + "logicFunction": [ + { + "errors": [ + { + "code": "INVALID_LOGIC_FUNCTION_INPUT", + "message": "Source handler path contains unsafe characters", + "userFriendlyMessage": "Source handler path contains unsafe characters", + }, + ], + "flatEntityMinimalInformation": { + "universalIdentifier": Any, + }, + "metadataName": "logicFunction", + "status": "fail", + "type": "update", + }, + ], + }, + "message": "Validation failed for 1 logicFunction", + "summary": { + "logicFunction": 1, + "totalErrors": 1, + }, + "userFriendlyMessage": "Source handler path contains unsafe characters", + }, + "message": "Validation errors occurred while syncing application manifest metadata", + "name": "GraphQLError", +} +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-creation.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-creation.integration-spec.ts new file mode 100644 index 00000000000..2ae7d080cde --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-creation.integration-spec.ts @@ -0,0 +1,128 @@ +import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { + type LogicFunctionManifest, + type Manifest, +} from 'twenty-shared/application'; +import { + type EachTestingContext, + eachTestingContextFilter, +} from 'twenty-shared/testing'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); + +const VALID_BUILT_PATH = 'src/logic-functions/handler.mjs'; + +const buildLogicFunction = ( + overrides: Partial = {}, +): LogicFunctionManifest => ({ + universalIdentifier: uuidv4(), + name: 'TestLogicFunction', + description: 'A test logic function', + sourceHandlerPath: 'src/logic-functions/handler.ts', + builtHandlerPath: VALID_BUILT_PATH, + builtHandlerChecksum: 'valid-checksum', + handlerName: 'handler', + ...overrides, +}); + +const buildManifest = ( + logicFunctionOverrides: Partial = {}, +): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + logicFunctions: [buildLogicFunction(logicFunctionOverrides)], + }, + }); + +type TestContext = { + manifest: Manifest; +}; + +const FAILING_CREATION_TEST_CASES: EachTestingContext[] = [ + { + title: 'when builtHandlerPath contains path traversal', + context: { + manifest: buildManifest({ + builtHandlerPath: + '../../../other-workspace/other-app/built-logic-function/stolen.mjs', + }), + }, + }, + { + title: 'when sourceHandlerPath contains path traversal', + context: { + manifest: buildManifest({ + sourceHandlerPath: '../../etc/passwd', + }), + }, + }, + { + title: 'when builtHandlerPath is an absolute path', + context: { + manifest: buildManifest({ + builtHandlerPath: '/etc/passwd', + }), + }, + }, + { + title: 'when builtHandlerPath contains backslash path traversal', + context: { + manifest: buildManifest({ + builtHandlerPath: '..\\..\\..\\etc\\passwd', + }), + }, + }, +]; + +describe('Logic function creation via manifest sync should fail for path traversal', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Path Traversal Logic App', + description: 'App for testing path traversal validation on creation', + sourcePath: 'test-path-traversal-logic', + }); + + jest.useRealTimers(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: TEST_APP_ID, + fileFolder: 'BuiltLogicFunction', + filePath: VALID_BUILT_PATH, + fileBuffer: Buffer.from('dummy built handler content'), + filename: 'handler.mjs', + contentType: 'application/javascript', + expectToFail: false, + }); + + jest.useFakeTimers(); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it.each(eachTestingContextFilter(FAILING_CREATION_TEST_CASES))( + '$title', + async ({ context }) => { + const { errors } = await syncApplication({ + manifest: context.manifest, + expectToFail: true, + }); + + expectOneNotInternalServerErrorSnapshot({ errors }); + }, + 60000, + ); +}); diff --git a/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-update-path-traversal.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-update-path-traversal.integration-spec.ts new file mode 100644 index 00000000000..9e991fd06fb --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/logic-function/failing-logic-function-update-path-traversal.integration-spec.ts @@ -0,0 +1,141 @@ +import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { + type LogicFunctionManifest, + type Manifest, +} from 'twenty-shared/application'; +import { + type EachTestingContext, + eachTestingContextFilter, +} from 'twenty-shared/testing'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); +const LOGIC_FUNCTION_ID = uuidv4(); + +const VALID_BUILT_PATH = 'src/logic-functions/handler.mjs'; + +const buildValidLogicFunction = (): LogicFunctionManifest => ({ + universalIdentifier: LOGIC_FUNCTION_ID, + name: 'TestLogicFunction', + description: 'A test logic function', + sourceHandlerPath: 'src/logic-functions/handler.ts', + builtHandlerPath: VALID_BUILT_PATH, + builtHandlerChecksum: 'valid-checksum', + handlerName: 'handler', +}); + +const buildManifest = ( + logicFunctionOverrides: Partial = {}, +): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + logicFunctions: [ + { + ...buildValidLogicFunction(), + ...logicFunctionOverrides, + }, + ], + }, + }); + +type TestContext = { + manifest: Manifest; +}; + +const FAILING_UPDATE_TEST_CASES: EachTestingContext[] = [ + { + title: + 'when builtHandlerPath is updated with path traversal via manifest sync', + context: { + manifest: buildManifest({ + builtHandlerPath: + '../../../other-workspace/other-app/built-logic-function/stolen.mjs', + }), + }, + }, + { + title: + 'when sourceHandlerPath is updated with path traversal via manifest sync', + context: { + manifest: buildManifest({ + sourceHandlerPath: '../../etc/passwd', + }), + }, + }, + { + title: + 'when builtHandlerPath is updated with absolute path via manifest sync', + context: { + manifest: buildManifest({ + builtHandlerPath: '/etc/passwd', + }), + }, + }, + { + title: + 'when builtHandlerPath is updated with backslash path via manifest sync', + context: { + manifest: buildManifest({ + builtHandlerPath: '..\\..\\..\\etc\\passwd', + }), + }, + }, +]; + +describe('Logic function update via manifest sync should fail for path traversal', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Path Traversal Logic App', + description: + 'App for testing path traversal validation on logic function update', + sourcePath: 'test-path-traversal-logic', + }); + + jest.useRealTimers(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: TEST_APP_ID, + fileFolder: 'BuiltLogicFunction', + filePath: VALID_BUILT_PATH, + fileBuffer: Buffer.from('dummy built handler content'), + filename: 'handler.mjs', + contentType: 'application/javascript', + expectToFail: false, + }); + + jest.useFakeTimers(); + + await syncApplication({ + manifest: buildManifest(), + expectToFail: false, + }); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it.each(eachTestingContextFilter(FAILING_UPDATE_TEST_CASES))( + '$title', + async ({ context }) => { + const { errors } = await syncApplication({ + manifest: context.manifest, + expectToFail: true, + }); + + expectOneNotInternalServerErrorSnapshot({ errors }); + }, + 60000, + ); +});