Skip to content

Commit 95065f3

Browse files
committed
Merge origin/main into claude/issue-18130-ccr-routes-probed-into-the-register
Claude-Session: https://claude.ai/code/session_01HZfg2AwVX191qCizp88gQr Co-authored-by: Claude <noreply@anthropic.com>
2 parents bb85769 + 917b87e commit 95065f3

4 files changed

Lines changed: 483 additions & 13 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/plugin-approvals": patch
3+
---
4+
5+
`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).
6+
7+
`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.**
8+
9+
- **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.
10+
- **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`.
11+
- **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‎

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

0 commit comments

Comments
 (0)