diff --git a/packages/@n8n/agents/src/runtime/model/model-factory.ts b/packages/@n8n/agents/src/runtime/model/model-factory.ts index 12915be20fd..5759a589309 100644 --- a/packages/@n8n/agents/src/runtime/model/model-factory.ts +++ b/packages/@n8n/agents/src/runtime/model/model-factory.ts @@ -28,15 +28,18 @@ function isLanguageModel(config: unknown): config is LanguageModel { } /** - * Fallback proxy `fetch` used only when the caller does not inject one. + * Env-proxy `fetch` fallback for standalone SDK use. * - * Prefer passing a `fetch` built by `@n8n/backend-network` into - * {@link createModel} / {@link createEmbeddingModel}. + * `@n8n/agents` is a standalone SDK and deliberately does not depend on `@n8n/backend-network`, + * so it cannot build the backend's centrally-guarded transport itself. + * Inside the n8n backend that guarded `fetch` is always injected into {@link createModel} / {@link createEmbeddingModel} + * (see cli's `createAiProxyFetch`, which wraps `@n8n/backend-network`), and this fallback is never reached. */ function getProxyFetch(): FetchFn | undefined { const proxyUrl = process.env.HTTPS_PROXY ?? process.env.HTTP_PROXY; if (!proxyUrl) return undefined; + // eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- standalone SDK cannot depend on @n8n/backend-network; the backend always injects its guarded transport, so this env-proxy path runs only outside the backend (see doc comment above). To drop this: make `fetch` a required arg of createModel/createEmbeddingModel and delete the fallback, so standalone callers always supply their own transport const { ProxyAgent } = require('undici') as typeof Undici; const dispatcher = new ProxyAgent(proxyUrl); return (async (url, init) => diff --git a/packages/@n8n/ai-utilities/src/utils/http-proxy-agent.ts b/packages/@n8n/ai-utilities/src/utils/http-proxy-agent.ts index a3cb7ec4c2f..85cae886674 100644 --- a/packages/@n8n/ai-utilities/src/utils/http-proxy-agent.ts +++ b/packages/@n8n/ai-utilities/src/utils/http-proxy-agent.ts @@ -1,3 +1,4 @@ +/* eslint-disable n8n-local-rules/no-uncentralized-http -- langchain consumers pin undici v6, incompatible with backend-network's v7 dispatchers; see block comment below */ /** * Proxy/transport helpers for the AI model suppliers. * diff --git a/packages/@n8n/ai-workflow-builder.ee/src/tools/utils/web-fetch.utils.ts b/packages/@n8n/ai-workflow-builder.ee/src/tools/utils/web-fetch.utils.ts index 9ec5f0bb115..54f02f3d5d6 100644 --- a/packages/@n8n/ai-workflow-builder.ee/src/tools/utils/web-fetch.utils.ts +++ b/packages/@n8n/ai-workflow-builder.ee/src/tools/utils/web-fetch.utils.ts @@ -1,4 +1,5 @@ import { HumanMessage, type BaseMessage } from '@langchain/core/messages'; +// eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- axios is the only client exposing manual per-hop redirect control + streaming the cross-host SSRF approval guard needs. To migrate: move this package off undici v6, then route via the factory's getDispatcher() + undici.request({ maxRedirections: 0 }) (see fetchUrl doc) import axios, { type AxiosRequestConfig } from 'axios'; import type { Readable } from 'node:stream'; diff --git a/packages/@n8n/backend-network/README.md b/packages/@n8n/backend-network/README.md index 217b3df7a26..0cfc6b18959 100644 --- a/packages/@n8n/backend-network/README.md +++ b/packages/@n8n/backend-network/README.md @@ -10,6 +10,57 @@ This package consolidates into one place behind a single factory contract: SSRF/DNS guarding, proxy handling, and the HTTP client plumbing. The eventual goal is to make backend network behavior reviewable and controllable from a single entry point. -## Roadmap +## Using the factory -Tracked in [CAT-3365](https://linear.app/n8n/issue/CAT-3365/unify-http-concerns-scope-the-backend-network-layer-into-n8nbackend). \ No newline at end of file +Backend code that needs to make an outbound HTTP request should obtain a client +from this package rather than importing an HTTP library directly. That way every +call inherits SSRF/DNS guarding and proxy handling from one place. + +Inject the `OutboundHttp` service and pick by intent: + +```ts +import { Service } from '@n8n/di'; +import { OutboundHttp } from '@n8n/backend-network'; + +@Service() +export class MyService { + constructor(private readonly http: OutboundHttp) {} + + // You make a request and get a response. + async fetchThing() { + return await this.http.requests().request({ url: 'https://api.example.com/thing' }); + } + + // You hand a guarded transport to a third-party SDK. + makeClient() { + const fetch = this.http.transport().asCustomFetch(); + return new SomeSdk({ fetch }); + } +} +``` + +In DI-less code (e.g. task-runner), build the transport directly from the +dependency-free subpath: `import { buildDispatcher } from '@n8n/backend-network/transport'`. + +## The boundary rule + +The `n8n-local-rules/no-uncentralized-http` ESLint rule enforces this. +It is on by default for every Node backend package. + +Two sanctioned escape hatches, depending on the shape of the exception: + +**1. Inline disable** When a single callsite legitimately cannot use the factory, disable the +rule on the line with a justifying comment: + +```ts +// eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- +import axios from 'axios'; +``` + +Always include the reason after `--`. + +**2. Central allow list** For whole packages that are out of scope, add the file +path (a substring of the absolute path is enough) to the `allow` list in +`packages/@n8n/eslint-config/src/configs/backend-network-boundary.ts`. + +Keep the list shrinking: every entry is debt or a documented carve-out, not a default. diff --git a/packages/@n8n/backend-network/src/http/__tests__/transport-subpath-purity.test.ts b/packages/@n8n/backend-network/src/http/__tests__/transport-subpath-purity.test.ts deleted file mode 100644 index a706f3be4ec..00000000000 --- a/packages/@n8n/backend-network/src/http/__tests__/transport-subpath-purity.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { existsSync, readFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; - -// Guards the DI-less bundle: the `@n8n/backend-network/transport` subpath -// must stay free of DI / config / backend-common at runtime, so DI-less callers -// can build transport without dragging the full `OutboundHttp` service and its -// backend dependencies into their bundle. -// -// This walks the *runtime* import graph from `src/transport.ts` (following only -// relative, non-type imports/exports — `import type` / `export type` are erased -// by tsc) and asserts no forbidden package is reachable. - -const FORBIDDEN_PACKAGES = ['@n8n/di', '@n8n/backend-common', '@n8n/config', 'cache-manager']; - -const ENTRY = resolve(__dirname, '../../transport.ts'); - -interface ImportRef { - specifier: string; - typeOnly: boolean; -} - -/** Extract `import`/`export ... from` specifiers from a source file. */ -function parseImports(source: string): ImportRef[] { - const refs: ImportRef[] = []; - - // `import ... from ''` / `export ... from ''`. Requiring `from` (and - // disallowing `;` before it) avoids matching value expressions like - // `?? 'env'` inside a function body. - const fromRe = /(?:^|\n)\s*(import|export)(\s+type)?\b[^;]*?\bfrom\s*['"]([^'"]+)['"]/g; - let match: RegExpExecArray | null; - while ((match = fromRe.exec(source)) !== null) { - const [, , typeKeyword, specifier] = match; - refs.push({ specifier, typeOnly: Boolean(typeKeyword) }); - } - - // Bare side-effect imports: `import '';` (always runtime). - const bareRe = /(?:^|\n)\s*import\s+['"]([^'"]+)['"]/g; - while ((match = bareRe.exec(source)) !== null) { - refs.push({ specifier: match[1], typeOnly: false }); - } - - return refs; -} - -function resolveRelative(fromFile: string, specifier: string): string | undefined { - const base = resolve(dirname(fromFile), specifier); - const candidates = [base, `${base}.ts`, resolve(base, 'index.ts')]; - return candidates.find((candidate) => existsSync(candidate) && candidate.endsWith('.ts')); -} - -/** All bare (non-relative) specifiers reachable at runtime from the entry file. */ -function collectRuntimeExternals(entry: string): Set { - const externals = new Set(); - const visited = new Set(); - - const visit = (file: string) => { - if (visited.has(file)) return; - visited.add(file); - - const source = readFileSync(file, 'utf8'); - for (const { specifier, typeOnly } of parseImports(source)) { - if (typeOnly) continue; // erased at compile time — no runtime dependency - if (specifier.startsWith('.')) { - const resolved = resolveRelative(file, specifier); - if (resolved) visit(resolved); - continue; - } - externals.add(specifier); - } - }; - - visit(entry); - return externals; -} - -describe('@n8n/backend-network/transport subpath purity', () => { - it('has a resolvable entry file', () => { - expect(existsSync(ENTRY)).toBe(true); - }); - - it('does not pull DI / config / backend-common into the runtime graph', () => { - const externals = collectRuntimeExternals(ENTRY); - - for (const forbidden of FORBIDDEN_PACKAGES) { - const leaked = [...externals].some( - (specifier) => specifier === forbidden || specifier.startsWith(`${forbidden}/`), - ); - expect( - leaked, - `forbidden runtime dependency reachable from transport subpath: ${forbidden}`, - ).toBe(false); - } - }); - - it('only depends on undici and n8n-workflow at runtime', () => { - const externals = collectRuntimeExternals(ENTRY); - - expect([...externals].sort()).toEqual(['n8n-workflow', 'undici']); - }); -}); diff --git a/packages/@n8n/eslint-config/src/configs/backend-network-boundary.ts b/packages/@n8n/eslint-config/src/configs/backend-network-boundary.ts new file mode 100644 index 00000000000..af88c63d5f7 --- /dev/null +++ b/packages/@n8n/eslint-config/src/configs/backend-network-boundary.ts @@ -0,0 +1,28 @@ +import tseslint from 'typescript-eslint'; + +/** + * Backend network boundary. + * + * Backend outbound HTTP must go through the `@n8n/backend-network` factory so + * SSRF/DNS guarding and proxy handling stay centrally controlled. This turns on + * `n8n-local-rules/no-uncentralized-http` for every Node backend package (it is + * part of `nodeConfig`). + * + * Out of natural scope: + * - Frontend packages (they use `frontendConfig`, not `nodeConfig`) + * - `@n8n/backend-network` itself (it uses `baseConfig`, not `nodeConfig`) + * + * Prefer an inline `// eslint-disable-next-line ... -- ` for a single + * callsite. Use the lists below only for whole-path scope exclusions or tracked + * migration debt. See `packages/@n8n/backend-network/README.md`. + */ +export const backendNetworkBoundaryConfig = tseslint.config({ + rules: { + 'n8n-local-rules/no-uncentralized-http': [ + 'error', + { + allow: ['packages/@n8n/benchmark/'], + }, + ], + }, +}); diff --git a/packages/@n8n/eslint-config/src/configs/node.ts b/packages/@n8n/eslint-config/src/configs/node.ts index ddf126d28f6..648adcb698d 100644 --- a/packages/@n8n/eslint-config/src/configs/node.ts +++ b/packages/@n8n/eslint-config/src/configs/node.ts @@ -1,8 +1,9 @@ import tseslint from 'typescript-eslint'; import globals from 'globals'; import { baseConfig } from './base.js'; +import { backendNetworkBoundaryConfig } from './backend-network-boundary.js'; -export const nodeConfig = tseslint.config(baseConfig, { +export const nodeConfig = tseslint.config(baseConfig, backendNetworkBoundaryConfig, { languageOptions: { ecmaVersion: 2024, globals: globals.node, diff --git a/packages/@n8n/eslint-config/src/rules/index.ts b/packages/@n8n/eslint-config/src/rules/index.ts index a8fd5f7bd7a..324eb2509a3 100644 --- a/packages/@n8n/eslint-config/src/rules/index.ts +++ b/packages/@n8n/eslint-config/src/rules/index.ts @@ -18,6 +18,7 @@ import { NoInternalPackageImportRule } from './no-internal-package-import.js'; import { NoImportEnterpriseEditionRule } from './no-import-enterprise-edition.js'; import { NoTypeOnlyImportInDiRule } from './no-type-only-import-in-di.js'; import { NoErrorInstanceInToThrowRule } from './no-error-instance-in-to-throw.js'; +import { NoUncentralizedHttpRule } from './no-uncentralized-http.js'; export const rules = { 'no-uncaught-json-parse': NoUncaughtJsonParseRule, @@ -39,4 +40,5 @@ export const rules = { 'no-import-enterprise-edition': NoImportEnterpriseEditionRule, 'no-type-only-import-in-di': NoTypeOnlyImportInDiRule, 'no-error-instance-in-to-throw': NoErrorInstanceInToThrowRule, + 'no-uncentralized-http': NoUncentralizedHttpRule, } satisfies Record; diff --git a/packages/@n8n/eslint-config/src/rules/no-uncentralized-http.test.ts b/packages/@n8n/eslint-config/src/rules/no-uncentralized-http.test.ts new file mode 100644 index 00000000000..b1ae7a0ee71 --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/no-uncentralized-http.test.ts @@ -0,0 +1,193 @@ +import { RuleTester, type InvalidTestCase } from '@typescript-eslint/rule-tester'; +import { NoUncentralizedHttpRule } from './no-uncentralized-http.js'; + +type MessageIds = 'useBackendNetwork' | 'addReviewedException'; +type Options = [{ allow?: string[] }]; + +const ruleTester = new RuleTester({ + languageOptions: { + parser: require('@typescript-eslint/parser'), + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + }, + }, +}); + +const runtimeFile = '/repo/packages/cli/src/service.ts'; + +const REVIEWED_EXCEPTION_COMMENT = + '// eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- TODO: explain why @n8n/backend-network cannot be used here'; + +const exceptionSuggestion = (code: string) => ({ + messageId: 'addReviewedException' as const, + output: `${REVIEWED_EXCEPTION_COMMENT}\n${code}`, +}); + +const withSuggestions = ( + cases: Array>, +): Array> => + cases.map((testCase) => ({ + ...testCase, + errors: testCase.errors.map((error) => ({ + ...error, + suggestions: [exceptionSuggestion(testCase.code)], + })), + })); + +ruleTester.run('no-uncentralized-http', NoUncentralizedHttpRule, { + valid: [ + // Type-only imports carry no runtime behavior. + { code: "import type { AxiosRequestConfig } from 'axios';", filename: runtimeFile }, + { code: "import { type AxiosRequestConfig } from 'axios';", filename: runtimeFile }, + { code: "import type { Dispatcher } from 'undici';", filename: runtimeFile }, + // axios error/guard symbols perform no request. + { code: "import { AxiosError } from 'axios';", filename: runtimeFile }, + { code: "import { isAxiosError, CanceledError } from 'axios';", filename: runtimeFile }, + // node http/https server primitives are unaffected; only `Agent` is restricted. + { code: "import { createServer } from 'node:http';", filename: runtimeFile }, + { code: "import type { Agent } from 'node:https';", filename: runtimeFile }, + // Re-exporting a type, or a node-http namespace, carries no request behavior. + { code: "export type { AxiosRequestConfig } from 'axios';", filename: runtimeFile }, + { code: "export { type Dispatcher } from 'undici';", filename: runtimeFile }, + { code: "export { isAxiosError } from 'axios';", filename: runtimeFile }, + { code: "export * from 'node:http';", filename: runtimeFile }, + // Dynamic import / require of unrelated or node-http modules. + { code: "const http = require('node:http');", filename: runtimeFile }, + { code: "async function f() { await import('node:https'); }", filename: runtimeFile }, + { code: "const lib = require('express');", filename: runtimeFile }, + // `import x = require(...)` — namespace binding; node-http parity and unrelated modules pass. + { code: "import http = require('node:http');", filename: runtimeFile }, + { code: "import express = require('express');", filename: runtimeFile }, + // Unrelated modules. + { code: "import express from 'express';", filename: runtimeFile }, + { code: "import { helper } from './local';", filename: runtimeFile }, + // Allow-listed file (reviewed exception). + { + code: "import axios from 'axios';", + filename: '/repo/packages/cli/src/oauth/oauth.service.ts', + options: [{ allow: ['packages/cli/src/oauth/oauth.service.ts'] }], + }, + { + code: "import { ProxyAgent } from 'undici';", + filename: '/repo/packages/nodes-base/credentials/foo.ts', + options: [{ allow: ['packages/nodes-base/'] }], + }, + // Allow-list substrings (forward-slash) match on Windows backslash paths. + { + code: "import axios from 'axios';", + filename: 'C:\\repo\\packages\\cli\\src\\oauth\\oauth.service.ts', + options: [{ allow: ['packages/cli/src/oauth/oauth.service.ts'] }], + }, + // Tests and fixtures import these libraries to mock them, not to call out. + { code: "import axios from 'axios';", filename: '/repo/packages/cli/src/service.test.ts' }, + { + code: "import axios from 'axios';", + filename: '/repo/packages/cli/src/__tests__/service.ts', + }, + { + code: "import axios from 'axios';", + filename: '/repo/packages/@n8n/ai-utilities/integration-tests/openai.fixtures.ts', + }, + ], + + invalid: withSuggestions([ + { + code: "import axios from 'axios';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'axios' } }], + }, + { + code: "import axios, { AxiosError } from 'axios';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import * as axios from 'axios';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import { request } from 'axios';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import { ProxyAgent } from 'undici';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'undici' } }], + }, + { + code: "import { Agent } from 'undici';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import { HttpsProxyAgent } from 'https-proxy-agent';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import proxyFromEnv from 'proxy-from-env';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import 'undici';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork' }], + }, + { + code: "import { Agent } from 'node:http';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'node:http' } }], + }, + // Re-exports pull the client into consumers just like a direct import. + { + code: "export { request } from 'axios';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'axios' } }], + }, + { + code: "export * from 'undici';", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'undici' } }], + }, + // Dynamic import / require load the whole client at runtime. + { + code: "async function f() { await import('axios'); }", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'axios' } }], + }, + { + code: "const { ProxyAgent } = require('undici');", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'undici' } }], + }, + // `import x = require('axios')` — TS import-equals form loads the client. + { + code: "import axios = require('axios');", + filename: runtimeFile, + errors: [{ messageId: 'useBackendNetwork', data: { module: 'axios' } }], + }, + ]).concat([ + // Indented callsite: the quick-fix preserves indentation on the inserted line. + { + code: 'function f() {\n\tconst x = require("axios");\n}', + filename: runtimeFile, + errors: [ + { + messageId: 'useBackendNetwork', + data: { module: 'axios' }, + suggestions: [ + { + messageId: 'addReviewedException', + output: + 'function f() {\n\t// eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- TODO: explain why @n8n/backend-network cannot be used here\n\tconst x = require("axios");\n}', + }, + ], + }, + ], + }, + ]), +}); diff --git a/packages/@n8n/eslint-config/src/rules/no-uncentralized-http.ts b/packages/@n8n/eslint-config/src/rules/no-uncentralized-http.ts new file mode 100644 index 00000000000..d344636077f --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/no-uncentralized-http.ts @@ -0,0 +1,241 @@ +import { ESLintUtils, type TSESTree } from '@typescript-eslint/utils'; +import { RuleContext } from '@typescript-eslint/utils/ts-eslint'; + +type Options = [{ allow?: string[] }]; +type MessageIds = 'useBackendNetwork' | 'addReviewedException'; + +const DOCS_URL = + 'https://github.com/n8n-io/n8n/blob/master/packages/@n8n/backend-network/README.md'; + +const RESTRICTED_MODULES = new Set([ + 'axios', + 'undici', + 'http-proxy-agent', + 'https-proxy-agent', + 'proxy-from-env', +]); + +const NODE_HTTP_MODULES = new Set(['http', 'https', 'node:http', 'node:https']); + +/** + * axios symbols that perform no request + */ +const ALLOWED_AXIOS_VALUE_IMPORTS = new Set([ + 'AxiosError', + 'AxiosHeaders', + 'CanceledError', + 'isAxiosError', + 'isCancel', +]); + +const NON_RUNTIME_FILE = /(\.test\.ts|\.spec\.ts|\/__tests__\/|\/test\/|\/integration-tests\/)/; + +export const NoUncentralizedHttpRule = ESLintUtils.RuleCreator.withoutDocs({ + meta: { + type: 'problem', + hasSuggestions: true, + docs: { + description: + 'Disallow direct backend imports of HTTP client/proxy libraries; outbound HTTP must go through the @n8n/backend-network factory.', + url: DOCS_URL, + }, + messages: { + useBackendNetwork: + "Importing '{{ module }}' opens an outbound connection that bypasses n8n's SSRF/DNS guarding and proxy handling. Route it through @n8n/backend-network instead: inject the `OutboundHttp` service, then `.requests()` to send a request or `.transport()` to hand a guarded fetch/dispatcher to an SDK (DI-less code: import from '@n8n/backend-network/transport'). Sanctioned exceptions and the full factory API are in the rule docs.", + addReviewedException: + 'Mark this line as a reviewed exception (inserts an eslint-disable with a TODO reason to complete)', + }, + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + allow: { + type: 'array', + items: { type: 'string' }, + description: 'File path substrings exempt from this rule (reviewed exceptions).', + }, + }, + }, + ], + }, + defaultOptions: [{ allow: [] }], + create(context, [options]) { + const filename = context.filename.replace(/\\/g, '/'); + + if (NON_RUNTIME_FILE.test(filename)) { + return {}; + } + + const allow = options?.allow ?? []; + if (allow.some((entry) => filename.includes(entry))) { + return {}; + } + + return { + ImportDeclaration(node) { + const module = node.source.value; + if (!RESTRICTED_MODULES.has(module) && !NODE_HTTP_MODULES.has(module)) { + return; + } + + if (node.importKind === 'type') { + return; + } + + // `import 'undici'`: bare side-effect import still loads the library. + if (RESTRICTED_MODULES.has(module) && node.specifiers.length === 0) { + report(node, module, context); + return; + } + + node.specifiers + .filter( + (specifier) => specifier.type !== 'ImportSpecifier' || specifier.importKind !== 'type', + ) + .forEach((specifier) => { + const importedName = + specifier.type === 'ImportSpecifier' && specifier.imported.type === 'Identifier' + ? specifier.imported.name + : undefined; + + reportNamedValue(specifier, module, importedName, context); + }); + }, + + // `export { request } from 'axios'`: re-exporting a value pulls the library into consumers exactly like a direct import. + ExportNamedDeclaration(node) { + if (!node.source) { + // local re-export (`export { x }`), no module. + return; + } + + const module = node.source.value; + if (!RESTRICTED_MODULES.has(module) && !NODE_HTTP_MODULES.has(module)) { + return; + } + + if (node.exportKind === 'type') { + return; + } + + node.specifiers + .filter((specifier) => specifier.exportKind !== 'type') + .forEach((specifier) => { + const importedName = + specifier.local.type === 'Identifier' ? specifier.local.name : undefined; + reportNamedValue(specifier, module, importedName, context); + }); + }, + + // `export * from 'undici'` re-exports the whole client + ExportAllDeclaration(node) { + const module = node.source.value; + if (!RESTRICTED_MODULES.has(module)) { + return; + } + if (node.exportKind === 'type') { + return; + } + report(node, module, context); + }, + + // Dynamic/runtime module loads (`import()`, `require`, `import =`) only restrict + // RESTRICTED_MODULES, not node:http/https: a runtime load returns the whole module, + // so blocking `Agent` would also block `createServer`, the same accepted gap as the + // namespace/default static import above. + // + // Dynamic `import('axios')` loads the whole client at runtime. + ImportExpression(node) { + if (node.source.type !== 'Literal' || typeof node.source.value !== 'string') { + return; + } + if (!RESTRICTED_MODULES.has(node.source.value)) { + return; + } + report(node, node.source.value, context); + }, + + // `require('axios')`: same as a dynamic import for our purposes. + CallExpression(node) { + if (node.callee.type !== 'Identifier' || node.callee.name !== 'require') { + return; + } + const [arg] = node.arguments; + if (!arg || arg.type !== 'Literal' || typeof arg.value !== 'string') { + return; + } + if (!RESTRICTED_MODULES.has(arg.value)) { + return; + } + report(node, arg.value, context); + }, + + // `import axios = require('axios')`: the TS import-equals form. + TSImportEqualsDeclaration(node) { + if (node.moduleReference.type !== 'TSExternalModuleReference') { + return; + } + const { expression } = node.moduleReference; + if (expression.type !== 'Literal' || typeof expression.value !== 'string') { + return; + } + if (!RESTRICTED_MODULES.has(expression.value)) { + return; + } + report(node, expression.value, context); + }, + }; + }, +}); + +const REVIEWED_EXCEPTION_COMMENT = + '// eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- TODO: explain why @n8n/backend-network cannot be used here'; +function report( + node: TSESTree.Node, + module: string, + context: Readonly>, +): void { + context.report({ + node, + messageId: 'useBackendNetwork', + data: { module }, + suggest: [ + { + messageId: 'addReviewedException', + fix: (fixer) => { + const { line } = node.loc.start; + const indent = /^\s*/.exec(context.sourceCode.lines[line - 1])?.[0] ?? ''; + const lineStart = context.sourceCode.getIndexFromLoc({ line, column: 0 }); + return fixer.insertTextBeforeRange( + [lineStart, lineStart], + `${indent}${REVIEWED_EXCEPTION_COMMENT}\n`, + ); + }, + }, + ], + }); +} + +function reportNamedValue( + node: TSESTree.Node, + module: string, + importedName: string | undefined, + context: Readonly>, +) { + if (NODE_HTTP_MODULES.has(module)) { + // Only the raw `Agent` class is restricted from node http/https. + // A namespace/default binding (`import http from 'node:http'`) can still reach `http.Agent`, + // but banning it would also forbid `createServer`; that gap is accepted. + if (importedName === 'Agent') { + report(node, module, context); + } + return; + } + + if (module === 'axios' && importedName && ALLOWED_AXIOS_VALUE_IMPORTS.has(importedName)) { + return; + } + + report(node, module, context); +} diff --git a/packages/nodes-base/credentials/common/aws/utils.ts b/packages/nodes-base/credentials/common/aws/utils.ts index 7b343f71793..3b41bd2a225 100644 --- a/packages/nodes-base/credentials/common/aws/utils.ts +++ b/packages/nodes-base/credentials/common/aws/utils.ts @@ -21,6 +21,7 @@ import { type AwsSecurityHeaders, } from './types'; import { sign } from 'aws4'; +// eslint-disable-next-line n8n-local-rules/no-uncentralized-http -- TODO: will be migrated in CAT-3538 import { ProxyAgent } from 'undici'; import { getSystemCredentials } from './system-credentials-utils'; diff --git a/packages/testing/code-health/src/index.ts b/packages/testing/code-health/src/index.ts index c38419c1f2b..a473173086d 100644 --- a/packages/testing/code-health/src/index.ts +++ b/packages/testing/code-health/src/index.ts @@ -6,6 +6,7 @@ import { CatalogViolationsRule } from './rules/catalog-violations.rule.js'; import { EndpointScopeCoverageRule } from './rules/endpoint-scope-coverage.rule.js'; import { MigrationTimestampRule } from './rules/migration-timestamp.rule.js'; import { StaleOverridesRule } from './rules/stale-overrides.rule.js'; +import { SubpathPurityRule } from './rules/subpath-purity.rule.js'; import { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js'; export type { CodeHealthContext } from './context.js'; @@ -13,6 +14,8 @@ export { CatalogViolationsRule } from './rules/catalog-violations.rule.js'; export { EndpointScopeCoverageRule } from './rules/endpoint-scope-coverage.rule.js'; export { MigrationTimestampRule } from './rules/migration-timestamp.rule.js'; export { StaleOverridesRule } from './rules/stale-overrides.rule.js'; +export { SubpathPurityRule } from './rules/subpath-purity.rule.js'; +export type { SubpathSpec } from './rules/subpath-purity.rule.js'; export { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js'; const defaultRuleSettings: RuleSettingsMap = { @@ -43,6 +46,20 @@ const defaultRuleSettings: RuleSettingsMap = { severity: 'warning', options: { packages: ['packages/cli'] }, }, + 'subpath-purity': { + enabled: true, + severity: 'error', + options: { + subpaths: [ + { + name: '@n8n/backend-network/transport', + entry: 'packages/@n8n/backend-network/src/transport.ts', + forbidden: ['@n8n/di', '@n8n/backend-common', '@n8n/config', 'cache-manager'], + allowedExternals: ['n8n-workflow', 'undici'], + }, + ], + }, + }, }; function mergeSettings(defaults: RuleSettingsMap, overrides?: RuleSettingsMap): RuleSettingsMap { @@ -65,6 +82,7 @@ export function createDefaultRunner(settings?: RuleSettingsMap): RuleRunner { + let rootDir: string; + const rule = new SubpathPurityRule(); + + const write = (relativePath: string, source: string) => { + const abs = path.join(rootDir, relativePath); + mkdirSync(path.dirname(abs), { recursive: true }); + writeFileSync(abs, source, 'utf8'); + }; + + const analyze = (subpath: SubpathSpec) => { + rule.configure({ options: { subpaths: [subpath] } }); + const context: CodeHealthContext = { rootDir }; + return rule.analyze(context); + }; + + const SPEC: SubpathSpec = { + name: 'pkg/transport', + entry: 'src/transport.ts', + forbidden: ['@n8n/di', '@n8n/config'], + allowedExternals: ['undici', 'n8n-workflow'], + }; + + beforeEach(() => { + rootDir = mkdtempSync(path.join(tmpdir(), 'subpath-purity-')); + }); + + afterEach(() => { + rmSync(rootDir, { recursive: true, force: true }); + }); + + it('passes when the runtime graph only reaches allowed externals', () => { + write('src/transport.ts', "export { send } from './client';"); + write('src/client.ts', "import { request } from 'undici';\nexport const send = request;"); + + expect(analyze(SPEC)).toEqual([]); + }); + + it('flags a forbidden package reached transitively through a value import', () => { + write('src/transport.ts', "export { send } from './client';"); + write('src/client.ts', "import { Container } from '@n8n/di';\nexport const send = Container;"); + + const violations = analyze(SPEC); + + expect(violations.map((v) => v.message)).toContainEqual( + expect.stringContaining('forbidden runtime dependency "@n8n/di"'), + ); + // Points at the file that actually imports it, not the entry. + expect(violations[0].file).toBe(path.join(rootDir, 'src/client.ts')); + }); + + it('reports a forbidden package once, not also as an unexpected external', () => { + write('src/transport.ts', "export { send } from './client';"); + write('src/client.ts', "import { Container } from '@n8n/di';\nexport const send = Container;"); + + const messages = analyze(SPEC).map((v) => v.message); + + expect(messages).toEqual([expect.stringContaining('forbidden runtime dependency "@n8n/di"')]); + expect(messages).not.toContainEqual(expect.stringContaining('unexpected runtime dependency')); + }); + + it('ignores a forbidden package imported only as a type', () => { + write('src/transport.ts', "import type { Container } from '@n8n/di';\nexport const x = 1;"); + + expect(analyze(SPEC)).toEqual([]); + }); + + it('catches a forbidden package behind a dynamic import', () => { + write('src/transport.ts', "export async function load() { return import('@n8n/config'); }"); + + const violations = analyze(SPEC); + + expect(violations.map((v) => v.message)).toContainEqual( + expect.stringContaining('forbidden runtime dependency "@n8n/config"'), + ); + }); + + it('flags an external outside the allowlist even when not forbidden', () => { + write('src/transport.ts', "import { z } from 'zod';\nexport const schema = z;"); + + const violations = analyze(SPEC); + + expect(violations.map((v) => v.message)).toContainEqual( + expect.stringContaining('unexpected runtime dependency "zod"'), + ); + }); + + it('reports a missing entry file instead of throwing', () => { + const violations = analyze({ ...SPEC, entry: 'src/does-not-exist.ts' }); + + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain('entry not found'); + }); +}); diff --git a/packages/testing/code-health/src/rules/subpath-purity.rule.ts b/packages/testing/code-health/src/rules/subpath-purity.rule.ts new file mode 100644 index 00000000000..a582182ea58 --- /dev/null +++ b/packages/testing/code-health/src/rules/subpath-purity.rule.ts @@ -0,0 +1,105 @@ +import { BaseRule } from '@n8n/rules-engine'; +import type { Violation } from '@n8n/rules-engine'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +import type { CodeHealthContext } from '../context.js'; +import { collectRuntimeExternals } from '../utils/import-graph-scanner.js'; + +/** One DI-less (or otherwise constrained) subpath to keep pure. */ +export interface SubpathSpec { + /** Human-readable subpath name, used in messages. */ + name: string; + /** Repo-relative path to the subpath's entry source file. */ + entry: string; + /** Bare packages that must never be reachable at runtime from `entry`. */ + forbidden: string[]; + /** + * Optional exact allowlist of runtime externals. When set, any reachable + * bare specifier outside this list is flagged, locking the dependency + * surface, not just the forbidden set. + */ + allowedExternals?: string[]; +} + +/** + * Guards constrained entry points (e.g. the DI-less `@n8n/backend-network/transport` subpath) + * by walking their runtime import graph and asserting no forbidden package (DI, config, backend-common) + * is reachable, so DI-less callers don't drag the full service into their bundle. + */ +export class SubpathPurityRule extends BaseRule { + readonly id = 'subpath-purity'; + readonly name = 'Subpath Import Purity'; + readonly description = + 'Constrained entry points must not reach forbidden packages at runtime (keeps DI-less bundles free of DI/config/backend dependencies)'; + readonly severity = 'error' as const; + + analyze(context: CodeHealthContext): Violation[] { + const subpaths = this.getSubpaths(); + return subpaths.flatMap((subpath) => this.analyzeSubpath(context.rootDir, subpath)); + } + + private getSubpaths(): SubpathSpec[] { + const raw = this.getOptions().subpaths; + return Array.isArray(raw) ? (raw as SubpathSpec[]) : []; + } + + private analyzeSubpath(rootDir: string, subpath: SubpathSpec): Violation[] { + const entry = path.resolve(rootDir, subpath.entry); + + if (!fs.existsSync(entry)) { + return [ + this.createViolation( + entry, + 1, + 1, + `Subpath "${subpath.name}" entry not found at ${subpath.entry}.`, + 'Update the rule options to point at the current entry file, or remove the obsolete subpath spec.', + ), + ]; + } + + const externals = collectRuntimeExternals(entry); + const violations: Violation[] = []; + + for (const forbidden of subpath.forbidden) { + for (const ref of externals.values()) { + const leaked = ref.specifier === forbidden || ref.specifier.startsWith(`${forbidden}/`); + if (!leaked) continue; + violations.push( + this.createViolation( + ref.file, + ref.line, + 1, + `"${subpath.name}" reaches forbidden runtime dependency "${ref.specifier}".`, + `Keep ${forbidden} out of the runtime graph: import it as a type (\`import type\`), move the value behind a DI-aware entry point, or load it lazily so the DI-less subpath stays clean.`, + ), + ); + } + } + + if (subpath.allowedExternals) { + const allowed = new Set(subpath.allowedExternals); + for (const ref of externals.values()) { + if (allowed.has(ref.specifier)) continue; + // Forbidden specifiers are already reported above with a clearer message. + const isForbidden = subpath.forbidden.some( + (forbidden) => ref.specifier === forbidden || ref.specifier.startsWith(`${forbidden}/`), + ); + if (!isForbidden) { + violations.push( + this.createViolation( + ref.file, + ref.line, + 1, + `"${subpath.name}" pulls in unexpected runtime dependency "${ref.specifier}".`, + `Allowed runtime externals are: ${subpath.allowedExternals.join(', ')}. Add this specifier to the allowlist only if it is genuinely safe for the DI-less subpath.`, + ), + ); + } + } + } + + return violations; + } +} diff --git a/packages/testing/code-health/src/utils/import-graph-scanner.ts b/packages/testing/code-health/src/utils/import-graph-scanner.ts new file mode 100644 index 00000000000..9ea6063aed4 --- /dev/null +++ b/packages/testing/code-health/src/utils/import-graph-scanner.ts @@ -0,0 +1,60 @@ +import { parseImports } from '@n8n/rules-engine/ast'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { Project } from 'ts-morph'; + +/** Where a bare specifier first enters the runtime graph. */ +export interface ExternalRef { + specifier: string; + /** Absolute path of the file that imports it. */ + file: string; + line: number; +} + +function resolveRelative(fromFile: string, specifier: string): string | undefined { + // Strip a NodeNext `.js`/`.jsx` extension back to its TS source. + const asTs = specifier.replace(/\.jsx?$/, ''); + const base = path.resolve(path.dirname(fromFile), asTs); + const candidates = [base, `${base}.ts`, `${base}.tsx`, path.resolve(base, 'index.ts')]; + return candidates.find((candidate) => fs.existsSync(candidate) && /\.tsx?$/.test(candidate)); +} + +/** + * Walk the runtime import graph from `entry`, following only relative, + * non-type imports/exports (`import type` / `export type` are erased by tsc), + * and return every bare (non-relative) specifier reachable at runtime, keyed + * by the first file that imports it. + */ +export function collectRuntimeExternals(entry: string): Map { + const externals = new Map(); + const visited = new Set(); + const project = new Project({ + skipAddingFilesFromTsConfig: true, + skipFileDependencyResolution: true, + }); + + const visit = (file: string) => { + if (visited.has(file)) return; + visited.add(file); + + const sourceFile = project.addSourceFileAtPath(file); + for (const { specifier, typeOnly, line } of parseImports(sourceFile)) { + // erased at compile time — no runtime dependency + if (!typeOnly) { + if (specifier.startsWith('.')) { + const resolved = resolveRelative(file, specifier); + if (resolved) { + visit(resolved); + } + continue; + } + if (!externals.has(specifier)) { + externals.set(specifier, { specifier, file, line }); + } + } + } + }; + + visit(entry); + return externals; +} diff --git a/packages/testing/rules-engine/src/ast/imports.test.ts b/packages/testing/rules-engine/src/ast/imports.test.ts new file mode 100644 index 00000000000..c4c22e9e76a --- /dev/null +++ b/packages/testing/rules-engine/src/ast/imports.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest'; + +import { parseImports } from './imports.js'; +import { createInMemoryProject } from './project.js'; + +describe('parseImports', () => { + const parse = (source: string) => { + const project = createInMemoryProject(); + const sourceFile = project.createSourceFile('module.ts', source); + return parseImports(sourceFile).map(({ specifier, typeOnly }) => ({ specifier, typeOnly })); + }; + + it('treats `import type` / `export type` as erased', () => { + expect(parse("import type { Foo } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: true }, + ]); + expect(parse("export type { Foo } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: true }, + ]); + }); + + it('treats inline type-only imports / re-exports as erased', () => { + expect(parse("import { type Foo } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: true }, + ]); + expect(parse("export { type Foo } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: true }, + ]); + }); + + it('treats a mixed inline-type import / re-export as runtime', () => { + expect(parse("import { type Foo, bar } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: false }, + ]); + expect(parse("export { type Foo, bar } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: false }, + ]); + }); + + it('treats static value imports and re-exports as runtime', () => { + expect(parse("import { foo } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: false }, + ]); + expect(parse("export { foo } from './foo';")).toEqual([ + { specifier: './foo', typeOnly: false }, + ]); + expect(parse("import './bare';")).toEqual([{ specifier: './bare', typeOnly: false }]); + }); + + it('treats default and namespace imports as runtime', () => { + expect(parse("import Foo from './foo';")).toEqual([{ specifier: './foo', typeOnly: false }]); + expect(parse("import * as foo from './foo';")).toEqual([ + { specifier: './foo', typeOnly: false }, + ]); + }); + + it('ignores a bare `export {}` with no module specifier', () => { + expect(parse('const foo = 1;\nexport { foo };')).toEqual([]); + }); + + // AST over regex: dynamic forms are runtime dependencies a regex over + // import/export statements would silently miss. + it('catches dynamic import() and require() as runtime', () => { + expect(parse("async function f() { await import('@n8n/di'); }")).toEqual([ + { specifier: '@n8n/di', typeOnly: false }, + ]); + expect(parse("const di = require('@n8n/di');")).toEqual([ + { specifier: '@n8n/di', typeOnly: false }, + ]); + }); +}); diff --git a/packages/testing/rules-engine/src/ast/imports.ts b/packages/testing/rules-engine/src/ast/imports.ts new file mode 100644 index 00000000000..49139ef6de4 --- /dev/null +++ b/packages/testing/rules-engine/src/ast/imports.ts @@ -0,0 +1,89 @@ +import { Node, SyntaxKind } from 'ts-morph'; +import type { ExportDeclaration, ImportDeclaration, SourceFile } from 'ts-morph'; + +export interface ImportRef { + /** The module specifier, e.g. `./foo` or `@n8n/di`. */ + specifier: string; + /** True when the reference is erased by tsc (`import type` / `export type` / inline `type`). */ + typeOnly: boolean; + /** 1-based line of the import/export/call. */ + line: number; +} + +/** A static `import` is erased only if the whole clause is type-only. */ +function importIsTypeOnly(declaration: ImportDeclaration): boolean { + if (declaration.isTypeOnly()) { + return true; + } + // A default or namespace binding always runs at runtime. + if (declaration.getDefaultImport() || declaration.getNamespaceImport()) { + return false; + } + const named = declaration.getNamedImports(); + + // A bare side-effect import (no clause / no names) runs at runtime. + if (named.length === 0) { + return false; + } + return named.every((element) => element.isTypeOnly()); +} + +/** An `export ... from` is erased only if the whole clause is type-only. */ +function exportIsTypeOnly(declaration: ExportDeclaration): boolean { + if (declaration.isTypeOnly()) { + return true; + } + const named = declaration.getNamedExports(); + return named.length > 0 && named.every((element) => element.isTypeOnly()); +} + +/** Collect dynamic `import('')` and `require('')` — runtime forms a regex would miss. */ +function parseDynamicImports(sourceFile: SourceFile): ImportRef[] { + const refs: ImportRef[] = []; + for (const call of sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression)) { + const expression = call.getExpression(); + const isDynamicImport = expression.getKind() === SyntaxKind.ImportKeyword; + const isRequire = Node.isIdentifier(expression) && expression.getText() === 'require'; + const [arg] = call.getArguments(); + if ((isDynamicImport || isRequire) && arg && Node.isStringLiteral(arg)) { + refs.push({ + specifier: arg.getLiteralText(), + typeOnly: false, + line: arg.getStartLineNumber(), + }); + } + } + return refs; +} + +/** + * Extract module specifiers from a source file via the ts-morph AST. Covers + * static `import`/`export ... from`, bare side-effect imports, and the runtime + * forms a regex would miss: dynamic `import('')` and `require('')`. + */ +export function parseImports(sourceFile: SourceFile): ImportRef[] { + const refs: ImportRef[] = []; + + for (const decl of sourceFile.getImportDeclarations()) { + refs.push({ + specifier: decl.getModuleSpecifierValue(), + typeOnly: importIsTypeOnly(decl), + line: decl.getStartLineNumber(), + }); + } + + for (const decl of sourceFile.getExportDeclarations()) { + // Bare `export {}` (no `from`) has no specifier. + const specifier = decl.getModuleSpecifierValue(); + if (specifier === undefined) continue; + refs.push({ + specifier, + typeOnly: exportIsTypeOnly(decl), + line: decl.getStartLineNumber(), + }); + } + + refs.push(...parseDynamicImports(sourceFile)); + + return refs; +} diff --git a/packages/testing/rules-engine/src/ast/index.ts b/packages/testing/rules-engine/src/ast/index.ts index c346885b24c..bdc11faf337 100644 --- a/packages/testing/rules-engine/src/ast/index.ts +++ b/packages/testing/rules-engine/src/ast/index.ts @@ -2,3 +2,5 @@ export { AstRule } from './ast-rule.js'; export { buildPackageProjects, createInMemoryProject } from './project.js'; export type { AstProjectConfig, PackageProjectSpec, PackageProject } from './project.js'; export { classHasDecorator, getDecoratorByName, getDecoratorObjectFlag } from './decorators.js'; +export { parseImports } from './imports.js'; +export type { ImportRef } from './imports.js';