Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Restrict read/write file paths access #6582

Merged
merged 17 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1c3e435
:zap: fix
michael-radency Jul 3, 2023
51e2ab0
Merge branch 'master' of https://github.com/n8n-io/n8n into n8n-6490-…
michael-radency Jul 3, 2023
354ac8f
:zap: clean up, allowed paths notice
michael-radency Jul 4, 2023
5e0c57e
Merge branch 'master' of https://github.com/n8n-io/n8n into n8n-6490-…
michael-radency Jul 4, 2023
7e665e5
:zap: resolve relative paths
michael-radency Jul 4, 2023
00a2246
:zap: added schema entries
michael-radency Jul 4, 2023
0c70615
Fix user n8n folder to work on windows
maspio Jul 18, 2023
d1adab5
removed ReadBinaryFiles throwing an error if no files
maspio Jul 18, 2023
feddafb
Merge branch 'master' of https://github.com/n8n-io/n8n into n8n-6490-…
michael-radency Jul 28, 2023
3f9270e
Merge branch 'master' of https://github.com/n8n-io/n8n into n8n-6490-…
michael-radency Jul 28, 2023
ad8647a
Merge branch 'master' of https://github.com/n8n-io/n8n into n8n-6490-…
michael-radency Jul 29, 2023
033d28d
:zap: based on https://github.com/n8n-io/n8n/commit/351670fbaffa844e8…
michael-radency Jul 31, 2023
03a9852
:zap: clean up
michael-radency Jul 31, 2023
a56a4af
Merge branch 'master' of https://github.com/n8n-io/n8n into n8n-6490-…
michael-radency Jul 31, 2023
58780c0
respect BLOCK_FILE_ACCESS_TO_N8N_FILES to disable blocking .n8n access
maspio Jul 31, 2023
05c925e
improve BLOCK_FILE_ACCESS_TO_N8N_FILES to check after N8N_RESTRICT_FI…
maspio Jul 31, 2023
2cbde3e
fix typo
maspio Jul 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,18 @@ export const schema = {
},

