From b33b0db040e782eea791b4ecdfe40fc54862c8a5 Mon Sep 17 00:00:00 2001 From: Guillaume Jacquart Date: Fri, 9 Oct 2026 14:33:37 +0000 Subject: [PATCH] feat(core): Add an extends selector to credential type policies (no-changelog) (#40305) Co-authored-by: Claude Opus 5.5 (1M context) --- packages/@n8n/api-types/src/dto/index.ts | 3 + .../__tests__/policy-rule.schema.test.ts | 21 +- .../__tests__/policy-rule.types.test.ts | 19 ++ .../__tests__/policy-selector.schema.test.ts | 12 ++ .../policy-rule.schema.ts | 6 +- .../policy-rule.types.ts | 22 +- .../policy-selector.schema.ts | 10 +- .../src/events/type-availability-policies.ts | 3 + .../__tests__/telemetry-event-relay.test.ts | 36 ++++ .../events/relays/telemetry.event-relay.ts | 3 +- .../type-availability-policies/README.md | 8 + .../__tests__/package-resolver.test.ts | 81 ++++++- .../__tests__/policy-evaluator.test.ts | 112 ++++++++++ .../__tests__/policy-shadow-lint.test.ts | 51 +++++ .../type-availability-policy.service.test.ts | 61 ++++++ .../package-resolver.ts | 52 ++++- .../policy-evaluator.ts | 9 +- .../policy-shadow-lint.ts | 49 +++-- .../type-availability-policy.service.ts | 50 ++++- ...de-type-policies.public.controller.test.ts | 11 + .../node-type-policies.public.controller.ts | 24 ++- ...CredentialTypePolicyDocument.generated.yml | 13 ++ ...CredentialTypePolicyDocument.generated.yml | 13 ++ ...redentialTypePolicyDocuments.generated.yml | 13 ++ ...CredentialTypeInstancePolicy.generated.yml | 13 ++ ...tCredentialTypeProjectPolicy.generated.yml | 13 ++ ...dentialTypePolicyAttachments.generated.yml | 13 ++ ...CredentialTypePolicyDocument.generated.yml | 13 ++ ...DocumentWriteResultPublicDto.generated.yml | 13 ++ ...TypePolicyEffectivePublicDto.generated.yml | 13 ++ ...ffectiveWriteResultPublicDto.generated.yml | 13 ++ .../credential-type-policy.extends.test.ts | 202 ++++++++++++++++++ .../credential-type-policies.test.ts | 52 +++++ .../public-api/node-type-policies.test.ts | 16 ++ 34 files changed, 996 insertions(+), 47 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.types.test.ts create mode 100644 packages/cli/test/integration/policy/credential-type-policy.extends.test.ts diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index f45d5f79564..98abcfd7527 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -354,12 +354,15 @@ export { export { PaginationDto, MAX_ITEMS_PER_PAGE } from './pagination/pagination.dto'; export type { + NodeTypePolicyRule, + NodeTypePolicySelector, NonDelegatingPolicyAction, NonDelegatingPolicyRule, PolicyAction, PolicyRule, PolicySelector, } from './type-availability-policies/policy-rule.types'; +export { isNodeTypePolicyRule } from './type-availability-policies/policy-rule.types'; export { credentialTypePolicySelectorSchema, nodeTypePolicySelectorSchema, diff --git a/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.schema.test.ts b/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.schema.test.ts index 199390490fd..9ffa97d2452 100644 --- a/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.schema.test.ts +++ b/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.schema.test.ts @@ -112,20 +112,29 @@ describe('nonDelegatingPolicyRuleListSchema', () => { }); describe('rule schemas per kind', () => { - const packageRule = { + const extendsRule = { id: 'r1', action: 'deny', - selector: { kind: 'package', value: 'n8n-nodes-base' }, + selector: { kind: 'extends', value: 'oAuth2Api' }, }; - it('accepts the same selectors for both kinds', () => { - expect(nodeTypePolicyRuleSchemas.ruleList.safeParse([packageRule]).success).toBe(true); - expect(credentialTypePolicyRuleSchemas.ruleList.safeParse([packageRule]).success).toBe(true); + it('rejects an extends rule in a node type policy', () => { + expect(nodeTypePolicyRuleSchemas.ruleList.safeParse([extendsRule]).success).toBe(false); + expect(nodeTypePolicyRuleSchemas.nonDelegatingRuleList.safeParse([extendsRule]).success).toBe( + false, + ); + }); + + it('accepts an extends rule in a credential type policy', () => { + expect(credentialTypePolicyRuleSchemas.ruleList.safeParse([extendsRule]).success).toBe(true); + expect( + credentialTypePolicyRuleSchemas.nonDelegatingRuleList.safeParse([extendsRule]).success, + ).toBe(true); }); it('rejects duplicate rule ids in a credential type policy', () => { expect( - credentialTypePolicyRuleSchemas.ruleList.safeParse([packageRule, packageRule]).success, + credentialTypePolicyRuleSchemas.ruleList.safeParse([extendsRule, extendsRule]).success, ).toBe(false); }); }); diff --git a/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.types.test.ts b/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.types.test.ts new file mode 100644 index 00000000000..dda2d4a19fa --- /dev/null +++ b/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-rule.types.test.ts @@ -0,0 +1,19 @@ +import { isNodeTypePolicyRule } from '../policy-rule.types'; + +describe('isNodeTypePolicyRule', () => { + it.each(['name', 'package'] as const)('accepts a %s rule', (kind) => { + expect(isNodeTypePolicyRule({ id: 'r1', action: 'deny', selector: { kind, value: 'x' } })).toBe( + true, + ); + }); + + it('rejects an extends rule, which only credential type policies accept', () => { + expect( + isNodeTypePolicyRule({ + id: 'r1', + action: 'deny', + selector: { kind: 'extends', value: 'oAuth2Api' }, + }), + ).toBe(false); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-selector.schema.test.ts b/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-selector.schema.test.ts index ae406774c01..96cb63f799f 100644 --- a/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-selector.schema.test.ts +++ b/packages/@n8n/api-types/src/dto/type-availability-policies/__tests__/policy-selector.schema.test.ts @@ -27,3 +27,15 @@ describe.each([ expect(schema.safeParse({ kind: 'name' }).success).toBe(false); }); }); + +describe('extends selector', () => { + const selector = { kind: 'extends', value: 'oAuth2Api' }; + + it('is rejected by the node type schema', () => { + expect(nodeTypePolicySelectorSchema.safeParse(selector).success).toBe(false); + }); + + it('is accepted by the credential type schema', () => { + expect(credentialTypePolicySelectorSchema.safeParse(selector).success).toBe(true); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.schema.ts b/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.schema.ts index 422ac974914..aff5c46e321 100644 --- a/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.schema.ts +++ b/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.schema.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; import type { + NodeTypePolicyRule, NonDelegatingPolicyAction, NonDelegatingPolicyRule, PolicyAction, @@ -70,7 +71,10 @@ type RuleSchemas = { export const nodeTypePolicyRuleSchemas = ruleSchemasFor( nodeTypePolicySelectorSchema, -) satisfies RuleSchemas; +) satisfies RuleSchemas< + NodeTypePolicyRule, + NodeTypePolicyRule & { readonly action: NonDelegatingPolicyAction } +>; export const credentialTypePolicyRuleSchemas = ruleSchemasFor( credentialTypePolicySelectorSchema, diff --git a/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.types.ts b/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.types.ts index 645b19fb0e6..1549e2b2c04 100644 --- a/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.types.ts +++ b/packages/@n8n/api-types/src/dto/type-availability-policies/policy-rule.types.ts @@ -1,12 +1,19 @@ /** - * Domain types for node type availability policies, shared by the internal and public API + * Domain types for type availability policies, shared by the internal and public API * request DTOs and by the backend evaluator. */ -/** Matches a node type by its exact full name, or by its package segment. */ +/** + * Matches a type by its exact full name, or by its package segment. `extends` is for credential + * types only: it matches the named type and every type built on it through `extends`. + */ export type PolicySelector = | { readonly kind: 'name'; readonly value: string } - | { readonly kind: 'package'; readonly value: string }; + | { readonly kind: 'package'; readonly value: string } + | { readonly kind: 'extends'; readonly value: string }; + +/** The selectors a node type policy accepts. */ +export type NodeTypePolicySelector = Exclude; export type PolicyAction = 'allow' | 'deny' | 'delegate'; @@ -17,6 +24,15 @@ export type PolicyRule = { readonly selector: PolicySelector; }; +/** A `PolicyRule` as a node type policy accepts it. */ +export type NodeTypePolicyRule = Omit & { + readonly selector: NodeTypePolicySelector; +}; + +export function isNodeTypePolicyRule(rule: PolicyRule): rule is NodeTypePolicyRule { + return rule.selector.kind !== 'extends'; +} + /** * The actions a project scope accepts. `delegate` defers to a narrower scope, and none exists * below project, so project-scope writes never accept it. diff --git a/packages/@n8n/api-types/src/dto/type-availability-policies/policy-selector.schema.ts b/packages/@n8n/api-types/src/dto/type-availability-policies/policy-selector.schema.ts index 7b2cc650c73..6d080500fd0 100644 --- a/packages/@n8n/api-types/src/dto/type-availability-policies/policy-selector.schema.ts +++ b/packages/@n8n/api-types/src/dto/type-availability-policies/policy-selector.schema.ts @@ -1,18 +1,22 @@ import { z } from 'zod'; -import type { PolicySelector } from './policy-rule.types'; +import type { NodeTypePolicySelector, PolicySelector } from './policy-rule.types'; const nameSelectorSchema = z.object({ kind: z.literal('name'), value: z.string().min(1) }); const packageSelectorSchema = z.object({ kind: z.literal('package'), value: z.string().min(1) }); +const extendsSelectorSchema = z + .object({ kind: z.literal('extends'), value: z.string().min(1) }) + .describe('Matches the named credential type and every type that extends it'); /** The selectors a node type policy accepts. `satisfies` keeps it honest against the type. */ export const nodeTypePolicySelectorSchema = z.discriminatedUnion('kind', [ nameSelectorSchema, packageSelectorSchema, -]) satisfies z.ZodType; +]) satisfies z.ZodType; -/** The selectors a credential type policy accepts. Each kind owns its list. */ +/** The selectors a credential type policy accepts: the node type ones plus `extends`. */ export const credentialTypePolicySelectorSchema = z.discriminatedUnion('kind', [ nameSelectorSchema, packageSelectorSchema, + extendsSelectorSchema, ]) satisfies z.ZodType; diff --git a/packages/@n8n/telemetry/src/events/type-availability-policies.ts b/packages/@n8n/telemetry/src/events/type-availability-policies.ts index f7b8c3ee444..0b18c02d6b6 100644 --- a/packages/@n8n/telemetry/src/events/type-availability-policies.ts +++ b/packages/@n8n/telemetry/src/events/type-availability-policies.ts @@ -65,6 +65,9 @@ export const TYPE_AVAILABILITY_POLICIES_TELEMETRY = defineTelemetryEvents({ ...ruleCounts, name_selector_count: z.number().describe('Rules that target one exact type name'), package_selector_count: z.number().describe('Rules that target a whole package'), + extends_selector_count: z + .number() + .describe('Rules that target a credential type and every type built on it'), evaluated_type_count: z .number() .describe( diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index cf6c3d7a718..7850b4ac2a7 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -802,6 +802,7 @@ describe('TelemetryEventRelay', () => { delegate_rule_count: 1, name_selector_count: 2, package_selector_count: 1, + extends_selector_count: 0, evaluated_type_count: 3, blocked_type_count: 2, allowed_type_count: 1, @@ -849,6 +850,41 @@ describe('TelemetryEventRelay', () => { ); }); + it('should count a credential type built on the base an extends rule names as blocked', () => { + Object.defineProperty(loadNodesAndCredentials, 'knownCredentials', { + configurable: true, + value: { + oAuth2Api: {}, + googleOAuth2Api: { extends: ['oAuth2Api'] }, + googleSheetsOAuth2Api: { extends: ['googleOAuth2Api'] }, + slackApi: {}, + }, + }); + + eventService.emit('node-type-policy-saved', { + updatedBy: 'user123', + kind: 'credential-types', + projectId: null, + scopeId: 'scope-1', + before: null, + after: { defaultAction: 'allow', version: 1 }, + rulesBefore: null, + rulesAfter: [ + { id: 'rule-1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } }, + ], + warningCount: 0, + }); + + expect(telemetry.track).toHaveBeenCalledWith( + TELEMETRY_EVENT.TYPE_AVAILABILITY_POLICIES.USER_SAVED_TYPE_AVAILABILITY_POLICY, + expect.objectContaining({ + extends_selector_count: 1, + blocked_types: ['oAuth2Api', 'googleOAuth2Api', 'googleSheetsOAuth2Api'], + allowed_types: ['slackApi'], + }), + ); + }); + it('should expand a credential type package rule using the credential loader package, not a dotted-type prefix', () => { const credentialPackageDenyRule: PolicyRule = { id: 'rule-1', diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index dfc43248e2f..182c6900d92 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -160,6 +160,7 @@ function countSelectorKinds(rules: readonly PolicyRule[]) { return { name_selector_count: rules.filter((rule) => rule.selector.kind === 'name').length, package_selector_count: rules.filter((rule) => rule.selector.kind === 'package').length, + extends_selector_count: rules.filter((rule) => rule.selector.kind === 'extends').length, }; } @@ -586,7 +587,7 @@ export class TelemetryEventRelay extends EventRelay { kind === CREDENTIAL_TYPES_KIND ? Object.keys(this.loadNodesAndCredentials.knownCredentials) : Object.keys(this.nodeTypes.getKnownTypes()); - const types = typeNames.map(policedTypeFor(kind, this.nodeTypes)); + const types = typeNames.map(policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials)); const resolvePackage = packageResolverFor(kind, this.loadNodesAndCredentials); this.telemetry.track( diff --git a/packages/cli/src/modules/type-availability-policies/README.md b/packages/cli/src/modules/type-availability-policies/README.md index 1c5bc145085..17613dad6dd 100644 --- a/packages/cli/src/modules/type-availability-policies/README.md +++ b/packages/cli/src/modules/type-availability-policies/README.md @@ -206,6 +206,14 @@ lint warns about it. The verdict names the variant the user placed and the rule Grandfathering compares literal type names, so adding `gmailTool` to a workflow that already stores `gmail` is a new type and is judged. +A credential type policy has a third selector, `extends`. It matches the named type and every +type built on it through the credential's `extends` list, at any depth: `extends oAuth2Api` +covers `googleOAuth2Api` and `googleSheetsOAuth2Api`. A `name` rule stays exact, so +`name oAuth2Api` blocks only the generic OAuth2 credential. Rule order decides as usual: an +`allow name googleSheetsOAuth2Api` placed before `deny extends oAuth2Api` keeps Sheets usable, +and placed after it, the shadow lint warns that it can never match. A write rejects an `extends` +rule in a node type policy, and one that names a credential type that is not installed. + ### Reading it on the execution path `workflowStart` runs for every execution and every sub-execution, under a 250 ms deadline it diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/package-resolver.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/package-resolver.test.ts index 3a0a2479fdf..72940387314 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/package-resolver.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/package-resolver.test.ts @@ -1,7 +1,13 @@ import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; +import type { NodeTypes } from '@/node-types'; import { CREDENTIAL_TYPES_KIND, NODE_TYPES_KIND } from '../constants'; -import { isPackageInstalled, packageResolverFor } from '../package-resolver'; +import { + isCredentialTypeKnown, + isPackageInstalled, + packageResolverFor, + policedTypeFor, +} from '../package-resolver'; /** A minimal stand-in for `LoadNodesAndCredentials`, built from just its public `loaders`. */ function makeRegistry(loaders: LoadNodesAndCredentials['loaders']): LoadNodesAndCredentials { @@ -89,3 +95,76 @@ describe('isPackageInstalled', () => { expect(isPackageInstalled(makeRegistry({}), 'constructor')).toBe(false); }); }); + +describe('policedTypeFor', () => { + const withKnownCredentials = ( + knownCredentials: LoadNodesAndCredentials['knownCredentials'], + ): LoadNodesAndCredentials => ({ knownCredentials }) as LoadNodesAndCredentials; + const nodeTypes = {} as NodeTypes; + + it('lists every credential type a type is built on, nearest first', () => { + const policedType = policedTypeFor( + CREDENTIAL_TYPES_KIND, + nodeTypes, + withKnownCredentials({ + oAuth2Api: { className: '', sourcePath: '' }, + googleOAuth2Api: { className: '', sourcePath: '', extends: ['oAuth2Api'] }, + googleSheetsOAuth2Api: { className: '', sourcePath: '', extends: ['googleOAuth2Api'] }, + }), + ); + + expect(policedType('googleSheetsOAuth2Api')).toEqual({ + name: 'googleSheetsOAuth2Api', + baseName: 'googleSheetsOAuth2Api', + ancestors: ['googleOAuth2Api', 'oAuth2Api'], + }); + }); + + it('lists a shared base once when two parents build on it', () => { + const policedType = policedTypeFor( + CREDENTIAL_TYPES_KIND, + nodeTypes, + withKnownCredentials({ + left: { className: '', sourcePath: '', extends: ['root'] }, + right: { className: '', sourcePath: '', extends: ['root'] }, + both: { className: '', sourcePath: '', extends: ['left', 'right'] }, + }), + ); + + expect(policedType('both').ancestors).toEqual(['left', 'right', 'root']); + }); + + it('stops on a cycle instead of looping', () => { + const policedType = policedTypeFor( + CREDENTIAL_TYPES_KIND, + nodeTypes, + withKnownCredentials({ + aApi: { className: '', sourcePath: '', extends: ['bApi'] }, + bApi: { className: '', sourcePath: '', extends: ['aApi'] }, + }), + ); + + expect(policedType('aApi').ancestors).toEqual(['bApi']); + }); + + it('gives an unknown credential type no ancestors', () => { + const policedType = policedTypeFor(CREDENTIAL_TYPES_KIND, nodeTypes, withKnownCredentials({})); + + expect(policedType('toString').ancestors).toEqual([]); + }); +}); + +describe('isCredentialTypeKnown', () => { + const registry = { + knownCredentials: { slackApi: { className: '', sourcePath: '' } }, + } as unknown as LoadNodesAndCredentials; + + it('is true for a known credential type', () => { + expect(isCredentialTypeKnown(registry, 'slackApi')).toBe(true); + }); + + it('is false for an unknown type, or one named after an Object.prototype property', () => { + expect(isCredentialTypeKnown(registry, 'unknownApi')).toBe(false); + expect(isCredentialTypeKnown(registry, 'constructor')).toBe(false); + }); +}); diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts index c15790b9c14..b9c7c1a4c01 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts @@ -253,6 +253,118 @@ describe('evaluateType', () => { }); }); +describe('extends selector', () => { + const SHEETS: PolicedType = { + name: 'googleSheetsOAuth2Api', + baseName: 'googleSheetsOAuth2Api', + ancestors: ['googleOAuth2Api', 'oAuth2Api'], + }; + const GENERIC: PolicedType = { name: 'oAuth2Api', baseName: 'oAuth2Api', ancestors: [] }; + const denyOAuth2Family: PolicyRule = { + id: 'deny-oauth2-family', + action: 'deny', + selector: { kind: 'extends', value: 'oAuth2Api' }, + }; + const allowSheets: PolicyRule = { + id: 'allow-sheets', + action: 'allow', + selector: { kind: 'name', value: 'googleSheetsOAuth2Api' }, + }; + + it('matches a type built on the named base, through every step of the chain', () => { + const attachments = [attachment({ rules: [denyOAuth2Family] })]; + + expect(evaluateType(attachments, 'allow', SHEETS)).toEqual({ + action: 'deny', + matchedRuleId: 'deny-oauth2-family', + }); + }); + + it('matches the named base itself', () => { + const attachments = [attachment({ rules: [denyOAuth2Family] })]; + + expect(evaluateType(attachments, 'allow', GENERIC).action).toBe('deny'); + }); + + it('does not match a type outside the family', () => { + const attachments = [attachment({ rules: [denyOAuth2Family] })]; + + expect(evaluateType(attachments, 'allow', type('slackApi'))).toEqual({ + action: 'allow', + matchedRuleId: null, + }); + }); + + it('leaves a name rule on the base exact, so derived types stay allowed', () => { + const attachments = [ + attachment({ + rules: [ + { id: 'deny-generic', action: 'deny', selector: { kind: 'name', value: 'oAuth2Api' } }, + ], + }), + ]; + + expect(evaluateType(attachments, 'allow', SHEETS).action).toBe('allow'); + expect(evaluateType(attachments, 'allow', GENERIC).action).toBe('deny'); + }); + + it('lets rule order decide between a derived-type allow and a family deny', () => { + const allowFirst = [attachment({ rules: [allowSheets, denyOAuth2Family] })]; + const denyFirst = [attachment({ rules: [denyOAuth2Family, allowSheets] })]; + + expect(evaluateType(allowFirst, 'allow', SHEETS)).toEqual({ + action: 'allow', + matchedRuleId: 'allow-sheets', + }); + expect(evaluateType(denyFirst, 'allow', SHEETS)).toEqual({ + action: 'deny', + matchedRuleId: 'deny-oauth2-family', + }); + }); + + it('keeps an instance family deny over a project allow of the derived type', () => { + const verdict = evaluateComposedType( + scopePolicy({ attachments: [attachment({ rules: [denyOAuth2Family] })] }), + scopePolicy({ attachments: [attachment({ rules: [allowSheets] })] }), + SHEETS, + ); + + expect(verdict).toEqual({ + action: 'deny', + scope: 'instance', + matchedRuleId: 'deny-oauth2-family', + optInAvailable: false, + }); + }); + + it('satisfies an instance delegate on a derived type with a project family allow', () => { + const verdict = evaluateComposedType( + scopePolicy({ defaultAction: 'delegate' }), + scopePolicy({ + attachments: [ + attachment({ + rules: [ + { + id: 'allow-family', + action: 'allow', + selector: { kind: 'extends', value: 'oAuth2Api' }, + }, + ], + }), + ], + }), + SHEETS, + ); + + expect(verdict).toEqual({ + action: 'allow', + scope: 'project', + matchedRuleId: 'allow-family', + optInAvailable: false, + }); + }); +}); + describe('evaluateComposedType', () => { const TYPE = 'n8n-nodes-base.slack'; const allowRule = (id: string) => diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/policy-shadow-lint.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/policy-shadow-lint.test.ts index 752e79fff09..046ebead2ee 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/policy-shadow-lint.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/policy-shadow-lint.test.ts @@ -177,6 +177,57 @@ describe('lintRulesForShadowing', () => { ]); }); + describe('extends selector', () => { + /** `googleSheetsOAuth2Api` is built on `googleOAuth2Api`, which is built on `oAuth2Api`. */ + const credentialType = (name: string) => ({ + name, + baseName: name, + ancestors: + { + googleSheetsOAuth2Api: ['googleOAuth2Api', 'oAuth2Api'], + googleOAuth2Api: ['oAuth2Api'], + }[name] ?? [], + }); + const lint = (rules: PolicyRule[]) => lintRulesForShadowing(rules, () => null, credentialType); + const family = (id: string, value: string) => + rule({ id, selector: { kind: 'extends', value } }); + const name = (id: string, value: string) => rule({ id, selector: { kind: 'name', value } }); + + it('flags a later name rule for a type built on an earlier family rule', () => { + expect( + lint([family('oauth2', 'oAuth2Api'), name('sheets', 'googleSheetsOAuth2Api')]), + ).toEqual([{ ruleId: 'sheets', shadowedByRuleId: 'oauth2' }]); + }); + + it('flags a later name rule for the family base itself', () => { + expect(lint([family('oauth2', 'oAuth2Api'), name('generic', 'oAuth2Api')])).toEqual([ + { ruleId: 'generic', shadowedByRuleId: 'oauth2' }, + ]); + }); + + it('flags a later family rule nested inside an earlier one', () => { + expect(lint([family('oauth2', 'oAuth2Api'), family('google', 'googleOAuth2Api')])).toEqual([ + { ruleId: 'google', shadowedByRuleId: 'oauth2' }, + ]); + }); + + it('does not flag a family rule after a name rule for its base, which matches one type only', () => { + expect(lint([name('generic', 'oAuth2Api'), family('oauth2', 'oAuth2Api')])).toEqual([]); + }); + + it('does not flag a wider family rule after a narrower one', () => { + expect(lint([family('google', 'googleOAuth2Api'), family('oauth2', 'oAuth2Api')])).toEqual( + [], + ); + }); + + it('does not flag a derived-type allow placed before the family rule', () => { + expect( + lint([name('sheets', 'googleSheetsOAuth2Api'), family('oauth2', 'oAuth2Api')]), + ).toEqual([]); + }); + }); + it('returns no warnings for an empty or single-rule list', () => { expect(lintRulesForShadowing([])).toEqual([]); expect( diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts index f9ba53aafd2..0ed8a9d2d2c 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts @@ -332,6 +332,67 @@ describe('TypeAvailabilityPolicyService', () => { expect(policyRepository.createPolicy).not.toHaveBeenCalled(); }); + describe('extends rules', () => { + const familyRule = (id: string, value: string): PolicyRule => ({ + id, + action: 'deny', + selector: { kind: 'extends', value }, + }); + + beforeEach(() => { + Object.defineProperty(loadNodesAndCredentials, 'knownCredentials', { + configurable: true, + value: { + oAuth2Api: { className: '', sourcePath: '' }, + googleSheetsOAuth2Api: { className: '', sourcePath: '', extends: ['oAuth2Api'] }, + }, + }); + }); + + afterEach(() => { + delete (loadNodesAndCredentials as { knownCredentials?: unknown }).knownCredentials; + }); + + it('rejects an extends rule in a node type policy', async () => { + await expect( + service.createPolicyDocument(KIND, [familyRule('r1', 'oAuth2Api')], 'user-1'), + ).rejects.toThrow('An "extends" rule is only valid in a credential type policy'); + expect(policyRepository.createPolicy).not.toHaveBeenCalled(); + }); + + it('rejects an extends rule naming a credential type that is not installed', async () => { + await expect( + service.createPolicyDocument( + CREDENTIAL_TYPES_KIND, + [familyRule('r1', 'notInstalledApi')], + 'user-1', + ), + ).rejects.toThrow( + 'Extends rule names a credential type that is not installed: notInstalledApi', + ); + expect(policyRepository.createPolicy).not.toHaveBeenCalled(); + }); + + it('warns about a derived-type rule placed after the family rule that covers it', async () => { + policyRepository.createPolicy.mockResolvedValue( + makePolicy({ kind: CREDENTIAL_TYPES_KIND }), + ); + const allowSheets: PolicyRule = { + id: 'r2', + action: 'allow', + selector: { kind: 'name', value: 'googleSheetsOAuth2Api' }, + }; + + const { warnings } = await service.createPolicyDocument( + CREDENTIAL_TYPES_KIND, + [familyRule('r1', 'oAuth2Api'), allowSheets], + 'user-1', + ); + + expect(warnings).toEqual([{ ruleId: 'r2', shadowedByRuleId: 'r1' }]); + }); + }); + it('accepts a package rule naming an installed package', async () => { policyRepository.createPolicy.mockResolvedValue(makePolicy()); const rule: PolicyRule = { diff --git a/packages/cli/src/modules/type-availability-policies/package-resolver.ts b/packages/cli/src/modules/type-availability-policies/package-resolver.ts index 2add7b98e7a..f0dfe5707a9 100644 --- a/packages/cli/src/modules/type-availability-policies/package-resolver.ts +++ b/packages/cli/src/modules/type-availability-policies/package-resolver.ts @@ -51,10 +51,54 @@ export function packageResolverFor( : nodeTypePackageResolver; } -export function policedTypeFor(kind: string, nodeTypes: NodeTypes): (name: string) => PolicedType { - return kind === NODE_TYPES_KIND - ? (name) => ({ name, baseName: nodeTypes.resolveBaseName(name).baseName }) - : (name) => ({ name, baseName: name }); +/** + * Every type `typeName` is built on, walking `extends` breadth-first. The visited set keeps a + * community package that declares a cycle from looping forever. + */ +function credentialTypeAncestors( + loadNodesAndCredentials: LoadNodesAndCredentials, + typeName: string, +): string[] { + const { knownCredentials } = loadNodesAndCredentials; + const parentsOf = (name: string) => + Object.hasOwn(knownCredentials, name) ? (knownCredentials[name].extends ?? []) : []; + + const seen = new Set([typeName]); + const queue = [...parentsOf(typeName)]; + + for (let index = 0; index < queue.length; index++) { + const parent = queue[index]; + if (seen.has(parent)) continue; + seen.add(parent); + queue.push(...parentsOf(parent)); + } + + seen.delete(typeName); + return [...seen]; +} + +export function policedTypeFor( + kind: string, + nodeTypes: NodeTypes, + loadNodesAndCredentials: LoadNodesAndCredentials, +): (name: string) => PolicedType { + if (kind === NODE_TYPES_KIND) { + return (name) => ({ name, baseName: nodeTypes.resolveBaseName(name).baseName }); + } + + return (name) => ({ + name, + baseName: name, + ancestors: credentialTypeAncestors(loadNodesAndCredentials, name), + }); +} + +/** Whether a credential type is known, checked at write time for an `extends` rule's value. */ +export function isCredentialTypeKnown( + loadNodesAndCredentials: LoadNodesAndCredentials, + typeName: string, +): boolean { + return Object.hasOwn(loadNodesAndCredentials.knownCredentials, typeName); } /** diff --git a/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts b/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts index b6ebdfa6736..6345530cd19 100644 --- a/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts +++ b/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts @@ -50,7 +50,12 @@ export type PackageResolver = (typeName: string) => string | null; */ export const nodeTypePackageResolver: PackageResolver = (typeName) => typeName.split('.')[0]; -export type PolicedType = { readonly name: string; readonly baseName: string }; +/** `ancestors` lists what a credential type is built on through `extends`; empty for node types. */ +export type PolicedType = { + readonly name: string; + readonly baseName: string; + readonly ancestors?: readonly string[]; +}; function selectorMatches( selector: PolicySelector, @@ -62,6 +67,8 @@ function selectorMatches( return selector.value === type.name || selector.value === type.baseName; case 'package': return resolvePackage(type.name) === selector.value; + case 'extends': + return selector.value === type.name || (type.ancestors ?? []).includes(selector.value); } } diff --git a/packages/cli/src/modules/type-availability-policies/policy-shadow-lint.ts b/packages/cli/src/modules/type-availability-policies/policy-shadow-lint.ts index 01a38a2783d..e28f7c25abe 100644 --- a/packages/cli/src/modules/type-availability-policies/policy-shadow-lint.ts +++ b/packages/cli/src/modules/type-availability-policies/policy-shadow-lint.ts @@ -3,7 +3,7 @@ import { type PackageResolver, type PolicedType, } from './policy-evaluator'; -import type { PolicyRule } from './policy-rule.types'; +import type { PolicyRule, PolicySelector } from './policy-rule.types'; /** * One case of an unreachable rule: `ruleId` can never match, because `shadowedByRuleId` @@ -54,6 +54,9 @@ function resolvePackageOrUndefined( * package, so it also shadows a later name selector scoped to that same package. A name * selector never shadows a package selector: one type can't cover a whole package. * + * An `extends` selector shadows a later `name` or `extends` selector whose type is the same one + * or built on it. Nothing else shadows an `extends`: its family can span several packages. + * * Runs in one O(n) pass: instead of comparing each rule against every rule before it, it * keeps, per selector kind, only the earliest rule seen so far for each selector value, and * looks that up once per rule. @@ -73,29 +76,47 @@ export function lintRulesForShadowing( ): ShadowWarning[] { const warnings: ShadowWarning[] = []; - const firstByName = new Map(); - const firstByPackage = new Map(); + const firstByKind: Record> = { + name: new Map(), + package: new Map(), + extends: new Map(), + }; + + /** The earliest `extends` rule naming the type itself or one it is built on. */ + const firstFamilyOccurrence = (type: PolicedType) => + [type.name, ...(type.ancestors ?? [])] + .map((name) => firstByKind.extends.get(name)) + .reduce(earlierOccurrence, undefined); for (let index = 0; index < rules.length; index++) { const rule = rules[index]; const { selector } = rule; - const shadowedBy = - selector.kind === 'package' - ? firstByPackage.get(selector.value) - : earlierOccurrence( - earlierOccurrence( - firstByName.get(selector.value), - firstByName.get(policedType(selector.value).baseName), - ), - resolvePackageOrUndefined(firstByPackage, resolvePackage, selector.value), - ); + let shadowedBy: FirstOccurrence | undefined; + switch (selector.kind) { + case 'package': + shadowedBy = firstByKind.package.get(selector.value); + break; + case 'extends': + shadowedBy = firstFamilyOccurrence(policedType(selector.value)); + break; + case 'name': { + const type = policedType(selector.value); + shadowedBy = [ + firstByKind.name.get(selector.value), + firstByKind.name.get(type.baseName), + resolvePackageOrUndefined(firstByKind.package, resolvePackage, selector.value), + firstFamilyOccurrence(type), + ].reduce(earlierOccurrence, undefined); + break; + } + } if (shadowedBy) { warnings.push({ ruleId: rule.id, shadowedByRuleId: shadowedBy.rule.id }); } - const firstOccurrenceByValue = selector.kind === 'package' ? firstByPackage : firstByName; + const firstOccurrenceByValue = firstByKind[selector.kind]; if (!firstOccurrenceByValue.has(selector.value)) { firstOccurrenceByValue.set(selector.value, { rule, index }); } diff --git a/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts b/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts index 9f009b8e44f..94d30d716be 100644 --- a/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts +++ b/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts @@ -21,8 +21,13 @@ import { TypeAvailabilityPolicyScopeRepository } from './database/repositories/t import { TypeAvailabilityPolicyRepository } from './database/repositories/type-availability-policy.repository'; import type { TypeAvailabilityPolicy } from './database/entities/type-availability-policy.entity'; import type { TypeAvailabilityPolicyScope } from './database/entities/type-availability-policy-scope.entity'; -import { NODE_TYPES_KIND } from './constants'; -import { isPackageInstalled, packageResolverFor, policedTypeFor } from './package-resolver'; +import { CREDENTIAL_TYPES_KIND, NODE_TYPES_KIND } from './constants'; +import { + isCredentialTypeKnown, + isPackageInstalled, + packageResolverFor, + policedTypeFor, +} from './package-resolver'; import { evaluateComposedType, orderedAttachments, type ComposedVerdict } from './policy-evaluator'; import type { PolicyAction, @@ -267,6 +272,34 @@ function assertNoCredentialOnlyNodeRules(kind: string, rules: readonly PolicyRul } } +/** + * Only credential types declare `extends`, so the selector means nothing for another kind. An + * unknown base would match nothing, the same trap `assertPackagesInstalled` closes. + */ +function assertExtendsSelectorsValid( + kind: string, + rules: readonly PolicyRule[], + loadNodesAndCredentials: LoadNodesAndCredentials, +): void { + const values = rules.flatMap((rule) => + rule.selector.kind === 'extends' ? [rule.selector.value] : [], + ); + if (values.length === 0) return; + + if (kind !== CREDENTIAL_TYPES_KIND) { + throw new UserError('An "extends" rule is only valid in a credential type policy'); + } + + const unknown = new Set( + values.filter((value) => !isCredentialTypeKnown(loadNodesAndCredentials, value)), + ); + if (unknown.size > 0) { + throw new UserError( + `Extends rule names a credential type that is not installed: ${[...unknown].join(', ')}`, + ); + } +} + /** Mirrors the DTO-level check in `ReplaceAttachmentsDto`, as a defensive service-level guard. */ function assertNoDuplicateAttachmentSlots(attachments: readonly AttachmentInput[]): void { const seenPolicyIds = new Set(); @@ -464,10 +497,11 @@ export class TypeAvailabilityPolicyService { ): Promise { assertPackagesInstalled(rules, this.loadNodesAndCredentials); assertNoCredentialOnlyNodeRules(kind, rules); + assertExtendsSelectorsValid(kind, rules, this.loadNodesAndCredentials); const warnings = lintRulesForShadowing( rules, packageResolverFor(kind, this.loadNodesAndCredentials), - policedTypeFor(kind, this.nodeTypes), + policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials), ); const policy = await this.policyRepository.createPolicy({ kind, rules, updatedBy }, {}); @@ -503,10 +537,11 @@ export class TypeAvailabilityPolicyService { ): Promise { assertPackagesInstalled(rules, this.loadNodesAndCredentials); assertNoCredentialOnlyNodeRules(kind, rules); + assertExtendsSelectorsValid(kind, rules, this.loadNodesAndCredentials); const warnings = lintRulesForShadowing( rules, packageResolverFor(kind, this.loadNodesAndCredentials), - policedTypeFor(kind, this.nodeTypes), + policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials), ); const result = await this.transactionRunner.run({}, async (ctx) => { @@ -758,11 +793,12 @@ export class TypeAvailabilityPolicyService { assertNoDelegateAtProjectScope(projectId, input.defaultAction, input.rules); assertPackagesInstalled(input.rules, this.loadNodesAndCredentials); assertNoCredentialOnlyNodeRules(kind, input.rules); + assertExtendsSelectorsValid(kind, input.rules, this.loadNodesAndCredentials); const warnings = lintRulesForShadowing( input.rules, packageResolverFor(kind, this.loadNodesAndCredentials), - policedTypeFor(kind, this.nodeTypes), + policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials), ); const result = await this.transactionRunner.run({}, async (ctx) => { @@ -977,7 +1013,7 @@ export class TypeAvailabilityPolicyService { return evaluateComposedType( instance, project, - policedTypeFor(kind, this.nodeTypes)(typeName), + policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials)(typeName), packageResolverFor(kind, this.loadNodesAndCredentials), ); } @@ -1054,7 +1090,7 @@ export class TypeAvailabilityPolicyService { private verdictComposer(kind: string, typeNames: readonly string[]) { const resolvePackage = packageResolverFor(kind, this.loadNodesAndCredentials); - const types = typeNames.map(policedTypeFor(kind, this.nodeTypes)); + const types = typeNames.map(policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials)); return (instance: EffectivePolicy, project: EffectivePolicy): ComposedTypeVerdict[] => types.map((type) => ({ diff --git a/packages/cli/src/public-api/v1/controllers/__tests__/node-type-policies.public.controller.test.ts b/packages/cli/src/public-api/v1/controllers/__tests__/node-type-policies.public.controller.test.ts index 22db9ebc194..62cf37e7d18 100644 --- a/packages/cli/src/public-api/v1/controllers/__tests__/node-type-policies.public.controller.test.ts +++ b/packages/cli/src/public-api/v1/controllers/__tests__/node-type-policies.public.controller.test.ts @@ -160,6 +160,17 @@ describe('NodeTypePoliciesPublicController handler bodies', () => { warnings: [{ ruleId: 'r2', shadowedByRuleId: 'r1' }], }; + it('fails loudly instead of returning an extends rule, which no node type write accepts', async () => { + service.getEffectivePolicy.mockResolvedValue({ + ...effectivePolicy, + rules: [{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } }], + }); + + await expect(controller.getNodeTypeInstancePolicy()).rejects.toThrow( + 'A node type policy holds an extends rule', + ); + }); + it('getNodeTypeInstancePolicy reads the null-project scope and maps the response', async () => { service.getEffectivePolicy.mockResolvedValue(effectivePolicy); diff --git a/packages/cli/src/public-api/v1/controllers/node-type-policies.public.controller.ts b/packages/cli/src/public-api/v1/controllers/node-type-policies.public.controller.ts index ae07a0d2194..ae6f3b1ad56 100644 --- a/packages/cli/src/public-api/v1/controllers/node-type-policies.public.controller.ts +++ b/packages/cli/src/public-api/v1/controllers/node-type-policies.public.controller.ts @@ -11,7 +11,10 @@ import { PutProjectPolicyDto, ReplaceAttachmentsDto, UpdatePolicyDocumentDto, + isNodeTypePolicyRule, nodeTypePolicyIdParamSchema, + type NodeTypePolicyRule, + type PolicyRule, nodeTypePolicyScopeIdParamSchema, projectIdParamSchema, } from '@n8n/api-types'; @@ -39,6 +42,7 @@ import { } from '@n8n/decorators'; import { Container } from '@n8n/di'; import type { Response } from 'express'; +import { UnexpectedError } from 'n8n-workflow'; import { NotFoundError, ServiceUnavailableError } from '@n8n/errors'; import { NODE_TYPES_KIND } from '@/modules/type-availability-policies/constants'; @@ -50,11 +54,19 @@ import { const tags = ['NodeTypePolicy']; +/** Every write rejects an `extends` rule for this kind, so meeting one here is a bug. */ +function toNodeTypeRules(rules: readonly PolicyRule[]): NodeTypePolicyRule[] { + if (!rules.every(isNodeTypePolicyRule)) { + throw new UnexpectedError('A node type policy holds an extends rule'); + } + return [...rules]; +} + function toPublicDocument(policy: TypeAvailabilityPolicy): PolicyDocumentPublicDto { return { id: policy.id, kind: policy.kind, - rules: [...policy.rules], + rules: toNodeTypeRules(policy.rules), version: policy.version, updatedBy: policy.updatedBy, createdAt: policy.createdAt.toISOString(), @@ -98,7 +110,7 @@ export class NodeTypePoliciesPublicController { return { scopeId: effective.scopeId, - rules: [...effective.rules], + rules: toNodeTypeRules(effective.rules), defaultAction: effective.defaultAction, version: effective.version, }; @@ -131,7 +143,7 @@ export class NodeTypePoliciesPublicController { return { scopeId: result.scopeId, - rules: result.rules, + rules: toNodeTypeRules(result.rules), defaultAction: result.defaultAction, version: result.version, warnings: [...result.warnings], @@ -158,7 +170,7 @@ export class NodeTypePoliciesPublicController { return { scopeId: effective.scopeId, - rules: [...effective.rules], + rules: toNodeTypeRules(effective.rules), defaultAction: effective.defaultAction, version: effective.version, }; @@ -192,7 +204,7 @@ export class NodeTypePoliciesPublicController { return { scopeId: result.scopeId, - rules: result.rules, + rules: toNodeTypeRules(result.rules), defaultAction: result.defaultAction, version: result.version, warnings: [...result.warnings], @@ -353,7 +365,7 @@ export class NodeTypePoliciesPublicController { return { attachments: result.attachments.map((attachment) => ({ policyId: attachment.policyId, - rules: [...attachment.rules], + rules: toNodeTypeRules(attachment.rules), priority: attachment.priority, isFloor: attachment.isFloor, })), diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/createCredentialTypePolicyDocument.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/createCredentialTypePolicyDocument.generated.yml index 5307513768f..cb1e1d64dec 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/createCredentialTypePolicyDocument.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/createCredentialTypePolicyDocument.generated.yml @@ -54,6 +54,19 @@ requestBody: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/getCredentialTypePolicyDocument.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/getCredentialTypePolicyDocument.generated.yml index 90ec4deeb1b..af9fa26ad74 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/getCredentialTypePolicyDocument.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/getCredentialTypePolicyDocument.generated.yml @@ -65,6 +65,19 @@ responses: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/listCredentialTypePolicyDocuments.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/listCredentialTypePolicyDocuments.generated.yml index 252b002ae77..bc59adb2354 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/listCredentialTypePolicyDocuments.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/listCredentialTypePolicyDocuments.generated.yml @@ -67,6 +67,19 @@ responses: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeInstancePolicy.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeInstancePolicy.generated.yml index cd0c127288a..fd44ea11c79 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeInstancePolicy.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeInstancePolicy.generated.yml @@ -54,6 +54,19 @@ requestBody: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeProjectPolicy.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeProjectPolicy.generated.yml index 95a4640db58..83f5209ccba 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeProjectPolicy.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/putCredentialTypeProjectPolicy.generated.yml @@ -60,6 +60,19 @@ requestBody: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/replaceCredentialTypePolicyAttachments.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/replaceCredentialTypePolicyAttachments.generated.yml index 74668c7686a..706b823f40f 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/replaceCredentialTypePolicyAttachments.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/replaceCredentialTypePolicyAttachments.generated.yml @@ -94,6 +94,19 @@ responses: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/updateCredentialTypePolicyDocument.generated.yml b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/updateCredentialTypePolicyDocument.generated.yml index 0d9c1065f1b..7ad573bd944 100644 --- a/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/updateCredentialTypePolicyDocument.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credential-type-policies/spec/paths/updateCredentialTypePolicyDocument.generated.yml @@ -61,6 +61,19 @@ requestBody: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyDocumentWriteResultPublicDto.generated.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyDocumentWriteResultPublicDto.generated.yml index 421646a75f1..094930d6fec 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyDocumentWriteResultPublicDto.generated.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyDocumentWriteResultPublicDto.generated.yml @@ -47,6 +47,19 @@ properties: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectivePublicDto.generated.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectivePublicDto.generated.yml index dfcd0436ef8..15783c8afda 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectivePublicDto.generated.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectivePublicDto.generated.yml @@ -43,6 +43,19 @@ properties: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectiveWriteResultPublicDto.generated.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectiveWriteResultPublicDto.generated.yml index 93194bd4e1c..8c3b41c4f29 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectiveWriteResultPublicDto.generated.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/credentialTypePolicyEffectiveWriteResultPublicDto.generated.yml @@ -43,6 +43,19 @@ properties: required: - kind - value + - type: object + properties: + kind: + type: string + enum: + - extends + value: + type: string + minLength: 1 + required: + - kind + - value + description: Matches the named credential type and every type that extends it type: object required: - id diff --git a/packages/cli/test/integration/policy/credential-type-policy.extends.test.ts b/packages/cli/test/integration/policy/credential-type-policy.extends.test.ts new file mode 100644 index 00000000000..d9967807b16 --- /dev/null +++ b/packages/cli/test/integration/policy/credential-type-policy.extends.test.ts @@ -0,0 +1,202 @@ +/** + * Pins what an `extends` rule decides for real host paths: a rule on a base credential type + * refuses a workflow that asks for a type built on it, exactly as a rule on that type would. + */ +import { createWorkflow, randomCredentialPayload, testDb } from '@n8n/backend-test-utils'; +import type { PolicyRule } from '@n8n/api-types'; +import { LICENSE_FEATURES } from '@n8n/constants'; +import { ExecutionRepository, WorkflowRepository, type User } from '@n8n/db'; +import { Container } from '@n8n/di'; +import { createRunExecutionData, type INode } from 'n8n-workflow'; +import { v4 as uuid } from 'uuid'; + +import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; +import { WorkflowRunner } from '@/workflow-runner'; + +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 { putCredentialTypePolicy } from './shared/credential-type-policy'; +import { clearPolicyCache } from './shared/policy-cache'; + +const MANUAL_TRIGGER = 'n8n-nodes-base.manualTrigger'; +const HTTP_REQUEST = 'n8n-nodes-base.httpRequest'; + +const BASE = 'oAuth2Api'; +const DERIVED = 'googleSheetsOAuth2Api'; + +const testServer = utils.setupTestServer({ + endpointGroups: ['workflows', 'type-availability-policies'], + modules: ['policy-infrastructure', 'type-availability-policies'], + enabledFeatures: [LICENSE_FEATURES.TYPE_AVAILABILITY_POLICIES, LICENSE_FEATURES.SHARING], +}); + +let owner: User; +let ownerAgent: SuperAgentTest; +let derivedCredentialId: string; +let workflowRepository: WorkflowRepository; +let executionRepository: ExecutionRepository; + +const denyFamily: PolicyRule = { + id: 'deny-oauth2-family', + action: 'deny', + selector: { kind: 'extends', value: BASE }, +}; + +const node = (type: string): INode => ({ + id: uuid(), + name: type, + type, + typeVersion: 1, + position: [0, 0], + parameters: {}, +}); + +const workflowBody = () => ({ + name: 'Sheets workflow', + nodes: [ + node(MANUAL_TRIGGER), + { + ...node(HTTP_REQUEST), + credentials: { [DERIVED]: { id: derivedCredentialId, name: 'Sheets account' } }, + }, + ], + connections: {}, +}); + +const derivedViolation = { + kind: 'credential-type-unavailable', + checkId: 'credential-type-availability', + message: `Credential type "${DERIVED}" is blocked by an instance policy`, + subject: DERIVED, + subjectType: 'credentialType', + scope: 'instance', + matchedRuleId: 'deny-oauth2-family', +}; + +beforeAll(async () => { + await utils.initNodeTypes(); + await utils.initCredentialsTypes(); + await utils.initBinaryDataService(); + + owner = await createOwner(); + ownerAgent = testServer.authAgentFor(owner); + workflowRepository = Container.get(WorkflowRepository); + executionRepository = Container.get(ExecutionRepository); + + const credential = await saveCredential(randomCredentialPayload({ type: DERIVED }), { + user: owner, + role: 'credential:owner', + }); + derivedCredentialId = credential.id; +}); + +// Per test: the shared setup restores every mock after each one. +beforeEach(() => { + vi.spyOn(Container.get(LoadNodesAndCredentials), 'knownCredentials', 'get').mockReturnValue({ + [BASE]: { className: '', sourcePath: '' }, + googleOAuth2Api: { className: '', sourcePath: '', extends: [BASE] }, + [DERIVED]: { className: '', sourcePath: '', extends: ['googleOAuth2Api'] }, + }); +}); + +afterEach(async () => { + await testDb.truncate([ + 'TypeAvailabilityPolicyAttachment', + 'TypeAvailabilityPolicyScope', + 'TypeAvailabilityPolicy', + 'ExecutionEntity', + 'WorkflowEntity', + 'SharedWorkflow', + 'WorkflowHistory', + ]); + await clearPolicyCache(); +}); + +describe('a rule on a base credential type', () => { + test('refuses saving a workflow that asks for a type built on it', async () => { + await putCredentialTypePolicy(ownerAgent, null, { rules: [denyFamily] }); + + const response = await ownerAgent.post('/workflows').send(workflowBody()).expect(403); + + expect(response.body.meta.violations).toEqual([derivedViolation]); + await expect(workflowRepository.count()).resolves.toBe(0); + }); + + test('fails a run of a workflow that asks for a type built on it', async () => { + const workflow = await createWorkflow(workflowBody(), owner); + await putCredentialTypePolicy(ownerAgent, null, { rules: [denyFamily] }); + + const executionId = await Container.get(WorkflowRunner).run( + { + workflowData: workflow, + executionMode: 'trigger', + executionData: createRunExecutionData({}), + }, + true, + ); + + await vi.waitFor( + async () => { + const execution = await executionRepository.findOneBy({ id: executionId }); + expect(execution?.status).toBe('error'); + }, + { timeout: 20000, interval: 50 }, + ); + const stored = await executionRepository.findSingleExecution(executionId, { + includeData: true, + unflattenData: true, + }); + const error = stored?.data.resultData.error as unknown as { violations?: unknown[] }; + + expect(error?.violations).toEqual([derivedViolation]); + }); + + test('lets an earlier allow for the derived type win over the family deny', async () => { + await putCredentialTypePolicy(ownerAgent, null, { + rules: [ + { id: 'allow-sheets', action: 'allow', selector: { kind: 'name', value: DERIVED } }, + denyFamily, + ], + }); + + await ownerAgent.post('/workflows').send(workflowBody()).expect(200); + }); + + test('leaves a type built on the base allowed when the rule names the base exactly', async () => { + await putCredentialTypePolicy(ownerAgent, null, { + rules: [{ id: 'deny-generic', action: 'deny', selector: { kind: 'name', value: BASE } }], + }); + + await ownerAgent.post('/workflows').send(workflowBody()).expect(200); + }); +}); + +describe('writing an extends rule', () => { + test('rejects a base credential type that is not installed', async () => { + const response = await ownerAgent.put('/credential-type-policies/instance').send({ + rules: [ + { id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'notInstalledApi' } }, + ], + defaultAction: 'allow', + version: 0, + }); + + expect(response.statusCode).toBe(400); + expect(response.body.message).toBe( + 'Extends rule names a credential type that is not installed: notInstalledApi', + ); + }); + + test('rejects the rule in a node type policy request', async () => { + const response = await ownerAgent.put('/node-type-policies/instance').send({ + rules: [denyFamily], + defaultAction: 'allow', + version: 0, + }); + + expect(response.statusCode).toBe(400); + expect(response.body.message).toBe("Invalid discriminator value. Expected 'name' | 'package'"); + }); +}); diff --git a/packages/cli/test/integration/public-api/credential-type-policies.test.ts b/packages/cli/test/integration/public-api/credential-type-policies.test.ts index 2c2948d9b3f..8fe685ffbe3 100644 --- a/packages/cli/test/integration/public-api/credential-type-policies.test.ts +++ b/packages/cli/test/integration/public-api/credential-type-policies.test.ts @@ -4,6 +4,7 @@ import { LICENSE_FEATURES } from '@n8n/constants'; import type { Project, User } from '@n8n/db'; import { Container } from '@n8n/di'; +import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { TypeAvailabilityPolicyRepository } from '@/modules/type-availability-policies/database/repositories/type-availability-policy.repository'; import { createMemberWithApiKey, createOwnerWithApiKey } from '@test-integration/db/users'; import * as utils from '@test-integration/utils'; @@ -669,3 +670,54 @@ describe('credential type policies public API attachments', () => { expect(response.statusCode).toBe(400); }); }); + +describe('credential type policies public API extends rules', () => { + const EXTENDS_RULE = { + id: 'r1', + action: 'deny', + selector: { kind: 'extends', value: 'oAuth2Api' }, + }; + + // Per test: the shared setup restores every mock after each one. + beforeEach(() => { + vi.spyOn(Container.get(LoadNodesAndCredentials), 'knownCredentials', 'get').mockReturnValue({ + oAuth2Api: { className: '', sourcePath: '' }, + }); + }); + + test('an extends rule reads back through every response shape', async () => { + const agent = testServer.publicApiAgentFor(owner); + + const instancePut = await agent + .put('/credential-type-policies/instance') + .send({ rules: [EXTENDS_RULE], defaultAction: 'allow', version: 0 }); + expect(instancePut.statusCode).toBe(200); + expect(instancePut.body.rules).toEqual([EXTENDS_RULE]); + + const instanceGet = await agent.get('/credential-type-policies/instance'); + expect(instanceGet.body.rules).toEqual([EXTENDS_RULE]); + + const created = await agent + .post('/credential-type-policies/policies') + .send({ rules: [EXTENDS_RULE] }); + expect(created.statusCode).toBe(201); + const policyId = created.body.policy.id as string; + + const document = await agent.get(`/credential-type-policies/policies/${policyId}`); + expect(document.body.rules).toEqual([EXTENDS_RULE]); + + const list = await agent.get('/credential-type-policies/policies'); + expect(list.statusCode).toBe(200); + expect(list.body.data.map((policy: { rules: unknown }) => policy.rules)).toContainEqual([ + EXTENDS_RULE, + ]); + + const attachments = await agent + .put(`/credential-type-policies/scopes/${instancePut.body.scopeId}/attachments`) + .send({ attachments: [{ policyId, priority: 1, isFloor: false }] }); + expect(attachments.statusCode).toBe(200); + expect(attachments.body.attachments).toEqual([ + { policyId, rules: [EXTENDS_RULE], priority: 1, isFloor: false }, + ]); + }); +}); diff --git a/packages/cli/test/integration/public-api/node-type-policies.test.ts b/packages/cli/test/integration/public-api/node-type-policies.test.ts index 59c13d3730f..396d9e44581 100644 --- a/packages/cli/test/integration/public-api/node-type-policies.test.ts +++ b/packages/cli/test/integration/public-api/node-type-policies.test.ts @@ -274,6 +274,22 @@ describe('node type policies public API instance scope', () => { }); }); +describe('node type policies public API extends rules', () => { + test('PUT rejects an extends rule with 400 and writes nothing', async () => { + const agent = testServer.publicApiAgentFor(owner); + + const response = await agent.put('/node-type-policies/instance').send({ + rules: [{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } }], + defaultAction: 'allow', + version: 0, + }); + + expect(response.statusCode).toBe(400); + const current = await agent.get('/node-type-policies/instance'); + expect(current.body.version).toBe(0); + }); +}); + describe('node type policies public API project scope', () => { test('GET on an unconfigured project reports allow-all at version 0', async () => { const response = await testServer.publicApiAgentFor(projectAdmin).get(projectRoute(project.id));