feat: Add lint rule to disallow httpRequest with manual authentication (#26624)

2026-08-29 01:39:24 +08:00 · 2026-03-06 10:02:27 +01:00
parent faf2267ab7
commit 82eae73d8a
10 changed files with 356 additions and 33 deletions
@@ -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                                                                        |      | ✅ ☑️ |    |    |

 <!-- end auto-generated rules list -->
@@ -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`.
+
+<!-- end auto-generated rule header -->
+
+## 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": "*"
+  }
+}
+```
@@ -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).
+
 <!-- end auto-generated rule header -->

 ## Options

 <!-- begin auto-generated rule options list -->

-| 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 |

 <!-- end auto-generated rule options list -->

@@ -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`.
+
+<!-- end auto-generated rule header -->
+
+## 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);
+```
@@ -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',
@@ -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,
@@ -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':
@@ -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' }],
+		},
+	],
+});
@@ -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);
+				}
+			},
+		};
+	},
+});
@@ -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<string>,