security: {
restrictFileAccessTo: {
doc: 'If set only files in that directories can be accessed. Multiple directories can be separated by semicolon (";").',
format: String,
default: '',
env: 'N8N_RESTRICT_FILE_ACCESS_TO',
},
blockFileAccessToN8nFiles: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be optional. we should always block access to .n8n folder

Copy link
Contributor Author

@michael-radency michael-radency Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree regarding .n8n folder and we should add this to NodeExecuteFunctions as in proposed changes commit, but as stated in specs it also forbid access to some user defined config/templates files, and this part, I think, should be optional.

doc: 'If set to true it will block access to all files in the ".n8n" directory and user defined config files.',
format: Boolean,
default: true,
env: 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES',
},
audit: {
daysAbandonedWorkflow: {
doc: 'Days for a workflow to be considered abandoned if not executed',
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,10 @@ export const RESPONSE_ERROR_MESSAGES = {
};

export const CUSTOM_NODES_CATEGORY = 'Custom Nodes';

export const RESTRICT_FILE_ACCESS_TO = 'N8N_RESTRICT_FILE_ACCESS_TO';
export const BLOCK_FILE_ACCESS_TO_N8N_FILES = 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES';
export const CONFIG_FILES = 'N8N_CONFIG_FILES';
export const BINARY_DATA_STORAGE_PATH = 'N8N_BINARY_DATA_STORAGE_PATH';
export const UM_EMAIL_TEMPLATES_INVITE = 'N8N_UM_EMAIL_TEMPLATES_INVITE';
export const UM_EMAIL_TEMPLATES_PWRESET = 'N8N_UM_EMAIL_TEMPLATES_PWRESET';
97 changes: 95 additions & 2 deletions packages/core/src/NodeExecuteFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,23 @@ import type {
import axios from 'axios';
import url, { URL, URLSearchParams } from 'url';
import { Readable } from 'stream';
import { access as fsAccess } from 'fs/promises';
import { access as fsAccess, writeFile as fsWriteFile } from 'fs/promises';
import { createReadStream } from 'fs';

import { BinaryDataManager } from './BinaryDataManager';
import type { ExtendedValidationResult, IResponseError, IWorkflowSettings } from './Interfaces';
import { extractValue } from './ExtractValue';
import { getClientCredentialsToken } from './OAuth2Helper';
import { PLACEHOLDER_EMPTY_EXECUTION_ID } from './Constants';
import {
CUSTOM_EXTENSION_ENV,
PLACEHOLDER_EMPTY_EXECUTION_ID,
BLOCK_FILE_ACCESS_TO_N8N_FILES,
RESTRICT_FILE_ACCESS_TO,
CONFIG_FILES,
BINARY_DATA_STORAGE_PATH,
UM_EMAIL_TEMPLATES_INVITE,
UM_EMAIL_TEMPLATES_PWRESET,
} from './Constants';
import { binaryToBuffer } from './BinaryDataManager/utils';
import {
getAllWorkflowExecutionMetadata,
Expand Down Expand Up @@ -2242,6 +2251,72 @@ const getRequestHelperFunctions = (
},
});

const getAllowedPaths = () => {
const restrictFileAccessTo = process.env[RESTRICT_FILE_ACCESS_TO];
if (!restrictFileAccessTo) {
return [];
}
const allowedPaths = restrictFileAccessTo
.split(';')
.map((path) => path.trim())
.filter((path) => path);
return allowedPaths;
};

function isFilePathBlocked(filePath: string): boolean {
const allowedPaths = getAllowedPaths();
const resolvedFilePath = path.resolve(filePath);
const userFolder = getUserN8nFolderPath();
const blockFileAccessToN8nFiles = process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] !== 'false';

//if allowed paths are defined, allow access only to those paths
if (allowedPaths.length) {
for (const path of allowedPaths) {
if (resolvedFilePath.startsWith(path)) {
return false;
}
}

return true;
}

//restrict access to .n8n folder and other .env config related paths
if (blockFileAccessToN8nFiles) {
const restrictedPaths: string[] = [userFolder];

if (process.env[CONFIG_FILES]) {
restrictedPaths.push(...process.env[CONFIG_FILES].split(','));
}

if (process.env[CUSTOM_EXTENSION_ENV]) {
const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV].split(';');
restrictedPaths.push(...customExtensionFolders);
}

if (process.env[BINARY_DATA_STORAGE_PATH]) {
restrictedPaths.push(process.env[BINARY_DATA_STORAGE_PATH]);
}

if (process.env[UM_EMAIL_TEMPLATES_INVITE]) {
restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_INVITE]);
}

if (process.env[UM_EMAIL_TEMPLATES_PWRESET]) {
restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_PWRESET]);
}

//check if the file path is restricted
for (const path of restrictedPaths) {
if (resolvedFilePath.startsWith(path)) {
return true;
}
}
}

//path is not restricted
return false;
}

