From 872b145ea8c2ed61d7a817b03decae4849059407 Mon Sep 17 00:00:00 2001 From: Matsu Date: Tue, 22 Sep 2026 08:08:02 +0300 Subject: [PATCH] refactor(core): Internalize cipher primitives (no-changelog) (#39148) --- AGENTS.md | 5 +- OWNERS | 2 - .../src/configs/encryption-boundary.ts | 8 - .../src/configs/functional-guardrails.ts | 2 - .../src/rules/fixtures/tsconfig.json | 2 +- .../@n8n/eslint-config/src/rules/index.ts | 4 - .../no-encryption-guardrail-disable.test.ts | 16 +- .../rules/no-encryption-guardrail-disable.ts | 2 - .../rules/no-legacy-cipher-methods.test.ts | 109 ------------ .../src/rules/no-legacy-cipher-methods.ts | 59 ------- .../no-misplaced-cipher-primitives.test.ts | 165 ------------------ .../rules/no-misplaced-cipher-primitives.ts | 121 ------------- .../src/__tests__/credentials-helper.test.ts | 41 ++--- .../__tests__/key-manager.service.test.ts | 15 +- ...namic-credentials-cors.integration.test.ts | 8 +- ...credentials-rate-limit.integration.test.ts | 4 +- .../source-control-export.service.test.ts | 12 +- .../controllers/oauth/oauth2.api.test.ts | 2 +- ...credentials-overwrites.integration.test.ts | 18 +- ...ets-provider-connection.repository.test.ts | 4 +- .../source-control-export.service.test.ts | 16 +- .../source-control.service.test.ts | 22 +-- .../external-secrets.events.test.ts | 4 +- .../external-secrets.module.test.ts | 4 +- .../secret-providers-connections.api.test.ts | 8 +- .../secret-providers-project.api.test.ts | 2 +- .../test/integration/ldap/ldap.api.test.ts | 2 +- ...external-secrets-to-entity-storage.test.ts | 20 +-- ...create-credential-dependency-table.test.ts | 10 +- packages/cli/test/shared/mocking.ts | 2 - .../core/src/__tests__/credentials.test.ts | 12 +- .../src/encryption/__tests__/cipher.test.ts | 124 ++++++------- packages/core/src/encryption/cipher.ts | 21 +-- packages/core/src/encryption/index.ts | 2 - .../rules/encryption-boundary.rule.test.ts | 33 ++-- .../src/rules/encryption-boundary.rule.ts | 7 +- scripts/lint-parity/expected-pr1.json | 56 ------ scripts/lint-parity/oxlint-gap.json | 14 -- 38 files changed, 167 insertions(+), 791 deletions(-) delete mode 100644 packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.test.ts delete mode 100644 packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.ts delete mode 100644 packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.test.ts delete mode 100644 packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.ts diff --git a/AGENTS.md b/AGENTS.md index 6e8db4f6e42..063f527a717 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -304,9 +304,8 @@ which output format. Enforced in CI by the rules in `backendConfig`, and so of `nodesConfig`; every package that runs on Node extends one of those layers): -- The deprecated `Cipher.encrypt` / `Cipher.decrypt` are banned outside tests. -- The raw AES classes and `encryptWithKey` / `decryptWithKey` stay inside - `packages/core/src/encryption/` and database migrations. +- `Cipher` does not expose the legacy or explicit-key methods. +- The raw AES classes stay inside `packages/core/src/encryption/`. - **Deployment keys are never deleted** — data encrypted with a key becomes unreadable without it. Deactivate keys instead; the repository's delete surface throws at runtime and the lint rule rejects call sites. diff --git a/OWNERS b/OWNERS index 7808cd4ac4d..f4838154099 100644 --- a/OWNERS +++ b/OWNERS @@ -266,7 +266,5 @@ packages/cli/src/modules/encryption-key-manager/ @n8n-io/iam required packages/@n8n/db/src/entities/deployment-key.ts @n8n-io/iam required packages/@n8n/db/src/repositories/deployment-key.repository.ts @n8n-io/iam required packages/@n8n/eslint-config/src/configs/encryption-boundary.ts @n8n-io/iam required -packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.ts @n8n-io/iam required -packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.ts @n8n-io/iam required packages/@n8n/eslint-config/src/rules/no-deployment-key-delete.ts @n8n-io/iam required packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.ts @n8n-io/iam required diff --git a/packages/@n8n/eslint-config/src/configs/encryption-boundary.ts b/packages/@n8n/eslint-config/src/configs/encryption-boundary.ts index fd140a625b9..10ce26ae416 100644 --- a/packages/@n8n/eslint-config/src/configs/encryption-boundary.ts +++ b/packages/@n8n/eslint-config/src/configs/encryption-boundary.ts @@ -11,12 +11,6 @@ import tseslint from 'typescript-eslint'; * `encryption-boundary` verifies that coverage in CI and rejects package-level * downgrades and inline directives that would silence these rules: * - * - `no-legacy-cipher-methods`: the deprecated `Cipher.encrypt` / `decrypt` - * always take the legacy instance-key AES-256-CBC path, which key rotation - * cannot manage. - * - `no-misplaced-cipher-primitives`: the raw AES classes and the explicit - * `encryptWithKey` / `decryptWithKey` stay inside the encryption area and - * database migrations. * - `no-deployment-key-delete`: data encrypted with a key becomes unreadable * without it — keys are deactivated, never deleted. * - `no-encryption-guardrail-disable`: in-editor feedback that the rules @@ -32,8 +26,6 @@ import tseslint from 'typescript-eslint'; */ export const encryptionBoundaryConfig = tseslint.config({ rules: { - 'n8n-local-rules/no-legacy-cipher-methods': 'error', - 'n8n-local-rules/no-misplaced-cipher-primitives': 'error', 'n8n-local-rules/no-deployment-key-delete': 'error', 'n8n-local-rules/no-encryption-guardrail-disable': 'error', }, diff --git a/packages/@n8n/eslint-config/src/configs/functional-guardrails.ts b/packages/@n8n/eslint-config/src/configs/functional-guardrails.ts index 849e9e86f30..a797c9c2ca1 100644 --- a/packages/@n8n/eslint-config/src/configs/functional-guardrails.ts +++ b/packages/@n8n/eslint-config/src/configs/functional-guardrails.ts @@ -13,8 +13,6 @@ export const functionalGuardrailsConfig = tseslint.config({ parserOptions: { projectService: true }, }, rules: { - 'n8n-local-rules/no-legacy-cipher-methods': 'error', - 'n8n-local-rules/no-misplaced-cipher-primitives': 'error', 'n8n-local-rules/no-deployment-key-delete': 'error', 'n8n-local-rules/no-unsealed-workflow-entity-write': 'error', 'n8n-local-rules/no-encryption-guardrail-disable': 'error', diff --git a/packages/@n8n/eslint-config/src/rules/fixtures/tsconfig.json b/packages/@n8n/eslint-config/src/rules/fixtures/tsconfig.json index 869f943fa0f..650299b0f06 100644 --- a/packages/@n8n/eslint-config/src/rules/fixtures/tsconfig.json +++ b/packages/@n8n/eslint-config/src/rules/fixtures/tsconfig.json @@ -1,3 +1,3 @@ { - "//": "Anchor for the project service in typed rule tests (see no-legacy-cipher-methods.test.ts). Virtual RuleTester files resolve against this project instead of the package's own tsconfig." + "//": "Anchor for the project service in typed rule tests. Virtual RuleTester files resolve against this project instead of the package's own tsconfig." } diff --git a/packages/@n8n/eslint-config/src/rules/index.ts b/packages/@n8n/eslint-config/src/rules/index.ts index dee2c80c038..c05c3f03984 100644 --- a/packages/@n8n/eslint-config/src/rules/index.ts +++ b/packages/@n8n/eslint-config/src/rules/index.ts @@ -27,11 +27,9 @@ import { NoRestrictedSleepDefinitionRule } from './no-restricted-sleep-definitio import { NoRestrictedSleepImportRule } from './no-restricted-sleep-import.js'; import { NoRepositoryInPublicApiHandlerRule } from './no-repository-in-public-api-handler.js'; import { RequirePublicApiControllerRule } from './require-public-api-controller.js'; -import { NoLegacyCipherMethodsRule } from './no-legacy-cipher-methods.js'; import { NoUnsealedWorkflowEntityWriteRule } from './no-unsealed-workflow-entity-write.js'; import { RequireEscapedQueryValuesRule } from './require-escaped-query-values.js'; import { NoOnLeaderTakeoverRule } from './no-on-leader-takeover.js'; -import { NoMisplacedCipherPrimitivesRule } from './no-misplaced-cipher-primitives.js'; import { NoDeploymentKeyDeleteRule } from './no-deployment-key-delete.js'; import { NoEncryptionGuardrailDisableRule } from './no-encryption-guardrail-disable.js'; @@ -64,11 +62,9 @@ export const rules = { 'no-restricted-sleep-import': NoRestrictedSleepImportRule, 'no-repository-in-public-api-handler': NoRepositoryInPublicApiHandlerRule, 'require-public-api-controller': RequirePublicApiControllerRule, - 'no-legacy-cipher-methods': NoLegacyCipherMethodsRule, 'no-unsealed-workflow-entity-write': NoUnsealedWorkflowEntityWriteRule, 'require-escaped-query-values': RequireEscapedQueryValuesRule, 'no-on-leader-takeover': NoOnLeaderTakeoverRule, - 'no-misplaced-cipher-primitives': NoMisplacedCipherPrimitivesRule, 'no-deployment-key-delete': NoDeploymentKeyDeleteRule, 'no-encryption-guardrail-disable': NoEncryptionGuardrailDisableRule, } satisfies Record; diff --git a/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.test.ts b/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.test.ts index a40de2df9fd..406be69276f 100644 --- a/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.test.ts +++ b/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.test.ts @@ -1,8 +1,6 @@ import { RuleTester } from '@typescript-eslint/rule-tester'; import { NoDeploymentKeyDeleteRule } from './no-deployment-key-delete.js'; import { NoEncryptionGuardrailDisableRule } from './no-encryption-guardrail-disable.js'; -import { NoLegacyCipherMethodsRule } from './no-legacy-cipher-methods.js'; -import { NoMisplacedCipherPrimitivesRule } from './no-misplaced-cipher-primitives.js'; // Register the guarded rules so ESLint can resolve the disable directives under // test (an unknown rule name in a directive would emit an extra "rule not found" @@ -14,8 +12,6 @@ const ruleTester = new RuleTester({ plugins: { 'n8n-local-rules': { rules: { - 'no-legacy-cipher-methods': NoLegacyCipherMethodsRule, - 'no-misplaced-cipher-primitives': NoMisplacedCipherPrimitivesRule, 'no-deployment-key-delete': NoDeploymentKeyDeleteRule, 'no-encryption-guardrail-disable': NoEncryptionGuardrailDisableRule, }, @@ -27,7 +23,7 @@ ruleTester.run('no-encryption-guardrail-disable', NoEncryptionGuardrailDisableRu valid: [ // A plain comment mentioning a rule name is not a disable directive { - code: '// enforced by n8n-local-rules/no-legacy-cipher-methods\nconst a = 1;', + code: '// enforced by n8n-local-rules/no-deployment-key-delete\nconst a = 1;', }, // Disabling unrelated rules is fine { @@ -35,22 +31,14 @@ ruleTester.run('no-encryption-guardrail-disable', NoEncryptionGuardrailDisableRu }, // A guarded rule mentioned only in the `--` explanation is not disabled { - code: '// eslint-disable-next-line no-console -- keep parity with no-legacy-cipher-methods docs\nconsole.log("x");', + code: '// eslint-disable-next-line no-console -- keep parity with no-deployment-key-delete docs\nconsole.log("x");', }, ], invalid: [ - { - code: '// eslint-disable-next-line n8n-local-rules/no-legacy-cipher-methods\nconst a = 1;', - errors: [{ messageId: 'noDisable', data: { rule: 'no-legacy-cipher-methods' } }], - }, { code: '/* eslint-disable n8n-local-rules/no-deployment-key-delete */\nconst a = 1;', errors: [{ messageId: 'noDisable', data: { rule: 'no-deployment-key-delete' } }], }, - { - code: '// eslint-disable-line n8n-local-rules/no-misplaced-cipher-primitives', - errors: [{ messageId: 'noDisable', data: { rule: 'no-misplaced-cipher-primitives' } }], - }, // Blanket line-form disables silence every rule on the target line { code: '// eslint-disable-next-line\nconst a = 1;', diff --git a/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.ts b/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.ts index a6d05ab48d5..d440328aab4 100644 --- a/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.ts +++ b/packages/@n8n/eslint-config/src/rules/no-encryption-guardrail-disable.ts @@ -1,8 +1,6 @@ import { ESLintUtils } from '@typescript-eslint/utils'; const TARGET_RULES = [ - 'no-legacy-cipher-methods', - 'no-misplaced-cipher-primitives', 'no-deployment-key-delete', // A `-next-line` directive naming this rule cannot suppress the report // this rule emits at the directive's own line. diff --git a/packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.test.ts b/packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.test.ts deleted file mode 100644 index a4f1a6021d7..00000000000 --- a/packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.test.ts +++ /dev/null @@ -1,109 +0,0 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; -import path from 'node:path'; -import { NoLegacyCipherMethodsRule } from './no-legacy-cipher-methods.js'; - -const ruleTester = new RuleTester({ - languageOptions: { - parserOptions: { - projectService: { - allowDefaultProject: ['*.ts'], - }, - tsconfigRootDir: path.join(import.meta.dirname, 'fixtures'), - }, - }, -}); - -/** Minimal stand-in mirroring the shape of `Cipher` from n8n-core. */ -const cipherClass = ` -class Cipher { - encrypt(data: string | object, customEncryptionKey?: string): string { return ''; } - decrypt(data: string, customEncryptionKey?: string): string { return ''; } - async encryptV2(data: string | object): Promise { return ''; } - async decryptV2(data: string): Promise { return ''; } - encryptWithKey(data: string, key: string): string { return ''; } - decryptWithKey(data: string, key: string): string { return ''; } -} -`; - -ruleTester.run('no-legacy-cipher-methods', NoLegacyCipherMethodsRule, { - valid: [ - // The V2 and explicit-key methods are the sanctioned paths - { - code: `${cipherClass} -declare const cipher: Cipher; -void cipher.encryptV2('data'); -void cipher.decryptV2('data'); -cipher.encryptWithKey('data', 'key'); -cipher.decryptWithKey('data', 'key');`, - filename: 'service.ts', - }, - // encrypt/decrypt on unrelated types stay allowed (e.g. CredentialsService.decrypt) - { - code: `class CredentialsService { - async decrypt(credential: object, full = false): Promise { return {}; } -} -declare const credentialsService: CredentialsService; -void credentialsService.decrypt({}, true);`, - filename: 'service.ts', - }, - // Test files are exempt: they cover reading the legacy CBC format - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher.encrypt('data'); -cipher.decrypt('data');`, - filename: 'cipher.test.ts', - }, - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher.encrypt('data');`, - filename: `${path.sep}repo${path.sep}packages${path.sep}cli${path.sep}test${path.sep}integration${path.sep}setup.ts`, - // The file-name exemption fires before the rule requests type - // services, so this out-of-project path must parse untyped. - languageOptions: { parserOptions: { projectService: false } }, - }, - ], - invalid: [ - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher.encrypt('data');`, - filename: 'service.ts', - errors: [{ messageId: 'noLegacyCipherMethods', data: { method: 'encrypt' } }], - }, - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher.decrypt('data');`, - filename: 'service.ts', - errors: [{ messageId: 'noLegacyCipherMethods', data: { method: 'decrypt' } }], - }, - // Receiver resolved through a property, as in DI (`this.cipher`) - { - code: `${cipherClass} -class MyService { - constructor(private readonly cipher: Cipher) {} - run() { return this.cipher.encrypt('data'); } -}`, - filename: 'service.ts', - errors: [{ messageId: 'noLegacyCipherMethods', data: { method: 'encrypt' } }], - }, - // Computed access with a string literal must not slip through - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher['decrypt']('data');`, - filename: 'service.ts', - errors: [{ messageId: 'noLegacyCipherMethods', data: { method: 'decrypt' } }], - }, - // Optional chaining must not slip through - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher?.encrypt('data');`, - filename: 'service.ts', - errors: [{ messageId: 'noLegacyCipherMethods', data: { method: 'encrypt' } }], - }, - ], -}); diff --git a/packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.ts b/packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.ts deleted file mode 100644 index 915ee88b44f..00000000000 --- a/packages/@n8n/eslint-config/src/rules/no-legacy-cipher-methods.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/utils'; - -const BANNED_METHODS = new Set(['encrypt', 'decrypt']); - -/** - * Test files may keep calling the legacy methods: they intentionally produce - * and read the legacy CBC format to cover read-support for existing data. - */ -const TEST_FILE_PATTERN = /(\.(test|spec)\.ts$)|([\\/]__tests__[\\/])|([\\/]test[\\/])/; - -export const NoLegacyCipherMethodsRule = ESLintUtils.RuleCreator.withoutDocs({ - meta: { - type: 'problem', - docs: { - description: - 'Disallow calling the deprecated `Cipher.encrypt` / `Cipher.decrypt`. They always use the legacy instance-key AES-256-CBC path and bypass the encryption key store, so new call sites would write data that key rotation cannot manage.', - }, - messages: { - noLegacyCipherMethods: - '`Cipher.{{ method }}()` is deprecated: it always uses the legacy instance-key AES-256-CBC path and bypasses the encryption key store. Use `await cipher.{{ method }}V2()` instead, or `{{ method }}WithKey()` when an explicit key and algorithm are required.', - }, - schema: [], - }, - defaultOptions: [], - create(context) { - if (TEST_FILE_PATTERN.test(context.filename)) return {}; - - return { - CallExpression(node) { - const callee = - node.callee.type === TSESTree.AST_NODE_TYPES.ChainExpression - ? node.callee.expression - : node.callee; - if (callee.type !== TSESTree.AST_NODE_TYPES.MemberExpression) return; - - const { property } = callee; - let method: string | null = null; - if (!callee.computed && property.type === TSESTree.AST_NODE_TYPES.Identifier) { - method = property.name; - } else if ( - callee.computed && - property.type === TSESTree.AST_NODE_TYPES.Literal && - typeof property.value === 'string' - ) { - method = property.value; - } - if (method === null || !BANNED_METHODS.has(method)) return; - - // Cheap syntactic prefilter passed — only now consult the type checker. - const services = ESLintUtils.getParserServices(context); - const receiverType = services.getTypeAtLocation(callee.object); - const symbolName = (receiverType.getSymbol() ?? receiverType.aliasSymbol)?.getName(); - if (symbolName !== 'Cipher') return; - - context.report({ node: property, messageId: 'noLegacyCipherMethods', data: { method } }); - }, - }; - }, -}); diff --git a/packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.test.ts b/packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.test.ts deleted file mode 100644 index 726c3edc42e..00000000000 --- a/packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.test.ts +++ /dev/null @@ -1,165 +0,0 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; -import path from 'node:path'; -import { NoMisplacedCipherPrimitivesRule } from './no-misplaced-cipher-primitives.js'; - -const ruleTester = new RuleTester({ - languageOptions: { - parserOptions: { - projectService: { - allowDefaultProject: ['*.ts'], - }, - tsconfigRootDir: path.join(import.meta.dirname, 'fixtures'), - }, - }, -}); - -/** Minimal stand-in mirroring the shape of `Cipher` from n8n-core. */ -const cipherClass = ` -class Cipher { - encryptWithKey(data: string, key: string, algorithm: string): string { return ''; } - decryptWithKey(data: string, key: string, algorithm: string): string { return ''; } - async encryptV2(data: string | object): Promise { return ''; } -} -`; - -const untypedParse = { parserOptions: { projectService: false as const } }; - -ruleTester.run('no-misplaced-cipher-primitives', NoMisplacedCipherPrimitivesRule, { - valid: [ - // The encryption area itself may use the primitives - { - code: "import { CipherAes256CBC } from './aes-256-cbc';", - filename: `${path.sep}repo${path.sep}packages${path.sep}core${path.sep}src${path.sep}encryption${path.sep}cipher.ts`, - languageOptions: untypedParse, - }, - // Database migrations may use them - { - code: "import { CipherAes256GCM } from 'n8n-core';", - filename: `${path.sep}repo${path.sep}packages${path.sep}@n8n${path.sep}db${path.sep}src${path.sep}migrations${path.sep}common${path.sep}123-Migrate.ts`, - languageOptions: untypedParse, - }, - // Test files stay free - { - code: "import { CipherAes256CBC } from 'n8n-core';", - filename: 'cipher.test.ts', - }, - // The V2 path is the sanctioned one - { - code: `${cipherClass} -declare const cipher: Cipher; -void cipher.encryptV2('data');`, - filename: 'service.ts', - }, - // Wildcard re-exports of unrelated modules are fine - { - code: "export * from './utils';", - filename: 'index.ts', - }, - // encryptWithKey on an unrelated type stays allowed - { - code: `class Vault { - encryptWithKey(data: string, key: string): string { return ''; } -} -declare const vault: Vault; -vault.encryptWithKey('d', 'k');`, - filename: 'service.ts', - }, - ], - invalid: [ - { - code: "import { CipherAes256CBC } from 'n8n-core';", - filename: 'service.ts', - errors: [{ messageId: 'noPrimitiveReference', data: { name: 'CipherAes256CBC' } }], - }, - { - code: "import { CipherAes256GCM } from '../encryption/aes-256-gcm';", - filename: 'service.ts', - errors: [{ messageId: 'noPrimitiveReference', data: { name: 'CipherAes256GCM' } }], - }, - // Re-exports launder the class behind a different import path - { - code: "export { CipherAes256CBC } from 'n8n-core';", - filename: 'helper.ts', - errors: [{ messageId: 'noPrimitiveReference', data: { name: 'CipherAes256CBC' } }], - }, - // … and so do wildcard re-exports, which carry no identifier - { - code: "export * from 'n8n-core';", - filename: 'helper.ts', - errors: [{ messageId: 'noWildcardReexport', data: { source: 'n8n-core' } }], - }, - // A directory merely named "migrations" is not the database migration root - { - code: "import { CipherAes256CBC } from 'n8n-core';", - filename: `${path.sep}repo${path.sep}packages${path.sep}cli${path.sep}src${path.sep}nodes${path.sep}migrations${path.sep}helper.ts`, - languageOptions: untypedParse, - errors: [{ messageId: 'noPrimitiveReference', data: { name: 'CipherAes256CBC' } }], - }, - // Namespace member access - { - code: `import * as core from 'n8n-core'; -export const probe = () => new core.CipherAes256CBC();`, - filename: 'service.ts', - errors: [{ messageId: 'noPrimitiveReference', data: { name: 'CipherAes256CBC' } }], - }, - // Dynamic import destructuring — the shape AGENTS.md recommends for lazy loads - { - code: `export const probe = async () => { - const { CipherAes256GCM } = await import('n8n-core'); - return new CipherAes256GCM(); -};`, - filename: 'service.ts', - errors: [ - { messageId: 'noPrimitiveReference', data: { name: 'CipherAes256GCM' } }, - { messageId: 'noPrimitiveReference', data: { name: 'CipherAes256GCM' } }, - ], - }, - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher.encryptWithKey('d', 'k', 'aes-256-cbc');`, - filename: 'service.ts', - errors: [{ messageId: 'noExplicitKeyCall', data: { method: 'encryptWithKey' } }], - }, - // Bound method references escape without ever being a call expression - { - code: `${cipherClass} -declare const cipher: Cipher; -export const leak = cipher.encryptWithKey;`, - filename: 'service.ts', - errors: [{ messageId: 'noExplicitKeyCall', data: { method: 'encryptWithKey' } }], - }, - // Subclasses inherit the explicit-key methods - { - code: `${cipherClass} -class ExtendedCipher extends Cipher {} -declare const extended: ExtendedCipher; -extended.decryptWithKey('d', 'k', 'aes-256-cbc');`, - filename: 'service.ts', - errors: [{ messageId: 'noExplicitKeyCall', data: { method: 'decryptWithKey' } }], - }, - // A generic receiver constrained to Cipher can be one at runtime - { - code: `${cipherClass} -export function encode(cipher: T) { - return cipher.encryptWithKey('d', 'k', 'aes-256-cbc'); -}`, - filename: 'service.ts', - errors: [{ messageId: 'noExplicitKeyCall', data: { method: 'encryptWithKey' } }], - }, - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher?.decryptWithKey('d', 'k', 'aes-256-cbc');`, - filename: 'service.ts', - errors: [{ messageId: 'noExplicitKeyCall', data: { method: 'decryptWithKey' } }], - }, - { - code: `${cipherClass} -declare const cipher: Cipher; -cipher['encryptWithKey']('d', 'k', 'aes-256-cbc');`, - filename: 'service.ts', - errors: [{ messageId: 'noExplicitKeyCall', data: { method: 'encryptWithKey' } }], - }, - ], -}); diff --git a/packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.ts b/packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.ts deleted file mode 100644 index a4bc347d159..00000000000 --- a/packages/@n8n/eslint-config/src/rules/no-misplaced-cipher-primitives.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/utils'; -import type * as ts from 'typescript'; - -const PRIMITIVE_CLASSES = new Set(['CipherAes256CBC', 'CipherAes256GCM']); -const EXPLICIT_KEY_METHODS = new Set(['encryptWithKey', 'decryptWithKey']); - -/** - * Wildcard re-exports from these sources would expose the primitives. - * `n8n-core`'s own barrel (`export * from './encryption'`) is the sanctioned - * public surface and is not matched. - */ -const REEXPORT_SOURCE = /(^n8n-core$)|(aes-256)/; - -/** - * The encryption area itself and the database migrations may use the raw - * primitives; everything else must encrypt through the key-store-aware - * `encryptV2` / `decryptV2`. Only the actual database migration root is - * exempt — an unrelated directory that happens to be called `migrations` - * is not. - */ -const ALLOWED_PATH_PATTERN = - /([\\/]core[\\/]src[\\/]encryption[\\/])|([\\/]@n8n[\\/]db[\\/]src[\\/]migrations[\\/])/; - -/** Test files stay free to exercise any format for coverage. */ -const TEST_FILE_PATTERN = /(\.(test|spec)\.ts$)|([\\/]__tests__[\\/])|([\\/]test[\\/])/; - -export const NoMisplacedCipherPrimitivesRule = ESLintUtils.RuleCreator.withoutDocs({ - meta: { - type: 'problem', - docs: { - description: - 'Restrict the raw cipher primitives (`CipherAes256CBC`/`CipherAes256GCM`, `Cipher.encryptWithKey`/`decryptWithKey`) to the encryption area and database migrations. New encryption elsewhere must go through `encryptV2`/`decryptV2` so the key store stays in charge of key and format.', - }, - messages: { - noPrimitiveReference: - 'Do not reference `{{name}}` outside the encryption area. Encrypt through `cipher.encryptV2()` / `cipher.decryptV2()` instead.', - noExplicitKeyCall: - '`Cipher.{{method}}` is restricted to the encryption area and database migrations. New encryption must go through `encryptV2`/`decryptV2` so the key store stays in charge of key and format.', - noWildcardReexport: - 'Do not `export *` from `{{source}}` outside the encryption area — it re-exports the raw cipher primitives.', - }, - schema: [], - }, - defaultOptions: [], - create(context) { - if (ALLOWED_PATH_PATTERN.test(context.filename) || TEST_FILE_PATTERN.test(context.filename)) { - return {}; - } - - // Import specifiers expose the same source range twice (imported + - // local identifier); report each range once. - const reportedAt = new Set(); - - /** - * True for `Cipher` itself and for anything that can be one: subclasses, - * union/intersection members, and type parameters constrained to it. - */ - const typeIsCipher = (type: ts.Type, seen = new Set()): boolean => { - if (seen.has(type)) return false; - seen.add(type); - if ((type.getSymbol() ?? type.aliasSymbol)?.getName() === 'Cipher') return true; - if (type.isUnionOrIntersection()) { - return type.types.some((member) => typeIsCipher(member, seen)); - } - // Re-widen after each guard: the narrowing chain would otherwise - // leave `never` for the later checks. - const nonUnion: ts.Type = type; - if (nonUnion.isTypeParameter()) { - const constraint = nonUnion.getConstraint(); - return constraint !== undefined && typeIsCipher(constraint, seen); - } - const concrete: ts.Type = nonUnion; - if (concrete.isClassOrInterface()) { - return (concrete.getBaseTypes() ?? []).some((base) => typeIsCipher(base, seen)); - } - return false; - }; - - return { - // A blanket identifier check catches every reference shape: static and - // dynamic imports, namespace member access, named re-exports, and - // direct use. - Identifier(node) { - if (!PRIMITIVE_CLASSES.has(node.name)) return; - if (reportedAt.has(node.range[0])) return; - reportedAt.add(node.range[0]); - context.report({ node, messageId: 'noPrimitiveReference', data: { name: node.name } }); - }, - // Wildcard re-exports carry no identifier, so catch them by source. - ExportAllDeclaration(node) { - const source = String(node.source.value); - if (REEXPORT_SOURCE.test(source)) { - context.report({ node, messageId: 'noWildcardReexport', data: { source } }); - } - }, - // Member references (not just calls) so bound or passed method - // references cannot escape the boundary either. - MemberExpression(node) { - const { property } = node; - let method: string | null = null; - if (!node.computed && property.type === TSESTree.AST_NODE_TYPES.Identifier) { - method = property.name; - } else if ( - node.computed && - property.type === TSESTree.AST_NODE_TYPES.Literal && - typeof property.value === 'string' - ) { - method = property.value; - } - if (method === null || !EXPLICIT_KEY_METHODS.has(method)) return; - - // Cheap syntactic prefilter passed — only now consult the type checker. - const services = ESLintUtils.getParserServices(context); - const receiverType = services.getTypeAtLocation(node.object); - if (!typeIsCipher(receiverType)) return; - - context.report({ node: property, messageId: 'noExplicitKeyCall', data: { method } }); - }, - }; - }, -}); diff --git a/packages/cli/src/__tests__/credentials-helper.test.ts b/packages/cli/src/__tests__/credentials-helper.test.ts index 4324a5d5d7b..563df922186 100644 --- a/packages/cli/src/__tests__/credentials-helper.test.ts +++ b/packages/cli/src/__tests__/credentials-helper.test.ts @@ -6,14 +6,7 @@ import { } from '@n8n/db'; import { Container } from '@n8n/di'; import { EntityNotFoundError } from '@n8n/typeorm'; -import { - type InstanceSettings, - type Credentials, - Cipher, - CipherAes256GCM, - CipherAes256CBC, - EncryptionKeyProxy, -} from 'n8n-core'; +import { type InstanceSettings, type Credentials, Cipher, EncryptionKeyProxy } from 'n8n-core'; import { SalesforceJwtApi } from 'n8n-nodes-base/credentials/SalesforceJwtApi.credentials'; import { WekanApi } from 'n8n-nodes-base/credentials/WekanApi.credentials'; import type { @@ -73,8 +66,6 @@ describe('CredentialsHelper', () => { const encryptionKeyProxy = new EncryptionKeyProxy(); const cipher = new Cipher( mock({ encryptionKey: 'test_key_for_testing' }), - new CipherAes256GCM(), - new CipherAes256CBC(), encryptionKeyProxy, ); Container.set(Cipher, cipher); @@ -1038,7 +1029,7 @@ describe('CredentialsHelper', () => { id: 'cred-123', name: 'Test OAuth2 Credential', type: 'oAuth2Api', - data: cipher.encrypt(existingCredentialData), + data: cipher.encryptWithInstanceKey(existingCredentialData), usageScope: 'project', }; @@ -1072,7 +1063,9 @@ describe('CredentialsHelper', () => { expect(updatedAt).toBeInstanceOf(Date); expect(updatedAt.getTime()).toBeGreaterThanOrEqual(beforeUpdateTime.getTime()); - const decryptedUpdatedData = cipher.decrypt(updatedCredentialData.data as string); + const decryptedUpdatedData = cipher.decryptWithInstanceKey( + updatedCredentialData.data as string, + ); const parsedUpdatedData = JSON.parse(decryptedUpdatedData); expect(parsedUpdatedData).toEqual({ @@ -1141,7 +1134,7 @@ describe('CredentialsHelper', () => { id: 'cred-789', name: 'Test OAuth2 Credential', type: 'oAuth2Api', - data: cipher.encrypt(existingCredentialData), + data: cipher.encryptWithInstanceKey(existingCredentialData), isResolvable: true, resolverId: 'resolver-123', usageScope: 'project', @@ -1196,7 +1189,7 @@ describe('CredentialsHelper', () => { id: 'cred-789', name: 'Test OAuth2 Credential', type: 'oAuth2Api', - data: cipher.encrypt(existingCredentialData), + data: cipher.encryptWithInstanceKey(existingCredentialData), isResolvable: true, resolverId: null, usageScope: 'project', @@ -1252,7 +1245,7 @@ describe('CredentialsHelper', () => { id: 'cred-789', name: 'Test OAuth2 Credential', type: 'oAuth2Api', - data: cipher.encrypt(existingCredentialData), + data: cipher.encryptWithInstanceKey(existingCredentialData), isResolvable: true, resolverId: 'resolver-123', usageScope: 'project', @@ -1293,7 +1286,7 @@ describe('CredentialsHelper', () => { // Verify OAuth token was updated in database const updateCall = credentialsRepository.update.mock.calls[0]; - const updatedData = cipher.decrypt(updateCall[1].data as string); + const updatedData = cipher.decryptWithInstanceKey(updateCall[1].data as string); const parsedData = JSON.parse(updatedData); expect(parsedData.oauthTokenData.access_token).toBe('new-token'); }); @@ -1304,7 +1297,7 @@ describe('CredentialsHelper', () => { id: 'cred-789', name: 'Test OAuth2 Credential', type: 'oAuth2Api', - data: cipher.encrypt(existingCredentialData), + data: cipher.encryptWithInstanceKey(existingCredentialData), isResolvable: true, resolverId: 'resolver-123', usageScope: 'project', @@ -1340,7 +1333,7 @@ describe('CredentialsHelper', () => { // Verify OAuth token was updated in database const updateCall = credentialsRepository.update.mock.calls[0]; - const updatedData = cipher.decrypt(updateCall[1].data as string); + const updatedData = cipher.decryptWithInstanceKey(updateCall[1].data as string); const parsedData = JSON.parse(updatedData); expect(parsedData.oauthTokenData.access_token).toBe('new-token'); }); @@ -1621,7 +1614,7 @@ describe('CredentialsHelper', () => { id: 'cred-license-test', name: 'License Test Credential', type: 'testApi', - data: cipher.encrypt({ apiKey: 'test' }), + data: cipher.encryptWithInstanceKey({ apiKey: 'test' }), isResolvable: false, usageScope: 'project', } as CredentialsEntity; @@ -1731,7 +1724,7 @@ describe('CredentialsHelper', () => { id: 'cred-456', name: 'Test Credentials', type: credentialType, - data: cipher.encrypt({ apiKey: 'static-key' }), + data: cipher.encryptWithInstanceKey({ apiKey: 'static-key' }), isResolvable: false, usageScope: 'project', } as CredentialsEntity; @@ -2349,7 +2342,7 @@ describe('CredentialsHelper', () => { id: 'cred-aaa', name: 'Account A Credential', type: credentialType, - data: cipher.encrypt(credentialDataA), + data: cipher.encryptWithInstanceKey(credentialDataA), isResolvable: false, resolverId: null, usageScope: 'project', @@ -2359,7 +2352,7 @@ describe('CredentialsHelper', () => { id: 'cred-bbb', name: 'Account B Credential', type: credentialType, - data: cipher.encrypt(credentialDataB), + data: cipher.encryptWithInstanceKey(credentialDataB), isResolvable: false, resolverId: null, usageScope: 'project', @@ -2528,7 +2521,7 @@ describe('CredentialsHelper', () => { // Simulate saving credential B with updated data (re-encrypt with new values) const updatedDataB = { apiKey: 'key_account_B_UPDATED', accountId: 'pn_B_UPDATED' }; - credEntityB.data = cipher.encrypt(updatedDataB); + credEntityB.data = cipher.encryptWithInstanceKey(updatedDataB); const resultA_after = await credentialsHelper.getDecrypted( additionalData, @@ -3024,7 +3017,7 @@ describe('CredentialsHelper', () => { id: 'cred-policy', name: 'Policy Test Credential', type: 'testApi', - data: cipher.encrypt({ apiKey: 'test' }), + data: cipher.encryptWithInstanceKey({ apiKey: 'test' }), isResolvable: false, usageScope: 'project', } as CredentialsEntity; diff --git a/packages/cli/src/encryption/__tests__/key-manager.service.test.ts b/packages/cli/src/encryption/__tests__/key-manager.service.test.ts index 451039b0bba..32a0274c647 100644 --- a/packages/cli/src/encryption/__tests__/key-manager.service.test.ts +++ b/packages/cli/src/encryption/__tests__/key-manager.service.test.ts @@ -3,13 +3,7 @@ import { mockInstance } from '@n8n/backend-test-utils'; import type { DeploymentKey } from '@n8n/db'; import { DeploymentKeyRepository } from '@n8n/db'; import { Container } from '@n8n/di'; -import { - Cipher, - CipherAes256CBC, - CipherAes256GCM, - type EncryptionKeyProxy, - InstanceSettings, -} from 'n8n-core'; +import { Cipher, type EncryptionKeyProxy, InstanceSettings } from 'n8n-core'; import { randomBytes } from 'node:crypto'; import { mock } from 'vitest-mock-extended'; @@ -699,12 +693,7 @@ describe('KeyManagerService', () => { // A real Cipher, so every legacy value is produced by real encryption and the // re-wrap round-trips for real. Only the repository is mocked. const realCipher = (encryptionKey: string) => - new Cipher( - mock({ encryptionKey }), - new CipherAes256GCM(), - new CipherAes256CBC(), - mock(), - ); + new Cipher(mock({ encryptionKey }), mock()); const makeRepairService = (encryptionKey = randomBytes(24).toString('base64')) => { const repo = mock(); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-cors.integration.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-cors.integration.test.ts index 0bd6d5951fe..fd46096df0f 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-cors.integration.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-cors.integration.test.ts @@ -91,7 +91,7 @@ describe('POST /credentials/:id/authorize - CORS Integration', () => { type: 'oAuth2Api', // These routes only serve end-user credentials. isResolvable: true, - data: cipher.encrypt({ clientId: 'test-client-id' }), + data: cipher.encryptWithInstanceKey({ clientId: 'test-client-id' }), }), ); credentialId = credential.id; @@ -101,7 +101,7 @@ describe('POST /credentials/:id/authorize - CORS Integration', () => { id: randomId(), name: 'Test Resolver', type: 'test-resolver', - config: cipher.encrypt(JSON.stringify({ apiKey: 'test-api-key' })), + config: cipher.encryptWithInstanceKey(JSON.stringify({ apiKey: 'test-api-key' })), }); resolverId = resolver.id; @@ -211,7 +211,7 @@ describe('DELETE /credentials/:id/revoke - CORS Integration', () => { type: 'oAuth2Api', // These routes only serve end-user credentials. isResolvable: true, - data: cipher.encrypt({ clientId: 'test-client-id' }), + data: cipher.encryptWithInstanceKey({ clientId: 'test-client-id' }), }), ); credentialId = credential.id; @@ -221,7 +221,7 @@ describe('DELETE /credentials/:id/revoke - CORS Integration', () => { id: randomId(), name: 'Test Resolver', type: 'test-resolver', - config: cipher.encrypt(JSON.stringify({ apiKey: 'test-api-key' })), + config: cipher.encryptWithInstanceKey(JSON.stringify({ apiKey: 'test-api-key' })), }); resolverId = resolver.id; diff --git a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-rate-limit.integration.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-rate-limit.integration.test.ts index c9d40d1f9f4..fe9f544f65d 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-rate-limit.integration.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials-rate-limit.integration.test.ts @@ -109,7 +109,7 @@ async function setupTestData() { type: 'oAuth2Api', // These routes only serve end-user credentials. isResolvable: true, - data: cipher.encrypt({ clientId: 'test-client-id' }), + data: cipher.encryptWithInstanceKey({ clientId: 'test-client-id' }), }), ); @@ -117,7 +117,7 @@ async function setupTestData() { id: randomId(), name: 'Test Resolver', type: 'test-resolver', - config: cipher.encrypt(JSON.stringify({ apiKey: 'test-api-key' })), + config: cipher.encryptWithInstanceKey(JSON.stringify({ apiKey: 'test-api-key' })), }); const registry = Container.get(DynamicCredentialResolverRegistry); diff --git a/packages/cli/src/modules/source-control.ee/__tests__/source-control-export.service.test.ts b/packages/cli/src/modules/source-control.ee/__tests__/source-control-export.service.test.ts index 15fecc19ed1..808e8c7369e 100644 --- a/packages/cli/src/modules/source-control.ee/__tests__/source-control-export.service.test.ts +++ b/packages/cli/src/modules/source-control.ee/__tests__/source-control-export.service.test.ts @@ -98,7 +98,7 @@ describe('SourceControlExportService', () => { id: 'cred1', name: 'Test Credential', type: 'oauth2', - data: cipher.encrypt(credentialData), + data: cipher.encryptWithInstanceKey(credentialData), }); it('should export credentials to work folder', async () => { @@ -208,7 +208,7 @@ describe('SourceControlExportService', () => { id: 'global-cred1', name: 'Global Test Credential', type: 'oauth2', - data: cipher.encrypt(credentialData), + data: cipher.encryptWithInstanceKey(credentialData), isGlobal: true, isResolvable: false, resolvableAllowFallback: false, @@ -267,7 +267,7 @@ describe('SourceControlExportService', () => { id: 'non-global-cred1', name: 'Non-Global Test Credential', type: 'oauth2', - data: cipher.encrypt(credentialData), + data: cipher.encryptWithInstanceKey(credentialData), isGlobal: false, isResolvable: false, resolvableAllowFallback: false, @@ -329,7 +329,7 @@ describe('SourceControlExportService', () => { id: 'resolvable-cred1', name: 'Resolvable Credential', type: 'oauth2', - data: cipher.encrypt(credentialData), + data: cipher.encryptWithInstanceKey(credentialData), isGlobal: false, isResolvable: true, resolvableAllowFallback: true, @@ -367,7 +367,7 @@ describe('SourceControlExportService', () => { id: 'cred-no-flag', name: 'Credential Without Flag', type: 'oauth2', - data: cipher.encrypt(credentialData), + data: cipher.encryptWithInstanceKey(credentialData), isGlobal: undefined, // explicitly undefined to test the default }); @@ -412,7 +412,7 @@ describe('SourceControlExportService', () => { id: credentialsId, name: `Credential ${credentialsId}`, type: 'httpBasicAuth', - data: cipher.encrypt({ user: 'u', password: 'p' }), + data: cipher.encryptWithInstanceKey({ user: 'u', password: 'p' }), }), } as never) as SharedCredentials; diff --git a/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts b/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts index d2e342b5f5f..8bcf94e18df 100644 --- a/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts +++ b/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts @@ -347,7 +347,7 @@ describe('OAuth2 API', () => { const fakeState = { token: 'forged-token', createdAt: Date.now(), - data: oauthService['cipher'].encrypt( + data: oauthService['cipher'].encryptWithInstanceKey( JSON.stringify({ cid: credential.id, origin: 'static-credential', diff --git a/packages/cli/test/integration/credentials/credentials-overwrites.integration.test.ts b/packages/cli/test/integration/credentials/credentials-overwrites.integration.test.ts index d19daa4e93e..6e61cadfbb5 100644 --- a/packages/cli/test/integration/credentials/credentials-overwrites.integration.test.ts +++ b/packages/cli/test/integration/credentials/credentials-overwrites.integration.test.ts @@ -70,7 +70,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { expect(savedSetting?.value).toBeTruthy(); // Step 3: Decrypt and verify the saved data - const decryptedData = cipher.decrypt(savedSetting!.value); + const decryptedData = cipher.decryptWithInstanceKey(savedSetting!.value); const parsedData = JSON.parse(decryptedData); expect(parsedData).toEqual(testOverwriteData); @@ -89,7 +89,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { // Verify first operation let savedSetting = await settingsRepository.findByKey('credentialsOverwrite'); expect(savedSetting).toBeTruthy(); - let decryptedData = cipher.decrypt(savedSetting!.value); + let decryptedData = cipher.decryptWithInstanceKey(savedSetting!.value); expect(JSON.parse(decryptedData)).toEqual(data1); await credentialsOverwrites.setData(data2, true, false); @@ -97,7 +97,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { // Verify second operation overwrote the first savedSetting = await settingsRepository.findByKey('credentialsOverwrite'); expect(savedSetting).toBeTruthy(); - decryptedData = cipher.decrypt(savedSetting!.value); + decryptedData = cipher.decryptWithInstanceKey(savedSetting!.value); expect(JSON.parse(decryptedData)).toEqual(data2); // The final state should be the last write @@ -113,7 +113,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { }; // Step 1: Save data to database first - const encryptedData = cipher.encrypt(JSON.stringify(testData)); + const encryptedData = cipher.encryptWithInstanceKey(JSON.stringify(testData)); const setting = settingsRepository.create({ key: 'credentialsOverwrite', value: encryptedData, @@ -131,7 +131,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { it('should prevent race conditions between PubSub reload calls', async () => { const testData: ICredentialsOverwrite = { race: { condition: 'test' } }; - const encryptedData = cipher.encrypt(JSON.stringify(testData)); + const encryptedData = cipher.encryptWithInstanceKey(JSON.stringify(testData)); // Save test data to database const setting = settingsRepository.create({ @@ -163,7 +163,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { (globalConfig as any).credentials.overwrite.data = JSON.stringify(staticData); // Save database data - const encryptedDbData = cipher.encrypt(JSON.stringify(dbData)); + const encryptedDbData = cipher.encryptWithInstanceKey(JSON.stringify(dbData)); const dbSetting = settingsRepository.create({ key: 'credentialsOverwrite', value: encryptedDbData, @@ -205,7 +205,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { // Verify updated data was saved to database const savedSetting = await settingsRepository.findByKey('credentialsOverwrite'); expect(savedSetting).toBeTruthy(); - const decryptedData = cipher.decrypt(savedSetting!.value); + const decryptedData = cipher.decryptWithInstanceKey(savedSetting!.value); expect(JSON.parse(decryptedData)).toEqual(updatedData); }); }); @@ -228,7 +228,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { it('should coordinate multiple instances receiving same PubSub event', async () => { const testData: ICredentialsOverwrite = { coordination: { test: 'value' } }; - const encryptedData = cipher.encrypt(JSON.stringify(testData)); + const encryptedData = cipher.encryptWithInstanceKey(JSON.stringify(testData)); // Save data to database const setting = settingsRepository.create({ @@ -345,7 +345,7 @@ describe('CredentialsOverwrites - Integration Tests', () => { // Step 2: Verify database persistence const savedSetting = await settingsRepository.findByKey('credentialsOverwrite'); expect(savedSetting).toBeTruthy(); - const decryptedData = cipher.decrypt(savedSetting!.value); + const decryptedData = cipher.decryptWithInstanceKey(savedSetting!.value); expect(JSON.parse(decryptedData)).toEqual(e2eData); // Step 3: Test PubSub reload diff --git a/packages/cli/test/integration/database/repositories/secrets-provider-connection.repository.test.ts b/packages/cli/test/integration/database/repositories/secrets-provider-connection.repository.test.ts index 767d3de2384..1f9c664c34c 100644 --- a/packages/cli/test/integration/database/repositories/secrets-provider-connection.repository.test.ts +++ b/packages/cli/test/integration/database/repositories/secrets-provider-connection.repository.test.ts @@ -47,7 +47,7 @@ describe('SecretsProviderConnectionRepository', () => { async function createConnection(providerKey: string, type: string, projectIds: string[] = []) { const cipher = Container.get(Cipher); - const encryptedSettings = cipher.encrypt({}); + const encryptedSettings = cipher.encryptWithInstanceKey({}); const connection = await connectionRepository.save( connectionRepository.create({ @@ -81,7 +81,7 @@ describe('SecretsProviderConnectionRepository', () => { credentialsRepository.create({ name: 'Test Credential', type: 'githubApi', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), }), ); diff --git a/packages/cli/test/integration/environments/source-control-export.service.test.ts b/packages/cli/test/integration/environments/source-control-export.service.test.ts index fbdac062a8f..1791300edb8 100644 --- a/packages/cli/test/integration/environments/source-control-export.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-export.service.test.ts @@ -141,7 +141,7 @@ describe('SourceControlExportService Integration', () => { { name: 'Test Personal Credential', type: 'testCredentialType', - data: Container.get(Cipher).encrypt(credentialData), + data: Container.get(Cipher).encryptWithInstanceKey(credentialData), }, personalProject, ); @@ -201,7 +201,7 @@ describe('SourceControlExportService Integration', () => { { name: 'Test Team Credential', type: 'teamCredentialType', - data: Container.get(Cipher).encrypt(credentialData), + data: Container.get(Cipher).encryptWithInstanceKey(credentialData), }, teamProject, ); @@ -275,7 +275,7 @@ describe('SourceControlExportService Integration', () => { { name: 'Existing Credential', type: 'existingType', - data: Container.get(Cipher).encrypt({ apiKey: 'test' }), + data: Container.get(Cipher).encryptWithInstanceKey({ apiKey: 'test' }), }, personalProject, ); @@ -307,7 +307,7 @@ describe('SourceControlExportService Integration', () => { Object.assign(credential, { name: 'Role Access Test Credential', type: 'roleTestType', - data: Container.get(Cipher).encrypt({ testField: 'test-value' }), + data: Container.get(Cipher).encryptWithInstanceKey({ testField: 'test-value' }), }); const savedCredential = await credentialsRepository.save(credential); @@ -360,7 +360,7 @@ describe('SourceControlExportService Integration', () => { { name: 'OAuth Test Credential', type: 'oauth2Credential', - data: Container.get(Cipher).encrypt(credentialData), + data: Container.get(Cipher).encryptWithInstanceKey(credentialData), }, personalProject, ); @@ -409,7 +409,7 @@ describe('SourceControlExportService Integration', () => { { name: 'Nested Data Credential', type: 'databaseCredential', - data: Container.get(Cipher).encrypt(credentialData), + data: Container.get(Cipher).encryptWithInstanceKey(credentialData), }, teamProject, ); @@ -456,7 +456,7 @@ describe('SourceControlExportService Integration', () => { { name: 'Global Test Credential', type: 'globalCredentialType', - data: Container.get(Cipher).encrypt(credentialData), + data: Container.get(Cipher).encryptWithInstanceKey(credentialData), isGlobal: true, }, personalProject, @@ -503,7 +503,7 @@ describe('SourceControlExportService Integration', () => { { name: 'Non-Global Credential', type: 'standardCredentialType', - data: Container.get(Cipher).encrypt(credentialData), + data: Container.get(Cipher).encryptWithInstanceKey(credentialData), isGlobal: false, }, teamProject, diff --git a/packages/cli/test/integration/environments/source-control.service.test.ts b/packages/cli/test/integration/environments/source-control.service.test.ts index 7008960247f..e5706e10e5f 100644 --- a/packages/cli/test/integration/environments/source-control.service.test.ts +++ b/packages/cli/test/integration/environments/source-control.service.test.ts @@ -341,14 +341,14 @@ describe('SourceControlService', () => { deletedInScopeCredential = Object.assign(new CredentialsEntity(), { id: 'deletedInScope', name: 'deletedInScope', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), type: '', }); deletedOutOfScopeCredential = Object.assign(new CredentialsEntity(), { id: 'deletedOutOfScope', name: 'deletedOutOfScope', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), type: '', }); @@ -361,7 +361,7 @@ describe('SourceControlService', () => { await createCredentials( { name: 'OutOfScope', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), type: '', }, projectB, @@ -369,7 +369,7 @@ describe('SourceControlService', () => { await createCredentials( { name: 'IntoScope', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), type: '', }, projectA, @@ -393,7 +393,7 @@ describe('SourceControlService', () => { await createCredentials( { name: `${project.name}-CredA`, - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), type: '', }, project, @@ -401,7 +401,7 @@ describe('SourceControlService', () => { await createCredentials( { name: `${project.name}-CredB‚`, - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), type: '', }, project, @@ -1468,7 +1468,7 @@ describe('SourceControlService', () => { { name: 'Test Credential isGlobal false->true', type: 'testType', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), isGlobal: false, }, testProject, @@ -1499,7 +1499,7 @@ describe('SourceControlService', () => { { name: 'Test Credential isGlobal true->false', type: 'testType', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), isGlobal: true, }, testProject, @@ -1527,7 +1527,7 @@ describe('SourceControlService', () => { { name: 'Test Credential isGlobal undefined vs false', type: 'testType', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), isGlobal: false, }, testProject, @@ -1555,7 +1555,7 @@ describe('SourceControlService', () => { { name: 'Test Credential isGlobal undefined->true', type: 'testType', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), isGlobal: false, }, testProject, @@ -1583,7 +1583,7 @@ describe('SourceControlService', () => { { name: 'Test Credential isGlobal same value', type: 'testType', - data: cipher.encrypt({}), + data: cipher.encryptWithInstanceKey({}), isGlobal: true, }, testProject, diff --git a/packages/cli/test/integration/external-secrets/external-secrets.events.test.ts b/packages/cli/test/integration/external-secrets/external-secrets.events.test.ts index 435d7eb7649..a3338a8a993 100644 --- a/packages/cli/test/integration/external-secrets/external-secrets.events.test.ts +++ b/packages/cli/test/integration/external-secrets/external-secrets.events.test.ts @@ -111,7 +111,7 @@ describe('External Secrets Event Handling', () => { await settingsRepository.save({ key: 'feature.externalSecrets', - value: cipher.encrypt(settings), + value: cipher.encryptWithInstanceKey(settings), loadOnStartup: false, }); @@ -143,7 +143,7 @@ describe('External Secrets Event Handling', () => { (config as any).externalSecretsForProjects = true; const cipher = Container.get(Cipher); - const encryptedSettings = cipher.encrypt(JSON.stringify({})); + const encryptedSettings = cipher.encryptWithInstanceKey(JSON.stringify({})); const connectionRepository = Container.get(SecretsProviderConnectionRepository); await connectionRepository.save({ diff --git a/packages/cli/test/integration/external-secrets/external-secrets.module.test.ts b/packages/cli/test/integration/external-secrets/external-secrets.module.test.ts index 2d7cacb4249..fbdb8f3793f 100644 --- a/packages/cli/test/integration/external-secrets/external-secrets.module.test.ts +++ b/packages/cli/test/integration/external-secrets/external-secrets.module.test.ts @@ -78,7 +78,7 @@ describe('ExternalSecretsModule', () => { await settingsRepository.save({ key: 'feature.externalSecrets', - value: cipher.encrypt(settings), + value: cipher.encryptWithInstanceKey(settings), loadOnStartup: false, }); }); @@ -145,7 +145,7 @@ describe('ExternalSecretsModule', () => { module = Container.get(ExternalSecretsModule); - const encryptedSettings = cipher.encrypt(JSON.stringify({})); + const encryptedSettings = cipher.encryptWithInstanceKey(JSON.stringify({})); await connectionRepository.save({ providerKey: 'my-vault', type: 'dummy', diff --git a/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts b/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts index 653623d9cc2..9ab5a6ee3cf 100644 --- a/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts +++ b/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts @@ -153,7 +153,9 @@ describe('Secret Providers Connections API', () => { // Check we can decrypt them const cipher = Container.get(Cipher); - const decryptedSettings = JSON.parse(cipher.decrypt(savedConnection.encryptedSettings)); + const decryptedSettings = JSON.parse( + cipher.decryptWithInstanceKey(savedConnection.encryptedSettings), + ); expect(decryptedSettings).toEqual(payload.settings); }); @@ -268,7 +270,9 @@ describe('Secret Providers Connections API', () => { }); const cipher = Container.get(Cipher); - const decryptedSettings = JSON.parse(cipher.decrypt(updated.encryptedSettings)); + const decryptedSettings = JSON.parse( + cipher.decryptWithInstanceKey(updated.encryptedSettings), + ); expect(decryptedSettings).toEqual({ region: 'eu-west-1', diff --git a/packages/cli/test/integration/external-secrets/secret-providers-project.api.test.ts b/packages/cli/test/integration/external-secrets/secret-providers-project.api.test.ts index 5f7570d0c23..eab3b8aac8d 100644 --- a/packages/cli/test/integration/external-secrets/secret-providers-project.api.test.ts +++ b/packages/cli/test/integration/external-secrets/secret-providers-project.api.test.ts @@ -80,7 +80,7 @@ describe('Secret Providers Project API', () => { ): Promise { const cipher = Container.get(Cipher); const mockSettings = { region: 'us-east-1', accessKeyId: 'test-key' }; - const encryptedSettings = cipher.encrypt(mockSettings); + const encryptedSettings = cipher.encryptWithInstanceKey(mockSettings); const connection = await connectionRepository.save( connectionRepository.create({ diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 55f1b177b0f..b5337b9b423 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -46,7 +46,7 @@ beforeAll(async () => { owner = await createUser({ role: GLOBAL_OWNER_ROLE }); authOwnerAgent = testServer.authAgentFor(owner); - defaultLdapConfig.bindingAdminPassword = Container.get(Cipher).encrypt( + defaultLdapConfig.bindingAdminPassword = Container.get(Cipher).encryptWithInstanceKey( defaultLdapConfig.bindingAdminPassword, ); }); diff --git a/packages/cli/test/migration/1771500000000-migrate-external-secrets-to-entity-storage.test.ts b/packages/cli/test/migration/1771500000000-migrate-external-secrets-to-entity-storage.test.ts index 200425ecb14..09d9d0574ba 100644 --- a/packages/cli/test/migration/1771500000000-migrate-external-secrets-to-entity-storage.test.ts +++ b/packages/cli/test/migration/1771500000000-migrate-external-secrets-to-entity-storage.test.ts @@ -107,7 +107,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { }, }; - const encrypted = cipher.encrypt(JSON.stringify(settings)); + const encrypted = cipher.encryptWithInstanceKey(JSON.stringify(settings)); await insertSettingsBlob(context, encrypted); await context.queryRunner.release(); @@ -122,13 +122,13 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { const aws = connections.find((c) => c.providerKey === 'awsSecretsManager'); expect(aws).toBeDefined(); expect(aws!.type).toBe('awsSecretsManager'); - const awsDecrypted = JSON.parse(cipher.decrypt(aws!.encryptedSettings)); + const awsDecrypted = JSON.parse(cipher.decryptWithInstanceKey(aws!.encryptedSettings)); expect(awsDecrypted).toEqual({ region: 'us-east-1', accessKeyId: 'AKIA...' }); const gcp = connections.find((c) => c.providerKey === 'gcpSecretsManager'); expect(gcp).toBeDefined(); expect(gcp!.type).toBe('gcpSecretsManager'); - const gcpDecrypted = JSON.parse(cipher.decrypt(gcp!.encryptedSettings)); + const gcpDecrypted = JSON.parse(cipher.decryptWithInstanceKey(gcp!.encryptedSettings)); expect(gcpDecrypted).toEqual({ projectId: 'my-project' }); await postContext.queryRunner.release(); @@ -150,7 +150,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { }, }; - const encrypted = cipher.encrypt(JSON.stringify(settings)); + const encrypted = cipher.encryptWithInstanceKey(JSON.stringify(settings)); await insertSettingsBlob(context, encrypted); await context.queryRunner.release(); @@ -169,7 +169,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { it('should skip providers that already exist in secrets_provider_connection', async () => { const context = createTestMigrationContext(dataSource); - const existingEncryptedSettings = cipher.encrypt({ region: 'eu-west-1' }); + const existingEncryptedSettings = cipher.encryptWithInstanceKey({ region: 'eu-west-1' }); await insertProviderConnection( context, 'awsSecretsManager', @@ -185,7 +185,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { }, }; - const encrypted = cipher.encrypt(JSON.stringify(settings)); + const encrypted = cipher.encryptWithInstanceKey(JSON.stringify(settings)); await insertSettingsBlob(context, encrypted); await context.queryRunner.release(); @@ -197,7 +197,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { expect(connections).toHaveLength(1); // Should still have the original settings, not the migrated ones - const decrypted = JSON.parse(cipher.decrypt(connections[0].encryptedSettings)); + const decrypted = JSON.parse(cipher.decryptWithInstanceKey(connections[0].encryptedSettings)); expect(decrypted).toEqual({ region: 'eu-west-1' }); await postContext.queryRunner.release(); @@ -206,7 +206,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { it('should skip when settings blob is empty', async () => { const context = createTestMigrationContext(dataSource); - const encrypted = cipher.encrypt(JSON.stringify({})); + const encrypted = cipher.encryptWithInstanceKey(JSON.stringify({})); await insertSettingsBlob(context, encrypted); await context.queryRunner.release(); @@ -245,7 +245,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { }, }; - const encrypted = cipher.encrypt(JSON.stringify(settings)); + const encrypted = cipher.encryptWithInstanceKey(JSON.stringify(settings)); await insertSettingsBlob(context, encrypted); await context.queryRunner.release(); @@ -256,7 +256,7 @@ describe('MigrateExternalSecretsToEntityStorage Migration', () => { const connections = await getProviderConnections(postContext); expect(connections).toHaveLength(1); - const decrypted = JSON.parse(cipher.decrypt(connections[0].encryptedSettings)); + const decrypted = JSON.parse(cipher.decryptWithInstanceKey(connections[0].encryptedSettings)); expect(decrypted).toEqual({}); await postContext.queryRunner.release(); diff --git a/packages/cli/test/migration/1773000000000-create-credential-dependency-table.test.ts b/packages/cli/test/migration/1773000000000-create-credential-dependency-table.test.ts index c84d3dc6124..46f2354f80a 100644 --- a/packages/cli/test/migration/1773000000000-create-credential-dependency-table.test.ts +++ b/packages/cli/test/migration/1773000000000-create-credential-dependency-table.test.ts @@ -65,7 +65,7 @@ describe('CreateCredentialDependencyTable Migration', () => { { providerKey: data.providerKey, type: data.type ?? data.providerKey, - encryptedSettings: cipher.encrypt(JSON.stringify({ region: 'us-east-1' })), + encryptedSettings: cipher.encryptWithInstanceKey(JSON.stringify({ region: 'us-east-1' })), isEnabled: true, createdAt: now, updatedAt: now, @@ -135,7 +135,7 @@ describe('CreateCredentialDependencyTable Migration', () => { await insertCredential(context, { id: credentialWithSecretsId, - encryptedData: cipher.encrypt( + encryptedData: cipher.encryptWithInstanceKey( JSON.stringify({ apiKey: '={{ $secrets.vault.primaryKey }}', nested: { @@ -148,12 +148,14 @@ describe('CreateCredentialDependencyTable Migration', () => { await insertCredential(context, { id: credentialWithoutSecretsId, - encryptedData: cipher.encrypt(JSON.stringify({ apiKey: 'plain-value' })), + encryptedData: cipher.encryptWithInstanceKey(JSON.stringify({ apiKey: 'plain-value' })), }); await insertCredential(context, { id: credentialUnknownProviderId, - encryptedData: cipher.encrypt(JSON.stringify({ apiKey: '={{ $secrets.unknown.key }}' })), + encryptedData: cipher.encryptWithInstanceKey( + JSON.stringify({ apiKey: '={{ $secrets.unknown.key }}' }), + ), }); return { vaultProviderId, awsProviderId }; diff --git a/packages/cli/test/shared/mocking.ts b/packages/cli/test/shared/mocking.ts index 306886644a0..2a1a921f840 100644 --- a/packages/cli/test/shared/mocking.ts +++ b/packages/cli/test/shared/mocking.ts @@ -15,8 +15,6 @@ export const mockEntityManager = (entityClass: Class) => { export const mockCipher = () => mock({ - encrypt: (data) => (typeof data === 'string' ? data : JSON.stringify(data)), - decrypt: (data) => data, encryptV2: async (data) => (typeof data === 'string' ? data : JSON.stringify(data)), decryptV2: async (data) => data, }); diff --git a/packages/core/src/__tests__/credentials.test.ts b/packages/core/src/__tests__/credentials.test.ts index 3587c1370d6..39c1d17ce3c 100644 --- a/packages/core/src/__tests__/credentials.test.ts +++ b/packages/core/src/__tests__/credentials.test.ts @@ -4,8 +4,6 @@ import { AssertionError } from 'node:assert'; import { mock } from 'vitest-mock-extended'; import { CREDENTIAL_ERRORS } from '@/constants'; -import { CipherAes256CBC } from '@/encryption/aes-256-cbc'; -import { CipherAes256GCM } from '@/encryption/aes-256-gcm'; import { Cipher } from '@/encryption/cipher'; import { EncryptionKeyProxy } from '@/encryption/encryption-key-proxy'; import type { InstanceSettings } from '@/instance-settings'; @@ -19,8 +17,6 @@ describe('Credentials', () => { const encryptionKeyProxy = new EncryptionKeyProxy(); const cipher = new Cipher( mock({ encryptionKey: 'password' }), - new CipherAes256GCM(), - new CipherAes256CBC(), encryptionKeyProxy, ); Container.set(Cipher, cipher); @@ -107,7 +103,7 @@ describe('Credentials', () => { test('should throw an error when JSON parsing fails', async () => { const credentials = new Credentials(nodeCredentials, credentialType); - credentials.data = cipher.encrypt('invalid-json-string'); + credentials.data = cipher.encryptWithInstanceKey('invalid-json-string'); await expect(credentials.getData()).rejects.toThrow(CREDENTIAL_ERRORS.INVALID_JSON); @@ -150,7 +146,7 @@ describe('Credentials', () => { const credentials = new Credentials( nodeCredentials, credentialType, - cipher.encrypt({ + cipher.encryptWithInstanceKey({ username: 'olduser', password: 'oldpass', apiKey: 'oldkey', @@ -170,7 +166,7 @@ describe('Credentials', () => { const credentials = new Credentials( nodeCredentials, credentialType, - cipher.encrypt({ + cipher.encryptWithInstanceKey({ username: 'testuser', password: 'testpass', apiKey: 'testkey', @@ -188,7 +184,7 @@ describe('Credentials', () => { const credentials = new Credentials( nodeCredentials, credentialType, - cipher.encrypt({ + cipher.encryptWithInstanceKey({ username: 'olduser', password: 'oldpass', apiKey: 'oldkey', diff --git a/packages/core/src/encryption/__tests__/cipher.test.ts b/packages/core/src/encryption/__tests__/cipher.test.ts index 3b746503027..89ee0c69aaf 100644 --- a/packages/core/src/encryption/__tests__/cipher.test.ts +++ b/packages/core/src/encryption/__tests__/cipher.test.ts @@ -3,50 +3,28 @@ import { Container } from '@n8n/di'; import { InstanceSettings } from '@/instance-settings'; import { mockInstance } from '@test/utils'; +import { CipherAes256CBC } from '../aes-256-cbc'; +import { CipherAes256GCM } from '../aes-256-gcm'; import { Cipher } from '../cipher'; import { EncryptionKeyProxy, type IEncryptionKeyProvider } from '../encryption-key-proxy'; describe('Cipher', () => { mockInstance(InstanceSettings, { encryptionKey: 'test_key' }); const cipher = Container.get(Cipher); + const cbc = new CipherAes256CBC(); + const gcm = new CipherAes256GCM(); // Decrypts CBC ciphertext with the given key, normalizing a padding-validation // throw into a sentinel. Used to assert wrong-key decryption never recovers the // original plaintext without relying on the flaky ~255/256 chance that CBC throws. const decryptWithCbcKey = (ciphertext: string, key: string): string => { try { - return cipher.decryptWithKey(ciphertext, key, 'aes-256-cbc'); + return cbc.decrypt(ciphertext, key); } catch { return '\0__decryption-threw__'; } }; - describe('encrypt', () => { - it('should encrypt strings', () => { - const encrypted = cipher.encrypt('random-string'); - const decrypted = cipher.decrypt(encrypted); - expect(decrypted).toEqual('random-string'); - }); - - it('should encrypt objects', () => { - const encrypted = cipher.encrypt({ key: 'value' }); - const decrypted = cipher.decrypt(encrypted); - expect(decrypted).toEqual('{"key":"value"}'); - }); - }); - - describe('decrypt', () => { - it('should decrypt string', () => { - const decrypted = cipher.decrypt('U2FsdGVkX194VEoX27o3+y5jUd1JTTmVwkOKjVhB6Jg='); - expect(decrypted).toEqual('random-string'); - }); - - it('should not try to decrypt if the input is shorter than 16 bytes', () => { - const decrypted = cipher.decrypt('U2FsdGVkX194VEo'); - expect(decrypted).toEqual(''); - }); - }); - describe('encryptWithInstanceKey / decryptWithInstanceKey', () => { it('should roundtrip strings using the instance key regardless of custom args', () => { const encrypted = cipher.encryptWithInstanceKey('random-string'); @@ -58,9 +36,19 @@ describe('Cipher', () => { expect(cipher.decryptWithInstanceKey(encrypted)).toEqual('{"key":"value"}'); }); + it('should decrypt ciphertext from earlier releases', () => { + expect(cipher.decryptWithInstanceKey('U2FsdGVkX194VEoX27o3+y5jUd1JTTmVwkOKjVhB6Jg=')).toEqual( + 'random-string', + ); + }); + + it('should return an empty string for input shorter than the CBC header', () => { + expect(cipher.decryptWithInstanceKey('U2FsdGVkX194VEo')).toEqual(''); + }); + it('should produce ciphertext that is interoperable with decryptWithKey(instance-key)', () => { const encrypted = cipher.encryptWithInstanceKey('random-string'); - expect(cipher.decryptWithKey(encrypted, 'test_key', 'aes-256-cbc')).toEqual('random-string'); + expect(cbc.decrypt(encrypted, 'test_key')).toEqual('random-string'); }); it('should not recover the plaintext with a non-instance key', () => { @@ -86,35 +74,29 @@ describe('Cipher', () => { }); }); - describe('encryptWithKey', () => { + describe('AES-256-CBC', () => { it('should roundtrip with decryptWithKey using the same key', () => { - const encrypted = cipher.encryptWithKey('random-string', 'explicit-key', 'aes-256-cbc'); - const decrypted = cipher.decryptWithKey(encrypted, 'explicit-key', 'aes-256-cbc'); + const encrypted = cbc.encrypt('random-string', 'explicit-key'); + const decrypted = cbc.decrypt(encrypted, 'explicit-key'); expect(decrypted).toEqual('random-string'); }); it('should produce different ciphertexts for the same plaintext on successive calls', () => { - const first = cipher.encryptWithKey('random-string', 'explicit-key', 'aes-256-cbc'); - const second = cipher.encryptWithKey('random-string', 'explicit-key', 'aes-256-cbc'); + const first = cbc.encrypt('random-string', 'explicit-key'); + const second = cbc.encrypt('random-string', 'explicit-key'); expect(first).not.toEqual(second); }); it('should not recover the plaintext with a different key', () => { - const encrypted = cipher.encryptWithKey('random-string', 'key-a', 'aes-256-cbc'); + const encrypted = cbc.encrypt('random-string', 'key-a'); // Unauthenticated CBC cannot reliably detect a wrong key: decryption // either throws on padding validation or yields garbage, but it must // never return the original plaintext. expect(decryptWithCbcKey(encrypted, 'key-b')).not.toEqual('random-string'); }); - - it('should be interoperable with legacy encrypt when given the same key', () => { - const encrypted = cipher.encrypt('random-string', 'shared-key'); - const decrypted = cipher.decryptWithKey(encrypted, 'shared-key', 'aes-256-cbc'); - expect(decrypted).toEqual('random-string'); - }); }); - describe('encryptWithKey / decryptWithKey — aes-256-gcm', () => { + describe('AES-256-GCM', () => { const gcmKey = '11'.repeat(32); const otherGcmKey = '22'.repeat(32); @@ -125,25 +107,25 @@ describe('Cipher', () => { const CIPHERTEXT_OFFSET = 49; it('should roundtrip with decryptWithKey using the same key', () => { - const encrypted = cipher.encryptWithKey('random-string', gcmKey, 'aes-256-gcm'); - const decrypted = cipher.decryptWithKey(encrypted, gcmKey, 'aes-256-gcm'); + const encrypted = gcm.encrypt('random-string', gcmKey); + const decrypted = gcm.decrypt(encrypted, gcmKey); expect(decrypted).toEqual('random-string'); }); it('should roundtrip an empty string', () => { - const encrypted = cipher.encryptWithKey('', gcmKey, 'aes-256-gcm'); - expect(cipher.decryptWithKey(encrypted, gcmKey, 'aes-256-gcm')).toEqual(''); + const encrypted = gcm.encrypt('', gcmKey); + expect(gcm.decrypt(encrypted, gcmKey)).toEqual(''); }); it('should produce different ciphertexts for the same plaintext on successive calls', () => { - const first = cipher.encryptWithKey('random-string', gcmKey, 'aes-256-gcm'); - const second = cipher.encryptWithKey('random-string', gcmKey, 'aes-256-gcm'); + const first = gcm.encrypt('random-string', gcmKey); + const second = gcm.encrypt('random-string', gcmKey); expect(first).not.toEqual(second); }); it('should fail to decrypt with a different key', () => { - const encrypted = cipher.encryptWithKey('random-string', gcmKey, 'aes-256-gcm'); - expect(() => cipher.decryptWithKey(encrypted, otherGcmKey, 'aes-256-gcm')).toThrow(); + const encrypted = gcm.encrypt('random-string', gcmKey); + expect(() => gcm.decrypt(encrypted, otherGcmKey)).toThrow(); }); it.each([ @@ -151,48 +133,42 @@ describe('Cipher', () => { { region: 'auth tag', offset: AUTH_TAG_OFFSET + 7 }, { region: 'ciphertext body', offset: CIPHERTEXT_OFFSET + 1 }, ])('should throw when the $region has been tampered with', ({ offset }) => { - const encrypted = cipher.encryptWithKey('some-longer-plaintext', gcmKey, 'aes-256-gcm'); + const encrypted = gcm.encrypt('some-longer-plaintext', gcmKey); const buf = Buffer.from(encrypted, 'base64'); buf[offset] = buf[offset] ^ 0x01; const tampered = buf.toString('base64'); - expect(() => cipher.decryptWithKey(tampered, gcmKey, 'aes-256-gcm')).toThrow(); + expect(() => gcm.decrypt(tampered, gcmKey)).toThrow(); }); it('should throw when a CBC ciphertext is fed into the GCM decryption path', () => { - const cbcCiphertext = cipher.encryptWithKey('random-string', gcmKey, 'aes-256-cbc'); - expect(() => cipher.decryptWithKey(cbcCiphertext, gcmKey, 'aes-256-gcm')).toThrow(); + const cbcCiphertext = cbc.encrypt('random-string', gcmKey); + expect(() => gcm.decrypt(cbcCiphertext, gcmKey)).toThrow(); }); it('should throw before any cipher operation when the key is not 32 bytes', () => { const shortKey = '00'.repeat(31); const expectedError = 'GCM key must be exactly 32 bytes (64 hex characters)'; - expect(() => cipher.encryptWithKey('random-string', shortKey, 'aes-256-gcm')).toThrow( - expectedError, - ); - expect(() => cipher.decryptWithKey('irrelevant', shortKey, 'aes-256-gcm')).toThrow( - expectedError, - ); + expect(() => gcm.encrypt('random-string', shortKey)).toThrow(expectedError); + expect(() => gcm.decrypt('irrelevant', shortKey)).toThrow(expectedError); }); it('should throw when ciphertext is shorter than the minimum header size', () => { // version(1) || salt(32) || truncated tag(4) = 37 bytes, below the 49-byte minimum const truncated = Buffer.concat([Buffer.from([0x01]), Buffer.alloc(32), Buffer.alloc(4)]); - expect(() => - cipher.decryptWithKey(truncated.toString('base64'), gcmKey, 'aes-256-gcm'), - ).toThrow('GCM ciphertext too short'); - }); - - it('should throw when given an empty ciphertext', () => { - expect(() => cipher.decryptWithKey('', gcmKey, 'aes-256-gcm')).toThrow( + expect(() => gcm.decrypt(truncated.toString('base64'), gcmKey)).toThrow( 'GCM ciphertext too short', ); }); + it('should throw when given an empty ciphertext', () => { + expect(() => gcm.decrypt('', gcmKey)).toThrow('GCM ciphertext too short'); + }); + it('should throw when ciphertext has an unsupported version byte', () => { - const encrypted = cipher.encryptWithKey('random-string', gcmKey, 'aes-256-gcm'); + const encrypted = gcm.encrypt('random-string', gcmKey); const buf = Buffer.from(encrypted, 'base64'); buf[VERSION_OFFSET] = 0xff; - expect(() => cipher.decryptWithKey(buf.toString('base64'), gcmKey, 'aes-256-gcm')).toThrow( + expect(() => gcm.decrypt(buf.toString('base64'), gcmKey)).toThrow( 'Unsupported GCM ciphertext version', ); }); @@ -230,7 +206,7 @@ describe('Cipher', () => { expect(encrypted.startsWith(`${keyId}:`)).toBe(true); const ciphertext = encrypted.slice(keyId.length + 1); - const decrypted = cipher.decryptWithKey(ciphertext, plaintextDataKey, 'aes-256-gcm'); + const decrypted = gcm.decrypt(ciphertext, plaintextDataKey); expect(decrypted).toEqual('hello'); }); @@ -272,7 +248,7 @@ describe('Cipher', () => { it('should decrypt using the keyId from the prefix when the provider is registered', async () => { const keyId = 'test-uuid-5678'; const encryptedDataKey = cipher.encryptDEKWithInstanceKey(plaintextDataKey); - const ciphertext = cipher.encryptWithKey('world', plaintextDataKey, 'aes-256-gcm'); + const ciphertext = gcm.encrypt('world', plaintextDataKey); const prefixed = `${keyId}:${ciphertext}`; withProvider({ @@ -314,7 +290,7 @@ describe('Cipher', () => { })); withProvider({ getKeyById, getLegacyKey }); - const legacyCiphertext = cipher.encryptWithKey('legacy-data', instanceKey, 'aes-256-cbc'); + const legacyCiphertext = cbc.encrypt('legacy-data', instanceKey); const decrypted = await cipher.decryptV2(legacyCiphertext); expect(decrypted).toEqual('legacy-data'); @@ -346,7 +322,7 @@ describe('Cipher', () => { it('should emit key-lookup (prefixed) and decrypt events for prefixed data', async () => { const keyId = 'test-uuid-9012'; const encryptedDataKey = cipher.encryptDEKWithInstanceKey(plaintextDataKey); - const ciphertext = cipher.encryptWithKey('metric', plaintextDataKey, 'aes-256-gcm'); + const ciphertext = gcm.encrypt('metric', plaintextDataKey); withProvider({ getKeyById: async (id: string) => id === keyId @@ -377,7 +353,7 @@ describe('Cipher', () => { format: 'no-prefix', }), }); - const legacyCiphertext = cipher.encryptWithKey('legacy-metric', instanceKey, 'aes-256-cbc'); + const legacyCiphertext = cbc.encrypt('legacy-metric', instanceKey); const emitSpy = vi.spyOn(cipher.events, 'emit'); await cipher.decryptV2(legacyCiphertext); @@ -441,7 +417,7 @@ describe('Cipher', () => { it('should not let a throwing metrics listener break a successful decrypt', async () => { const keyId = 'test-uuid-listener'; const encryptedDataKey = cipher.encryptDEKWithInstanceKey(plaintextDataKey); - const ciphertext = cipher.encryptWithKey('safe', plaintextDataKey, 'aes-256-gcm'); + const ciphertext = gcm.encrypt('safe', plaintextDataKey); withProvider({ getKeyById: async (id: string) => id === keyId diff --git a/packages/core/src/encryption/cipher.ts b/packages/core/src/encryption/cipher.ts index 9c692659787..b1625b9a983 100644 --- a/packages/core/src/encryption/cipher.ts +++ b/packages/core/src/encryption/cipher.ts @@ -36,27 +36,14 @@ export class Cipher { * once per object spares a DEK unwrap on every legacy-format write. */ private readonly verifiedLegacyDescriptors = new WeakSet(); + private readonly cipherAES256GCM = new CipherAes256GCM(); + private readonly cipherAES256CBC = new CipherAes256CBC(); constructor( private readonly instanceSettings: InstanceSettings, - private readonly cipherAES256GCM: CipherAes256GCM, - private readonly cipherAES256CBC: CipherAes256CBC, private readonly encryptionKeyProxy: EncryptionKeyProxy, ) {} - /** @deprecated Use {@link encryptV2} instead, or {@link encryptWithKey} for an explicit key. */ - encrypt(data: string | object, customEncryptionKey?: string): string { - const key = customEncryptionKey ?? this.instanceSettings.encryptionKey; - const plaintext = typeof data === 'string' ? data : JSON.stringify(data); - return this.encryptWithKey(plaintext, key, 'aes-256-cbc'); - } - - /** @deprecated Use {@link decryptV2} instead, or {@link decryptWithKey} for an explicit key. */ - decrypt(data: string, customEncryptionKey?: string): string { - const key = customEncryptionKey ?? this.instanceSettings.encryptionKey; - return this.decryptWithKey(data, key, 'aes-256-cbc'); - } - /** * Encrypts with whatever active key the provider's descriptor names. The * descriptor carries the key, the algorithm, and the output format — the @@ -210,7 +197,7 @@ export class Cipher { return createHash('sha256').update(this.instanceSettings.encryptionKey).digest('hex'); } - encryptWithKey(data: string, key: string, algorithm: CipherAlgorithm): string { + private encryptWithKey(data: string, key: string, algorithm: CipherAlgorithm): string { switch (algorithm) { case 'aes-256-cbc': return this.cipherAES256CBC.encrypt(data, key); @@ -220,7 +207,7 @@ export class Cipher { assertUnreachable(algorithm); } - decryptWithKey(data: string, key: string, algorithm: CipherAlgorithm): string { + private decryptWithKey(data: string, key: string, algorithm: CipherAlgorithm): string { switch (algorithm) { case 'aes-256-cbc': return this.cipherAES256CBC.decrypt(data, key); diff --git a/packages/core/src/encryption/index.ts b/packages/core/src/encryption/index.ts index 4e9f93c2a4c..cb8edf62c13 100644 --- a/packages/core/src/encryption/index.ts +++ b/packages/core/src/encryption/index.ts @@ -1,6 +1,4 @@ export { Cipher } from './cipher'; -export { CipherAes256GCM } from './aes-256-gcm'; -export { CipherAes256CBC } from './aes-256-cbc'; export type { CipherAlgorithm } from './interface'; export { EncryptionKeyProxy } from './encryption-key-proxy'; export type { KeyInfo, KeyFormat, IEncryptionKeyProvider } from './encryption-key-proxy'; diff --git a/packages/testing/code-health/src/rules/encryption-boundary.rule.test.ts b/packages/testing/code-health/src/rules/encryption-boundary.rule.test.ts index 9287356b7c0..3ec9ef66415 100644 --- a/packages/testing/code-health/src/rules/encryption-boundary.rule.test.ts +++ b/packages/testing/code-health/src/rules/encryption-boundary.rule.test.ts @@ -121,7 +121,6 @@ describe('EncryptionBoundaryRule', () => { export const extra = { rules: { 'n8n-local-rules/no-deployment-key-delete': 'off', - 'n8n-local-rules/no-legacy-cipher-methods': ['warn'], 'n8n-local-rules/no-uncaught-json-parse': 'off', }, }; @@ -130,11 +129,9 @@ export const extra = { const violations = await analyze(); - expect(violations).toHaveLength(2); + expect(violations).toHaveLength(1); expect(violations[0]).toContain('eslint.config.mjs:7'); expect(violations[0]).toContain('no-deployment-key-delete'); - expect(violations[1]).toContain('eslint.config.mjs:8'); - expect(violations[1]).toContain('no-legacy-cipher-methods'); }); }); @@ -185,21 +182,20 @@ export const extra = { write( 'packages/a/src/index.ts', [ - '// oxlint-disable-next-line n8n-local-rules/no-legacy-cipher-methods', - 'export const a = 1; // oxlint-disable-line no-console, no-deployment-key-delete', + '// oxlint-disable-next-line n8n-local-rules/no-deployment-key-delete', + 'export const a = 1; // oxlint-disable-line no-console', '/* oxlint-disable */', - '/* oxlint n8n-local-rules/no-misplaced-cipher-primitives: "off" */', + '/* oxlint n8n-local-rules/no-encryption-guardrail-disable: "off" */', '', ].join('\n'), ); const violations = await analyze(); - expect(violations).toHaveLength(4); - expect(violations[0]).toContain('no-legacy-cipher-methods'); - expect(violations[1]).toContain('no-deployment-key-delete'); - expect(violations[2]).toContain('`oxlint-disable` directive silences every lint rule'); - expect(violations[3]).toContain('no-misplaced-cipher-primitives'); + expect(violations).toHaveLength(3); + expect(violations[0]).toContain('no-deployment-key-delete'); + expect(violations[1]).toContain('`oxlint-disable` directive silences every lint rule'); + expect(violations[2]).toContain('inline `oxlint` configuration comment'); }); it('accepts oxlint directives that name unrelated rules', async () => { @@ -219,9 +215,8 @@ export const extra = { write( 'packages/a/src/index.ts', [ - '// eslint-disable-next-line n8n-local-rules/no-legacy-cipher-methods', - 'export const a = 1; // eslint-disable-line no-console, no-deployment-key-delete', - '/* eslint-disable n8n-local-rules/no-misplaced-cipher-primitives */', + '// eslint-disable-next-line n8n-local-rules/no-deployment-key-delete', + 'export const a = 1; // eslint-disable-line no-console', '/* eslint n8n-local-rules/no-encryption-guardrail-disable: "off" */', '', ].join('\n'), @@ -229,11 +224,9 @@ export const extra = { const violations = await analyze(); - expect(violations).toHaveLength(4); - expect(violations[0]).toContain('no-legacy-cipher-methods'); - expect(violations[1]).toContain('no-deployment-key-delete'); - expect(violations[2]).toContain('no-misplaced-cipher-primitives'); - expect(violations[3]).toContain('inline `eslint` configuration comment'); + expect(violations).toHaveLength(2); + expect(violations[0]).toContain('no-deployment-key-delete'); + expect(violations[1]).toContain('inline `eslint` configuration comment'); }); it('ignores directive text inside string literals', async () => { diff --git a/packages/testing/code-health/src/rules/encryption-boundary.rule.ts b/packages/testing/code-health/src/rules/encryption-boundary.rule.ts index a7c9251039b..047b222e299 100644 --- a/packages/testing/code-health/src/rules/encryption-boundary.rule.ts +++ b/packages/testing/code-health/src/rules/encryption-boundary.rule.ts @@ -19,12 +19,7 @@ import { const DEFAULT_TRIGGER_DEPENDENCIES = ['n8n-core', '@n8n/db']; /** The rules `encryptionBoundaryConfig` enables, without the plugin prefix. */ -const DEFAULT_GUARDED_RULES = [ - 'no-legacy-cipher-methods', - 'no-misplaced-cipher-primitives', - 'no-deployment-key-delete', - 'no-encryption-guardrail-disable', -]; +const DEFAULT_GUARDED_RULES = ['no-deployment-key-delete', 'no-encryption-guardrail-disable']; const BOUNDARY_CONFIG_PATH = 'packages/@n8n/eslint-config/src/configs/encryption-boundary.ts'; diff --git a/scripts/lint-parity/expected-pr1.json b/scripts/lint-parity/expected-pr1.json index 0e10f6e6aa9..1def3a82a72 100644 --- a/scripts/lint-parity/expected-pr1.json +++ b/scripts/lint-parity/expected-pr1.json @@ -120,20 +120,6 @@ "after": "error", "reason": "package moved from base to the backend layer, which carries the network boundary" }, - { - "rule": "n8n-local-rules/no-legacy-cipher-methods", - "pkg": "packages/", - "before": "off", - "after": "error", - "reason": "package moved from base to the backend layer, which carries the encryption boundary" - }, - { - "rule": "n8n-local-rules/no-misplaced-cipher-primitives", - "pkg": "packages/", - "before": "off", - "after": "error", - "reason": "package moved from base to the backend layer, which carries the encryption boundary" - }, { "rule": "n8n-local-rules/no-deployment-key-delete", "pkg": "packages/", @@ -163,20 +149,6 @@ "after": "off", "reason": "runtime-agnostic library moved to base: no Node runtime, no HTTP imports" }, - { - "rule": "n8n-local-rules/no-legacy-cipher-methods", - "pkg": "packages/@n8n/utils", - "before": "error", - "after": "off", - "reason": "runtime-agnostic library moved to base" - }, - { - "rule": "n8n-local-rules/no-misplaced-cipher-primitives", - "pkg": "packages/@n8n/utils", - "before": "error", - "after": "off", - "reason": "runtime-agnostic library moved to base" - }, { "rule": "n8n-local-rules/no-deployment-key-delete", "pkg": "packages/@n8n/utils", @@ -198,20 +170,6 @@ "after": "off", "reason": "pure library moved to base: no Node APIs" }, - { - "rule": "n8n-local-rules/no-legacy-cipher-methods", - "pkg": "packages/@n8n/json-schema-to-zod", - "before": "error", - "after": "off", - "reason": "pure library moved to base" - }, - { - "rule": "n8n-local-rules/no-misplaced-cipher-primitives", - "pkg": "packages/@n8n/json-schema-to-zod", - "before": "error", - "after": "off", - "reason": "pure library moved to base" - }, { "rule": "n8n-local-rules/no-deployment-key-delete", "pkg": "packages/@n8n/json-schema-to-zod", @@ -233,20 +191,6 @@ "after": "off", "reason": "yjs-only library moved to base: no Node APIs" }, - { - "rule": "n8n-local-rules/no-legacy-cipher-methods", - "pkg": "packages/@n8n/crdt", - "before": "error", - "after": "off", - "reason": "yjs-only library moved to base" - }, - { - "rule": "n8n-local-rules/no-misplaced-cipher-primitives", - "pkg": "packages/@n8n/crdt", - "before": "error", - "after": "off", - "reason": "yjs-only library moved to base" - }, { "rule": "n8n-local-rules/no-deployment-key-delete", "pkg": "packages/@n8n/crdt", diff --git a/scripts/lint-parity/oxlint-gap.json b/scripts/lint-parity/oxlint-gap.json index dc8e5769389..2e23a4e66c5 100644 --- a/scripts/lint-parity/oxlint-gap.json +++ b/scripts/lint-parity/oxlint-gap.json @@ -57,20 +57,6 @@ "impact": "The WorkflowEntity node-write seal is unenforced in migrated packages. None of the three spike packages depends on @n8n/db, but this blocks any package that does.", "replacement": "Move this type-aware functional guardrail to @n8n/code-health in the follow-up PR." }, - "n8n-local-rules/no-legacy-cipher-methods": { - "layer": "backend", - "native": null, - "reason": "Needs type information. Oxlint gives jsPlugin rules an empty parserServices object, so the rule cannot resolve a Cipher receiver.", - "impact": "The deprecated Cipher.encrypt and Cipher.decrypt APIs are not guarded by an Oxlint-only package.", - "replacement": "Move this type-aware functional guardrail to @n8n/code-health in the follow-up PR." - }, - "n8n-local-rules/no-misplaced-cipher-primitives": { - "layer": "backend", - "native": null, - "reason": "Needs type information. Oxlint gives jsPlugin rules an empty parserServices object, so the rule cannot resolve the imported cipher primitive.", - "impact": "Raw cipher primitive placement is not guarded by an Oxlint-only package.", - "replacement": "Move this type-aware functional guardrail to @n8n/code-health in the follow-up PR." - }, "n8n-local-rules/no-deployment-key-delete": { "layer": "backend", "native": null,