feat: Add configurable require support to js task runner (no-changelog) (#11184)

This commit is contained in:
Tomi Turtiainen 2024-10-10 12:44:02 +03:00 committed by GitHub
parent cd59f33e59
commit eb2d1ca357
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 404 additions and 20 deletions

View file

@ -1,7 +1,10 @@
import { DateTime } from 'luxon';
import type { CodeExecutionMode, IDataObject } from 'n8n-workflow';
import fs from 'node:fs';
import { builtinModules } from 'node:module';
import { ValidationError } from '@/js-task-runner/errors/validation-error';
import type { JsTaskRunnerOpts } from '@/js-task-runner/js-task-runner';
import {
JsTaskRunner,
type AllCodeTaskData,
@ -14,17 +17,27 @@ import { newAllCodeTaskData, newTaskWithSettings, withPairedItem, wrapIntoJson }
jest.mock('ws');
describe('JsTaskRunner', () => {
const jsTaskRunner = new JsTaskRunner('taskType', 'ws://localhost', 'grantToken', 1);
const createRunnerWithOpts = (opts: Partial<JsTaskRunnerOpts> = {}) =>
new JsTaskRunner({
wsUrl: 'ws://localhost',
grantToken: 'grantToken',
maxConcurrency: 1,
...opts,
});
const defaultTaskRunner = createRunnerWithOpts();
const execTaskWithParams = async ({
task,
taskData,
runner = defaultTaskRunner,
}: {
task: Task<JSExecSettings>;
taskData: AllCodeTaskData;
runner?: JsTaskRunner;
}) => {
jest.spyOn(jsTaskRunner, 'requestData').mockResolvedValue(taskData);
return await jsTaskRunner.executeTask(task);
jest.spyOn(runner, 'requestData').mockResolvedValue(taskData);
return await runner.executeTask(task);
};
afterEach(() => {
@ -35,7 +48,13 @@ describe('JsTaskRunner', () => {
code,
inputItems,
settings,
}: { code: string; inputItems: IDataObject[]; settings?: Partial<JSExecSettings> }) => {
runner,
}: {
code: string;
inputItems: IDataObject[];
settings?: Partial<JSExecSettings>;
runner?: JsTaskRunner;
}) => {
return await execTaskWithParams({
task: newTaskWithSettings({
code,
@ -43,6 +62,7 @@ describe('JsTaskRunner', () => {
...settings,
}),
taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson)),
runner,
});
};
@ -50,7 +70,14 @@ describe('JsTaskRunner', () => {
code,
inputItems,
settings,
}: { code: string; inputItems: IDataObject[]; settings?: Partial<JSExecSettings> }) => {
runner,
}: {
code: string;
inputItems: IDataObject[];
settings?: Partial<JSExecSettings>;
runner?: JsTaskRunner;
}) => {
return await execTaskWithParams({
task: newTaskWithSettings({
code,
@ -58,6 +85,7 @@ describe('JsTaskRunner', () => {
...settings,
}),
taskData: newAllCodeTaskData(inputItems.map(wrapIntoJson)),
runner,
});
};
@ -65,7 +93,7 @@ describe('JsTaskRunner', () => {
test.each<[CodeExecutionMode]>([['runOnceForAllItems'], ['runOnceForEachItem']])(
'should make an rpc call for console log in %s mode',
async (nodeMode) => {
jest.spyOn(jsTaskRunner, 'makeRpcCall').mockResolvedValue(undefined);
jest.spyOn(defaultTaskRunner, 'makeRpcCall').mockResolvedValue(undefined);
const task = newTaskWithSettings({
code: "console.log('Hello', 'world!'); return {}",
nodeMode,
@ -76,7 +104,7 @@ describe('JsTaskRunner', () => {
taskData: newAllCodeTaskData([wrapIntoJson({})]),
});
expect(jsTaskRunner.makeRpcCall).toHaveBeenCalledWith(task.taskId, 'logNodeOutput', [
expect(defaultTaskRunner.makeRpcCall).toHaveBeenCalledWith(task.taskId, 'logNodeOutput', [
'Hello world!',
]);
},
@ -452,4 +480,230 @@ describe('JsTaskRunner', () => {
},
);
});
describe('require', () => {
const inputItems = [{ a: 1 }];
const packageJson = JSON.parse(fs.readFileSync('package.json', 'utf8'));
describe('blocked by default', () => {
const testCases = [...builtinModules, ...Object.keys(packageJson.dependencies)];
test.each(testCases)(
'should throw an error when requiring %s in runOnceForAllItems mode',
async (module) => {
await expect(
executeForAllItems({
code: `return require('${module}')`,
inputItems,
}),
).rejects.toThrow(`Cannot find module '${module}'`);
},
);
test.each(testCases)(
'should throw an error when requiring %s in runOnceForEachItem mode',
async (module) => {
await expect(
executeForEachItem({
code: `return require('${module}')`,
inputItems,
}),
).rejects.toThrow(`Cannot find module '${module}'`);
},
);
});
describe('all built-ins allowed with *', () => {
const testCases = builtinModules;
const runner = createRunnerWithOpts({
allowedBuiltInModules: '*',
});
test.each(testCases)(
'should be able to require %s in runOnceForAllItems mode',
async (module) => {
await expect(
executeForAllItems({
code: `return { val: require('${module}') }`,
inputItems,
runner,
}),
).resolves.toBeDefined();
},
);
test.each(testCases)(
'should be able to require %s in runOnceForEachItem mode',
async (module) => {
await expect(
executeForEachItem({
code: `return { val: require('${module}') }`,
inputItems,
runner,
}),
).resolves.toBeDefined();
},
);
});
describe('all external modules allowed with *', () => {
const testCases = Object.keys(packageJson.dependencies);
const runner = createRunnerWithOpts({
allowedExternalModules: '*',
});
test.each(testCases)(
'should be able to require %s in runOnceForAllItems mode',
async (module) => {
await expect(
executeForAllItems({
code: `return { val: require('${module}') }`,
inputItems,
runner,
}),
).resolves.toBeDefined();
},
);
test.each(testCases)(
'should be able to require %s in runOnceForEachItem mode',
async (module) => {
await expect(
executeForEachItem({
code: `return { val: require('${module}') }`,
inputItems,
runner,
}),
).resolves.toBeDefined();
},
);
});
describe('specifically allowed built-in modules', () => {
const runner = createRunnerWithOpts({
allowedBuiltInModules: 'crypto,path',
});
const allowedCases = [
['crypto', 'require("crypto").randomBytes(16).toString("hex")', expect.any(String)],
['path', 'require("path").normalize("/root/./dir")', '/root/dir'],
];
const blockedCases = [['http'], ['process']];
test.each(allowedCases)(
'should allow requiring %s in runOnceForAllItems mode',
async (_moduleName, expression, expected) => {
const outcome = await executeForAllItems({
code: `return { val: ${expression} }`,
inputItems,
runner,
});
expect(outcome.result).toEqual([wrapIntoJson({ val: expected })]);
},
);
test.each(allowedCases)(
'should allow requiring %s in runOnceForEachItem mode',
async (_moduleName, expression, expected) => {
const outcome = await executeForEachItem({
code: `return { val: ${expression} }`,
inputItems,
runner,
});
expect(outcome.result).toEqual([withPairedItem(0, wrapIntoJson({ val: expected }))]);
},
);
test.each(blockedCases)(
'should throw when trying to require %s in runOnceForAllItems mode',
async (moduleName) => {
await expect(
executeForAllItems({
code: `require("${moduleName}")`,
inputItems,
runner,
}),
).rejects.toThrow(`Cannot find module '${moduleName}'`);
},
);
test.each(blockedCases)(
'should throw when trying to require %s in runOnceForEachItem mode',
async (moduleName) => {
await expect(
executeForEachItem({
code: `require("${moduleName}")`,
inputItems,
runner,
}),
).rejects.toThrow(`Cannot find module '${moduleName}'`);
},
);
});
describe('specifically allowed external modules', () => {
const runner = createRunnerWithOpts({
allowedExternalModules: 'nanoid',
});
const allowedCases = [['nanoid', 'require("nanoid").nanoid()', expect.any(String)]];
const blockedCases = [['n8n-core']];
test.each(allowedCases)(
'should allow requiring %s in runOnceForAllItems mode',
async (_moduleName, expression, expected) => {
const outcome = await executeForAllItems({
code: `return { val: ${expression} }`,
inputItems,
runner,
});
expect(outcome.result).toEqual([wrapIntoJson({ val: expected })]);
},
);
test.each(allowedCases)(
'should allow requiring %s in runOnceForEachItem mode',
async (_moduleName, expression, expected) => {
const outcome = await executeForEachItem({
code: `return { val: ${expression} }`,
inputItems,
runner,
});
expect(outcome.result).toEqual([withPairedItem(0, wrapIntoJson({ val: expected }))]);
},
);
test.each(blockedCases)(
'should throw when trying to require %s in runOnceForAllItems mode',
async (moduleName) => {
await expect(
executeForAllItems({
code: `require("${moduleName}")`,
inputItems,
runner,
}),
).rejects.toThrow(`Cannot find module '${moduleName}'`);
},
);
test.each(blockedCases)(
'should throw when trying to require %s in runOnceForEachItem mode',
async (moduleName) => {
await expect(
executeForEachItem({
code: `require("${moduleName}")`,
inputItems,
runner,
}),
).rejects.toThrow(`Cannot find module '${moduleName}'`);
},
);
});
});
});

View file

@ -25,6 +25,8 @@ import { runInNewContext, type Context } from 'node:vm';
import type { TaskResultData } from '@/runner-types';
import { type Task, TaskRunner } from '@/task-runner';
import type { RequireResolver } from './require-resolver';
import { createRequireResolver } from './require-resolver';
import { validateRunForAllItemsOutput, validateRunForEachItemOutput } from './result-validation';
export interface JSExecSettings {
@ -72,19 +74,47 @@ export interface AllCodeTaskData {
additionalData: PartialAdditionalData;
}
export interface JsTaskRunnerOpts {
wsUrl: string;
grantToken: string;
maxConcurrency: number;
name?: string;
/**
* List of built-in nodejs modules that are allowed to be required in the
* execution sandbox. Asterisk (*) can be used to allow all.
*/
allowedBuiltInModules?: string;
/**
* List of npm modules that are allowed to be required in the execution
* sandbox. Asterisk (*) can be used to allow all.
*/
allowedExternalModules?: string;
}
type CustomConsole = {
log: (...args: unknown[]) => void;
};
export class JsTaskRunner extends TaskRunner {
constructor(
taskType: string,
wsUrl: string,
grantToken: string,
maxConcurrency: number,
name?: string,
) {
super(taskType, wsUrl, grantToken, maxConcurrency, name ?? 'JS Task Runner');
private readonly requireResolver: RequireResolver;
constructor({
grantToken,
maxConcurrency,
wsUrl,
name = 'JS Task Runner',
allowedBuiltInModules,
allowedExternalModules,
}: JsTaskRunnerOpts) {
super('javascript', wsUrl, grantToken, maxConcurrency, name);
const parseModuleAllowList = (moduleList: string) =>
moduleList === '*' ? null : new Set(moduleList.split(',').map((x) => x.trim()));
this.requireResolver = createRequireResolver({
allowedBuiltInModules: parseModuleAllowList(allowedBuiltInModules ?? ''),
allowedExternalModules: parseModuleAllowList(allowedExternalModules ?? ''),
});
}
async executeTask(task: Task<JSExecSettings>): Promise<TaskResultData> {
@ -145,7 +175,7 @@ export class JsTaskRunner extends TaskRunner {
const inputItems = allData.connectionInputData;
const context: Context = {
require,
require: this.requireResolver,
module: {},
console: customConsole,
@ -192,7 +222,7 @@ export class JsTaskRunner extends TaskRunner {
const item = inputItems[index];
const dataProxy = this.createDataProxy(allData, workflow, index);
const context: Context = {
require,
require: this.requireResolver,
module: {},
console: customConsole,
item,

View file

@ -0,0 +1,43 @@
import { ApplicationError } from 'n8n-workflow';
import { isBuiltin } from 'node:module';
import { ExecutionError } from './errors/execution-error';
export type RequireResolverOpts = {
/**
* List of built-in nodejs modules that are allowed to be required in the
* execution sandbox. `null` means all are allowed.
*/
allowedBuiltInModules: Set<string> | null;
/**
* List of external modules that are allowed to be required in the
* execution sandbox. `null` means all are allowed.
*/
allowedExternalModules: Set<string> | null;
};
export type RequireResolver = (request: string) => unknown;
export function createRequireResolver({
allowedBuiltInModules,
allowedExternalModules,
}: RequireResolverOpts) {
return (request: string) => {
const checkIsAllowed = (allowList: Set<string> | null, moduleName: string) => {
return allowList ? allowList.has(moduleName) : true;
};
const isAllowed = isBuiltin(request)
? checkIsAllowed(allowedBuiltInModules, request)
: checkIsAllowed(allowedExternalModules, request);
if (!isAllowed) {
const error = new ApplicationError(`Cannot find module '${request}'`);
throw new ExecutionError(error);
}
// eslint-disable-next-line @typescript-eslint/no-var-requires
return require(request) as unknown;
};
}

View file

@ -66,7 +66,13 @@ void (async function start() {
}
const wsUrl = `ws://${config.n8nUri}/runners/_ws`;
runner = new JsTaskRunner('javascript', wsUrl, grantToken, 5);
runner = new JsTaskRunner({
wsUrl,
grantToken,
maxConcurrency: 5,
allowedBuiltInModules: process.env.NODE_FUNCTION_ALLOW_BUILTIN,
allowedExternalModules: process.env.NODE_FUNCTION_ALLOW_EXTERNAL,
});
process.on('SIGINT', createSignalHandler('SIGINT'));
process.on('SIGTERM', createSignalHandler('SIGTERM'));

View file

@ -0,0 +1,48 @@
import { GlobalConfig } from '@n8n/config';
import { mock } from 'jest-mock-extended';
import type { ChildProcess, SpawnOptions } from 'node:child_process';
import { mockInstance } from '../../../test/shared/mocking';
import type { TaskRunnerAuthService } from '../auth/task-runner-auth.service';
import { TaskRunnerProcess } from '../task-runner-process';
const spawnMock = jest.fn(() =>
mock<ChildProcess>({
stdout: {
pipe: jest.fn(),
},
stderr: {
pipe: jest.fn(),
},
}),
);
require('child_process').spawn = spawnMock;
describe('TaskRunnerProcess', () => {
const globalConfig = mockInstance(GlobalConfig);
const authService = mock<TaskRunnerAuthService>();
const taskRunnerProcess = new TaskRunnerProcess(globalConfig, authService);
afterEach(async () => {
spawnMock.mockClear();
});
describe('start', () => {
it('should propagate NODE_FUNCTION_ALLOW_BUILTIN and NODE_FUNCTION_ALLOW_EXTERNAL from env', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
process.env.NODE_FUNCTION_ALLOW_BUILTIN = '*';
process.env.NODE_FUNCTION_ALLOW_EXTERNAL = '*';
await taskRunnerProcess.start();
// @ts-expect-error The type is not correct
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(options.env).toEqual(
expect.objectContaining({
NODE_FUNCTION_ALLOW_BUILTIN: '*',
NODE_FUNCTION_ALLOW_EXTERNAL: '*',
}),
);
});
});
});

View file

@ -1,6 +1,7 @@
import { GlobalConfig } from '@n8n/config';
import * as a from 'node:assert/strict';
import { spawn } from 'node:child_process';
import * as process from 'node:process';
import { Service } from 'typedi';
import { TaskRunnerAuthService } from './auth/task-runner-auth.service';
@ -45,6 +46,8 @@ export class TaskRunnerProcess {
PATH: process.env.PATH,
N8N_RUNNERS_GRANT_TOKEN: grantToken,
N8N_RUNNERS_N8N_URI: `127.0.0.1:${this.globalConfig.taskRunners.port}`,
NODE_FUNCTION_ALLOW_BUILTIN: process.env.NODE_FUNCTION_ALLOW_BUILTIN,
NODE_FUNCTION_ALLOW_EXTERNAL: process.env.NODE_FUNCTION_ALLOW_EXTERNAL,
},
});
@ -69,9 +72,9 @@ export class TaskRunnerProcess {
this.isShuttingDown = false;
}
private monitorProcess(process: ChildProcess) {
private monitorProcess(taskRunnerProcess: ChildProcess) {
this.runPromise = new Promise((resolve) => {
process.on('exit', (code) => {
taskRunnerProcess.on('exit', (code) => {
this.onProcessExit(code, resolve);
});
});