diff --git a/packages/@n8n/eslint-plugin-community-nodes/README.md b/packages/@n8n/eslint-plugin-community-nodes/README.md index df1d16fd89f..f4cd8903ca0 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/README.md +++ b/packages/@n8n/eslint-plugin-community-nodes/README.md @@ -43,18 +43,20 @@ export default [ 🔧 Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix).\ 💡 Manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). -| Name                             | Description | 💼 | ⚠️ | 🔧 | 💡 | -| :--------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------- | :--- | :--- | :- | :- | -| [credential-documentation-url](docs/rules/credential-documentation-url.md) | Enforce valid credential documentationUrl format (URL or camelCase slug) | ✅ ☑️ | | | | -| [credential-password-field](docs/rules/credential-password-field.md) | Ensure credential fields with sensitive names have typeOptions.password = true | ✅ ☑️ | | 🔧 | | -| [credential-test-required](docs/rules/credential-test-required.md) | Ensure credentials have a credential test | ✅ ☑️ | | | 💡 | -| [icon-validation](docs/rules/icon-validation.md) | Validate node and credential icon files exist, are SVG format, and light/dark icons are different | ✅ ☑️ | | | 💡 | -| [no-credential-reuse](docs/rules/no-credential-reuse.md) | Prevent credential re-use security issues by ensuring nodes only reference credentials from the same package | ✅ ☑️ | | | 💡 | -| [no-deprecated-workflow-functions](docs/rules/no-deprecated-workflow-functions.md) | Disallow usage of deprecated functions and types from n8n-workflow package | ✅ ☑️ | | | 💡 | -| [no-restricted-globals](docs/rules/no-restricted-globals.md) | Disallow usage of restricted global variables in community nodes. | ✅ | | | | -| [no-restricted-imports](docs/rules/no-restricted-imports.md) | Disallow usage of restricted imports in community nodes. | ✅ | | | | -| [node-usable-as-tool](docs/rules/node-usable-as-tool.md) | Ensure node classes have usableAsTool property | ✅ ☑️ | | 🔧 | | -| [package-name-convention](docs/rules/package-name-convention.md) | Enforce correct package naming convention for n8n community nodes | ✅ ☑️ | | | 💡 | -| [resource-operation-pattern](docs/rules/resource-operation-pattern.md) | Enforce proper resource/operation pattern for better UX in n8n nodes | | ✅ ☑️ | | | +| Name                             | Description | 💼 | ⚠️ | 🔧 | 💡 | +| :--------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------ | :--- | :--- | :- | :- | +| [ai-node-package-json](docs/rules/ai-node-package-json.md) | Enforce consistency between n8n.aiNodeSdkVersion and ai-node-sdk peer dependency in community node packages | ✅ ☑️ | | | | +| [credential-documentation-url](docs/rules/credential-documentation-url.md) | Enforce valid credential documentationUrl format (URL or lowercase alphanumeric slug) | ✅ ☑️ | | 🔧 | | +| [credential-password-field](docs/rules/credential-password-field.md) | Ensure credential fields with sensitive names have typeOptions.password = true | ✅ ☑️ | | 🔧 | | +| [credential-test-required](docs/rules/credential-test-required.md) | Ensure credentials have a credential test | ✅ ☑️ | | | 💡 | +| [icon-validation](docs/rules/icon-validation.md) | Validate node and credential icon files exist, are SVG format, and light/dark icons are different | ✅ ☑️ | | | 💡 | +| [no-credential-reuse](docs/rules/no-credential-reuse.md) | Prevent credential re-use security issues by ensuring nodes only reference credentials from the same package | ✅ ☑️ | | | 💡 | +| [no-deprecated-workflow-functions](docs/rules/no-deprecated-workflow-functions.md) | Disallow usage of deprecated functions and types from n8n-workflow package | ✅ ☑️ | | | 💡 | +| [no-http-request-with-manual-auth](docs/rules/no-http-request-with-manual-auth.md) | Disallow this.helpers.httpRequest() in functions that call this.getCredentials(). Use this.helpers.httpRequestWithAuthentication() instead. | ✅ ☑️ | | | | +| [no-restricted-globals](docs/rules/no-restricted-globals.md) | Disallow usage of restricted global variables in community nodes. | ✅ | | | | +| [no-restricted-imports](docs/rules/no-restricted-imports.md) | Disallow usage of restricted imports in community nodes. | ✅ | | | | +| [node-usable-as-tool](docs/rules/node-usable-as-tool.md) | Ensure node classes have usableAsTool property | ✅ ☑️ | | 🔧 | | +| [package-name-convention](docs/rules/package-name-convention.md) | Enforce correct package naming convention for n8n community nodes | ✅ ☑️ | | | 💡 | +| [resource-operation-pattern](docs/rules/resource-operation-pattern.md) | Enforce proper resource/operation pattern for better UX in n8n nodes | | ✅ ☑️ | | | diff --git a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/ai-node-package-json.md b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/ai-node-package-json.md new file mode 100644 index 00000000000..3074d32baf8 --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/ai-node-package-json.md @@ -0,0 +1,62 @@ +# Enforce consistency between n8n.aiNodeSdkVersion and ai-node-sdk peer dependency in community node packages (`@n8n/community-nodes/ai-node-package-json`) + +💼 This rule is enabled in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`. + + + +## Rule Details + +Enforces consistency between `n8n.aiNodeSdkVersion` in `package.json` and the `ai-node-sdk` peer dependency. When a community node uses the AI Node SDK, both fields must be present and correct. + +The rule checks four conditions: + +- `aiNodeSdkVersion` is declared inside the `n8n` section (not at the root level) +- `aiNodeSdkVersion` is a positive integer +- If `n8n.aiNodeSdkVersion` is set, `ai-node-sdk` must appear in `peerDependencies` +- If `ai-node-sdk` is in `peerDependencies`, `n8n.aiNodeSdkVersion` must be set + +## Examples + +### ❌ Incorrect + +```json +{ + "name": "n8n-nodes-my-ai-node", + "n8n": { + "aiNodeSdkVersion": "1" + } +} +``` + +```json +{ + "name": "n8n-nodes-my-ai-node", + "aiNodeSdkVersion": 1, + "peerDependencies": { + "ai-node-sdk": "*" + } +} +``` + +```json +{ + "name": "n8n-nodes-my-ai-node", + "peerDependencies": { + "ai-node-sdk": "*" + } +} +``` + +### ✅ Correct + +```json +{ + "name": "n8n-nodes-my-ai-node", + "n8n": { + "aiNodeSdkVersion": 1 + }, + "peerDependencies": { + "ai-node-sdk": "*" + } +} +``` diff --git a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-documentation-url.md b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-documentation-url.md index c3d08033996..99aa610cbd7 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-documentation-url.md +++ b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-documentation-url.md @@ -2,16 +2,18 @@ 💼 This rule is enabled in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`. +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + ## Options -| Name | Description | Type | -| :----------- | :----------------------------------------------------- | :------ | +| Name | Description | Type | +| :----------- | :--------------------------------------------------------- | :------ | | `allowSlugs` | Whether to allow lowercase alphanumeric slugs with slashes | Boolean | -| `allowUrls` | Whether to allow valid URLs | Boolean | +| `allowUrls` | Whether to allow valid URLs | Boolean | diff --git a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/no-http-request-with-manual-auth.md b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/no-http-request-with-manual-auth.md new file mode 100644 index 00000000000..cbce9cbb27b --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/no-http-request-with-manual-auth.md @@ -0,0 +1,57 @@ +# Disallow this.helpers.httpRequest() in functions that call this.getCredentials(). Use this.helpers.httpRequestWithAuthentication() instead (`@n8n/community-nodes/no-http-request-with-manual-auth`) + +💼 This rule is enabled in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`. + + + +## Rule Details + +When a function calls `this.getCredentials()` to retrieve credentials, it should use `this.helpers.httpRequestWithAuthentication()` for HTTP requests instead of `this.helpers.httpRequest()`. + +Manually extracting credentials and setting auth headers (e.g. `Authorization`) bypasses n8n's authentication layer, which provides: + +- Consistent credential handling across all nodes +- Future improvements like token refresh and audit logging +- Better security review surface + +## Examples + +### ❌ Incorrect + +```typescript +async function apiRequest(this: IExecuteFunctions, endpoint: string) { + const credentials = await this.getCredentials('myServiceApi'); + + const options: IHttpRequestOptions = { + method: 'GET', + url: `https://api.example.com/${endpoint}`, + headers: { + Authorization: `Bearer ${credentials.apiKey}`, + }, + }; + + return this.helpers.httpRequest(options); +} +``` + +### ✅ Correct + +```typescript +async function apiRequest(this: IExecuteFunctions, endpoint: string) { + const options: IHttpRequestOptions = { + method: 'GET', + url: `https://api.example.com/${endpoint}`, + }; + + return this.helpers.httpRequestWithAuthentication.call(this, 'myServiceApi', options); +} +``` + +## When to Disable + +If a function genuinely retrieves credentials for a non-HTTP purpose (e.g. reading a config value) *and* also makes an unauthenticated HTTP request, you can suppress this rule with an inline comment: + +```typescript +// eslint-disable-next-line @n8n/community-nodes/no-http-request-with-manual-auth +return this.helpers.httpRequest(options); +``` diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts b/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts index 24e39e425b8..bb2e88aa6f4 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts @@ -29,6 +29,7 @@ const configs = { '@n8n/community-nodes/package-name-convention': 'error', '@n8n/community-nodes/credential-test-required': 'error', '@n8n/community-nodes/no-credential-reuse': 'error', + '@n8n/community-nodes/no-http-request-with-manual-auth': 'error', '@n8n/community-nodes/icon-validation': 'error', '@n8n/community-nodes/resource-operation-pattern': 'warn', '@n8n/community-nodes/credential-documentation-url': 'error', @@ -47,6 +48,7 @@ const configs = { '@n8n/community-nodes/package-name-convention': 'error', '@n8n/community-nodes/credential-test-required': 'error', '@n8n/community-nodes/no-credential-reuse': 'error', + '@n8n/community-nodes/no-http-request-with-manual-auth': 'error', '@n8n/community-nodes/icon-validation': 'error', '@n8n/community-nodes/credential-documentation-url': 'error', '@n8n/community-nodes/resource-operation-pattern': 'warn', diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts index 115e616516f..241c508bf17 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts @@ -7,6 +7,7 @@ import { CredentialTestRequiredRule } from './credential-test-required.js'; import { IconValidationRule } from './icon-validation.js'; import { NoCredentialReuseRule } from './no-credential-reuse.js'; import { NoDeprecatedWorkflowFunctionsRule } from './no-deprecated-workflow-functions.js'; +import { NoHttpRequestWithManualAuthRule } from './no-http-request-with-manual-auth.js'; import { NoRestrictedGlobalsRule } from './no-restricted-globals.js'; import { NoRestrictedImportsRule } from './no-restricted-imports.js'; import { NodeUsableAsToolRule } from './node-usable-as-tool.js'; @@ -23,6 +24,7 @@ export const rules = { 'package-name-convention': PackageNameConventionRule, 'credential-test-required': CredentialTestRequiredRule, 'no-credential-reuse': NoCredentialReuseRule, + 'no-http-request-with-manual-auth': NoHttpRequestWithManualAuthRule, 'icon-validation': IconValidationRule, 'resource-operation-pattern': ResourceOperationPatternRule, 'credential-documentation-url': CredentialDocumentationUrlRule, diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-deprecated-workflow-functions.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-deprecated-workflow-functions.ts index c67deba2c57..42ed7e785d7 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-deprecated-workflow-functions.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-deprecated-workflow-functions.ts @@ -1,7 +1,6 @@ -import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule } from '../utils/index.js'; +import { createRule, isThisHelpersAccess } from '../utils/index.js'; const DEPRECATED_FUNCTIONS = { request: 'httpRequest', @@ -167,21 +166,6 @@ export const NoDeprecatedWorkflowFunctionsRule = createRule({ }, }); -/** - * Check if the MemberExpression follows the this.helpers.* pattern - */ -function isThisHelpersAccess(node: TSESTree.MemberExpression): boolean { - if (node.object?.type === AST_NODE_TYPES.MemberExpression) { - const outerObject = node.object; - return ( - outerObject.object?.type === AST_NODE_TYPES.ThisExpression && - outerObject.property?.type === AST_NODE_TYPES.Identifier && - outerObject.property.name === 'helpers' - ); - } - return false; -} - function getDeprecationMessage(functionName: string): string { switch (functionName) { case 'request': diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-http-request-with-manual-auth.test.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-http-request-with-manual-auth.test.ts new file mode 100644 index 00000000000..30e38e1e8dd --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-http-request-with-manual-auth.test.ts @@ -0,0 +1,104 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import { NoHttpRequestWithManualAuthRule } from './no-http-request-with-manual-auth.js'; + +const ruleTester = new RuleTester(); + +ruleTester.run('no-http-request-with-manual-auth', NoHttpRequestWithManualAuthRule, { + valid: [ + { + name: 'httpRequest and getCredentials at module top level (no function scope)', + code: ` +const credentials = await this.getCredentials('myApi'); +const result = await this.helpers.httpRequest({ url: 'https://api.example.com' });`, + }, + { + name: 'httpRequest without getCredentials (unauthenticated call)', + code: ` +async function makeRequest() { + return this.helpers.httpRequest({ url: 'https://public.api.com' }); +}`, + }, + { + name: 'httpRequestWithAuthentication with getCredentials (correct pattern)', + code: ` +async function makeRequest() { + const credentials = await this.getCredentials('myApi'); + return this.helpers.httpRequestWithAuthentication.call(this, 'myApi', { + url: 'https://api.example.com', + }); +}`, + }, + { + name: 'getCredentials called but no httpRequest in same function', + code: ` +async function loadConfig() { + const credentials = await this.getCredentials('myApi'); + return credentials.apiKey; +}`, + }, + { + name: 'getCredentials in outer function, httpRequest in nested function (separate scopes)', + code: ` +async function execute() { + const credentials = await this.getCredentials('myApi'); + const makeRequest = async () => { + return this.helpers.httpRequest({ url: 'https://api.example.com' }); + }; + return makeRequest(); +}`, + }, + { + name: 'other object with helpers.httpRequest is not flagged', + code: ` +async function test() { + const credentials = await this.getCredentials('myApi'); + const otherObject = { helpers: { httpRequest: async () => {} } }; + return otherObject.helpers.httpRequest({ url: 'https://example.com' }); +}`, + }, + ], + invalid: [ + { + name: 'function calls both getCredentials and httpRequest', + code: ` +async function makeRequest() { + const credentials = await this.getCredentials('myApi'); + const options = { + headers: { Authorization: \`Bearer \${credentials.apiKey}\` }, + url: 'https://api.example.com', + }; + return this.helpers.httpRequest(options); +}`, + errors: [{ messageId: 'useHttpRequestWithAuthentication' }], + }, + { + name: 'two httpRequest calls in a function that also calls getCredentials — both flagged', + code: ` +async function makeRequests() { + const credentials = await this.getCredentials('myApi'); + const r1 = await this.helpers.httpRequest({ url: 'https://api.example.com/a' }); + const r2 = await this.helpers.httpRequest({ url: 'https://api.example.com/b' }); + return [r1, r2]; +}`, + errors: [ + { messageId: 'useHttpRequestWithAuthentication' }, + { messageId: 'useHttpRequestWithAuthentication' }, + ], + }, + { + name: 'class method pattern', + code: ` +class MyNode { + async execute() { + const credentials = await this.getCredentials('myApi'); + return this.helpers.httpRequest({ + headers: { Authorization: credentials.apiKey }, + url: 'https://api.example.com', + }); + } +}`, + errors: [{ messageId: 'useHttpRequestWithAuthentication' }], + }, + ], +}); diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-http-request-with-manual-auth.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-http-request-with-manual-auth.ts new file mode 100644 index 00000000000..d2e623ede05 --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-http-request-with-manual-auth.ts @@ -0,0 +1,78 @@ +/** + * Flags `this.helpers.httpRequest()` in functions that also call `this.getCredentials()`. + * Those functions should use `this.helpers.httpRequestWithAuthentication()` instead. + * + * Uses a function-scope stack: if both `getCredentials` and `httpRequest` appear in + * the same function body, every `httpRequest` call is reported. Nested functions are + * checked independently. + * + * Alternatives considered: + * - Checking for credential variables in `httpRequest` arguments — misses the common + * pattern where options are built in a separate variable first. + * - Matching auth header names (`Authorization`, etc.) — brittle and requires deep + * AST traversal with no guarantee of coverage. + * + * Known false positive: a function that fetches credentials for a non-HTTP purpose + * and also makes an unauthenticated request. Use eslint-disable to suppress. + */ + +import type { TSESTree } from '@typescript-eslint/utils'; + +import { createRule, isThisHelpersMethodCall, isThisMethodCall } from '../utils/index.js'; + +type FunctionScope = { + getCredentialsCall: TSESTree.CallExpression | null; + httpRequestCalls: TSESTree.CallExpression[]; +}; + +export const NoHttpRequestWithManualAuthRule = createRule({ + name: 'no-http-request-with-manual-auth', + meta: { + type: 'suggestion', + docs: { + description: + 'Disallow this.helpers.httpRequest() in functions that call this.getCredentials(). Use this.helpers.httpRequestWithAuthentication() instead.', + }, + messages: { + useHttpRequestWithAuthentication: + "Avoid calling 'this.helpers.httpRequest()' in a function that retrieves credentials via 'this.getCredentials()'. Use 'this.helpers.httpRequestWithAuthentication()' instead — it handles authentication internally and benefits from future n8n improvements like token refresh and audit logging.", + }, + schema: [], + hasSuggestions: false, + }, + defaultOptions: [], + create(context) { + const scopeStack: FunctionScope[] = []; + + const pushScope = () => scopeStack.push({ getCredentialsCall: null, httpRequestCalls: [] }); + + const popScope = () => { + const scope = scopeStack.pop(); + if (scope?.getCredentialsCall && scope.httpRequestCalls.length > 0) { + for (const call of scope.httpRequestCalls) { + context.report({ node: call, messageId: 'useHttpRequestWithAuthentication' }); + } + } + }; + + return { + FunctionDeclaration: pushScope, + FunctionExpression: pushScope, + ArrowFunctionExpression: pushScope, + 'FunctionDeclaration:exit': popScope, + 'FunctionExpression:exit': popScope, + 'ArrowFunctionExpression:exit': popScope, + + CallExpression(node: TSESTree.CallExpression) { + const scope = scopeStack[scopeStack.length - 1]; + if (!scope) return; + if (isThisMethodCall(node, 'getCredentials')) { + scope.getCredentialsCall = node; + } + if (isThisHelpersMethodCall(node, 'httpRequest')) { + scope.httpRequestCalls.push(node); + } + }, + }; + }, +}); diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts b/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts index d47a81d6e55..70394156ce3 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts @@ -197,6 +197,36 @@ export function extractCredentialNameFromArray( return info ? { name: info.name, node: info.node } : null; } +/** Matches the `this.helpers` MemberExpression (the object part of `this.helpers.foo`). */ +export function isThisHelpersAccess(node: TSESTree.MemberExpression): boolean { + return ( + node.object?.type === AST_NODE_TYPES.MemberExpression && + node.object.object?.type === AST_NODE_TYPES.ThisExpression && + node.object.property?.type === AST_NODE_TYPES.Identifier && + node.object.property.name === 'helpers' + ); +} + +/** Matches a call expression of the form `this.methodName(...)`. */ +export function isThisMethodCall(node: TSESTree.CallExpression, method: string): boolean { + return ( + node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.ThisExpression && + node.callee.property.type === AST_NODE_TYPES.Identifier && + node.callee.property.name === method + ); +} + +/** Matches a call expression of the form `this.helpers.methodName(...)`. */ +export function isThisHelpersMethodCall(node: TSESTree.CallExpression, method: string): boolean { + return ( + node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.property.type === AST_NODE_TYPES.Identifier && + node.callee.property.name === method && + isThisHelpersAccess(node.callee) + ); +} + export function findSimilarStrings( target: string, candidates: Set,