houseassassin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Public assets server s3 redirection (#21108)

Browse Source 
# Introduction
Avoid overloading the server on file streaming
Take profit of the different origin implied by the redirection to the s3
Only concern being the expiration date on a public file which is
acceptable

closes https://github.com/twentyhq/private-issues/issues/483
related https://github.com/twentyhq/private-issues/issues/491
This commit is contained in:
Paul Rastoin
2026-06-01 18:25:58 +02:00
committed by GitHub
parent 989b45db15
commit d6b3527552
3 changed files with 175 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts
+120 -56
View File
@@ -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,
packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts
+21 -13
View File
@@ -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',
packages/twenty-server/src/engine/core-modules/file/services/file.service.ts
+34 -28
View File
@@ -36,7 +36,7 @@ export class FileService {
private readonly applicationRepository: Repository<ApplicationEntity>,
) {}
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<FileResponse | null> {
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<FileResponse | null> {
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 (