diff --git a/packages/cli/src/active-workflow-manager.ts b/packages/cli/src/active-workflow-manager.ts index 523bb05500d..0d2be6495fc 100644 --- a/packages/cli/src/active-workflow-manager.ts +++ b/packages/cli/src/active-workflow-manager.ts @@ -44,6 +44,7 @@ import { import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import { ExternalHooks } from '@/external-hooks'; import { NodeTypes } from '@/node-types'; +import { enforceWorkflowPublishPolicy } from '@/policy/enforce-workflow-publish'; import { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; import { isPolicyRefusal } from '@/policy/policy-violation.error'; import { Publisher } from '@/scaling/pubsub/publisher.service'; @@ -600,14 +601,11 @@ export class ActiveWorkflowManager { // Trigger and poller nodes run code at registration, so this gates startup // and leadership change too, not just the activate button. - if (this.policyEnforcementService.hasChecksFor('workflowPublish')) { - const project = await this.ownershipService.getWorkflowProjectCached(dbWorkflow.id); - - await this.policyEnforcementService.enforceWorkflowPublish({ - workflow: { id: dbWorkflow.id, name: dbWorkflow.name, nodes }, - projectId: project.id, - }); - } + await enforceWorkflowPublishPolicy(this.policyEnforcementService, this.ownershipService, { + id: dbWorkflow.id, + name: dbWorkflow.name, + nodes, + }); workflow = new Workflow({ id: dbWorkflow.id, diff --git a/packages/cli/src/policy/__tests__/enforce-workflow-publish.test.ts b/packages/cli/src/policy/__tests__/enforce-workflow-publish.test.ts new file mode 100644 index 00000000000..8f7e44bd654 --- /dev/null +++ b/packages/cli/src/policy/__tests__/enforce-workflow-publish.test.ts @@ -0,0 +1,53 @@ +import type { Project } from '@n8n/db'; +import { mock } from 'vitest-mock-extended'; + +import type { OwnershipService } from '@/services/ownership.service'; + +import { enforceWorkflowPublishPolicy } from '../enforce-workflow-publish'; +import type { PolicyEnforcementService } from '../policy-enforcement.service'; + +describe('enforceWorkflowPublishPolicy', () => { + const policyEnforcementService = mock(); + const ownershipService = mock(); + const workflow = { id: 'workflow-1', name: 'Test Workflow', nodes: [] }; + + beforeEach(() => { + vi.clearAllMocks(); + ownershipService.getWorkflowProjectCached.mockResolvedValue(mock({ id: 'project-1' })); + }); + + // A feature that is merely absent must not cost a lookup on every publish. + it('skips the project lookup and the check when nothing is registered for workflowPublish', async () => { + policyEnforcementService.hasChecksFor.mockReturnValue(false); + + await enforceWorkflowPublishPolicy(policyEnforcementService, ownershipService, workflow); + + expect(policyEnforcementService.hasChecksFor).toHaveBeenCalledWith('workflowPublish'); + expect(ownershipService.getWorkflowProjectCached).not.toHaveBeenCalled(); + expect(policyEnforcementService.enforceWorkflowPublish).not.toHaveBeenCalled(); + }); + + it('enforces with the owning project when a check is registered', async () => { + policyEnforcementService.hasChecksFor.mockReturnValue(true); + + await enforceWorkflowPublishPolicy(policyEnforcementService, ownershipService, workflow); + + expect(ownershipService.getWorkflowProjectCached).toHaveBeenCalledWith('workflow-1'); + expect(policyEnforcementService.enforceWorkflowPublish).toHaveBeenCalledWith({ + workflow, + projectId: 'project-1', + }); + }); + + // An unevaluated project rule is not a passed one, so the lookup is unguarded. + it('propagates a failed project lookup instead of policing a null scope', async () => { + policyEnforcementService.hasChecksFor.mockReturnValue(true); + ownershipService.getWorkflowProjectCached.mockRejectedValue(new Error('no owner row')); + + await expect( + enforceWorkflowPublishPolicy(policyEnforcementService, ownershipService, workflow), + ).rejects.toThrow('no owner row'); + + expect(policyEnforcementService.enforceWorkflowPublish).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/policy/enforce-workflow-publish.ts b/packages/cli/src/policy/enforce-workflow-publish.ts new file mode 100644 index 00000000000..db0c25c2372 --- /dev/null +++ b/packages/cli/src/policy/enforce-workflow-publish.ts @@ -0,0 +1,29 @@ +import type { PolicedWorkflow } from '@n8n/decorators'; + +import type { OwnershipService } from '@/services/ownership.service'; + +import type { PolicyEnforcementService } from './policy-enforcement.service'; + +/** + * Enforces the `workflowPublish` policy for a workflow. It resolves the owning project only + * when a check will run. + * + * The short-circuit is deliberate. The owning project is read only to give the check its + * scope, so nothing consumes it when no check is registered. `getWorkflowProjectCached` is + * a read-through cache, so a caller that later needs the project fills it on its own miss. + * An absent feature must not cost a lookup on every publish. + * + * When a check is registered, the lookup is unguarded, as in `PolicyLifecycleHandler`: an + * unevaluated project rule is not a passed one, so a failed lookup fails the publish. + */ +export async function enforceWorkflowPublishPolicy( + policyEnforcementService: PolicyEnforcementService, + ownershipService: OwnershipService, + workflow: PolicedWorkflow & { id: string }, +): Promise { + if (!policyEnforcementService.hasChecksFor('workflowPublish')) return; + + const project = await ownershipService.getWorkflowProjectCached(workflow.id); + + await policyEnforcementService.enforceWorkflowPublish({ workflow, projectId: project.id }); +} diff --git a/packages/cli/src/workflows/publication/workflow-publication-applier.ts b/packages/cli/src/workflows/publication/workflow-publication-applier.ts index 3a752eb7995..a9875af419f 100644 --- a/packages/cli/src/workflows/publication/workflow-publication-applier.ts +++ b/packages/cli/src/workflows/publication/workflow-publication-applier.ts @@ -15,6 +15,7 @@ import { ensureError } from '@n8n/utils/errors/ensure-error'; import { UserError, type INode, type WorkflowActivateMode } from 'n8n-workflow'; import { NodeTypes } from '@/node-types'; +import { enforceWorkflowPublishPolicy } from '@/policy/enforce-workflow-publish'; import { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; import { PolicyViolationError } from '@/policy/policy-violation.error'; import { DurableJobProvisioner } from '@/scheduling/durable-job-provisioner'; @@ -441,16 +442,11 @@ export class WorkflowPublicationApplier { workflow: WorkflowEntity, newVersion: WorkflowHistory, ): Promise { - if (!this.policyEnforcementService.hasChecksFor('workflowPublish')) return null; - - // Unguarded, as in `PolicyLifecycleHandler`: an unevaluated project rule is not - // a passed one, so a failed lookup fails the publication. - const project = await this.ownershipService.getWorkflowProjectCached(workflow.id); - try { - await this.policyEnforcementService.enforceWorkflowPublish({ - workflow: { id: workflow.id, name: workflow.name, nodes: newVersion.nodes }, - projectId: project.id, + await enforceWorkflowPublishPolicy(this.policyEnforcementService, this.ownershipService, { + id: workflow.id, + name: workflow.name, + nodes: newVersion.nodes, }); return null; diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 10f37ea35dd..124f6a7d1da 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -58,6 +58,7 @@ import { validateEntity } from '@/generic-helpers'; import { RedactionEnforcementService } from '@/modules/redaction/redaction-enforcement.service'; import { NodeTypes } from '@/node-types'; import { userHasScopes } from '@/permissions.ee/check-access'; +import { enforceWorkflowPublishPolicy } from '@/policy/enforce-workflow-publish'; import { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; import type { ListQuery } from '@/requests'; import { hasSharing } from '@/requests'; @@ -1024,20 +1025,11 @@ export class WorkflowService { // Polices what gets registered — the version row, not the hook's candidate. // Enforced on a same-version republish too. - if (this.policyEnforcementService.hasChecksFor('workflowPublish')) { - // Unguarded, as in `PolicyLifecycleHandler`: an unevaluated project rule is - // not a passed one, so a failed lookup fails the publish. - const project = await this.ownershipService.getWorkflowProjectCached(workflowId); - - await this.policyEnforcementService.enforceWorkflowPublish({ - workflow: { - id: workflowId, - name: workflow.name, - nodes: nodesToPublish, - }, - projectId: project.id, - }); - } + await enforceWorkflowPublishPolicy(this.policyEnforcementService, this.ownershipService, { + id: workflowId, + name: workflow.name, + nodes: nodesToPublish, + }); // re-applying the already-published version (e.g. a settings-only update) // publishes no new version, so the review gate must not block it.