mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(core): Add the blocking project and opt-in state to policy violations (#40985)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
52aed7ac97
commit
4419fc4ba2
|
|
@ -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<typeof policyViolationSchema>;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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<string> = 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,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
}
|
||||
|
|
@ -143,6 +143,10 @@ responses:
|
|||
type: string
|
||||
matchedRuleId:
|
||||
type: string
|
||||
projectId:
|
||||
type: string
|
||||
optInAvailable:
|
||||
type: boolean
|
||||
required:
|
||||
- kind
|
||||
- checkId
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<Workflow>();
|
||||
|
||||
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<Workflow>();
|
||||
|
||||
const result = objectToError(errorObject, workflow);
|
||||
|
||||
expect(Object.hasOwn(result, 'isPolicyRefusal')).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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<typeof saveCredential>[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 })],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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 }),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}),
|
||||
]);
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user