From aca37be257b3e88742e7b16ef84b61e779219f7c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:11:57 +0000 Subject: [PATCH] docs(spec): correct AutomationContext.flowName's attribution prose (#14011) The published contract said a `runAs:'system'` run "resolves no user", so the `svc:flow:` actor label stands in for the audit row's attribution. That reads as "system elevation costs you the operator in the audit trail" and has not been true since #5494: `resolveRunDataContext` carries the triggering user through unchanged, `isSystem` alone decides authorization, and the audit writers record `session.userId ?? session.actor` with no `isSystem` gate. The `svc:flow:` label is the FALLBACK for a genuinely user-less run (a schedule). Runtime behaviour is untouched. `runas-attribution-contract.test.ts` pins both limbs against the real ObjectQL stack so the next drift goes red on its own. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- ...tion-context-flowname-attribution-prose.md | 39 +++ .../src/runas-attribution-contract.test.ts | 266 ++++++++++++++++++ .../spec/src/contracts/automation-service.ts | 23 +- 3 files changed, 324 insertions(+), 4 deletions(-) create mode 100644 .changeset/automation-context-flowname-attribution-prose.md create mode 100644 packages/services/service-automation/src/runas-attribution-contract.test.ts diff --git a/.changeset/automation-context-flowname-attribution-prose.md b/.changeset/automation-context-flowname-attribution-prose.md new file mode 100644 index 0000000000..9adde5fc4c --- /dev/null +++ b/.changeset/automation-context-flowname-attribution-prose.md @@ -0,0 +1,39 @@ +--- +"@objectstack/spec": patch +--- + +docs(spec): correct `AutomationContext.flowName`'s attribution prose — elevation decides authorization, not attribution (#14011) + +The published contract for `AutomationContext.flowName` (shipped in +`dist/contracts/index.d.ts`) said a `runAs:'system'` run "resolves no user", so +`resolveRunDataContext` labels its data operations `svc:flow:` on +`ExecutionContext.actor` "instead of leaving the audit row unattributed". + +That reads as **"system elevation costs you the operator in the audit trail"**, +and it has not been true since #5494. What ships: `resolveRunDataContext` +carries the triggering user through UNCHANGED under elevation — `isSystem` +alone decides authorization, while the user drives the platform's attribution +stamps. A write made with `{ ...callerCtx, isSystem: true }` leaves +`created_by` / `updated_by` naming the caller, identical to the same write on +the plain user path; the audit writer records `session.userId ?? session.actor` +on `sys_audit_log.actor`, in that order, with no `isSystem` gate anywhere in +either path. + +The `svc:flow:` labelling the sentence described is real, but it is the +FALLBACK for a run that genuinely has no operator — a schedule, or a +`runAs:'system'` flow fired by a write that itself carried no user. The +sentence generalised it to every `runAs:'system'` run. + +Runtime behaviour is unchanged: this corrects the description of shipped +behaviour, nothing else. The correction is now also pinned by +`runas-attribution-contract.test.ts` in `@objectstack/service-automation`, which +asserts both limbs against the real ObjectQL stack — so if the code ever +becomes what the old prose described, a test goes red rather than a reader +having to re-measure. + +Why it earned a card rather than a shrug: downstream, the stale sentence was +written into an adjudication as the explicit stop-condition for a security +design ("if elevation erases the operator, stop and report a fork"). The +correct design was one measurement away from being abandoned on a false +premise. Prose that talks a reader out of the right answer is worth more than a +cosmetic fix. diff --git a/packages/services/service-automation/src/runas-attribution-contract.test.ts b/packages/services/service-automation/src/runas-attribution-contract.test.ts new file mode 100644 index 0000000000..b03b18b595 --- /dev/null +++ b/packages/services/service-automation/src/runas-attribution-contract.test.ts @@ -0,0 +1,266 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The prose-to-code tie for `AutomationContext.flowName`'s attribution + * contract — the one sentence a reader is most likely to act on, pinned + * against the behaviour it describes. + * + * WHAT IT DEFENDS. `packages/spec/src/contracts/automation-service.ts` + * (`AutomationContext.flowName`, published as `dist/contracts/index.d.ts`) + * used to say a `runAs:'system'` run "resolves no user", so the + * `svc:flow:` actor label stands in for the audit row's + * attribution. That reads as **"system elevation costs you the operator in + * the audit trail"**, and it is not what ships: since #5494 elevation decides + * AUTHORIZATION and leaves ATTRIBUTION alone — `resolveRunDataContext` + * carries the triggering user through unchanged. The `svc:flow:` label is the + * FALLBACK for a run that genuinely has no operator (a schedule). + * + * WHY A TEST AND NOT ONLY A DOC FIX. The cost of that sentence was never + * cosmetic: downstream it was written into an adjudication as the explicit + * STOP CONDITION for a security design ("if elevation erases the operator, + * stop and report a fork"). A correct design was one measurement away from + * being abandoned on a false premise. A doc fix alone leaves the next drift + * silent, so the invariant the prose now states is asserted here: if the code + * ever becomes what the old prose described, this file goes red instead of a + * human having to re-measure. + * + * WHERE IT ASSERTS. At the END of the chain — the envelope the audit writers + * actually read — not at `resolveRunDataContext`'s return shape, which + * `builtin/crud-runas.test.ts` already pins: + * + * - `packages/objectql/src/plugin.ts` `sys_stamp_audit_insert` / + * `sys_stamp_audit_update` stamp `created_by` / `updated_by` under + * `if (session?.userId)` — no `isSystem` test anywhere in that path; + * - `packages/plugins/plugin-audit/src/audit-writers.ts` records + * `session.userId ?? session.actor` on `sys_audit_log.actor` — the + * fallback, in that order. + * + * The hook session captured below IS that envelope (ObjectQL's + * `buildSession` propagates `userId`, `isSystem` and `actor` into it), so the + * two limbs are measured where the prose's claim lands. + * + * Directions decided before running (reverse-verification discipline): + * - elevated + user-triggered → `updated_by` / `created_by` = the triggering + * user, identical to the same write under a plain user context; + * - elevated + genuinely user-less (schedule shape) → user column stays + * NULL and `session.actor` is `svc:flow:` (ADR-0118 D1 forbids a + * sentinel or pseudo-user in the user column). + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin, type ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AutomationServicePlugin } from './plugin.js'; +import type { AutomationEngine } from './engine.js'; + +/** Real backend: better-sqlite3 `:memory:` through driver-sql. */ +function makeSqliteDriver() { + return new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); +} + +/** + * A plain business object. The audit family (`created_by` / `updated_by`) is + * NOT declared — the registry injects it (`applySystemFields`), exactly like a + * production app object, so the stamps land on the injected platform columns. + */ +const crmTask = { + name: 'crm_task', + label: 'Task', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + status: { name: 'status', label: 'Status', type: 'text' }, + }, +}; + +/** The trigger envelope a manual / record-change firing supplies: a real user. */ +const OPERATOR = { + userId: 'usr_operator', + tenantId: 'org_1', + positions: [] as string[], + permissions: [] as string[], +}; + +/** + * A DIFFERENT user, who creates the rows the operator later touches. + * + * Load-bearing, and the reverse-verification found out why: seeded by the + * operator instead, both rows already carry `updated_by = 'usr_operator'` from + * their own insert, so the column assertion below stays green even when + * elevation drops the operator — it would be asserting the insert, not the + * elevated update. Seeded by someone else, the column has to MOVE for the + * assertion to pass, which is the claim being made. + */ +const CREATOR = { + userId: 'usr_creator', + tenantId: 'org_1', + positions: [] as string[], + permissions: [] as string[], +}; + +/** start → update_record(crm_task, id) → end, under runAs:'system'. */ +const elevatedUpdateFlow = (name: string, recordId: string) => ({ + name, + label: name, + type: 'autolaunched', + runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'touch', + type: 'update_record', + label: 'Touch', + config: { objectName: 'crm_task', filter: { id: recordId }, fields: { status: 'done' } }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'touch' }, + { id: 'e2', source: 'touch', target: 'end' }, + ], +}); + +/** start → create_record(crm_task) → end, under runAs:'system'. */ +const elevatedCreateFlow = (name: string, title: string) => ({ + name, + label: name, + type: 'autolaunched', + runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'mk', + type: 'create_record', + label: 'Create', + config: { objectName: 'crm_task', fields: { title, status: 'open' } }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'mk' }, + { id: 'e2', source: 'mk', target: 'end' }, + ], +}); + +/** The three fields every audit writer keys on, as the hook layer sees them. */ +interface SeenSession { + event: string; + isSystem: boolean | undefined; + userId: string | undefined; + actor: string | undefined; +} + +describe("runAs:'system' attribution contract — elevation decides authorization, not attribution", () => { + let kernel: ObjectKernel; + let ql: ObjectQL; + let automation: AutomationEngine; + let seen: SeenSession[]; + + afterEach(async () => { + try { await kernel?.shutdown(); } catch { /* noop */ } + }); + + async function boot() { + kernel = new ObjectKernel({ logger: { level: 'fatal' } }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' })); + await kernel.bootstrap(); + + ql = kernel.getService('objectql'); + automation = kernel.getService('automation'); + + const driver = makeSqliteDriver(); + await driver.connect(); + ql.registerDriver(driver, true); + ql.registry.registerObject(crmTask as any, 'attribution-test', 'attribution-test'); + await ql.syncSchemas(); + + // Observe the SAME session envelope the audit stamp hooks and + // plugin-audit's `writeAudit` read. Registered at priority 100 so it runs + // after the built-in stamp hooks (priority 10) — it only reads. + seen = []; + for (const event of ['beforeInsert', 'beforeUpdate'] as const) { + (ql as any).registerHook( + event, + async (hookCtx: any) => { + const s = hookCtx.session ?? {}; + seen.push({ event, isSystem: s.isSystem, userId: s.userId, actor: s.actor }); + }, + { object: 'crm_task', priority: 100 }, + ); + } + } + + const SYS = { isSystem: true } as const; + const taskByTitle = (title: string) => + ql.findOne('crm_task', { where: { title }, context: SYS }); + + it('an ELEVATED, user-triggered run still stamps the OPERATOR — the same value a plain user write produces (#5494)', async () => { + await boot(); + + // Two identical rows, both created by SOMEONE ELSE on the ordinary user + // path — so `updated_by` must MOVE to the operator for the assertions + // below to pass (see CREATOR). + await ql.insert('crm_task', { title: 'elevated', status: 'open' }, { context: { ...CREATOR } }); + await ql.insert('crm_task', { title: 'control', status: 'open' }, { context: { ...CREATOR } }); + const elevatedRow = await taskByTitle('elevated'); + const controlRow = await taskByTitle('control'); + + // (a) the elevated path: a `runAs:'system'` flow updates the row. + automation.registerFlow('elevated_touch', elevatedUpdateFlow('elevated_touch', String(elevatedRow.id)) as any); + const res = await automation.execute('elevated_touch', { ...OPERATOR }); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + + // (b) the control: the same write, plain user context, no elevation. + await ql.update('crm_task', { id: controlRow.id, status: 'done' }, { context: { ...OPERATOR } }); + + const afterElevated = await taskByTitle('elevated'); + const afterControl = await taskByTitle('control'); + + // THE INVARIANT. The old prose said an elevated run "resolves no user", so + // its writes would land unattributed and lean on the actor label instead. + // They do not: the operator is stamped, and byte-identically to the + // unelevated write. + expect(afterElevated.updated_by, 'the elevated run must stamp the triggering operator').toBe('usr_operator'); + expect(afterElevated.updated_by, 'elevated attribution must equal the plain user path').toBe(afterControl.updated_by); + // The column MOVED off the creator — the assertion above is about this + // update, not about the insert that seeded the row. + expect(afterElevated.created_by, 'the original creator is untouched').toBe('usr_creator'); + expect(afterElevated.status, 'the run must actually have written').toBe('done'); + + // …and the envelope the audit writers read carries BOTH: elevation on + // `isSystem` (authorization) and the operator on `userId` (attribution), + // with the flow label riding beside them rather than replacing the user. + const elevatedUpdate = seen.find((s) => s.event === 'beforeUpdate' && s.isSystem === true); + expect(elevatedUpdate, 'the elevated update must have reached the hook layer').toBeTruthy(); + expect(elevatedUpdate!.userId, 'elevation must not strip the operator (#5494)').toBe('usr_operator'); + expect(elevatedUpdate!.actor, 'the flow label names WHICH automation wrote (ADR-0014 D2)').toBe('svc:flow:elevated_touch'); + }); + + it('a genuinely USER-LESS run falls back to the `svc:flow:` label — that is the case the label exists for (#4366)', async () => { + await boot(); + + // What ScheduleTrigger actually supplies: an event and params, NO user. + automation.registerFlow('night_sweep', elevatedCreateFlow('night_sweep', 'nightly') as any); + const res = await automation.execute('night_sweep', { event: 'schedule', params: {} } as any); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + + const row = await taskByTitle('nightly'); + expect(row, 'the sweep must have created the row').toBeTruthy(); + + // There is no operator to carry, so the user column stays NULL — ADR-0118 + // D1 forbids a sentinel or pseudo-user standing in for one. + expect(row.created_by ?? null, 'a user-less run has no operator to stamp').toBeNull(); + + // …and the actor label is what keeps the write attributable anyway. This + // is the half of the old prose that was TRUE — it was only ever true here. + const userlessInsert = seen.find((s) => s.event === 'beforeInsert' && s.isSystem === true); + expect(userlessInsert, 'the user-less insert must have reached the hook layer').toBeTruthy(); + expect(userlessInsert!.userId ?? null, 'a schedule resolves no user').toBeNull(); + expect(userlessInsert!.actor, 'the svc:flow: label is the fallback attribution').toBe('svc:flow:night_sweep'); + }); +}); diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index d32816bef0..bf277eb62e 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -70,10 +70,25 @@ export interface AutomationContext { * Machine name of the flow this run executes, stamped by the engine at run * setup alongside {@link runAs} / {@link flowRunId} (same single * construction point, same lifetime). Provenance, not authorization — no - * security middleware keys on it. Its consumer is audit attribution: a - * `runAs:'system'` run resolves no user, so `resolveRunDataContext` labels - * its data operations `svc:flow:` on `ExecutionContext.actor` - * (ADR-0014 D2) instead of leaving the audit row unattributed (#4366). + * security middleware keys on it. + * + * Its consumer is audit attribution: `resolveRunDataContext` labels a + * `runAs:'system'` run's data operations `svc:flow:` on + * `ExecutionContext.actor` (ADR-0014 D2), naming WHICH automation performed + * the write. The label is a FALLBACK, not a replacement — the audit writer + * records `session.userId ?? session.actor` on `sys_audit_log.actor` — and + * elevation never costs the run its operator: a `runAs:'system'` run + * carries the triggering user through UNCHANGED whenever the trigger + * resolved one (#5494), so its writes still stamp `created_by` / + * `updated_by` and `sys_audit_log.user_id` with that human, exactly as the + * same trigger would under `runAs:'user'`. `runAs` declares the run's + * AUTHORIZATION posture and leaves ATTRIBUTION alone (ADR-0073 D2). + * + * The `svc:flow:` label is therefore what a genuinely USER-LESS run falls + * back to — a schedule, or a `runAs:'system'` flow fired by a write that + * itself carried no user — instead of leaving the audit row unattributed + * (#4366). There the user column stays null: ADR-0118 D1 forbids inventing + * a sentinel or pseudo-user in its place. * * Callers do NOT set this — the engine derives it, exactly like {@link runAs}. */