diff --git a/docker/images/n8n/n8n-task-runners.json b/docker/images/n8n/n8n-task-runners.json index b30baed892..091e539593 100644 --- a/docker/images/n8n/n8n-task-runners.json +++ b/docker/images/n8n/n8n-task-runners.json @@ -23,7 +23,6 @@ "N8N_RUNNERS_HEALTH_CHECK_SERVER_PORT", "NODE_FUNCTION_ALLOW_BUILTIN", "NODE_FUNCTION_ALLOW_EXTERNAL", - "N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION", "NODE_OPTIONS", "NODE_PATH", "N8N_SENTRY_DSN", diff --git a/packages/@n8n/config/src/configs/runners.config.ts b/packages/@n8n/config/src/configs/runners.config.ts index ff0a1ba747..7c1390ee8d 100644 --- a/packages/@n8n/config/src/configs/runners.config.ts +++ b/packages/@n8n/config/src/configs/runners.config.ts @@ -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. */ @Env('N8N_RUNNERS_HEARTBEAT_INTERVAL') 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; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index 931e784979..731fdfb85d 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -255,6 +255,7 @@ describe('GlobalConfig', () => { maxConcurrency: 10, taskTimeout: 300, heartbeatInterval: 30, + insecureMode: false, }, sentry: { backendDsn: '', diff --git a/packages/@n8n/task-runner/src/config/js-runner-config.ts b/packages/@n8n/task-runner/src/config/js-runner-config.ts index 93b782fc49..338e70e2c7 100644 --- a/packages/@n8n/task-runner/src/config/js-runner-config.ts +++ b/packages/@n8n/task-runner/src/config/js-runner-config.ts @@ -8,13 +8,6 @@ export class JsRunnerConfig { @Env('NODE_FUNCTION_ALLOW_EXTERNAL') allowedExternalModules: string = ''; - /** - * Whether to allow prototype mutation for external libraries. Set to `true` - * 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; + @Env('N8N_RUNNERS_INSECURE_MODE') + insecureMode: boolean = false; } diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts index d2dac70778..422bef3e88 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts @@ -38,7 +38,7 @@ const defaultConfig = new MainConfig(); defaultConfig.jsRunnerConfig ??= { allowedBuiltInModules: '', allowedExternalModules: '', - allowPrototypeMutation: true, // needed for jest + insecureMode: false, }; describe('JsTaskRunner', () => { @@ -1455,9 +1455,9 @@ describe('JsTaskRunner', () => { 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({ - allowPrototypeMutation: true, + insecureMode: true, }); const outcome = await executeForAllItems({ diff --git a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts index b372f1cfb8..017a20eb40 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -30,15 +30,15 @@ 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'; -import { EXPOSED_RPC_METHODS, UNSUPPORTED_HELPER_FUNCTIONS } from '@/runner-types'; import type { DataRequestResponse, InputDataChunkDefinition, PartialAdditionalData, TaskResultData, } 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 type { TaskParams } from '@/task-runner'; import { BuiltInsParser } from './built-ins-parser/built-ins-parser'; 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 mode: 'secure' | 'insecure' = 'secure'; + constructor(config: MainConfig, name = 'JS Task Runner') { super({ taskType: 'javascript', @@ -117,19 +119,17 @@ export class JsTaskRunner extends TaskRunner { const allowedExternalModules = parseModuleAllowList( jsRunnerConfig.allowedExternalModules ?? '', ); + this.mode = jsRunnerConfig.insecureMode ? 'insecure' : 'secure'; this.requireResolver = createRequireResolver({ allowedBuiltInModules, allowedExternalModules, }); - this.preventPrototypePollution(allowedExternalModules, jsRunnerConfig.allowPrototypeMutation); + if (this.mode === 'secure') this.preventPrototypePollution(allowedExternalModules); } - private preventPrototypePollution( - allowedExternalModules: Set | '*', - allowPrototypeMutation: boolean, - ) { + private preventPrototypePollution(allowedExternalModules: Set | '*') { if (allowedExternalModules instanceof Set) { // This is a workaround to enable the allowed external libraries to mutate // prototypes directly. For example momentjs overrides .toString() directly @@ -141,11 +141,11 @@ export class JsTaskRunner extends TaskRunner { } } - // Freeze globals if needed - if (!allowPrototypeMutation) { + // Freeze globals, except in tests because Jest needs to be able to mutate prototypes + if (process.env.NODE_ENV !== 'test') { Object.getOwnPropertyNames(globalThis) // @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]) .filter((value) => typeof value === 'function') // 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 }); - const taskResult = runInContext(this.createVmExecutableCode(settings.code), context, { - timeout: this.taskTimeout * 1000, - }) as Promise; + let taskResult: Promise; + + if (this.mode === 'secure') { + taskResult = runInContext(this.createVmExecutableCode(settings.code), context, { + timeout: this.taskTimeout * 1000, + }) as Promise; + } else { + taskResult = this.runDirectly(settings.code, context); + } void taskResult .then(resolve) @@ -319,9 +325,15 @@ export class JsTaskRunner extends TaskRunner { signal.addEventListener('abort', abortHandler); - const taskResult = runInContext(this.createVmExecutableCode(settings.code), context, { - timeout: this.taskTimeout * 1000, - }) as Promise; + let taskResult: Promise; + + if (this.mode === 'secure') { + taskResult = runInContext(this.createVmExecutableCode(settings.code), context, { + timeout: this.taskTimeout * 1000, + }) as Promise; + } else { + taskResult = this.runDirectly(settings.code, context); + } void taskResult .then(resolve) @@ -551,4 +563,14 @@ export class JsTaskRunner extends TaskRunner { `module.exports = async function VmCodeWrapper() {${code}\n}()`, ].join('; '); } + + private async runDirectly(code: string, context: Context): Promise { + // 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); + } } diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md index f4a53f3e65..5af3513422 100644 --- a/packages/cli/BREAKING-CHANGES.md +++ b/packages/cli/BREAKING-CHANGES.md @@ -2,6 +2,20 @@ 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 ### What changed? diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index 6cac0c2aac..876374f985 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -138,7 +138,15 @@ export abstract class BaseCommand { 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'); await Container.get(TaskRunnerModule).start(); } diff --git a/packages/cli/src/task-runners/__tests__/task-runner-process.test.ts b/packages/cli/src/task-runners/__tests__/task-runner-process.test.ts index 5b23ba86a4..45c204caff 100644 --- a/packages/cli/src/task-runners/__tests__/task-runner-process.test.ts +++ b/packages/cli/src/task-runners/__tests__/task-runner-process.test.ts @@ -26,6 +26,7 @@ describe('TaskRunnerProcess', () => { const runnerConfig = mockInstance(TaskRunnersConfig); runnerConfig.enabled = true; runnerConfig.mode = 'internal'; + runnerConfig.insecureMode = false; const authService = mock(); let taskRunnerProcess = new TaskRunnerProcess(logger, runnerConfig, authService, mock()); @@ -78,7 +79,7 @@ describe('TaskRunnerProcess', () => { 'DEPLOYMENT_NAME', 'NODE_PATH', 'GENERIC_TIMEZONE', - 'N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION', + 'N8N_RUNNERS_INSECURE_MODE', ])('should propagate %s from env as is', async (envVar) => { jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken'); 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'); await taskRunnerProcess.start(); @@ -161,5 +162,22 @@ describe('TaskRunnerProcess', () => { 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'), + ]); + }); }); }); diff --git a/packages/cli/src/task-runners/task-runner-process.ts b/packages/cli/src/task-runners/task-runner-process.ts index 6854636b6c..86ebe3c923 100644 --- a/packages/cli/src/task-runners/task-runner-process.ts +++ b/packages/cli/src/task-runners/task-runner-process.ts @@ -59,7 +59,7 @@ export class TaskRunnerProcess extends TypedEmitter { 'NODE_FUNCTION_ALLOW_BUILTIN', 'NODE_FUNCTION_ALLOW_EXTERNAL', 'N8N_SENTRY_DSN', - 'N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION', + 'N8N_RUNNERS_INSECURE_MODE', // Metadata about the environment 'N8N_VERSION', 'ENVIRONMENT', @@ -67,6 +67,8 @@ export class TaskRunnerProcess extends TypedEmitter { 'NODE_PATH', ] as const; + private readonly mode: 'insecure' | 'secure' = 'secure'; + constructor( logger: Logger, private readonly runnerConfig: TaskRunnersConfig, @@ -80,6 +82,8 @@ export class TaskRunnerProcess extends TypedEmitter { 'Task Runner Process cannot be used in external mode', ); + this.mode = this.runnerConfig.insecureMode ? 'insecure' : 'secure'; + this.logger = logger.scoped('task-runner'); this.runnerLifecycleEvents.on('runner:failed-heartbeat-check', () => { @@ -109,13 +113,14 @@ export class TaskRunnerProcess extends TypedEmitter { startNode(grantToken: string, taskBrokerUri: string) { const startScript = require.resolve('@n8n/task-runner/start'); - return spawn( - 'node', - ['--disallow-code-generation-from-strings', '--disable-proto=delete', startScript], - { - env: this.getProcessEnvVars(grantToken, taskBrokerUri), - }, - ); + const flags = + this.mode === 'secure' + ? ['--disallow-code-generation-from-strings', '--disable-proto=delete'] + : []; + + return spawn('node', [...flags, startScript], { + env: this.getProcessEnvVars(grantToken, taskBrokerUri), + }); } @OnShutdown()