From 9c8a5f9c57e65a56a3b7da5dfa805d23c55e5ee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 2 Apr 2025 14:55:15 +0200 Subject: [PATCH] fix(core): Sandbox HTML binary files in viewing mode (#14350) --- .../__tests__/binary-data.controller.test.ts | 152 ++++++++++++++++++ .../src/controllers/binary-data.controller.ts | 11 +- 2 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/controllers/__tests__/binary-data.controller.test.ts diff --git a/packages/cli/src/controllers/__tests__/binary-data.controller.test.ts b/packages/cli/src/controllers/__tests__/binary-data.controller.test.ts new file mode 100644 index 0000000000..ac18f79860 --- /dev/null +++ b/packages/cli/src/controllers/__tests__/binary-data.controller.test.ts @@ -0,0 +1,152 @@ +import type { Response } from 'express'; +import { mock } from 'jest-mock-extended'; +import type { BinaryDataService } from 'n8n-core'; +import { FileNotFoundError } from 'n8n-core'; +import type { Readable } from 'node:stream'; + +import type { BinaryDataRequest } from '@/requests'; + +import { BinaryDataController } from '../binary-data.controller'; + +describe('BinaryDataController', () => { + const request = mock(); + const response = mock(); + const binaryDataService = mock(); + const controller = new BinaryDataController(binaryDataService); + + beforeEach(() => { + jest.resetAllMocks(); + response.status.mockReturnThis(); + }); + + describe('get', () => { + it('should return 400 if binary data ID is missing', async () => { + // @ts-expect-error invalid query object + request.query = {}; + + await controller.get(request, response); + + expect(response.status).toHaveBeenCalledWith(400); + expect(response.end).toHaveBeenCalledWith('Missing binary data ID'); + }); + + it('should return 400 if binary data mode is missing', async () => { + request.query = { id: '123', action: 'view' }; + + await controller.get(request, response); + + expect(response.status).toHaveBeenCalledWith(400); + expect(response.end).toHaveBeenCalledWith('Missing binary data mode'); + }); + + it('should return 400 if binary data mode is invalid', async () => { + request.query = { id: 'invalidMode:123', action: 'view' }; + + await controller.get(request, response); + + expect(response.status).toHaveBeenCalledWith(400); + expect(response.end).toHaveBeenCalledWith('Invalid binary data mode'); + }); + + it('should return 404 if file is not found', async () => { + request.query = { id: 'filesystem:123', action: 'view' }; + binaryDataService.getAsStream.mockRejectedValue(new FileNotFoundError('File not found')); + + await controller.get(request, response); + + expect(response.status).toHaveBeenCalledWith(404); + expect(response.end).toHaveBeenCalled(); + }); + + it('should set Content-Type and Content-Length from query if provided', async () => { + request.query = { + id: 'filesystem:123', + action: 'view', + fileName: 'test.txt', + mimeType: 'text/plain', + }; + + binaryDataService.getAsStream.mockResolvedValue(mock()); + + await controller.get(request, response); + + expect(binaryDataService.getMetadata).not.toHaveBeenCalled(); + expect(response.setHeader).toHaveBeenCalledWith('Content-Type', 'text/plain'); + expect(response.setHeader).not.toHaveBeenCalledWith('Content-Length'); + expect(response.setHeader).not.toHaveBeenCalledWith('Content-Disposition'); + }); + + it('should set Content-Type and Content-Length from metadata if not provided', async () => { + request.query = { id: 'filesystem:123', action: 'view' }; + + binaryDataService.getMetadata.mockResolvedValue({ + fileName: 'test.txt', + mimeType: 'text/plain', + fileSize: 100, + }); + binaryDataService.getAsStream.mockResolvedValue(mock()); + + await controller.get(request, response); + + expect(binaryDataService.getMetadata).toHaveBeenCalledWith('filesystem:123'); + expect(response.setHeader).toHaveBeenCalledWith('Content-Type', 'text/plain'); + expect(response.setHeader).toHaveBeenCalledWith('Content-Length', 100); + expect(response.setHeader).not.toHaveBeenCalledWith('Content-Disposition'); + }); + + it('should set Content-Disposition for download action', async () => { + request.query = { id: 'filesystem:123', action: 'download', fileName: 'test.txt' }; + + binaryDataService.getAsStream.mockResolvedValue(mock()); + + await controller.get(request, response); + + expect(response.setHeader).toHaveBeenCalledWith( + 'Content-Disposition', + 'attachment; filename="test.txt"', + ); + }); + + it('should set Content-Security-Policy for HTML in view mode', async () => { + request.query = { + id: 'filesystem:123', + action: 'view', + fileName: 'test.html', + mimeType: 'text/html', + }; + + binaryDataService.getAsStream.mockResolvedValue(mock()); + + await controller.get(request, response); + + expect(response.header).toHaveBeenCalledWith('Content-Security-Policy', 'sandbox'); + }); + + it('should not set Content-Security-Policy for HTML in download mode', async () => { + request.query = { + id: 'filesystem:123', + action: 'download', + fileName: 'test.html', + mimeType: 'text/html', + }; + + binaryDataService.getAsStream.mockResolvedValue(mock()); + + await controller.get(request, response); + + expect(response.header).not.toHaveBeenCalledWith('Content-Security-Policy', 'sandbox'); + }); + + it('should return the file stream on success', async () => { + request.query = { id: 'filesystem:123', action: 'view' }; + + const stream = mock(); + binaryDataService.getAsStream.mockResolvedValue(stream); + + const result = await controller.get(request, response); + + expect(result).toBe(stream); + expect(binaryDataService.getAsStream).toHaveBeenCalledWith('filesystem:123'); + }); + }); +}); diff --git a/packages/cli/src/controllers/binary-data.controller.ts b/packages/cli/src/controllers/binary-data.controller.ts index ecf3460be0..1b009fd44b 100644 --- a/packages/cli/src/controllers/binary-data.controller.ts +++ b/packages/cli/src/controllers/binary-data.controller.ts @@ -38,7 +38,14 @@ export class BinaryDataController { } catch {} } - if (mimeType) res.setHeader('Content-Type', mimeType); + if (mimeType) { + res.setHeader('Content-Type', mimeType); + + // Sandbox html files when viewed in a browser + if (mimeType.includes('html') && action === 'view') { + res.header('Content-Security-Policy', 'sandbox'); + } + } if (action === 'download' && fileName) { const encodedFilename = encodeURIComponent(fileName); @@ -47,7 +54,7 @@ export class BinaryDataController { return await this.binaryDataService.getAsStream(binaryDataId); } catch (error) { - if (error instanceof FileNotFoundError) return res.writeHead(404).end(); + if (error instanceof FileNotFoundError) return res.status(404).end(); else throw error; } }