mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-16 21:19:43 +02:00
fix(core): Save policy rules that name uninstalled packages (no-changelog) (#41210)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
This commit is contained in:
Guillaume Jacquart
GitHub
parent
24621ad4ca
commit
532e8ef930
|
|
@ -14,11 +14,15 @@ import { publicApiPaginationSchema } from '../pagination/pagination.dto';
|
|||
* `PutCredentialTypeInstancePolicyDto`, …), so validation is identical on both surfaces.
|
||||
*/
|
||||
|
||||
/** A rule that never matches because an earlier rule in the same list already covers it. */
|
||||
const shadowWarningSchema = z.object({
|
||||
ruleId: z.string(),
|
||||
shadowedByRuleId: z.string(),
|
||||
});
|
||||
/**
|
||||
* A saved rule worth a second look: `shadowed` when an earlier rule in the same list already
|
||||
* covers it, so it never matches. `notLoaded` when it names a package or type this instance
|
||||
* has not loaded, so nothing here uses it yet. `NODES_EXCLUDE` and `NODES_INCLUDE` count too.
|
||||
*/
|
||||
const ruleWarningSchema = z.discriminatedUnion('kind', [
|
||||
z.object({ kind: z.literal('shadowed'), ruleId: z.string(), shadowedByRuleId: z.string() }),
|
||||
z.object({ kind: z.literal('notLoaded'), ruleId: z.string() }),
|
||||
]);
|
||||
|
||||
/** Plain rule arrays on purpose: a response never re-runs the duplicate-id refinement. */
|
||||
function publicSchemasFor<Rule extends z.ZodTypeAny>(rule: Rule) {
|
||||
|
|
@ -52,9 +56,9 @@ function publicSchemasFor<Rule extends z.ZodTypeAny>(rule: Rule) {
|
|||
|
||||
return {
|
||||
effective: effectivePolicy.shape,
|
||||
effectiveWriteResult: { ...effectivePolicy.shape, warnings: z.array(shadowWarningSchema) },
|
||||
effectiveWriteResult: { ...effectivePolicy.shape, warnings: z.array(ruleWarningSchema) },
|
||||
document: policyDocument.shape,
|
||||
documentWriteResult: { policy: policyDocument, warnings: z.array(shadowWarningSchema) },
|
||||
documentWriteResult: { policy: policyDocument, warnings: z.array(ruleWarningSchema) },
|
||||
documentList: { data: z.array(policyDocument), nextCursor: z.string().nullable() },
|
||||
// The scope's version after a replace, which always bumps it.
|
||||
attachments: { attachments: z.array(attachment), version: z.number().int().positive() },
|
||||
|
|
@ -67,13 +71,13 @@ const credentialType = publicSchemasFor(credentialTypePolicyRuleSchemas.rule);
|
|||
/** `GET` of a scope's composed node type policy. */
|
||||
export class PolicyEffectivePublicDto extends Z.class(nodeType.effective) {}
|
||||
|
||||
/** `PUT` of a scope's composed node type policy: the new state plus shadowing warnings. */
|
||||
/** `PUT` of a scope's composed node type policy: the new state plus rule warnings. */
|
||||
export class PolicyEffectiveWriteResultPublicDto extends Z.class(nodeType.effectiveWriteResult) {}
|
||||
|
||||
/** One reusable node type policy document. */
|
||||
export class PolicyDocumentPublicDto extends Z.class(nodeType.document) {}
|
||||
|
||||
/** Create or update of a node type policy document, plus shadowing warnings. */
|
||||
/** Create or update of a node type policy document, plus rule warnings. */
|
||||
export class PolicyDocumentWriteResultPublicDto extends Z.class(nodeType.documentWriteResult) {}
|
||||
|
||||
export class PolicyDocumentListPublicDto extends Z.class(nodeType.documentList) {}
|
||||
|
|
|
|||
|
|
@ -90,6 +90,9 @@ export const TYPE_AVAILABILITY_POLICIES_TELEMETRY = defineTelemetryEvents({
|
|||
shadow_warning_count: z
|
||||
.number()
|
||||
.describe('Saved rules that can never match, because an earlier rule already covers them'),
|
||||
not_loaded_warning_count: z
|
||||
.number()
|
||||
.describe('Saved rules that name a package or type this instance has not loaded'),
|
||||
version: z.number().describe('The scope version after the write'),
|
||||
}),
|
||||
},
|
||||
|
|
|
|||
|
|
@ -758,7 +758,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [denyRule],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
|
|
@ -780,7 +781,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [denyRule, allowPackageRule, delegateRule],
|
||||
warningCount: 2,
|
||||
shadowWarningCount: 2,
|
||||
notLoadedWarningCount: 1,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
|
@ -810,6 +812,7 @@ describe('TelemetryEventRelay', () => {
|
|||
allowed_types: ['n8n-nodes-base.code'],
|
||||
previous_rule_count: null,
|
||||
shadow_warning_count: 2,
|
||||
not_loaded_warning_count: 1,
|
||||
version: 1,
|
||||
},
|
||||
);
|
||||
|
|
@ -831,7 +834,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [credentialDenyRule],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
|
@ -871,7 +875,8 @@ describe('TelemetryEventRelay', () => {
|
|||
rulesAfter: [
|
||||
{ id: 'rule-1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } },
|
||||
],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
|
|
@ -900,7 +905,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [credentialPackageDenyRule],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
|
@ -927,7 +933,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
|
@ -945,7 +952,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'deny', version: 5 },
|
||||
rulesBefore: [denyRule],
|
||||
rulesAfter: [allowPackageRule],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
|
@ -975,7 +983,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
|
@ -1097,7 +1106,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [denyRule],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
|
|
@ -1121,7 +1131,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [{ ...allowPackageRule, action: 'deny' }],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
|
|
@ -1151,7 +1162,8 @@ describe('TelemetryEventRelay', () => {
|
|||
action: 'allow' as const,
|
||||
selector: { kind: 'name' as const, value: name },
|
||||
})),
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
|
||||
const properties = vi.mocked(telemetry.track).mock.calls.at(-1)?.[1];
|
||||
|
|
@ -1174,7 +1186,8 @@ describe('TelemetryEventRelay', () => {
|
|||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
|
|
|
|||
|
|
@ -1450,7 +1450,8 @@ export type RelayEventMap = {
|
|||
after: { defaultAction: PolicyAction; version: number };
|
||||
rulesBefore: readonly PolicyRule[] | null;
|
||||
rulesAfter: readonly PolicyRule[];
|
||||
warningCount: number;
|
||||
shadowWarningCount: number;
|
||||
notLoadedWarningCount: number;
|
||||
};
|
||||
|
||||
'node-type-policy-attachments-updated': {
|
||||
|
|
|
|||
|
|
@ -595,7 +595,8 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
after,
|
||||
rulesBefore,
|
||||
rulesAfter,
|
||||
warningCount,
|
||||
shadowWarningCount,
|
||||
notLoadedWarningCount,
|
||||
}: RelayEventMap['node-type-policy-saved']) {
|
||||
if (!isPolicyKind(kind)) return;
|
||||
|
||||
|
|
@ -619,7 +620,8 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
...countSelectorKinds(rulesAfter),
|
||||
...summarizeTypeAvailability(rulesAfter, after.defaultAction, types, resolvePackage),
|
||||
previous_rule_count: rulesBefore?.length ?? null,
|
||||
shadow_warning_count: warningCount,
|
||||
shadow_warning_count: shadowWarningCount,
|
||||
not_loaded_warning_count: notLoadedWarningCount,
|
||||
version: after.version,
|
||||
},
|
||||
);
|
||||
|
|
|
|||
|
|
@ -220,7 +220,15 @@ covers `googleOAuth2Api` and `googleSheetsOAuth2Api`. A `name` rule stays exact,
|
|||
`name oAuth2Api` blocks only the generic OAuth2 credential. Rule order decides as usual: an
|
||||
`allow name googleSheetsOAuth2Api` placed before `deny extends oAuth2Api` keeps Sheets usable,
|
||||
and placed after it, the shadow lint warns that it can never match. A write rejects an `extends`
|
||||
rule in a node type policy, and one that names a credential type that is not installed.
|
||||
rule in a node type policy.
|
||||
|
||||
A rule can name a package or type that this instance has not loaded. The write saves it and
|
||||
returns a `notLoaded` warning for it, next to the `shadowed` warnings. This applies to all
|
||||
three selectors. Thus, when you uninstall a package, its rules do not block later writes to the
|
||||
policy. An installed node that `NODES_EXCLUDE` or `NODES_INCLUDE` filters out is not loaded
|
||||
either. The policy still enforces its rule, because evaluation matches type names, but a policy
|
||||
cannot make a filtered-out node available. One exception: a write rejects a `package` rule on `n8n-creds-base`, because no package
|
||||
can ever provide it.
|
||||
|
||||
### Reading it on the execution path
|
||||
|
||||
|
|
@ -324,6 +332,7 @@ through a sealed repository method, and the lint rule that guards that has no al
|
|||
| `credential-type-policy.check.ts` | Credential types: all eight points, including `credentialSave` |
|
||||
| `policy-evaluator.ts` | Pure evaluation: first match per scope, then the instance ∩ project composition |
|
||||
| `policy-shadow-lint.ts` | Warns at write time about rules an earlier rule already covers |
|
||||
| `policy-rule-lint.ts` | Adds `notLoaded` warnings and merges all write-time warnings in rule order |
|
||||
| `package-resolver.ts` | Resolves a type's package per `kind`, for the `package` selector |
|
||||
| `type-availability-policy.service.ts` | Reads and writes the store, with versioning and row locks |
|
||||
| `type-availability-policy-instance.controller.ts` | Instance scope, documents and attachments, `node-types` |
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import type { NodeTypes } from '@/node-types';
|
|||
|
||||
import { CREDENTIAL_TYPES_KIND, NODE_TYPES_KIND } from '../constants';
|
||||
import {
|
||||
loadedCheckFor,
|
||||
isCredentialTypeKnown,
|
||||
isPackageInstalled,
|
||||
packageResolverFor,
|
||||
|
|
@ -168,3 +169,122 @@ describe('isCredentialTypeKnown', () => {
|
|||
expect(isCredentialTypeKnown(registry, 'constructor')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadedCheckFor', () => {
|
||||
/** Stands in for `NodeTypes.resolveBaseName`: `slackTool` is a synthetic variant of `slack`. */
|
||||
const nodeTypes = {
|
||||
resolveBaseName: (name: string) => ({
|
||||
baseName: name === 'n8n-nodes-base.slackTool' ? 'n8n-nodes-base.slack' : name,
|
||||
isSyntheticTool: name === 'n8n-nodes-base.slackTool',
|
||||
}),
|
||||
} as unknown as NodeTypes;
|
||||
|
||||
/** A stand-in for `LoadNodesAndCredentials`, built from just the fields this check reads. */
|
||||
function makeFullRegistry(overrides: {
|
||||
loaders?: LoadNodesAndCredentials['loaders'];
|
||||
knownNodes?: LoadNodesAndCredentials['knownNodes'];
|
||||
knownCredentials?: LoadNodesAndCredentials['knownCredentials'];
|
||||
}): LoadNodesAndCredentials {
|
||||
return {
|
||||
loaders: overrides.loaders ?? {},
|
||||
knownNodes: overrides.knownNodes ?? {},
|
||||
knownCredentials: overrides.knownCredentials ?? {},
|
||||
} as unknown as LoadNodesAndCredentials;
|
||||
}
|
||||
|
||||
describe('package selector', () => {
|
||||
it('is true for a package that has a registered loader', () => {
|
||||
const registry = makeFullRegistry({
|
||||
loaders: { 'n8n-nodes-base': {} as LoadNodesAndCredentials['loaders'][string] },
|
||||
});
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, registry);
|
||||
|
||||
expect(isLoaded({ kind: 'package', value: 'n8n-nodes-base' })).toBe(true);
|
||||
});
|
||||
|
||||
it('is false for a package with no registered loader', () => {
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'package', value: 'n8n-nodes-not-installed' })).toBe(false);
|
||||
});
|
||||
|
||||
it('is false for a package named after an inherited Object.prototype property', () => {
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'package', value: 'constructor' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extends selector', () => {
|
||||
it('is true for a known credential type, checked the same way for either kind', () => {
|
||||
const registry = makeFullRegistry({
|
||||
knownCredentials: { oAuth2Api: { className: '', sourcePath: '' } },
|
||||
});
|
||||
const selector = { kind: 'extends', value: 'oAuth2Api' } as const;
|
||||
|
||||
expect(loadedCheckFor(NODE_TYPES_KIND, nodeTypes, registry)(selector)).toBe(true);
|
||||
expect(loadedCheckFor(CREDENTIAL_TYPES_KIND, {} as NodeTypes, registry)(selector)).toBe(true);
|
||||
});
|
||||
|
||||
it('is false for an unknown credential type', () => {
|
||||
const isLoaded = loadedCheckFor(CREDENTIAL_TYPES_KIND, {} as NodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'extends', value: 'unknownApi' })).toBe(false);
|
||||
});
|
||||
|
||||
it('is false for a value named after an inherited Object.prototype property', () => {
|
||||
const isLoaded = loadedCheckFor(CREDENTIAL_TYPES_KIND, {} as NodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'extends', value: 'constructor' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('name selector, node-types kind', () => {
|
||||
it('is true when the resolved base name is a known node', () => {
|
||||
const registry = makeFullRegistry({
|
||||
knownNodes: { 'n8n-nodes-base.slack': { className: '', sourcePath: '' } },
|
||||
});
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, registry);
|
||||
|
||||
expect(isLoaded({ kind: 'name', value: 'n8n-nodes-base.slack' })).toBe(true);
|
||||
});
|
||||
|
||||
it('is false when the resolved base name is not a known node', () => {
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'name', value: 'n8n-nodes-base.slack' })).toBe(false);
|
||||
});
|
||||
|
||||
it('treats a synthetic tool variant as installed when its base node is known', () => {
|
||||
const registry = makeFullRegistry({
|
||||
knownNodes: { 'n8n-nodes-base.slack': { className: '', sourcePath: '' } },
|
||||
});
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, registry);
|
||||
|
||||
expect(isLoaded({ kind: 'name', value: 'n8n-nodes-base.slackTool' })).toBe(true);
|
||||
});
|
||||
|
||||
it('is false for a name resolving to an inherited Object.prototype property', () => {
|
||||
const isLoaded = loadedCheckFor(NODE_TYPES_KIND, nodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'name', value: 'constructor' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('name selector, credential-types kind', () => {
|
||||
it('is true when the value is a known credential type', () => {
|
||||
const registry = makeFullRegistry({
|
||||
knownCredentials: { slackApi: { className: '', sourcePath: '' } },
|
||||
});
|
||||
const isLoaded = loadedCheckFor(CREDENTIAL_TYPES_KIND, {} as NodeTypes, registry);
|
||||
|
||||
expect(isLoaded({ kind: 'name', value: 'slackApi' })).toBe(true);
|
||||
});
|
||||
|
||||
it('is false when the value is not a known credential type', () => {
|
||||
const isLoaded = loadedCheckFor(CREDENTIAL_TYPES_KIND, {} as NodeTypes, makeFullRegistry({}));
|
||||
|
||||
expect(isLoaded({ kind: 'name', value: 'unknownApi' })).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,98 @@
|
|||
import { lintPolicyRules, lintRulesForMissingTypes } from '../policy-rule-lint';
|
||||
import type { PolicyRule, PolicySelector } from '../policy-rule.types';
|
||||
|
||||
type RuleOverrides = Partial<PolicyRule> & Pick<PolicyRule, 'id' | 'selector'>;
|
||||
|
||||
const rule = (overrides: RuleOverrides): PolicyRule => ({
|
||||
action: 'allow',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe('lintRulesForMissingTypes', () => {
|
||||
it('returns no warnings when every rule is installed', () => {
|
||||
const rules = [
|
||||
rule({ id: 'a', selector: { kind: 'name', value: 'typeA' } }),
|
||||
rule({ id: 'b', selector: { kind: 'package', value: 'n8n-nodes-base' } }),
|
||||
];
|
||||
|
||||
expect(lintRulesForMissingTypes(rules, () => true)).toEqual([]);
|
||||
});
|
||||
|
||||
it('flags each rule whose selector is not installed, carrying its id', () => {
|
||||
const rules = [
|
||||
rule({ id: 'installed', selector: { kind: 'name', value: 'typeA' } }),
|
||||
rule({ id: 'missing-one', selector: { kind: 'name', value: 'typeB' } }),
|
||||
rule({ id: 'missing-two', selector: { kind: 'package', value: 'n8n-nodes-missing' } }),
|
||||
];
|
||||
const isLoaded = (selector: PolicySelector) => selector.value === 'typeA';
|
||||
|
||||
expect(lintRulesForMissingTypes(rules, isLoaded)).toEqual([
|
||||
{ kind: 'notLoaded', ruleId: 'missing-one' },
|
||||
{ kind: 'notLoaded', ruleId: 'missing-two' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns no warnings for an empty rule list', () => {
|
||||
expect(lintRulesForMissingTypes([], () => false)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('lintPolicyRules', () => {
|
||||
const policedType = (name: string) => ({ name, baseName: name });
|
||||
const resolvePackage = () => null;
|
||||
|
||||
it('returns no warnings when every rule is installed and none are shadowed', () => {
|
||||
const rules = [
|
||||
rule({ id: 'a', selector: { kind: 'name', value: 'typeA' } }),
|
||||
rule({ id: 'b', selector: { kind: 'name', value: 'typeB' } }),
|
||||
];
|
||||
|
||||
expect(lintPolicyRules(rules, { resolvePackage, policedType, isLoaded: () => true })).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it('flags not-installed rules that are not shadowed, in rule order', () => {
|
||||
const rules = [
|
||||
rule({ id: 'a', selector: { kind: 'name', value: 'typeA' } }),
|
||||
rule({ id: 'b', selector: { kind: 'name', value: 'typeB' } }),
|
||||
];
|
||||
const isLoaded = (selector: PolicySelector) => selector.value !== 'typeB';
|
||||
|
||||
expect(lintPolicyRules(rules, { resolvePackage, policedType, isLoaded })).toEqual([
|
||||
{ kind: 'notLoaded', ruleId: 'b' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('merges shadow and not-installed warnings, sorted by rule position rather than warning kind', () => {
|
||||
const rules = [
|
||||
rule({ id: 'a', selector: { kind: 'name', value: 'typeA' } }),
|
||||
rule({ id: 'b', selector: { kind: 'name', value: 'typeB' } }),
|
||||
// identical selector to `a`, so shadowed by it
|
||||
rule({ id: 'c', selector: { kind: 'name', value: 'typeA' } }),
|
||||
];
|
||||
const isLoaded = (selector: PolicySelector) => selector.value !== 'typeB';
|
||||
|
||||
expect(lintPolicyRules(rules, { resolvePackage, policedType, isLoaded })).toEqual([
|
||||
{ kind: 'notLoaded', ruleId: 'b' },
|
||||
{ kind: 'shadowed', ruleId: 'c', shadowedByRuleId: 'a' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('lists the shadowed warning before the notLoaded warning for the same rule', () => {
|
||||
const ruleX = rule({ id: 'x', selector: { kind: 'name', value: 'typeX' } });
|
||||
// identical selector to `ruleX`, so shadowed by it, and separately stubbed as not installed
|
||||
const ruleY = rule({ id: 'y', selector: { kind: 'name', value: 'typeX' } });
|
||||
const notLoadedSelectors = new Set<PolicySelector>([ruleY.selector]);
|
||||
const isLoaded = (selector: PolicySelector) => !notLoadedSelectors.has(selector);
|
||||
|
||||
expect(lintPolicyRules([ruleX, ruleY], { resolvePackage, policedType, isLoaded })).toEqual([
|
||||
{ kind: 'shadowed', ruleId: 'y', shadowedByRuleId: 'x' },
|
||||
{ kind: 'notLoaded', ruleId: 'y' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns no warnings for an empty rule list', () => {
|
||||
expect(lintPolicyRules([], { resolvePackage, policedType, isLoaded: () => true })).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
@ -26,7 +26,7 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(rules)).toEqual([
|
||||
{ ruleId: 'deny-slack', shadowedByRuleId: 'allow-package' },
|
||||
{ kind: 'shadowed', ruleId: 'deny-slack', shadowedByRuleId: 'allow-package' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -66,7 +66,7 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(rules, undefined, policedType)).toEqual([
|
||||
{ ruleId: 'deny-gmail-tool', shadowedByRuleId: 'allow-gmail' },
|
||||
{ kind: 'shadowed', ruleId: 'deny-gmail-tool', shadowedByRuleId: 'allow-gmail' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -109,7 +109,7 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(rules)).toEqual([
|
||||
{ ruleId: 'deny-slack', shadowedByRuleId: 'allow-slack' },
|
||||
{ kind: 'shadowed', ruleId: 'deny-slack', shadowedByRuleId: 'allow-slack' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -121,8 +121,8 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(rules)).toEqual([
|
||||
{ ruleId: 'second-package', shadowedByRuleId: 'first-package' },
|
||||
{ ruleId: 'later-name', shadowedByRuleId: 'first-package' },
|
||||
{ kind: 'shadowed', ruleId: 'second-package', shadowedByRuleId: 'first-package' },
|
||||
{ kind: 'shadowed', ruleId: 'later-name', shadowedByRuleId: 'first-package' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -137,7 +137,7 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(rules, resolvePackage)).toEqual([
|
||||
{ ruleId: 'deny-slack-api', shadowedByRuleId: 'allow-package' },
|
||||
{ kind: 'shadowed', ruleId: 'deny-slack-api', shadowedByRuleId: 'allow-package' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -162,7 +162,7 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(nameFirst)).toEqual([
|
||||
{ ruleId: 'later-name', shadowedByRuleId: 'earlier-name' },
|
||||
{ kind: 'shadowed', ruleId: 'later-name', shadowedByRuleId: 'earlier-name' },
|
||||
]);
|
||||
|
||||
const packageFirst = [
|
||||
|
|
@ -172,8 +172,8 @@ describe('lintRulesForShadowing', () => {
|
|||
];
|
||||
|
||||
expect(lintRulesForShadowing(packageFirst)).toEqual([
|
||||
{ ruleId: 'later-name-1', shadowedByRuleId: 'earlier-package' },
|
||||
{ ruleId: 'later-name-2', shadowedByRuleId: 'earlier-package' },
|
||||
{ kind: 'shadowed', ruleId: 'later-name-1', shadowedByRuleId: 'earlier-package' },
|
||||
{ kind: 'shadowed', ruleId: 'later-name-2', shadowedByRuleId: 'earlier-package' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -196,18 +196,18 @@ describe('lintRulesForShadowing', () => {
|
|||
it('flags a later name rule for a type built on an earlier family rule', () => {
|
||||
expect(
|
||||
lint([family('oauth2', 'oAuth2Api'), name('sheets', 'googleSheetsOAuth2Api')]),
|
||||
).toEqual([{ ruleId: 'sheets', shadowedByRuleId: 'oauth2' }]);
|
||||
).toEqual([{ kind: 'shadowed', ruleId: 'sheets', shadowedByRuleId: 'oauth2' }]);
|
||||
});
|
||||
|
||||
it('flags a later name rule for the family base itself', () => {
|
||||
expect(lint([family('oauth2', 'oAuth2Api'), name('generic', 'oAuth2Api')])).toEqual([
|
||||
{ ruleId: 'generic', shadowedByRuleId: 'oauth2' },
|
||||
{ kind: 'shadowed', ruleId: 'generic', shadowedByRuleId: 'oauth2' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('flags a later family rule nested inside an earlier one', () => {
|
||||
expect(lint([family('oauth2', 'oAuth2Api'), family('google', 'googleOAuth2Api')])).toEqual([
|
||||
{ ruleId: 'google', shadowedByRuleId: 'oauth2' },
|
||||
{ kind: 'shadowed', ruleId: 'google', shadowedByRuleId: 'oauth2' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -46,6 +46,8 @@ const RULE: PolicyRule = {
|
|||
selector: { kind: 'name', value: 'n8n-nodes-base.slack' },
|
||||
};
|
||||
|
||||
const RULE_2: PolicyRule = { ...RULE, id: 'r2' };
|
||||
|
||||
const DELEGATE_RULE: PolicyRule = {
|
||||
id: 'r-delegate',
|
||||
action: 'delegate',
|
||||
|
|
@ -113,6 +115,11 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
scopeRepository.findScopeKeysByIds.mockResolvedValue([]);
|
||||
// Every fixture rule names `n8n-nodes-base`, so it must resolve as an installed package.
|
||||
loadNodesAndCredentials.loaders = { 'n8n-nodes-base': makeLoader('n8n-nodes-base') };
|
||||
// Fixture `name` rules name Slack, so it must resolve as a loaded node.
|
||||
Object.defineProperty(loadNodesAndCredentials, 'knownNodes', {
|
||||
configurable: true,
|
||||
value: { 'n8n-nodes-base.slack': { className: '', sourcePath: '' } },
|
||||
});
|
||||
nodeTypes.resolveBaseName.mockImplementation((name) => ({
|
||||
baseName: name,
|
||||
isSyntheticTool: false,
|
||||
|
|
@ -315,21 +322,72 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
|
||||
const { warnings } = await service.createPolicyDocument(KIND, [RULE, shadowed], 'user-1');
|
||||
|
||||
expect(warnings).toEqual([{ ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
expect(warnings).toEqual([{ kind: 'shadowed', ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
expect(policyRepository.createPolicy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a package rule naming a package that is not installed', async () => {
|
||||
it('saves a package rule naming a package that is not installed, with a warning', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(makePolicy());
|
||||
const rule: PolicyRule = {
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'package', value: 'n8n-nodes-not-installed' },
|
||||
};
|
||||
|
||||
await expect(service.createPolicyDocument(KIND, [rule], 'user-1')).rejects.toThrow(
|
||||
'Package rule names a package that is not installed: n8n-nodes-not-installed',
|
||||
const { warnings } = await service.createPolicyDocument(KIND, [rule], 'user-1');
|
||||
|
||||
expect(warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r1' }]);
|
||||
expect(policyRepository.createPolicy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('saves a name rule naming a node type that is not loaded, with a warning', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(makePolicy());
|
||||
const rule: PolicyRule = {
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'name', value: 'n8n-nodes-base.notLoaded' },
|
||||
};
|
||||
|
||||
const { warnings } = await service.createPolicyDocument(KIND, [rule], 'user-1');
|
||||
|
||||
expect(warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r1' }]);
|
||||
});
|
||||
|
||||
it('checks a synthetic tool variant rule against its base node', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(makePolicy());
|
||||
nodeTypes.resolveBaseName.mockImplementation((name) => ({
|
||||
baseName: name.replace(/Tool$/, ''),
|
||||
isSyntheticTool: name.endsWith('Tool'),
|
||||
}));
|
||||
const rules: PolicyRule[] = [
|
||||
{ id: 'r1', action: 'deny', selector: { kind: 'name', value: 'n8n-nodes-base.slackTool' } },
|
||||
{ id: 'r2', action: 'deny', selector: { kind: 'name', value: 'n8n-nodes-base.fooTool' } },
|
||||
];
|
||||
|
||||
const { warnings } = await service.createPolicyDocument(KIND, rules, 'user-1');
|
||||
|
||||
expect(warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r2' }]);
|
||||
});
|
||||
|
||||
it('lists a shadowed warning before a notLoaded warning for the same rule, in rule order', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(makePolicy());
|
||||
const missing = (id: string): PolicyRule => ({
|
||||
id,
|
||||
action: 'deny',
|
||||
selector: { kind: 'name', value: 'n8n-nodes-base.notLoaded' },
|
||||
});
|
||||
|
||||
const { warnings } = await service.createPolicyDocument(
|
||||
KIND,
|
||||
[missing('r1'), RULE_2, missing('r3')],
|
||||
'user-1',
|
||||
);
|
||||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
|
||||
expect(warnings).toEqual([
|
||||
{ kind: 'notLoaded', ruleId: 'r1' },
|
||||
{ kind: 'shadowed', ruleId: 'r3', shadowedByRuleId: 'r1' },
|
||||
{ kind: 'notLoaded', ruleId: 'r3' },
|
||||
]);
|
||||
});
|
||||
|
||||
describe('extends rules', () => {
|
||||
|
|
@ -360,17 +418,37 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects an extends rule naming a credential type that is not installed', async () => {
|
||||
await expect(
|
||||
service.createPolicyDocument(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
[familyRule('r1', 'notInstalledApi')],
|
||||
'user-1',
|
||||
),
|
||||
).rejects.toThrow(
|
||||
'Extends rule names a credential type that is not installed: notInstalledApi',
|
||||
it('saves an extends rule naming a credential type that is not installed, with a warning', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(
|
||||
makePolicy({ kind: CREDENTIAL_TYPES_KIND }),
|
||||
);
|
||||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
|
||||
const { warnings } = await service.createPolicyDocument(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
[familyRule('r1', 'notLoadedApi')],
|
||||
'user-1',
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r1' }]);
|
||||
});
|
||||
|
||||
it('saves a credential name rule naming a type that is not installed, with a warning', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(
|
||||
makePolicy({ kind: CREDENTIAL_TYPES_KIND }),
|
||||
);
|
||||
const rule: PolicyRule = {
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'name', value: 'notLoadedApi' },
|
||||
};
|
||||
|
||||
const { warnings } = await service.createPolicyDocument(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
[rule],
|
||||
'user-1',
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r1' }]);
|
||||
});
|
||||
|
||||
it('warns about a derived-type rule placed after the family rule that covers it', async () => {
|
||||
|
|
@ -389,7 +467,7 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
'user-1',
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([{ ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
expect(warnings).toEqual([{ kind: 'shadowed', ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -438,7 +516,7 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
};
|
||||
|
||||
await expect(service.createPolicyDocument(KIND, [rule], 'user-1')).rejects.toThrow(
|
||||
'Package rule names a package that is not installed: n8n-creds-base',
|
||||
'Package rule names "n8n-creds-base", which no package provides. Write a credential type rule instead.',
|
||||
);
|
||||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
|
@ -451,19 +529,29 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
scopeRepository.containsProjectScope.mockResolvedValue(false);
|
||||
});
|
||||
|
||||
it('rejects a package rule naming a package that is not installed, before opening a transaction', async () => {
|
||||
const rule: PolicyRule = {
|
||||
id: 'r1',
|
||||
it('keeps accepting edits to a document whose package rule names an uninstalled package', async () => {
|
||||
const missingPackage: PolicyRule = {
|
||||
id: 'r-missing',
|
||||
action: 'deny',
|
||||
selector: { kind: 'package', value: 'n8n-nodes-not-installed' },
|
||||
selector: { kind: 'package', value: 'n8n-nodes-mcp' },
|
||||
};
|
||||
|
||||
await expect(
|
||||
service.updatePolicyDocument(KIND, 'policy-1', [rule], 1, 'user-1'),
|
||||
).rejects.toThrow(
|
||||
'Package rule names a package that is not installed: n8n-nodes-not-installed',
|
||||
policyRepository.findByIdAndKind.mockResolvedValue(
|
||||
makePolicy({ rules: [missingPackage], version: 1 }),
|
||||
);
|
||||
expect(transactionRunner.run).not.toHaveBeenCalled();
|
||||
policyRepository.updateRules.mockResolvedValue(
|
||||
makePolicy({ rules: [missingPackage, RULE], version: 2 }),
|
||||
);
|
||||
|
||||
const { warnings } = await service.updatePolicyDocument(
|
||||
KIND,
|
||||
'policy-1',
|
||||
[missingPackage, RULE],
|
||||
1,
|
||||
'user-1',
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r-missing' }]);
|
||||
expect(policyRepository.updateRules).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a node type rule naming a credential-only node, before opening a transaction', async () => {
|
||||
|
|
@ -868,11 +956,11 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue(['scope-1']);
|
||||
});
|
||||
|
||||
it('rejects a package rule naming a package that is not installed, before opening a transaction', async () => {
|
||||
it('rejects a package rule on the credential-only prefix, before opening a transaction', async () => {
|
||||
const rule: PolicyRule = {
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'package', value: 'n8n-nodes-not-installed' },
|
||||
selector: { kind: 'package', value: 'n8n-creds-base' },
|
||||
};
|
||||
|
||||
await expect(
|
||||
|
|
@ -883,9 +971,7 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
0,
|
||||
'user-1',
|
||||
),
|
||||
).rejects.toThrow(
|
||||
'Package rule names a package that is not installed: n8n-nodes-not-installed',
|
||||
);
|
||||
).rejects.toThrow('Package rule names "n8n-creds-base"');
|
||||
expect(transactionRunner.run).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -1137,7 +1223,8 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [RULE],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -1188,10 +1275,47 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
after: { defaultAction: 'deny', version: 2 },
|
||||
rulesBefore: [],
|
||||
rulesAfter: [RULE],
|
||||
warningCount: 0,
|
||||
shadowWarningCount: 0,
|
||||
notLoadedWarningCount: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('saves a rule for an uninstalled package, returns its warning and counts it apart', async () => {
|
||||
const missingPackage: PolicyRule = {
|
||||
id: 'r-missing',
|
||||
action: 'deny',
|
||||
selector: { kind: 'package', value: 'n8n-nodes-mcp' },
|
||||
};
|
||||
const scope = makeScope({ defaultAction: 'allow', version: 1 });
|
||||
scopeRepository.findScopeByKindAndProject.mockResolvedValue(scope);
|
||||
const existingPolicy = makePolicy({ rules: [missingPackage], version: 1 });
|
||||
attachmentRepository.listAttachmentsForScope.mockResolvedValue([
|
||||
{ policyId: existingPolicy.id, rules: [missingPackage], priority: 0, isFloor: false },
|
||||
]);
|
||||
policyRepository.findByIdAndKind.mockResolvedValue(existingPolicy);
|
||||
policyRepository.updateRules.mockResolvedValue(
|
||||
makePolicy({ rules: [missingPackage, RULE, RULE_2], version: 2 }),
|
||||
);
|
||||
scopeRepository.findScopeById.mockResolvedValue(makeScope({ version: 2 }));
|
||||
|
||||
const result = await service.setEffectivePolicy(
|
||||
KIND,
|
||||
null,
|
||||
{ rules: [missingPackage, RULE, RULE_2], defaultAction: 'allow' },
|
||||
1,
|
||||
'user-2',
|
||||
);
|
||||
|
||||
expect(result.warnings).toEqual([
|
||||
{ kind: 'notLoaded', ruleId: 'r-missing' },
|
||||
{ kind: 'shadowed', ruleId: 'r2', shadowedByRuleId: 'r1' },
|
||||
]);
|
||||
expect(eventService.emit).toHaveBeenCalledWith(
|
||||
'node-type-policy-saved',
|
||||
expect.objectContaining({ shadowWarningCount: 1, notLoadedWarningCount: 1 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('throws NotFoundError when the document update unexpectedly finds no row', async () => {
|
||||
const scope = makeScope({ defaultAction: 'allow', version: 1 });
|
||||
scopeRepository.findScopeByKindAndProject.mockResolvedValue(scope);
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import {
|
|||
type PackageResolver,
|
||||
type PolicedType,
|
||||
} from './policy-evaluator';
|
||||
import type { PolicySelector } from './policy-rule.types';
|
||||
|
||||
/**
|
||||
* A credential type name (e.g. `slackApi`) carries no package prefix, unlike a node type
|
||||
|
|
@ -93,7 +94,7 @@ export function policedTypeFor(
|
|||
});
|
||||
}
|
||||
|
||||
/** Whether a credential type is known, checked at write time for an `extends` rule's value. */
|
||||
/** Whether a credential type is known, checked at write time for a rule's value. */
|
||||
export function isCredentialTypeKnown(
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
typeName: string,
|
||||
|
|
@ -102,8 +103,7 @@ export function isCredentialTypeKnown(
|
|||
}
|
||||
|
||||
/**
|
||||
* Whether `packageName` names a package this instance actually has loaded — checked at write
|
||||
* time so a `package` rule can never be saved unable to match anything.
|
||||
* Whether `packageName` names a package this instance actually has loaded.
|
||||
*
|
||||
* `Object.hasOwn`, not `in`: `in` also matches an inherited property, so a rule naming
|
||||
* `toString` or `constructor` would pass this check through `Object.prototype` even though
|
||||
|
|
@ -115,3 +115,30 @@ export function isPackageInstalled(
|
|||
): boolean {
|
||||
return Object.hasOwn(loadNodesAndCredentials.loaders, packageName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a selector names something this instance has loaded, for the write-time
|
||||
* `notLoaded` warning. A `name` rule on a synthetic tool variant checks its base node.
|
||||
*/
|
||||
export function loadedCheckFor(
|
||||
kind: string,
|
||||
nodeTypes: NodeTypes,
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
): (selector: PolicySelector) => boolean {
|
||||
const policedType = policedTypeFor(kind, nodeTypes, loadNodesAndCredentials);
|
||||
const isNameKnown =
|
||||
kind === NODE_TYPES_KIND
|
||||
? (name: string) => Object.hasOwn(loadNodesAndCredentials.knownNodes, name)
|
||||
: (name: string) => isCredentialTypeKnown(loadNodesAndCredentials, name);
|
||||
|
||||
return (selector) => {
|
||||
switch (selector.kind) {
|
||||
case 'package':
|
||||
return isPackageInstalled(loadNodesAndCredentials, selector.value);
|
||||
case 'extends':
|
||||
return isCredentialTypeKnown(loadNodesAndCredentials, selector.value);
|
||||
case 'name':
|
||||
return isNameKnown(policedType(selector.value).baseName);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,48 @@
|
|||
import type { PackageResolver, PolicedType } from './policy-evaluator';
|
||||
import type { PolicyRule, PolicySelector } from './policy-rule.types';
|
||||
import { lintRulesForShadowing, type ShadowWarning } from './policy-shadow-lint';
|
||||
|
||||
/**
|
||||
* `ruleId` names a package or type this instance has not loaded. The rule still applies by
|
||||
* name, for example to an imported workflow. `NODES_EXCLUDE` and `NODES_INCLUDE` count too.
|
||||
*/
|
||||
export type NotLoadedWarning = {
|
||||
readonly kind: 'notLoaded';
|
||||
readonly ruleId: string;
|
||||
};
|
||||
|
||||
export type PolicyRuleWarning = ShadowWarning | NotLoadedWarning;
|
||||
|
||||
/**
|
||||
* Warns instead of rejecting: a package can be uninstalled after its rule is saved, and the
|
||||
* rule must not block later, unrelated writes to the same policy.
|
||||
*/
|
||||
export function lintRulesForMissingTypes(
|
||||
rules: readonly PolicyRule[],
|
||||
isLoaded: (selector: PolicySelector) => boolean,
|
||||
): NotLoadedWarning[] {
|
||||
return rules
|
||||
.filter((rule) => !isLoaded(rule.selector))
|
||||
.map((rule) => ({ kind: 'notLoaded', ruleId: rule.id }));
|
||||
}
|
||||
|
||||
/**
|
||||
* Every warning for a rule list, in rule order. A rule with both kinds lists `shadowed` first,
|
||||
* since the sort is stable.
|
||||
*/
|
||||
export function lintPolicyRules(
|
||||
rules: readonly PolicyRule[],
|
||||
resolvers: {
|
||||
resolvePackage: PackageResolver;
|
||||
policedType: (name: string) => PolicedType;
|
||||
isLoaded: (selector: PolicySelector) => boolean;
|
||||
},
|
||||
): PolicyRuleWarning[] {
|
||||
const position = new Map(rules.map((rule, index) => [rule.id, index]));
|
||||
const warnings: PolicyRuleWarning[] = [
|
||||
...lintRulesForShadowing(rules, resolvers.resolvePackage, resolvers.policedType),
|
||||
...lintRulesForMissingTypes(rules, resolvers.isLoaded),
|
||||
];
|
||||
|
||||
return warnings.sort((a, b) => (position.get(a.ruleId) ?? 0) - (position.get(b.ruleId) ?? 0));
|
||||
}
|
||||
|
|
@ -10,6 +10,7 @@ import type { PolicyRule, PolicySelector } from './policy-rule.types';
|
|||
* appears earlier in the same rule list and matches every type that `ruleId` would match.
|
||||
*/
|
||||
export type ShadowWarning = {
|
||||
readonly kind: 'shadowed';
|
||||
readonly ruleId: string;
|
||||
readonly shadowedByRuleId: string;
|
||||
};
|
||||
|
|
@ -113,7 +114,7 @@ export function lintRulesForShadowing(
|
|||
}
|
||||
|
||||
if (shadowedBy) {
|
||||
warnings.push({ ruleId: rule.id, shadowedByRuleId: shadowedBy.rule.id });
|
||||
warnings.push({ kind: 'shadowed', ruleId: rule.id, shadowedByRuleId: shadowedBy.rule.id });
|
||||
}
|
||||
|
||||
const firstOccurrenceByValue = firstByKind[selector.kind];
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { TransactionRunner, type OperationContext } from '@n8n/db';
|
|||
import { Service } from '@n8n/di';
|
||||
import { LRUCache } from 'lru-cache';
|
||||
import {
|
||||
CREDENTIAL_ONLY_NODE_PREFIX,
|
||||
getCredentialOnlyNodeCredentialType,
|
||||
isCredentialOnlyNodeType,
|
||||
OperationalError,
|
||||
|
|
@ -22,12 +23,7 @@ import { TypeAvailabilityPolicyRepository } from './database/repositories/type-a
|
|||
import type { TypeAvailabilityPolicy } from './database/entities/type-availability-policy.entity';
|
||||
import type { TypeAvailabilityPolicyScope } from './database/entities/type-availability-policy-scope.entity';
|
||||
import { CREDENTIAL_TYPES_KIND, NODE_TYPES_KIND } from './constants';
|
||||
import {
|
||||
isCredentialTypeKnown,
|
||||
isPackageInstalled,
|
||||
packageResolverFor,
|
||||
policedTypeFor,
|
||||
} from './package-resolver';
|
||||
import { loadedCheckFor, packageResolverFor, policedTypeFor } from './package-resolver';
|
||||
import { evaluateComposedType, orderedAttachments, type ComposedVerdict } from './policy-evaluator';
|
||||
import type {
|
||||
PolicyAction,
|
||||
|
|
@ -35,7 +31,7 @@ import type {
|
|||
PolicyRule,
|
||||
PolicyScopeKey,
|
||||
} from './policy-rule.types';
|
||||
import { lintRulesForShadowing, type ShadowWarning } from './policy-shadow-lint';
|
||||
import { lintPolicyRules, type PolicyRuleWarning } from './policy-rule-lint';
|
||||
|
||||
/**
|
||||
* A scope that has never been written has no row. That "unconfigured" state behaves as
|
||||
|
|
@ -187,7 +183,7 @@ export type ComposedTypeEvaluation = {
|
|||
|
||||
type PolicyDocumentWrite = {
|
||||
readonly policy: TypeAvailabilityPolicy;
|
||||
readonly warnings: readonly ShadowWarning[];
|
||||
readonly warnings: readonly PolicyRuleWarning[];
|
||||
};
|
||||
|
||||
function flattenRules(attachments: readonly PolicyAttachment[]): PolicyRule[] {
|
||||
|
|
@ -226,33 +222,6 @@ function assertNoDelegateAtProjectScope(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A `package` rule that names a package this instance never loaded can never match anything —
|
||||
* worse than not supporting the rule at all, since it looks like it works. Checked at every
|
||||
* write so a policy is never saved with one.
|
||||
*/
|
||||
function assertPackagesInstalled(
|
||||
rules: readonly PolicyRule[],
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
): void {
|
||||
const missing = new Set<string>();
|
||||
|
||||
for (const rule of rules) {
|
||||
if (
|
||||
rule.selector.kind === 'package' &&
|
||||
!isPackageInstalled(loadNodesAndCredentials, rule.selector.value)
|
||||
) {
|
||||
missing.add(rule.selector.value);
|
||||
}
|
||||
}
|
||||
|
||||
if (missing.size > 0) {
|
||||
throw new UserError(
|
||||
`Package rule names a package that is not installed: ${[...missing].join(', ')}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A credential-only node (`n8n-creds-base.<type>`) exists only in the editor. It is stored and
|
||||
* executed as `n8n-nodes-base.httpRequest`, so a node type rule on the generated name would
|
||||
|
|
@ -273,30 +242,25 @@ function assertNoCredentialOnlyNodeRules(kind: string, rules: readonly PolicyRul
|
|||
}
|
||||
|
||||
/**
|
||||
* Only credential types declare `extends`, so the selector means nothing for another kind. An
|
||||
* unknown base would match nothing, the same trap `assertPackagesInstalled` closes.
|
||||
* No loader provides the credential-only prefix, so a `package` rule on it can never match on
|
||||
* any instance. Unlike a package that is merely not installed, nothing can fix it later.
|
||||
*/
|
||||
function assertExtendsSelectorsValid(
|
||||
kind: string,
|
||||
rules: readonly PolicyRule[],
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
): void {
|
||||
const values = rules.flatMap((rule) =>
|
||||
rule.selector.kind === 'extends' ? [rule.selector.value] : [],
|
||||
);
|
||||
if (values.length === 0) return;
|
||||
|
||||
if (kind !== CREDENTIAL_TYPES_KIND) {
|
||||
throw new UserError('An "extends" rule is only valid in a credential type policy');
|
||||
function assertNoCredentialOnlyPackageRules(rules: readonly PolicyRule[]): void {
|
||||
for (const rule of rules) {
|
||||
if (rule.selector.kind === 'package' && rule.selector.value === CREDENTIAL_ONLY_NODE_PREFIX) {
|
||||
throw new UserError(
|
||||
`Package rule names "${CREDENTIAL_ONLY_NODE_PREFIX}", which no package provides. Write a credential type rule instead.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const unknown = new Set(
|
||||
values.filter((value) => !isCredentialTypeKnown(loadNodesAndCredentials, value)),
|
||||
);
|
||||
if (unknown.size > 0) {
|
||||
throw new UserError(
|
||||
`Extends rule names a credential type that is not installed: ${[...unknown].join(', ')}`,
|
||||
);
|
||||
/** Only credential types declare `extends`, so the selector means nothing for another kind. */
|
||||
function assertExtendsSelectorsValid(kind: string, rules: readonly PolicyRule[]): void {
|
||||
if (kind === CREDENTIAL_TYPES_KIND) return;
|
||||
|
||||
if (rules.some((rule) => rule.selector.kind === 'extends')) {
|
||||
throw new UserError('An "extends" rule is only valid in a credential type policy');
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -490,19 +454,28 @@ export class TypeAvailabilityPolicyService {
|
|||
return scope;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rejects a rule that is malformed for this kind. A rule that only names something not
|
||||
* loaded saves, and comes back as a `notLoaded` warning.
|
||||
*/
|
||||
private validateAndLintRules(kind: string, rules: readonly PolicyRule[]): PolicyRuleWarning[] {
|
||||
assertNoCredentialOnlyPackageRules(rules);
|
||||
assertNoCredentialOnlyNodeRules(kind, rules);
|
||||
assertExtendsSelectorsValid(kind, rules);
|
||||
|
||||
return lintPolicyRules(rules, {
|
||||
resolvePackage: packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedType: policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
isLoaded: loadedCheckFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
});
|
||||
}
|
||||
|
||||
async createPolicyDocument(
|
||||
kind: string,
|
||||
rules: readonly PolicyRule[],
|
||||
updatedBy: string,
|
||||
): Promise<PolicyDocumentWrite> {
|
||||
assertPackagesInstalled(rules, this.loadNodesAndCredentials);
|
||||
assertNoCredentialOnlyNodeRules(kind, rules);
|
||||
assertExtendsSelectorsValid(kind, rules, this.loadNodesAndCredentials);
|
||||
const warnings = lintRulesForShadowing(
|
||||
rules,
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
);
|
||||
const warnings = this.validateAndLintRules(kind, rules);
|
||||
|
||||
const policy = await this.policyRepository.createPolicy({ kind, rules, updatedBy }, {});
|
||||
|
||||
|
|
@ -535,14 +508,7 @@ export class TypeAvailabilityPolicyService {
|
|||
expectedVersion: number,
|
||||
updatedBy: string,
|
||||
): Promise<PolicyDocumentWrite> {
|
||||
assertPackagesInstalled(rules, this.loadNodesAndCredentials);
|
||||
assertNoCredentialOnlyNodeRules(kind, rules);
|
||||
assertExtendsSelectorsValid(kind, rules, this.loadNodesAndCredentials);
|
||||
const warnings = lintRulesForShadowing(
|
||||
rules,
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
);
|
||||
const warnings = this.validateAndLintRules(kind, rules);
|
||||
|
||||
const result = await this.transactionRunner.run({}, async (ctx) => {
|
||||
let invalidated: PolicyScopeKey[] = [];
|
||||
|
|
@ -791,18 +757,10 @@ export class TypeAvailabilityPolicyService {
|
|||
defaultAction: PolicyAction;
|
||||
version: number;
|
||||
rules: PolicyRule[];
|
||||
warnings: readonly ShadowWarning[];
|
||||
warnings: readonly PolicyRuleWarning[];
|
||||
}> {
|
||||
assertNoDelegateAtProjectScope(projectId, input.defaultAction, input.rules);
|
||||
assertPackagesInstalled(input.rules, this.loadNodesAndCredentials);
|
||||
assertNoCredentialOnlyNodeRules(kind, input.rules);
|
||||
assertExtendsSelectorsValid(kind, input.rules, this.loadNodesAndCredentials);
|
||||
|
||||
const warnings = lintRulesForShadowing(
|
||||
input.rules,
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
);
|
||||
const warnings = this.validateAndLintRules(kind, input.rules);
|
||||
|
||||
const result = await this.transactionRunner.run({}, async (ctx) => {
|
||||
const scope = await this.scopeRepository.findScopeByKindAndProject(
|
||||
|
|
@ -991,7 +949,8 @@ export class TypeAvailabilityPolicyService {
|
|||
after: result.scopeAfter,
|
||||
rulesBefore: result.documentBefore?.rules ?? null,
|
||||
rulesAfter: result.documentAfter.rules,
|
||||
warningCount: warnings.length,
|
||||
shadowWarningCount: warnings.filter((warning) => warning.kind === 'shadowed').length,
|
||||
notLoadedWarningCount: warnings.filter((warning) => warning.kind === 'notLoaded').length,
|
||||
});
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -162,7 +162,7 @@ describe('CredentialTypePoliciesPublicController handler bodies', () => {
|
|||
defaultAction: 'deny' as const,
|
||||
version: 4,
|
||||
rules,
|
||||
warnings: [{ ruleId: 'r2', shadowedByRuleId: 'r1' }],
|
||||
warnings: [{ kind: 'shadowed' as const, ruleId: 'r2', shadowedByRuleId: 'r1' }],
|
||||
};
|
||||
|
||||
it('getCredentialTypeInstancePolicy reads the null-project scope and maps the response', async () => {
|
||||
|
|
|
|||
|
|
@ -157,7 +157,7 @@ describe('NodeTypePoliciesPublicController handler bodies', () => {
|
|||
defaultAction: 'deny' as const,
|
||||
version: 4,
|
||||
rules,
|
||||
warnings: [{ ruleId: 'r2', shadowedByRuleId: 'r1' }],
|
||||
warnings: [{ kind: 'shadowed' as const, ruleId: 'r2', shadowedByRuleId: 'r1' }],
|
||||
};
|
||||
|
||||
it('fails loudly instead of returning an extends rule, which no node type write accepts', async () => {
|
||||
|
|
|
|||
|
|
@ -88,15 +88,33 @@ properties:
|
|||
warnings:
|
||||
type: array
|
||||
items:
|
||||
oneOf:
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- shadowed
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- notLoaded
|
||||
ruleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
type: object
|
||||
properties:
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
required:
|
||||
- policy
|
||||
- warnings
|
||||
|
|
|
|||
|
|
@ -73,15 +73,33 @@ properties:
|
|||
warnings:
|
||||
type: array
|
||||
items:
|
||||
oneOf:
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- shadowed
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- notLoaded
|
||||
ruleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
type: object
|
||||
properties:
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
required:
|
||||
- scopeId
|
||||
- rules
|
||||
|
|
|
|||
|
|
@ -75,15 +75,33 @@ properties:
|
|||
warnings:
|
||||
type: array
|
||||
items:
|
||||
oneOf:
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- shadowed
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- notLoaded
|
||||
ruleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
type: object
|
||||
properties:
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
required:
|
||||
- policy
|
||||
- warnings
|
||||
|
|
|
|||
|
|
@ -60,15 +60,33 @@ properties:
|
|||
warnings:
|
||||
type: array
|
||||
items:
|
||||
oneOf:
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- shadowed
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- notLoaded
|
||||
ruleId:
|
||||
type: string
|
||||
required:
|
||||
- kind
|
||||
- ruleId
|
||||
type: object
|
||||
properties:
|
||||
ruleId:
|
||||
type: string
|
||||
shadowedByRuleId:
|
||||
type: string
|
||||
required:
|
||||
- ruleId
|
||||
- shadowedByRuleId
|
||||
required:
|
||||
- scopeId
|
||||
- rules
|
||||
|
|
|
|||
|
|
@ -183,19 +183,19 @@ describe('a rule on a base credential type', () => {
|
|||
});
|
||||
|
||||
describe('writing an extends rule', () => {
|
||||
test('rejects a base credential type that is not installed', async () => {
|
||||
test('saves a base credential type that is not installed, with a warning', async () => {
|
||||
const response = await ownerAgent.put('/credential-type-policies/instance').send({
|
||||
rules: [
|
||||
{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'notInstalledApi' } },
|
||||
],
|
||||
rules: [{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'notLoadedApi' } }],
|
||||
defaultAction: 'allow',
|
||||
version: 0,
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toBe(
|
||||
'Extends rule names a credential type that is not installed: notInstalledApi',
|
||||
);
|
||||
expect(response.statusCode).toBe(200);
|
||||
const persisted = await ownerAgent.get('/credential-type-policies/instance');
|
||||
expect(persisted.body.data.rules).toEqual([
|
||||
{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'notLoadedApi' } },
|
||||
]);
|
||||
expect(response.body.data.warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r1' }]);
|
||||
});
|
||||
|
||||
test('rejects the rule in a node type policy request', async () => {
|
||||
|
|
|
|||
|
|
@ -32,8 +32,7 @@ beforeAll(async () => {
|
|||
owner = await createOwner();
|
||||
member = await createMember();
|
||||
|
||||
// A `package` rule is rejected at write time unless the package is actually loaded — see
|
||||
// `n8n-nodes-base` used as a stand-in for "an installed package" below.
|
||||
// `n8n-nodes-base` stands in for an installed package, so its rules save without a warning.
|
||||
Container.get(LoadNodesAndCredentials).loaders = {
|
||||
'n8n-nodes-base': mock<NodeLoader>({ known: { nodes: {}, credentials: {} } }),
|
||||
};
|
||||
|
|
@ -236,6 +235,29 @@ describe('node type availability policy instance controller admin happy path', (
|
|||
expect(persisted.body.data.rules).toEqual([rule('r2', 'c.d')]);
|
||||
});
|
||||
|
||||
test('a package rule for an uninstalled package saves with a warning, and does not block later writes', async () => {
|
||||
const agent = testServer.authAgentFor(owner);
|
||||
const missingPackage = {
|
||||
id: 'r-mcp',
|
||||
action: 'deny' as const,
|
||||
selector: { kind: 'package' as const, value: 'n8n-nodes-mcp' },
|
||||
};
|
||||
|
||||
const first = await agent
|
||||
.put('/node-type-policies/instance')
|
||||
.send({ rules: [missingPackage], defaultAction: 'allow', version: 0 });
|
||||
expect(first.statusCode).toBe(200);
|
||||
expect(first.body.data.warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r-mcp' }]);
|
||||
|
||||
const unrelatedChange = await agent.put('/node-type-policies/instance').send({
|
||||
rules: [missingPackage],
|
||||
defaultAction: 'deny',
|
||||
version: first.body.data.version,
|
||||
});
|
||||
expect(unrelatedChange.statusCode).toBe(200);
|
||||
expect(unrelatedChange.body.data.warnings).toEqual([{ kind: 'notLoaded', ruleId: 'r-mcp' }]);
|
||||
});
|
||||
|
||||
test('a PUT /instance that changes the rules and the default action together moves the version by one', async () => {
|
||||
const agent = testServer.authAgentFor(owner);
|
||||
|
||||
|
|
|
|||
|
|
@ -61,6 +61,13 @@ afterEach(async () => {
|
|||
]);
|
||||
});
|
||||
|
||||
// Fixture rules name `slackApi`; without it loaded, every write would also warn `notLoaded`.
|
||||
beforeEach(() => {
|
||||
vi.spyOn(Container.get(LoadNodesAndCredentials), 'knownCredentials', 'get').mockReturnValue({
|
||||
slackApi: { className: '', sourcePath: '' },
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Least privilege first: the API-key scope check and the RBAC check are independent, so a key
|
||||
* that lacks the scope and a key whose user lacks the RBAC grant are both rejected with 403.
|
||||
|
|
@ -425,7 +432,9 @@ describe('credential type policies public API policy documents', () => {
|
|||
.send({ rules: [DENY_RULE, { ...DENY_RULE, id: 'r2', action: 'allow' }] });
|
||||
|
||||
expect(response.statusCode).toBe(201);
|
||||
expect(response.body.warnings).toEqual([{ ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
expect(response.body.warnings).toEqual([
|
||||
{ kind: 'shadowed', ruleId: 'r2', shadowedByRuleId: 'r1' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('POST rejects duplicate rule ids with 400', async () => {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { LICENSE_FEATURES } from '@n8n/constants';
|
|||
import type { Project, User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
|
||||
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import { TypeAvailabilityPolicyRepository } from '@/modules/type-availability-policies/database/repositories/type-availability-policy.repository';
|
||||
import { createMemberWithApiKey, createOwnerWithApiKey } from '@test-integration/db/users';
|
||||
import * as utils from '@test-integration/utils';
|
||||
|
|
@ -56,6 +57,13 @@ afterEach(async () => {
|
|||
]);
|
||||
});
|
||||
|
||||
// Fixture rules name `a.b`; without it loaded, every write would also warn `notLoaded`.
|
||||
beforeEach(() => {
|
||||
vi.spyOn(Container.get(LoadNodesAndCredentials), 'knownNodes', 'get').mockReturnValue({
|
||||
'a.b': { className: '', sourcePath: '' },
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Least privilege first: the API-key scope check and the RBAC check are independent, so a key
|
||||
* that lacks the scope and a key whose user lacks the RBAC grant are both rejected with 403.
|
||||
|
|
@ -416,7 +424,9 @@ describe('node type policies public API policy documents', () => {
|
|||
.send({ rules: [DENY_RULE, { ...DENY_RULE, id: 'r2', action: 'allow' }] });
|
||||
|
||||
expect(response.statusCode).toBe(201);
|
||||
expect(response.body.warnings).toEqual([{ ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
expect(response.body.warnings).toEqual([
|
||||
{ kind: 'shadowed', ruleId: 'r2', shadowedByRuleId: 'r1' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('POST rejects duplicate rule ids with 400', async () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user