diff --git a/packages/@n8n/db/src/repositories/__tests__/workflow-update-content.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/workflow-update-content.repository.test.ts new file mode 100644 index 00000000000..2cadf2095a1 --- /dev/null +++ b/packages/@n8n/db/src/repositories/__tests__/workflow-update-content.repository.test.ts @@ -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(), + ); + + 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(); + }); +}); diff --git a/packages/@n8n/db/src/repositories/workflow.repository.ts b/packages/@n8n/db/src/repositories/workflow.repository.ts index 3f1fb17339a..7878a1b3eba 100644 --- a/packages/@n8n/db/src/repositories/workflow.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow.repository.ts @@ -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 { return count > 0; } + async updateContent( + id: string, + content: QueryDeepPartialEntity, + ctx: OperationContext, + ) { + assertClearedFor(ctx.policyCleared, 'workflowSave', { type: 'workflow', id }); + await this.managerFor(ctx).update(WorkflowEntity, id, content); + } + async findByCredentialResolverId( resolverId: string, ): Promise>> { diff --git a/packages/@n8n/db/src/services/transaction.ts b/packages/@n8n/db/src/services/transaction.ts index 0728afc759a..afa8729cb10 100644 --- a/packages/@n8n/db/src/services/transaction.ts +++ b/packages/@n8n/db/src/services/transaction.ts @@ -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; } export interface RunOptions { diff --git a/packages/@n8n/decorators/package.json b/packages/@n8n/decorators/package.json index d56a1f5acfb..326faae920c 100644 --- a/packages/@n8n/decorators/package.json +++ b/packages/@n8n/decorators/package.json @@ -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/**/*" ], diff --git a/packages/@n8n/decorators/src/policy-check/__tests__/assert-cleared-for.test.ts b/packages/@n8n/decorators/src/policy-check/__tests__/assert-cleared-for.test.ts new file mode 100644 index 00000000000..f680f5700a4 --- /dev/null +++ b/packages/@n8n/decorators/src/policy-check/__tests__/assert-cleared-for.test.ts @@ -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(); + }); +}); diff --git a/packages/cli/src/policy/__tests__/policy-cleared.test.ts b/packages/@n8n/decorators/src/policy-check/__tests__/policy-mint.test.ts similarity index 64% rename from packages/cli/src/policy/__tests__/policy-cleared.test.ts rename to packages/@n8n/decorators/src/policy-check/__tests__/policy-mint.test.ts index 6a49601b9bf..459bd2a1018 100644 --- a/packages/cli/src/policy/__tests__/policy-cleared.test.ts +++ b/packages/@n8n/decorators/src/policy-check/__tests__/policy-mint.test.ts @@ -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); + }); +}); diff --git a/packages/@n8n/decorators/src/policy-check/index.ts b/packages/@n8n/decorators/src/policy-check/index.ts index cb92c0f081a..b7af177e7cd 100644 --- a/packages/@n8n/decorators/src/policy-check/index.ts +++ b/packages/@n8n/decorators/src/policy-check/index.ts @@ -13,3 +13,4 @@ export { UnknownEnforcementPointError, } from './policy-check-metadata'; export * from './policy-check'; +export * from './policy-cleared'; diff --git a/packages/@n8n/decorators/src/policy-check/policy-brand.ts b/packages/@n8n/decorators/src/policy-check/policy-brand.ts new file mode 100644 index 00000000000..d8f3b359c3a --- /dev/null +++ b/packages/@n8n/decorators/src/policy-check/policy-brand.ts @@ -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'); diff --git a/packages/@n8n/decorators/src/policy-check/policy-cleared.ts b/packages/@n8n/decorators/src/policy-check/policy-cleared.ts new file mode 100644 index 00000000000..80494b641ab --- /dev/null +++ b/packages/@n8n/decorators/src/policy-check/policy-cleared.ts @@ -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 = { + 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 | 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}`, + ); +} diff --git a/packages/@n8n/decorators/src/policy-check/policy-mint.ts b/packages/@n8n/decorators/src/policy-check/policy-mint.ts new file mode 100644 index 00000000000..eea40fa38be --- /dev/null +++ b/packages/@n8n/decorators/src/policy-check/policy-mint.ts @@ -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, + subject, + decision, +}: { + point: Point; + subject: PolicySubject; + decision: PolicyDecision; +}): PolicyCleared { + 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 }; +} diff --git a/packages/@n8n/eslint-config/src/rules/index.ts b/packages/@n8n/eslint-config/src/rules/index.ts index 54d820d5913..b37db899e63 100644 --- a/packages/@n8n/eslint-config/src/rules/index.ts +++ b/packages/@n8n/eslint-config/src/rules/index.ts @@ -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; diff --git a/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts new file mode 100644 index 00000000000..6ccfa07147e --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts @@ -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(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(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(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' }], + }, + ], +}); diff --git a/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts new file mode 100644 index 00000000000..f92afa73520 --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts @@ -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' }); + } + } + }, + }; + }, +}); diff --git a/packages/cli/eslint.config.mjs b/packages/cli/eslint.config.mjs index bc05c624887..5f859e50eee 100644 --- a/packages/cli/eslint.config.mjs +++ b/packages/cli/eslint.config.mjs @@ -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: { diff --git a/packages/cli/src/policy/policy-cleared.ts b/packages/cli/src/policy/policy-cleared.ts deleted file mode 100644 index d3f6391ea9e..00000000000 --- a/packages/cli/src/policy/policy-cleared.ts +++ /dev/null @@ -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 = { - 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, - subject, - decision, -}: { - point: Point; - subject: PolicySubject; - decision: PolicyDecision; -}): PolicyCleared { - 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 ?? [], - }; -} diff --git a/packages/cli/src/policy/policy-enforcement.service.ts b/packages/cli/src/policy/policy-enforcement.service.ts index 1794169144b..26753f62013 100644 --- a/packages/cli/src/policy/policy-enforcement.service.ts +++ b/packages/cli/src/policy/policy-enforcement.service.ts @@ -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. * diff --git a/packages/cli/src/workflows/__tests__/workflow.service.test.ts b/packages/cli/src/workflows/__tests__/workflow.service.test.ts index 7eb95e56b98..89f811cb374 100644 --- a/packages/cli/src/workflows/__tests__/workflow.service.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow.service.test.ts @@ -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; 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; let ownershipServiceMock: MockProxy; let licenseStateMock: MockProxy; - 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; let workflowHistoryServiceMock: MockProxy; let policyEnforcementServiceMock: MockProxy; - 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>(); + policyEnforcementServiceMock.enforceWorkflowSave.mockResolvedValue(cleared); + + await workflowService.update( + mock({ 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(); }); diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index ba637ab08e7..7b73cd562cc 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -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) {