diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts index a65a4e221f..a114bfa3ab 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts @@ -61,8 +61,8 @@ describe('FileController', () => { provide: FileService, useValue: { getFileStreamById: jest.fn(), - getFileStreamByPath: jest.fn(), - getFileResponseById: jest.fn(), + getFilePresignedUrlOrStreamByPath: jest.fn(), + getFilePresignedUrlOrStreamById: jest.fn(), }, }, ], @@ -90,10 +90,12 @@ describe('FileController', () => { describe('getFileById', () => { it('should 302 redirect when presigned URL is available', async () => { - jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({ - type: 'redirect', - presignedUrl: 'https://s3.example.com/file?signed=abc', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') + .mockResolvedValue({ + type: 'redirect', + presignedUrl: 'https://s3.example.com/file?signed=abc', + }); const mockRequest = { workspaceId: 'workspace-id' } as any; const mockResponse = createMockResponse() as any; @@ -105,7 +107,7 @@ describe('FileController', () => { 'file-123', ); - expect(fileService.getFileResponseById).toHaveBeenCalledWith({ + expect(fileService.getFilePresignedUrlOrStreamById).toHaveBeenCalledWith({ fileId: 'file-123', workspaceId: 'workspace-id', fileFolder: FileFolder.Workflow, @@ -119,11 +121,13 @@ describe('FileController', () => { it('should stream with headers when no presigned URL (local driver)', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({ - type: 'stream', - stream: mockStream, - mimeType: 'image/png', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/png', + }); const mockRequest = { workspaceId: 'workspace-id' } as any; const mockResponse = createMockResponse() as any; @@ -153,11 +157,13 @@ describe('FileController', () => { it('should force attachment disposition for non-safe MIME types', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({ - type: 'stream', - stream: mockStream, - mimeType: 'text/html', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'text/html', + }); const mockRequest = { workspaceId: 'workspace-id' } as any; const mockResponse = createMockResponse() as any; @@ -180,7 +186,9 @@ describe('FileController', () => { }); it('should throw FILE_NOT_FOUND when the service yields null', async () => { - jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue(null); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') + .mockResolvedValue(null); const mockRequest = { workspaceId: 'workspace-id' } as any; const mockResponse = createMockResponse() as any; @@ -206,7 +214,7 @@ describe('FileController', () => { ); jest - .spyOn(fileService, 'getFileResponseById') + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') .mockRejectedValue(underlyingError); const mockRequest = { workspaceId: 'workspace-id' } as any; @@ -228,7 +236,7 @@ describe('FileController', () => { await expect(promise).rejects.not.toThrow(/secret-host/); expect(loggerSpy).toHaveBeenCalledWith( - 'getFileResponseById failed unexpectedly', + 'getFilePresignedUrlOrStreamById failed unexpectedly', { error: underlyingError }, ); }); @@ -236,11 +244,13 @@ describe('FileController', () => { it('should throw INTERNAL_SERVER_ERROR when the stream errors before headers are sent', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({ - type: 'stream', - stream: mockStream, - mimeType: 'image/png', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/png', + }); mockPipeline.mockRejectedValue(new Error('source backend exploded')); @@ -267,11 +277,13 @@ describe('FileController', () => { it('should destroy the response without throwing when the stream errors after headers are sent', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({ - type: 'stream', - stream: mockStream, - mimeType: 'image/png', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamById') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/png', + }); mockPipeline.mockRejectedValue(new Error('socket reset mid-flight')); @@ -292,13 +304,14 @@ describe('FileController', () => { }); describe('getPublicAssets', () => { - it('should call fileService.getFileStreamByPath and pipe with headers', async () => { - const mockStream = createMockStream(); - - jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue({ - stream: mockStream, - mimeType: 'image/png', - }); + it('should 302 redirect when presigned URL is available', async () => { + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') + .mockResolvedValue({ + type: 'redirect', + presignedUrl: + 'https://s3.example.com/public-asset/logo.png?signed=abc', + }); const mockRequest = { params: { path: ['images', 'logo.png'] }, @@ -313,7 +326,47 @@ describe('FileController', () => { 'app-id', ); - expect(fileService.getFileStreamByPath).toHaveBeenCalledWith({ + expect( + fileService.getFilePresignedUrlOrStreamByPath, + ).toHaveBeenCalledWith({ + workspaceId: 'workspace-id', + applicationId: 'app-id', + fileFolder: FileFolder.PublicAsset, + filepath: 'images/logo.png', + }); + expect(mockResponse.redirect).toHaveBeenCalledWith( + 'https://s3.example.com/public-asset/logo.png?signed=abc', + ); + expect(mockResponse.setHeader).not.toHaveBeenCalled(); + }); + + it('should stream with headers when no presigned URL (local driver)', async () => { + const mockStream = createMockStream(); + + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/png', + }); + + const mockRequest = { + params: { path: ['images', 'logo.png'] }, + } as any; + + const mockResponse = createMockResponse() as any; + + await controller.getPublicAssets( + mockResponse, + mockRequest, + 'workspace-id', + 'app-id', + ); + + expect( + fileService.getFilePresignedUrlOrStreamByPath, + ).toHaveBeenCalledWith({ workspaceId: 'workspace-id', applicationId: 'app-id', fileFolder: FileFolder.PublicAsset, @@ -337,10 +390,13 @@ describe('FileController', () => { it('should handle single-segment path', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue({ - stream: mockStream, - mimeType: 'image/x-icon', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/x-icon', + }); const mockRequest = { params: { path: ['favicon.ico'] }, @@ -355,7 +411,9 @@ describe('FileController', () => { 'app-id', ); - expect(fileService.getFileStreamByPath).toHaveBeenCalledWith({ + expect( + fileService.getFilePresignedUrlOrStreamByPath, + ).toHaveBeenCalledWith({ workspaceId: 'workspace-id', applicationId: 'app-id', fileFolder: FileFolder.PublicAsset, @@ -364,7 +422,9 @@ describe('FileController', () => { }); it('should throw FILE_NOT_FOUND when the service yields null', async () => { - jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue(null); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') + .mockResolvedValue(null); const mockRequest = { params: { path: ['missing-asset.png'] }, @@ -393,7 +453,7 @@ describe('FileController', () => { ); jest - .spyOn(fileService, 'getFileStreamByPath') + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') .mockRejectedValue(underlyingError); const mockRequest = { @@ -418,7 +478,7 @@ describe('FileController', () => { await expect(promise).rejects.not.toThrow(/secret-host/); expect(loggerSpy).toHaveBeenCalledWith( - 'getFileStreamByPath failed unexpectedly', + 'getFilePresignedUrlOrStreamByPath failed unexpectedly', { error: underlyingError }, ); }); @@ -426,10 +486,13 @@ describe('FileController', () => { it('should throw INTERNAL_SERVER_ERROR when the stream errors before headers are sent', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue({ - stream: mockStream, - mimeType: 'image/png', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/png', + }); mockPipeline.mockRejectedValue(new Error('source backend exploded')); @@ -459,10 +522,13 @@ describe('FileController', () => { it('should destroy the response without throwing when the stream errors after headers are sent', async () => { const mockStream = createMockStream(); - jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue({ - stream: mockStream, - mimeType: 'image/png', - }); + jest + .spyOn(fileService, 'getFilePresignedUrlOrStreamByPath') + .mockResolvedValue({ + type: 'stream', + stream: mockStream, + mimeType: 'image/png', + }); mockPipeline.mockRejectedValue(new Error('socket reset mid-flight')); @@ -472,8 +538,6 @@ describe('FileController', () => { const mockResponse = createMockResponse({ headersSent: true }) as any; - // No throw expected — once headers are out, the controller cannot honestly - // switch to a 500 response, so it tears the socket down instead. await controller.getPublicAssets( mockResponse, mockRequest, diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts index c2de628a38..cf2a18f38b 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts @@ -60,17 +60,20 @@ export class FileController { ); } - const fileStream = await this.fileService - .getFileStreamByPath({ + const fileResponse = await this.fileService + .getFilePresignedUrlOrStreamByPath({ workspaceId, applicationId, fileFolder: FileFolder.PublicAsset, filepath, }) .catch((error) => { - this.logger.error('getFileStreamByPath failed unexpectedly', { - error, - }); + this.logger.error( + 'getFilePresignedUrlOrStreamByPath failed unexpectedly', + { + error, + }, + ); throw new FileException( 'Error retrieving file', @@ -78,19 +81,21 @@ export class FileController { ); }); - if (fileStream === null) { + if (fileResponse === null) { throw new FileException( 'File not found', FileExceptionCode.FILE_NOT_FOUND, ); } - const { stream, mimeType } = fileStream; + if (fileResponse.type === 'redirect') { + return res.redirect(fileResponse.presignedUrl); + } - setFileResponseHeaders(res, mimeType); + setFileResponseHeaders(res, fileResponse.mimeType); try { - await pipeline(stream, res); + await pipeline(fileResponse.stream, res); } catch (error) { this.logger.error('Public asset stream failed mid-transfer', { error }); @@ -117,15 +122,18 @@ export class FileController { const workspaceId = (req as any)?.workspaceId; const fileResponse = await this.fileService - .getFileResponseById({ + .getFilePresignedUrlOrStreamById({ fileId, workspaceId, fileFolder, }) .catch((error) => { - this.logger.error('getFileResponseById failed unexpectedly', { - error, - }); + this.logger.error( + 'getFilePresignedUrlOrStreamById failed unexpectedly', + { + error, + }, + ); throw new FileException( 'Error retrieving file', diff --git a/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts b/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts index 3489090e6c..d272d35c5a 100644 --- a/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts @@ -36,7 +36,7 @@ export class FileService { private readonly applicationRepository: Repository, ) {} - async getFileStreamByPath({ + async getFilePresignedUrlOrStreamByPath({ workspaceId, applicationId, filepath, @@ -46,7 +46,7 @@ export class FileService { applicationId: string; filepath: string; fileFolder: FileFolder; - }): Promise<{ stream: Readable; mimeType: string } | null> { + }): Promise { const application = await this.applicationRepository.findOne({ where: { id: applicationId, @@ -69,28 +69,13 @@ export class FileService { return null; } - try { - const stream = await this.fileStorageService.readFile({ - resourcePath: filepath, - fileFolder, - applicationUniversalIdentifier: application.universalIdentifier, - workspaceId, - }); - - return { - stream, - mimeType: file.mimeType, - }; - } catch (error) { - if ( - error instanceof FileStorageException && - error.code === FileStorageExceptionCode.FILE_NOT_FOUND - ) { - return null; - } - - throw error; - } + return this.getFilePresignedUrlOrStream({ + resourcePath: filepath, + fileFolder, + applicationUniversalIdentifier: application.universalIdentifier, + workspaceId, + mimeType: file.mimeType, + }); } async getFileStreamById({ @@ -152,7 +137,7 @@ export class FileService { } } - async getFileResponseById(params: { + async getFilePresignedUrlOrStreamById(params: { fileId: string; workspaceId: string; fileFolder: FileFolder; @@ -183,12 +168,33 @@ export class FileService { return null; } - const mimeType = file.mimeType ?? 'application/octet-stream'; - const resourceIdentifier = { + return this.getFilePresignedUrlOrStream({ resourcePath: removeFileFolderFromFileEntityPath(file.path), fileFolder: params.fileFolder, applicationUniversalIdentifier: application.universalIdentifier, workspaceId: params.workspaceId, + mimeType: file.mimeType, + }); + } + + private async getFilePresignedUrlOrStream({ + resourcePath, + fileFolder, + applicationUniversalIdentifier, + workspaceId, + mimeType, + }: { + resourcePath: string; + fileFolder: FileFolder; + applicationUniversalIdentifier: string; + workspaceId: string; + mimeType: string; + }): Promise { + const resourceIdentifier = { + resourcePath, + fileFolder, + applicationUniversalIdentifier, + workspaceId, }; const presignedUrl = await this.fileStorageService.getPresignedUrl({ @@ -267,7 +273,7 @@ export class FileService { return { buffer, - mimeType: file.mimeType ?? 'application/octet-stream', + mimeType: file.mimeType, }; } catch (error) { if (