fix(approvals): isOverrideActor resolves TENANT-admin standing from the ADR-0095 rung, never from a position name

.changeset/16166-override-actor-tenant-arm-rung.md

@@ -0,0 +1,11 @@
+---
+"@objectstack/plugin-approvals": patch
+---
+
+`ApprovalService`'s privileged-override gate now resolves TENANT-admin standing from the ADR-0095 capability rung alone. Its tenant arm previously also admitted any principal whose `current_user.positions` contained the built-in identity names `org_owner` or `org_admin`, and a name on that array is not evidence of the capability behind it (#16166).
+
+`positions[]` carries two different things at once: the ADR-0068 D2 **projection** of a membership role, whose source of truth is `sys_member.role`, and ADR-0057 D4 `sys_user_position` assignment values. A stored assignment row spelling one of those built-in names therefore arrived on the array with no org-administration grant behind it and satisfied the override gate anyway — for `decideNode`, `recall` and the console's participant-visibility read, within that organization. This is the tenant half of the same defect the platform arm of the same predicate had (#15981), and it lands the same way: **read the rung, never the name.**
+
+- **The tenant rung is not the platform one.** ADR-0095 D3 resolves `TENANT_ADMIN` in `derivePosture` from the org-admin capability grants (`organization_admin` / `organization_admin_no_bypass`) and from nothing else, and those grants are what `packages/spec` declares that rung's source of truth. So the surviving two arms — the derived `posture` and the held capability — are one authority read in two spellings, kept apart only so a transport that never resolved `posture` still reads the grant.
+- **The #3424 stuck-approval escape hatch is unchanged** for anyone who actually holds org-admin standing: a genuine `organization_admin` grant still overrides, still only inside its own organization, and the decision is still audited as `via_override`.
+- **Who could notice.** A principal whose only claim to tenant-admin override was a stored `sys_user_position` row spelling `org_owner` / `org_admin` loses it. That row was never an assignment of the identity it spells — the platform refuses new ones on write — and the supported route to override standing is the org-admin capability grant, which the membership role provisions automatically for owners and admins.

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

@@ -22,15 +22,14 @@ import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formul
 // a third answer to a question the codebase already answered two ways.
 import { createRecordOrganizationResolver, type RecordOrganizationResolver } from '@objectstack/metadata-core';
 import { keysetWalk, strandedDecisionFailure } from '@objectstack/types';
-// [#15981] `BUILTIN_IDENTITY_PLATFORM_ADMIN` is deliberately absent: the
-// platform arm of `isOverrideActor` reads the ADR-0095 rung, never the name.
-// The two org-level built-ins below are a NARROWER question and are untouched
-// here — see that predicate's doc block.
+// [#15981 / #16166] Every built-in identity NAME is deliberately absent here:
+// both arms of `isOverrideActor` read an ADR-0095 capability rung, never a name.
+// `BUILTIN_IDENTITY_PLATFORM_ADMIN` went with the platform arm's name read and
+// `BUILTIN_IDENTITY_ORG_OWNER` / `_ORG_ADMIN` with the tenant arm's — see that
+// predicate's doc block.
 import {
   ADMIN_FULL_ACCESS,
   ORGANIZATION_ADMIN_GRANTS,
-  BUILTIN_IDENTITY_ORG_OWNER,
-  BUILTIN_IDENTITY_ORG_ADMIN,
 } from '@objectstack/spec/identity';
 import type {
   IApprovalService,
@@ -1367,15 +1366,16 @@ export class ApprovalService implements IApprovalService {
    * A platform admin crosses the tenant wall (matching the unscoped
    * `admin_full_access` evidence); a tenant admin may override only within their
    * own org (or an org-less request). A system context always passes. Signals are
-   * read defensively off the resolved exec context (`permissions` / `positions` /
-   * the derived `posture`, ADR-0095) so any transport that resolves through the
-   * shared authz resolver lights this up without extra wiring.
+   * read defensively off the resolved exec context (`permissions` and the derived
+   * `posture`, ADR-0095) so any transport that resolves through the shared authz
+   * resolver lights this up without extra wiring. ⛔ NOT `positions` — on BOTH
+   * rungs now: that array carries names, and a name is not an authority (see the
+   * two blocks inside).
    */
   private isOverrideActor(context: ExecutionContext, requestOrg?: string | null): boolean {
     if (!context) return false;
     if (context.isSystem) return true;
     const perms = Array.isArray(context.permissions) ? context.permissions : [];
-    const positions = Array.isArray(context.positions) ? context.positions : [];
     // [#7135] A DECLARED read. `posture` (ADR-0095 D2) is resolved by
     // `resolveAuthzContext` and is a field of the envelope the contract has
     // named here since #6523 — the doc block above already says it is the
@@ -1402,10 +1402,29 @@ export class ApprovalService implements IApprovalService {
     const isPlatformAdmin = posture === 'PLATFORM_ADMIN'
       || perms.includes(ADMIN_FULL_ACCESS);
     if (isPlatformAdmin) return true;
+    // ⛔ [#16166] The tenant counterpart of the platform rule above — and
+    // deliberately NOT the same expression. ADR-0095 D3 derives `TENANT_ADMIN`
+    // in `derivePosture` from `ORGANIZATION_ADMIN_GRANTS.some(n =>
+    // permissions.includes(n))` and from nothing else, and
+    // `packages/spec/src/identity/eval-user.zod.ts` declares those two grants
+    // the source of truth for that rung. So the two arms below are ONE authority
+    // read in two spellings — kept apart only so a transport that never resolved
+    // `posture` still reads the held capability — and neither of them is the
+    // platform side's literal copied across.
+    //
+    // There is NO `positions.includes(BUILTIN_IDENTITY_ORG_OWNER | _ORG_ADMIN)`
+    // arm any more, for the same reason the platform arm lost its name read:
+    // ADR-0068 D2 declares those names a normalized PROJECTION into `positions`
+    // whose sources of truth are elsewhere (`sys_member.role`), while the same
+    // array also carries ADR-0057 D4 `sys_user_position` values — so a stored
+    // row spelling one arrived here with no authority behind it, and an OR is
+    // only as strong as its weakest arm. The write door refuses such a row today
+    // (`plugin-security`'s `reserved_identity_position` rule), but that ruling
+    // refused new writes WITHOUT a migration, so rows predating it still resolve
+    // on every request — and a reader that trusts a name is not an invariant in
+    // any case. Driven in `approval-tenant-positions-name-authority.test.ts`.
     const isTenantAdmin = posture === 'TENANT_ADMIN'
-      || ORGANIZATION_ADMIN_GRANTS.some((n) => perms.includes(n))
-      || positions.includes(BUILTIN_IDENTITY_ORG_OWNER)
-      || positions.includes(BUILTIN_IDENTITY_ORG_ADMIN);
+      || ORGANIZATION_ADMIN_GRANTS.some((n) => perms.includes(n));
     if (!isTenantAdmin) return false;
     // A tenant admin's authority stops at their own org; a null-org request is
     // global and any admin may release it.