fix(service-automation)!: a runAs:'user' hook fired by a user-triggered runAs:'system' flow's write runs on that user's own grants

.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.
+
+<!-- adr-0087: not-required (no-migration-prescription) A runtime semantics change of the data context a runAs 'system' flow run hands ObjectQL when it names a user, not a metadata shape change. No spec key, enum value, export, option, config field, response field or stored shape is removed, renamed or re-shaped, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. What narrows is whose grants a runAs 'user' hook fired by such a run's write carries: the named user's own, resolved in the run's organization, instead of the member baseline, so for a user whose own grants admit less than the baseline that hook reads less. The run's own operations keep full system privileges. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this behaviour and this diff adds none (not registered / already-registered); and the change is a behaviour change, not only an interface or type change (not runtime-interface-only / type-surface-only). -->
+
+**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'`.

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 () => {

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: [],
     });
   });
 

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.

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: <decider> }`.
-      expect(after).toMatchObject({ isSystem: true, userId: 'usr_mgr', positions: [], permissions: [], tenantId: 'org_a' });
+      // Ruling 6267719791 Q1 A: `{ isSystem: true, userId: <decider> }`, 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 () => {

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();
   });