mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
fix(core): Only resolve the filepath once (#22767)
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
This commit is contained in:
+86
-37
@@ -1,7 +1,7 @@
|
||||
import { SecurityConfig } from '@n8n/config';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { createReadStream } from 'node:fs';
|
||||
import { constants, createReadStream } from 'node:fs';
|
||||
import { access as fsAccess, realpath as fsRealpath } from 'node:fs/promises';
|
||||
import { join } from 'node:path';
|
||||
|
||||
@@ -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,63 @@ 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);
|
||||
|
||||
// Mock createReadStream to return a proper stream-like object
|
||||
const mockStream: { once: jest.Mock } = {
|
||||
once: jest.fn((event: string, callback: (error?: Error) => void): typeof mockStream => {
|
||||
if (event === 'open') {
|
||||
// Immediately call the open callback
|
||||
setImmediate(() => callback());
|
||||
}
|
||||
return mockStream;
|
||||
}),
|
||||
};
|
||||
(createReadStream as jest.Mock).mockReturnValueOnce(mockStream);
|
||||
|
||||
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 return a stream that emits an error event
|
||||
const mockStream: { once: jest.Mock } = {
|
||||
once: jest.fn((event: string, callback: (error?: Error) => void): typeof mockStream => {
|
||||
if (event === 'error') {
|
||||
// Emit the error asynchronously
|
||||
setImmediate(() => callback(eloopError));
|
||||
}
|
||||
return mockStream;
|
||||
}),
|
||||
};
|
||||
(createReadStream as jest.Mock).mockReturnValueOnce(mockStream);
|
||||
|
||||
await expect(
|
||||
helperFunctions.createReadStream(await helperFunctions.resolvePath(filePath)),
|
||||
).rejects.toThrow('ELOOP: too many symbolic links encountered');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -253,9 +302,9 @@ describe('getFileSystemHelperFunctions', () => {
|
||||
|
||||
await expect(
|
||||
helperFunctions.writeContentToFile(
|
||||
instanceSettings.n8nFolder + '/test.txt',
|
||||
await helperFunctions.resolvePath(instanceSettings.n8nFolder + '/test.txt'),
|
||||
'content',
|
||||
'w',
|
||||
constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC,
|
||||
),
|
||||
).rejects.toThrow('not writable');
|
||||
});
|
||||
|
||||
+50
-21
@@ -1,9 +1,10 @@
|
||||
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 { FileSystemHelperFunctions, INode, ResolvedFilePath } from 'n8n-workflow';
|
||||
import type { PathLike } from 'node:fs';
|
||||
import { constants, createReadStream } from 'node:fs';
|
||||
import {
|
||||
access as fsAccess,
|
||||
writeFile as fsWriteFile,
|
||||
@@ -35,18 +36,19 @@ const getAllowedPaths = () => {
|
||||
return allowedPaths;
|
||||
};
|
||||
|
||||
export async function isFilePathBlocked(filePath: string): Promise<boolean> {
|
||||
const allowedPaths = getAllowedPaths();
|
||||
let resolvedFilePath = '';
|
||||
async function resolvePath(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 +66,8 @@ export async function isFilePathBlocked(filePath: string): Promise<boolean> {
|
||||
}
|
||||
|
||||
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 +76,61 @@ export const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunct
|
||||
}
|
||||
|
||||
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 createReadStream from following symlinks. We require that the path
|
||||
// already be resolved beforehand.
|
||||
const stream = createReadStream(resolvedFilePath, {
|
||||
flags: (constants.O_RDONLY | constants.O_NOFOLLOW) as unknown as string,
|
||||
});
|
||||
|
||||
return await new Promise<ReturnType<typeof createReadStream>>((resolve, reject) => {
|
||||
stream.once('error', (error) => {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ELOOP') {
|
||||
reject(
|
||||
new NodeOperationError(node, error, {
|
||||
level: 'warning',
|
||||
description: 'Symlinks are not allowed.',
|
||||
}),
|
||||
);
|
||||
} else {
|
||||
reject(error);
|
||||
}
|
||||
});
|
||||
stream.once('open', () => resolve(stream));
|
||||
});
|
||||
},
|
||||
|
||||
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: (flag ?? 0) | constants.O_NOFOLLOW,
|
||||
});
|
||||
},
|
||||
|
||||
resolvePath,
|
||||
isFilePathBlocked,
|
||||
});
|
||||
|
||||
|
||||
@@ -10,7 +10,12 @@ describe('Test Crypto Node', () => {
|
||||
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;
|
||||
fs.createReadStream = () => {
|
||||
const stream = Readable.from(Buffer.from('test')) as fs.ReadStream;
|
||||
// Emit 'open' event asynchronously to match real fs.ReadStream behavior
|
||||
setImmediate(() => stream.emit('open'));
|
||||
return stream;
|
||||
};
|
||||
|
||||
new NodeTestHarness().setupTests();
|
||||
});
|
||||
|
||||
@@ -105,7 +105,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) {
|
||||
|
||||
@@ -10,6 +10,7 @@ import type { Readable } from 'stream';
|
||||
import { updateDisplayOptions } from '@utils/utilities';
|
||||
|
||||
import { errorMapper } from '../helpers/utils';
|
||||
import { constants } from 'node:fs';
|
||||
|
||||
export const properties: INodeProperties[] = [
|
||||
{
|
||||
@@ -68,7 +69,9 @@ export async function execute(this: IExecuteFunctions, items: INodeExecutionData
|
||||
const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex);
|
||||
fileName = this.getNodeParameter('fileName', itemIndex) as string;
|
||||
const options = this.getNodeParameter('options', itemIndex, {});
|
||||
const flag: string = options.append ? 'a' : 'w';
|
||||
const flag: number = options.append
|
||||
? constants.O_APPEND
|
||||
: constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC;
|
||||
|
||||
item = items[itemIndex];
|
||||
|
||||
@@ -90,7 +93,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
|
||||
|
||||
@@ -287,7 +287,8 @@ export class Git implements INodeType {
|
||||
for (let itemIndex = 0; itemIndex < items.length; itemIndex++) {
|
||||
try {
|
||||
const repositoryPath = this.getNodeParameter('repositoryPath', itemIndex, '') as string;
|
||||
const isFilePathBlocked = await this.helpers.isFilePathBlocked(repositoryPath);
|
||||
const resolvedRepositoryPath = await this.helpers.resolvePath(repositoryPath);
|
||||
const isFilePathBlocked = this.helpers.isFilePathBlocked(resolvedRepositoryPath);
|
||||
if (isFilePathBlocked) {
|
||||
throw new NodeOperationError(
|
||||
this.getNode(),
|
||||
@@ -300,9 +301,9 @@ export class Git implements INodeType {
|
||||
if (operation === 'clone') {
|
||||
// Create repository folder if it does not exist
|
||||
try {
|
||||
await access(repositoryPath);
|
||||
await access(resolvedRepositoryPath);
|
||||
} catch (error) {
|
||||
await mkdir(repositoryPath);
|
||||
await mkdir(resolvedRepositoryPath);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -321,7 +322,7 @@ export class Git implements INodeType {
|
||||
}
|
||||
|
||||
const gitOptions: Partial<SimpleGitOptions> = {
|
||||
baseDir: repositoryPath,
|
||||
baseDir: resolvedRepositoryPath,
|
||||
config: gitConfig,
|
||||
};
|
||||
|
||||
|
||||
@@ -145,11 +145,42 @@ describe('Git Node', () => {
|
||||
|
||||
describe('Restricted file paths', () => {
|
||||
it('should throw an error if the repository path is blocked', async () => {
|
||||
(executeFunctions.helpers.isFilePathBlocked as jest.Mock).mockResolvedValue(true);
|
||||
(executeFunctions.helpers.isFilePathBlocked as jest.Mock).mockReturnValue(true);
|
||||
(executeFunctions.helpers.resolvePath as jest.Mock).mockResolvedValue('/tmp/test-repo');
|
||||
|
||||
await expect(gitNode.execute.call(executeFunctions)).rejects.toThrow(
|
||||
'Access to the repository path is not allowed',
|
||||
);
|
||||
});
|
||||
|
||||
it('should use the resolved repository path for git operations', async () => {
|
||||
const originalPath = '/tmp/link-to-repo';
|
||||
const resolvedPath = '/tmp/actual-repo';
|
||||
|
||||
executeFunctions.getNodeParameter.mockImplementation((name: string) => {
|
||||
switch (name) {
|
||||
case 'operation':
|
||||
return 'log';
|
||||
case 'repositoryPath':
|
||||
return originalPath;
|
||||
case 'options':
|
||||
return {};
|
||||
default:
|
||||
return '';
|
||||
}
|
||||
});
|
||||
|
||||
(executeFunctions.helpers.resolvePath as jest.Mock).mockResolvedValue(resolvedPath);
|
||||
(executeFunctions.helpers.isFilePathBlocked as jest.Mock).mockReturnValue(false);
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
// Verify git is initialized with the resolved path, not the original
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseDir: resolvedPath,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -50,6 +50,8 @@ describe('Git Node', () => {
|
||||
continueOnFail: jest.fn(() => false),
|
||||
helpers: {
|
||||
returnJsonArray: jest.fn((data: any[]) => data.map((item: any) => ({ json: item }))),
|
||||
resolvePath: jest.fn(async (path: string) => path as any),
|
||||
isFilePathBlocked: jest.fn(() => false),
|
||||
},
|
||||
});
|
||||
jest.clearAllMocks();
|
||||
|
||||
@@ -69,7 +69,9 @@ export class ReadBinaryFile implements INodeType {
|
||||
|
||||
const filePath = this.getNodeParameter('filePath', itemIndex);
|
||||
|
||||
const stream = await this.helpers.createReadStream(filePath);
|
||||
const stream = await this.helpers.createReadStream(
|
||||
await this.helpers.resolvePath(filePath),
|
||||
);
|
||||
const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex);
|
||||
|
||||
newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData(stream, filePath);
|
||||
|
||||
@@ -54,7 +54,7 @@ export class ReadBinaryFiles implements INodeType {
|
||||
|
||||
const items: INodeExecutionData[] = [];
|
||||
for (const filePath of files) {
|
||||
const stream = await this.helpers.createReadStream(filePath);
|
||||
const stream = await this.helpers.createReadStream(await this.helpers.resolvePath(filePath));
|
||||
items.push({
|
||||
binary: {
|
||||
[dataPropertyName]: await this.helpers.prepareBinaryData(stream, filePath),
|
||||
|
||||
@@ -5,6 +5,7 @@ import type {
|
||||
INodeType,
|
||||
INodeTypeDescription,
|
||||
} from 'n8n-workflow';
|
||||
import { constants } from 'node:fs';
|
||||
import type { Readable } from 'stream';
|
||||
|
||||
export class WriteBinaryFile implements INodeType {
|
||||
@@ -75,7 +76,9 @@ export class WriteBinaryFile implements INodeType {
|
||||
|
||||
const options = this.getNodeParameter('options', 0, {});
|
||||
|
||||
const flag: string = options.append ? 'a' : 'w';
|
||||
const flag: number = options.append
|
||||
? constants.O_APPEND
|
||||
: constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC;
|
||||
|
||||
item = items[itemIndex];
|
||||
|
||||
@@ -98,7 +101,11 @@ export class WriteBinaryFile implements INodeType {
|
||||
|
||||
// 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
|
||||
|
||||
@@ -694,14 +694,30 @@ export interface BaseHelperFunctions {
|
||||
returnJsonArray(jsonData: IDataObject | IDataObject[]): INodeExecutionData[];
|
||||
}
|
||||
|
||||
const __brand = Symbol('resolvedFilePath');
|
||||
|
||||
export type ResolvedFilePath = string & {
|
||||
[__brand]: 'ResolvedFilePath';
|
||||
};
|
||||
|
||||
export interface FileSystemHelperFunctions {
|
||||
isFilePathBlocked(filePath: string): Promise<boolean>;
|
||||
createReadStream(path: PathLike): Promise<Readable>;
|
||||
resolvePath(path: PathLike): Promise<ResolvedFilePath>;
|
||||
/**
|
||||
* Use {@link resolvePath} to resolve the path first.
|
||||
*/
|
||||
isFilePathBlocked(filePath: ResolvedFilePath): boolean;
|
||||
/**
|
||||
* Use {@link resolvePath} to resolve the path first.
|
||||
*/
|
||||
createReadStream(filePath: ResolvedFilePath): Promise<Readable>;
|
||||
getStoragePath(): string;
|
||||
/**
|
||||
* Use {@link resolvePath} to resolve the path first.
|
||||
*/
|
||||
writeContentToFile(
|
||||
path: PathLike,
|
||||
path: ResolvedFilePath,
|
||||
content: string | Buffer | Readable,
|
||||
flag?: string,
|
||||
flag?: number,
|
||||
): Promise<void>;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user