fix(plugin-approvals): an approval's status write-back runs its automation as the person who acted, in the record's organisation

.changeset/24519-approvals-context-identity.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): an approval's status write-back runs its automation as the person who acted, in the record's organisation
+
+Clause-②: no
+
+- **What changes.** The write that mirrors a request's status onto the record's `approvalStatusField` (submit, approve, reject, recall, send back, the action link) now names the organisation of the record it writes, and no longer carries an empty set of positions and permission sets. A `runAs: 'user'` hook on that object, and a `runAs: 'user'` record-change flow fired by that write, now run with the acting person's own request envelope in the record's organisation: their positions, their permission sets and their organisation's members, resolved exactly as their own request in that organisation resolves them.
+- **Before.** Such a hook ran on the member baseline and saw only the acting person in `sys_user`. Such a flow resolved the person's grants with no organisation, so it saw only the person as a peer and did not run with what the record's organisation grants them.
+- **Every acting person, one organisation.** An approver drawn from another organisation's directory also acts in the record's organisation. The organisation is read from the record when the status is written, so a record moved to another organisation while its approval was pending is written, and cascades, in its new organisation.
+- **Unchanged.** The write stays elevated (`isSystem`), so it still lands on a locked record and on a read-only status field. A status written by a sweep with no person behind it (the dead-run release) stays user-less, so a `runAs: 'user'` automation it fires is still refused. `ctx.session.organizationId` in a hook on the write now carries the organisation, and `ctx.session.positions` is absent instead of an empty list.
+
+A patch: no export, type, accepted value or stored column changes.

packages/plugins/plugin-approvals/src/approval-service.test.ts

@@ -5121,13 +5121,20 @@ describe('status mirror identity (#3783)', () => {
     expect(mirrorContext()).toMatchObject({ isSystem: true });
   });
 
