diff --git a/packages/@n8n/eslint-plugin-community-nodes/README.md b/packages/@n8n/eslint-plugin-community-nodes/README.md index 1ce8f1e0cdc..939fb1c43ab 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/README.md +++ b/packages/@n8n/eslint-plugin-community-nodes/README.md @@ -50,8 +50,9 @@ export default [ | [cred-class-field-icon-missing](docs/rules/cred-class-field-icon-missing.md) | Credential class must have an `icon` property defined | ✅ ☑️ | | | 💡 | | | [cred-class-oauth2-naming](docs/rules/cred-class-oauth2-naming.md) | OAuth2 credentials must include `OAuth2` in the class name, `name`, and `displayName` | ✅ ☑️ | | 🔧 | | | | [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-password-field](docs/rules/credential-password-field.md) | Ensure fields with sensitive names have typeOptions.password = true | ✅ ☑️ | | 🔧 | | | | [credential-test-required](docs/rules/credential-test-required.md) | Ensure credentials have a credential test | ✅ ☑️ | | | 💡 | | +| [credential-unnecessary-password](docs/rules/credential-unnecessary-password.md) | Warn when a credential field with no sensitive name uses typeOptions.password = true | | ✅ ☑️ | | | | | [icon-prefer-themed-variants](docs/rules/icon-prefer-themed-variants.md) | Encourage node and credential icons to provide light/dark variants instead of a single icon file | | ✅ ☑️ | | | | | [icon-validation](docs/rules/icon-validation.md) | Validate node and credential icon files exist, use the file: protocol, and that light/dark icons are different | ✅ ☑️ | | | 💡 | | | [missing-paired-item](docs/rules/missing-paired-item.md) | Require pairedItem on INodeExecutionData objects in execute() methods to preserve item linking. | ✅ ☑️ | | | | | diff --git a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-password-field.md b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-password-field.md index bb0cdca5139..15abbca3e3d 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-password-field.md +++ b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-password-field.md @@ -1,4 +1,4 @@ -# Ensure credential fields with sensitive names have typeOptions.password = true (`@n8n/community-nodes/credential-password-field`) +# Ensure fields with sensitive names have typeOptions.password = true (`@n8n/community-nodes/credential-password-field`) 💼 This rule is enabled in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`. @@ -10,6 +10,8 @@ Ensures that credential fields with names like "password", "secret", "token", or "key" are properly masked in the UI by having `typeOptions.password = true`. +The inverse case — non-sensitive fields that should **not** be masked — is covered by the separate [`credential-unnecessary-password`](credential-unnecessary-password.md) rule. + ## Examples ### ❌ Incorrect diff --git a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-unnecessary-password.md b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-unnecessary-password.md new file mode 100644 index 00000000000..9b6a02f9294 --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/credential-unnecessary-password.md @@ -0,0 +1,46 @@ +# Warn when a credential field with no sensitive name uses typeOptions.password = true (`@n8n/community-nodes/credential-unnecessary-password`) + +⚠️ This rule _warns_ in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`. + + + +## Rule Details + +This is the inverse of [`credential-password-field`](credential-password-field.md). A field that clearly does not hold a secret — URLs, IDs, regions, and similar — should not be masked with `typeOptions.password = true`, since masking a non-secret value only hurts usability. + +The check only flags a masked field when its name carries **no** sensitive marker at all (`password`, `secret`, `token`, `cert`, `key`-like words, …). A name that contains a sensitive marker is left alone even if it also looks like a URL or ID — for example `androidToken` is a real secret despite containing the substring `id`, so it is never flagged. + +It is reported as a **warning without an autofix**: the sensitivity check is a name-based heuristic, so removing masking could be wrong for a secret whose name isn't in the recognised list. The author must confirm before removing it. + +## Examples + +### ❌ Incorrect + +```typescript +export class MyApiCredential implements ICredentialType { + properties: INodeProperties[] = [ + { + displayName: 'Base URL', + name: 'baseUrl', + type: 'string', + default: '', + typeOptions: { password: true }, // A URL is not a secret + }, + ]; +} +``` + +### ✅ Correct + +```typescript +export class MyApiCredential implements ICredentialType { + properties: INodeProperties[] = [ + { + displayName: 'Base URL', + name: 'baseUrl', + type: 'string', + default: '', + }, + ]; +} +``` diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts b/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts index a1d8ffc53dc..51904175e62 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts @@ -24,6 +24,7 @@ const configs = { '@n8n/community-nodes/no-restricted-globals': 'error', '@n8n/community-nodes/no-restricted-imports': 'error', '@n8n/community-nodes/credential-password-field': 'error', + '@n8n/community-nodes/credential-unnecessary-password': 'warn', '@n8n/community-nodes/n8n-object-validation': 'error', '@n8n/community-nodes/no-deprecated-workflow-functions': 'error', '@n8n/community-nodes/no-emoji-in-options': 'error', @@ -67,6 +68,7 @@ const configs = { rules: { '@n8n/community-nodes/ai-node-package-json': 'error', '@n8n/community-nodes/credential-password-field': 'error', + '@n8n/community-nodes/credential-unnecessary-password': 'warn', '@n8n/community-nodes/n8n-object-validation': 'error', '@n8n/community-nodes/no-deprecated-workflow-functions': 'error', '@n8n/community-nodes/no-emoji-in-options': 'error', diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-password-field.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-password-field.ts index ad4aefe4f0c..739756d2ba4 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-password-field.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-password-field.ts @@ -6,47 +6,11 @@ import { findClassProperty, findObjectProperty, getStringLiteralValue, - getBooleanLiteralValue, + isSensitiveFieldName, + hasPasswordTypeOption, createRule, } from '../utils/index.js'; -const SENSITIVE_PATTERNS = [ - 'password', - 'secret', - 'token', - 'cert', - 'passphrase', - 'apikey', - 'secretkey', - 'privatekey', - 'authkey', -]; - -const NON_SENSITIVE_PATTERNS = ['url', 'pub', 'id']; - -function isSensitiveFieldName(name: string): boolean { - const lowerName = name.toLowerCase(); - - if (NON_SENSITIVE_PATTERNS.some((pattern) => lowerName.includes(pattern))) { - return false; - } - - return SENSITIVE_PATTERNS.some((pattern) => lowerName.includes(pattern)); -} - -function hasPasswordTypeOption(element: TSESTree.ObjectExpression): boolean { - const typeOptionsProperty = findObjectProperty(element, 'typeOptions'); - - if (typeOptionsProperty?.value.type !== TSESTree.AST_NODE_TYPES.ObjectExpression) { - return false; - } - - const passwordProperty = findObjectProperty(typeOptionsProperty.value, 'password'); - const passwordValue = passwordProperty ? getBooleanLiteralValue(passwordProperty.value) : null; - - return passwordValue === true; -} - function createPasswordFix( element: TSESTree.ObjectExpression, typeOptionsProperty: TSESTree.Property | null, @@ -87,7 +51,7 @@ export const CredentialPasswordFieldRule = createRule({ meta: { type: 'problem', docs: { - description: 'Ensure credential fields with sensitive names have typeOptions.password = true', + description: 'Ensure fields with sensitive names have typeOptions.password = true', }, messages: { missingPasswordOption: diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-unnecessary-password.test.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-unnecessary-password.test.ts new file mode 100644 index 00000000000..54ac0ab9ed6 --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-unnecessary-password.test.ts @@ -0,0 +1,105 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import { CredentialUnnecessaryPasswordRule } from './credential-unnecessary-password.js'; + +const ruleTester = new RuleTester(); + +function createCredentialCode(properties: string[]): string { + return ` +import type { ICredentialType, INodeProperties } from 'n8n-workflow'; + +export class TestCredential implements ICredentialType { + name = 'testApi'; + displayName = 'Test API'; + + properties: INodeProperties[] = [ +${properties.map((prop) => `\t\t${prop}`).join(',\n')}, + ]; +}`; +} + +function createProperty( + displayName: string, + name: string, + options: { password?: boolean } = {}, +): string { + const typeOptionsStr = + options.password !== undefined ? `\n\t\t\ttypeOptions: { password: ${options.password} },` : ''; + + return `{ + displayName: '${displayName}', + name: '${name}', + type: 'string', + default: '',${typeOptionsStr} + }`; +} + +function createRegularClass(): string { + return ` +export class RegularClass { + properties = [ + { + name: 'region', + type: 'string', + typeOptions: { password: true }, + }, + ]; +}`; +} + +ruleTester.run('credential-unnecessary-password', CredentialUnnecessaryPasswordRule, { + valid: [ + { + name: 'non-sensitive field without password option', + code: createCredentialCode([createProperty('Region', 'region')]), + }, + { + name: 'non-sensitive field with password: false', + code: createCredentialCode([createProperty('Base URL', 'baseUrl', { password: false })]), + }, + { + name: 'sensitive field correctly masked', + code: createCredentialCode([createProperty('API Key', 'apiKey', { password: true })]), + }, + { + name: 'masked field with a sensitive marker is left alone even when its name contains a non-sensitive substring', + code: createCredentialCode([ + // 'androidToken' contains the non-sensitive substring 'id' but is a real secret + createProperty('Android Token', 'androidToken', { password: true }), + createProperty('Token URL', 'tokenUrl', { password: true }), + ]), + }, + { + name: 'class does not implement ICredentialType', + code: createRegularClass(), + }, + ], + invalid: [ + { + name: 'region field should not use password option', + code: createCredentialCode([createProperty('Region', 'region', { password: true })]), + errors: [{ messageId: 'unnecessaryPasswordOption', data: { fieldName: 'region' } }], + }, + { + name: 'URL field should not use password option', + code: createCredentialCode([createProperty('Base URL', 'baseUrl', { password: true })]), + errors: [{ messageId: 'unnecessaryPasswordOption', data: { fieldName: 'baseUrl' } }], + }, + { + name: 'ID field should not use password option', + code: createCredentialCode([createProperty('Client ID', 'clientId', { password: true })]), + errors: [{ messageId: 'unnecessaryPasswordOption', data: { fieldName: 'clientId' } }], + }, + { + name: 'multiple non-sensitive masked fields are each flagged', + code: createCredentialCode([ + createProperty('Region', 'region', { password: true }), + createProperty('Host', 'host', { password: true }), + ]), + errors: [ + { messageId: 'unnecessaryPasswordOption', data: { fieldName: 'region' } }, + { messageId: 'unnecessaryPasswordOption', data: { fieldName: 'host' } }, + ], + }, + ], +}); diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-unnecessary-password.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-unnecessary-password.ts new file mode 100644 index 00000000000..233915cec0b --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/credential-unnecessary-password.ts @@ -0,0 +1,72 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { + isCredentialTypeClass, + findClassProperty, + findObjectProperty, + getStringLiteralValue, + containsSensitivePattern, + hasPasswordTypeOption, + createRule, +} from '../utils/index.js'; + +export const CredentialUnnecessaryPasswordRule = createRule({ + name: 'credential-unnecessary-password', + meta: { + type: 'suggestion', + docs: { + description: + 'Warn when a credential field with no sensitive name uses typeOptions.password = true', + }, + messages: { + unnecessaryPasswordOption: + "Field '{{ fieldName }}' does not appear to be sensitive but uses 'typeOptions: { password: true }'. Remove it unless the field holds a secret", + }, + schema: [], + }, + defaultOptions: [], + create(context) { + return { + ClassDeclaration(node) { + if (!isCredentialTypeClass(node)) { + return; + } + + const propertiesProperty = findClassProperty(node, 'properties'); + if ( + !propertiesProperty?.value || + propertiesProperty.value.type !== TSESTree.AST_NODE_TYPES.ArrayExpression + ) { + return; + } + + for (const element of propertiesProperty.value.elements) { + if (element?.type !== TSESTree.AST_NODE_TYPES.ObjectExpression) { + continue; + } + + const nameProperty = findObjectProperty(element, 'name'); + const fieldName = nameProperty ? getStringLiteralValue(nameProperty.value) : null; + + if (!fieldName || !hasPasswordTypeOption(element)) { + continue; + } + + // Only flag a masked field when its name carries no sensitive marker at all. + // Requiring the *absence* of any sensitive pattern (rather than reusing + // isSensitiveFieldName) avoids flagging a real secret like `androidToken`, + // which is classified non-sensitive only because it contains the substring + // `id`. Reported without a fix: removing masking could still be wrong for an + // unlisted secret name, so the author confirms. + if (!containsSensitivePattern(fieldName)) { + context.report({ + node: element, + messageId: 'unnecessaryPasswordOption', + data: { fieldName }, + }); + } + } + }, + }; + }, +}); 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 4432af214fb..93b22638902 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts @@ -6,6 +6,7 @@ import { CredClassOAuth2NamingRule } from './cred-class-oauth2-naming.js'; import { CredentialDocumentationUrlRule } from './credential-documentation-url.js'; import { CredentialPasswordFieldRule } from './credential-password-field.js'; import { CredentialTestRequiredRule } from './credential-test-required.js'; +import { CredentialUnnecessaryPasswordRule } from './credential-unnecessary-password.js'; import { IconPreferThemedVariantsRule } from './icon-prefer-themed-variants.js'; import { IconValidationRule } from './icon-validation.js'; import { MissingPairedItemRule } from './missing-paired-item.js'; @@ -49,6 +50,7 @@ export const rules = { 'node-usable-as-tool': NodeUsableAsToolRule, 'package-name-convention': PackageNameConventionRule, 'credential-test-required': CredentialTestRequiredRule, + 'credential-unnecessary-password': CredentialUnnecessaryPasswordRule, 'no-credential-reuse': NoCredentialReuseRule, 'no-dangerous-functions': NoDangerousFunctionsRule, 'no-forbidden-lifecycle-scripts': NoForbiddenLifecycleScriptsRule, diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/utils/credential-fields.ts b/packages/@n8n/eslint-plugin-community-nodes/src/utils/credential-fields.ts new file mode 100644 index 00000000000..648d9a4f9a0 --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/utils/credential-fields.ts @@ -0,0 +1,52 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { findObjectProperty, getBooleanLiteralValue } from './ast-utils.js'; + +const SENSITIVE_PATTERNS = [ + 'password', + 'secret', + 'token', + 'cert', + 'passphrase', + 'apikey', + 'secretkey', + 'privatekey', + 'authkey', +]; + +const NON_SENSITIVE_PATTERNS = ['url', 'pub', 'id']; + +/** Whether the field name contains any marker that suggests it may hold a secret. */ +export function containsSensitivePattern(name: string): boolean { + const lowerName = name.toLowerCase(); + return SENSITIVE_PATTERNS.some((pattern) => lowerName.includes(pattern)); +} + +/** + * Whether the field name should be treated as sensitive (and therefore masked). + * A name that contains a non-sensitive marker (URL, ID, public) is never treated + * as sensitive, even if it also contains a sensitive one. + */ +export function isSensitiveFieldName(name: string): boolean { + const lowerName = name.toLowerCase(); + + if (NON_SENSITIVE_PATTERNS.some((pattern) => lowerName.includes(pattern))) { + return false; + } + + return containsSensitivePattern(name); +} + +/** Whether the credential field element sets `typeOptions: { password: true }`. */ +export function hasPasswordTypeOption(element: TSESTree.ObjectExpression): boolean { + const typeOptionsProperty = findObjectProperty(element, 'typeOptions'); + + if (typeOptionsProperty?.value.type !== TSESTree.AST_NODE_TYPES.ObjectExpression) { + return false; + } + + const passwordProperty = findObjectProperty(typeOptionsProperty.value, 'password'); + const passwordValue = passwordProperty ? getBooleanLiteralValue(passwordProperty.value) : null; + + return passwordValue === true; +} diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/utils/index.ts b/packages/@n8n/eslint-plugin-community-nodes/src/utils/index.ts index 147b4e4f090..e088ea0bb82 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/utils/index.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/utils/index.ts @@ -1,4 +1,5 @@ export * from './ast-utils.js'; +export * from './credential-fields.js'; export * from './constants.js'; export * from './file-utils.js'; export * from './rule-creator.js';