fix(core): Fix usage of external libs in task runner (#12788)

Co-authored-by: Cornelius Suermann <cornelius@n8n.io>

packages/@n8n/task-runner/src/js-task-runner/__tests__/require-resolver.test.ts

@@ -0,0 +1,78 @@
+import { ApplicationError } from 'n8n-workflow';
+
+import { ExecutionError } from '@/js-task-runner/errors/execution-error';
+
+import { createRequireResolver, type RequireResolverOpts } from '../require-resolver';
+
+describe('require resolver', () => {
+	let defaultOpts: RequireResolverOpts;
+
+	beforeEach(() => {
+		defaultOpts = {
+			allowedBuiltInModules: new Set(['path', 'fs']),
+			allowedExternalModules: new Set(['lodash']),
+		};
+	});
+
+	describe('built-in modules', () => {
+		it('should allow requiring whitelisted built-in modules', () => {
+			const resolver = createRequireResolver(defaultOpts);
+			expect(() => resolver('path')).not.toThrow();
+			expect(() => resolver('fs')).not.toThrow();
+		});
+
+		it('should throw when requiring non-whitelisted built-in modules', () => {
+			const resolver = createRequireResolver(defaultOpts);
+			expect(() => resolver('crypto')).toThrow(ExecutionError);
+		});
+
+		it('should allow all built-in modules when allowedBuiltInModules is "*"', () => {
+			const resolver = createRequireResolver({
+				...defaultOpts,
+				allowedBuiltInModules: '*',
+			});
+
+			expect(() => resolver('path')).not.toThrow();
+			expect(() => resolver('crypto')).not.toThrow();
+			expect(() => resolver('fs')).not.toThrow();
+		});
+	});
+
+	describe('external modules', () => {
+		it('should allow requiring whitelisted external modules', () => {
+			const resolver = createRequireResolver(defaultOpts);
+			expect(() => resolver('lodash')).not.toThrow();
+		});
+
+		it('should throw when requiring non-whitelisted external modules', () => {
+			const resolver = createRequireResolver(defaultOpts);
+			expect(() => resolver('express')).toThrow(
+				new ExecutionError(new ApplicationError("Cannot find module 'express'")),
+			);
+		});
+
+		it('should allow all external modules when allowedExternalModules is "*"', () => {
+			const resolver = createRequireResolver({
+				...defaultOpts,
+				allowedExternalModules: '*',
+			});
+
+			expect(() => resolver('lodash')).not.toThrow();
+			expect(() => resolver('express')).not.toThrow();
+		});
+	});
+
+	describe('error handling', () => {
+		it('should wrap ApplicationError in ExecutionError', () => {
+			const resolver = createRequireResolver(defaultOpts);
+			expect(() => resolver('non-existent-module')).toThrow(ExecutionError);
+		});
+
+		it('should include the module name in the error message', () => {
+			const resolver = createRequireResolver(defaultOpts);
+			expect(() => resolver('non-existent-module')).toThrow(
+				"Cannot find module 'non-existent-module'",
+			);
+		});
+	});
+});

packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts

@@ -98,17 +98,33 @@ export class JsTaskRunner extends TaskRunner {
 		const { jsRunnerConfig } = config;
 
 		const parseModuleAllowList = (moduleList: string) =>
-			moduleList === '*' ? null : new Set(moduleList.split(',').map((x) => x.trim()));
+			moduleList === '*' ? '*' : new Set(moduleList.split(',').map((x) => x.trim()));
+
+		const allowedBuiltInModules = parseModuleAllowList(jsRunnerConfig.allowedBuiltInModules ?? '');
+		const allowedExternalModules = parseModuleAllowList(
+			jsRunnerConfig.allowedExternalModules ?? '',
+		);
 
 		this.requireResolver = createRequireResolver({
-			allowedBuiltInModules: parseModuleAllowList(jsRunnerConfig.allowedBuiltInModules ?? ''),
-			allowedExternalModules: parseModuleAllowList(jsRunnerConfig.allowedExternalModules ?? ''),
+			allowedBuiltInModules,
+			allowedExternalModules,
 		});
 
-		this.preventPrototypePollution();
+		this.preventPrototypePollution(allowedExternalModules);
+	}
+
+	private preventPrototypePollution(allowedExternalModules: Set<string> | '*') {
+		if (allowedExternalModules instanceof Set) {
+			// This is a workaround to enable the allowed external libraries to mutate
+			// prototypes directly. For example momentjs overrides .toString() directly
+			// on the Moment.prototype, which doesn't work if Object.prototype has been
+			// frozen. This works as long as the overrides are done when the library is
+			// imported.
+			for (const module of allowedExternalModules) {
+				require(module);
+			}
 		}
 
-	private preventPrototypePollution() {
 		// Freeze globals, except for Jest
 		if (process.env.NODE_ENV !== 'test') {
 			Object.getOwnPropertyNames(globalThis)

packages/@n8n/task-runner/src/js-task-runner/require-resolver.ts

@@ -6,15 +6,15 @@ 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.
+	 * execution sandbox. `"*"` means all are allowed.
 	 */
-	allowedBuiltInModules: Set<string> | null;
+	allowedBuiltInModules: Set<string> | '*';
 
 	/**
 	 * List of external modules that are allowed to be required in the
-	 * execution sandbox. `null` means all are allowed.
+	 * execution sandbox. `"*"` means all are allowed.
 	 */
-	allowedExternalModules: Set<string> | null;
+	allowedExternalModules: Set<string> | '*';
 };
 
 export type RequireResolver = (request: string) => unknown;
@@ -24,8 +24,8 @@ export function createRequireResolver({
 	allowedExternalModules,
 }: RequireResolverOpts) {
 	return (request: string) => {
-		const checkIsAllowed = (allowList: Set<string> | null, moduleName: string) => {
-			return allowList ? allowList.has(moduleName) : true;
+		const checkIsAllowed = (allowList: Set<string> | '*', moduleName: string) => {
+			return allowList === '*' || allowList.has(moduleName);
 		};
 
 		const isAllowed = isBuiltin(request)