From bd157492c1dedc18ac98385aa6684247049967b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 03:37:23 +0000 Subject: [PATCH 1/3] =?UTF-8?q?test(approvals):=20drive=20the=20tenant=20a?= =?UTF-8?q?rm=20of=20isOverrideActor=20=E2=80=94=20a=20stored=20org=20buil?= =?UTF-8?q?t-in=20name=20still=20confers=20override?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 1 of the card: measure before deciding what to build. This harness resolves a principal through the REAL `resolveUserAuthzGrants` and drives three doors (`decideNode`, `recall`, `listRequests`) for a non-slate, non-submitter actor in the request's own organization. Committed RED on purpose, so the fix's before/after readings are taken from committed states rather than from a working tree. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude --- ...al-tenant-positions-name-authority.test.ts | 430 ++++++++++++++++++ 1 file changed, 430 insertions(+) create mode 100644 packages/plugins/plugin-approvals/src/approval-tenant-positions-name-authority.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-tenant-positions-name-authority.test.ts b/packages/plugins/plugin-approvals/src/approval-tenant-positions-name-authority.test.ts new file mode 100644 index 00000000000..033ffce0265 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-tenant-positions-name-authority.test.ts @@ -0,0 +1,430 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16166] A `sys_user_position` row SPELLING `org_owner` / `org_admin` confers + * no TENANT-admin override authority on the approvals surface. + * + * ## The defect this pins + * + * `isOverrideActor`'s TENANT arm read the capability rung FIRST and then ORed + * two NAMES onto it: + * + * const isTenantAdmin = posture === 'TENANT_ADMIN' + * || ORGANIZATION_ADMIN_GRANTS.some((n) => perms.includes(n)) + * || positions.includes(BUILTIN_IDENTITY_ORG_OWNER) // ← the holes + * || positions.includes(BUILTIN_IDENTITY_ORG_ADMIN); + * + * the same species #15981 / PR #16148 closed on the PLATFORM arm of the same + * predicate, and for the same reason: an OR is only as strong as its weakest + * arm, so reading the rung first protects nothing. + * + * ## What the tenant-authority RUNG is — established, not copied across + * + * ⛔ The platform arm's expression is NOT reusable here. `posture` is resolved + * by `derivePosture` (`packages/core/src/security/posture-ladder.ts`) from HELD + * CAPABILITY GRANTS: `PLATFORM_ADMIN` from the unscoped `admin_full_access` + * grant, `TENANT_ADMIN` from `ORGANIZATION_ADMIN_GRANTS.some(n => + * permissions.includes(n))` — the identical expression the second arm above + * already spells. So on the tenant side the rung and the capability arm are ONE + * reading of ONE authority (`ORGANIZATION_ADMIN` / `ORGANIZATION_ADMIN_NO_BYPASS` + * are declared in `packages/spec/src/identity/eval-user.zod.ts` as "the source + * of truth for the `TENANT_ADMIN` posture rung"), kept as two arms only so a + * transport that never resolved `posture` still reads the capability. The two + * NAMES are the other thing entirely: ADR-0068 D2 declares them "a normalized + * PROJECTION into `current_user.positions`", whose sources of truth live + * elsewhere. A projection is not an authority, and that is the whole fix. + * + * ## POPULATION OF THIS PIN — stated because a pin proves only what it covers + * + * Covers three doors of `isOverrideActor` — `decideNode` (approve/reject), + * `recall`, and `listRequests` (the console participant-visibility read) — for + * a NON-slate, NON-submitter actor in the SAME organization as the request + * (the tenant arm is org-confined by construction, so same-org is where it can + * reach at all), in four shapes resolved through the REAL + * `resolveUserAuthzGrants`: a stored assignment row spelling `org_owner`, one + * spelling `org_admin`, a genuine org-scoped `organization_admin` capability + * grant, and a plain member (the floor that proves the arms are not all passing + * for an unrelated reason). + * + * Does NOT cover: the PLATFORM arm (its own suite, + * `approval-positions-name-authority.test.ts`), `reassign` / `decide` / + * `getRequest`, the SLA escalation paths, or the ADR-0091 validity window. + * Those are other suites' populations and their passing is NOT evidence about + * this one. + * + * ⚠️ The stored row is modelled as one that ALREADY EXISTS. The write door was + * closed separately (`plugin-security` refuses a reserved identity name on + * `sys_user_position.position`), and that ruling was explicitly "refuse new + * writes only. No migration." — so a row predating it still resolves into + * `positions` on every request, which is exactly why the READER has to be fixed + * too. A reader is not an invariant; neither is a writer. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { + BUILTIN_IDENTITY_ORG_OWNER, + BUILTIN_IDENTITY_ORG_ADMIN, + ADMIN_FULL_ACCESS, + ORGANIZATION_ADMIN, + ORGANIZATION_ADMIN_GRANTS, +} from '@objectstack/spec/identity'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { resolveUserAuthzGrants } from '@objectstack/core'; +// The engine's OWN dispatch predicates, so this double cannot be looser than +// the engine it stands in for (`check:engine-double-contract`). +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import type { ApprovalRequestRow } from '@objectstack/spec/contracts'; +import { ApprovalService, type ApprovalNodeAutoOutcome } from './approval-service.js'; + +interface FakeRow { [k: string]: any } + +/** The same minimal engine shape `approval-positions-name-authority.test.ts` uses. */ +function makeFakeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + function matches(row: FakeRow, filter: any): boolean { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or') { + if (!(v as any[]).some((sub) => matches(row, sub))) return false; + continue; + } + if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`); + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (rv !== v) return false; + } + return true; + } + return { + _tables: tables, + async find(object: string, options?: any) { + const rows = ensure(object).filter((r) => matches(r, options?.filter ?? options?.where)); + const start = options?.offset ?? 0; + return rows.slice(start, start + (options?.limit ?? 1000)); + }, + async insert(object: string, data: any) { ensure(object).push({ ...data }); return { ...data }; }, + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const table = ensure(object); + if (dispatch.kind === 'multi') { + let n = 0; + for (let i = 0; i < table.length; i++) { + if (matches(table[i], options?.where)) { table[i] = { ...table[i], ...data }; n++; } + } + return { updated: n }; + } + const i = table.findIndex((r) => r.id === dispatch.id); + if (i >= 0) table[i] = { ...table[i], ...data }; + return i >= 0 ? { ...table[i] } : null; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const table = ensure(object); + if (dispatch.kind === 'multi') { + const survivors = table.filter((r) => !matches(r, options?.where)); + const deleted = table.length - survivors.length; + table.splice(0, table.length, ...survivors); + return { deleted }; + } + const i = table.findIndex((r) => r.id === dispatch.id); + if (i >= 0) table.splice(i, 1); + return { id: dispatch.id }; + }, + registerHook() {}, unregisterHooksByPackage() { return 0; }, async fire() {}, + }; +} + +/** ONE organization: the tenant arm is org-confined, so this is where it bites. */ +const ORG = 't_acme'; +const ACTOR = 'usr_delegate'; +const PS_ORG_ADMIN = 'ps_organization_admin'; + +/** + * A minimal ObjectQL double for the AUTHZ resolver — the shape (and the + * top-level `$` refusal) of `resolve-authz-context.platform-admin-config.test.ts`. + */ +function makeAuthzQl(tables: Record>>) { + const matches = (row: Record, where: any): boolean => + Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(row[k]); + return row[k] === v; + }); + return { + async find(object: string, opts: any) { + const rows = (tables[object] ?? []).filter((r) => matches(r, opts?.where)); + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + }, + }; +} + +type Shape = 'row-org-owner' | 'row-org-admin' | 'genuine' | 'plain'; + +const STORED_NAME: Partial> = { + 'row-org-owner': BUILTIN_IDENTITY_ORG_OWNER, + 'row-org-admin': BUILTIN_IDENTITY_ORG_ADMIN, +}; + +function authzTables(shape: Shape) { + const stored = STORED_NAME[shape]; + return { + sys_user: [{ id: ACTOR, email: 'delegate@example.com', email_verified: true }], + // An ordinary member of the organization — `member` normalizes to + // `org_member`, so nothing here carries administration standing. + sys_member: [{ organization_id: ORG, user_id: ACTOR, role: 'member' }], + sys_user_position: stored + // A row of exactly the ADR-0057 D4 shape that survives the write-side + // refusal by predating it (that ruling declined a migration). + ? [{ user_id: ACTOR, position: stored, organization_id: ORG }] + : [], + // An ACTIVE catalogue row, so ADR-0049's deactivated-position filter is not + // what carries the arm. + sys_position: stored + ? [{ id: `pos_${stored}`, name: stored, label: stored, active: true }] + : [], + sys_position_permission_set: [], + sys_user_permission_set: + shape === 'genuine' + ? [{ user_id: ACTOR, permission_set_id: PS_ORG_ADMIN, organization_id: ORG }] + : [], + sys_permission_set: [{ id: PS_ORG_ADMIN, name: ORGANIZATION_ADMIN, active: true }], + }; +} + +/** + * Resolve one principal through the REAL resolver, then build the context a + * transport would. The `rung` reading is `grants.posture === 'TENANT_ADMIN'` — + * ADR-0095 D3's tenant rung, derived by `derivePosture` from the org-admin + * capability grants and from nothing else. + */ +async function resolve(shape: Shape) { + const ql = makeAuthzQl(authzTables(shape)); + const grants = await resolveUserAuthzGrants(ql as any, ACTOR, { tenantId: ORG }); + const context = { + userId: ACTOR, + tenantId: ORG, + positions: grants.positions, + permissions: grants.permissions, + systemPermissions: grants.systemPermissions, + ...(grants.posture ? { posture: grants.posture } : {}), + } as ExecutionContext; + return { context, grants, rung: grants.posture === 'TENANT_ADMIN' }; +} + +/** + * Narrow `openNodeRequest`'s union, and REFUSE the auto-approval outcome — a + * slate that resolved empty would auto-approve with no request to attack, and + * every arm below would then be vacuously satisfied. + */ +function opened(result: ApprovalRequestRow | ApprovalNodeAutoOutcome): ApprovalRequestRow { + if ('autoApproved' in result) { + throw new Error('expected an OPENED approval request, got an auto-approval outcome'); + } + return result; +} + +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +/** The submitter — same organization, so the request is reachable by the arm at all. */ +const SUBMITTER = { userId: 'usr_submitter', tenantId: ORG, positions: [], permissions: [] } as any; + +function harness() { + const engine = makeFakeEngine(); + let n = 0; + const baseTime = new Date('2026-01-15T10:00:00Z').getTime(); + const svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(baseTime + (n++) * 1000) }, + }); + let seq = 0; + /** + * A request in the SAME organization, with a staffed slate the actor is not + * on. Each call takes its OWN record: `openNodeRequest` refuses a second + * pending request on the same one (`DUPLICATE_REQUEST`). + */ + const openRequest = () => { + const i = ++seq; + return svc.openNodeRequest( + { + object: 'opportunity', recordId: `opp${i}`, runId: `run_${i}`, nodeId: 'sign_off', + flowName: 'deal_flow', + config: { + approvers: [{ type: 'user' as const, value: 'usr_designated' }], + behavior: 'first_response' as const, + }, + record: { id: `opp${i}`, amount: 100 }, + } as any, + SUBMITTER, + ).then(opened); + }; + /** + * Attempt a decision and report what it DID, not merely what it threw — an + * ADMITTED write is the observable, and a throw-shaped assertion would go + * green on one (the sibling platform suite measured exactly that trap). + */ + const attemptDecide = async (ctx: ExecutionContext, id: string) => { + let threw: string | null = null; + try { + await svc.decideNode(id, { decision: 'approve', actorId: ACTOR }, ctx); + } catch (e) { + threw = String((e as any)?.message ?? e).split(':')[0]; + } + const [row] = await engine.find('sys_approval_request', { where: { id } }); + const actions = (await svc.listActions(id, SYS)).filter((a: any) => a.actor_id === ACTOR); + return { threw, status: row?.status, decided: actions.length > 0 }; + }; + /** The same shape for the recall door: did the request actually leave `pending`? */ + const attemptRecall = async (ctx: ExecutionContext, id: string) => { + let threw: string | null = null; + try { + await svc.recall(id, { actorId: ACTOR }, ctx); + } catch (e) { + threw = String((e as any)?.message ?? e).split(':')[0]; + } + const [row] = await engine.find('sys_approval_request', { where: { id } }); + return { threw, status: row?.status }; + }; + return { engine, svc, openRequest, attemptDecide, attemptRecall }; +} + +describe('[#16166] a stored `sys_user_position` row spelling an org built-in confers NO override authority', () => { + let h: ReturnType; + beforeEach(() => { h = harness(); }); + + it.each(['row-org-owner', 'row-org-admin'] as const)( + '%s — the name IS in positions[] while the rung says MEMBER (the premise, without which the rest is vacuous)', + async (shape) => { + const { context, grants, rung } = await resolve(shape); + const name = STORED_NAME[shape]!; + + expect(context.positions, JSON.stringify(context.positions)).toContain(name); + expect(grants.posture).toBe('MEMBER'); + expect(rung).toBe(false); + // No capability arm of `isOverrideActor` is satisfied either, so the NAME + // is the only thing that could admit this actor. + expect(context.permissions).not.toContain(ADMIN_FULL_ACCESS); + for (const g of ORGANIZATION_ADMIN_GRANTS) expect(context.permissions).not.toContain(g); + }, + ); + + it('the request under attack really is in the actor’s own organization — the tenant arm reaches nothing else', async () => { + const req = await h.openRequest(); + const [raw] = await h.engine.find('sys_approval_request', { where: { id: req.id } }); + expect(raw.organization_id).toBe(ORG); + expect(raw.status).toBe('pending'); + // …and the actor holds no slot in the staffed slate. + expect(String(raw.pending_approvers ?? '')).not.toContain(ACTOR); + }); + + it.each(['row-org-owner', 'row-org-admin'] as const)( + '%s — THREE-WAY AGREEMENT: the name says yes; the site gate and the rung both say no, and agree', + async (shape) => { + const { context, rung } = await resolve(shape); + const req = await h.openRequest(); + + const nameRead = (context.positions ?? []).includes(STORED_NAME[shape]!); + const outcome = await h.attemptDecide(context, req.id); + // The gate ADMITTED the actor iff a decision was recorded for them. + expect({ nameRead, gate: outcome.decided, rung }).toEqual({ nameRead: true, gate: false, rung: false }); + }, + ); + + it.each(['row-org-owner', 'row-org-admin'] as const)( + '%s — refuses to decide, and leaves the request pending', + async (shape) => { + const { context } = await resolve(shape); + const req = await h.openRequest(); + + const outcome = await h.attemptDecide(context, req.id); + + expect(outcome.threw).toBe('FORBIDDEN'); + // The state half, asserted because a refusal that still moved the request + // would be no refusal at all. + expect({ status: outcome.status, decided: outcome.decided }) + .toEqual({ status: 'pending', decided: false }); + }, + ); + + it.each(['row-org-owner', 'row-org-admin'] as const)( + '%s — refuses to RECALL someone else’s pending request', + async (shape) => { + const { context } = await resolve(shape); + const req = await h.openRequest(); + + const outcome = await h.attemptRecall(context, req.id); + + expect(outcome).toEqual({ threw: 'FORBIDDEN', status: 'pending' }); + }, + ); + + it.each(['row-org-owner', 'row-org-admin'] as const)( + '%s — sees NOTHING in the console list: a non-participant is not an override viewer', + async (shape) => { + const { context } = await resolve(shape); + await h.openRequest(); + + expect(await h.svc.listRequests({}, context)).toEqual([]); + }, + ); + + it.each(['row-org-owner', 'row-org-admin'] as const)( + '%s — answers the same as a PLAIN member: the stored row buys nothing', + async (shape) => { + const named = await resolve(shape); + const plain = await resolve('plain'); + + const a = await h.openRequest(); + const viaName = await h.attemptDecide(named.context, a.id); + const b = await h.openRequest(); + const viaPlain = await h.attemptDecide(plain.context, b.id); + + // The floor: a plain member is refused. If this arm ever stops being + // FORBIDDEN, the comparison above is measuring nothing. + expect(viaPlain).toEqual({ threw: 'FORBIDDEN', status: 'pending', decided: false }); + expect(viaName).toEqual(viaPlain); + expect(await h.svc.listRequests({}, plain.context)).toEqual([]); + }, + ); +}); + +describe('[#16166] CONTROL — a genuine org-admin capability grant still overrides', () => { + let h: ReturnType; + beforeEach(() => { h = harness(); }); + + it('resolves the TENANT_ADMIN rung from the capability, holding NO org built-in name', async () => { + const { context, grants, rung } = await resolve('genuine'); + + expect(grants.posture).toBe('TENANT_ADMIN'); + expect(rung).toBe(true); + expect(context.permissions).toContain(ORGANIZATION_ADMIN); + // The separation the fix rests on: authority here is the GRANT, and this + // principal carries neither built-in NAME. + expect(context.positions).not.toContain(BUILTIN_IDENTITY_ORG_OWNER); + expect(context.positions).not.toContain(BUILTIN_IDENTITY_ORG_ADMIN); + }); + + it('decides a request it holds no slot on — the #3424 stuck-request escape hatch survives', async () => { + const { context, rung } = await resolve('genuine'); + const req = await h.openRequest(); + + const outcome = await h.attemptDecide(context, req.id); + expect({ gate: outcome.decided, rung }).toEqual({ gate: true, rung: true }); + expect(outcome.status).toBe('approved'); + + // …and it is still recorded AS an override (#4466), not as an ordinary + // approval — the audit half the escape hatch is allowed to keep. + const decision = (await h.svc.listActions(req.id, SYS)).at(-1)!; + expect(decision).toMatchObject({ action: 'approve', via_override: true }); + }); + + it('recalls, and sees the request in the console list — the other two doors of the same hatch', async () => { + const { context } = await resolve('genuine'); + const req = await h.openRequest(); + + expect((await h.svc.listRequests({}, context)).map((r) => r.id)).toContain(req.id); + expect(await h.attemptRecall(context, req.id)).toEqual({ threw: null, status: 'recalled' }); + }); +}); From d40b9afd5d82e99c63398c4c473f90953f059bc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 03:39:32 +0000 Subject: [PATCH 2/3] fix(approvals): the tenant override arm reads the ADR-0095 rung, never a position NAME MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isOverrideActor`'s tenant arm ORed `positions.includes(org_owner)` and `positions.includes(org_admin)` onto the capability rung. `positions[]` carries ADR-0057 D4 assignment values alongside the ADR-0068 D2 projection, so a stored row spelling either name conferred tenant-admin override authority on approvals inside that organization — measured on three doors with the harness committed in the previous commit. The tenant rung is established from the resolver's own source rather than copied from the platform side: ADR-0095 D3's `derivePosture` resolves `TENANT_ADMIN` from `ORGANIZATION_ADMIN_GRANTS` and from nothing else, and the spec declares those grants that rung's source of truth — so the capability arm IS an authority read, and only the two name arms go. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude --- .../plugin-approvals/src/approval-service.ts | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 9529b312d3a..bbae6c8f215 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/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. From 783180eba89e942b9103830303c2475d48e5593e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 03:54:55 +0000 Subject: [PATCH 3/3] chore(approvals): changeset + engine-double ledger row for the tenant-arm pin The new pin declares its own `update`/`delete` doubles through the engine's dispatch predicates, so `check:engine-double-contract` needs the ledger row or the file is never protected. Additive only: 2 rows added, 0 lost. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude --- .changeset/16166-override-actor-tenant-arm-rung.md | 11 +++++++++++ scripts/engine-double-contract.pinned.json | 10 ++++++++++ 2 files changed, 21 insertions(+) create mode 100644 .changeset/16166-override-actor-tenant-arm-rung.md diff --git a/.changeset/16166-override-actor-tenant-arm-rung.md b/.changeset/16166-override-actor-tenant-arm-rung.md new file mode 100644 index 00000000000..b974fdfd1d7 --- /dev/null +++ b/.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. diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 49e035716db..02d21027f58 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2226,6 +2226,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-approvals/src/approval-tenant-positions-name-authority.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-tenant-positions-name-authority.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts", "verb": "update",