From 2327ae7122ee6c327d3653c9c7a21ca0f8f7323c Mon Sep 17 00:00:00 2001 From: martmull Date: Mon, 6 Jul 2026 13:47:45 +0200 Subject: [PATCH] Revert "feat(server): add instance-level file storage layer" (#22579) Reverts twentyhq/twenty#22560 Review in cubic --- .../prefer-workspace-scoped-repository.ts | 1 - ...t-1783240670564-add-instance-file-table.ts | 19 - ...ile-table-upgrade-command-name.constant.ts | 2 - .../instance-commands.constant.ts | 2 - .../instance-file-storage.service.spec.ts | 422 ------------------ .../instance-file-storage-prefix.constant.ts | 1 - .../file-storage/file-storage.module.ts | 15 +- .../instance-file-storage.service.ts | 233 ---------- ...ithin-instance-scope-or-throw.util.spec.ts | 79 ---- .../utils/validate-file-extension.util.ts | 4 +- .../utils/validate-file-path.util.ts | 4 +- ...-is-within-instance-scope-or-throw.util.ts | 32 -- .../file/entities/instance-file.entity.ts | 62 --- .../src/types/InstanceFileFolder.ts | 3 - packages/twenty-shared/src/types/index.ts | 1 - 15 files changed, 6 insertions(+), 874 deletions(-) delete mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/2-19/2-19-instance-command-fast-1783240670564-add-instance-file-table.ts delete mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/2-19/add-instance-file-table-upgrade-command-name.constant.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/__tests__/instance-file-storage.service.spec.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/constants/instance-file-storage-prefix.constant.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/instance-file-storage.service.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/validate-storage-path-is-within-instance-scope-or-throw.util.spec.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-storage-path-is-within-instance-scope-or-throw.util.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file/entities/instance-file.entity.ts delete mode 100644 packages/twenty-shared/src/types/InstanceFileFolder.ts diff --git a/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts b/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts index e51675f5b3..b293a02ab4 100644 --- a/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts +++ b/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts @@ -23,7 +23,6 @@ const STRUCTURAL_EXEMPTIONS = new Set([ 'ConnectedAccountEntity', 'ConnectionProviderEntity', 'FrontComponentEntity', - 'InstanceFileEntity', 'LogicFunctionEntity', 'MessageFolderEntity', 'RolePermissionFlagEntity', diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-19/2-19-instance-command-fast-1783240670564-add-instance-file-table.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-19/2-19-instance-command-fast-1783240670564-add-instance-file-table.ts deleted file mode 100644 index 8cd608bc3c..0000000000 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-19/2-19-instance-command-fast-1783240670564-add-instance-file-table.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { QueryRunner } from 'typeorm'; - -import { RegisteredInstanceCommand } from 'src/engine/core-modules/upgrade/decorators/registered-instance-command.decorator'; -import { FastInstanceCommand } from 'src/engine/core-modules/upgrade/interfaces/fast-instance-command.interface'; - -@RegisteredInstanceCommand('2.19.0', 1783240670564) -export class AddInstanceFileTableFastInstanceCommand implements FastInstanceCommand { - public async up(queryRunner: QueryRunner): Promise { - await queryRunner.query('CREATE TABLE "core"."instanceFile" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "path" text NOT NULL, "size" bigint NOT NULL, "mimeType" character varying NOT NULL DEFAULT \'application/octet-stream\', "applicationRegistrationId" uuid, "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "deletedAt" TIMESTAMP WITH TIME ZONE, CONSTRAINT "IDX_INSTANCE_FILE_PATH_UNIQUE" UNIQUE ("path"), CONSTRAINT "PK_3d753f7415af93f4dfbd92d58ca" PRIMARY KEY ("id"))'); - await queryRunner.query('CREATE INDEX "IDX_INSTANCE_FILE_APPLICATION_REGISTRATION_ID" ON "core"."instanceFile" ("applicationRegistrationId") '); - await queryRunner.query('ALTER TABLE "core"."instanceFile" ADD CONSTRAINT "FK_19422e6d5c43d71b516c10fe755" FOREIGN KEY ("applicationRegistrationId") REFERENCES "core"."applicationRegistration"("id") ON DELETE CASCADE ON UPDATE NO ACTION'); - } - - public async down(queryRunner: QueryRunner): Promise { - await queryRunner.query('ALTER TABLE "core"."instanceFile" DROP CONSTRAINT "FK_19422e6d5c43d71b516c10fe755"'); - await queryRunner.query('DROP INDEX "core"."IDX_INSTANCE_FILE_APPLICATION_REGISTRATION_ID"'); - await queryRunner.query('DROP TABLE "core"."instanceFile"'); - } -} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-19/add-instance-file-table-upgrade-command-name.constant.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-19/add-instance-file-table-upgrade-command-name.constant.ts deleted file mode 100644 index 275fbedb72..0000000000 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/2-19/add-instance-file-table-upgrade-command-name.constant.ts +++ /dev/null @@ -1,2 +0,0 @@ -export const ADD_INSTANCE_FILE_TABLE_UPGRADE_COMMAND_NAME = - '2.19.0_AddInstanceFileTableFastInstanceCommand_1783240670564'; diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts index 42a8202a51..cc5d934d7b 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts @@ -98,7 +98,6 @@ import { AddLogoToApplicationRegistrationFastInstanceCommand } from './2-19/2-19 import { BackfillLogoOnApplicationRegistrationSlowInstanceCommand } from './2-19/2-19-instance-command-slow-1783069673191-backfill-logo-on-application-registration'; import { AddDisplayFieldsToApplicationRegistrationFastInstanceCommand } from './2-19/2-19-instance-command-fast-1783073776590-add-display-fields-to-application-registration'; import { BackfillDisplayFieldsOnApplicationRegistrationSlowInstanceCommand } from './2-19/2-19-instance-command-slow-1783073776591-backfill-display-fields-on-application-registration'; -import { AddInstanceFileTableFastInstanceCommand } from 'src/database/commands/upgrade-version-command/2-19/2-19-instance-command-fast-1783240670564-add-instance-file-table'; export const INSTANCE_COMMANDS = [ AddViewFieldGroupIdIndexOnViewFieldFastInstanceCommand, @@ -199,5 +198,4 @@ export const INSTANCE_COMMANDS = [ BackfillDisplayFieldsOnApplicationRegistrationSlowInstanceCommand, AddStatusToFileFastInstanceCommand, AddPendingMimeCheckToFileFastInstanceCommand, - AddInstanceFileTableFastInstanceCommand, ]; diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/instance-file-storage.service.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/instance-file-storage.service.spec.ts deleted file mode 100644 index a4849ce143..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/instance-file-storage.service.spec.ts +++ /dev/null @@ -1,422 +0,0 @@ -import { InstanceFileFolder } from 'twenty-shared/types'; -import { Test, type TestingModule } from '@nestjs/testing'; -import { getRepositoryToken } from '@nestjs/typeorm'; - -import { Readable } from 'stream'; - -import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory'; -import { InstanceFileStorageService } from 'src/engine/core-modules/file-storage/instance-file-storage.service'; -import { - FileStorageException, - FileStorageExceptionCode, -} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; -import { InstanceFileEntity } from 'src/engine/core-modules/file/entities/instance-file.entity'; - -describe('InstanceFileStorageService', () => { - let service: InstanceFileStorageService; - - const mockFileStorageDriverFactory = { - getCurrentDriver: jest.fn(), - }; - - const mockInstanceFileRepository = { - upsert: jest.fn(), - findOneBy: jest.fn(), - findOneByOrFail: jest.fn(), - findBy: jest.fn(), - delete: jest.fn(), - }; - - const mockDriver = { - writeFile: jest.fn(), - readFile: jest.fn(), - delete: jest.fn(), - checkFileExists: jest.fn(), - }; - - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [ - InstanceFileStorageService, - { - provide: FileStorageDriverFactory, - useValue: mockFileStorageDriverFactory, - }, - { - provide: getRepositoryToken(InstanceFileEntity), - useValue: mockInstanceFileRepository, - }, - ], - }).compile(); - - service = module.get( - InstanceFileStorageService, - ); - - jest.clearAllMocks(); - - mockFileStorageDriverFactory.getCurrentDriver.mockReturnValue(mockDriver); - }); - - describe.each([ - [ - 'readInstanceFile', - (resourcePath: string) => - service.readInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath, - }), - ], - [ - 'checkInstanceFileExists', - (resourcePath: string) => - service.checkInstanceFileExists({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath, - }), - ], - [ - 'deleteInstanceFile', - (resourcePath: string) => - service.deleteInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath, - }), - ], - ] as const)('%s traversal protection', (_methodName, invoke) => { - it.each(['../workspace-id/stolen.json', 'a/../../escape.json'])( - 'should reject traversal resource path %s without touching storage', - async (resourcePath) => { - await expect( - (async () => { - await invoke(resourcePath); - })(), - ).rejects.toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.ACCESS_DENIED, - }), - ); - - expect(mockDriver.readFile).not.toHaveBeenCalled(); - expect(mockDriver.checkFileExists).not.toHaveBeenCalled(); - expect(mockDriver.delete).not.toHaveBeenCalled(); - }, - ); - }); - - describe('writeInstanceFile', () => { - it('should write bytes with the instance prefix and upsert the row on path conflict', async () => { - const instanceFile = { - id: 'instance-file-id', - path: 'application-registration/manifests/manifest.json', - } as InstanceFileEntity; - - mockInstanceFileRepository.findOneByOrFail.mockResolvedValue( - instanceFile, - ); - - const result = await service.writeInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifests/manifest.json', - contents: '{"name":"my-app"}', - mimeType: 'application/json', - applicationRegistrationId: 'registration-id', - }); - - expect(mockDriver.writeFile).toHaveBeenCalledWith({ - filePath: 'instance/application-registration/manifests/manifest.json', - mimeType: 'application/json', - sourceFile: '{"name":"my-app"}', - }); - expect(mockInstanceFileRepository.upsert).toHaveBeenCalledWith( - { - path: 'application-registration/manifests/manifest.json', - size: Buffer.byteLength('{"name":"my-app"}'), - mimeType: 'application/json', - applicationRegistrationId: 'registration-id', - }, - { conflictPaths: ['path'] }, - ); - expect(result).toEqual(instanceFile); - }); - - it('should store a null applicationRegistrationId when none is provided', async () => { - mockInstanceFileRepository.findOneByOrFail.mockResolvedValue( - {} as InstanceFileEntity, - ); - - await service.writeInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifest.json', - contents: Buffer.from('{}'), - mimeType: 'application/json', - }); - - expect(mockInstanceFileRepository.upsert).toHaveBeenCalledWith( - expect.objectContaining({ applicationRegistrationId: null, size: 2 }), - { conflictPaths: ['path'] }, - ); - }); - - it('should reject a traversal resource path without touching storage', async () => { - await expect( - service.writeInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: '../workspace-id/stolen.json', - contents: '{}', - mimeType: 'application/json', - }), - ).rejects.toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.ACCESS_DENIED, - }), - ); - - expect(mockDriver.writeFile).not.toHaveBeenCalled(); - expect(mockInstanceFileRepository.upsert).not.toHaveBeenCalled(); - }); - - it('should propagate driver write failures without upserting the row', async () => { - mockDriver.writeFile.mockRejectedValueOnce(new Error('Write failed')); - - await expect( - service.writeInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifest.json', - contents: '{}', - mimeType: 'application/json', - }), - ).rejects.toThrow('Write failed'); - - expect(mockInstanceFileRepository.upsert).not.toHaveBeenCalled(); - }); - }); - - describe('readInstanceFile', () => { - it('should read from the instance-prefixed storage path', async () => { - const stream = Readable.from(['{}']); - - mockInstanceFileRepository.findOneBy.mockResolvedValue({ - id: 'instance-file-id', - path: 'application-registration/manifests/manifest.json', - } as InstanceFileEntity); - mockDriver.readFile.mockResolvedValue(stream); - - const result = await service.readInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifests/manifest.json', - }); - - expect(mockDriver.readFile).toHaveBeenCalledWith({ - filePath: 'instance/application-registration/manifests/manifest.json', - }); - expect(result).toBe(stream); - }); - - it('should propagate the missing-file exception from the driver', async () => { - mockInstanceFileRepository.findOneBy.mockResolvedValue({ - id: 'instance-file-id', - path: 'application-registration/missing.json', - } as InstanceFileEntity); - mockDriver.readFile.mockRejectedValueOnce( - new FileStorageException( - 'File not found', - FileStorageExceptionCode.FILE_NOT_FOUND, - ), - ); - - await expect( - service.readInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'missing.json', - }), - ).rejects.toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.FILE_NOT_FOUND, - }), - ); - }); - - it('should throw FILE_NOT_FOUND without reading bytes when the row is missing', async () => { - mockInstanceFileRepository.findOneBy.mockResolvedValue(null); - - await expect( - service.readInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'deleted.json', - }), - ).rejects.toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.FILE_NOT_FOUND, - }), - ); - - expect(mockDriver.readFile).not.toHaveBeenCalled(); - }); - }); - - describe('readInstanceFileById', () => { - it('should read the bytes of the row storage path', async () => { - const stream = Readable.from(['{}']); - - mockInstanceFileRepository.findOneBy.mockResolvedValue({ - id: 'instance-file-id', - path: 'application-registration/manifest.json', - } as InstanceFileEntity); - mockDriver.readFile.mockResolvedValue(stream); - - const result = await service.readInstanceFileById('instance-file-id'); - - expect(mockInstanceFileRepository.findOneBy).toHaveBeenCalledWith({ - id: 'instance-file-id', - }); - expect(mockDriver.readFile).toHaveBeenCalledWith({ - filePath: 'instance/application-registration/manifest.json', - }); - expect(result).toBe(stream); - }); - - it('should throw a missing-file exception when the row does not exist', async () => { - mockInstanceFileRepository.findOneBy.mockResolvedValue(null); - - await expect(service.readInstanceFileById('unknown-id')).rejects.toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.FILE_NOT_FOUND, - }), - ); - - expect(mockDriver.readFile).not.toHaveBeenCalled(); - }); - }); - - describe('checkInstanceFileExists', () => { - it('should check existence on the instance-prefixed storage path', async () => { - mockDriver.checkFileExists.mockResolvedValue(true); - - const result = await service.checkInstanceFileExists({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifest.json', - }); - - expect(mockDriver.checkFileExists).toHaveBeenCalledWith({ - filePath: 'instance/application-registration/manifest.json', - }); - expect(result).toBe(true); - }); - }); - - describe('deleteInstanceFile', () => { - it('should delete the bytes and the row', async () => { - await service.deleteInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifests/manifest.json', - }); - - expect(mockDriver.delete).toHaveBeenCalledWith({ - folderPath: 'instance/application-registration/manifests', - filename: 'manifest.json', - }); - expect(mockInstanceFileRepository.delete).toHaveBeenCalledWith({ - path: 'application-registration/manifests/manifest.json', - }); - }); - - it('should still delete the row when the bytes deletion fails', async () => { - mockDriver.delete.mockRejectedValueOnce(new Error('Delete failed')); - - await service.deleteInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifest.json', - }); - - expect(mockInstanceFileRepository.delete).toHaveBeenCalledWith({ - path: 'application-registration/manifest.json', - }); - }); - - it('should propagate row deletion failures', async () => { - mockInstanceFileRepository.delete.mockRejectedValueOnce( - new Error('Row deletion failed'), - ); - - await expect( - service.deleteInstanceFile({ - fileFolder: InstanceFileFolder.ApplicationRegistration, - resourcePath: 'manifest.json', - }), - ).rejects.toThrow('Row deletion failed'); - }); - }); - - describe('deleteByInstanceFileId', () => { - it('should delete the bytes and the row of the given id', async () => { - mockInstanceFileRepository.findOneBy.mockResolvedValue({ - id: 'instance-file-id', - path: 'application-registration/manifest.json', - } as InstanceFileEntity); - - await service.deleteByInstanceFileId('instance-file-id'); - - expect(mockDriver.delete).toHaveBeenCalledWith({ - folderPath: 'instance/application-registration', - filename: 'manifest.json', - }); - expect(mockInstanceFileRepository.delete).toHaveBeenCalledWith({ - id: 'instance-file-id', - }); - }); - - it('should throw a missing-file exception when the row does not exist', async () => { - mockInstanceFileRepository.findOneBy.mockResolvedValue(null); - - await expect( - service.deleteByInstanceFileId('unknown-id'), - ).rejects.toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.FILE_NOT_FOUND, - }), - ); - - expect(mockInstanceFileRepository.delete).not.toHaveBeenCalled(); - }); - }); - - describe('deleteByApplicationRegistrationId', () => { - it('should delete the bytes of every file then the rows', async () => { - mockInstanceFileRepository.findBy.mockResolvedValue([ - { id: 'file-1', path: 'application-registration/manifest.json' }, - { id: 'file-2', path: 'application-registration/nested/settings.json' }, - ] as InstanceFileEntity[]); - - await service.deleteByApplicationRegistrationId('registration-id'); - - expect(mockInstanceFileRepository.findBy).toHaveBeenCalledWith({ - applicationRegistrationId: 'registration-id', - }); - expect(mockDriver.delete).toHaveBeenCalledWith({ - folderPath: 'instance/application-registration', - filename: 'manifest.json', - }); - expect(mockDriver.delete).toHaveBeenCalledWith({ - folderPath: 'instance/application-registration/nested', - filename: 'settings.json', - }); - expect(mockInstanceFileRepository.delete).toHaveBeenCalledWith({ - applicationRegistrationId: 'registration-id', - }); - }); - - it('should still delete the rows when a bytes deletion fails', async () => { - mockInstanceFileRepository.findBy.mockResolvedValue([ - { id: 'file-1', path: 'application-registration/manifest.json' }, - ] as InstanceFileEntity[]); - mockDriver.delete.mockRejectedValueOnce(new Error('Delete failed')); - - await service.deleteByApplicationRegistrationId('registration-id'); - - expect(mockInstanceFileRepository.delete).toHaveBeenCalledWith({ - applicationRegistrationId: 'registration-id', - }); - }); - }); -}); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/constants/instance-file-storage-prefix.constant.ts b/packages/twenty-server/src/engine/core-modules/file-storage/constants/instance-file-storage-prefix.constant.ts deleted file mode 100644 index 6c901f6799..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file-storage/constants/instance-file-storage-prefix.constant.ts +++ /dev/null @@ -1 +0,0 @@ -export const INSTANCE_FILE_STORAGE_PREFIX = 'instance'; diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.module.ts b/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.module.ts index e77f9e7121..ee7d9a5dc3 100644 --- a/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.module.ts +++ b/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.module.ts @@ -6,9 +6,7 @@ import { ApplicationEntity } from 'src/engine/core-modules/application/applicati import { FileStorageExceptionFilter } from 'src/engine/core-modules/file-storage/file-storage-exception-filter'; 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 { InstanceFileStorageService } from 'src/engine/core-modules/file-storage/instance-file-storage.service'; import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity'; -import { InstanceFileEntity } from 'src/engine/core-modules/file/entities/instance-file.entity'; import { TwentyConfigModule } from 'src/engine/core-modules/twenty-config/twenty-config.module'; import { provideWorkspaceScopedRepository } from 'src/engine/twenty-orm/workspace-scoped-repository/provide-workspace-scoped-repository'; @Global() @@ -18,27 +16,18 @@ export class FileStorageModule { module: FileStorageModule, imports: [ TwentyConfigModule, - TypeOrmModule.forFeature([ - FileEntity, - InstanceFileEntity, - ApplicationEntity, - ]), + TypeOrmModule.forFeature([FileEntity, ApplicationEntity]), ], providers: [ FileStorageDriverFactory, FileStorageService, - InstanceFileStorageService, provideWorkspaceScopedRepository(FileEntity), { provide: APP_FILTER, useClass: FileStorageExceptionFilter, }, ], - exports: [ - FileStorageDriverFactory, - FileStorageService, - InstanceFileStorageService, - ], + exports: [FileStorageDriverFactory, FileStorageService], }; } } diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/instance-file-storage.service.ts b/packages/twenty-server/src/engine/core-modules/file-storage/instance-file-storage.service.ts deleted file mode 100644 index ecda1c9e80..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file-storage/instance-file-storage.service.ts +++ /dev/null @@ -1,233 +0,0 @@ -import { Injectable, Logger } from '@nestjs/common'; -import { InjectRepository } from '@nestjs/typeorm'; - -import { basename, dirname, join } from 'path'; -import { type Readable } from 'stream'; - -import { type InstanceFileFolder } from 'twenty-shared/types'; -import { isDefined } from 'twenty-shared/utils'; -import { Repository } from 'typeorm'; - -import { INSTANCE_FILE_STORAGE_PREFIX } from 'src/engine/core-modules/file-storage/constants/instance-file-storage-prefix.constant'; -import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory'; -import { - FileStorageException, - FileStorageExceptionCode, -} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; -import { validateFilePath } from 'src/engine/core-modules/file-storage/utils/validate-file-path.util'; -import { validateStoragePathIsWithinInstanceScopeOrThrow } from 'src/engine/core-modules/file-storage/utils/validate-storage-path-is-within-instance-scope-or-throw.util'; -import { InstanceFileEntity } from 'src/engine/core-modules/file/entities/instance-file.entity'; - -export type InstanceResourceIdentifier = { - fileFolder: InstanceFileFolder; - resourcePath: string; -}; - -@Injectable() -export class InstanceFileStorageService { - private readonly logger = new Logger(InstanceFileStorageService.name); - - constructor( - private readonly fileStorageDriverFactory: FileStorageDriverFactory, - @InjectRepository(InstanceFileEntity) - private readonly instanceFileRepository: Repository, - ) {} - - private validateAndBuildInstanceFileStoragePathOrThrow({ - fileFolder, - resourcePath, - }: InstanceResourceIdentifier): { - onStorageFilePath: string; - filePath: string; - } { - const validationResult = validateFilePath({ resourcePath, fileFolder }); - - if (!validationResult.isValid) { - throw new FileStorageException( - validationResult.error, - FileStorageExceptionCode.ACCESS_DENIED, - ); - } - - const filePath = join(fileFolder, resourcePath).replace(/\/+/g, '/'); - - const onStorageFilePath = join( - INSTANCE_FILE_STORAGE_PREFIX, - filePath, - ).replace(/\/+/g, '/'); - - validateStoragePathIsWithinInstanceScopeOrThrow({ - onStoragePath: onStorageFilePath, - fileFolder, - }); - - return { onStorageFilePath, filePath }; - } - - async writeInstanceFile({ - fileFolder, - resourcePath, - contents, - mimeType, - applicationRegistrationId, - }: InstanceResourceIdentifier & { - contents: Buffer | string; - mimeType: string; - applicationRegistrationId?: string; - }): Promise { - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - const { onStorageFilePath, filePath } = - this.validateAndBuildInstanceFileStoragePathOrThrow({ - fileFolder, - resourcePath, - }); - - await driver.writeFile({ - filePath: onStorageFilePath, - mimeType, - sourceFile: contents, - }); - - await this.instanceFileRepository.upsert( - { - path: filePath, - size: - typeof contents === 'string' - ? Buffer.byteLength(contents) - : contents.length, - mimeType, - applicationRegistrationId: applicationRegistrationId ?? null, - }, - { conflictPaths: ['path'] }, - ); - - return this.instanceFileRepository.findOneByOrFail({ path: filePath }); - } - - async readInstanceFile({ - fileFolder, - resourcePath, - }: InstanceResourceIdentifier): Promise { - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - const { onStorageFilePath, filePath } = - this.validateAndBuildInstanceFileStoragePathOrThrow({ - fileFolder, - resourcePath, - }); - - const instanceFile = await this.instanceFileRepository.findOneBy({ - path: filePath, - }); - - if (!isDefined(instanceFile)) { - throw new FileStorageException( - `Instance file ${filePath} not found`, - FileStorageExceptionCode.FILE_NOT_FOUND, - ); - } - - return driver.readFile({ filePath: onStorageFilePath }); - } - - async readInstanceFileById(id: string): Promise { - const instanceFile = await this.findInstanceFileByIdOrThrow(id); - - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - return driver.readFile({ - filePath: this.buildOnStorageFilePath(instanceFile), - }); - } - - checkInstanceFileExists({ - fileFolder, - resourcePath, - }: InstanceResourceIdentifier): Promise { - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - const { onStorageFilePath } = - this.validateAndBuildInstanceFileStoragePathOrThrow({ - fileFolder, - resourcePath, - }); - - return driver.checkFileExists({ filePath: onStorageFilePath }); - } - - async deleteInstanceFile({ - fileFolder, - resourcePath, - }: InstanceResourceIdentifier): Promise { - const { onStorageFilePath, filePath } = - this.validateAndBuildInstanceFileStoragePathOrThrow({ - fileFolder, - resourcePath, - }); - - await this.deleteBytesBestEffort(onStorageFilePath); - - await this.instanceFileRepository.delete({ path: filePath }); - } - - async deleteByInstanceFileId(id: string): Promise { - const instanceFile = await this.findInstanceFileByIdOrThrow(id); - - await this.deleteBytesBestEffort(this.buildOnStorageFilePath(instanceFile)); - - await this.instanceFileRepository.delete({ id }); - } - - async deleteByApplicationRegistrationId( - applicationRegistrationId: string, - ): Promise { - const instanceFiles = await this.instanceFileRepository.findBy({ - applicationRegistrationId, - }); - - for (const instanceFile of instanceFiles) { - await this.deleteBytesBestEffort( - this.buildOnStorageFilePath(instanceFile), - ); - } - - await this.instanceFileRepository.delete({ applicationRegistrationId }); - } - - private async findInstanceFileByIdOrThrow( - id: string, - ): Promise { - const instanceFile = await this.instanceFileRepository.findOneBy({ id }); - - if (!isDefined(instanceFile)) { - throw new FileStorageException( - `Instance file ${id} not found`, - FileStorageExceptionCode.FILE_NOT_FOUND, - ); - } - - return instanceFile; - } - - private buildOnStorageFilePath(instanceFile: InstanceFileEntity): string { - return join(INSTANCE_FILE_STORAGE_PREFIX, instanceFile.path); - } - - private async deleteBytesBestEffort( - onStorageFilePath: string, - ): Promise { - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - try { - await driver.delete({ - folderPath: dirname(onStorageFilePath), - filename: basename(onStorageFilePath), - }); - } catch (error) { - this.logger.warn( - `Failed to delete instance file bytes at ${onStorageFilePath}: ${error}`, - ); - } - } -} diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/validate-storage-path-is-within-instance-scope-or-throw.util.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/validate-storage-path-is-within-instance-scope-or-throw.util.spec.ts deleted file mode 100644 index b691ef3c0e..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file-storage/utils/__tests__/validate-storage-path-is-within-instance-scope-or-throw.util.spec.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { InstanceFileFolder } from 'twenty-shared/types'; -import { FileStorageExceptionCode } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; - -import { validateStoragePathIsWithinInstanceScopeOrThrow } from 'src/engine/core-modules/file-storage/utils/validate-storage-path-is-within-instance-scope-or-throw.util'; - -const primitives = { - fileFolder: InstanceFileFolder.ApplicationRegistration, -} as const; - -describe('validateStoragePathIsWithinInstanceScopeOrThrow', () => { - it.each([ - { - title: 'nested path within prefix', - onStoragePath: - 'instance/application-registration/manifests/manifest.json', - }, - { - title: 'file directly under prefix', - onStoragePath: 'instance/application-registration/manifest.json', - }, - ])('should accept valid path: $title', ({ onStoragePath }) => { - expect(() => - validateStoragePathIsWithinInstanceScopeOrThrow({ - onStoragePath, - ...primitives, - }), - ).not.toThrow(); - }); - - it.each([ - { - title: 'workspace-like prefix instead of instance prefix', - onStoragePath: 'workspace-id/app-uid/source/file.json', - }, - { - title: 'different file folder', - onStoragePath: 'instance/other-folder/file.json', - }, - { - title: 'prefix without trailing file', - onStoragePath: 'instance/application-registration', - }, - { - title: 'partial prefix match (malicious suffix)', - onStoragePath: 'instance/application-registrationMalicious/file.json', - }, - { - title: 'traversal out of the instance prefix', - onStoragePath: - 'instance/application-registration/../../workspace-id/file.json', - }, - { - title: 'traversal segments kept after normalization', - onStoragePath: 'instance/application-registration/../../../etc/passwd', - }, - { - title: 'absolute path', - onStoragePath: '/instance/application-registration/file.json', - }, - { - title: 'null byte in path', - onStoragePath: 'instance/application-registration/file\0.json', - }, - ])( - 'should reject path that escapes instance scope: $title', - ({ onStoragePath }) => { - expect(() => - validateStoragePathIsWithinInstanceScopeOrThrow({ - onStoragePath, - ...primitives, - }), - ).toThrow( - expect.objectContaining({ - code: FileStorageExceptionCode.ACCESS_DENIED, - }), - ); - }, - ); -}); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-extension.util.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-extension.util.ts index 323b93dffd..b3a2977834 100644 --- a/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-extension.util.ts +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-extension.util.ts @@ -1,5 +1,5 @@ import { t } from '@lingui/core/macro'; -import { type FileFolder, type InstanceFileFolder } from 'twenty-shared/types'; +import { type FileFolder } from 'twenty-shared/types'; import { ALLOWED_EXTENSIONS_BY_APPLICATION_FILE_FOLDER } from 'src/engine/core-modules/file-storage/constants/allowed-extensions-by-application-file-folder.constant'; import { type ResourcePathValidationResult } from 'src/engine/core-modules/file-storage/types/resource-path-validation-result.type'; @@ -10,7 +10,7 @@ export const validateFileExtension = ({ fileFolder, }: { resourcePath: string; - fileFolder: FileFolder | InstanceFileFolder; + fileFolder: FileFolder; }): ResourcePathValidationResult => { const allowedExtensions = ALLOWED_EXTENSIONS_BY_APPLICATION_FILE_FOLDER[ diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-path.util.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-path.util.ts index 3040e6facf..12b4bc1b43 100644 --- a/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-path.util.ts +++ b/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-file-path.util.ts @@ -1,5 +1,5 @@ import { t } from '@lingui/core/macro'; -import { type FileFolder, type InstanceFileFolder } from 'twenty-shared/types'; +import { type FileFolder } from 'twenty-shared/types'; import { type ResourcePathValidationResult } from 'src/engine/core-modules/file-storage/types/resource-path-validation-result.type'; import { validateFileExtension } from 'src/engine/core-modules/file-storage/utils/validate-file-extension.util'; @@ -11,7 +11,7 @@ export const validateFilePath = ({ fileFolder, }: { resourcePath: string; - fileFolder: FileFolder | InstanceFileFolder; + fileFolder: FileFolder; }): ResourcePathValidationResult => { const safePathResult = validateSafeRelativePath({ resourcePath }); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-storage-path-is-within-instance-scope-or-throw.util.ts b/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-storage-path-is-within-instance-scope-or-throw.util.ts deleted file mode 100644 index c0f641cfe4..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file-storage/utils/validate-storage-path-is-within-instance-scope-or-throw.util.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { join, normalize } from 'path'; - -import { type InstanceFileFolder } from 'twenty-shared/types'; - -import { - FileStorageException, - FileStorageExceptionCode, -} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; -import { assertStoragePathIsSafe } from 'src/engine/core-modules/file-storage/utils/assert-storage-path-is-safe.util'; -import { INSTANCE_FILE_STORAGE_PREFIX } from 'src/engine/core-modules/file-storage/constants/instance-file-storage-prefix.constant'; - -export const validateStoragePathIsWithinInstanceScopeOrThrow = ({ - onStoragePath, - fileFolder, -}: { - onStoragePath: string; - fileFolder: InstanceFileFolder; -}): void => { - assertStoragePathIsSafe(onStoragePath); - - const expectedPrefix = join(INSTANCE_FILE_STORAGE_PREFIX, fileFolder); - - const normalizedPath = normalize(onStoragePath); - const normalizedPrefix = normalize(expectedPrefix + '/'); - - if (!normalizedPath.startsWith(normalizedPrefix)) { - throw new FileStorageException( - 'Invalid storage path: resolved path escapes the instance scope', - FileStorageExceptionCode.ACCESS_DENIED, - ); - } -}; diff --git a/packages/twenty-server/src/engine/core-modules/file/entities/instance-file.entity.ts b/packages/twenty-server/src/engine/core-modules/file/entities/instance-file.entity.ts deleted file mode 100644 index 6482100feb..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file/entities/instance-file.entity.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { - Column, - CreateDateColumn, - DeleteDateColumn, - Entity, - Index, - JoinColumn, - ManyToOne, - PrimaryGeneratedColumn, - type Relation, - Unique, - UpdateDateColumn, -} from 'typeorm'; - -import { ADD_INSTANCE_FILE_TABLE_UPGRADE_COMMAND_NAME } from 'src/database/commands/upgrade-version-command/2-19/add-instance-file-table-upgrade-command-name.constant'; -import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity'; -import { WasIntroducedInUpgrade } from 'src/engine/core-modules/upgrade/decorators/was-introduced-in-upgrade.decorator'; - -@WasIntroducedInUpgrade({ - upgradeCommandName: ADD_INSTANCE_FILE_TABLE_UPGRADE_COMMAND_NAME, -}) -@Entity('instanceFile') -@Index('IDX_INSTANCE_FILE_APPLICATION_REGISTRATION_ID', [ - 'applicationRegistrationId', -]) -@Unique('IDX_INSTANCE_FILE_PATH_UNIQUE', ['path']) -export class InstanceFileEntity { - @PrimaryGeneratedColumn('uuid') - id: string; - - @Column({ nullable: false, type: 'text' }) - path: string; - - @Column({ nullable: false, type: 'bigint' }) - size: number; - - @Column({ - nullable: false, - type: 'varchar', - default: 'application/octet-stream', - }) - mimeType: string; - - @Column({ nullable: true, type: 'uuid' }) - applicationRegistrationId: string | null; - - @ManyToOne(() => ApplicationRegistrationEntity, { - onDelete: 'CASCADE', - nullable: true, - }) - @JoinColumn({ name: 'applicationRegistrationId' }) - applicationRegistration: Relation | null; - - @CreateDateColumn({ type: 'timestamptz' }) - createdAt: Date; - - @UpdateDateColumn({ type: 'timestamptz' }) - updatedAt: Date; - - @DeleteDateColumn({ type: 'timestamptz' }) - deletedAt: Date | null; -} diff --git a/packages/twenty-shared/src/types/InstanceFileFolder.ts b/packages/twenty-shared/src/types/InstanceFileFolder.ts deleted file mode 100644 index 047b422945..0000000000 --- a/packages/twenty-shared/src/types/InstanceFileFolder.ts +++ /dev/null @@ -1,3 +0,0 @@ -export enum InstanceFileFolder { - ApplicationRegistration = 'application-registration', -} diff --git a/packages/twenty-shared/src/types/index.ts b/packages/twenty-shared/src/types/index.ts index 2b0f955aa1..994260fec6 100644 --- a/packages/twenty-shared/src/types/index.ts +++ b/packages/twenty-shared/src/types/index.ts @@ -132,7 +132,6 @@ export type { FormatRecordSerializedRelationProperties } from './FormatRecordSer export type { FromTo } from './FromToType'; export { HTTPMethod } from './HttpMethod'; export type { IndexOf } from './IndexOf.type'; -export { InstanceFileFolder } from './InstanceFileFolder'; export type { IsEmptyObject } from './IsEmptyObject.type'; export type { IsEmptyRecord } from './IsEmptyRecord.type'; export type { IsExactly } from './IsExactly';