feat(core)!: Introduce insecure mode in task runner (#16911)

This commit is contained in:
Iván Ovejero
2025-07-04 08:32:49 +02:00
committed by GitHub
parent 60e78a4fec
commit 7317f67797
10 changed files with 107 additions and 40 deletions

View File

@@ -23,7 +23,6 @@
"N8N_RUNNERS_HEALTH_CHECK_SERVER_PORT", "N8N_RUNNERS_HEALTH_CHECK_SERVER_PORT",
"NODE_FUNCTION_ALLOW_BUILTIN", "NODE_FUNCTION_ALLOW_BUILTIN",
"NODE_FUNCTION_ALLOW_EXTERNAL", "NODE_FUNCTION_ALLOW_EXTERNAL",
"N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION",
"NODE_OPTIONS", "NODE_OPTIONS",
"NODE_PATH", "NODE_PATH",
"N8N_SENTRY_DSN", "N8N_SENTRY_DSN",

View File

@@ -62,4 +62,11 @@ export class TaskRunnersConfig {
/** How often (in seconds) the runner must send a heartbeat to the broker, else the task will be aborted. (In internal mode, the runner will also be restarted.) Must be greater than 0. */ /** How often (in seconds) the runner must send a heartbeat to the broker, else the task will be aborted. (In internal mode, the runner will also be restarted.) Must be greater than 0. */
@Env('N8N_RUNNERS_HEARTBEAT_INTERVAL') @Env('N8N_RUNNERS_HEARTBEAT_INTERVAL')
heartbeatInterval: number = 30; heartbeatInterval: number = 30;
/**
* Whether to disable all security measures in the task runner. **Discouraged for production use.**
* Set to `true` for compatibility with modules that rely on insecure JS features.
*/
@Env('N8N_RUNNERS_INSECURE_MODE')
insecureMode: boolean = false;
} }

View File

