fix(core): Preserve NODE_PATH for globally installed npm packages in Docker (#26898)

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-10-12 02:19:43 +00:00 · 2026-03-11 16:41:07 +00:00
parent 4e1e0cd1c0
commit 3612e4b1ce
4 changed files with 176 additions and 1 deletions
@@ -14,6 +14,7 @@
 				"PATH",
 				"GENERIC_TIMEZONE",
 				"NODE_OPTIONS",
+				"NODE_PATH",
 				"N8N_RUNNERS_AUTO_SHUTDOWN_TIMEOUT",
 				"N8N_RUNNERS_TASK_TIMEOUT",
 				"N8N_RUNNERS_MAX_CONCURRENCY",
@@ -0,0 +1,121 @@
+import { execSync } from 'node:child_process';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+
+/**
+ * Regression tests for https://github.com/n8n-io/n8n/issues/24191
+ *
+ * External npm packages installed globally in Docker (via `npm install -g`)
+ * must be resolvable by the task runner's require(). This requires:
+ *
+ * 1. NODE_PATH must include the global npm modules directory
+ * 2. NODE_PATH must not be clobbered by load-nodes-and-credentials.ts
+ * 3. Module._initPaths() must be called to pick up NODE_PATH at runtime
+ *
+ * Tests use child processes because Jest intercepts require() with its own
+ * resolver which doesn't respect NODE_PATH changes.
+ *
+ * IMPORTANT: Scripts are placed in a separate directory from the installed
+ * package so that Node's standard module resolution (walking up parent dirs)
+ * cannot find the package — only NODE_PATH can.
+ */
+describe('external npm modules in task runner (issue #24191)', () => {
+	const tmpDir = path.join(os.tmpdir(), `n8n-test-global-modules-${Date.now()}`);
+	// Install package in an isolated location (simulates /opt/nodejs/.../lib/node_modules)
+	const installDir = path.join(tmpDir, 'global-install');
+	const nodeModulesPath = path.join(installDir, 'node_modules');
+	// Scripts live in a separate tree so require() can't find the package via parent traversal
+	const scriptsDir = path.join(tmpDir, 'scripts');
+	const packageName = 'cowsay';
+	const scriptPath = path.join(scriptsDir, 'test-require.js');
+	const scriptNoInitPath = path.join(scriptsDir, 'test-require-no-init.js');
+
+	beforeAll(() => {
+		fs.mkdirSync(installDir, { recursive: true });
+		fs.mkdirSync(scriptsDir, { recursive: true });
+		execSync(`npm install ${packageName} --prefix ${installDir}`, { stdio: 'pipe' });
+
+		fs.writeFileSync(
+			scriptPath,
+			`const Module = require("module");
+if (process.env.NODE_PATH) { Module._initPaths(); }
+try {
+  require("${packageName}");
+  console.log(JSON.stringify({ success: true }));
+} catch (e) {
+  console.log(JSON.stringify({ success: false, error: e.message }));
+}`,
+		);
+
+		fs.writeFileSync(
+			scriptNoInitPath,
+			`process.env.NODE_PATH = "${nodeModulesPath}";
+try {
+  require("${packageName}");
+  console.log(JSON.stringify({ success: true }));
+} catch (e) {
+  console.log(JSON.stringify({ success: false, error: e.message }));
+}`,
+		);
+	});
+
+	afterAll(() => {
+		fs.rmSync(tmpDir, { recursive: true, force: true });
+	});
+
+	function runRequireTest(env: Record<string, string | undefined>): {
+		success: boolean;
+		error?: string;
+	} {
+		const result = execSync(`node ${scriptPath}`, {
+			env: { ...process.env, ...env, NODE_PATH: env.NODE_PATH },
+			encoding: 'utf-8',
+			cwd: scriptsDir,
+		});
+
+		return JSON.parse(result.trim()) as { success: boolean; error?: string };
+	}
+
+	it('should fail without NODE_PATH pointing to the install location', () => {
+		const result = runRequireTest({ NODE_PATH: undefined });
+
+		expect(result.success).toBe(false);
+		expect(result.error).toContain('Cannot find module');
+	});
+
+	it('should succeed when NODE_PATH includes the install location', () => {
+		const result = runRequireTest({ NODE_PATH: nodeModulesPath });
+
+		expect(result.success).toBe(true);
+	});
+
+	it('should succeed when install location is appended to other paths', () => {
+		// Simulates the fix: load-nodes-and-credentials.ts appends the original
+		// NODE_PATH rather than clobbering it, so the task runner receives both
+		// the n8n internal paths AND the global modules path.
+		const combinedNodePath = `/some/internal/path:/another/path:${nodeModulesPath}`;
+		const result = runRequireTest({ NODE_PATH: combinedNodePath });
+
+		expect(result.success).toBe(true);
+	});
+
+	it('should fail when NODE_PATH is clobbered without the install location', () => {
+		// Simulates the bug: load-nodes-and-credentials.ts overwrites NODE_PATH
+		// with only n8n internal paths, losing the global modules path.
+		const result = runRequireTest({ NODE_PATH: '/some/internal/path:/another/path' });
+
+		expect(result.success).toBe(false);
+	});
+
+	it('should fail when NODE_PATH is set at runtime without _initPaths()', () => {
+		const result = execSync(`node ${scriptNoInitPath}`, {
+			env: { ...process.env, NODE_PATH: undefined },
+			encoding: 'utf-8',
+			cwd: scriptsDir,
+		});
+
+		const parsed = JSON.parse(result.trim()) as { success: boolean };
+		expect(parsed.success).toBe(false);
+	});
+});
@@ -30,6 +30,55 @@ jest.mock('@/tool-generation', () => ({
 	createHitlTools: jest.fn(),
 }));

+/**
+ * Regression test for https://github.com/n8n-io/n8n/issues/24191
+ *
+ * LoadNodesAndCredentials.init() sets process.env.NODE_PATH to module.paths
+ * for node/credential resolution. It must PRESERVE any existing NODE_PATH
+ * (e.g. set by Docker ENV for global npm packages) so the task runner
+ * subprocess can resolve externally installed modules.
+ *
+ * The init() method cannot be called directly in tests (guarded by inTest),
+ * so this test validates the NODE_PATH preservation logic directly.
+ */
+describe('NODE_PATH preservation (issue #24191)', () => {
+	const originalNodePath = process.env.NODE_PATH;
+
+	afterEach(() => {
+		if (originalNodePath === undefined) {
+			delete process.env.NODE_PATH;
+		} else {
+			process.env.NODE_PATH = originalNodePath;
+		}
+	});
+
+	it('should preserve existing NODE_PATH when setting module paths', () => {
+		const existingPath = '/opt/nodejs/node-v24.13.1/lib/node_modules';
+		process.env.NODE_PATH = existingPath;
+
+		// This is the exact logic from LoadNodesAndCredentials.init()
+		const delimiter = process.platform === 'win32' ? ';' : ':';
+		process.env.NODE_PATH = [module.paths.join(delimiter), process.env.NODE_PATH]
+			.filter(Boolean)
+			.join(delimiter);
+
+		expect(process.env.NODE_PATH).toContain(existingPath);
+		expect(process.env.NODE_PATH?.endsWith(existingPath)).toBe(true);
+	});
+
+	it('should work when no existing NODE_PATH is set', () => {
+		delete process.env.NODE_PATH;
+
+		const delimiter = process.platform === 'win32' ? ';' : ':';
+		process.env.NODE_PATH = [module.paths.join(delimiter), process.env.NODE_PATH]
+			.filter(Boolean)
+			.join(delimiter);
+
+		expect(process.env.NODE_PATH).toBe(module.paths.join(delimiter));
+		expect(process.env.NODE_PATH).not.toContain('undefined');
+	});
+});
+
 describe('LoadNodesAndCredentials', () => {
 	describe('resolveIcon', () => {
 		let instance: LoadNodesAndCredentials;
@@ -67,8 +67,12 @@ export class LoadNodesAndCredentials {
 		if (inTest) throw new UnexpectedError('Not available in tests');

 		// Make sure the imported modules can resolve dependencies fine.
+		// Preserve any existing NODE_PATH (e.g. set by Docker ENV for global npm packages)
+		// so that the task runner subprocess can also resolve externally installed modules.
 		const delimiter = process.platform === 'win32' ? ';' : ':';
-		process.env.NODE_PATH = module.paths.join(delimiter);
+		process.env.NODE_PATH = [module.paths.join(delimiter), process.env.NODE_PATH]
+			.filter(Boolean)
+			.join(delimiter);

 		// @ts-ignore
 		// eslint-disable-next-line @typescript-eslint/no-unsafe-call