Skip to content

Commit 3306692

Browse files
committed
fix(plugin-approvals): the status mirror writes in the record's organisation and carries no grants of its own
A named actor's mirror write now carries { isSystem, userId, tenantId } with tenantId read live from the subject record (the request's organisation for an object with none), and no positions / permissions, so a runAs:'user' hook or record-change flow it fires resolves the actor's own request envelope in that organisation. Claude-Session: https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5a871e4 commit 3306692

1 file changed

Lines changed: 78 additions & 20 deletions

File tree

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

Lines changed: 78 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3159,39 +3159,97 @@ export class ApprovalService implements IApprovalService {
31593159
* most natural approvals automation there is, had to declare `runAs:'system'`
31603160
* and take blanket elevation for a case where a perfectly good scoped identity
31613161
* existed. Re-attaching the actor lets those cascades run as the deciding user
3162-
* with RLS enforced. Same shape the approval node already uses when it calls
3163-
* into this service (`approval-node.ts`).
3162+
* with RLS enforced.
31643163
*
31653164
* `actorId` is `null` for the genuinely machine-driven transitions (the SLA
31663165
* escalation's auto-decision, the dead-run sweep). There is no human to name
31673166
* there, and naming a sentinel would put a non-user in `updated_by` and in
3168-
* every downstream flow's identity. Those cascades stay user-less — a flow
3169-
* that wants to react to them still has to declare `runAs:'system'`, which is
3170-
* the honest answer rather than an oversight.
3171-
*
3172-
* Deliberately carries `userId` ONLY, not the request's org. On an
3173-
* ExecutionContext `tenantId` is a driver-scoping knob, not attribution
3174-
* (`buildDriverOptions` turns it into a tenant predicate on the update), so
3175-
* passing it would newly org-scope this write and silently no-op the mirror on
3176-
* a record whose org differs from the request's — while buying nothing: the
3177-
* automation engine back-fills the run's `tenantId` from the resolved user's
3178-
* own grants.
3167+
* every downstream flow's identity. Those cascades stay user-less, under
3168+
* {@link SYSTEM_CTX} — a flow that wants to react to them still has to
3169+
* declare `runAs:'system'`, which is the honest answer rather than an
3170+
* oversight.
3171+
*
3172+
* **A named actor's write acts in the record's organisation, and carries no
3173+
* grants of its own (#24519, ruling 6271152775).** Its context is
3174+
* {@link attributedMirrorContext}: `isSystem`, the actor's `userId`, and
3175+
* `tenantId` = the organisation of the record being written. Every consumer
3176+
* that runs as the actor then resolves the actor's own request envelope in
3177+
* that organisation, through the one resolver the request path uses
3178+
* (`resolveUserAuthzGrants`):
3179+
* - a `runAs:'user'` hook's pinned api is seeded with the actor's positions,
3180+
* permission sets and organisation peers. Positions and permission sets a
3181+
* context CARRIES are kept verbatim, so the empty arrays this write used to
3182+
* carry left such a hook on the member baseline instead of the actor's own
3183+
* grants;
3184+
* - a record-change flow receives the actor and the organisation as its
3185+
* `tenantId`, and a `runAs:'user'` run resolves the same envelope under it.
3186+
* Nothing downstream supplies a missing organisation: the automation
3187+
* engine's grants bridge answers a `tenantId` only when it was handed one,
3188+
* and an organisation-less resolution is not the actor's request in the
3189+
* record's organisation — its peers are the actor alone, and its positions
3190+
* come from every organisation the actor belongs to.
3191+
* The write itself is authorized by `isSystem` alone — the security
3192+
* middleware short-circuits before it reads any grant — so leaving the arrays
3193+
* off changes nothing about what this write may do.
3194+
*
3195+
* The organisation is the record's, for every actor: an approver drawn from
3196+
* another organisation's directory (ADR-0105 D9) included, which is the
3197+
* reading #24321 applies to a `runAs:'user'` flow's decider. On an
3198+
* ExecutionContext `tenantId` is also the driver's scoping knob
3199+
* (`buildDriverOptions`): under a walled posture it narrows this update to that
3200+
* organisation's rows (and organisation-less ones), under `single` it scopes
3201+
* nothing (ADR-0131 D11). That is why the organisation is the record's LIVE
3202+
* one, read at write time, and not the request's stamp: the two agree by
3203+
* construction (#10101 stamps the request with the subject record's
3204+
* organisation), except for a record re-parented while its approval was
3205+
* pending, which the stamp would scope this write away from.
31793206
*/
31803207
private async mirrorStatusField(
31813208
object: string,
31823209
recordId: string,
31833210
field: string,
31843211
status: string,
31853212
actorId: string | null,
3213+
requestOrg: string | null | undefined,
31863214
): Promise<void> {
31873215
try {
3188-
const context = actorId ? { ...SYSTEM_CTX, userId: actorId } : SYSTEM_CTX;
3216+
const context = actorId
3217+
? await this.attributedMirrorContext(object, recordId, actorId, requestOrg)
3218+
: SYSTEM_CTX;
31893219
await this.engine.update(object, { id: recordId, [field]: status }, { context });
31903220
} catch (err: any) {
31913221
this.logger?.warn?.(`[approvals] mirrorStatusField failed: ${err?.message ?? err}`);
31923222
}
31933223
}
31943224

3225+
/**
3226+
* The context a named actor's status mirror writes under (see
3227+
* {@link mirrorStatusField}): `{ isSystem: true, userId, tenantId }` and
3228+
* nothing else. The organisation is the subject record's live one, read
3229+
* through {@link recordOrgResolver} — the resolver the request row was
3230+
* stamped by — and the request's own organisation for an object that has
3231+
* none (or a record that carries none), as at the stamp. ⛔ No `positions` /
3232+
* `permissions`: a carried array is kept verbatim by every `runAs:'user'`
3233+
* consumer, so an empty one would stand in for the actor's own grants.
3234+
*/
3235+
private async attributedMirrorContext(
3236+
object: string,
3237+
recordId: string,
3238+
actorId: string,
3239+
requestOrg: string | null | undefined,
3240+
): Promise<ExecutionContext> {
3241+
const column = this.recordOrgResolver.organizationFieldFor(object);
3242+
let organization: string | null = null;
3243+
if (column) {
3244+
const rows = await this.engine.find(object, {
3245+
where: { id: recordId }, fields: ['id', column], limit: 1, context: SYSTEM_CTX,
3246+
} as any);
3247+
organization = this.recordOrgResolver.organizationOf(object, Array.isArray(rows) ? rows[0] : rows);
3248+
}
3249+
if (!organization && requestOrg != null && String(requestOrg) !== '') organization = String(requestOrg);
3250+
return { isSystem: true, userId: actorId, ...(organization ? { tenantId: organization } : {}) };
3251+
}
3252+
31953253
/**
31963254
* Re-read a business record's CURRENT state by id so approver resolution binds
31973255
* to live data at node entry, not the trigger snapshot the flow froze into
@@ -3673,7 +3731,7 @@ export class ApprovalService implements IApprovalService {
36733731
// first-party path.
36743732
await this.mirrorStatusField(
36753733
input.object, input.recordId, input.config.approvalStatusField, 'pending',
3676-
row.submitter_id ?? null,
3734+
row.submitter_id ?? null, requestOrg,
36773735
);
36783736
}
36793737

@@ -3984,7 +4042,7 @@ export class ApprovalService implements IApprovalService {
39844042
if (config.approvalStatusField) {
39854043
await this.mirrorStatusField(
39864044
raw.object_name, raw.record_id, config.approvalStatusField, finalStatus,
3987-
actingUserId(context),
4045+
actingUserId(context), org,
39884046
);
39894047
}
39904048
const fresh = await this.readBackRequest(requestId, context);
@@ -4466,7 +4524,7 @@ export class ApprovalService implements IApprovalService {
44664524
if (config.approvalStatusField) {
44674525
await this.mirrorStatusField(
44684526
raw.object_name, raw.record_id, config.approvalStatusField, 'recalled',
4469-
actingUserId(context),
4527+
actingUserId(context), org,
44704528
);
44714529
}
44724530

@@ -4805,7 +4863,7 @@ export class ApprovalService implements IApprovalService {
48054863
if (config.approvalStatusField) {
48064864
await this.mirrorStatusField(
48074865
raw.object_name, raw.record_id, config.approvalStatusField, 'rejected',
4808-
actingUserId(context),
4866+
actingUserId(context), org,
48094867
);
48104868
}
48114869
let resumed = false;
@@ -4847,7 +4905,7 @@ export class ApprovalService implements IApprovalService {
48474905
if (config.approvalStatusField) {
48484906
await this.mirrorStatusField(
48494907
raw.object_name, raw.record_id, config.approvalStatusField, 'returned',
4850-
actingUserId(context),
4908+
actingUserId(context), org,
48514909
);
48524910
}
48534911

@@ -6510,7 +6568,7 @@ export class ApprovalService implements IApprovalService {
65106568
// No human did this — a sweep did. Left user-less on purpose (#3783): a
65116569
// flow that wants to react to a dead-run release declares runAs:'system'.
65126570
await this.mirrorStatusField(
6513-
raw.object_name, raw.record_id, config.approvalStatusField, 'recalled', null,
6571+
raw.object_name, raw.record_id, config.approvalStatusField, 'recalled', null, org,
65146572
);
65156573
}
65166574

0 commit comments

Comments
 (0)