fix(core): Only init the expression engine for commands that evaluate expressions (backport to release-candidate/2.35.x) (#36648)

Co-authored-by: Mike Repeć <mike.repec@n8n.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Danny Martini <danny@n8n.io>

packages/cli/src/commands/__tests__/execute-batch.test.ts

@@ -110,3 +110,7 @@ test('should start a task runner', async () => {
 
 	expect(taskRunnerModule.start).toHaveBeenCalledTimes(1);
 });
+
+test('execute:batch needs the expression engine', () => {
+	expect(new ExecuteBatch().needsExpressionEngine).toBe(true);
+});

packages/cli/src/commands/__tests__/execute.test.ts

@@ -1,4 +1,4 @@
-import { LicenseState } from '@n8n/backend-common';
+import { LicenseState, Logger } from '@n8n/backend-common';
 import { mockInstance } from '@n8n/backend-test-utils';
 import { GlobalConfig } from '@n8n/config';
 import type { User, WorkflowEntity, Project } from '@n8n/db';
@@ -10,7 +10,9 @@ import {
 	DeploymentKeyRepository,
 } from '@n8n/db';
 import { Container } from '@n8n/di';
+import { ErrorReporter } from 'n8n-core';
 import type { IRun } from 'n8n-workflow';
+import { Expression } from 'n8n-workflow';
 import { mock } from 'vitest-mock-extended';
 
 import { ActiveExecutions } from '@/active-executions';
@@ -41,7 +43,7 @@ const loadNodesAndCredentials = mockInstance(LoadNodesAndCredentials);
 const shutdownService = mockInstance(ShutdownService);
 const deprecationService = mockInstance(DeprecationService);
 mockInstance(MessageEventBus);
-mockInstance(ExpressionObservabilityProvider);
+const expressionObservability = mockInstance(ExpressionObservabilityProvider);
 const posthogClient = mockInstance(PostHogClient);
 const telemetryEventRelay = mockInstance(TelemetryEventRelay);
 const externalHooks = mockInstance(ExternalHooks);
@@ -50,6 +52,8 @@ mockInstance(LicenseState);
 mockInstance(CommunityPackagesService);
 mockInstance(WorkflowFailureNotificationEventRelay);
 
+const logger = mockInstance(Logger);
+const errorReporter = mockInstance(ErrorReporter);
 const dbConnection = mockInstance(DbConnection);
 dbConnection.init.mockResolvedValue(undefined);
 dbConnection.migrate.mockResolvedValue(undefined);
@@ -60,6 +64,26 @@ const deploymentKeyRepository = mockInstance(DeploymentKeyRepository);
 deploymentKeyRepository.findActiveByType.mockResolvedValue(null);
 deploymentKeyRepository.insertOrIgnore.mockResolvedValue(undefined);
 
+// minimal command for exercising BaseCommand.init():
+// Execute.init() chains singletons that cannot init twice per process
+class ReadOnlyCommand extends BaseCommand {}
+
+// default config for every test, so none depends on what an earlier test leaked
+// into the container; tests that need different values set their own
+beforeEach(() => {
+	Container.set(
+		GlobalConfig,
+		mock<GlobalConfig>({
+			taskRunners: {},
+			nodes: {},
+			expressionEngine: { engine: 'legacy' },
+			// must be numeric: the SIGTERM/SIGINT handlers registered by init() compute
+			// a setTimeout delay from it, and a mock proxy yields NaN at pool teardown
+			generic: { gracefulShutdownTimeout: 30 },
+		}),
+	);
+});
+
 test('should start a task runner', async () => {
 	// arrange
 
@@ -85,14 +109,6 @@ test('should start a task runner', async () => {
 	workflowRunner.run.mockResolvedValue('123');
 	activeExecutions.getPostExecutePromise.mockResolvedValue(run);
 
-	Container.set(
-		GlobalConfig,
-		mock<GlobalConfig>({
-			taskRunners: {},
-			nodes: {},
-		}),
-	);
-
 	const cmd = new Execute();
 	// @ts-expect-error Protected property
 	cmd.flags = { id: '123' };
@@ -113,8 +129,6 @@ test('should not seed the instance identity and should tolerate deployment key r
 	deploymentKeyRepository.insertOrIgnore.mockClear();
 	deploymentKeyRepository.findActiveByType.mockRejectedValueOnce(new Error('permission denied'));
 
-	// minimal command: Execute.init() chains singletons that cannot init twice per process
-	class ReadOnlyCommand extends BaseCommand {}
 	const cmd = new ReadOnlyCommand();
 
 	// act
@@ -125,3 +139,118 @@ test('should not seed the instance identity and should tolerate deployment key r
 
 	expect(deploymentKeyRepository.insertOrIgnore).not.toHaveBeenCalled();
 });
+
+test('should not init the expression engine for commands that do not need it', async () => {
+	// arrange
+
+	const initSpy = vi.spyOn(Expression, 'initExpressionEngine').mockResolvedValue(undefined);
+	const setEngineSpy = vi.spyOn(Expression, 'setExpressionEngine').mockReturnValue(undefined);
+	Container.set(
+		GlobalConfig,
+		mock<GlobalConfig>({
+			taskRunners: {},
+			nodes: {},
+			expressionEngine: { engine: 'vm' },
+			generic: { gracefulShutdownTimeout: 30 },
+		}),
+	);
+
+	const cmd = new ReadOnlyCommand();
+
+	// act
+
+	await cmd.init();
+
+	// assert
+
+	expect(initSpy).not.toHaveBeenCalled();
+	// the configured engine is still recorded, so evaluating an expression without
+	// an initialized engine throws instead of silently using the legacy engine
+	expect(setEngineSpy).toHaveBeenCalledWith('vm');
+});
+
+test('should exit with a crash when expression engine init fails', async () => {
+	// arrange
+
+	const initSpy = vi
+		.spyOn(Expression, 'initExpressionEngine')
+		.mockRejectedValue(new Error('isolated-vm failed to load'));
+	const exitSpy = vi
+		// @ts-expect-error Protected method
+		.spyOn(BaseCommand.prototype, 'exitWithCrash')
+		.mockResolvedValue(undefined);
+
+	Container.set(
+		GlobalConfig,
+		mock<GlobalConfig>({
+			taskRunners: {},
+			nodes: {},
+			expressionEngine: {
+				engine: 'vm',
+				poolSize: 4,
+				maxCodeCacheSize: 1024,
+				bridgeTimeout: 5000,
+				bridgeMemoryLimit: 128,
+				idleTimeout: 30,
+			},
+			generic: { gracefulShutdownTimeout: 30 },
+		}),
+	);
+
+	class ExpressionCommand extends BaseCommand {
+		override needsExpressionEngine = true;
+	}
+	const cmd = new ExpressionCommand();
+
+	// act
+
+	await cmd.init();
+
+	// assert
+
+	expect(initSpy).toHaveBeenCalledTimes(1);
+	expect(initSpy).toHaveBeenCalledWith({
+		engine: 'vm',
+		poolSize: 4,
+		maxCodeCacheSize: 1024,
+		bridgeTimeout: 5000,
+		bridgeMemoryLimit: 128,
+		idleTimeoutMs: 30_000, // the config value is in seconds
+		observability: expressionObservability,
+	});
+	expect(exitSpy).toHaveBeenCalledWith(expect.stringContaining('isolated-vm'), expect.any(Error));
+});
+
+test('exitWithCrash logs the crash message to the console', async () => {
+	// arrange
+
+	const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
+	vi.useFakeTimers();
+
+	const cmd = new ReadOnlyCommand();
+	// @ts-expect-error Protected property, set directly to avoid another init() traversal
+	cmd.errorReporter = errorReporter;
+
+	const cause = new Error('isolated-vm failed to load');
+
+	// act
+
+	try {
+		// @ts-expect-error Protected method
+		const promise: Promise<void> = cmd.exitWithCrash('something broke', cause);
+		await vi.advanceTimersByTimeAsync(2000);
+		await promise;
+	} finally {
+		vi.useRealTimers();
+	}
+
+	// assert
+
+	// the error reporter may only reach Sentry, so the message must also hit the console
+	expect(logger.error).toHaveBeenCalledWith('something broke', { error: cause });
+	expect(exitSpy).toHaveBeenCalledWith(1);
+});
+
+test('execute needs the expression engine', () => {
+	expect(new Execute().needsExpressionEngine).toBe(true);
+});

packages/cli/src/commands/__tests__/start.test.ts

@@ -363,3 +363,7 @@ describe('Start - AuthRolesService initialization', () => {
 		});
 	});
 });
+
+test('start needs the expression engine', () => {
+	expect(new Start().needsExpressionEngine).toBe(true);
+});

packages/cli/src/commands/__tests__/webhook.test.ts

@@ -142,3 +142,7 @@ describe('Webhook', () => {
 		});
 	});
 });