const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => ({
async createReadStream(filePath) {
try {
Expand All @@ -2250,14 +2325,32 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions =>
throw error.code === 'ENOENT'
? new NodeOperationError(node, error, {
message: `The file "${String(filePath)}" could not be accessed.`,
severity: 'warning',
})
: error;
}
if (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}`, {
severity: 'warning',
});
}
return createReadStream(filePath);
},

getStoragePath() {
return path.join(getUserN8nFolderPath(), `storage/${node.type}`);
},

async writeContentToFile(filePath, content, flag) {
if (isFilePathBlocked(filePath as string)) {
throw new NodeOperationError(node, `The file "${String(filePath)}" is not writable.`, {
severity: 'warning',
});
}
return fsWriteFile(filePath, content, { encoding: 'binary', flag });
},
});

const getNodeHelperFunctions = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,17 @@ export class ReadBinaryFile implements INodeType {
}

const filePath = this.getNodeParameter('filePath', itemIndex);

const stream = await this.helpers.createReadStream(filePath);
const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData(stream, filePath);
returnData.push(newItem);
} catch (error) {
if (this.continueOnFail()) {
returnData.push({
json: {
error: error.message,
error: (error as Error).message,
},
pairedItem: {
item: itemIndex,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
INodeType,
INodeTypeDescription,
} from 'n8n-workflow';

import glob from 'fast-glob';

export class ReadBinaryFiles implements INodeType {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import type { Readable } from 'stream';

import { BINARY_ENCODING } from 'n8n-workflow';
import type {
IExecuteFunctions,
INodeExecutionData,
INodeType,
INodeTypeDescription,
} from 'n8n-workflow';
import { BINARY_ENCODING } from 'n8n-workflow';

import { writeFile as fsWriteFile } from 'fs/promises';
import type { Readable } from 'stream';

export class WriteBinaryFile implements INodeType {
description: INodeTypeDescription = {
Expand Down Expand Up @@ -73,9 +72,10 @@ export class WriteBinaryFile implements INodeType {
const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex);

const fileName = this.getNodeParameter('fileName', itemIndex) as string;

const options = this.getNodeParameter('options', 0, {});

const flag = options.append ? 'a' : 'w';
const flag: string = options.append ? 'a' : 'w';

item = items[itemIndex];

Expand All @@ -97,7 +97,8 @@ export class WriteBinaryFile implements INodeType {
}

// Write the file to disk
await fsWriteFile(fileName, fileContent, { encoding: 'binary', flag });
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
await this.helpers.writeContentToFile(fileName, fileContent, flag);

if (item.binary !== undefined) {
// Create a shallow copy of the binary data so that the old
Expand All @@ -116,7 +117,7 @@ export class WriteBinaryFile implements INodeType {
if (this.continueOnFail()) {
returnData.push({
json: {
error: error.message,
error: (error as Error).message,
},
pairedItem: {
item: itemIndex,
Expand Down
18 changes: 10 additions & 8 deletions packages/nodes-base/utils/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import { isEqual, isNull, merge } from 'lodash';
* // => [['a', 'b', 'c'], ['d']]
*/

export function chunk(array: any[], size = 1) {
const length = array == null ? 0 : array.length;
export function chunk<T>(array: T[], size = 1) {
const length = array === null ? 0 : array.length;
if (!length || size < 1) {
return [];
}
Expand All @@ -37,7 +37,7 @@ export function chunk(array: any[], size = 1) {
while (index < length) {
result[resIndex++] = array.slice(index, (index += size));
}
return result;
return result as T[][];
}

/**
Expand All @@ -51,20 +51,22 @@ export function chunk(array: any[], size = 1) {
*
*/

export function flatten(nestedArray: any[][]) {
export function flatten<T>(nestedArray: T[][]) {
const result = [];

(function loop(array: any[] | any) {
(function loop(array: T[] | T[][]) {
for (let i = 0; i < array.length; i++) {
if (Array.isArray(array[i])) {
loop(array[i]);
loop(array[i] as T[]);
} else {
result.push(array[i]);
}
}
})(nestedArray);

return result;
//TODO: check logic in MicrosoftSql.node.ts
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-return
return result as any;
}

export function updateDisplayOptions(
Expand Down Expand Up @@ -210,7 +212,7 @@ export function wrapData(data: IDataObject | IDataObject[]): INodeExecutionData[
export const keysToLowercase = <T>(headers: T) => {
if (typeof headers !== 'object' || Array.isArray(headers) || headers === null) return headers;
return Object.entries(headers).reduce((acc, [key, value]) => {
acc[key.toLowerCase()] = value;
acc[key.toLowerCase()] = value as IDataObject;
return acc;
}, {} as IDataObject);
};
Expand Down
5 changes: 5 additions & 0 deletions packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,11 @@ interface JsonHelperFunctions {
export interface FileSystemHelperFunctions {
createReadStream(path: PathLike): Promise<Readable>;
getStoragePath(): string;
writeContentToFile(
path: PathLike,
content: string | Buffer | Readable,
flag?: string,
): Promise<void>;
}

export interface BinaryHelperFunctions {
Expand Down