chore: Extract the duplicated workflowPublish policy-enforcement (#37674)

Co-authored-by: n8n-cat-bot[bot] <n8n-cat-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

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,

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<PolicyEnforcementService>();
+	const ownershipService = mock<OwnershipService>();
+	const workflow = { id: 'workflow-1', name: 'Test Workflow', nodes: [] };
+
+	beforeEach(() => {
+		vi.clearAllMocks();
+		ownershipService.getWorkflowProjectCached.mockResolvedValue(mock<Project>({ 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();
+	});
+});

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<void> {
+	if (!policyEnforcementService.hasChecksFor('workflowPublish')) return;
+
+	const project = await ownershipService.getWorkflowProjectCached(workflow.id);
+
+	await policyEnforcementService.enforceWorkflowPublish({ workflow, projectId: project.id });
+}

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<PublicationResult | null> {
-		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;

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.