fix(core): Handle symlinks in blocked paths (#17735)

Author: RomanDavydchuk

packages/core/src/execution-engine/node-execution-context/utils/__tests__/file-system-helper-functions.test.ts

@@ -1,7 +1,7 @@
 import { Container } from '@n8n/di';
 import type { INode } from 'n8n-workflow';
 import { createReadStream } from 'node:fs';
-import { access as fsAccess } from 'node:fs/promises';
+import { access as fsAccess, realpath as fsRealpath } from 'node:fs/promises';
 import { join } from 'node:path';
 
 import {
@@ -30,6 +30,7 @@ beforeEach(() => {
 	// @ts-expect-error undefined property
 	error.code = 'ENOENT';
 	(fsAccess as jest.Mock).mockRejectedValue(error);
+	(fsRealpath as jest.Mock).mockImplementation((path: string) => path);
 
 	instanceSettings = Container.get(InstanceSettings);
 });
@@ -39,115 +40,125 @@ describe('isFilePathBlocked', () => {
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
 	});
 
-	it('should return true for static cache dir', () => {
+	it('should return true for static cache dir', async () => {
 		const filePath = instanceSettings.staticCacheDir;
-		expect(isFilePathBlocked(filePath)).toBe(true);
+		expect(await isFilePathBlocked(filePath)).toBe(true);
 	});
 
-	it('should return true for restricted paths', () => {
+	it('should return true for restricted paths', async () => {
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(isFilePathBlocked(restrictedPath)).toBe(true);
+		expect(await isFilePathBlocked(restrictedPath)).toBe(true);
 	});
 
-	it('should handle empty allowed paths', () => {
+	it('should handle empty allowed paths', async () => {
 		delete process.env[RESTRICT_FILE_ACCESS_TO];
-		const result = isFilePathBlocked('/some/random/path');
+		const result = await isFilePathBlocked('/some/random/path');
 		expect(result).toBe(false);
 	});
 
-	it('should handle multiple allowed paths', () => {
+	it('should handle multiple allowed paths', async () => {
 		process.env[RESTRICT_FILE_ACCESS_TO] = '/path1;/path2;/path3';
 		const allowedPath = '/path2/somefile';
-		expect(isFilePathBlocked(allowedPath)).toBe(false);
+		expect(await isFilePathBlocked(allowedPath)).toBe(false);
 	});
 
-	it('should handle empty strings in allowed paths', () => {
+	it('should handle empty strings in allowed paths', async () => {
 		process.env[RESTRICT_FILE_ACCESS_TO] = '/path1;;/path2';
 		const allowedPath = '/path2/somefile';
-		expect(isFilePathBlocked(allowedPath)).toBe(false);
+		expect(await isFilePathBlocked(allowedPath)).toBe(false);
 	});
 
-	it('should trim whitespace in allowed paths', () => {
+	it('should trim whitespace in allowed paths', async () => {
 		process.env[RESTRICT_FILE_ACCESS_TO] = ' /path1 ; /path2 ; /path3 ';
 		const allowedPath = '/path2/somefile';
-		expect(isFilePathBlocked(allowedPath)).toBe(false);
+		expect(await isFilePathBlocked(allowedPath)).toBe(false);
 	});
 
-	it('should return false when BLOCK_FILE_ACCESS_TO_N8N_FILES is false', () => {
+	it('should return false when BLOCK_FILE_ACCESS_TO_N8N_FILES is false', async () => {
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'false';
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(isFilePathBlocked(restrictedPath)).toBe(false);
+		expect(await isFilePathBlocked(restrictedPath)).toBe(false);
 	});
 
-	it('should return true when path is in allowed paths but still restricted', () => {
+	it('should return true when path is in allowed paths but still restricted', async () => {
 		process.env[RESTRICT_FILE_ACCESS_TO] = '/some/allowed/path';
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(isFilePathBlocked(restrictedPath)).toBe(true);
+		expect(await isFilePathBlocked(restrictedPath)).toBe(true);
 	});
 
-	it('should return false when path is in allowed paths', () => {
+	it('should return false when path is in allowed paths', async () => {
 		const allowedPath = '/some/allowed/path';
 		process.env[RESTRICT_FILE_ACCESS_TO] = allowedPath;
-		expect(isFilePathBlocked(allowedPath)).toBe(false);
+		expect(await isFilePathBlocked(allowedPath)).toBe(false);
 	});
 
-	it('should return true when file paths in CONFIG_FILES', () => {
+	it('should return true when file paths in CONFIG_FILES', async () => {
 		process.env[CONFIG_FILES] = '/path/to/config1,/path/to/config2';
 		const configPath = '/path/to/config1/somefile';
-		expect(isFilePathBlocked(configPath)).toBe(true);
+		expect(await isFilePathBlocked(configPath)).toBe(true);
 	});
 
-	it('should return true when file paths in CUSTOM_EXTENSION_ENV', () => {
+	it('should return true when file paths in CUSTOM_EXTENSION_ENV', async () => {
 		process.env[CUSTOM_EXTENSION_ENV] = '/path/to/extensions1;/path/to/extensions2';
 		const extensionPath = '/path/to/extensions1/somefile';
-		expect(isFilePathBlocked(extensionPath)).toBe(true);
+		expect(await isFilePathBlocked(extensionPath)).toBe(true);
 	});
 
-	it('should return true when file paths in BINARY_DATA_STORAGE_PATH', () => {
+	it('should return true when file paths in BINARY_DATA_STORAGE_PATH', async () => {
 		process.env[BINARY_DATA_STORAGE_PATH] = '/path/to/binary/storage';
 		const binaryPath = '/path/to/binary/storage/somefile';
-		expect(isFilePathBlocked(binaryPath)).toBe(true);
+		expect(await isFilePathBlocked(binaryPath)).toBe(true);
 	});
 
-	it('should block file paths in email template paths', () => {
+	it('should block file paths in email template paths', async () => {
 		process.env[UM_EMAIL_TEMPLATES_INVITE] = '/path/to/invite/templates';
 		process.env[UM_EMAIL_TEMPLATES_PWRESET] = '/path/to/pwreset/templates';
 
 		const invitePath = '/path/to/invite/templates/invite.html';
 		const pwResetPath = '/path/to/pwreset/templates/reset.html';
 
-		expect(isFilePathBlocked(invitePath)).toBe(true);
-		expect(isFilePathBlocked(pwResetPath)).toBe(true);
+		expect(await isFilePathBlocked(invitePath)).toBe(true);
+		expect(await isFilePathBlocked(pwResetPath)).toBe(true);
 	});
 
-	it('should block access to n8n files if restrict and block are set', () => {
+	it('should block access to n8n files if restrict and block are set', async () => {
 		const homeVarName = process.platform === 'win32' ? 'USERPROFILE' : 'HOME';
 		const userHome = process.env.N8N_USER_FOLDER ?? process.env[homeVarName] ?? process.cwd();
 
 		process.env[RESTRICT_FILE_ACCESS_TO] = userHome;
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(isFilePathBlocked(restrictedPath)).toBe(true);
+		expect(await isFilePathBlocked(restrictedPath)).toBe(true);
 	});
 
-	it('should allow access to parent folder if restrict and block are set', () => {
+	it('should allow access to parent folder if restrict and block are set', async () => {
 		const homeVarName = process.platform === 'win32' ? 'USERPROFILE' : 'HOME';
 		const userHome = process.env.N8N_USER_FOLDER ?? process.env[homeVarName] ?? process.cwd();
 
 		process.env[RESTRICT_FILE_ACCESS_TO] = userHome;
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
 		const restrictedPath = join(userHome, 'somefile.txt');
-		expect(isFilePathBlocked(restrictedPath)).toBe(false);
+		expect(await isFilePathBlocked(restrictedPath)).toBe(false);
 	});
 
-	it('should not block similar paths', () => {
+	it('should not block similar paths', async () => {
 		const homeVarName = process.platform === 'win32' ? 'USERPROFILE' : 'HOME';
 		const userHome = process.env.N8N_USER_FOLDER ?? process.env[homeVarName] ?? process.cwd();
 
 		process.env[RESTRICT_FILE_ACCESS_TO] = userHome;
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
 		const restrictedPath = join(userHome, '.n8n_x');
-		expect(isFilePathBlocked(restrictedPath)).toBe(false);
+		expect(await isFilePathBlocked(restrictedPath)).toBe(false);
+	});
+
+	it('should return true for a symlink in a allowed path to a restricted path', async () => {
+		process.env[RESTRICT_FILE_ACCESS_TO] = '/path1';
+		const allowedPath = '/path1/symlink';
+		const actualPath = '/path2/realfile';
+		(fsRealpath as jest.Mock).mockImplementation((path: string) =>
+			path === allowedPath ? actualPath : path,
+		);
+		expect(await isFilePathBlocked(allowedPath)).toBe(true);
 	});
 });
 

packages/core/src/execution-engine/node-execution-context/utils/file-system-helper-functions.ts

@@ -3,8 +3,11 @@ import { Container } from '@n8n/di';
 import type { FileSystemHelperFunctions, INode } from 'n8n-workflow';
 import { NodeOperationError } from 'n8n-workflow';
 import { createReadStream } from 'node:fs';
-import { access as fsAccess, writeFile as fsWriteFile } from 'node:fs/promises';
-import { resolve } from 'node:path';
+import {
+	access as fsAccess,
+	writeFile as fsWriteFile,
+	realpath as fsRealpath,
+} from 'node:fs/promises';
 
 import {
 	BINARY_DATA_STORAGE_PATH,
@@ -29,9 +32,9 @@ const getAllowedPaths = () => {
 	return allowedPaths;
 };
 
-export function isFilePathBlocked(filePath: string): boolean {
+export async function isFilePathBlocked(filePath: string): Promise<boolean> {
 	const allowedPaths = getAllowedPaths();
-	const resolvedFilePath = resolve(filePath);
+	const resolvedFilePath = await fsRealpath(filePath);
 	const blockFileAccessToN8nFiles = process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] !== 'false';
 
 	const restrictedPaths = blockFileAccessToN8nFiles ? getN8nRestrictedPaths() : [];
@@ -62,7 +65,7 @@ export const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunct
 					})
 				: error;
 		}
-		if (isFilePathBlocked(filePath as string)) {
+		if (await isFilePathBlocked(filePath as string)) {
 			const allowedPaths = getAllowedPaths();
 			const message = allowedPaths.length ? ` Allowed paths: ${allowedPaths.join(', ')}` : '';
 			throw new NodeOperationError(node, `Access to the file is not allowed.${message}`, {
@@ -77,7 +80,7 @@ export const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunct
 	},
 
 	async writeContentToFile(filePath, content, flag) {
-		if (isFilePathBlocked(filePath as string)) {
+		if (await isFilePathBlocked(filePath as string)) {
 			throw new NodeOperationError(node, `The file "${String(filePath)}" is not writable.`, {
 				level: 'warning',
 			});

packages/nodes-base/nodes/Crypto/test/Crypto.test.ts

@@ -7,6 +7,8 @@ describe('Test Crypto Node', () => {
 	jest.mock('fast-glob', () => async () => ['/test/binary.data']);
 	jest.mock('fs/promises');
 	fsPromises.access = async () => {};
+	const realpathSpy = jest.spyOn(fsPromises, 'realpath');
+	realpathSpy.mockImplementation(async (path) => path as string);
 	jest.mock('fs');
 	fs.createReadStream = () => Readable.from(Buffer.from('test')) as fs.ReadStream;
 

packages/nodes-base/nodes/Files/ReadWriteFile/test/ReadWriteFile.test.ts

@@ -1,4 +1,5 @@
 import { NodeTestHarness } from '@nodes-testing/node-test-harness';
+import fsPromises from 'fs/promises';
 import type { WorkflowTestData } from 'n8n-workflow';
 
 describe('Test ReadWriteFile Node', () => {
@@ -13,6 +14,9 @@ describe('Test ReadWriteFile Node', () => {
 	const writeFileNode = workflowData.nodes.find((n) => n.name === 'Write to Disk')!;
 	writeFileNode.parameters.fileName = `${testHarness.temporaryDir}/image-written.jpg`;
 
+	const realpathSpy = jest.spyOn(fsPromises, 'realpath');
+	realpathSpy.mockImplementation(async (path) => path as string);
+
 	const tests: WorkflowTestData[] = [
 		{
 			description: 'nodes/Files/ReadWriteFile/test/ReadWriteFile.workflow.json',

packages/nodes-base/nodes/WriteBinaryFile/test/WriteBinaryFile.test.ts

@@ -1,4 +1,5 @@
 import { NodeTestHarness } from '@nodes-testing/node-test-harness';
+import fsPromises from 'fs/promises';
 import type { WorkflowTestData } from 'n8n-workflow';
 import path from 'path';
 
@@ -12,6 +13,9 @@ describe('Test Write Binary File Node', () => {
 	const writeFilePath = path.join(testHarness.temporaryDir, 'image-written.jpg');
 	writeFileNode.parameters.fileName = writeFilePath;
 
+	const realpathSpy = jest.spyOn(fsPromises, 'realpath');
+	realpathSpy.mockImplementation(async (path) => path as string);
+
 	const tests: WorkflowTestData[] = [
 		{
 			description: 'nodes/WriteBinaryFile/test/WriteBinaryFile.workflow.json',