Skip to content

Commit

Permalink
fix(core): Disallow code generation in task runner (#12522)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomi authored Jan 9, 2025
1 parent 46f13cf commit 35b6180
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 12 deletions.
5 changes: 4 additions & 1 deletion docker/images/n8n/n8n-task-runners.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
"runner-type": "javascript",
"workdir": "/home/node",
"command": "/usr/local/bin/node",
"args": ["/usr/local/lib/node_modules/n8n/node_modules/@n8n/task-runner/dist/start.js"],
"args": [
"--disallow-code-generation-from-strings",
"/usr/local/lib/node_modules/n8n/node_modules/@n8n/task-runner/dist/start.js"
],
"allowed-env": [
"PATH",
"GENERIC_TIMEZONE",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ describe('JsTaskRunner', () => {
['typeof clearInterval', 'function'],
['typeof clearImmediate', 'function'],
],
eval: [['eval("1+2")', 3]],
'JS built-ins': [
['typeof btoa', 'function'],
['typeof atob', 'function'],
Expand Down
16 changes: 6 additions & 10 deletions packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type {
} from 'n8n-workflow';
import * as a from 'node:assert';
import { inspect } from 'node:util';
import { runInNewContext, type Context } from 'node:vm';
import { type Context, createContext, runInContext } from 'node:vm';

import type { MainConfig } from '@/config/main-config';
import { UnsupportedFunctionError } from '@/js-task-runner/errors/unsupported-function.error';
Expand Down Expand Up @@ -158,10 +158,8 @@ export class JsTaskRunner extends TaskRunner {

private getNativeVariables() {
return {
// Exposed Node.js globals in vm2
// Exposed Node.js globals
Buffer,
Function,
eval,
setTimeout,
setInterval,
setImmediate,
Expand Down Expand Up @@ -205,7 +203,7 @@ export class JsTaskRunner extends TaskRunner {

signal.addEventListener('abort', abortHandler, { once: true });

const taskResult = runInNewContext(
const taskResult = runInContext(
`globalThis.global = globalThis; module.exports = async function VmCodeWrapper() {${settings.code}\n}()`,
context,
{ timeout: this.taskTimeout * 1000 },
Expand Down Expand Up @@ -268,7 +266,7 @@ export class JsTaskRunner extends TaskRunner {

signal.addEventListener('abort', abortHandler);

const taskResult = runInNewContext(
const taskResult = runInContext(
`module.exports = async function VmCodeWrapper() {${settings.code}\n}()`,
context,
{ timeout: this.taskTimeout * 1000 },
Expand Down Expand Up @@ -470,7 +468,7 @@ export class JsTaskRunner extends TaskRunner {
dataProxy: IWorkflowDataProxyData,
additionalProperties: Record<string, unknown> = {},
): Context {
const context: Context = {
return createContext({
[inspect.custom]: () => '[[ExecutionContext]]',
require: this.requireResolver,
module: {},
Expand All @@ -480,8 +478,6 @@ export class JsTaskRunner extends TaskRunner {
...dataProxy,
...this.buildRpcCallObject(taskId),
...additionalProperties,
};

return context;
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,5 +117,16 @@ describe('TaskRunnerProcess', () => {
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(options.env).not.toHaveProperty('NODE_OPTIONS');
});

it('should use --disallow-code-generation-from-strings flag', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');

await taskRunnerProcess.start();

expect(spawnMock.mock.calls[0].at(1)).toEqual([
'--disallow-code-generation-from-strings',
expect.stringContaining('/packages/@n8n/task-runner/dist/start.js'),
]);
});
});
});
2 changes: 1 addition & 1 deletion packages/cli/src/task-runners/task-runner-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> {
startNode(grantToken: string, taskBrokerUri: string) {
const startScript = require.resolve('@n8n/task-runner/start');

return spawn('node', [startScript], {
return spawn('node', ['--disallow-code-generation-from-strings', startScript], {
env: this.getProcessEnvVars(grantToken, taskBrokerUri),
});
}
Expand Down

0 comments on commit 35b6180

Please sign in to comment.