diff --git a/packages/@n8n/api-types/src/schemas/policy-violation.schema.ts b/packages/@n8n/api-types/src/schemas/policy-violation.schema.ts index c53a94413a1..2ffaeea7cd9 100644 --- a/packages/@n8n/api-types/src/schemas/policy-violation.schema.ts +++ b/packages/@n8n/api-types/src/schemas/policy-violation.schema.ts @@ -29,6 +29,15 @@ export const policyViolationSchema = z.object({ /** The rule that decided, if a rule did rather than a fallback default. */ matchedRuleId: z.string().optional(), + + /** + * The project whose policy was evaluated, also when `scope` is `'instance'`. For a + * sub-workflow this is the sub-workflow's project, not the caller's. + */ + projectId: z.string().optional(), + + /** The project can lift this block with its own allow rule. Absent means it cannot. */ + optInAvailable: z.boolean().optional(), }); export type PolicyViolation = z.infer; diff --git a/packages/cli/src/modules/type-availability-policies/README.md b/packages/cli/src/modules/type-availability-policies/README.md index 17613dad6dd..504a294aa6f 100644 --- a/packages/cli/src/modules/type-availability-policies/README.md +++ b/packages/cli/src/modules/type-availability-policies/README.md @@ -166,13 +166,24 @@ One violation per blocked type, deduplicated, in the order the types first appea "subject": "n8n-nodes-base.slack", "subjectType": "nodeType", "scope": "instance", - "matchedRuleId": "rule-7" + "matchedRuleId": "rule-7", + "projectId": "project-1", + "optInAvailable": false } ``` `matchedRuleId` is absent when the scope's default action decided and no rule matched. `scope` names the scope whose verdict decided, which is what a user needs to know who to ask. +`projectId` names the project whose policy was evaluated, also for an instance denial. A +sub-workflow is checked against its own project, so its refusal names that project, not the +caller's. It is absent when no project applied. A project denial's message says "a project +policy" and never names the project: the client resolves the name, and only for a project the +user can see. + +`optInAvailable` is `true` when the instance delegated the type, so the project can lift the +block with its own allow rule. It is always `false` when no project applied. + The violations reach the caller as HTTP 403 with `meta.violations`, and a blocked run stores them on the execution's error. Today that error is the whole story a user gets: the builder does not yet read `GET /projects/:projectId/available-types`, so a denied node looks normal diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/credential-type-policy.check.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/credential-type-policy.check.test.ts index 267b3bb7eb7..deb30480056 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/credential-type-policy.check.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/credential-type-policy.check.test.ts @@ -651,6 +651,8 @@ describe('CredentialTypePolicyCheck', () => { subjectType: 'credentialType', scope: 'instance', matchedRuleId: 'rule-7', + projectId: 'project-1', + optInAvailable: false, }); expect(service.evaluateComposedTypesFor).toHaveBeenCalledWith( 'credential-types', @@ -716,11 +718,13 @@ describe('CredentialTypePolicyCheck', () => { expect(result.violations[0]).toStrictEqual({ kind: 'credential-type-unavailable', checkId: 'credential-type-availability', - message: `Credential type "${SLACK_API}" is blocked by this project's policy`, + message: `Credential type "${SLACK_API}" is blocked by a project policy`, subject: SLACK_API, subjectType: 'credentialType', scope: 'project', matchedRuleId: 'rule-3', + projectId: 'project-1', + optInAvailable: false, }); }); @@ -739,8 +743,54 @@ describe('CredentialTypePolicyCheck', () => { subject: SLACK_API, subjectType: 'credentialType', scope: 'instance', + projectId: 'project-1', + optInAvailable: false, }); }); + + it('carries the projectId from the context on a project-scope denial', async () => { + denying([SLACK_API], { scope: 'project' }); + + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK_NODE, { [SLACK_API]: 'cred-1' })]), + projectId: 'project-1', + }); + + expect(result.violations[0].projectId).toBe('project-1'); + }); + + it('also carries the projectId on an instance-scope denial', async () => { + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK_NODE, { [SLACK_API]: 'cred-1' })]), + projectId: 'project-1', + }); + + expect(result.violations[0].scope).toBe('instance'); + expect(result.violations[0].projectId).toBe('project-1'); + }); + + it('reports optInAvailable true when the verdict allows it and a project applies', async () => { + denying([SLACK_API], { optInAvailable: true }); + + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK_NODE, { [SLACK_API]: 'cred-1' })]), + projectId: 'project-1', + }); + + expect(result.violations[0].optInAvailable).toBe(true); + }); + + it('drops projectId and forces optInAvailable false with no project, even when the verdict allows opt-in', async () => { + denying([SLACK_API], { optInAvailable: true }); + + const result = await check.onCredentialDecrypt({ + ...decryptOf(SLACK_API), + projectId: null, + }); + + expect(Object.keys(result.violations[0])).not.toContain('projectId'); + expect(result.violations[0].optInAvailable).toBe(false); + }); }); describe('when the store is not worth reading', () => { 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 e50b2972878..8cb9aa2511e 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 @@ -228,6 +228,8 @@ describe('NodeTypePolicyCheck', () => { subjectType: 'nodeType', scope: 'instance', matchedRuleId: 'rule-7', + projectId: 'project-1', + optInAvailable: false, }); expect(service.evaluateComposedTypesFor).toHaveBeenCalledWith('node-types', 'project-1', [ SLACK, @@ -297,11 +299,13 @@ describe('NodeTypePolicyCheck', () => { expect(result.violations[0]).toStrictEqual({ kind: 'node-type-unavailable', checkId: 'node-type-availability', - message: `Node type "${SLACK}" is blocked by this project's policy`, + message: `Node type "${SLACK}" is blocked by a project policy`, subject: SLACK, subjectType: 'nodeType', scope: 'project', matchedRuleId: 'rule-3', + projectId: 'project-1', + optInAvailable: false, }); }); @@ -320,8 +324,64 @@ describe('NodeTypePolicyCheck', () => { subject: SLACK, subjectType: 'nodeType', scope: 'instance', + projectId: 'project-1', + optInAvailable: false, }); }); + + it('carries the projectId from the context on a project-scope denial', async () => { + denying([SLACK], { scope: 'project' }); + + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK)]), + projectId: 'project-1', + }); + + expect(result.violations[0].projectId).toBe('project-1'); + }); + + it('also carries the projectId on an instance-scope denial', async () => { + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK)]), + projectId: 'project-1', + }); + + expect(result.violations[0].scope).toBe('instance'); + expect(result.violations[0].projectId).toBe('project-1'); + }); + + it('reports optInAvailable true when the verdict allows it and a project applies', async () => { + denying([SLACK], { optInAvailable: true }); + + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK)]), + projectId: 'project-1', + }); + + expect(result.violations[0].optInAvailable).toBe(true); + }); + + it('drops projectId and forces optInAvailable false with no project, even when the verdict allows opt-in', async () => { + denying([SLACK], { optInAvailable: true }); + + const result = await check.onContentImport({ + workflow: workflow([node(SLACK)]), + projectId: null, + transport: 'cli', + }); + + expect(Object.keys(result.violations[0])).not.toContain('projectId'); + expect(result.violations[0].optInAvailable).toBe(false); + }); + + it('names the projectId of the workflow-start context, as when a sub-workflow is checked against its own project', async () => { + const result = await check.onWorkflowStart({ + workflow: workflow([node(SLACK)]), + projectId: 'sub-workflow-project', + }); + + expect(result.violations[0].projectId).toBe('sub-workflow-project'); + }); }); describe('when the store is not worth reading', () => { diff --git a/packages/cli/src/modules/type-availability-policies/credential-type-policy.check.ts b/packages/cli/src/modules/type-availability-policies/credential-type-policy.check.ts index 58904f01559..a43a33fec4e 100644 --- a/packages/cli/src/modules/type-availability-policies/credential-type-policy.check.ts +++ b/packages/cli/src/modules/type-availability-policies/credential-type-policy.check.ts @@ -7,7 +7,6 @@ import type { CredentialTransferContext, PolicedWorkflow, PolicyCheckResult, - PolicyViolation, RegisteredPolicyCheck, WorkflowPublishContext, WorkflowSaveContext, @@ -21,10 +20,8 @@ import { getActiveCredentialTypes } from 'n8n-workflow'; import { NodeTypes } from '@/node-types'; import { CREDENTIAL_TYPES_KIND } from './constants'; -import { - TypeAvailabilityPolicyService, - type ComposedTypeVerdict, -} from './type-availability-policy.service'; +import { TypeAvailabilityPolicyService } from './type-availability-policy.service'; +import { CREDENTIAL_TYPE_SUBJECT, toTypeViolation } from './type-violation'; const NO_VIOLATIONS: PolicyCheckResult = { violations: [] }; @@ -33,21 +30,6 @@ const NOTHING_GRANDFATHERED: ReadonlySet = new Set(); const CREDENTIAL_TYPE_PARAMETERS = ['nodeCredentialType', 'genericAuthType'] as const; -function toViolation(checkId: string, verdict: ComposedTypeVerdict): PolicyViolation { - const by = verdict.scope === 'instance' ? 'an instance policy' : "this project's policy"; - - return { - kind: 'credential-type-unavailable', - checkId, - message: `Credential type "${verdict.name}" is blocked by ${by}`, - subject: verdict.name, - subjectType: 'credentialType', - scope: verdict.scope, - // The scope's default action decided when no rule matched, and there is no id to report. - ...(verdict.matchedRuleId !== null && { matchedRuleId: verdict.matchedRuleId }), - }; -} - /** * Enforces credential type availability policies, at every point that handles a workflow and * at both points that handle a credential. @@ -241,7 +223,7 @@ export class CredentialTypePolicyCheck implements RegisteredPolicyCheck { return { violations: verdicts .filter((verdict) => verdict.action === 'deny') - .map((verdict) => toViolation(this.id, verdict)), + .map((verdict) => toTypeViolation(this.id, CREDENTIAL_TYPE_SUBJECT, verdict, projectId)), // Reported even when nothing is denied: the audit line records what a partial run read. policyVersions: versions, }; 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 9fc0ea9f502..5734804df5e 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 @@ -5,7 +5,6 @@ import type { CredentialDecryptContext, PolicedWorkflow, PolicyCheckResult, - PolicyViolation, RegisteredPolicyCheck, WorkflowPublishContext, WorkflowSaveContext, @@ -15,10 +14,8 @@ import type { import { PolicyCheck } from '@n8n/decorators'; import { NODE_TYPES_KIND } from './constants'; -import { - TypeAvailabilityPolicyService, - type ComposedTypeVerdict, -} from './type-availability-policy.service'; +import { TypeAvailabilityPolicyService } from './type-availability-policy.service'; +import { NODE_TYPE_SUBJECT, toTypeViolation } from './type-violation'; const NO_VIOLATIONS: PolicyCheckResult = { violations: [] }; @@ -30,21 +27,6 @@ function distinctTypes(nodes: PolicedWorkflow['nodes']): string[] { return [...new Set(nodes.map((node) => node.type))]; } -function toViolation(checkId: string, verdict: ComposedTypeVerdict): PolicyViolation { - const by = verdict.scope === 'instance' ? 'an instance policy' : "this project's policy"; - - return { - kind: 'node-type-unavailable', - checkId, - message: `Node type "${verdict.name}" is blocked by ${by}`, - subject: verdict.name, - subjectType: 'nodeType', - scope: verdict.scope, - // The scope's default action decided when no rule matched, and there is no id to report. - ...(verdict.matchedRuleId !== null && { matchedRuleId: verdict.matchedRuleId }), - }; -} - /** * Enforces node type availability policies at every point that handles a workflow. * @@ -157,7 +139,7 @@ export class NodeTypePolicyCheck implements RegisteredPolicyCheck { return { violations: verdicts .filter((verdict) => verdict.action === 'deny') - .map((verdict) => toViolation(this.id, verdict)), + .map((verdict) => toTypeViolation(this.id, NODE_TYPE_SUBJECT, verdict, projectId)), // Reported even when nothing is denied: the audit line records what a partial run read. policyVersions: versions, }; diff --git a/packages/cli/src/modules/type-availability-policies/type-violation.ts b/packages/cli/src/modules/type-availability-policies/type-violation.ts new file mode 100644 index 00000000000..4231a6e5620 --- /dev/null +++ b/packages/cli/src/modules/type-availability-policies/type-violation.ts @@ -0,0 +1,46 @@ +import type { PolicyViolation } from '@n8n/decorators'; + +import type { ComposedTypeVerdict } from './type-availability-policy.service'; + +/** How one policy kind names what it blocked. */ +export type TypeViolationSubject = { + readonly kind: string; + readonly subjectType: string; + readonly label: string; +}; + +export const NODE_TYPE_SUBJECT: TypeViolationSubject = { + kind: 'node-type-unavailable', + subjectType: 'nodeType', + label: 'Node type', +}; + +export const CREDENTIAL_TYPE_SUBJECT: TypeViolationSubject = { + kind: 'credential-type-unavailable', + subjectType: 'credentialType', + label: 'Credential type', +}; + +/** The message stays neutral about which project: the client names it from `projectId`. */ +export function toTypeViolation( + checkId: string, + subject: TypeViolationSubject, + verdict: ComposedTypeVerdict, + projectId: string | null, +): PolicyViolation { + const by = verdict.scope === 'instance' ? 'an instance policy' : 'a project policy'; + + return { + kind: subject.kind, + checkId, + message: `${subject.label} "${verdict.name}" is blocked by ${by}`, + subject: verdict.name, + subjectType: subject.subjectType, + scope: verdict.scope, + // The scope's default action decided when no rule matched, and there is no id to report. + ...(verdict.matchedRuleId !== null && { matchedRuleId: verdict.matchedRuleId }), + ...(projectId !== null && { projectId }), + // With no project there is nowhere to opt in, whatever the instance delegated. + optInAvailable: projectId !== null && verdict.optInAvailable, + }; +} diff --git a/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/getSourceControlStatus.generated.yml b/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/getSourceControlStatus.generated.yml index 6569bc009c2..b922e6c2087 100644 --- a/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/getSourceControlStatus.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/getSourceControlStatus.generated.yml @@ -143,6 +143,10 @@ responses: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId diff --git a/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pullSourceControl.generated.yml b/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pullSourceControl.generated.yml index c1002e339d7..029a0e9018c 100644 --- a/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pullSourceControl.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pullSourceControl.generated.yml @@ -153,6 +153,10 @@ responses: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId @@ -310,6 +314,10 @@ responses: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId diff --git a/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pushSourceControl.generated.yml b/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pushSourceControl.generated.yml index c02d6fb4e28..861d154bdc2 100644 --- a/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pushSourceControl.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/source-control/spec/paths/pushSourceControl.generated.yml @@ -173,6 +173,10 @@ responses: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId @@ -337,6 +341,10 @@ responses: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/importBlockingIssue.generated.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/importBlockingIssue.generated.yml index 7fd9ecbc6f2..1b5a5f485b2 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/importBlockingIssue.generated.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/importBlockingIssue.generated.yml @@ -652,6 +652,10 @@ oneOf: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId @@ -699,6 +703,10 @@ oneOf: type: string matchedRuleId: type: string + projectId: + type: string + optInAvailable: + type: boolean required: - kind - checkId diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/policyViolation.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/policyViolation.yml index 38a4133312c..776604696e9 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/policyViolation.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/policyViolation.yml @@ -29,3 +29,11 @@ properties: matchedRuleId: type: string description: The rule that decided, if a rule did rather than a fallback default. + projectId: + type: string + description: > + The project whose policy was evaluated, also when `scope` is `instance`. For a + sub-workflow this is the sub-workflow's project, not the caller's. + optInAvailable: + type: boolean + description: The project can lift this block with its own allow rule. Absent means it cannot. diff --git a/packages/cli/src/utils/__tests__/object-to-error.test.ts b/packages/cli/src/utils/__tests__/object-to-error.test.ts index 1256eebfe12..a7656cdf076 100644 --- a/packages/cli/src/utils/__tests__/object-to-error.test.ts +++ b/packages/cli/src/utils/__tests__/object-to-error.test.ts @@ -40,4 +40,40 @@ describe('objectToError', () => { expect(result.message).toBe('Test error'); }); }); + + describe('policy violations', () => { + it('should copy `violations` from the error object', () => { + const violation = { + kind: 'node-type-unavailable', + checkId: 'node-type-availability', + message: 'Node type "n8n-nodes-base.set" is blocked by a project policy', + subject: 'n8n-nodes-base.set', + subjectType: 'nodeType', + scope: 'project', + projectId: 'project-1', + optInAvailable: false, + }; + const errorObject = { + message: 'Test error', + violations: [violation], + }; + const workflow = mock(); + + const result = objectToError(errorObject, workflow); + + expect((result as unknown as { violations: unknown[] }).violations).toEqual([violation]); + }); + + it('should not copy `isPolicyRefusal`', () => { + const errorObject = { + message: 'Test error', + isPolicyRefusal: true, + }; + const workflow = mock(); + + const result = objectToError(errorObject, workflow); + + expect(Object.hasOwn(result, 'isPolicyRefusal')).toBe(false); + }); + }); }); diff --git a/packages/cli/src/utils/object-to-error.ts b/packages/cli/src/utils/object-to-error.ts index 9fb6d57ed5e..fc2157afdea 100644 --- a/packages/cli/src/utils/object-to-error.ts +++ b/packages/cli/src/utils/object-to-error.ts @@ -4,8 +4,11 @@ import type { Workflow } from 'n8n-workflow'; /** * Optional properties that should be propagated from an error object to the new Error instance. + * + * `violations` keeps a sub-workflow's policy refusal readable in the parent. The + * `isPolicyRefusal` marker is left out: the parent's own graph was not refused. */ -const errorProperties = ['description', 'stack', 'executionId', 'workflowId']; +const errorProperties = ['description', 'stack', 'executionId', 'workflowId', 'violations']; export function objectToError(errorObject: unknown, workflow: Workflow): Error { // TODO: Expand with other error types diff --git a/packages/cli/test/integration/policy/credential-type-policy.credential-save.test.ts b/packages/cli/test/integration/policy/credential-type-policy.credential-save.test.ts index 40c81a9fd97..99d5e38654e 100644 --- a/packages/cli/test/integration/policy/credential-type-policy.credential-save.test.ts +++ b/packages/cli/test/integration/policy/credential-type-policy.credential-save.test.ts @@ -3,7 +3,12 @@ * while a credential that predates the rule stays editable. The lock belongs at decryption, * so the refusal here is about not letting someone set up what can never work. */ -import { createTeamProject, randomCredentialPayload, testDb } from '@n8n/backend-test-utils'; +import { + createTeamProject, + getPersonalProject, + randomCredentialPayload, + testDb, +} from '@n8n/backend-test-utils'; import { LICENSE_FEATURES } from '@n8n/constants'; import { CredentialsRepository, type User } from '@n8n/db'; import { PolicyCheckMetadata } from '@n8n/decorators'; @@ -38,14 +43,21 @@ let credentialsRepository: CredentialsRepository; const denyGithubAtInstanceScope = async () => await putCredentialTypePolicy(ownerAgent, null, { rules: [denyRule('deny-github', BLOCKED)] }); -const violationFor = (credentialType: string, scope: 'instance' | 'project', ruleId: string) => ({ +const violationFor = ( + credentialType: string, + scope: 'instance' | 'project', + ruleId: string, + { projectId, optInAvailable = false }: { projectId?: string; optInAvailable?: boolean } = {}, +) => ({ kind: 'credential-type-unavailable', checkId: CHECK_ID, - message: `Credential type "${credentialType}" is blocked by ${scope === 'instance' ? 'an instance policy' : "this project's policy"}`, + message: `Credential type "${credentialType}" is blocked by ${scope === 'instance' ? 'an instance policy' : 'a project policy'}`, subject: credentialType, subjectType: 'credentialType', scope, matchedRuleId: ruleId, + ...(projectId !== undefined && { projectId }), + optInAvailable, }); beforeAll(async () => { @@ -111,7 +123,7 @@ describe('POST /credentials', () => { .expect(403); expect(response.body.meta.violations).toEqual([ - violationFor(BLOCKED, 'project', 'deny-github'), + violationFor(BLOCKED, 'project', 'deny-github', { projectId: project.id }), ]); await expect(credentialsRepository.count()).resolves.toBe(0); }); @@ -147,8 +159,9 @@ describe('PATCH /credentials/:credentialId', () => { .send({ ...randomCredentialPayload({ type: BLOCKED }), name: credential.name }) .expect(403); + const { id: projectId } = await getPersonalProject(owner); expect(response.body.meta.violations).toEqual([ - violationFor(BLOCKED, 'instance', 'deny-github'), + violationFor(BLOCKED, 'instance', 'deny-github', { projectId }), ]); const stored = await credentialsRepository.findOneBy({ id: credential.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 index d9967807b16..73a34fd05f1 100644 --- a/packages/cli/test/integration/policy/credential-type-policy.extends.test.ts +++ b/packages/cli/test/integration/policy/credential-type-policy.extends.test.ts @@ -2,7 +2,12 @@ * 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 { + createWorkflow, + getPersonalProject, + 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'; @@ -65,7 +70,7 @@ const workflowBody = () => ({ connections: {}, }); -const derivedViolation = { +const derivedViolation = (projectId: string) => ({ kind: 'credential-type-unavailable', checkId: 'credential-type-availability', message: `Credential type "${DERIVED}" is blocked by an instance policy`, @@ -73,7 +78,9 @@ const derivedViolation = { subjectType: 'credentialType', scope: 'instance', matchedRuleId: 'deny-oauth2-family', -}; + projectId, + optInAvailable: false, +}); beforeAll(async () => { await utils.initNodeTypes(); @@ -120,7 +127,8 @@ describe('a rule on a base credential type', () => { const response = await ownerAgent.post('/workflows').send(workflowBody()).expect(403); - expect(response.body.meta.violations).toEqual([derivedViolation]); + const { id: projectId } = await getPersonalProject(owner); + expect(response.body.meta.violations).toEqual([derivedViolation(projectId)]); await expect(workflowRepository.count()).resolves.toBe(0); }); @@ -150,7 +158,8 @@ describe('a rule on a base credential type', () => { }); const error = stored?.data.resultData.error as unknown as { violations?: unknown[] }; - expect(error?.violations).toEqual([derivedViolation]); + const { id: projectId } = await getPersonalProject(owner); + expect(error?.violations).toEqual([derivedViolation(projectId)]); }); test('lets an earlier allow for the derived type win over the family deny', async () => { diff --git a/packages/cli/test/integration/policy/credential-type-policy.workflow.test.ts b/packages/cli/test/integration/policy/credential-type-policy.workflow.test.ts index 37f3c0b0f3c..46a9e0b1c7f 100644 --- a/packages/cli/test/integration/policy/credential-type-policy.workflow.test.ts +++ b/packages/cli/test/integration/policy/credential-type-policy.workflow.test.ts @@ -10,6 +10,7 @@ import { createTeamProject, createWorkflow, createWorkflowWithHistory, + getPersonalProject, randomCredentialPayload, testDb, } from '@n8n/backend-test-utils'; @@ -103,14 +104,17 @@ const violationFor = ( credentialType: string, scope: 'instance' | 'project', matchedRuleId: string, + { projectId, optInAvailable = false }: { projectId?: string; optInAvailable?: boolean } = {}, ) => ({ kind: 'credential-type-unavailable', checkId: CHECK_ID, - message: `Credential type "${credentialType}" is blocked by ${scope === 'instance' ? 'an instance policy' : "this project's policy"}`, + message: `Credential type "${credentialType}" is blocked by ${scope === 'instance' ? 'an instance policy' : 'a project policy'}`, subject: credentialType, subjectType: 'credentialType', scope, matchedRuleId, + ...(projectId !== undefined && { projectId }), + optInAvailable, }); type CredentialOwner = Parameters[1]; @@ -248,8 +252,9 @@ describe('saving a workflow', () => { }) .expect(403); + const { id: projectId } = await getPersonalProject(owner); expect(response.body.meta.violations).toEqual([ - violationFor(BLOCKED, 'instance', 'deny-github'), + violationFor(BLOCKED, 'instance', 'deny-github', { projectId }), ]); await expect(workflowRepository.count()).resolves.toBe(0); }); @@ -286,8 +291,9 @@ describe('saving a workflow', () => { .send({ name: 'New workflow', nodes: [node(MANUAL_TRIGGER), inlineAgent], connections: {} }) .expect(403); + const { id: projectId } = await getPersonalProject(owner); expect(response.body.meta.violations).toEqual([ - violationFor(BLOCKED, 'instance', 'deny-github'), + violationFor(BLOCKED, 'instance', 'deny-github', { projectId }), ]); await expect(workflowRepository.count()).resolves.toBe(0); }); @@ -387,7 +393,10 @@ describe('starting a run', () => { }); const error = stored?.data.resultData.error as unknown as { violations?: unknown[] }; - expect(error?.violations).toEqual([violationFor(BLOCKED, 'instance', 'deny-github')]); + const { id: projectId } = await getPersonalProject(owner); + expect(error?.violations).toEqual([ + violationFor(BLOCKED, 'instance', 'deny-github', { projectId }), + ]); }); test('runs as usual when no rule matches', async () => { @@ -423,7 +432,9 @@ describe('PUT /workflows/:workflowId/transfer', () => { expect(response.body).toMatchObject({ code: 403, - meta: { violations: [violationFor(BLOCKED, 'project', 'deny-github')] }, + meta: { + violations: [violationFor(BLOCKED, 'project', 'deny-github', { projectId: project.id })], + }, }); }); diff --git a/packages/cli/test/integration/policy/node-type-policy.publish-and-start.test.ts b/packages/cli/test/integration/policy/node-type-policy.publish-and-start.test.ts index 9866f74eb0c..73f9f63794d 100644 --- a/packages/cli/test/integration/policy/node-type-policy.publish-and-start.test.ts +++ b/packages/cli/test/integration/policy/node-type-policy.publish-and-start.test.ts @@ -6,7 +6,12 @@ * behaviour the RFC asks for and the one users will call a bug — "my old workflow saves but * will not run" — so it is worth pinning on the real paths rather than in the check's own tests. */ -import { createWorkflow, createWorkflowWithHistory, testDb } from '@n8n/backend-test-utils'; +import { + createWorkflow, + createWorkflowWithHistory, + getPersonalProject, + testDb, +} from '@n8n/backend-test-utils'; import { WorkflowsConfig } from '@n8n/config'; import { LICENSE_FEATURES } from '@n8n/constants'; import { ExecutionRepository, WorkflowRepository, type IWorkflowDb, type User } from '@n8n/db'; @@ -97,6 +102,7 @@ const expectedViolation = { subjectType: 'nodeType', scope: 'instance', matchedRuleId: 'deny-set', + optInAvailable: false, }; beforeAll(async () => { @@ -254,7 +260,8 @@ describe('starting a run', () => { violations?: unknown[]; }; - expect(error?.violations).toEqual([expectedViolation]); + const { id: projectId } = await getPersonalProject(owner); + expect(error?.violations).toEqual([{ ...expectedViolation, projectId }]); }); test('fails the run of a workflow whose inline agent has a blocked node tool', async () => { @@ -273,7 +280,8 @@ describe('starting a run', () => { unflattenData: true, }); const error = stored?.data.resultData.error as unknown as { violations?: unknown[] }; - expect(error?.violations).toEqual([expectedViolation]); + const { id: projectId } = await getPersonalProject(owner); + expect(error?.violations).toEqual([{ ...expectedViolation, projectId }]); }); test('runs as usual when no rule matches the workflow', async () => { diff --git a/packages/cli/test/integration/policy/node-type-policy.save.test.ts b/packages/cli/test/integration/policy/node-type-policy.save.test.ts index ce35f0c1ca7..1200686cbf9 100644 --- a/packages/cli/test/integration/policy/node-type-policy.save.test.ts +++ b/packages/cli/test/integration/policy/node-type-policy.save.test.ts @@ -7,7 +7,13 @@ * to be an integration test: it depends on the stored workflow the host loads, which no unit * test supplies for real. */ -import { createTeamProject, createWorkflow, mockInstance, testDb } from '@n8n/backend-test-utils'; +import { + createTeamProject, + createWorkflow, + getPersonalProject, + mockInstance, + testDb, +} from '@n8n/backend-test-utils'; import { LICENSE_FEATURES } from '@n8n/constants'; import { WorkflowRepository, type Project, type User } from '@n8n/db'; import { PolicyCheckMetadata } from '@n8n/decorators'; @@ -94,14 +100,21 @@ async function putProjectPolicy( return response.body as { version: number }; } -const violationFor = (typeName: string, scope: 'instance' | 'project', matchedRuleId: string) => ({ +const violationFor = ( + typeName: string, + scope: 'instance' | 'project', + matchedRuleId: string, + { projectId, optInAvailable = false }: { projectId?: string; optInAvailable?: boolean } = {}, +) => ({ kind: 'node-type-unavailable', checkId: CHECK_ID, - message: `Node type "${typeName}" is blocked by ${scope === 'instance' ? 'an instance policy' : "this project's policy"}`, + message: `Node type "${typeName}" is blocked by ${scope === 'instance' ? 'an instance policy' : 'a project policy'}`, subject: typeName, subjectType: 'nodeType', scope, matchedRuleId, + ...(projectId !== undefined && { projectId }), + optInAvailable, }); beforeAll(async () => { @@ -316,9 +329,10 @@ describe('POST /workflows', () => { }) .expect(403); + const { id: projectId } = await getPersonalProject(owner); expect(response.body.meta.violations).toEqual([ - violationFor(SET, 'instance', 'deny-set'), - violationFor(SCHEDULE_TRIGGER, 'instance', 'deny-schedule'), + violationFor(SET, 'instance', 'deny-set', { projectId }), + violationFor(SCHEDULE_TRIGGER, 'instance', 'deny-schedule', { projectId }), ]); }); }); @@ -338,7 +352,7 @@ describe('PUT /workflows/:workflowId/transfer', () => { expect(response.body).toMatchObject({ code: 403, - meta: { violations: [violationFor(SET, 'project', 'deny-set')] }, + meta: { violations: [violationFor(SET, 'project', 'deny-set', { projectId: project.id })] }, }); }); @@ -382,7 +396,12 @@ describe('instance and project scope composition', () => { const response = await saveWithSet().expect(403); - expect(response.body.meta.violations).toEqual([violationFor(SET, 'instance', 'delegate-set')]); + expect(response.body.meta.violations).toEqual([ + violationFor(SET, 'instance', 'delegate-set', { + projectId: project.id, + optInAvailable: true, + }), + ]); }); test('keeps an instance deny in force over a project allow', async () => { @@ -391,6 +410,8 @@ describe('instance and project scope composition', () => { const response = await saveWithSet().expect(403); - expect(response.body.meta.violations).toEqual([violationFor(SET, 'instance', 'deny-set')]); + expect(response.body.meta.violations).toEqual([ + violationFor(SET, 'instance', 'deny-set', { projectId: project.id }), + ]); }); }); diff --git a/packages/cli/test/integration/policy/node-type-policy.sub-workflow.test.ts b/packages/cli/test/integration/policy/node-type-policy.sub-workflow.test.ts new file mode 100644 index 00000000000..68e57a92f06 --- /dev/null +++ b/packages/cli/test/integration/policy/node-type-policy.sub-workflow.test.ts @@ -0,0 +1,136 @@ +/** + * Pins whose project a sub-workflow refusal names. The child is checked against its own + * project's policy, and that refusal travels up to the parent's execution unchanged. + */ +import { createTeamProject, createWorkflow, testDb } from '@n8n/backend-test-utils'; +import { LICENSE_FEATURES } from '@n8n/constants'; +import { ExecutionRepository, type IWorkflowDb, type Project, type User } from '@n8n/db'; +import { Container } from '@n8n/di'; +import { v4 as uuid } from 'uuid'; + +import { createOwner } from '../shared/db/users'; +import type { SuperAgentTest } from '../shared/types'; +import * as utils from '../shared/utils/'; +import { loadNodesFromDist } from '../shared/utils/node-types-data'; +import { createParentWorkflowFixture, createSubWorkflowFixture } from '../shared/workflow-fixtures'; +import { clearPolicyCache } from './shared/policy-cache'; + +const SET = 'n8n-nodes-base.set'; + +const testServer = utils.setupTestServer({ + endpointGroups: ['workflows', 'executions', 'type-availability-policies'], + modules: ['policy-infrastructure', 'type-availability-policies'], + enabledFeatures: [LICENSE_FEATURES.TYPE_AVAILABILITY_POLICIES], +}); + +let owner: User; +let ownerAgent: SuperAgentTest; +let childProject: Project; +let executionRepository: ExecutionRepository; + +beforeAll(async () => { + await utils.initNodeTypes( + loadNodesFromDist([ + 'n8n-nodes-base.manualTrigger', + 'n8n-nodes-base.executeWorkflow', + 'n8n-nodes-base.executeWorkflowTrigger', + SET, + ]), + ); + await utils.initBinaryDataService(); + + owner = await createOwner(); + ownerAgent = testServer.authAgentFor(owner); + executionRepository = Container.get(ExecutionRepository); +}); + +beforeEach(async () => { + childProject = await createTeamProject('Child project', owner); +}); + +afterEach(async () => { + await testDb.truncate([ + 'TypeAvailabilityPolicyAttachment', + 'TypeAvailabilityPolicyScope', + 'TypeAvailabilityPolicy', + 'ExecutionEntity', + 'WorkflowEntity', + 'SharedWorkflow', + ]); + await clearPolicyCache(); +}); + +async function waitForFinalStatus(executionId: string, timeout = 20000) { + const start = Date.now(); + + while (Date.now() - start < timeout) { + const execution = await executionRepository.findOne({ where: { id: executionId } }); + if (execution && ['error', 'success', 'crashed'].includes(execution.status)) { + return execution; + } + await new Promise((resolve) => setTimeout(resolve, 50)); + } + + throw new Error(`Execution ${executionId} did not finish within ${timeout}ms`); +} + +test("names the sub-workflow's project when that project blocks a node", async () => { + const childFixture = createSubWorkflowFixture(); + const child = await createWorkflow( + { + name: 'Child', + ...childFixture, + // Unconnected: the check reads the node list, so the node never has to run. + nodes: [ + ...childFixture.nodes, + { + id: uuid(), + name: 'Set', + type: SET, + typeVersion: 3.4, + position: [200, 0], + parameters: {}, + }, + ], + settings: { callerPolicy: 'any' }, + } as unknown as IWorkflowDb, + childProject, + ); + const parent = await createWorkflow( + { name: 'Parent', ...createParentWorkflowFixture(child.id) } as unknown as IWorkflowDb, + owner, + ); + + await ownerAgent + .put(`/projects/${childProject.id}/node-type-policies/project`) + .send({ + rules: [{ id: 'deny-set', action: 'deny', selector: { kind: 'name', value: SET } }], + defaultAction: 'allow', + version: 0, + }) + .expect(200); + + const response = await ownerAgent + .post(`/workflows/${parent.id}/run`) + .send({ triggerToStartFrom: { name: 'Trigger' } }) + .expect(200); + const parentExecutionId = response.body.data.executionId as string; + + const execution = await waitForFinalStatus(parentExecutionId); + expect(execution.status).toBe('error'); + + const stored = await executionRepository.findSingleExecution(parentExecutionId, { + includeData: true, + unflattenData: true, + }); + const error = stored?.data.resultData.error as unknown as { violations?: unknown[] }; + + expect(error?.violations).toEqual([ + expect.objectContaining({ + subject: SET, + scope: 'project', + projectId: childProject.id, + optInAvailable: false, + }), + ]); +});