@@ -255,6 +255,7 @@ describe('GlobalConfig', () => {
maxConcurrency: 10, maxConcurrency: 10,
taskTimeout: 300, taskTimeout: 300,
heartbeatInterval: 30, heartbeatInterval: 30,
insecureMode: false,
}, },
sentry: { sentry: {
backendDsn: '', backendDsn: '',

View File

@@ -8,13 +8,6 @@ export class JsRunnerConfig {
@Env('NODE_FUNCTION_ALLOW_EXTERNAL') @Env('NODE_FUNCTION_ALLOW_EXTERNAL')
allowedExternalModules: string = ''; allowedExternalModules: string = '';
/** @Env('N8N_RUNNERS_INSECURE_MODE')
* Whether to allow prototype mutation for external libraries. Set to `true` insecureMode: boolean = false;
* to allow modules that rely on runtime prototype mutation, e.g. `puppeteer`,
* at the cost of security.
*
* @default false
*/
@Env('N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION')
allowPrototypeMutation: boolean = false;
} }

View File

@@ -38,7 +38,7 @@ const defaultConfig = new MainConfig();
defaultConfig.jsRunnerConfig ??= { defaultConfig.jsRunnerConfig ??= {
allowedBuiltInModules: '', allowedBuiltInModules: '',
allowedExternalModules: '', allowedExternalModules: '',
allowPrototypeMutation: true, // needed for jest insecureMode: false,
}; };
describe('JsTaskRunner', () => { describe('JsTaskRunner', () => {
@@ -1455,9 +1455,9 @@ describe('JsTaskRunner', () => {
expect(Duration.fromObject({ hours: 1 }).maliciousKey).toBeUndefined(); expect(Duration.fromObject({ hours: 1 }).maliciousKey).toBeUndefined();
}); });
it('should allow prototype mutation when `allowPrototypeMutation` is true', async () => { it('should allow prototype mutation when `insecureMode` is true', async () => {
const runner = createRunnerWithOpts({ const runner = createRunnerWithOpts({
allowPrototypeMutation: true, insecureMode: true,
}); });
const outcome = await executeForAllItems({ const outcome = await executeForAllItems({

View File

@@ -30,15 +30,15 @@ import { type Context, createContext, runInContext } from 'node:vm';
import type { MainConfig } from '@/config/main-config'; import type { MainConfig } from '@/config/main-config';
import { UnsupportedFunctionError } from '@/js-task-runner/errors/unsupported-function.error'; import { UnsupportedFunctionError } from '@/js-task-runner/errors/unsupported-function.error';
import { EXPOSED_RPC_METHODS, UNSUPPORTED_HELPER_FUNCTIONS } from '@/runner-types';
import type { import type {
DataRequestResponse, DataRequestResponse,
InputDataChunkDefinition, InputDataChunkDefinition,
PartialAdditionalData, PartialAdditionalData,
TaskResultData, TaskResultData,
} from '@/runner-types'; } from '@/runner-types';
import type { TaskParams } from '@/task-runner'; import { EXPOSED_RPC_METHODS, UNSUPPORTED_HELPER_FUNCTIONS } from '@/runner-types';
import { noOp, TaskRunner } from '@/task-runner'; import { noOp, TaskRunner } from '@/task-runner';
import type { TaskParams } from '@/task-runner';
import { BuiltInsParser } from './built-ins-parser/built-ins-parser'; import { BuiltInsParser } from './built-ins-parser/built-ins-parser';
import { BuiltInsParserState } from './built-ins-parser/built-ins-parser-state'; import { BuiltInsParserState } from './built-ins-parser/built-ins-parser-state';
@@ -95,6 +95,8 @@ export class JsTaskRunner extends TaskRunner {
private readonly taskDataReconstruct = new DataRequestResponseReconstruct(); private readonly taskDataReconstruct = new DataRequestResponseReconstruct();
private readonly mode: 'secure' | 'insecure' = 'secure';
constructor(config: MainConfig, name = 'JS Task Runner') { constructor(config: MainConfig, name = 'JS Task Runner') {
super({ super({
taskType: 'javascript', taskType: 'javascript',
@@ -117,19 +119,17 @@ export class JsTaskRunner extends TaskRunner {
const allowedExternalModules = parseModuleAllowList( const allowedExternalModules = parseModuleAllowList(
jsRunnerConfig.allowedExternalModules ?? '', jsRunnerConfig.allowedExternalModules ?? '',
); );
this.mode = jsRunnerConfig.insecureMode ? 'insecure' : 'secure';
this.requireResolver = createRequireResolver({ this.requireResolver = createRequireResolver({
allowedBuiltInModules, allowedBuiltInModules,
allowedExternalModules, allowedExternalModules,
}); });
this.preventPrototypePollution(allowedExternalModules, jsRunnerConfig.allowPrototypeMutation); if (this.mode === 'secure') this.preventPrototypePollution(allowedExternalModules);
} }
private preventPrototypePollution( private preventPrototypePollution(allowedExternalModules: Set<string> | '*') {
allowedExternalModules: Set<string> | '*',
allowPrototypeMutation: boolean,
) {
if (allowedExternalModules instanceof Set) { if (allowedExternalModules instanceof Set) {
// This is a workaround to enable the allowed external libraries to mutate // This is a workaround to enable the allowed external libraries to mutate
// prototypes directly. For example momentjs overrides .toString() directly // prototypes directly. For example momentjs overrides .toString() directly
@@ -141,11 +141,11 @@ export class JsTaskRunner extends TaskRunner {
} }
} }
// Freeze globals if needed // Freeze globals, except in tests because Jest needs to be able to mutate prototypes
if (!allowPrototypeMutation) { if (process.env.NODE_ENV !== 'test') {
Object.getOwnPropertyNames(globalThis) Object.getOwnPropertyNames(globalThis)
// @ts-expect-error globalThis does not have string in index signature // @ts-expect-error globalThis does not have string in index signature
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access // eslint-disable-next-line @typescript-eslint/no-unsafe-return
.map((name) => globalThis[name]) .map((name) => globalThis[name])
.filter((value) => typeof value === 'function') .filter((value) => typeof value === 'function')
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access
@@ -256,9 +256,15 @@ export class JsTaskRunner extends TaskRunner {
signal.addEventListener('abort', abortHandler, { once: true }); signal.addEventListener('abort', abortHandler, { once: true });
const taskResult = runInContext(this.createVmExecutableCode(settings.code), context, { let taskResult: Promise<TaskResultData['result']>;
timeout: this.taskTimeout * 1000,
}) as Promise<TaskResultData['result']>; if (this.mode === 'secure') {
taskResult = runInContext(this.createVmExecutableCode(settings.code), context, {
timeout: this.taskTimeout * 1000,
}) as Promise<TaskResultData['result']>;
} else {
taskResult = this.runDirectly<TaskResultData['result']>(settings.code, context);
}
void taskResult void taskResult
.then(resolve) .then(resolve)
@@ -319,9 +325,15 @@ export class JsTaskRunner extends TaskRunner {
signal.addEventListener('abort', abortHandler); signal.addEventListener('abort', abortHandler);
const taskResult = runInContext(this.createVmExecutableCode(settings.code), context, { let taskResult: Promise<INodeExecutionData>;
timeout: this.taskTimeout * 1000,
}) as Promise<INodeExecutionData>; if (this.mode === 'secure') {
taskResult = runInContext(this.createVmExecutableCode(settings.code), context, {
timeout: this.taskTimeout * 1000,
}) as Promise<INodeExecutionData>;
} else {
taskResult = this.runDirectly<INodeExecutionData>(settings.code, context);
}
void taskResult void taskResult
.then(resolve) .then(resolve)
@@ -551,4 +563,14 @@ export class JsTaskRunner extends TaskRunner {
`module.exports = async function VmCodeWrapper() {${code}\n}()`, `module.exports = async function VmCodeWrapper() {${code}\n}()`,
].join('; '); ].join('; ');
} }
private async runDirectly<T>(code: string, context: Context): Promise<T> {
// eslint-disable-next-line @typescript-eslint/no-implied-eval
const fn = new Function(
'context',
`with(context) { return (async function() {${code}\n})(); }`,
);
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-return
return await fn(context);
}
} }

View File

@@ -2,6 +2,20 @@
This list shows all the versions which include breaking changes and how to upgrade. This list shows all the versions which include breaking changes and how to upgrade.
## 1.102.0
### What changed?
The `N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION` flag has been replaced with `N8N_RUNNERS_INSECURE_MODE`. The new flag
disables all task runner security measures and is intended as an escape hatch for users who value compatibility
with libraries like `puppeteer` at the cost of security.
### When is action necessary?
If you are using the `N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION` flag, or if you find that the task runner does not
currently support an external module that you rely on, then consider setting `N8N_RUNNERS_INSECURE_MODE=true`,
at your own risk.
## 1.98.0 ## 1.98.0
### What changed? ### What changed?

View File

@@ -138,7 +138,15 @@ export abstract class BaseCommand<F = never> {
await Container.get(CommunityPackagesService).init(); await Container.get(CommunityPackagesService).init();
} }
if (this.needsTaskRunner && this.globalConfig.taskRunners.enabled) { const taskRunnersConfig = this.globalConfig.taskRunners;
if (this.needsTaskRunner && taskRunnersConfig.enabled) {
if (taskRunnersConfig.insecureMode) {
this.logger.warn(
'TASK RUNNER CONFIGURED TO START IN INSECURE MODE. This is discouraged for production use. Please consider using secure mode instead.',
);
}
const { TaskRunnerModule } = await import('@/task-runners/task-runner-module'); const { TaskRunnerModule } = await import('@/task-runners/task-runner-module');
await Container.get(TaskRunnerModule).start(); await Container.get(TaskRunnerModule).start();
} }

View File

@@ -26,6 +26,7 @@ describe('TaskRunnerProcess', () => {
const runnerConfig = mockInstance(TaskRunnersConfig); const runnerConfig = mockInstance(TaskRunnersConfig);
runnerConfig.enabled = true; runnerConfig.enabled = true;
runnerConfig.mode = 'internal'; runnerConfig.mode = 'internal';
runnerConfig.insecureMode = false;
const authService = mock<TaskBrokerAuthService>(); const authService = mock<TaskBrokerAuthService>();
let taskRunnerProcess = new TaskRunnerProcess(logger, runnerConfig, authService, mock()); let taskRunnerProcess = new TaskRunnerProcess(logger, runnerConfig, authService, mock());
@@ -78,7 +79,7 @@ describe('TaskRunnerProcess', () => {
'DEPLOYMENT_NAME', 'DEPLOYMENT_NAME',
'NODE_PATH', 'NODE_PATH',
'GENERIC_TIMEZONE', 'GENERIC_TIMEZONE',
'N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION', 'N8N_RUNNERS_INSECURE_MODE',
])('should propagate %s from env as is', async (envVar) => { ])('should propagate %s from env as is', async (envVar) => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken'); jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
process.env[envVar] = 'custom value'; process.env[envVar] = 'custom value';
@@ -150,7 +151,7 @@ describe('TaskRunnerProcess', () => {
); );
}); });
it('should use --disallow-code-generation-from-strings and --disable-proto=delete flags', async () => { it('on secure mode, should use --disallow-code-generation-from-strings and --disable-proto=delete flags', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken'); jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
await taskRunnerProcess.start(); await taskRunnerProcess.start();
@@ -161,5 +162,22 @@ describe('TaskRunnerProcess', () => {
expect.stringContaining('/packages/@n8n/task-runner/dist/start.js'), expect.stringContaining('/packages/@n8n/task-runner/dist/start.js'),
]); ]);
}); });
it('on insecure mode, should not use --disallow-code-generation-from-strings and --disable-proto=delete flags', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
runnerConfig.insecureMode = true;
const insecureTaskRunnerProcess = new TaskRunnerProcess(
logger,
runnerConfig,
authService,
mock(),
);
await insecureTaskRunnerProcess.start();
expect(spawnMock.mock.calls[0].at(1)).toEqual([
expect.stringContaining('/packages/@n8n/task-runner/dist/start.js'),
]);
});
}); });
}); });

View File

@@ -59,7 +59,7 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> {
'NODE_FUNCTION_ALLOW_BUILTIN', 'NODE_FUNCTION_ALLOW_BUILTIN',
'NODE_FUNCTION_ALLOW_EXTERNAL', 'NODE_FUNCTION_ALLOW_EXTERNAL',
'N8N_SENTRY_DSN', 'N8N_SENTRY_DSN',
'N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION', 'N8N_RUNNERS_INSECURE_MODE',
// Metadata about the environment // Metadata about the environment
'N8N_VERSION', 'N8N_VERSION',
'ENVIRONMENT', 'ENVIRONMENT',
@@ -67,6 +67,8 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> {
'NODE_PATH', 'NODE_PATH',
] as const; ] as const;
private readonly mode: 'insecure' | 'secure' = 'secure';
constructor( constructor(
logger: Logger, logger: Logger,
private readonly runnerConfig: TaskRunnersConfig, private readonly runnerConfig: TaskRunnersConfig,
@@ -80,6 +82,8 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> {
'Task Runner Process cannot be used in external mode', 'Task Runner Process cannot be used in external mode',
); );
this.mode = this.runnerConfig.insecureMode ? 'insecure' : 'secure';
this.logger = logger.scoped('task-runner'); this.logger = logger.scoped('task-runner');
this.runnerLifecycleEvents.on('runner:failed-heartbeat-check', () => { this.runnerLifecycleEvents.on('runner:failed-heartbeat-check', () => {
@@ -109,13 +113,14 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> {
startNode(grantToken: string, taskBrokerUri: string) { startNode(grantToken: string, taskBrokerUri: string) {
const startScript = require.resolve('@n8n/task-runner/start'); const startScript = require.resolve('@n8n/task-runner/start');
return spawn( const flags =
'node', this.mode === 'secure'
['--disallow-code-generation-from-strings', '--disable-proto=delete', startScript], ? ['--disallow-code-generation-from-strings', '--disable-proto=delete']
{ : [];
env: this.getProcessEnvVars(grantToken, taskBrokerUri),
}, return spawn('node', [...flags, startScript], {
); env: this.getProcessEnvVars(grantToken, taskBrokerUri),
});
} }
@OnShutdown() @OnShutdown()