+
+test('webhook needs the expression engine', () => {
+	expect(new Webhook().needsExpressionEngine).toBe(true);
+});

packages/cli/src/commands/__tests__/worker.test.ts

@@ -178,3 +178,7 @@ describe('Worker', () => {
 		});
 	});
 });
+
+test('worker needs the expression engine', () => {
+	expect(new Worker().needsExpressionEngine).toBe(true);
+});

packages/cli/src/commands/base-command.ts

@@ -88,6 +88,9 @@ export abstract class BaseCommand<F = never> {
 	/** Whether to init task runner. */
 	protected needsTaskRunner = false;
 
+	/** Whether to init the expression engine. Only commands that evaluate workflow expressions need it. */
+	protected needsExpressionEngine = false;
+
 	/**
 	 * Whether to seed missing `instance.id` / `signing.hmac` deployment-key rows.
 	 * Only server processes hold the encryption key these are derived from.
@@ -226,17 +229,31 @@ export abstract class BaseCommand<F = never> {
 		await Container.get(TelemetryEventRelay).init();
 		Container.get(WorkflowFailureNotificationEventRelay).init();
 
-		const { engine, poolSize, maxCodeCacheSize, bridgeTimeout, bridgeMemoryLimit, idleTimeout } =
-			this.globalConfig.expressionEngine;
-		await Expression.initExpressionEngine({
-			engine,
-			poolSize,
-			maxCodeCacheSize,
-			bridgeTimeout,
-			bridgeMemoryLimit,
-			idleTimeoutMs: idleTimeout === undefined ? undefined : idleTimeout * 1000,
-			observability: Container.get(ExpressionObservabilityProvider),
-		});
+		if (this.needsExpressionEngine) {
+			const { engine, poolSize, maxCodeCacheSize, bridgeTimeout, bridgeMemoryLimit, idleTimeout } =
+				this.globalConfig.expressionEngine;
+			const observability = Container.get(ExpressionObservabilityProvider);
+			try {
+				await Expression.initExpressionEngine({
+					engine,
+					poolSize,
+					maxCodeCacheSize,
+					bridgeTimeout,
+					bridgeMemoryLimit,
+					idleTimeoutMs: idleTimeout === undefined ? undefined : idleTimeout * 1000,
+					observability,
+				});
+			} catch (error) {
+				await this.exitWithCrash(
+					'Could not initialize the vm expression engine (see errors above for details). If they point at isolated-vm, check that it installed correctly, e.g. that native build scripts were not skipped.',
+					error,
+				);
+			}
+		} else {
+			// Record the configured engine so an unexpected expression evaluation on a
+			// vm-configured instance fails loudly instead of silently using the legacy engine
+			Expression.setExpressionEngine(this.globalConfig.expressionEngine.engine);
+		}
 	}
 
 	protected async stopProcess() {
@@ -270,7 +287,9 @@ export abstract class BaseCommand<F = never> {
 		}
 	}
 
-	protected async exitWithCrash(message: string, error: unknown) {
+	protected async exitWithCrash(message: string, error: unknown): Promise<never> {
+		// the error reporter only sends to Sentry when a DSN is configured, so also log to the console
+		this.logger.error(message, { error });
 		this.errorReporter.error(new Error(message, { cause: error }), { level: 'fatal' });
 		await sleep(2000);
 		process.exit(1);

packages/cli/src/commands/execute-batch.ts

@@ -133,6 +133,8 @@ export class ExecuteBatch extends BaseCommand<z.infer<typeof flagsSchema>> {
 
 	override needsCommunityPackages = true;
 
+	override needsExpressionEngine = true;
+
 	override needsTaskRunner = true;
 
 	/**

packages/cli/src/commands/execute.ts

@@ -30,6 +30,8 @@ const flagsSchema = z.object({
 export class Execute extends BaseCommand<z.infer<typeof flagsSchema>> {
 	override needsCommunityPackages = true;
 
+	override needsExpressionEngine = true;
+
 	override needsTaskRunner = true;
 
 	async init() {

packages/cli/src/commands/import/__tests__/workflow.test.ts

@@ -74,4 +74,8 @@ describe('ImportWorkflowsCommand', () => {
 			await expect(command.run()).resolves.toBeUndefined();
 		});
 	});
+
+	test('needs the expression engine', () => {
+		expect(new ImportWorkflowsCommand().needsExpressionEngine).toBe(true);
+	});
 });

packages/cli/src/commands/import/workflow.ts

@@ -101,6 +101,9 @@ const flagsSchema = z.object({
 	flagsSchema,
 })
 export class ImportWorkflowsCommand extends BaseCommand<z.infer<typeof flagsSchema>> {
+	// (De)activating imported workflows evaluates webhook parameters, which may be expressions
+	override needsExpressionEngine = true;
+
 	async run(): Promise<void> {
 		const { flags } = this;
 

packages/cli/src/commands/ldap/reset.ts

@@ -59,6 +59,9 @@ const flagsSchema = z.object({
 	flagsSchema,
 })
 export class Reset extends BaseCommand<z.infer<typeof flagsSchema>> {
+	// Deleting active workflows deregisters their webhooks, whose parameters may be expressions
+	override needsExpressionEngine = true;
+
 	async run(): Promise<void> {
 		const { flags } = this;
 		const numberOfOptions =

packages/cli/src/commands/start.ts

@@ -68,6 +68,8 @@ export class Start extends BaseCommand<z.infer<typeof flagsSchema>> {
 
 	override needsCommunityPackages = true;
 
+	override needsExpressionEngine = true;
+
 	override needsTaskRunner = true;
 
 	override seedsInstanceIdentity = true;

packages/cli/src/commands/webhook.ts

@@ -25,6 +25,8 @@ export class Webhook extends BaseCommand {
 
 	override needsCommunityPackages = true;
 
+	override needsExpressionEngine = true;
+
 	override seedsInstanceIdentity = true;
 
 	/**

packages/cli/src/commands/worker.ts

@@ -47,6 +47,8 @@ export class Worker extends BaseCommand<z.infer<typeof flagsSchema>> {
 
 	override needsCommunityPackages = true;
 
+	override needsExpressionEngine = true;
+
 	override needsTaskRunner = true;
 
 	override seedsInstanceIdentity = true;

packages/cli/test/integration/commands/ldap/reset.test.ts

@@ -363,3 +363,7 @@ describe('--projectId', () => {
 		).resolves.not.toBeNull();
 	});
 });
+
+test('ldap:reset needs the expression engine', () => {
+	expect(new Reset().needsExpressionEngine).toBe(true);
+});

packages/workflow/src/expression.ts

@@ -320,8 +320,9 @@ export class Expression {
 	 *
 	 * WARNING: This is a global setting — switching engines mid-execution could
 	 * cause a workflow to evaluate some expressions with one engine and some with
-	 * another. Only use this in benchmarks and tests, never in production code.
-	 * In production, set `N8N_EXPRESSION_ENGINE` before process startup instead.
+	 * another. Only call this during process startup (or in benchmarks and tests),
+	 * never mid-execution. In production, set `N8N_EXPRESSION_ENGINE` before
+	 * process startup instead.
 	 */
 	static setExpressionEngine(engine: 'legacy' | 'vm'): void {
 		this.expressionEngine = engine;