diff --git a/.changeset/24614-system-run-user-grants.md b/.changeset/24614-system-run-user-grants.md new file mode 100644 index 00000000000..b7e61bb2213 --- /dev/null +++ b/.changeset/24614-system-run-user-grants.md @@ -0,0 +1,22 @@ +--- +'@objectstack/service-automation': minor +--- + +fix(service-automation)!: a `runAs: 'user'` hook fired by a user-triggered `runAs: 'system'` flow's write runs on that user's own grants + +Clause-②: no (narrowing) — a `runAs: 'user'` hook fired by a write of a `runAs: 'system'` flow that names a user now runs on that user's own grants instead of the member baseline, so for a user whose own grants admit less than the baseline its reads narrow; the published `RunIdentityContext` / `RunDataContext` types make `positions` and `permissions` optional, and no published schema or export moves. + + + +**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +A `runAs: 'system'` flow run that names a user (a manual run, a record change fired by a user's write, and every post-decision node of a `runAs: 'system'` approval flow, where the user is the decider) handed ObjectQL `{ isSystem: true, userId, tenantId, positions: [], permissions: [] }`. Its own operations never read the two arrays: elevation short-circuits every gate before they are read. But a `runAs: 'user'` hook fired by one of its writes de-elevates to that context, and the engine keeps the arrays a context carries, resolving the user's grants only when none are carried. So that hook ran on the member baseline, whoever the user was. + +- **Now:** a `runAs: 'system'` run that names a user carries no `positions` or `permissions` in its data context. A `runAs: 'user'` hook its writes fire resolves the named user's own positions, permission sets and organization peers in the run's organization, exactly as that user's own request resolves them. After an approval decision that user is the decider, so the hooks the post-decision writes fire run on the decider's grants. +- **Wider:** for a user granted more than the baseline, such a hook now reads and writes what the user may, where it was refused on objects the baseline does not grant. +- **Narrower:** for a user whose own grants admit less than the baseline, such a hook now reads only what that user's own request reads. +- **Unchanged:** the run's own elevation, `isSystem`, its `actor` label, its `userId` and its `tenantId`; a user-less `runAs: 'system'` run (a schedule), which keeps `positions: []` and `permissions: []`, and a `runAs: 'user'` hook it fires is still refused for want of a user; hooks declaring `runAs: 'system'` or `'inherit'`; and `runAs: 'user'` flows. + +The published `RunIdentityContext` and `RunDataContext` types of `@objectstack/service-automation` declare `positions` and `permissions` optional, because a `runAs: 'system'` envelope that names a user no longer carries them. Code that reads either member from `resolveRunDataContext`'s answer must allow for its absence. + +What changes for you: a `runAs: 'user'` hook that a user-triggered `runAs: 'system'` flow fires now sees exactly what that user sees. If such a hook must read or write something the user may not, declare it `runAs: 'system'`. diff --git a/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts b/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts index d90533affac..ffe2516fa21 100644 --- a/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts +++ b/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts @@ -981,7 +981,7 @@ describe('ruling A — a decision resumes the run as the person who decided', () } as never); expect(seen.before, 'the nodes before the decision run as the submitter').toEqual(flow === 'who_flow' ? { userId: 'submitter', permissions: ['deal_rep'] } - : { userId: 'submitter', permissions: [], isSystem: true }); + : { userId: 'submitter', isSystem: true }); return (await data.find('sys_approval_request', { where: { status: 'pending' } }))[0]; } @@ -998,7 +998,7 @@ describe('ruling A — a decision resumes the run as the person who decided', () const req = await parked(automation, 'who_flow_sys'); const out = await service.decide(req.id, { decision, actorId: 'u1', comment: decision }, DECIDER_CTX); expect(out.resumed, JSON.stringify(out)).toBe(true); - expect(seen[`after_${decision}`]).toEqual({ userId: 'u1', permissions: [], isSystem: true }); + expect(seen[`after_${decision}`]).toEqual({ userId: 'u1', isSystem: true }); }); it('CONTROL: an SLA auto-decision names no decider, so the run keeps the identity it had', async () => { diff --git a/packages/services/service-automation/src/builtin/crud-runas.test.ts b/packages/services/service-automation/src/builtin/crud-runas.test.ts index 07a4106b823..aef3a70b023 100644 --- a/packages/services/service-automation/src/builtin/crud-runas.test.ts +++ b/packages/services/service-automation/src/builtin/crud-runas.test.ts @@ -210,7 +210,26 @@ describe('flow.runAs identity enforcement at the data layer (#1888)', () => { describe('resolveRunDataContext (#1888 unit)', () => { it("maps runAs:'system' to an elevated context attributed to the flow AND the acting user (#4366, #5494)", () => { expect(resolveRunDataContext({ runAs: 'system', userId: 'u1', flowName: 'mirror_status' })).toEqual({ - isSystem: true, actor: 'svc:flow:mirror_status', userId: 'u1', positions: [], permissions: [], + isSystem: true, actor: 'svc:flow:mirror_status', userId: 'u1', + }); + }); + + // `toEqual` reads an absent key and an `undefined` one alike, so absence is + // asserted by key: the `runAs:'user'` hook seed resolves the user's grants + // only when no `permissions` key carries an array. + it("a system run that names a user carries no positions or permission sets, even when its trigger forwarded some", () => { + const ctx = resolveRunDataContext({ + runAs: 'system', userId: 'u1', tenantId: 'org_a', flowName: 'sweep', flowRunId: 'r1', + positions: ['sales_rep'], permissions: ['deal_rep'], + }); + expect(ctx).toEqual({ isSystem: true, actor: 'svc:flow:sweep', userId: 'u1', tenantId: 'org_a', flowRunId: 'r1' }); + expect(Object.keys(ctx!)).not.toContain('positions'); + expect(Object.keys(ctx!)).not.toContain('permissions'); + }); + + it('a user-less system run keeps the empty positions and permission sets, its organisation included', () => { + expect(resolveRunDataContext({ runAs: 'system', tenantId: 'org_a', flowName: 'night' })).toEqual({ + isSystem: true, actor: 'svc:flow:night', tenantId: 'org_a', positions: [], permissions: [], }); }); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 0e77233a0aa..618c13e08aa 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -6598,8 +6598,10 @@ export class AutomationEngine implements IAutomationService { * decision run as `{ isSystem: true, userId: DECIDER }` — the shape the * approvals verdict mirror already writes with — so `updated_by`, the audit * row and `ctx.user` in the hooks those writes fire name the decider, while - * elevation is untouched and no grant is resolved (a system run's data - * context carries none: {@link resolveRunDataContext}'s system arm). The + * elevation is untouched and no grant is resolved for the run. Its data + * context carries no positions or permission sets + * ({@link resolveRunDataContext}'s system arm), so a `runAs:'user'` hook a + * post-decision write fires resolves the decider's own grants. The * trigger user's positions, permission sets, peers and email are removed * here too, so `current_user` in the segment is the decider and never the * decider's id over the submitter's grants. diff --git a/packages/services/service-automation/src/resume-decider-identity.integration.test.ts b/packages/services/service-automation/src/resume-decider-identity.integration.test.ts index 64dd66b0036..17d8216151a 100644 --- a/packages/services/service-automation/src/resume-decider-identity.integration.test.ts +++ b/packages/services/service-automation/src/resume-decider-identity.integration.test.ts @@ -273,8 +273,13 @@ describe("after an approval decision a runAs:'user' run's data operations carry await h.engine.resume(paused.runId!, decided(decision, 'usr_mgr')); const after = h.ctxOf(nodeId); - // Ruling 6267719791 Q1 A: `{ isSystem: true, userId: }`. - expect(after).toMatchObject({ isSystem: true, userId: 'usr_mgr', positions: [], permissions: [], tenantId: 'org_a' }); + // Ruling 6267719791 Q1 A: `{ isSystem: true, userId: }`, and + // no positions or permission sets beside the decider, so a + // `runAs:'user'` hook this write fires resolves the decider's own grants + // (`runas-system-user-hook-grants.integration.test.ts` on the real stack). + expect(after).toMatchObject({ isSystem: true, userId: 'usr_mgr', tenantId: 'org_a' }); + expect(Object.keys(after)).not.toContain('positions'); + expect(Object.keys(after)).not.toContain('permissions'); expect(after.org_user_ids).toBeUndefined(); // No permission resolution for a system run, and none of the submitter's // grants left under the decider's name. @@ -300,7 +305,9 @@ describe("after an approval decision a runAs:'user' run's data operations carry h.engine.registerFlow('sys_review', reviewFlow('sys_review', { runAs: 'system', recorder: true }) as never); const paused = await h.engine.execute('sys_review', { ...TRIGGER } as never); await h.engine.resume(paused.runId!, decided('approve')); - expect(h.ctxOf('after1')).toMatchObject({ isSystem: true, userId: 'usr_rep', positions: [], permissions: [] }); + const after = h.ctxOf('after1'); + expect(after).toMatchObject({ isSystem: true, userId: 'usr_rep' }); + expect(Object.keys(after)).not.toContain('permissions'); }); it('CONTROL: a decider named WITHOUT the owning service\'s marker changes nothing', async () => { diff --git a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts index 8129fbc400f..6104851a557 100644 --- a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts +++ b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts @@ -130,10 +130,16 @@ describe("AutomationServicePlugin bridges the runAs:'user' grant resolver (#3356 // #5494 — the acting user rides the elevated context as ATTRIBUTION (it // drives the created_by/updated_by stamps and the audit actor; the // isSystem short-circuit precedes every gate that reads it). What proves - // "the resolver is not consulted" is the untouched authz envelope: + // "the resolver is not consulted" is that no grant table was read and no + // authz fact was filled in. The envelope carries no positions or + // permission sets at all beside a named user: a `runAs:'user'` hook this + // write fires then resolves the user's own grants instead of running on + // the member baseline (`runas-system-user-hook-grants.integration.test.ts`). expect(update!.ctx.userId).toBe('u1'); - expect(update!.ctx.positions).toEqual([]); - expect(update!.ctx.permissions).toEqual([]); + expect(Object.keys(update!.ctx)).not.toContain('positions'); + expect(Object.keys(update!.ctx)).not.toContain('permissions'); + expect(update!.ctx.org_user_ids).toBeUndefined(); + expect(crud.filter((c) => c.obj in AUTHZ_TABLES), 'a system run read a grant table').toEqual([]); await kernel.shutdown(); }); diff --git a/packages/services/service-automation/src/runas-system-user-hook-grants.integration.test.ts b/packages/services/service-automation/src/runas-system-user-hook-grants.integration.test.ts new file mode 100644 index 00000000000..f4c0c9e673e --- /dev/null +++ b/packages/services/service-automation/src/runas-system-user-hook-grants.integration.test.ts @@ -0,0 +1,297 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A `runAs:'user'` hook fired by a write of a user-triggered `runAs:'system'` + * flow runs on that user's own grants, as the user's own request does — not + * on the member baseline. + * + * The defect this pins: `resolveRunDataContext`'s system arm carried empty + * `positions` / `permissions` beside the run's `userId`. The hook de-elevates + * to that context (`ScopedContext.withRunAs('user')`), and the engine's hook + * seed keeps carried arrays verbatim, resolving a user's grants only when none + * are carried. So the hook read under the baseline sets alone, whoever the + * user was: less than a user granted more, and more than a user whose own sets + * scope a read the baseline leaves open. The system arm now carries no arrays + * when it names a user, and the seed resolves that user's grants in the run's + * organisation. The run's own elevation is unchanged. + * + * Directions decided before running: + * - a user granted more than the baseline: the hook reads what they may (was: + * refused on an object the baseline does not grant); + * - a user whose own sets read FEWER rows of an object than the baseline: + * the hook reads those fewer rows (was: every row the baseline reads); + * - after an approval decision the run's user is the decider, so a hook a + * post-decision write fires runs on the decider's grants (was: baseline); + * - CONTROL, a user-less system run (a schedule): its envelope is unchanged + * and the hook is still refused for want of a user; + * - CONTROL, the former envelope fired by hand: the baseline answer, which + * is what each direction above moved away from. + * + * The real stack, as `runas-user-hook-org-peers.integration.test.ts` composes + * it: ObjectKernel, ObjectQLPlugin, driver-sql on better-sqlite3 `:memory:`, + * the identity objects and the real SecurityPlugin, plus the real + * AutomationEngine and its CRUD nodes. The defect sits in the hand-off from + * the flow's data context through the hook's derivation to the security + * middleware, which no single-layer test sees. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectKernel, resolveUserAuthzGrants, type PluginContext } from '@objectstack/core'; +import { ObjectQLPlugin, bindHooksToEngine, type ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { createIdentityObjectsPlugin } from '@objectstack/plugin-auth'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import { RESUME_AUTHORITY_SERVICE, type ResumeSignal } from '@objectstack/spec/contracts'; +import { AutomationServicePlugin } from './plugin.js'; +import type { AutomationEngine } from './engine.js'; + +const SYS = { isSystem: true } as const; +const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; + +const text = (name: string) => ({ name, label: name, type: 'text' as const }); + +/** The object the flows write; its insert fires the hook under test. */ +const triggerObject = { name: 'hgp_trigger', label: 'Hook grants trigger', fields: { name: text('name') } }; +/** An object the baseline does not grant: only `hgp_ledger_reader` reads it. */ +const ledgerObject = { name: 'hgp_ledger', label: 'Ledger', sharingModel: 'public_read', fields: { title: text('title') } }; +/** An object the baseline reads in full. */ +const noteObject = { name: 'hgp_note', label: 'Note', sharingModel: 'public_read', fields: { title: text('title') } }; + +/** The deployment's member profile, composed into the baseline beside `member_default`. */ +const memberProfile = PermissionSetSchema.parse({ + name: 'hgp_member', label: 'Hook grants: member profile', + objects: { hgp_note: { allowRead: true } }, +}); +const ledgerReader = PermissionSetSchema.parse({ + name: 'hgp_ledger_reader', label: 'Hook grants: ledger reader', + objects: { hgp_ledger: { allowRead: true } }, +}); +const noteOwnRows = PermissionSetSchema.parse({ + name: 'hgp_note_own_rows', label: 'Hook grants: own notes', + objects: { hgp_note: { allowRead: true } }, + rowLevelSecurity: [{ name: 'hgp_note_own', object: 'hgp_note', operation: 'select', using: 'owner_id == current_user.id' }], +}); + +/** The approval node's stand-in: a pause only its owning service may continue. */ +const GATED = defineActionDescriptor({ + type: 'decision_gate', version: '1.0.0', name: 'Decision Gate', + supportsPause: true, resumeAuthority: 'service', +}); + +/** What `ApprovalService.decide` hands the engine for a human decision. */ +const decided = (resumedBy?: string): ResumeSignal => ({ + branchLabel: 'approve', + output: { decision: 'approve' }, + ...(resumedBy ? { resumedBy } : {}), + [RESUME_AUTHORITY_SERVICE]: true, +}); + +const create = (id: string, name: string) => ({ + id, type: 'create_record', label: id, config: { objectName: 'hgp_trigger', fields: { name } }, +}); + +/** start → create_record(hgp_trigger) → end, under runAs:'system'. */ +const sweepFlow = { + name: 'hgp_sweep', label: 'hgp_sweep', type: 'autolaunched', runAs: 'system', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, create('mk', 'sweep'), { id: 'end', type: 'end', label: 'End' }], + edges: [{ id: 'e1', source: 'start', target: 'mk' }, { id: 'e2', source: 'mk', target: 'end' }], +}; + +/** start → create (submit) → gate → create (after the decision) → end, under runAs:'system'. */ +const reviewFlow = { + name: 'hgp_review', label: 'hgp_review', type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + create('submit', 'submit'), + { id: 'gate', type: 'decision_gate', label: 'Review' }, + create('after', 'after'), + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'submit' }, + { id: 'e2', source: 'submit', target: 'gate' }, + { id: 'e3', source: 'gate', target: 'after', label: 'approve' }, + { id: 'e4', source: 'after', target: 'end' }, + ], +}; + +/** A read's answer: the ids it returned, or the refusal's envelope. */ +type Answer = string[] | { code: unknown; status: unknown }; +interface Firing { trigger: string; ledger: Answer; note: Answer } + +describe("a runAs:'user' hook fired by a user-triggered runAs:'system' flow's write runs on that user's own grants", () => { + let kernel: ObjectKernel; + let ql: ObjectQL; + let automation: AutomationEngine; + let orgA = ''; + /** What the hook's reads answered, per firing. */ + const firings: Firing[] = []; + /** The context each `hgp_trigger` insert carried to the middleware chain. */ + const writeContexts: Array> = []; + /** The context the hook's `hgp_note` read carried, per firing. */ + const hookReadContexts: Array> = []; + + const ids = (rows: unknown): string[] => (Array.isArray(rows) ? rows.map((r: any) => String(r.id)).sort() : []); + const answer = async (read: () => Promise): Promise => { + try { + return ids(await read()); + } catch (e: any) { + return { code: e?.code, status: e?.statusCode ?? e?.status }; + } + }; + + beforeAll(async () => { + kernel = new ObjectKernel({ logger: { level: 'fatal' } }); + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await kernel.use({ + name: 'test.driver.sqlite', type: 'standard', version: '1.0.0', + async init(ctx: PluginContext) { ctx.registerService('driver.default', driver); }, + }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' })); + await kernel.use(createIdentityObjectsPlugin()); + await kernel.use(new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, memberProfile, ledgerReader, noteOwnRows], + fallbackPermissionSet: 'hgp_member', + })); + await kernel.bootstrap(); + + ql = kernel.getService('objectql'); + automation = kernel.getService('automation'); + automation.registerNodeExecutor({ type: 'decision_gate', descriptor: GATED, async execute() { return { success: true, suspend: true }; } }); + automation.registerFlow('hgp_sweep', sweepFlow as never); + automation.registerFlow('hgp_review', reviewFlow as never); + for (const o of [triggerObject, ledgerObject, noteObject]) ql.registry.registerObject(o as any, 'hook-grants-test', 'hook-grants-test'); + await ql.syncSchemas(); + + const orgs = await ql.find('sys_organization', { context: SYS }); + orgA = String(orgs[0].id); + const inOrg = { isSystem: true, tenantId: orgA } as const; + for (const id of ['usr_above', 'usr_below', 'usr_peer']) { + await ql.insert('sys_user', { id, name: id, email: `${id}@example.test` }, { context: SYS }); + await ql.insert('sys_member', { id: `mem_${id}`, user_id: id, organization_id: orgA, role: 'member' }, { context: SYS }); + } + await ql.insert('sys_user_permission_set', { id: 'ups_above', user_id: 'usr_above', permission_set: 'hgp_ledger_reader', organization_id: orgA }, { context: SYS }); + await ql.insert('sys_user_permission_set', { id: 'ups_below', user_id: 'usr_below', permission_set: 'hgp_note_own_rows', organization_id: orgA }, { context: SYS }); + await ql.insert('hgp_ledger', { id: 'ledger_1', title: 'Ledger' }, { context: inOrg }); + await ql.insert('hgp_note', { id: 'note_below', title: 'Below', owner_id: 'usr_below' }, { context: inOrg }); + await ql.insert('hgp_note', { id: 'note_peer', title: 'Peer', owner_id: 'usr_peer' }, { context: inOrg }); + + let inHook = false; + ql.registerMiddleware(async (opCtx: any, next: any) => { + if (opCtx.object === 'hgp_trigger' && opCtx.operation === 'insert') writeContexts.push({ ...(opCtx.context ?? {}) }); + if (inHook && opCtx.object === 'hgp_note') hookReadContexts.push({ ...(opCtx.context ?? {}) }); + return next(); + }); + bindHooksToEngine(ql, [ + { + name: 'read_as_user', + object: 'hgp_trigger', + events: ['afterInsert'], + priority: 100, + runAs: 'user', + handler: async (ctx: any) => { + inHook = true; + try { + firings.push({ + trigger: String(ctx.input?.name ?? ''), + ledger: await answer(() => ctx.api.object('hgp_ledger').find({ fields: ['id'] })), + note: await answer(() => ctx.api.object('hgp_note').find({ fields: ['id'] })), + }); + } finally { + inHook = false; + } + }, + } as any, + ], { packageId: 'app:test', logger: silentLogger }); + }, 30_000); + + afterAll(async () => { + try { await kernel?.shutdown(); } catch { /* noop */ } + }); + + /** What the request path hands ObjectQL for `userId` acting in organisation A. */ + async function requestContext(userId: string) { + const g = await resolveUserAuthzGrants(ql, userId, { tenantId: orgA, bypassGrantsCache: true }); + return { userId, tenantId: orgA, isSystem: false, positions: g.positions, permissions: g.permissions, org_user_ids: g.org_user_ids }; + } + /** What the user's own request reads, the answer the hook must match. */ + async function ownRequest(userId: string): Promise> { + const context = await requestContext(userId); + return { + ledger: await answer(() => ql.find('hgp_ledger', { fields: ['id'], context: context as any })), + note: await answer(() => ql.find('hgp_note', { fields: ['id'], context: context as any })), + }; + } + /** Run the system sweep as triggered by `userId` in organisation A; answer the hook's firing and the write's context. */ + async function sweepAs(userId: string) { + const before = firings.length; + const res = await automation.execute('hgp_sweep', { userId, tenantId: orgA } as never); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + expect(firings.length, 'the hook ran exactly once').toBe(before + 1); + return { firing: firings[firings.length - 1], write: writeContexts[writeContexts.length - 1] }; + } + /** Fire the hook with `context` as the triggering write's context, by hand. */ + async function fireWith(context: Record) { + const before = firings.length; + await ql.insert('hgp_trigger', { name: 'by-hand' }, { context: context as any }); + expect(firings.length, 'the hook ran exactly once').toBe(before + 1); + return firings[firings.length - 1]; + } + const DENIED = { code: 'PERMISSION_DENIED', status: 403 }; + + it('GUARD: the two users\' own requests differ from the baseline in opposite directions', async () => { + // The baseline alone: no ledger, every note. + expect(await fireWith({ ...SYS, userId: 'usr_peer', tenantId: orgA, positions: [], permissions: [] })) + .toMatchObject({ ledger: DENIED, note: ['note_below', 'note_peer'] }); + expect(await ownRequest('usr_above')).toEqual({ ledger: ['ledger_1'], note: ['note_below', 'note_peer'] }); + expect(await ownRequest('usr_below')).toEqual({ ledger: DENIED, note: ['note_below'] }); + }); + + it('a user granted more than the baseline: the hook reads what their own request reads', async () => { + const { firing, write } = await sweepAs('usr_above'); + expect(write).toMatchObject({ isSystem: true, userId: 'usr_above', tenantId: orgA }); + expect(Object.keys(write), 'the system write carried permission sets beside its user').not.toContain('permissions'); + expect({ ledger: firing.ledger, note: firing.note }).toEqual(await ownRequest('usr_above')); + expect(firing.ledger).toEqual(['ledger_1']); + const hookCtx = hookReadContexts[hookReadContexts.length - 1]; + expect(hookCtx.isSystem, 'the hook de-elevated').toBe(false); + expect(hookCtx.permissions).toContain('hgp_ledger_reader'); + }); + + it('a user whose own sets read fewer rows than the baseline: the hook reads those fewer rows', async () => { + const { firing, write } = await sweepAs('usr_below'); + expect(write).toMatchObject({ isSystem: true, userId: 'usr_below', tenantId: orgA }); + expect({ ledger: firing.ledger, note: firing.note }).toEqual(await ownRequest('usr_below')); + expect(firing.note).toEqual(['note_below']); + expect(hookReadContexts[hookReadContexts.length - 1].permissions).toContain('hgp_note_own_rows'); + }); + + it("after an approval decision a post-decision write's hook runs on the decider's grants, and the submit write's on the submitter's", async () => { + const before = firings.length; + const paused = await automation.execute('hgp_review', { userId: 'usr_below', tenantId: orgA } as never); + expect(paused.status, JSON.stringify(paused)).toBe('paused'); + const res = await automation.resume(paused.runId!, decided('usr_above')); + expect(res.success, JSON.stringify(res)).not.toBe(false); + const [submit, after] = firings.slice(before); + expect(submit).toMatchObject({ trigger: 'submit', ledger: DENIED, note: ['note_below'] }); + expect(after).toMatchObject({ trigger: 'after', ledger: ['ledger_1'], note: ['note_below', 'note_peer'] }); + expect(writeContexts[writeContexts.length - 1]).toMatchObject({ isSystem: true, userId: 'usr_above' }); + }); + + it('CONTROL: a user-less system run keeps its envelope, and the hook is still refused for want of a user', async () => { + const before = firings.length; + const res = await automation.execute('hgp_sweep', { event: 'schedule', params: {} } as never); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + const write = writeContexts[writeContexts.length - 1]; + expect(write).toMatchObject({ isSystem: true, actor: 'svc:flow:hgp_sweep', positions: [], permissions: [] }); + expect(write.userId).toBeUndefined(); + expect(firings.slice(before)).toEqual([{ + trigger: 'sweep', + ledger: { code: 'HOOK_UNSCOPED_DATA_ACCESS', status: 403 }, + note: { code: 'HOOK_UNSCOPED_DATA_ACCESS', status: 403 }, + }]); + }); +}); diff --git a/packages/services/service-automation/src/runtime-identity.ts b/packages/services/service-automation/src/runtime-identity.ts index 01817d7d8c0..4817a3334e1 100644 --- a/packages/services/service-automation/src/runtime-identity.ts +++ b/packages/services/service-automation/src/runtime-identity.ts @@ -6,10 +6,12 @@ import { markGuardRefusal } from './guard-refusal.js'; /** * The IDENTITY envelope a flow's data nodes pass to ObjectQL as * `options.context` when the run resolves a principal. A structural subset of - * the kernel `ExecutionContext` — it always carries the three fields the - * security middleware keys on (`isSystem`, `positions`, `permissions`) so it is - * directly assignable to the engine's `context` option, plus the optional - * `userId`/`tenantId` of the acting user. + * the kernel `ExecutionContext`, so it is directly assignable to the engine's + * `context` option: always `isSystem`, plus the optional `userId`/`tenantId` of + * the acting user, and the `positions` / `permissions` the security middleware + * reads for a non-system principal — always on a `runAs:'user'` run, on a + * user-less `runAs:'system'` run, and never on a `runAs:'system'` run that + * names a user (see {@link positions}). */ export interface RunIdentityContext { /** Elevated, RLS-bypassing system principal (full access) when true. */ @@ -39,10 +41,25 @@ export interface RunIdentityContext { * human instead of being refused user-less (#3760). */ userId?: string; - /** Acting user's role names (RLS parity with a direct REST request). */ - positions: string[]; - /** Acting user's explicit permission-set names. */ - permissions: string[]; + /** + * Acting user's role names (RLS parity with a direct REST request). Always + * present on a `runAs:'user'` run, and `[]` on a user-less `runAs:'system'` + * run. ABSENT on a `runAs:'system'` run that names a user, with + * {@link permissions}: the run's own operations never read them (the + * security middleware's `isSystem` short-circuit precedes every gate that + * does), and a `runAs:'user'` hook fired by one of its writes de-elevates to + * this context, whose carried arrays the hook keeps verbatim. Carried empty, + * they ran that hook on the member baseline; absent, the hook resolves the + * named user's own grants in {@link tenantId}, as that user's own request + * resolves them (ruling 6271152775 on the approvals writes, applied here). + */ + positions?: string[]; + /** + * Acting user's explicit permission-set names. Present and absent exactly as + * {@link positions} is: the hook seed resolves a user's grants only when + * this one is absent. + */ + permissions?: string[]; /** * The acting user's fellow-organization peers, the user included: the value * RLS reads as `current_user.org_user_ids` (`sys_user_org_members` and every @@ -205,10 +222,13 @@ export function runOrgUserIds(context: AutomationContext | undefined): string[] * `owner_only_writes` RLS keys on `created_by`). Same envelope shape as * the action-body seam's `{ ...caller, isSystem: true }` (the * hotcrm#548-family fix in @objectstack/runtime) — one platform posture - * for "elevated, user-triggered" writers. A run whose trigger genuinely - * resolved no user (a schedule) stays user-less: the actor label + - * `flowRunId` are its provenance, and ADR-0118 D1 forbids inventing a - * sentinel or pseudo-user in its place. + * for "elevated, user-triggered" writers. A run that names a user carries + * no `positions` / `permissions`, so a `runAs:'user'` hook its writes fire + * resolves that user's own grants instead of running on the member + * baseline (see {@link RunIdentityContext.positions}). A run whose trigger + * genuinely resolved no user (a schedule) stays user-less and keeps them + * empty: the actor label + `flowRunId` are its provenance, and ADR-0118 D1 + * forbids inventing a sentinel or pseudo-user in its place. * - `runAs:'user'` (default) → the triggering user's identity * (`{ userId, positions, permissions, org_user_ids?, email?, tenantId? }`), so the * security middleware enforces that user's row-level security. The run can @@ -281,8 +301,20 @@ export function resolveRunDataContext(context: AutomationContext | undefined): R // A run that must genuinely cross orgs is a user-less one (a schedule // carries no org, nothing narrows) or sets explicit fields. ...(context.tenantId ? { tenantId: context.tenantId } : {}), - positions: [], - permissions: [], + // Elevation grants this run everything, so no `positions` / + // `permissions` are read for its own operations. They matter only to a + // `runAs:'user'` hook one of its writes fires: that hook de-elevates to + // this context and keeps any arrays it carries verbatim (a narrowed + // ceiling is never re-broadened), resolving the user's grants only when + // none are carried. So a run that names a user carries none, and such a + // hook runs on that user's own grants in `tenantId`, exactly as the + // user's own request would — never on the member baseline that empty + // arrays left it with. After an approval decision the user is the + // decider, so the hooks the post-decision writes fire run on the + // decider's grants. A user-less run (a schedule) keeps the empty arrays: + // there is no user to resolve, and a `runAs:'user'` hook it fires is + // refused for want of one. + ...(context.userId ? {} : { positions: [], permissions: [] }), ...(flowRunId ? { flowRunId } : {}), }; }