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

This commit is contained in:
RomanDavydchuk
2025-08-01 10:59:53 +03:00
committed by GitHub
parent e8e7b23d47
commit c2c3e08cdf
5 changed files with 64 additions and 40 deletions

View File

@@ -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);
});
});

View File

@@ -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',
});