-  it('carries the actor WITHOUT org-scoping the write', async () => {
-    // `tenantId` on an ExecutionContext is a driver-scoping knob, not
-    // attribution: ObjectQL turns it into a tenant predicate on the update. The
-    // submitter's org (`t1` on CTX) must therefore not ride along, or the mirror
-    // would silently no-op on a record whose org differs from the request's.
+  it('names the organisation it acts in, and carries no grants of its own (#24519)', async () => {
+    // Ruling 6271152775 on #24519 reverses what this pin used to assert: a
+    // named actor's mirror names its organisation through `tenantId`, so a
+    // `runAs:'user'` consumer resolves the actor's own request envelope in
+    // it. The organisation is the subject record's, read live; this fake
+    // engine exposes no schema, so the object has none of its own and the
+    // request's organisation stands in (`t1`, from the opening context), as
+    // it does for the request row's stamp. No `positions` / `permissions`
+    // ride along — a consumer would keep them verbatim — and no
+    // `organizationId`, which is not an envelope field. The record-organisation
+    // read and the end-to-end envelope are pinned in
+    // `status-mirror-envelope.integration.test.ts`.
     await open();
-    expect(mirrorContext()).not.toHaveProperty('tenantId');
-    expect(mirrorContext()).not.toHaveProperty('organizationId');
+    expect(engine._tables['sys_approval_request'][0].organization_id).toBe('t1');
+    expect(mirrorContext()).toEqual({ isSystem: true, userId: 'u1', tenantId: 't1' });
   });
 });

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -3183,39 +3183,97 @@ export class ApprovalService implements IApprovalService {
    * most natural approvals automation there is, had to declare `runAs:'system'`
    * and take blanket elevation for a case where a perfectly good scoped identity
    * existed. Re-attaching the actor lets those cascades run as the deciding user
-   * with RLS enforced. Same shape the approval node already uses when it calls
-   * into this service (`approval-node.ts`).
+   * with RLS enforced.
    *
    * `actorId` is `null` for the genuinely machine-driven transitions (the SLA
    * escalation's auto-decision, the dead-run sweep). There is no human to name
    * there, and naming a sentinel would put a non-user in `updated_by` and in
-   * every downstream flow's identity. Those cascades stay user-less — a flow
-   * that wants to react to them still has to declare `runAs:'system'`, which is
-   * the honest answer rather than an oversight.
-   *
-   * Deliberately carries `userId` ONLY, not the request's org. On an
-   * ExecutionContext `tenantId` is a driver-scoping knob, not attribution
-   * (`buildDriverOptions` turns it into a tenant predicate on the update), so
-   * passing it would newly org-scope this write and silently no-op the mirror on
-   * a record whose org differs from the request's — while buying nothing: the
-   * automation engine back-fills the run's `tenantId` from the resolved user's
-   * own grants.
+   * every downstream flow's identity. Those cascades stay user-less, under
+   * {@link SYSTEM_CTX} — a flow that wants to react to them still has to
+   * declare `runAs:'system'`, which is the honest answer rather than an
+   * oversight.
+   *
+   * **A named actor's write acts in the record's organisation, and carries no
+   * grants of its own (#24519, ruling 6271152775).** Its context is
+   * {@link attributedMirrorContext}: `isSystem`, the actor's `userId`, and
+   * `tenantId` = the organisation of the record being written. Every consumer
+   * that runs as the actor then resolves the actor's own request envelope in
+   * that organisation, through the one resolver the request path uses
+   * (`resolveUserAuthzGrants`):
+   *  - a `runAs:'user'` hook's pinned api is seeded with the actor's positions,
+   *    permission sets and organisation peers. Positions and permission sets a
+   *    context CARRIES are kept verbatim, so the empty arrays this write used to
+   *    carry left such a hook on the member baseline instead of the actor's own
+   *    grants;
+   *  - a record-change flow receives the actor and the organisation as its
+   *    `tenantId`, and a `runAs:'user'` run resolves the same envelope under it.
+   *    Nothing downstream supplies a missing organisation: the automation
+   *    engine's grants bridge answers a `tenantId` only when it was handed one,
+   *    and an organisation-less resolution is not the actor's request in the
+   *    record's organisation — its peers are the actor alone, and its grants
+   *    are not the ones that organisation gives them.
+   * The write itself is authorized by `isSystem` alone — the security
+   * middleware short-circuits before it reads any grant — so leaving the arrays
+   * off changes nothing about what this write may do.
+   *
+   * The organisation is the record's, for every actor: an approver drawn from
+   * another organisation's directory (ADR-0105 D9) included, which is the
+   * reading #24321 applies to a `runAs:'user'` flow's decider. On an
+   * ExecutionContext `tenantId` is also the driver's scoping knob
+   * (`buildDriverOptions`): under a walled posture it narrows this update to that
+   * organisation's rows (and organisation-less ones), under `single` it scopes
+   * nothing (ADR-0131 D11). That is why the organisation is the record's LIVE
+   * one, read at write time, and not the request's stamp: the two agree by
+   * construction (#10101 stamps the request with the subject record's
+   * organisation), except for a record re-parented while its approval was
+   * pending, which the stamp would scope this write away from.
    */
   private async mirrorStatusField(
     object: string,
     recordId: string,
     field: string,
     status: string,
     actorId: string | null,
+    requestOrg: string | null | undefined,
   ): Promise<void> {
     try {
-      const context = actorId ? { ...SYSTEM_CTX, userId: actorId } : SYSTEM_CTX;
+      const context = actorId
+        ? await this.attributedMirrorContext(object, recordId, actorId, requestOrg)
+        : SYSTEM_CTX;
       await this.engine.update(object, { id: recordId, [field]: status }, { context });
     } catch (err: any) {
       this.logger?.warn?.(`[approvals] mirrorStatusField failed: ${err?.message ?? err}`);
     }
   }
 
+  /**
+   * The context a named actor's status mirror writes under (see
+   * {@link mirrorStatusField}): `{ isSystem: true, userId, tenantId }` and
+   * nothing else. The organisation is the subject record's live one, read
+   * through {@link recordOrgResolver} — the resolver the request row was
+   * stamped by — and the request's own organisation for an object that has
+   * none (or a record that carries none), as at the stamp. ⛔ No `positions` /
+   * `permissions`: a carried array is kept verbatim by every `runAs:'user'`
+   * consumer, so an empty one would stand in for the actor's own grants.
+   */
+  private async attributedMirrorContext(
+    object: string,
+    recordId: string,
+    actorId: string,
+    requestOrg: string | null | undefined,
+  ): Promise<ExecutionContext> {
+    const column = this.recordOrgResolver.organizationFieldFor(object);
+    let organization: string | null = null;
+    if (column) {
+      const rows = await this.engine.find(object, {
+        where: { id: recordId }, fields: ['id', column], limit: 1, context: SYSTEM_CTX,
+      });
+      organization = this.recordOrgResolver.organizationOf(object, Array.isArray(rows) ? rows[0] : undefined);
+    }
+    if (!organization && requestOrg != null && String(requestOrg) !== '') organization = String(requestOrg);
+    return { isSystem: true, userId: actorId, ...(organization ? { tenantId: organization } : {}) };
+  }
+
   /**
    * Re-read a business record's CURRENT state by id so approver resolution binds
    * to live data at node entry, not the trigger snapshot the flow froze into
@@ -3697,7 +3755,7 @@ export class ApprovalService implements IApprovalService {
       // first-party path.
       await this.mirrorStatusField(
         input.object, input.recordId, input.config.approvalStatusField, 'pending',
-        row.submitter_id ?? null,
+        row.submitter_id ?? null, requestOrg,
       );
     }
 
@@ -4008,7 +4066,7 @@ export class ApprovalService implements IApprovalService {
     if (config.approvalStatusField) {
       await this.mirrorStatusField(
         raw.object_name, raw.record_id, config.approvalStatusField, finalStatus,
-        actingUserId(context),
+        actingUserId(context), org,
       );
     }
     const fresh = await this.readBackRequest(requestId, context);
@@ -4498,7 +4556,7 @@ export class ApprovalService implements IApprovalService {
     if (config.approvalStatusField) {
       await this.mirrorStatusField(
         raw.object_name, raw.record_id, config.approvalStatusField, 'recalled',
-        actingUserId(context),
+        actingUserId(context), org,
       );
     }
 
@@ -4837,7 +4895,7 @@ export class ApprovalService implements IApprovalService {
       if (config.approvalStatusField) {
         await this.mirrorStatusField(
           raw.object_name, raw.record_id, config.approvalStatusField, 'rejected',
-          actingUserId(context),
+          actingUserId(context), org,
         );
       }
       let resumed = false;
@@ -4879,7 +4937,7 @@ export class ApprovalService implements IApprovalService {
     if (config.approvalStatusField) {
       await this.mirrorStatusField(
         raw.object_name, raw.record_id, config.approvalStatusField, 'returned',
-        actingUserId(context),
+        actingUserId(context), org,
       );
     }
 
@@ -6552,7 +6610,7 @@ export class ApprovalService implements IApprovalService {
       // No human did this — a sweep did. Left user-less on purpose (#3783): a
       // flow that wants to react to a dead-run release declares runAs:'system'.
       await this.mirrorStatusField(
-        raw.object_name, raw.record_id, config.approvalStatusField, 'recalled', null,
+        raw.object_name, raw.record_id, config.approvalStatusField, 'recalled', null, org,
       );
     }