mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
feat(core): Gate workflow content writes on a policy clearance token (no-changelog) (#37397)
This commit is contained in:
parent
19582fb20d
commit
f50f19d3fe
|
|
@ -0,0 +1,64 @@
|
|||
import { GlobalConfig } from '@n8n/config';
|
||||
import { mintPolicyCleared } from '@n8n/decorators/policy-internal';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { WorkflowEntity } from '../../entities';
|
||||
import type { TransactionRunner } from '../../services/transaction';
|
||||
import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager';
|
||||
import { mockInstance } from '../../utils/test-utils/mock-instance';
|
||||
import { FolderRepository } from '../folder.repository';
|
||||
import { SharedWorkflowRepository } from '../shared-workflow.repository';
|
||||
import { WorkflowHistoryRepository } from '../workflow-history.repository';
|
||||
import { WorkflowRepository } from '../workflow.repository';
|
||||
|
||||
describe('WorkflowRepository.updateContent', () => {
|
||||
const entityManager = mockEntityManager(WorkflowEntity);
|
||||
const workflowRepository = new WorkflowRepository(
|
||||
entityManager.connection,
|
||||
mockInstance(GlobalConfig, { database: { type: 'postgresdb' } }),
|
||||
mockInstance(FolderRepository),
|
||||
mockInstance(SharedWorkflowRepository),
|
||||
mockInstance(WorkflowHistoryRepository),
|
||||
mock<TransactionRunner>(),
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
|
||||
// The load-bearing red: the write must reach the manager only past the clearance gate.
|
||||
it('writes the content through the resolved manager when the clearance matches', async () => {
|
||||
const cleared = mintPolicyCleared({
|
||||
point: 'workflowSave',
|
||||
subject: { type: 'workflow', id: 'wf-1' },
|
||||
decision: { violations: [] },
|
||||
});
|
||||
|
||||
await workflowRepository.updateContent('wf-1', { name: 'renamed' }, { policyCleared: cleared });
|
||||
|
||||
expect(entityManager.update).toHaveBeenCalledWith(WorkflowEntity, 'wf-1', { name: 'renamed' });
|
||||
});
|
||||
|
||||
it('throws and writes nothing when the context carries no clearance', async () => {
|
||||
await expect(
|
||||
workflowRepository.updateContent('wf-1', { name: 'renamed' }, {}),
|
||||
).rejects.toThrow();
|
||||
|
||||
expect(entityManager.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The gate binds the assert to the id being written, not just to any clearance on the context.
|
||||
it('throws and writes nothing when the clearance is for a different workflow', async () => {
|
||||
const cleared = mintPolicyCleared({
|
||||
point: 'workflowSave',
|
||||
subject: { type: 'workflow', id: 'wf-1' },
|
||||
decision: { violations: [] },
|
||||
});
|
||||
|
||||
await expect(
|
||||
workflowRepository.updateContent('wf-2', { name: 'renamed' }, { policyCleared: cleared }),
|
||||
).rejects.toThrow();
|
||||
|
||||
expect(entityManager.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
import { GlobalConfig } from '@n8n/config';
|
||||
import { assertClearedFor } from '@n8n/decorators';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { Scope } from '@n8n/permissions';
|
||||
import { DataSource, In, Like, Not, IsNull } from '@n8n/typeorm';
|
||||
|
|
@ -11,6 +12,7 @@ import type {
|
|||
FindOptionsRelations,
|
||||
EntityManager,
|
||||
} from '@n8n/typeorm';
|
||||
import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity';
|
||||
import { PROJECT_ROOT, UserError } from 'n8n-workflow';
|
||||
|
||||
import { BaseRepository } from './base-repository';
|
||||
|
|
@ -167,6 +169,15 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
|
|||
return count > 0;
|
||||
}
|
||||
|
||||
async updateContent(
|
||||
id: string,
|
||||
content: QueryDeepPartialEntity<WorkflowEntity>,
|
||||
ctx: OperationContext,
|
||||
) {
|
||||
assertClearedFor(ctx.policyCleared, 'workflowSave', { type: 'workflow', id });
|
||||
await this.managerFor(ctx).update(WorkflowEntity, id, content);
|
||||
}
|
||||
|
||||
async findByCredentialResolverId(
|
||||
resolverId: string,
|
||||
): Promise<Array<Pick<WorkflowEntity, 'id' | 'name'>>> {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import type { EnforcementPoint, PolicyCleared } from '@n8n/decorators';
|
||||
|
||||
/**
|
||||
* Transaction ports — the TypeORM-agnostic surface business logic depends on.
|
||||
*
|
||||
|
|
@ -38,6 +40,7 @@ export abstract class Transaction {
|
|||
*/
|
||||
export interface OperationContext {
|
||||
readonly trx?: Transaction;
|
||||
readonly policyCleared?: PolicyCleared<EnforcementPoint>;
|
||||
}
|
||||
|
||||
export interface RunOptions {
|
||||
|
|
|
|||
|
|
@ -19,6 +19,18 @@
|
|||
"main": "dist/index.js",
|
||||
"module": "src/index.ts",
|
||||
"types": "dist/index.d.ts",
|
||||
"exports": {
|
||||
".": {
|
||||
"types": "./dist/index.d.ts",
|
||||
"import": "./src/index.ts",
|
||||
"require": "./dist/index.js"
|
||||
},
|
||||
"./policy-internal": {
|
||||
"types": "./dist/policy-check/policy-mint.d.ts",
|
||||
"import": "./src/policy-check/policy-mint.ts",
|
||||
"require": "./dist/policy-check/policy-mint.js"
|
||||
}
|
||||
},
|
||||
"files": [
|
||||
"dist/**/*"
|
||||
],
|
||||
|
|
|
|||
|
|
@ -0,0 +1,52 @@
|
|||
import { UnexpectedError } from 'n8n-workflow';
|
||||
|
||||
import type { PolicyDecision } from '../policy-check';
|
||||
import { assertClearedFor, type PolicySubject } from '../policy-cleared';
|
||||
import { mintPolicyCleared } from '../policy-mint';
|
||||
|
||||
const subject: PolicySubject = { type: 'workflow', id: 'wf-1' };
|
||||
const cleared: PolicyDecision = { violations: [] };
|
||||
|
||||
describe('assertClearedFor', () => {
|
||||
it('throws when there is no clearance token', () => {
|
||||
expect(() => assertClearedFor(undefined, 'workflowSave', subject)).toThrow(UnexpectedError);
|
||||
});
|
||||
|
||||
it('throws when the token cleared a different point', () => {
|
||||
const token = mintPolicyCleared({ point: 'workflowStart', subject, decision: cleared });
|
||||
|
||||
expect(() => assertClearedFor(token, 'workflowSave', subject)).toThrow(UnexpectedError);
|
||||
});
|
||||
|
||||
it('throws when the token cleared a different subject id', () => {
|
||||
const token = mintPolicyCleared({ point: 'workflowSave', subject, decision: cleared });
|
||||
|
||||
expect(() => assertClearedFor(token, 'workflowSave', { type: 'workflow', id: 'wf-2' })).toThrow(
|
||||
UnexpectedError,
|
||||
);
|
||||
});
|
||||
|
||||
it('throws when the token cleared a different subject type', () => {
|
||||
const token = mintPolicyCleared({
|
||||
point: 'workflowSave',
|
||||
subject: { type: 'credential', id: 'wf-1' },
|
||||
decision: cleared,
|
||||
});
|
||||
|
||||
expect(() => assertClearedFor(token, 'workflowSave', subject)).toThrow(UnexpectedError);
|
||||
});
|
||||
|
||||
it('throws on an object that was never minted (missing brand)', () => {
|
||||
const forged = { point: 'workflowSave', subject, decision: cleared, policyVersions: [] };
|
||||
|
||||
expect(() => assertClearedFor(forged as never, 'workflowSave', subject)).toThrow(
|
||||
UnexpectedError,
|
||||
);
|
||||
});
|
||||
|
||||
it('does not throw when point and subject both match', () => {
|
||||
const token = mintPolicyCleared({ point: 'workflowSave', subject, decision: cleared });
|
||||
|
||||
expect(() => assertClearedFor(token, 'workflowSave', subject)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,7 +1,9 @@
|
|||
import type { PolicyDecision } from '@n8n/decorators';
|
||||
import { UnexpectedError } from 'n8n-workflow';
|
||||
import { createHash } from 'node:crypto';
|
||||
|
||||
import { mintPolicyCleared, type PolicyCleared, type PolicySubject } from '../policy-cleared';
|
||||
import type { PolicedWorkflow, PolicyDecision } from '../policy-check';
|
||||
import type { PolicyCleared, PolicySubject } from '../policy-cleared';
|
||||
import { mintPolicyCleared, workflowSubject } from '../policy-mint';
|
||||
|
||||
const subject: PolicySubject = { type: 'workflow', id: 'wf-1' };
|
||||
|
||||
|
|
@ -78,3 +80,32 @@ describe('mintPolicyCleared', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('workflowSubject', () => {
|
||||
it('binds to the workflow id when it exists', () => {
|
||||
const workflow = { id: 'wf-9', name: 'x', nodes: [] } as unknown as PolicedWorkflow;
|
||||
|
||||
expect(workflowSubject(workflow)).toEqual({ type: 'workflow', id: 'wf-9' });
|
||||
});
|
||||
|
||||
it('binds a new workflow to the sha256 of its nodes', () => {
|
||||
const nodes = [{ name: 'Start' }] as unknown as PolicedWorkflow['nodes'];
|
||||
const workflow = { id: null, name: 'x', nodes } as unknown as PolicedWorkflow;
|
||||
const expected = createHash('sha256').update(JSON.stringify(nodes)).digest('hex');
|
||||
|
||||
expect(workflowSubject(workflow)).toEqual({ type: 'workflow', id: expected });
|
||||
});
|
||||
|
||||
it('gives the same id for the same nodes and a different id for different nodes', () => {
|
||||
const base = { id: null, name: 'x' };
|
||||
const a = workflowSubject({ ...base, nodes: [{ name: 'A' }] } as unknown as PolicedWorkflow);
|
||||
const aAgain = workflowSubject({
|
||||
...base,
|
||||
nodes: [{ name: 'A' }],
|
||||
} as unknown as PolicedWorkflow);
|
||||
const b = workflowSubject({ ...base, nodes: [{ name: 'B' }] } as unknown as PolicedWorkflow);
|
||||
|
||||
expect(a.id).toBe(aAgain.id);
|
||||
expect(a.id).not.toBe(b.id);
|
||||
});
|
||||
});
|
||||
|
|
@ -13,3 +13,4 @@ export {
|
|||
UnknownEnforcementPointError,
|
||||
} from './policy-check-metadata';
|
||||
export * from './policy-check';
|
||||
export * from './policy-cleared';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,7 @@
|
|||
/**
|
||||
* The clearance brand. It lives in its own module so that neither the public
|
||||
* barrel nor the `policy-internal` subpath re-exports it: the symbol is
|
||||
* unreachable outside `@n8n/decorators`, which is what makes a `PolicyCleared`
|
||||
* unforgeable — only code inside this package can produce the branded key.
|
||||
*/
|
||||
export const brand = Symbol('policyCleared');
|
||||
62
packages/@n8n/decorators/src/policy-check/policy-cleared.ts
Normal file
62
packages/@n8n/decorators/src/policy-check/policy-cleared.ts
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
import { UnexpectedError } from 'n8n-workflow';
|
||||
|
||||
import { brand } from './policy-brand';
|
||||
import type { EnforcementPoint, PolicyDecision, PolicyVersionRef } from './policy-check';
|
||||
|
||||
/**
|
||||
* What kind of thing was policed. Closed, so a typo is a compile error rather than a binding
|
||||
* that silently never matches. Open to widening: `contentImport` in particular is generic over
|
||||
* artifact kind, and adding a kind here touches no signature.
|
||||
*/
|
||||
export type PolicySubjectType = 'workflow' | 'credential';
|
||||
|
||||
/** What a decision was about. */
|
||||
export type PolicySubject = {
|
||||
readonly type: PolicySubjectType;
|
||||
|
||||
/** Stable id, or a content hash where there is none yet — a workflow being created. */
|
||||
readonly id: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Proof that policy enforcement cleared one specific action, returned by every `enforce*`
|
||||
* method. `evaluate*` never returns one: an advisory answer must not unlock anything.
|
||||
*
|
||||
* Only `mintPolicyCleared` (in `./policy-mint`, off the public barrel) can produce one, so
|
||||
* receiving one is evidence the checks ran and passed, and it records what was cleared so a
|
||||
* value made for another point or workflow can't stand in.
|
||||
*/
|
||||
export type PolicyCleared<Point extends EnforcementPoint> = {
|
||||
readonly [brand]: 'Use the policy enforcement service — a cleared action cannot be forged';
|
||||
|
||||
readonly point: Point;
|
||||
|
||||
readonly subject: PolicySubject;
|
||||
|
||||
/** Cleared, so `violations` is empty. Carries whatever else the envelope grows. */
|
||||
readonly decision: PolicyDecision;
|
||||
|
||||
/** `decision.policyVersions`, always present. */
|
||||
readonly policyVersions: readonly PolicyVersionRef[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Verifies a clearance is present, was minted for this exact point, and binds to this subject.
|
||||
* Called at the persistence boundary before a policed write.
|
||||
*
|
||||
* @throws UnexpectedError if the clearance is missing, unbranded (never minted), or bound to a
|
||||
* different point or subject — each means a call site failed to thread the right token.
|
||||
*/
|
||||
export function assertClearedFor(
|
||||
cleared: PolicyCleared<EnforcementPoint> | undefined,
|
||||
point: EnforcementPoint,
|
||||
subject: PolicySubject,
|
||||
): void {
|
||||
if (!cleared) throw new UnexpectedError(`No policy clearance for ${point}`);
|
||||
if (cleared[brand] === undefined || cleared.point !== point)
|
||||
throw new UnexpectedError(`Clearance is not valid for ${point}`);
|
||||
if (cleared.subject.type !== subject.type || cleared.subject.id !== subject.id)
|
||||
throw new UnexpectedError(
|
||||
`Clearance is for ${cleared.subject.type} ${cleared.subject.id}, not ${subject.type} ${subject.id}`,
|
||||
);
|
||||
}
|
||||
56
packages/@n8n/decorators/src/policy-check/policy-mint.ts
Normal file
56
packages/@n8n/decorators/src/policy-check/policy-mint.ts
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
import { UnexpectedError } from 'n8n-workflow';
|
||||
import { createHash } from 'node:crypto';
|
||||
|
||||
import { brand } from './policy-brand';
|
||||
import type { EnforcementPoint, PolicedWorkflow, PolicyDecision } from './policy-check';
|
||||
import type { PolicyCleared, PolicySubject } from './policy-cleared';
|
||||
|
||||
/**
|
||||
* The only way to produce a `PolicyCleared`. Kept off the public barrel and
|
||||
* reachable only through the `@n8n/decorators/policy-internal` subpath, so the
|
||||
* enforcement service is the single caller that can mint one.
|
||||
*
|
||||
* @throws UnexpectedError if the decision has violations, or records a check that never ran.
|
||||
* Either would make the value a lie — something objected, or something never got to look.
|
||||
*/
|
||||
export function mintPolicyCleared<Point extends EnforcementPoint>({
|
||||
point,
|
||||
subject,
|
||||
decision,
|
||||
}: {
|
||||
point: Point;
|
||||
subject: PolicySubject;
|
||||
decision: PolicyDecision;
|
||||
}): PolicyCleared<Point> {
|
||||
const refusal = `Refusing to clear ${point} for ${subject.type} ${subject.id}`;
|
||||
|
||||
if (decision.violations.length > 0) {
|
||||
throw new UnexpectedError(
|
||||
`${refusal}: decision has ${decision.violations.length} violation(s)`,
|
||||
);
|
||||
}
|
||||
|
||||
// `checkErrors` is `evaluate`-only by contract — under `enforce` a check that breaks blocks —
|
||||
// so one here means the backend has a bug, and clearing would hide that a check never ran.
|
||||
if (decision.checkErrors && decision.checkErrors.length > 0) {
|
||||
throw new UnexpectedError(`${refusal}: ${decision.checkErrors.length} check(s) failed to run`);
|
||||
}
|
||||
|
||||
return {
|
||||
[brand]: 'Use the policy enforcement service — a cleared action cannot be forged',
|
||||
point,
|
||||
subject,
|
||||
decision,
|
||||
policyVersions: decision.policyVersions ?? [],
|
||||
};
|
||||
}
|
||||
|
||||
/** A workflow being created has no id yet, so it binds to its nodes instead. */
|
||||
export function workflowSubject(workflow: PolicedWorkflow): PolicySubject {
|
||||
if (workflow.id !== null) return { type: 'workflow', id: workflow.id };
|
||||
|
||||
// Same object within one request, so key order is stable.
|
||||
const nodes = createHash('sha256').update(JSON.stringify(workflow.nodes)).digest('hex');
|
||||
|
||||
return { type: 'workflow', id: nodes };
|
||||
}
|
||||
|
|
@ -30,6 +30,7 @@ import { NoRestrictedSleepImportRule } from './no-restricted-sleep-import.js';
|
|||
import { NoRepositoryInPublicApiHandlerRule } from './no-repository-in-public-api-handler.js';
|
||||
import { RequirePublicApiControllerRule } from './require-public-api-controller.js';
|
||||
import { NoPublicApiGuardrailDisableRule } from './no-public-api-guardrail-disable.js';
|
||||
import { NoUnsealedWorkflowEntityWriteRule } from './no-unsealed-workflow-entity-write.js';
|
||||
|
||||
export const rules = {
|
||||
'no-uncaught-json-parse': NoUncaughtJsonParseRule,
|
||||
|
|
@ -63,4 +64,5 @@ export const rules = {
|
|||
'no-repository-in-public-api-handler': NoRepositoryInPublicApiHandlerRule,
|
||||
'require-public-api-controller': RequirePublicApiControllerRule,
|
||||
'no-public-api-guardrail-disable': NoPublicApiGuardrailDisableRule,
|
||||
'no-unsealed-workflow-entity-write': NoUnsealedWorkflowEntityWriteRule,
|
||||
} satisfies Record<string, AnyRuleModule>;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,95 @@
|
|||
import { RuleTester } from '@typescript-eslint/rule-tester';
|
||||
|
||||
// Syntactic detection: it flags WorkflowEntity writes by the shapes that actually occur — a
|
||||
// `WorkflowEntity` generic arg or first-arg identifier on an EntityManager/query-builder write,
|
||||
// a full-entity write on a `workflowRepository` receiver, or a `.update(…, { nodes })` payload.
|
||||
// Aliased/injected receivers and `getRepository(WorkflowEntity)` are the known ceiling (see the
|
||||
// rule's doc comment).
|
||||
import { NoUnsealedWorkflowEntityWriteRule } from './no-unsealed-workflow-entity-write.js';
|
||||
|
||||
const ruleTester = new RuleTester();
|
||||
|
||||
const businessLogic = '/repo/packages/cli/src/services/foo.service.ts';
|
||||
|
||||
ruleTester.run('no-unsealed-workflow-entity-write', NoUnsealedWorkflowEntityWriteRule, {
|
||||
valid: [
|
||||
// Persistence layer (path contains `@n8n/db`) may write WorkflowEntity directly.
|
||||
{
|
||||
code: 'manager.save<WorkflowEntity>(wf);',
|
||||
filename: '/repo/packages/@n8n/db/src/repositories/workflow.repository.ts',
|
||||
},
|
||||
// The sanctioned, token-gated write method is allowed.
|
||||
{ code: 'this.workflowRepository.updateContent(id, content, ctx);', filename: businessLogic },
|
||||
// A non-node partial update (active/archive/folder) is untouched.
|
||||
{ code: 'this.workflowRepository.update(id, { active: true });', filename: businessLogic },
|
||||
// A delete changes existence, not node content — out of the node seal's scope.
|
||||
{ code: 'this.workflowRepository.delete(id);', filename: businessLogic },
|
||||
// A raw EntityManager update to WorkflowEntity that touches no nodes (settings/active) is
|
||||
// out of the node seal's scope (the typeorm-boundary rule governs raw manager use).
|
||||
{
|
||||
code: 'trx.update(WorkflowEntity, { id: workflowId }, { settings });',
|
||||
filename: businessLogic,
|
||||
},
|
||||
{
|
||||
code: 'trx.update(WorkflowEntity, { id: workflowId }, { active: false });',
|
||||
filename: businessLogic,
|
||||
},
|
||||
// Writes to other entities are irrelevant.
|
||||
{ code: 'manager.save<CredentialsEntity>(cred);', filename: businessLogic },
|
||||
{ code: 'this.userRepository.save(user);', filename: businessLogic },
|
||||
// A sibling repository that merely ends in "workflowRepository" is a different entity.
|
||||
{ code: 'this.sharedWorkflowRepository.save(sw);', filename: businessLogic },
|
||||
{
|
||||
code: 'this.sharedWorkflowRepository.insert({ workflowId, projectId });',
|
||||
filename: businessLogic,
|
||||
},
|
||||
// Tests/fixtures write WorkflowEntity for setup — exempt.
|
||||
{
|
||||
code: 'await workflowRepository.save(workflow);',
|
||||
filename: '/repo/packages/cli/test/integration/foo.test.ts',
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
code: 'manager.save<WorkflowEntity>(wf);',
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
{
|
||||
code: "tx.upsert(WorkflowEntity, wf, ['id']);",
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
{
|
||||
code: 'transactionManager.save(WorkflowEntity, wf);',
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
{
|
||||
code: 'manager.createQueryBuilder().update(WorkflowEntity).set({ nodes: [] }).execute();',
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
{
|
||||
code: 'this.workflowRepository.save(wf);',
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
{
|
||||
code: 'this.workflowRepository.update(id, { nodes: [] });',
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
// A string-literal or computed `nodes` key is the same node write as `{ nodes }`.
|
||||
{
|
||||
code: "this.workflowRepository.update(id, { 'nodes': [] });",
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
{
|
||||
code: "this.workflowRepository.update(id, { ['nodes']: [] });",
|
||||
filename: businessLogic,
|
||||
errors: [{ messageId: 'unsealedWrite' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
|
@ -0,0 +1,100 @@
|
|||
import { ESLintUtils, type TSESTree } from '@typescript-eslint/utils';
|
||||
|
||||
// Node-content writes only. `delete`/`remove`/`softDelete`/`recover` change a row's existence,
|
||||
// not its node content, so the node seal does not govern them.
|
||||
const WRITE_METHODS = new Set(['save', 'insert', 'update', 'upsert']);
|
||||
|
||||
/** Tests and fixtures write `WorkflowEntity` directly for setup; the seal governs runtime code. */
|
||||
const NON_RUNTIME_FILE = /(\.test\.ts|\.spec\.ts|\/__tests__\/|\/test\/)/;
|
||||
|
||||
const isWorkflowEntity = (node: TSESTree.Node | undefined) =>
|
||||
node?.type === 'Identifier' && node.name === 'WorkflowEntity';
|
||||
|
||||
const receiverName = (node: TSESTree.Node): string | undefined => {
|
||||
if (node.type === 'Identifier') return node.name;
|
||||
if (node.type === 'MemberExpression' && node.property.type === 'Identifier')
|
||||
return node.property.name;
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const hasNodesKey = (node: TSESTree.Node) =>
|
||||
node.type === 'ObjectExpression' &&
|
||||
node.properties.some(
|
||||
(p) =>
|
||||
p.type === 'Property' &&
|
||||
((p.key.type === 'Identifier' && p.key.name === 'nodes') ||
|
||||
(p.key.type === 'Literal' && p.key.value === 'nodes')),
|
||||
);
|
||||
|
||||
/**
|
||||
* Seals `WorkflowEntity` node writes to the token-gated `WorkflowRepository` methods.
|
||||
*
|
||||
* Flags, outside `@n8n/db`, a write call (`save/insert/update/upsert`) that targets
|
||||
* `WorkflowEntity` — by a `WorkflowEntity` generic arg or first-arg identifier, a full-entity
|
||||
* write on the `workflowRepository`, or a `.update(…, { nodes })` payload.
|
||||
*
|
||||
* Syntactic by design. The known ceilings, all requiring type information to close, are:
|
||||
* - an aliased or injected receiver (`const r = this.workflowRepository`, or a repository
|
||||
* injected under any name other than `workflowRepository`) — the receiver match is by name;
|
||||
* - a hoisted payload (`.update(id, payload)` where `payload` names `nodes` off-site) — only
|
||||
* inline `{ nodes }` object literals are inspected;
|
||||
* - `getRepository(WorkflowEntity)` and raw-SQL string writes.
|
||||
* The runtime `assertClearedFor` gate in `WorkflowRepository.updateContent` is the enforcing
|
||||
* half; this rule is the compile-time backstop for the common shapes. The receiver match is
|
||||
* exact, so a sibling like `sharedWorkflowRepository` is untouched.
|
||||
*/
|
||||
export const NoUnsealedWorkflowEntityWriteRule = ESLintUtils.RuleCreator.withoutDocs({
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description: 'Seal WorkflowEntity writes behind a policy-cleared repository method.',
|
||||
},
|
||||
messages: {
|
||||
unsealedWrite:
|
||||
'This writes `WorkflowEntity` outside the sealed repository path. Route it through a token-gated `WorkflowRepository` method (e.g. `updateContent`).',
|
||||
},
|
||||
schema: [],
|
||||
},
|
||||
defaultOptions: [],
|
||||
create(context) {
|
||||
if (context.filename.includes('@n8n/db') || NON_RUNTIME_FILE.test(context.filename)) return {};
|
||||
return {
|
||||
CallExpression(node: TSESTree.CallExpression) {
|
||||
const { callee } = node;
|
||||
if (callee.type !== 'MemberExpression' || callee.property.type !== 'Identifier') return;
|
||||
const method = callee.property.name;
|
||||
if (!WRITE_METHODS.has(method)) return;
|
||||
|
||||
const typeArg = node.typeArguments?.params?.[0];
|
||||
const genericIsWorkflowEntity =
|
||||
typeArg?.type === 'TSTypeReference' &&
|
||||
typeArg.typeName.type === 'Identifier' &&
|
||||
typeArg.typeName.name === 'WorkflowEntity';
|
||||
const firstArgIsWorkflowEntity = isWorkflowEntity(node.arguments[0]);
|
||||
const onWorkflowRepo = receiverName(callee.object) === 'workflowRepository';
|
||||
const targetsWorkflow =
|
||||
genericIsWorkflowEntity || firstArgIsWorkflowEntity || onWorkflowRepo;
|
||||
if (!targetsWorkflow) return;
|
||||
|
||||
// Full-entity writes always carry `nodes`.
|
||||
if (method === 'save' || method === 'insert' || method === 'upsert') {
|
||||
context.report({ node, messageId: 'unsealedWrite' });
|
||||
return;
|
||||
}
|
||||
|
||||
if (method === 'update') {
|
||||
// Query-builder form `.update(WorkflowEntity)` — a raw write whose payload is a chained `.set()`.
|
||||
if (firstArgIsWorkflowEntity && node.arguments.length === 1) {
|
||||
context.report({ node, messageId: 'unsealedWrite' });
|
||||
return;
|
||||
}
|
||||
// A partial update is only a node write when its payload names `nodes`; a
|
||||
// settings-/active-only update to WorkflowEntity is out of the node seal's scope.
|
||||
if (node.arguments.some(hasNodesKey)) {
|
||||
context.report({ node, messageId: 'unsealedWrite' });
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
},
|
||||
});
|
||||
|
|
@ -13,6 +13,12 @@ const acknowledgedProjectOwnedEntities = [
|
|||
const INSTANCE_AI_LAZY_IMPORT_MESSAGE =
|
||||
'Use an existing lazy loader, or add one near first use. Static runtime imports of this dependency undo the Instance AI idle-memory guardrail.';
|
||||
|
||||
const POLICY_INTERNAL_RESTRICTION = {
|
||||
name: '@n8n/decorators/policy-internal',
|
||||
message:
|
||||
'Only PolicyEnforcementService may mint a policy clearance. Call enforce*/evaluate* instead.',
|
||||
};
|
||||
|
||||
const instanceAiLazyRuntimeImports = [
|
||||
'@joplin/turndown-plugin-gfm',
|
||||
'@mozilla/readability',
|
||||
|
|
@ -39,6 +45,15 @@ export default defineConfig(
|
|||
// Public API handler-pattern ratchet — the allowlist is the only escape hatch; block inline disables.
|
||||
'n8n-local-rules/no-public-api-guardrail-disable': 'error',
|
||||
'n8n-local-rules/no-type-unsafe-event-emitter': 'error',
|
||||
// Seal WorkflowEntity node-writes to the token-gated repository methods.
|
||||
// The allowlist below must shrink to empty; a new unsealed write fails CI.
|
||||
'n8n-local-rules/no-unsealed-workflow-entity-write': 'error',
|
||||
// The clearance minter lives on the `policy-internal` subpath, off the public barrel.
|
||||
// Only PolicyEnforcementService may reach it; callers use enforce*/evaluate*.
|
||||
'@typescript-eslint/no-restricted-imports': [
|
||||
'error',
|
||||
{ paths: [POLICY_INTERNAL_RESTRICTION] },
|
||||
],
|
||||
'n8n-local-rules/project-owned-entity-transfer': [
|
||||
'error',
|
||||
{ acknowledged: acknowledgedProjectOwnedEntities },
|
||||
|
|
@ -169,10 +184,27 @@ export default defineConfig(
|
|||
rules: {
|
||||
'@typescript-eslint/no-restricted-imports': [
|
||||
'error',
|
||||
{ paths: instanceAiLazyRuntimeImports },
|
||||
{ paths: [POLICY_INTERNAL_RESTRICTION, ...instanceAiLazyRuntimeImports] },
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
// Shrink-only ratchet: node-write sites not yet migrated to the sealed repository path.
|
||||
// NEVER add — a new unsealed write must fail CI. Remove each entry as its site migrates.
|
||||
files: [
|
||||
'./src/workflows/workflow-creation.service.ts',
|
||||
'./src/services/import.service.ts',
|
||||
'./src/modules/source-control.ee/source-control-import.service.ee.ts',
|
||||
'./src/modules/instance-ai/instance-ai.adapter.service.ts',
|
||||
'./src/modules/chat-hub/chat-hub-workflow.service.ts',
|
||||
],
|
||||
rules: { 'n8n-local-rules/no-unsealed-workflow-entity-write': 'off' },
|
||||
},
|
||||
{
|
||||
// Only the PEP may import the clearance minter.
|
||||
files: ['./src/policy/policy-enforcement.service.ts'],
|
||||
rules: { '@typescript-eslint/no-restricted-imports': 'off' },
|
||||
},
|
||||
{
|
||||
files: ['./src/databases/migrations/**/*.ts'],
|
||||
rules: {
|
||||
|
|
|
|||
|
|
@ -1,82 +0,0 @@
|
|||
import type { EnforcementPoint, PolicyDecision, PolicyVersionRef } from '@n8n/decorators';
|
||||
import { UnexpectedError } from 'n8n-workflow';
|
||||
|
||||
/** Unexported, so nothing outside this file can build a `PolicyCleared`. */
|
||||
const brand = Symbol('policyCleared');
|
||||
|
||||
/**
|
||||
* What kind of thing was policed. Closed, so a typo is a compile error rather than a binding
|
||||
* that silently never matches. Open to widening: `contentImport` in particular is generic over
|
||||
* artifact kind, and adding a kind here touches no signature.
|
||||
*/
|
||||
export type PolicySubjectType = 'workflow' | 'credential';
|
||||
|
||||
/** What a decision was about. */
|
||||
export type PolicySubject = {
|
||||
readonly type: PolicySubjectType;
|
||||
|
||||
/** Stable id, or a content hash where there is none yet — a workflow being created. */
|
||||
readonly id: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Proof that policy enforcement cleared one specific action, returned by every `enforce*`
|
||||
* method. `evaluate*` never returns one: an advisory answer must not unlock anything.
|
||||
*
|
||||
* Only `mintPolicyCleared` can produce one, so receiving one is evidence the checks ran and
|
||||
* passed, and it records what was cleared so a value made for another point or workflow can't
|
||||
* stand in. Nothing takes one as an argument yet — a return type today, versus six signature
|
||||
* changes later.
|
||||
*/
|
||||
export type PolicyCleared<Point extends EnforcementPoint> = {
|
||||
readonly [brand]: 'Use the policy enforcement service — a cleared action cannot be forged';
|
||||
|
||||
readonly point: Point;
|
||||
|
||||
readonly subject: PolicySubject;
|
||||
|
||||
/** Cleared, so `violations` is empty. Carries whatever else the envelope grows. */
|
||||
readonly decision: PolicyDecision;
|
||||
|
||||
/** `decision.policyVersions`, always present. */
|
||||
readonly policyVersions: readonly PolicyVersionRef[];
|
||||
};
|
||||
|
||||
/**
|
||||
* The only way to produce a `PolicyCleared`. Internal to `@/policy` — the enforcement service
|
||||
* being its only caller is what makes the type mean anything.
|
||||
*
|
||||
* @throws UnexpectedError if the decision has violations, or records a check that never ran.
|
||||
* Either would make the value a lie — something objected, or something never got to look.
|
||||
*/
|
||||
export function mintPolicyCleared<Point extends EnforcementPoint>({
|
||||
point,
|
||||
subject,
|
||||
decision,
|
||||
}: {
|
||||
point: Point;
|
||||
subject: PolicySubject;
|
||||
decision: PolicyDecision;
|
||||
}): PolicyCleared<Point> {
|
||||
const refusal = `Refusing to clear ${point} for ${subject.type} ${subject.id}`;
|
||||
|
||||
if (decision.violations.length > 0) {
|
||||
throw new UnexpectedError(
|
||||
`${refusal}: decision has ${decision.violations.length} violation(s)`,
|
||||
);
|
||||
}
|
||||
|
||||
// `checkErrors` is `evaluate`-only by contract — under `enforce` a check that breaks blocks —
|
||||
// so one here means the backend has a bug, and clearing would hide that a check never ran.
|
||||
if (decision.checkErrors && decision.checkErrors.length > 0) {
|
||||
throw new UnexpectedError(`${refusal}: ${decision.checkErrors.length} check(s) failed to run`);
|
||||
}
|
||||
|
||||
return {
|
||||
[brand]: 'Use the policy enforcement service — a cleared action cannot be forged',
|
||||
point,
|
||||
subject,
|
||||
decision,
|
||||
policyVersions: decision.policyVersions ?? [],
|
||||
};
|
||||
}
|
||||
|
|
@ -2,34 +2,24 @@ import type {
|
|||
ContentImportContext,
|
||||
CredentialDecryptContext,
|
||||
EnforcementPoint,
|
||||
PolicedWorkflow,
|
||||
PolicyDecision,
|
||||
WorkflowPublishContext,
|
||||
WorkflowSaveContext,
|
||||
WorkflowStartContext,
|
||||
WorkflowTransferContext,
|
||||
PolicyCleared,
|
||||
PolicySubject,
|
||||
} from '@n8n/decorators';
|
||||
import { mintPolicyCleared, workflowSubject } from '@n8n/decorators/policy-internal';
|
||||
import { Service } from '@n8n/di';
|
||||
import { UnexpectedError } from 'n8n-workflow';
|
||||
import { createHash } from 'node:crypto';
|
||||
|
||||
import { mintPolicyCleared, type PolicyCleared, type PolicySubject } from './policy-cleared';
|
||||
import type { PolicyContext, PolicyEnforcementBackend } from './policy-enforcement-backend';
|
||||
import { hasViolations, PolicyViolationError } from './policy-violation.error';
|
||||
|
||||
/** Fresh each time — `violations` is mutable. */
|
||||
const emptyDecision = (): PolicyDecision => ({ violations: [] });
|
||||
|
||||
/** A workflow being created has no id yet, so it binds to its nodes instead. */
|
||||
function workflowSubject(workflow: PolicedWorkflow): PolicySubject {
|
||||
if (workflow.id !== null) return { type: 'workflow', id: workflow.id };
|
||||
|
||||
// Same object within one request, so key order is stable.
|
||||
const nodes = createHash('sha256').update(JSON.stringify(workflow.nodes)).digest('hex');
|
||||
|
||||
return { type: 'workflow', id: nodes };
|
||||
}
|
||||
|
||||
/**
|
||||
* The policy enforcement point every host call site talks to.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ import type { EventService } from '@/events/event.service';
|
|||
import type { ExecutionPersistence } from '@/executions/execution-persistence';
|
||||
import type { ExternalHooks, WorkflowLifecycleHookActor } from '@/external-hooks';
|
||||
import type { RedactionEnforcementService } from '@/modules/redaction/redaction-enforcement.service';
|
||||
import type { PolicyCleared } from '@n8n/decorators';
|
||||
import { userHasScopes } from '@/permissions.ee/check-access';
|
||||
import type { PolicyEnforcementService } from '@/policy/policy-enforcement.service';
|
||||
import type { PollTriggerJobRegistrar } from '@/scheduling/poll-trigger-node/poll-trigger-job-registrar';
|
||||
|
|
@ -332,6 +333,7 @@ describe('WorkflowService', () => {
|
|||
let workflowHookContextServiceMock: MockProxy<WorkflowHookContextService>;
|
||||
let workflowRepositoryMock: MockProxy<{
|
||||
update: Mock;
|
||||
updateContent: Mock;
|
||||
findOne: Mock;
|
||||
}>;
|
||||
|
||||
|
|
@ -449,11 +451,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
versionId: expect.not.stringMatching('v1'),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -485,11 +488,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
versionId: 'v1',
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -617,11 +621,12 @@ describe('WorkflowService', () => {
|
|||
expect(userHasScopesMock).toHaveBeenCalledWith(user, ['workflow:enableRedaction'], false, {
|
||||
projectId: 'project-1',
|
||||
});
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.not.objectContaining({ redactionPolicy: 'all' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -640,11 +645,12 @@ describe('WorkflowService', () => {
|
|||
expect(userHasScopesMock).toHaveBeenCalledWith(user, ['workflow:enableRedaction'], false, {
|
||||
projectId: 'project-1',
|
||||
});
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.objectContaining({ redactionPolicy: 'all' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -685,11 +691,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.not.objectContaining({ redactionPolicy: 'all' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -706,11 +713,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.objectContaining({ redactionPolicy: 'all' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -771,7 +779,7 @@ describe('WorkflowService', () => {
|
|||
'none',
|
||||
undefined,
|
||||
);
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.objectContaining({
|
||||
|
|
@ -779,6 +787,7 @@ describe('WorkflowService', () => {
|
|||
timezone: 'Europe/Berlin',
|
||||
}),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -799,11 +808,12 @@ describe('WorkflowService', () => {
|
|||
'none',
|
||||
'none',
|
||||
);
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.objectContaining({ redactionPolicy: 'none' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -952,11 +962,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.not.objectContaining({ redactionPolicy: 'non-manual' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -972,11 +983,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.objectContaining({ redactionPolicy: 'non-manual' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -997,11 +1009,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.not.objectContaining({ redactionPolicy: 'manual-only' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -1022,11 +1035,12 @@ describe('WorkflowService', () => {
|
|||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
settings: expect.not.objectContaining({ redactionPolicy: 'manual-only' }),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -1972,7 +1986,7 @@ describe('WorkflowService', () => {
|
|||
let externalHooksMock: MockProxy<ExternalHooks>;
|
||||
let ownershipServiceMock: MockProxy<OwnershipService>;
|
||||
let licenseStateMock: MockProxy<LicenseState>;
|
||||
let workflowRepositoryMock: MockProxy<{ update: Mock; findOne: Mock }>;
|
||||
let workflowRepositoryMock: MockProxy<{ update: Mock; updateContent: Mock; findOne: Mock }>;
|
||||
|
||||
const WORKFLOW_ID = 'workflow-1';
|
||||
|
||||
|
|
@ -2098,9 +2112,10 @@ describe('WorkflowService', () => {
|
|||
WORKFLOW_ID,
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
WORKFLOW_ID,
|
||||
expect.not.objectContaining({ isArchived: expect.anything() }),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -2112,7 +2127,7 @@ describe('WorkflowService', () => {
|
|||
let ownershipServiceMock: MockProxy<OwnershipService>;
|
||||
let workflowHistoryServiceMock: MockProxy<WorkflowHistoryService>;
|
||||
let policyEnforcementServiceMock: MockProxy<PolicyEnforcementService>;
|
||||
let workflowRepositoryMock: MockProxy<{ update: Mock; findOne: Mock }>;
|
||||
let workflowRepositoryMock: MockProxy<{ update: Mock; updateContent: Mock; findOne: Mock }>;
|
||||
|
||||
const WORKFLOW_ID = 'workflow-1';
|
||||
const storedNodes = [{ name: 'Start' }] as unknown as INode[];
|
||||
|
|
@ -2229,7 +2244,26 @@ describe('WorkflowService', () => {
|
|||
WORKFLOW_ID,
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledTimes(1);
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// The content write must go through the token-gated `updateContent`, carrying the
|
||||
// clearance minted by `enforceWorkflowSave` on the context.
|
||||
it('persists the content through updateContent carrying the policy clearance', async () => {
|
||||
const cleared = mock<PolicyCleared<'workflowSave'>>();
|
||||
policyEnforcementServiceMock.enforceWorkflowSave.mockResolvedValue(cleared);
|
||||
|
||||
await workflowService.update(
|
||||
mock<User>({ id: 'user-1' }),
|
||||
{ nodes: [], connections: {} } as unknown as WorkflowEntity,
|
||||
WORKFLOW_ID,
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.updateContent).toHaveBeenCalledWith(
|
||||
WORKFLOW_ID,
|
||||
expect.anything(),
|
||||
{ policyCleared: cleared },
|
||||
);
|
||||
});
|
||||
|
||||
it('persists nothing when the check throws', async () => {
|
||||
|
|
@ -2244,7 +2278,7 @@ describe('WorkflowService', () => {
|
|||
),
|
||||
).rejects.toThrow(violation);
|
||||
|
||||
expect(workflowRepositoryMock.update).not.toHaveBeenCalled();
|
||||
expect(workflowRepositoryMock.updateContent).not.toHaveBeenCalled();
|
||||
expect(workflowHistoryServiceMock.saveVersion).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -544,7 +544,7 @@ export class WorkflowService {
|
|||
// Gate the save on policy before persisting, so the author learns about a violation
|
||||
// while editing rather than at runtime. Carries the stored workflow alongside the
|
||||
// submitted one so a check can restrict its verdict to what this save adds.
|
||||
await this.policyEnforcementService.enforceWorkflowSave({
|
||||
const cleared = await this.policyEnforcementService.enforceWorkflowSave({
|
||||
workflow: {
|
||||
id: workflow.id,
|
||||
name: workflowUpdateData.name ?? workflow.name,
|
||||
|
|
@ -606,7 +606,9 @@ export class WorkflowService {
|
|||
}
|
||||
updatePayload.parentFolder = parentFolderId === PROJECT_ROOT ? null : { id: parentFolderId };
|
||||
}
|
||||
await this.workflowRepository.update(workflowId, updatePayload);
|
||||
await this.workflowRepository.updateContent(workflowId, updatePayload, {
|
||||
policyCleared: cleared,
|
||||
});
|
||||
const tagsDisabled = this.globalConfig.tags.disabled;
|
||||
|
||||
if (tagIds && !tagsDisabled) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user