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

fix: add security headers to file serving endpoints to prevent stored XSS (#18857)

Browse Source 
## Summary

- File serving endpoints (`GET file/:fileFolder/:id` and `GET
public-assets/...`) were piping S3/local file streams directly to the
response without any HTTP headers, allowing a stored XSS attack via
uploaded HTML files rendered inline on the CRM origin.
- Adds `Content-Type`, `Content-Disposition`, and
`X-Content-Type-Options: nosniff` headers to all file serving responses.
Only known-safe MIME types (images, PDF, plain text, audio, video) are
served inline; everything else (HTML, SVG, XML, etc.) forces
`Content-Disposition: attachment` to trigger download instead of
rendering.
- New `setFileResponseHeaders` utility with an explicit allowlist of
inline-safe MIME types.

## Test plan

- [x] Unit tests pass (9 tests including 2 new ones: header assertions
and attachment-disposition for HTML)
- [x] Lint clean (`lint:diff-with-main`)
- [x] Typecheck clean (`nx typecheck twenty-server`)
- [ ] Manual: upload an HTML file via `uploadWorkflowFile`, access the
returned URL — should download instead of rendering
- [ ] Manual: upload a PNG image, access the URL — should render inline
with correct `Content-Type: image/png`
- [ ] Manual: verify `X-Content-Type-Options: nosniff` header is present
on all file responses


Made with [Cursor](https://cursor.com)

---------

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Etienne <etiennejouan@users.noreply.github.com>
Co-authored-by: Etienne <45695613+etiennejouan@users.noreply.github.com>
This commit is contained in:
Félix Malfait
2026-03-23 16:19:16 +01:00
committed by GitHub
parent 0626f3e469
commit 6f4f7a1198
7 changed files with 247 additions and 431 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts
+2
View File
@@ -178,6 +178,8 @@ export class ApplicationInstallService {
);
}
// TODO: mimeType should be defined, default to application/octet-stream, which won't be displayed
// inline by the browser (forced download) due to Content-Disposition security headers.
await this.fileStorageService.writeFile({
sourceFile: content,
mimeType: undefined,
packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts
+77 -18
View File
@@ -32,6 +32,10 @@ const createMockStream = (): Readable => {
return stream;
};
const createMockResponse = () => ({
setHeader: jest.fn(),
});
describe('FileController', () => {
let controller: FileController;
let fileService: FileService;
@@ -75,15 +79,16 @@ describe('FileController', () => {
});
describe('getFileById', () => {
it('should call fileService.getFileStreamById and pipe the result', async () => {
it('should call fileService.getFileStreamById and pipe the result with headers', async () => {
const mockStream = createMockStream();
jest
.spyOn(fileService, 'getFileStreamById')
.mockResolvedValue(mockStream);
jest.spyOn(fileService, 'getFileStreamById').mockResolvedValue({
stream: mockStream,
mimeType: 'image/png',
});
const mockRequest = { workspaceId: 'workspace-id' } as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await controller.getFileById(
mockResponse,
@@ -97,9 +102,49 @@ describe('FileController', () => {
workspaceId: 'workspace-id',
fileFolder: FileFolder.CorePicture,
});
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'Content-Type',
'image/png',
);
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'X-Content-Type-Options',
'nosniff',
);
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'Content-Disposition',
'inline',
);
expect(mockStream.pipe).toHaveBeenCalledWith(mockResponse);
});
it('should force attachment disposition for non-safe MIME types', async () => {
const mockStream = createMockStream();
jest.spyOn(fileService, 'getFileStreamById').mockResolvedValue({
stream: mockStream,
mimeType: 'text/html',
});
const mockRequest = { workspaceId: 'workspace-id' } as any;
const mockResponse = createMockResponse() as any;
await controller.getFileById(
mockResponse,
mockRequest,
FileFolder.Workflow,
'file-123',
);
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'Content-Type',
'text/html',
);
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'Content-Disposition',
'attachment',
);
});
it('should throw FileException with FILE_NOT_FOUND when file is not found', async () => {
jest
.spyOn(fileService, 'getFileStreamById')
@@ -111,7 +156,7 @@ describe('FileController', () => {
);
const mockRequest = { workspaceId: 'workspace-id' } as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await expect(
controller.getFileById(
@@ -131,7 +176,7 @@ describe('FileController', () => {
.mockRejectedValue(new Error('Storage unavailable'));
const mockRequest = { workspaceId: 'workspace-id' } as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await expect(
controller.getFileById(
@@ -150,18 +195,19 @@ describe('FileController', () => {
});
describe('getPublicAssets', () => {
it('should call fileService.getFileStreamByPath and pipe the result', async () => {
it('should call fileService.getFileStreamByPath and pipe with headers', async () => {
const mockStream = createMockStream();
jest
.spyOn(fileService, 'getFileStreamByPath')
.mockResolvedValue(mockStream);
jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue({
stream: mockStream,
mimeType: 'image/png',
});
const mockRequest = {
params: { path: ['images', 'logo.png'] },
} as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await controller.getPublicAssets(
mockResponse,
@@ -176,21 +222,34 @@ describe('FileController', () => {
fileFolder: FileFolder.PublicAsset,
filepath: 'images/logo.png',
});
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'Content-Type',
'image/png',
);
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'X-Content-Type-Options',
'nosniff',
);
expect(mockResponse.setHeader).toHaveBeenCalledWith(
'Content-Disposition',
'inline',
);
expect(mockStream.pipe).toHaveBeenCalledWith(mockResponse);
});
it('should handle single-segment path', async () => {
const mockStream = createMockStream();
jest
.spyOn(fileService, 'getFileStreamByPath')
.mockResolvedValue(mockStream);
jest.spyOn(fileService, 'getFileStreamByPath').mockResolvedValue({
stream: mockStream,
mimeType: 'image/x-icon',
});
const mockRequest = {
params: { path: ['favicon.ico'] },
} as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await controller.getPublicAssets(
mockResponse,
@@ -221,7 +280,7 @@ describe('FileController', () => {
params: { path: ['missing-asset.png'] },
} as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await expect(
controller.getPublicAssets(
@@ -244,7 +303,7 @@ describe('FileController', () => {
params: { path: ['broken-asset.png'] },
} as any;
const mockResponse = {} as any;
const mockResponse = createMockResponse() as any;
await expect(
controller.getPublicAssets(
packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts
+11 -6
View File
@@ -28,6 +28,7 @@ import {
SupportedFileFolder,
} from 'src/engine/core-modules/file/guards/file-by-id.guard';
import { FileService } from 'src/engine/core-modules/file/services/file.service';
import { setFileResponseHeaders } from 'src/engine/core-modules/file/utils/set-file-response-headers.utils';
import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard';
import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard';
@@ -48,21 +49,23 @@ export class FileController {
const filepath = join(...req.params.path);
try {
const fileStream = await this.fileService.getFileStreamByPath({
const { stream, mimeType } = await this.fileService.getFileStreamByPath({
workspaceId,
applicationId,
fileFolder: FileFolder.PublicAsset,
filepath,
});
fileStream.on('error', () => {
setFileResponseHeaders(res, mimeType);
stream.on('error', () => {
throw new FileException(
'Error streaming file from storage',
FileExceptionCode.INTERNAL_SERVER_ERROR,
);
});
fileStream.pipe(res);
stream.pipe(res);
} catch (error) {
if (
error instanceof FileStorageException &&
@@ -93,20 +96,22 @@ export class FileController {
const workspaceId = (req as any)?.workspaceId;
try {
const fileStream = await this.fileService.getFileStreamById({
const { stream, mimeType } = await this.fileService.getFileStreamById({
fileId,
workspaceId,
fileFolder,
});
fileStream.on('error', () => {
setFileResponseHeaders(res, mimeType);
stream.on('error', () => {
throw new FileException(
'Error streaming file from storage',
FileExceptionCode.INTERNAL_SERVER_ERROR,
);
});
fileStream.pipe(res);
stream.pipe(res);
} catch (error) {
if (
error instanceof FileStorageException &&
packages/twenty-server/src/engine/core-modules/file/services/file.service.ts
+22 -4
View File
@@ -45,7 +45,15 @@ export class FileService {
applicationId: string;
filepath: string;
fileFolder: FileFolder;
}) {
}): Promise<{ stream: Readable; mimeType: string }> {
const file = await this.fileRepository.findOneOrFail({
where: {
path: `${fileFolder}/${filepath}`,
workspaceId,
applicationId,
},
});
const application = await this.applicationRepository.findOneOrFail({
where: {
id: applicationId,
@@ -53,12 +61,17 @@ export class FileService {
},
});
return this.fileStorageService.readFile({
const stream = await this.fileStorageService.readFile({
resourcePath: filepath,
fileFolder,
applicationUniversalIdentifier: application.universalIdentifier,
workspaceId,
});
return {
stream,
mimeType: file.mimeType,
};
}
async getFileStreamById({
@@ -69,7 +82,7 @@ export class FileService {
fileId: string;
workspaceId: string;
fileFolder: FileFolder;
}): Promise<Readable> {
}): Promise<{ stream: Readable; mimeType: string }> {
const file = await this.fileRepository.findOneOrFail({
where: {
id: fileId,
@@ -85,12 +98,17 @@ export class FileService {
},
});
return this.fileStorageService.readFile({
const stream = await this.fileStorageService.readFile({
resourcePath: removeFileFolderFromFileEntityPath(file.path),
fileFolder,
applicationUniversalIdentifier: application.universalIdentifier,
workspaceId,
});
return {
stream,
mimeType: file.mimeType,
};
}
async getFileContentById({
packages/twenty-server/src/engine/core-modules/file/utils/set-file-response-headers.utils.ts
+31
View File
@@ -0,0 +1,31 @@
import { type Response } from 'express';
const INLINE_SAFE_MIME_TYPES = new Set([
'image/jpeg',
'image/png',
'image/gif',
'image/webp',
'image/avif',
'image/bmp',
'image/tiff',
'application/pdf',
'text/plain',
'audio/mpeg',
'audio/wav',
'audio/ogg',
'video/mp4',
'video/webm',
'video/ogg',
'image/x-icon',
]);
export const setFileResponseHeaders = (res: Response, mimeType: string) => {
const contentType = mimeType || 'application/octet-stream';
const disposition = INLINE_SAFE_MIME_TYPES.has(contentType)
? 'inline'
: 'attachment';
res.setHeader('Content-Type', contentType);
res.setHeader('X-Content-Type-Options', 'nosniff');
res.setHeader('Content-Disposition', disposition);
};
packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts
+1 -1
View File
@@ -191,7 +191,7 @@ export class EmailComposerService {
const attachments: MessageAttachment[] = [];
for (const fileMetadata of files) {
const stream = await this.fileService.getFileStreamById({
const { stream } = await this.fileService.getFileStreamById({
fileId: fileMetadata.id,
workspaceId,
fileFolder: FileFolder.Workflow,
packages/twenty-server/src/engine/metadata-modules/ai/ai-models/ai-providers.json
+103 -402
View File
File diff suppressed because it is too large Load Diff