diff --git a/packages/cli/src/modules/type-availability-policies/README.md b/packages/cli/src/modules/type-availability-policies/README.md index 34ef2c33ec0..88d1b9b4478 100644 --- a/packages/cli/src/modules/type-availability-policies/README.md +++ b/packages/cli/src/modules/type-availability-policies/README.md @@ -1,8 +1,8 @@ # Node type availability policies Rules that say which node types a project may use. An admin writes the rules. The policy -infrastructure enforces them at the points that handle a workflow: save, publish, start, -transfer and import. +infrastructure enforces them at the points that handle a workflow — save, publish, start, +transfer and import — and at the point where a node asks for a credential. This module is the first tenant of that infrastructure. It adds a check and a store, and nothing else: no enforcement path of its own, no error shape, no audit line. Read @@ -27,13 +27,14 @@ The break-glass levers stay the ones the substrate documents: remove this module ## What each point decides -| Point | Scope it reads | Verdict | -| ------------------ | -------------------------------- | ------------------------------------------- | -| `workflowSave` | the workflow's project | violations for node **types** the save adds | -| `workflowPublish` | the workflow's project | every blocked type in the workflow | -| `workflowStart` | the workflow's project | every blocked type in the workflow | -| `workflowTransfer` | the **target** project | every blocked type in the workflow | -| `contentImport` | the project the content lands in | every blocked type in the workflow | +| Point | Scope it reads | Verdict | +| ------------------- | -------------------------------- | ---------------------------------------------- | +| `workflowSave` | the workflow's project | violations for node **types** the save adds | +| `workflowPublish` | the workflow's project | every blocked type in the workflow | +| `workflowStart` | the workflow's project | every blocked type in the workflow | +| `workflowTransfer` | the **target** project | every blocked type in the workflow | +| `contentImport` | the project the content lands in | every blocked type in the workflow | +| `credentialDecrypt` | the executing project | the type of the node asking for the credential | The check reads the license again per decision. A license that lapses while the instance runs stops enforcement, which matches the routes that author the policy: an admin who can no longer @@ -66,6 +67,26 @@ does not travel into a project whose rules the content has never met. A node counts even when it is disabled. Exempting disabled nodes would open a two-step path around the save diff: add the node disabled, then enable it once its type is stored. +## The credential lock + +`credentialDecrypt` is the one point that reads no workflow. A node asks for a credential, and +the verdict is about **the node asking**: a Slack credential is refused to a blocked Slack node +and handed to an allowed HTTP Request. The credential's own type is never evaluated. Deriving +availability from "some usable node accepts this type" is close to vacuous, because HTTP +Request accepts almost any credential, and it would let installing an unrelated node package +change which credentials work. + +`workflowStart` already fails a run that carries a blocked type, so this point is there for the +decryptions no run covers: + +- a parameter dropdown loading its options, which decrypts without starting an execution; +- a node the workflow points cannot see, such as one inside a workflow carried in another + node's parameters. + +A refusal fails the decryption with the same `node-type-unavailable` violation the workflow +points report, naming the **node type** as `subject`. The audit line carries the credential's +id and type from the context. + ## What a violation looks like One violation per blocked type, deduplicated, in the order the types first appear: @@ -144,6 +165,11 @@ through a sealed repository method, and the lint rule that guards that has no al deny `n8n-nodes-base.gmailTool`. The registry holds them as two types. - **A workflow carried inside a node's parameters is not read.** The check reads `workflow.nodes`. Node types inside an inline sub-workflow definition are invisible to it. + The credential lock still catches such a node once it asks for a credential. +- **A decryption with no consumer is not policed.** `consumer` is `null` wherever no node is + asking — an OAuth authorize or revoke, the agents adapter, a log-streaming destination. There + is no node type to evaluate there, and the credential's type is deliberately not a stand-in, + so the decryption goes through. - **Cache staleness is bounded by the TTL, not eliminated.** A write drops the cached scope it changed, which is enough wherever the processes share one Redis cache — which is every deployment that has more than one, unless `N8N_CACHE_BACKEND=memory` is set by hand in queue @@ -176,7 +202,7 @@ through a sealed repository method, and the lint rule that guards that has no al | File | Role | | ------------------------------------------------- | ------------------------------------------------------------------------------- | -| `node-type-policy.check.ts` | The `@PolicyCheck()` class: the five points, the save diff, the violations | +| `node-type-policy.check.ts` | The `@PolicyCheck()` class: the six points, the save diff, the violations | | `policy-evaluator.ts` | Pure evaluation: first match per scope, then the instance ∩ project composition | | `policy-shadow-lint.ts` | Warns at write time about rules an earlier rule already covers | | `type-availability-policy.service.ts` | Reads and writes the store, with versioning and row locks | diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/node-type-policy.check.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/node-type-policy.check.test.ts index 976937b6b53..5e78a954734 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/node-type-policy.check.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/node-type-policy.check.test.ts @@ -1,5 +1,9 @@ import type { LicenseState } from '@n8n/backend-common'; -import type { ContentImportTransport, PolicedWorkflow } from '@n8n/decorators'; +import type { + ContentImportTransport, + CredentialDecryptContext, + PolicedWorkflow, +} from '@n8n/decorators'; import type { INode } from 'n8n-workflow'; import { mock } from 'vitest-mock-extended'; @@ -13,6 +17,14 @@ import type { const SLACK = 'n8n-nodes-base.slack'; const GMAIL = 'n8n-nodes-base.gmail'; const SET = 'n8n-nodes-base.set'; +const HTTP_REQUEST = 'n8n-nodes-base.httpRequest'; + +const slackCredentialFor = (nodeType: string | null): CredentialDecryptContext => ({ + credentialType: 'slackApi', + credentialId: 'cred-1', + consumer: nodeType === null ? null : { nodeType }, + projectId: 'project-1', +}); const node = (type: string, name = type, disabled = false): INode => ({ id: name, name, type, typeVersion: 1, position: [0, 0], parameters: {}, disabled }) as INode; @@ -193,6 +205,51 @@ describe('NodeTypePolicyCheck', () => { }); }); + describe('onCredentialDecrypt', () => { + it('vetoes the decryption when the node asking is a denied type', async () => { + const result = await check.onCredentialDecrypt(slackCredentialFor(SLACK)); + + expect(result.violations[0]).toStrictEqual({ + kind: 'node-type-unavailable', + checkId: 'node-type-availability', + message: `Node type "${SLACK}" is blocked by an instance policy`, + subject: SLACK, + subjectType: 'nodeType', + scope: 'instance', + matchedRuleId: 'rule-7', + }); + expect(service.evaluateComposedTypesFor).toHaveBeenCalledWith('node-types', 'project-1', [ + SLACK, + ]); + }); + + it('hands the same credential to a node whose type is available', async () => { + const result = await check.onCredentialDecrypt(slackCredentialFor(HTTP_REQUEST)); + + expect(result.violations).toEqual([]); + expect(service.evaluateComposedTypesFor).toHaveBeenCalledWith('node-types', 'project-1', [ + HTTP_REQUEST, + ]); + }); + + it('reports nothing when no node is asking, and never reads the policy', async () => { + const result = await check.onCredentialDecrypt(slackCredentialFor(null)); + + expect(result.violations).toEqual([]); + expect(service.evaluateComposedTypesFor).not.toHaveBeenCalled(); + }); + + it('evaluates instance-only when the execution has no project', async () => { + const result = await check.onCredentialDecrypt({ + ...slackCredentialFor(SLACK), + projectId: null, + }); + + expect(service.evaluateComposedTypesFor).toHaveBeenCalledWith('node-types', null, [SLACK]); + expect(result.violations).toHaveLength(1); + }); + }); + describe('scope resolution', () => { it('evaluates instance-only when no project applies', async () => { const result = await check.onWorkflowStart({ @@ -269,6 +326,15 @@ describe('NodeTypePolicyCheck', () => { expect(service.evaluateComposedTypesFor).not.toHaveBeenCalled(); }); + it('reports nothing on a credential decrypt without a license', async () => { + licenseState.isLicensed.mockReturnValue(false); + + const result = await check.onCredentialDecrypt(slackCredentialFor(SLACK)); + + expect(result.violations).toEqual([]); + expect(service.evaluateComposedTypesFor).not.toHaveBeenCalled(); + }); + it('reports nothing for a workflow with no nodes', async () => { const result = await check.onWorkflowSave({ workflow: workflow([], null), diff --git a/packages/cli/src/modules/type-availability-policies/node-type-policy.check.ts b/packages/cli/src/modules/type-availability-policies/node-type-policy.check.ts index eef3bb930ef..cf1f68853de 100644 --- a/packages/cli/src/modules/type-availability-policies/node-type-policy.check.ts +++ b/packages/cli/src/modules/type-availability-policies/node-type-policy.check.ts @@ -2,6 +2,7 @@ import { LicenseState } from '@n8n/backend-common'; import { LICENSE_FEATURES } from '@n8n/constants'; import type { ContentImportContext, + CredentialDecryptContext, PolicedWorkflow, PolicyCheckResult, PolicyViolation, @@ -106,20 +107,40 @@ export class NodeTypePolicyCheck implements RegisteredPolicyCheck { } /** - * No `signal` is threaded: nothing below accepts one (TypeORM does not), and the deadline - * race in the decision service already turns an overrun into a check failure. + * Locks on the node asking, never on the credential's type: the same credential is refused + * to a blocked Slack node and handed to an allowed HTTP Request. This catches what a run + * cannot — a dropdown loading its options, a node inside an inline workflow definition. */ + async onCredentialDecrypt({ + consumer, + projectId, + }: CredentialDecryptContext): Promise { + // No node is asking — an OAuth flow, a credential test — so there is no consumer to + // police. Deriving a verdict from the credential's type instead is what the RFC rejects. + if (consumer === null) return NO_VIOLATIONS; + + return await this.checkTypes([consumer.nodeType], projectId); + } + private async check( workflow: PolicedWorkflow, projectId: string | null, grandfathered: ReadonlySet, ): Promise { + const types = distinctTypes(workflow.nodes).filter((type) => !grandfathered.has(type)); + + return await this.checkTypes(types, projectId); + } + + /** + * No `signal` is threaded: nothing below accepts one (TypeORM does not), and the deadline + * race in the decision service already turns an overrun into a check failure. + */ + private async checkTypes(types: string[], projectId: string | null): Promise { // An expired license stops enforcing, matching the `@Licensed` routes that author the // policy: a customer who cannot edit the policy must not keep being blocked by it. if (!this.licenseState.isLicensed(LICENSE_FEATURES.NODE_TYPE_POLICIES)) return NO_VIOLATIONS; - const types = distinctTypes(workflow.nodes).filter((type) => !grandfathered.has(type)); - if (types.length === 0) return NO_VIOLATIONS; const { verdicts, versions } = await this.service.evaluateComposedTypesFor( diff --git a/packages/cli/test/integration/policy/node-type-policy.credential-decrypt.test.ts b/packages/cli/test/integration/policy/node-type-policy.credential-decrypt.test.ts new file mode 100644 index 00000000000..889d87bde99 --- /dev/null +++ b/packages/cli/test/integration/policy/node-type-policy.credential-decrypt.test.ts @@ -0,0 +1,132 @@ +/** + * Pins the credential lock on the real path: the real store behind the real `@PolicyCheck()`, + * with `CredentialsHelper.getDecrypted` as the host. The pair of cases is the point — a lock + * keyed on the credential's type would pass the refusal and fail the hand-over. + */ +import { testDb } from '@n8n/backend-test-utils'; +import { LICENSE_FEATURES } from '@n8n/constants'; +import { ProjectRepository, type User } from '@n8n/db'; +import { PolicyCheckMetadata } from '@n8n/decorators'; +import { Container } from '@n8n/di'; +import type { IExecuteData, IWorkflowExecuteAdditionalData } from 'n8n-workflow'; + +import { CredentialsHelper } from '@/credentials-helper'; + +import { saveCredential } from '../shared/db/credentials'; +import { createOwner } from '../shared/db/users'; +import type { SuperAgentTest } from '../shared/types'; +import * as utils from '../shared/utils/'; +import { clearPolicyCache } from './shared/policy-cache'; + +const CHECK_ID = 'node-type-availability'; + +const SLACK = 'n8n-nodes-base.slack'; +const HTTP_REQUEST = 'n8n-nodes-base.httpRequest'; + +const testServer = utils.setupTestServer({ + endpointGroups: ['type-availability-policies'], + modules: ['policy-infrastructure', 'type-availability-policies'], + enabledFeatures: [LICENSE_FEATURES.NODE_TYPE_POLICIES], +}); + +let owner: User; +let ownerAgent: SuperAgentTest; +let additionalData: IWorkflowExecuteAdditionalData; + +/** Only `node` is read: the helper passes it on as the check's consumer. */ +const askingAs = (nodeType: string) => ({ node: { type: nodeType } }) as IExecuteData; + +/** + * `raw` returns the stored data before overwrites and defaults, which keeps the credential type + * out of the node registry — the check reads the asking node's type, never the credential's. + */ +async function decryptAskingAs(nodeType?: string) { + const credential = await saveCredential( + { name: 'Slack account', type: 'slackApi', data: { accessToken: 'secret' } }, + { user: owner, role: 'credential:owner' }, + ); + + return await Container.get(CredentialsHelper).getDecrypted( + additionalData, + { id: credential.id, name: credential.name }, + credential.type, + 'internal', + nodeType === undefined ? undefined : askingAs(nodeType), + true, + ); +} + +async function denySlackAtInstanceScope() { + const response = await ownerAgent.put('/node-type-policies/instance').send({ + rules: [{ id: 'deny-slack', action: 'deny', selector: { kind: 'name', value: SLACK } }], + defaultAction: 'allow', + version: 0, + }); + + expect(response.statusCode).toBe(200); +} + +beforeAll(async () => { + // Asserted by id, not by importing the class: the import would register the check itself, + // and the refusal below would pass with the module no longer registering it. + const registeredIds = Container.get(PolicyCheckMetadata) + .getClasses() + .map((checkClass) => Container.get(checkClass).id); + expect(registeredIds).toContain(CHECK_ID); + + owner = await createOwner(); + ownerAgent = testServer.authAgentFor(owner); + + const personalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( + owner.id, + ); + additionalData = { projectId: personalProject.id } as IWorkflowExecuteAdditionalData; +}); + +afterEach(async () => { + await testDb.truncate([ + 'TypeAvailabilityPolicyAttachment', + 'TypeAvailabilityPolicyScope', + 'TypeAvailabilityPolicy', + 'SharedCredentials', + 'CredentialsEntity', + ]); + await clearPolicyCache(); +}); + +describe('credential lock', () => { + test('refuses to decrypt for a node type the policy blocks', async () => { + await denySlackAtInstanceScope(); + + await expect(decryptAskingAs(SLACK)).rejects.toMatchObject({ + httpStatusCode: 403, + violations: [ + { + kind: 'node-type-unavailable', + checkId: CHECK_ID, + message: `Node type "${SLACK}" is blocked by an instance policy`, + subject: SLACK, + subjectType: 'nodeType', + scope: 'instance', + matchedRuleId: 'deny-slack', + }, + ], + }); + }); + + test('hands the same credential to a node type the policy allows', async () => { + await denySlackAtInstanceScope(); + + await expect(decryptAskingAs(HTTP_REQUEST)).resolves.toEqual({ accessToken: 'secret' }); + }); + + test('decrypts when no node is asking, because the lock has no consumer to read', async () => { + await denySlackAtInstanceScope(); + + await expect(decryptAskingAs()).resolves.toEqual({ accessToken: 'secret' }); + }); + + test('decrypts for any node type while no rule denies one', async () => { + await expect(decryptAskingAs(SLACK)).resolves.toEqual({ accessToken: 'secret' }); + }); +});