fix(core): Only resolve the filepath once

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

@@ -15,7 +15,7 @@ import {
 } from '@/constants';
 import { InstanceSettings } from '@/instance-settings';
 
-import { getFileSystemHelperFunctions, isFilePathBlocked } from '../file-system-helper-functions';
+import { getFileSystemHelperFunctions } from '../file-system-helper-functions';
 
 jest.mock('node:fs');
 jest.mock('node:fs/promises');
@@ -39,78 +39,80 @@ beforeEach(() => {
 });
 
 describe('isFilePathBlocked', () => {
+	const node = { type: 'TestNode' } as INode;
+	const { isFilePathBlocked, resolvePath } = getFileSystemHelperFunctions(node);
 	beforeEach(() => {
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
 	});
 
 	it('should return true for static cache dir', async () => {
 		const filePath = instanceSettings.staticCacheDir;
-		expect(await isFilePathBlocked(filePath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(filePath))).toBe(true);
 	});
 
 	it('should return true for restricted paths', async () => {
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(await isFilePathBlocked(restrictedPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(restrictedPath))).toBe(true);
 	});
 
 	it('should handle empty allowed paths', async () => {
 		securityConfig.restrictFileAccessTo = '';
-		const result = await isFilePathBlocked('/some/random/path');
+		const result = isFilePathBlocked(await resolvePath('/some/random/path'));
 		expect(result).toBe(false);
 	});
 
 	it('should handle multiple allowed paths', async () => {
 		securityConfig.restrictFileAccessTo = '/path1;/path2;/path3';
 		const allowedPath = '/path2/somefile';
-		expect(await isFilePathBlocked(allowedPath)).toBe(false);
+		expect(isFilePathBlocked(await resolvePath(allowedPath))).toBe(false);
 	});
 
 	it('should handle empty strings in allowed paths', async () => {
 		securityConfig.restrictFileAccessTo = '/path1;;/path2';
 		const allowedPath = '/path2/somefile';
-		expect(await isFilePathBlocked(allowedPath)).toBe(false);
+		expect(isFilePathBlocked(await resolvePath(allowedPath))).toBe(false);
 	});
 
 	it('should trim whitespace in allowed paths', async () => {
 		securityConfig.restrictFileAccessTo = ' /path1 ; /path2 ; /path3 ';
 		const allowedPath = '/path2/somefile';
-		expect(await isFilePathBlocked(allowedPath)).toBe(false);
+		expect(isFilePathBlocked(await resolvePath(allowedPath))).toBe(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(await isFilePathBlocked(restrictedPath)).toBe(false);
+		expect(isFilePathBlocked(await resolvePath(restrictedPath))).toBe(false);
 	});
 
 	it('should return true when path is in allowed paths but still restricted', async () => {
 		securityConfig.restrictFileAccessTo = '/some/allowed/path';
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(await isFilePathBlocked(restrictedPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(restrictedPath))).toBe(true);
 	});
 
 	it('should return false when path is in allowed paths', async () => {
 		const allowedPath = '/some/allowed/path';
 		securityConfig.restrictFileAccessTo = allowedPath;
-		expect(await isFilePathBlocked(allowedPath)).toBe(false);
+		expect(isFilePathBlocked(await resolvePath(allowedPath))).toBe(false);
 	});
 
 	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(await isFilePathBlocked(configPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(configPath))).toBe(true);
 	});
 
 	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(await isFilePathBlocked(extensionPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(extensionPath))).toBe(true);
 	});
 
 	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(await isFilePathBlocked(binaryPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(binaryPath))).toBe(true);
 	});
 
 	it('should block file paths in email template paths', async () => {
@@ -120,8 +122,8 @@ describe('isFilePathBlocked', () => {
 		const invitePath = '/path/to/invite/templates/invite.html';
 		const pwResetPath = '/path/to/pwreset/templates/reset.html';
 
-		expect(await isFilePathBlocked(invitePath)).toBe(true);
-		expect(await isFilePathBlocked(pwResetPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(invitePath))).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(pwResetPath))).toBe(true);
 	});
 
 	it('should block access to n8n files if restrict and block are set', async () => {
@@ -131,7 +133,7 @@ describe('isFilePathBlocked', () => {
 		securityConfig.restrictFileAccessTo = userHome;
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
 		const restrictedPath = instanceSettings.n8nFolder;
-		expect(await isFilePathBlocked(restrictedPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(restrictedPath))).toBe(true);
 	});
 
 	it('should allow access to parent folder if restrict and block are set', async () => {
@@ -140,8 +142,8 @@ describe('isFilePathBlocked', () => {
 
 		securityConfig.restrictFileAccessTo = userHome;
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
-		const restrictedPath = join(userHome, 'somefile.txt');
-		expect(await isFilePathBlocked(restrictedPath)).toBe(false);
+		const restrictedPath = await resolvePath(join(userHome, 'somefile.txt'));
+		expect(isFilePathBlocked(restrictedPath)).toBe(false);
 	});
 
 	it('should not block similar paths', async () => {
@@ -150,8 +152,8 @@ describe('isFilePathBlocked', () => {
 
 		securityConfig.restrictFileAccessTo = userHome;
 		process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true';
-		const restrictedPath = join(userHome, '.n8n_x');
-		expect(await isFilePathBlocked(restrictedPath)).toBe(false);
+		const restrictedPath = await resolvePath(join(userHome, '.n8n_x'));
+		expect(isFilePathBlocked(restrictedPath)).toBe(false);
 	});
 
 	it('should return true for a symlink in a allowed path to a restricted path', async () => {
@@ -161,7 +163,7 @@ describe('isFilePathBlocked', () => {
 		(fsRealpath as jest.Mock).mockImplementation((path: string) =>
 			path === allowedPath ? actualPath : path,
 		);
-		expect(await isFilePathBlocked(allowedPath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(allowedPath))).toBe(true);
 	});
 
 	it('should handle non-existent file when it is allowed', async () => {
@@ -170,7 +172,7 @@ describe('isFilePathBlocked', () => {
 		// @ts-expect-error undefined property
 		error.code = 'ENOENT';
 		(fsRealpath as jest.Mock).mockRejectedValueOnce(error);
-		expect(await isFilePathBlocked(filePath)).toBe(false);
+		expect(isFilePathBlocked(await resolvePath(filePath))).toBe(false);
 	});
 
 	it('should handle non-existent file when it is not allowed', async () => {
@@ -181,7 +183,7 @@ describe('isFilePathBlocked', () => {
 		// @ts-expect-error undefined property
 		error.code = 'ENOENT';
 		(fsRealpath as jest.Mock).mockRejectedValueOnce(error);
-		expect(await isFilePathBlocked(filePath)).toBe(true);
+		expect(isFilePathBlocked(await resolvePath(filePath))).toBe(true);
 	});
 });
 
@@ -213,17 +215,17 @@ describe('getFileSystemHelperFunctions', () => {
 			error.code = 'ENOENT';
 			(fsAccess as jest.Mock).mockRejectedValueOnce(error);
 
-			await expect(helperFunctions.createReadStream(filePath)).rejects.toThrow(
-				`The file "${filePath}" could not be accessed.`,
-			);
+			await expect(
+				helperFunctions.createReadStream(await helperFunctions.resolvePath(filePath)),
+			).rejects.toThrow(`The file "${filePath}" could not be accessed.`);
 		});
 
 		it('should throw when file access is blocked', async () => {
 			securityConfig.restrictFileAccessTo = '/allowed/path';
 			(fsAccess as jest.Mock).mockResolvedValueOnce({});
-			await expect(helperFunctions.createReadStream('/blocked/path')).rejects.toThrow(
-				'Access to the file is not allowed',
-			);
+			await expect(
+				helperFunctions.createReadStream(await helperFunctions.resolvePath('/blocked/path')),
+			).rejects.toThrow('Access to the file is not allowed');
 		});
 
 		it('should not reveal if file exists if it is within restricted path', async () => {
@@ -234,16 +236,43 @@ describe('getFileSystemHelperFunctions', () => {
 			error.code = 'ENOENT';
 			(fsAccess as jest.Mock).mockRejectedValueOnce(error);
 
-			await expect(helperFunctions.createReadStream('/blocked/path')).rejects.toThrow(
-				'Access to the file is not allowed',
-			);
+			await expect(
+				helperFunctions.createReadStream(await helperFunctions.resolvePath('/blocked/path')),
+			).rejects.toThrow('Access to the file is not allowed');
 		});
 
 		it('should create a read stream if file access is permitted', async () => {
 			const filePath = '/allowed/path';
 			(fsAccess as jest.Mock).mockResolvedValueOnce({});
-			await helperFunctions.createReadStream(filePath);
-			expect(createReadStream).toHaveBeenCalledWith(filePath);
+			await helperFunctions.createReadStream(await helperFunctions.resolvePath(filePath));
+			expect(createReadStream).toHaveBeenCalledWith(
+				filePath,
+				expect.objectContaining({
+					flags: expect.any(Number),
+				}),
+			);
+		});
+
+		it('should reject symlinks with O_NOFOLLOW to prevent TOCTOU attacks', async () => {
+			const filePath = '/allowed/path/file';
+
+			// Clear previous mocks and set up fresh mocks
+			(fsAccess as jest.Mock).mockReset();
+			(fsAccess as jest.Mock).mockResolvedValue(undefined);
+
+			// Simulate the ELOOP error that occurs when O_NOFOLLOW encounters a symlink
+			const eloopError = new Error('ELOOP: too many symbolic links encountered');
+			// @ts-expect-error undefined property
+			eloopError.code = 'ELOOP';
+
+			// Mock createReadStream to throw ELOOP synchronously (which gets caught by try-catch)
+			(createReadStream as jest.Mock).mockImplementationOnce(() => {
+				throw eloopError;
+			});
+
+			await expect(
+				helperFunctions.createReadStream(await helperFunctions.resolvePath(filePath)),
+			).rejects.toThrow('could not be opened');
 		});
 	});
 
@@ -253,7 +282,7 @@ describe('getFileSystemHelperFunctions', () => {
 
 			await expect(
 				helperFunctions.writeContentToFile(
-					instanceSettings.n8nFolder + '/test.txt',
+					await helperFunctions.resolvePath(instanceSettings.n8nFolder + '/test.txt'),
 					'content',
 					'w',
 				),

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

@@ -1,9 +1,11 @@
 import { isContainedWithin, safeJoinPath } from '@n8n/backend-common';
 import { SecurityConfig } from '@n8n/config';
 import { Container } from '@n8n/di';
 import type { FileSystemHelperFunctions, INode } from 'n8n-workflow';
 import { NodeOperationError } from 'n8n-workflow';
-import { createReadStream } from 'node:fs';
+import type { PathLike } from 'node:fs';
+import { constants, createReadStream } from 'node:fs';
+import type { ResolvedFilePath } from 'n8n-workflow';
 import {
 	access as fsAccess,
 	writeFile as fsWriteFile,
@@ -35,18 +37,19 @@
 	return allowedPaths;
 };
 
-export async function isFilePathBlocked(filePath: string): Promise<boolean> {
-	const allowedPaths = getAllowedPaths();
-	let resolvedFilePath = '';
+const resolvePath = async (path: PathLike): Promise<ResolvedFilePath> => {
 	try {
-		resolvedFilePath = await fsRealpath(filePath);
+		return (await fsRealpath(path)) as ResolvedFilePath; // apply brand, since we know it's resolved now
 	} catch (error: unknown) {
 		if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
-			resolvedFilePath = resolve(filePath);
-		} else {
-			throw error;
+			return resolve(path.toString()) as ResolvedFilePath; // apply brand, since we know it's resolved now
 		}
+		throw error;
 	}
+};
+
+function isFilePathBlocked(resolvedFilePath: ResolvedFilePath): boolean {
+	const allowedPaths = getAllowedPaths();
 	const blockFileAccessToN8nFiles = process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] !== 'false';
 
 	const restrictedPaths = blockFileAccessToN8nFiles ? getN8nRestrictedPaths() : [];
@@ -64,8 +67,8 @@
 }
 
 export const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => ({
-	async createReadStream(filePath) {
-		if (await isFilePathBlocked(filePath.toString())) {
+	async createReadStream(resolvedFilePath) {
+		if (isFilePathBlocked(resolvedFilePath)) {
 			const allowedPaths = getAllowedPaths();
 			const message = allowedPaths.length ? ` Allowed paths: ${allowedPaths.join(', ')}` : '';
 			throw new NodeOperationError(node, `Access to the file is not allowed.${message}`, {
@@ -74,34 +77,58 @@
 		}
 
 		try {
-			await fsAccess(filePath);
+			await fsAccess(resolvedFilePath);
 		} catch (error) {
 			// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
 			throw error.code === 'ENOENT'
 				? // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
 					new NodeOperationError(node, error, {
-						message: `The file "${String(filePath)}" could not be accessed.`,
+						message: `The file "${String(resolvedFilePath)}" could not be accessed.`,
 						level: 'warning',
 					})
 				: error;
 		}
 
-		return createReadStream(filePath);
+		// Use O_NOFOLLOW to prevent TOCTOU race conditions where the file could be
+		// swapped to a symlink after validation but before opening
+		try {
+			return createReadStream(resolvedFilePath, {
+				// TypeScript types expect flags to be a string, but Node.js accepts numbers
+				// We use numeric flags here to combine O_RDONLY and O_NOFOLLOW bitwise
+				flags: (constants.O_RDONLY | constants.O_NOFOLLOW) as unknown as string,
+			});
+		} catch (error) {
+			// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+			if (error.code === 'ELOOP') {
+				throw new NodeOperationError(
+					node,
+					`The file "${String(resolvedFilePath)}" could not be opened.`,
+					{
+						level: 'warning',
+					},
+				);
+			}
+			throw error;
+		}
 	},
 
 	getStoragePath() {
 		return safeJoinPath(Container.get(InstanceSettings).n8nFolder, `storage/${node.type}`);
 	},
 
-	async writeContentToFile(filePath, content, flag) {
-		if (await isFilePathBlocked(filePath as string)) {
-			throw new NodeOperationError(node, `The file "${String(filePath)}" is not writable.`, {
-				level: 'warning',
-			});
+	async writeContentToFile(resolvedFilePath, content, flag) {
+		if (isFilePathBlocked(resolvedFilePath)) {
+			throw new NodeOperationError(
+				node,
+				`The file "${String(resolvedFilePath)}" is not writable.`,
+				{
+					level: 'warning',
+				},
+			);
 		}
-		return await fsWriteFile(filePath, content, { encoding: 'binary', flag });
+		return await fsWriteFile(resolvedFilePath, content, { encoding: 'binary', flag });
 	},
-
+	resolvePath,
 	isFilePathBlocked,
 });
 

packages/nodes-base/nodes/Files/ReadWriteFile/actions/read.operation.ts

@@ -101,7 +101,9 @@ export async function execute(this: IExecuteFunctions, items: INodeExecutionData
 
 			const newItems: INodeExecutionData[] = [];
 			for (const filePath of files) {
-				const stream = await this.helpers.createReadStream(filePath);
+				const stream = await this.helpers.createReadStream(
+					await this.helpers.resolvePath(filePath),
+				);
 				const binaryData = await this.helpers.prepareBinaryData(stream, filePath);
 
 				if (options.fileName !== undefined) {

packages/nodes-base/nodes/Files/ReadWriteFile/actions/write.operation.ts

@@ -90,7 +90,11 @@ export async function execute(this: IExecuteFunctions, items: INodeExecutionData
 			}
 
 			// Write the file to disk
-			await this.helpers.writeContentToFile(fileName, fileContent, flag);
+			await this.helpers.writeContentToFile(
+				await this.helpers.resolvePath(fileName),
+				fileContent,
+				flag,
+			);
 
 			if (item.binary !== undefined) {
 				// Create a shallow copy of the binary data so that the old