From ba0f19abb31d75be62e2cf0bdf277a9b667fbf70 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Wed, 26 Aug 2026 10:34:52 +0200 Subject: [PATCH] fix(core): Address review feedback on Python Code node lint - Thread the node execution mode into `lintPythonCode` and flag reading the accessor the runner only defines in the other mode, mirroring the JS rule's `CODE_MODE_API_MISUSE`. - Detect relative imports; the runner rejects them with "Relative imports are disallowed" and the previous root extraction produced no module for them. - Make the import message module-neutral. It now names both allowlists rather than reporting the standard-library error for an external package. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/lint/code-node/code-node.test.ts | 41 ++++++++++++++- .../workflow-sdk/src/lint/code-node/python.ts | 52 +++++++++++++++++-- .../src/lint/lint-workflow-source.test.ts | 26 ++++++++++ .../src/lint/lint-workflow-source.ts | 2 +- .../descriptions/PythonCodeDescription.ts | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-) diff --git a/packages/@n8n/workflow-sdk/src/lint/code-node/code-node.test.ts b/packages/@n8n/workflow-sdk/src/lint/code-node/code-node.test.ts index 58e4ddf1ede..06c5a9c715a 100644 --- a/packages/@n8n/workflow-sdk/src/lint/code-node/code-node.test.ts +++ b/packages/@n8n/workflow-sdk/src/lint/code-node/code-node.test.ts @@ -136,11 +136,50 @@ describe('lintPythonCode — native runner constraints', () => { ]); }); - it('does not flag _items or _item', () => { + it('does not flag _items or _item when the mode is unknown', () => { expect(lintPythonCode('return _items').map((i) => i.code)).toEqual([]); expect(lintPythonCode('return _item').map((i) => i.code)).toEqual([]); }); + it('flags _item in runOnceForAllItems mode', () => { + const issues = lintPythonCode('return [_item]', { mode: 'runOnceForAllItems' }); + expect(issues.map((i) => i.code)).toEqual(['CODE_MODE_API_MISUSE']); + expect(issues[0].message).toContain('_items'); + }); + + it('flags _items in runOnceForEachItem mode', () => { + const issues = lintPythonCode('return _items[0]', { mode: 'runOnceForEachItem' }); + expect(issues.map((i) => i.code)).toEqual(['CODE_MODE_API_MISUSE']); + expect(issues[0].message).toContain('_item'); + }); + + it('does not flag the accessor that matches the mode', () => { + expect( + lintPythonCode('return _items', { mode: 'runOnceForAllItems' }).map((i) => i.code), + ).toEqual([]); + expect( + lintPythonCode('return _item', { mode: 'runOnceForEachItem' }).map((i) => i.code), + ).toEqual([]); + }); + + it('flags a relative import, which the runner rejects outright', () => { + const issues = lintPythonCode('from .helpers import parse\nreturn []'); + expect(issues.map((i) => i.code)).toEqual(['CODE_NODE_PYTHON_IMPORT']); + expect(issues[0].message).toContain('.helpers'); + }); + + it('flags a bare relative import', () => { + expect(lintPythonCode('from . import helpers\nreturn []').map((i) => i.code)).toEqual([ + 'CODE_NODE_PYTHON_IMPORT', + ]); + }); + + it('names both allowlists rather than assuming the module is stdlib', () => { + const issues = lintPythonCode('import pandas\nreturn []'); + expect(issues[0].message).toContain('N8N_RUNNERS_STDLIB_ALLOW'); + expect(issues[0].message).toContain('N8N_RUNNERS_EXTERNAL_ALLOW'); + }); + it('does not flag an unsupported global named only in a comment', () => { expect( lintPythonCode('# _json and $input are not available\nreturn _items').map((i) => i.code), diff --git a/packages/@n8n/workflow-sdk/src/lint/code-node/python.ts b/packages/@n8n/workflow-sdk/src/lint/code-node/python.ts index d4e522dd102..deaa4837719 100644 --- a/packages/@n8n/workflow-sdk/src/lint/code-node/python.ts +++ b/packages/@n8n/workflow-sdk/src/lint/code-node/python.ts @@ -1,3 +1,4 @@ +import type { CodeExecutionMode } from './extract-snippets'; import { lintIssue, type SourceLintIssue } from '../types'; /** Modules that reach the network — reported as CODE_NODE_NETWORK_CALL instead. */ @@ -29,6 +30,12 @@ const UNSUPPORTED_GLOBAL = new RegExp( String.raw`(? = { + runOnceForAllItems: { defined: '_items', undefined: '_item' }, + runOnceForEachItem: { defined: '_item', undefined: '_items' }, +}; + /** `import a.b, c as d` and `from a.b import c`, anchored so prose never matches. */ const IMPORT_LINE = /^[ \t]*(?:import[ \t]+([\w.,\t ]+)|from[ \t]+([\w.]+)[ \t]+import\b)/gm; @@ -37,19 +44,37 @@ function stripComments(pythonCode: string): string { return pythonCode.replace(/#.*$/gm, ''); } -/** Top-level module of every import in the snippet — `http.client` yields `http`. */ +/** + * Top-level module of an import specifier — `http.client` yields `http`. Relative + * specifiers keep their leading dots: they resolve to no top-level module, and the + * runner rejects them outright with "Relative imports are disallowed". + */ +function importRoot(specifier: string): string { + const trimmed = specifier.trim(); + return trimmed.startsWith('.') ? trimmed : trimmed.split(/[\s.]/)[0]; +} + +/** Every module the snippet imports, keyed by top-level name. */ function importedRootModules(pythonCode: string): Set { const roots = new Set(); for (const [, names, fromModule] of pythonCode.matchAll(IMPORT_LINE)) { for (const specifier of names ? names.split(',') : [fromModule]) { - const root = specifier.trim().split(/[\s.]/)[0]; + const root = importRoot(specifier); if (root) roots.add(root); } } return roots; } +/** The accessor belonging to the other mode, when the snippet reads it. */ +function wrongModeAccessor(pythonCode: string, mode?: CodeExecutionMode): string | undefined { + if (!mode) return undefined; + const { undefined: wrong } = MODE_ACCESSOR[mode]; + return new RegExp(String.raw`(? { + const source = ` +const transform = node({ + type: 'n8n-nodes-base.code', + version: 2, + config: { + name: 'Transform', + parameters: { + mode: 'runOnceForEachItem', + language: 'pythonNative', + pythonCode: 'return _items[0]', + }, + }, +}); +export default workflow('id', 'name').add(transform); +`; + expect(lintWorkflowSource(source)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + lintTarget: 'pythonCode', + code: 'CODE_MODE_API_MISUSE', + }), + ]), + ); + }); }); diff --git a/packages/@n8n/workflow-sdk/src/lint/lint-workflow-source.ts b/packages/@n8n/workflow-sdk/src/lint/lint-workflow-source.ts index dd4609b10e5..f1dadd97287 100644 --- a/packages/@n8n/workflow-sdk/src/lint/lint-workflow-source.ts +++ b/packages/@n8n/workflow-sdk/src/lint/lint-workflow-source.ts @@ -48,7 +48,7 @@ export function lintWorkflowSource(source: string): SourceLintIssue[] { ); } else { embeddedIssues.push( - ...lintPythonCode(snippet.code).map((issue) => ({ + ...lintPythonCode(snippet.code, { mode: snippet.mode }).map((issue) => ({ ...issue, ...base, })), diff --git a/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts b/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts index fddbb8c8a2d..b169bcb4cce 100644 --- a/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts +++ b/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts @@ -14,7 +14,7 @@ const commonDescription: INodeProperties = { noDataExpression: true, builderHint: { propertyHint: - 'Runs in a locked-down native Python sandbox. NO imports: the stdlib allowlist (N8N_RUNNERS_STDLIB_ALLOW) is empty by default, so `import re`, `import json`, `import math`, `import datetime` and every third-party package FAIL at runtime with "Import of standard library module \'…\' is disallowed". Write import-free Python using builtins and str/list/dict methods, or set language to javaScript when the task genuinely needs a library. NO network access either: requests, urllib, httpx and other HTTP libraries are unavailable — use the HTTP Request node and process its output in this node instead. The ONLY globals are _items (runOnceForAllItems mode), _item (runOnceForEachItem mode) and print(); there are no cross-node helpers — _("Node Name"), _input, _json, _today and _jmespath are all undefined and raise NameError, so read data from the connected upstream node only.', + 'Runs in a locked-down native Python sandbox. NO imports: both allowlists (N8N_RUNNERS_STDLIB_ALLOW for standard-library modules, N8N_RUNNERS_EXTERNAL_ALLOW for packages) are empty by default, so `import re`, `import json`, `import math`, `import datetime`, `import pandas` and relative imports all FAIL at runtime with "Import of ... is disallowed". Write import-free Python using builtins and str/list/dict methods, or set language to javaScript when the task genuinely needs a library. NO network access either: requests, urllib, httpx and other HTTP libraries are unavailable — use the HTTP Request node and process its output in this node instead. The ONLY globals are _items (runOnceForAllItems mode ONLY), _item (runOnceForEachItem mode ONLY) and print(); reading the accessor belonging to the other mode raises NameError. There are no cross-node helpers either — _("Node Name"), _input, _json, _today and _jmespath are all undefined, so read data from the connected upstream node only.', }, };