From 55b1a28ce72db6803b738e24ba349a506cd6a898 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 01:59:53 +0000 Subject: [PATCH 1/2] fix(sharing,approvals,runtime,security): platform authority is the posture RUNG, never a name in positions[] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four server-side readers derived platform-operator authority from a NAME in `ExecutionContext.positions` rather than from the ADR-0095 capability rung. `positions[]` is the security axis, so it carries ADR-0057 D4 `sys_user_position` names alongside the built-ins. That table is `apiEnabled` with unconstrained `position` values, so a tenant could mint a row spelling `platform_admin` for one of their own users: `resolveUserAuthzGrants` pushes the name straight onto `grants.positions`, while `grants.posture` is derived from the unscoped `admin_full_access` grant and nothing else and correctly stays `MEMBER`. Every reader of the name therefore answered true for a principal enforcement treats as an ordinary member. `resolve-authz-context.ts` states the rule at `hasPlatformAdminStanding` -- "read the RUNG, never positions.includes(...)" -- but a comment is not a gate, and it stopped none of these four. Each now tests `posture === 'PLATFORM_ADMIN'`, byte-for-byte what that predicate returns. Each site was DRIVEN rather than read, with the D4 row present, before and after. Three were live; the fourth is reported as what it measured, not as what the shape suggested: - plugin-sharing `hasPlatformAuthority` -- the minted row satisfied `assertResolvableAdminScope`, so an org-less caller holding only the ORG-scoped `manage_sharing` capability was answered with every tenant's sharing rules, and could delete platform-global rows. - plugin-approvals `isOverrideActor` -- already read the rung and then ORed the name onto it, which is no protection. Because that arm crosses the tenant wall, the minted row moved another organization's pending request to `approved`. - runtime ADR-0126 §5 activation gate -- under a walled posture this is the only thing between a tenant org admin and the install-wide activation row, so the minted row reopened #10243 with a durable row behind it. - plugin-security `derivePosture` -- NARROWER, and measured: the name-read sits behind an early `ctx.posture` return that `buildContextForUser` always populates, so the shipping path was already gated and the D4 row never moved it. What the read did reach was a posture-less hand-built context, where it made the panel REPORT `PLATFORM_ADMIN` for a MEMBER. Each site gets a mutated three-way-agreement pin whose population is stated in its own header, and each contains the case whose absence let #15948 round 1 pass its author's own check: a D4 row that spells the built-in name, with the name-read, the site's own gate and the rung asserted together. Six existing fixtures pinned the removed branches and are migrated, not deleted: four spelled a platform operator as the bare NAME (they now carry the rung a resolved operator always has, plus a new name-only refusal arm), and two in `explain-engine.test.ts` asserted the removed derivation directly and are inverted with the reason recorded in place. Refs #15981 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- ...ositions-name-is-not-platform-authority.md | 21 + content/docs/permissions/system-context.mdx | 6 +- .../approval-positions-name-authority.test.ts | 404 ++++++++++++++++++ .../plugin-approvals/src/approval-service.ts | 23 +- .../src/explain-engine.test.ts | 40 +- .../plugin-security/src/explain-engine.ts | 33 +- .../explain-positions-name-authority.test.ts | 233 ++++++++++ ...ring-rule-positions-name-authority.test.ts | 288 +++++++++++++ .../src/sharing-rule-service.ts | 31 +- .../plugin-sharing/src/sharing-rule.test.ts | 65 ++- .../action-activation-posture-gate.test.ts | 38 ++ ...tion-gate-positions-name-authority.test.ts | 254 +++++++++++ .../runtime/src/domains/activation-gate.ts | 54 ++- ...automation-activation-posture-gate.test.ts | 37 ++ scripts/engine-double-contract.pinned.json | 10 + 15 files changed, 1485 insertions(+), 52 deletions(-) create mode 100644 .changeset/positions-name-is-not-platform-authority.md create mode 100644 packages/plugins/plugin-approvals/src/approval-positions-name-authority.test.ts create mode 100644 packages/plugins/plugin-security/src/explain-positions-name-authority.test.ts create mode 100644 packages/plugins/plugin-sharing/src/sharing-rule-positions-name-authority.test.ts create mode 100644 packages/runtime/src/domains/activation-gate-positions-name-authority.test.ts diff --git a/.changeset/positions-name-is-not-platform-authority.md b/.changeset/positions-name-is-not-platform-authority.md new file mode 100644 index 0000000000..76c63b3a8f --- /dev/null +++ b/.changeset/positions-name-is-not-platform-authority.md @@ -0,0 +1,21 @@ +--- +"@objectstack/plugin-sharing": patch +"@objectstack/plugin-approvals": patch +"@objectstack/plugin-security": patch +"@objectstack/runtime": patch +--- + +Four server-side authorization sites stop deriving platform-operator authority from a NAME in `ExecutionContext.positions`, and read the ADR-0095 posture rung instead. + +`positions[]` is the security axis, so it carries ADR-0057 D4 `sys_user_position` names alongside the built-ins. `sys_user_position` is `apiEnabled` and its `position` values are unconstrained, so a tenant could mint a row spelling `platform_admin` for one of their own users: `resolveUserAuthzGrants` pushed that name straight onto `grants.positions`, while `grants.posture` — derived from the unscoped `admin_full_access` grant and nothing else — correctly stayed `MEMBER`. Every reader of the name therefore answered `true` for a principal enforcement treats as an ordinary member. `resolve-authz-context.ts` states the rule at `hasPlatformAdminStanding` ("read the RUNG — never `positions.includes(...)`"), but a comment is not a gate and these four had not followed it. + +Each site now tests `posture === 'PLATFORM_ADMIN'`, byte-for-byte what `hasPlatformAdminStanding` returns: + +- **`plugin-sharing`** — `hasPlatformAuthority`. The minted row satisfied `assertResolvableAdminScope`, so an org-less caller holding only the ORG-scoped `manage_sharing` capability was answered with **every tenant's** sharing rules, and could delete platform-global rules. The `manage_platform_settings` capability spelling is unchanged. +- **`plugin-approvals`** — `isOverrideActor`. This predicate already read the rung and then ORed the name onto it, which is no protection: an OR is only as strong as its weakest arm. Because the platform arm deliberately crosses the tenant wall, the minted row let a member of one organization approve, reject or recall a **different organization's** pending request while holding no slot in its slate. The `ADMIN_FULL_ACCESS` capability arm and both TENANT_ADMIN arms are unchanged. +- **`runtime`** — the ADR-0126 §5 activation gate. Under a `group` or `isolated` posture this gate is the only thing between a tenant org admin and the **install-wide** `sys_metadata_activation` row, so the minted row reopened #10243 with a durable row behind it. +- **`plugin-security`** — `derivePosture` in the explain engine. Narrower than the other three, and stated precisely rather than overclaimed: the name-read sat behind an early `ctx.posture` return that `buildContextForUser` always populates, so the shipping path was already gated and a D4 row never moved it. What the read did reach was a posture-less hand-built context, where it made the panel **report** `PLATFORM_ADMIN` for a principal enforcement treats as a MEMBER — a misreport rather than an admission, but in the one tool an administrator opens to check exactly this. + +No behaviour changes for a genuine platform operator: their resolved context carries the rung, and the built-in position is still projected onto `positions[]` for display and predicate use. What changes is that the name alone no longer answers the authorization question. + +Graded `patch` on the surface it moves: no exported type, signature or contract changes, and no authorable metadata is added, removed or renamed. The only observable difference is that a principal who never held the capability grant stops being admitted — which is the defect, not a feature anyone could have depended on. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8480433783..d72a894c8d 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -137,7 +137,7 @@ The largest single consumer — **17 of the 105 sites**. | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1189` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:459`, `:513`, `:517`, `:590`, `:620` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:278`, `:503` | +| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:279`, `:518` | ### 4. Approvals, reports, attachments, comments, knowledge @@ -145,7 +145,7 @@ The largest single consumer — **17 of the 105 sites**. |:--|:---|:---|:---|:---| | 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` | | 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` | -| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:1093`, `:1202`, `:3458`, `:3606`, `:3774`, `:3845`, `:4034`, `:4074` | +| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:1096`, `:1219`, `:3475`, `:3623`, `:3791`, `:3862`, `:4051`, `:4091` | | 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` | | 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` | | 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` | @@ -164,7 +164,7 @@ The largest single consumer — **17 of the 105 sites**. | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:241`, `:274` | -| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:138`, `:189` | +| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:157`, `:211` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | | 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | | 59 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `email-template-provenance.ts:59`, `webhook-provenance.ts:50` | diff --git a/packages/plugins/plugin-approvals/src/approval-positions-name-authority.test.ts b/packages/plugins/plugin-approvals/src/approval-positions-name-authority.test.ts new file mode 100644 index 0000000000..b75bdbf35b --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-positions-name-authority.test.ts @@ -0,0 +1,404 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15981] A `sys_user_position` row SPELLING `platform_admin` confers no + * override authority on the approvals surface. + * + * ## The defect this pins + * + * `isOverrideActor` read the capability rung FIRST and then ORed a NAME onto + * it: + * + * const isPlatformAdmin = posture === 'PLATFORM_ADMIN' + * || perms.includes(ADMIN_FULL_ACCESS) + * || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); // ← the hole + * + * Reading the rung first does not close the hole; an OR is only as strong as + * its weakest arm. `sys_user_position` is `apiEnabled` and its `position` + * values are unconstrained, so a tenant can mint an ADR-0057 D4 row spelling + * that built-in name, `resolveUserAuthzGrants` §4 pushes it into + * `grants.positions`, and the third arm answered `true` while `grants.posture` + * — derived from the unscoped `admin_full_access` grant and nothing else — + * stayed `MEMBER`. + * + * ⭐ WHAT THE ESCALATION BUYS, driven rather than argued: the platform arm of + * `isOverrideActor` deliberately CROSSES THE TENANT WALL (the tenant-admin arm + * below it is confined to the actor's own org). So a member of one tenant + * holding the minted row could decide a PENDING request belonging to a + * DIFFERENT tenant, holding no slot in its slate — see the cross-tenant arm. + * + * ## POPULATION OF THIS PIN — stated because a pin proves only what it covers + * + * Covers: `decideNode` (the approve/reject door) and `listRequests` + * (the console's participant-visibility read, `visibleRequestIds`), for a + * NON-slate-holding actor in a DIFFERENT organization from the request, in + * three shapes — a D4 row spelling the built-in name with no capability grant + * (`name-only`), a genuine unscoped `admin_full_access` grant (`genuine`), and + * a plain member (`plain`, the floor that proves the arms are not all passing + * for some unrelated reason). The `name-only` and `genuine` contexts are built + * by inserting rows and resolving them through the REAL + * `resolveUserAuthzGrants`. + * + * Does NOT cover: the `ADMIN_FULL_ACCESS` permission-name arm or the + * TENANT_ADMIN arms of `isOverrideActor` (untouched by this change; the + * `org_owner` / `org_admin` name-reads on the tenant arm are a NARROWER + * question left to #15972's write-side card and not silently widened here), + * the SLA/escalation paths, or the ADR-0091 validity window. Those are other + * suites' populations, and their passing is NOT evidence about this one. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { hasPlatformAdminStanding, 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-override-audit.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 }; }, + // `update` / `delete` route through the ENGINE'S OWN dispatch predicates + // rather than a hand-mirrored copy: a double looser than the engine it + // stands in for is how #4434 shipped a dead REST route with its suite + // green, and `check:engine-double-contract` grades exactly this. + 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() {}, + }; +} + +const VICTIM_ORG = 't_victim'; +const ATTACKER_ORG = 't_attacker'; +const ATTACKER = 'usr_attacker'; +const PS_ADMIN = 'ps_admin_full_access'; + +/** + * 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 = 'name-only' | 'genuine' | 'plain'; + +function authzTables(shape: Shape) { + return { + sys_user: [{ id: ATTACKER, email: 'attacker@example.com', email_verified: true }], + sys_member: [{ organization_id: ATTACKER_ORG, user_id: ATTACKER, role: 'member' }], + sys_user_position: + shape === 'name-only' + ? [ + // Exactly what a tenant admin can write through the `apiEnabled` + // `sys_user_position` surface: a row whose NAME is the built-in. + { user_id: ATTACKER, position: BUILTIN_IDENTITY_PLATFORM_ADMIN, organization_id: null }, + ] + : [], + // An ACTIVE catalogue row, so ADR-0049's deactivated-position filter is not + // what carries the arm. + sys_position: + shape === 'name-only' + ? [{ id: 'pos_pa', name: BUILTIN_IDENTITY_PLATFORM_ADMIN, label: 'Platform Admin', active: true }] + : [], + sys_position_permission_set: [], + sys_user_permission_set: + shape === 'genuine' + ? [{ user_id: ATTACKER, permission_set_id: PS_ADMIN, organization_id: null }] + : [], + sys_permission_set: [{ id: PS_ADMIN, name: ADMIN_FULL_ACCESS, active: true }], + }; +} + +/** + * Resolve one principal through the REAL resolver, then build the context a + * transport would. `tenantId` is the ATTACKER's own organization — the request + * under test belongs to a different one, which is what makes the platform arm + * (and only the platform arm) able to reach it. + */ +async function resolve(shape: Shape) { + const ql = makeAuthzQl(authzTables(shape)); + const grants = await resolveUserAuthzGrants(ql as any, ATTACKER, { tenantId: ATTACKER_ORG }); + const context = { + userId: ATTACKER, + tenantId: ATTACKER_ORG, + positions: grants.positions, + permissions: grants.permissions, + systemPermissions: grants.systemPermissions, + ...(grants.posture ? { posture: grants.posture } : {}), + } as ExecutionContext; + return { context, grants, rung: await hasPlatformAdminStanding(ql as any, ATTACKER) }; +} + +/** + * Narrow `openNodeRequest`'s union, and REFUSE the auto-approval outcome. + * Same spelling as `business-unit-member-org-screen.test.ts` (#10230). It is + * load-bearing here: 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 victim tenant's submitter — the request being reached across the wall. */ +const SUBMITTER = { userId: 'usr_victim', tenantId: VICTIM_ORG, positions: [], permissions: [] } as any; + +describe('[#15981] a D4 `sys_user_position` row spelling `platform_admin` confers NO override authority', () => { + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + const baseTime = new Date('2026-01-15T10:00:00Z').getTime(); + + beforeEach(() => { + engine = makeFakeEngine(); + n = 0; + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(baseTime + (n++) * 1000) }, + }); + }); + + /** + * A request in the VICTIM tenant, with a staffed slate the attacker is not on. + * + * Each call takes its OWN record: `openNodeRequest` refuses a second pending + * request on the same one (`DUPLICATE_REQUEST`), and an arm that leaves the + * first request correctly PENDING would otherwise collide with itself. + */ + let seq = 0; + const openVictimRequest = () => { + const n = ++seq; + return svc.openNodeRequest( + { + object: 'opportunity', recordId: `opp${n}`, runId: `run_${n}`, nodeId: 'sign_off', + flowName: 'victim_flow', + config: { + approvers: [{ type: 'user' as const, value: 'usr_designated' }], + behavior: 'first_response' as const, + }, + record: { id: `opp${n}`, amount: 100 }, + } as any, + SUBMITTER, + ).then(opened); + }; + + it('the name IS in positions[] while the rung says MEMBER — the premise, without which the rest is vacuous', async () => { + const { context, grants, rung } = await resolve('name-only'); + + expect(context.positions, JSON.stringify(context.positions)).toContain( + BUILTIN_IDENTITY_PLATFORM_ADMIN, + ); + expect(grants.posture).not.toBe('PLATFORM_ADMIN'); + 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); + }); + + /** + * Attempt a decision and report what it DID, not merely what it threw. + * + * ⭐ The thrown message is NOT a sound gate signal here, and reading it as one + * would have hidden the escalation. A cross-tenant decision that is ADMITTED + * writes the decision and then fails building its echo, throwing + * `READ_BACK_FAILED` — whose own message says "The write is NOT rolled back". + * A pin that asserted `rejects.toThrow()` would therefore have gone GREEN on + * a successful escalation. So the observable is STATE: did the request leave + * `pending`, and was a decision action recorded? + */ + const attemptDecide = async (ctx: ExecutionContext, id: string) => { + let threw: string | null = null; + try { + await svc.decideNode(id, { decision: 'approve', actorId: ATTACKER }, 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 === ATTACKER); + return { threw, status: row?.status, decided: actions.length > 0 }; + }; + + it('THREE-WAY AGREEMENT — the name says yes; the site gate and the rung both say no, and agree', async () => { + const { context, rung } = await resolve('name-only'); + const req = await openVictimRequest(); + + const nameRead = (context.positions ?? []).includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + const outcome = await attemptDecide(context, req.id); + // The gate ADMITTED the actor iff a decision was recorded for them. + const gate = outcome.decided; + + expect({ nameRead, gate, rung }).toEqual({ nameRead: true, gate: false, rung: false }); + }); + + it('refuses to decide ANOTHER tenant’s pending request, and leaves it pending', async () => { + const { context } = await resolve('name-only'); + const req = await openVictimRequest(); + + const outcome = await 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 — and because `READ_BACK_FAILED` is exactly + // what an ADMITTED cross-tenant write throws. + expect({ status: outcome.status, decided: outcome.decided }).toEqual({ + status: 'pending', + decided: false, + }); + }); + + it('answers the same as a PLAIN member — the minted row buys nothing', async () => { + const nameOnly = await resolve('name-only'); + const plain = await resolve('plain'); + + const a = await openVictimRequest(); + const viaName = await attemptDecide(nameOnly.context, a.id); + const b = await openVictimRequest(); + const viaPlain = await attemptDecide(plain.context, b.id); + + // The floor: a plain member of another tenant 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); + }); +}); + +describe('[#15981] CONTROL — a genuine unscoped admin_full_access grant still overrides', () => { + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + const baseTime = new Date('2026-01-15T10:00:00Z').getTime(); + + beforeEach(() => { + engine = makeFakeEngine(); + n = 0; + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(baseTime + (n++) * 1000) }, + }); + }); + + /** + * A request in the VICTIM tenant, with a staffed slate the attacker is not on. + * + * Each call takes its OWN record: `openNodeRequest` refuses a second pending + * request on the same one (`DUPLICATE_REQUEST`), and an arm that leaves the + * first request correctly PENDING would otherwise collide with itself. + */ + let seq = 0; + const openVictimRequest = () => { + const n = ++seq; + return svc.openNodeRequest( + { + object: 'opportunity', recordId: `opp${n}`, runId: `run_${n}`, nodeId: 'sign_off', + flowName: 'victim_flow', + config: { + approvers: [{ type: 'user' as const, value: 'usr_designated' }], + behavior: 'first_response' as const, + }, + record: { id: `opp${n}`, amount: 100 }, + } as any, + SUBMITTER, + ).then(opened); + }; + + it('all three answers are TRUE and agree — the stuck-request escape hatch survives', async () => { + const { context, rung } = await resolve('genuine'); + const req = await openVictimRequest(); + + const nameRead = (context.positions ?? []).includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + // ⚠️ The echo cannot be built across the tenancy wall, so the call throws + // `READ_BACK_FAILED` *after* recording the decision — pre-existing + // behaviour of `readBackRequest`, untouched here and deliberately NOT + // caught into a `null`. The decision itself is the observable. + await expect( + svc.decideNode(req.id, { decision: 'approve', actorId: ATTACKER }, context), + ).rejects.toThrow(/READ_BACK_FAILED/); + + const decision = (await svc.listActions(req.id, SYS)).at(-1)!; + const gate = decision?.actor_id === ATTACKER; + expect({ nameRead, gate, rung }).toEqual({ nameRead: true, gate: true, rung: true }); + // …and it is still recorded AS an override (#4466), not as an ordinary + // approval — the audit half the escape hatch is allowed to keep. + expect(decision).toMatchObject({ action: 'approve', via_override: true }); + }); + + it('the two shapes are INDISTINGUISHABLE by name and separable only by the rung', async () => { + const escalation = await resolve('name-only'); + const genuine = await resolve('genuine'); + + expect(escalation.context.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(genuine.context.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(escalation.rung).toBe(false); + expect(genuine.rung).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index e83aa47a3b..81d7226ff4 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -19,10 +19,13 @@ 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. import { ADMIN_FULL_ACCESS, ORGANIZATION_ADMIN_GRANTS, - BUILTIN_IDENTITY_PLATFORM_ADMIN, BUILTIN_IDENTITY_ORG_OWNER, BUILTIN_IDENTITY_ORG_ADMIN, } from '@objectstack/spec/identity'; @@ -1100,10 +1103,24 @@ export class ApprovalService implements IApprovalService { // unchecked `as any` on an enforcement input: a typo (`postures`, // `'PLATFORM-ADMIN'`) would have compiled and silently denied every // override, leaving a stuck approval with no in-product recovery. + // + // ⛔ [#15981] The rung and the capability name are the whole platform test: + // there is NO `positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)` arm any + // more, and reading the rung FIRST never made one safe — an OR is only as + // strong as its weakest arm. `positions[]` is the security axis and carries + // ADR-0057 D4 `sys_user_position` names; that table is `apiEnabled` with + // unconstrained `position` values, so a tenant could mint a row spelling + // that built-in and `resolveUserAuthzGrants` §4 would push it onto the + // array. `resolve-authz-context.ts` states the rule at + // `hasPlatformAdminStanding`, whose answer IS `posture === 'PLATFORM_ADMIN'`. + // + // This arm is the one that CROSSES THE TENANT WALL (the tenant-admin arm + // below is confined to the actor's own org), so the minted row bought a + // member of one tenant the power to decide another tenant's pending + // request. Driven in `approval-positions-name-authority.test.ts`. const posture = context.posture; const isPlatformAdmin = posture === 'PLATFORM_ADMIN' - || perms.includes(ADMIN_FULL_ACCESS) - || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + || perms.includes(ADMIN_FULL_ACCESS); if (isPlatformAdmin) return true; const isTenantAdmin = posture === 'TENANT_ADMIN' || ORGANIZATION_ADMIN_GRANTS.some((n) => perms.includes(n)) diff --git a/packages/plugins/plugin-security/src/explain-engine.test.ts b/packages/plugins/plugin-security/src/explain-engine.test.ts index d87944bc1f..82f1aa2d89 100644 --- a/packages/plugins/plugin-security/src/explain-engine.test.ts +++ b/packages/plugins/plugin-security/src/explain-engine.test.ts @@ -442,14 +442,27 @@ describe('explainAccess — record-grained (C2 / ADR-0095)', () => { expect(d.record).toMatchObject({ recordId: 'r1', visible: true }); }); - it('derives PLATFORM_ADMIN posture from the platform_admin position', async () => { + it('derives PLATFORM_ADMIN posture from the unscoped-grant flag', async () => { + // [#15981] Was "from the platform_admin position". The NAME is no longer + // that evidence: `positions[]` is the security axis and carries ADR-0057 D4 + // `sys_user_position` names from an `apiEnabled` table, so a tenant can mint + // one spelling the built-in. `hasPlatformAdminGrant` is what + // `buildContextForUser` sets from `grants.posture === 'PLATFORM_ADMIN'`. const d = await explainAccess( recDeps({ sets: [ADMIN], layered: { layer0: null, layer1: null } }), - { object: 'leave_request', operation: 'read', context: { userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] }, recordId: 'r1' }, + { object: 'leave_request', operation: 'read', context: { userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [], hasPlatformAdminGrant: true }, recordId: 'r1' }, ); expect(d.principal.posture).toBe('PLATFORM_ADMIN'); }); + it('[#15981] the platform_admin NAME alone does not — it reports MEMBER', async () => { + const d = await explainAccess( + recDeps({ sets: [ADMIN], layered: { layer0: null, layer1: null } }), + { object: 'leave_request', operation: 'read', context: { userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] }, recordId: 'r1' }, + ); + expect(d.principal.posture).toBe('MEMBER'); + }); + it('Layer 0 (the tenant wall) excludes a cross-org record — decidedBy tenant_isolation', async () => { const d = await explainAccess( recDeps({ layered: { layer0: { organization_id: 'org1' }, layer1: null }, record: { id: 'r1', organization_id: 'org2', owner_id: 'u1' } }), @@ -624,8 +637,27 @@ describe('posture derivation aligns with enforcement (label-drift elimination)', expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'], hasPlatformAdminGrant: true })).toBe('PLATFORM_ADMIN'); }); - it('the projected platform_admin built-in position still yields PLATFORM_ADMIN', async () => { - expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] })).toBe('PLATFORM_ADMIN'); + it('[#15981] the platform_admin built-in position NAME no longer yields PLATFORM_ADMIN', async () => { + // This assertion is INVERTED from what it pinned before, deliberately. It + // used to read "the PROJECTED platform_admin built-in position still yields + // PLATFORM_ADMIN", whose premise was that the name is only ever projected + // from the unscoped `admin_full_access` grant. Ruling A made `positions[]` + // the security axis, so it now also carries ADR-0057 D4 + // `sys_user_position` names — and that table is `apiEnabled` with + // unconstrained values, so the name can be MINTED by a tenant. The genuine + // projection is not lost: a real operator's context carries + // `hasPlatformAdminGrant` (the case immediately above) and, off + // `buildContextForUser`, `posture` itself. + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] })).toBe('MEMBER'); + }); + + it('[#15981] …and the two are separable only by the rung, not by the array', async () => { + const minted = { userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] }; + const genuine = { ...minted, hasPlatformAdminGrant: true }; + // Identical on the axis a name-reading predicate would consult … + expect(minted.positions).toEqual(genuine.positions); + // … and opposite on the axis that actually decides. + expect([await postureOf(minted), await postureOf(genuine)]).toEqual(['MEMBER', 'PLATFORM_ADMIN']); }); it('org_owner / org_admin better-auth role positions no longer confer TENANT_ADMIN (ADR-0095 D3)', async () => { diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index e9ea653f2b..aa05bdae78 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -27,7 +27,7 @@ import { resolveUserAuthzGrants, } from '@objectstack/core'; import { matchesFilterCondition } from '@objectstack/formula'; -import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec'; +import { ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec'; import type { FieldMaskingRule } from '@objectstack/spec/data'; import type { PermissionSet } from '@objectstack/spec/security'; import type { @@ -96,12 +96,27 @@ function isAuthzPosture(v: unknown): v is AuthzPosture { * `resolveAuthzContext` uses — NOT the previous loose permission-set-NAME * match: * - `PLATFORM_ADMIN` ← the **unscoped `admin_full_access` USER grant** - * (`hasPlatformAdminGrant`, which `buildContextForUser` now READS OFF the - * resolver's own verdict rather than recomputing), OR the - * `platform_admin` built-in position (which is itself only ever - * PROJECTED from that same grant — ADR-0068 D2). A merely-SCOPED - * `admin_full_access` grant (name present in `permissions`, not held - * unscoped) no longer over-labels. + * (`hasPlatformAdminGrant`, which `buildContextForUser` READS OFF the + * resolver's own verdict rather than recomputing), and nothing else. A + * merely-SCOPED `admin_full_access` grant (name present in + * `permissions`, not held unscoped) does not over-label. + * + * ⛔ [#15981] It is NOT ALSO the `platform_admin` built-in POSITION. + * That arm rested on the position being "only ever PROJECTED from that + * same grant", which stopped being true when `positions[]` became the + * security axis: it now carries ADR-0057 D4 `sys_user_position` names, + * and that table is `apiEnabled` with unconstrained `position` values, + * so a tenant could mint a row spelling the built-in. ⚠️ Reachability + * here was NARROWER than at #15981's three sibling sites, and it is + * worth stating precisely rather than overclaiming: preference (2) + * above returns first for any principal `buildContextForUser` builds, + * so the SHIPPING path never reached this line and a D4 row never moved + * it. What the name-read did reach was a posture-less HAND-BUILT + * context — and there it made the panel REPORT `PLATFORM_ADMIN` for a + * principal enforcement treats as a MEMBER. That is a misreport, not an + * admission — this engine explains, it does not admit — but it is a + * misreport in the tool an administrator opens to check exactly this, + * so it answered reassuringly wrong at the worst moment. * - `TENANT_ADMIN` ← the `organization_admin` **capability** grant, exactly * like enforcement (ADR-0095 D3). The better-auth `org_owner`/`org_admin` * role positions are a provisioning source only and are no longer read @@ -110,11 +125,9 @@ function isAuthzPosture(v: unknown): v is AuthzPosture { function derivePosture(context: any): AuthzPosture { if (!context?.userId || context?.principalKind === 'guest') return 'EXTERNAL'; if (isAuthzPosture(context?.posture)) return context.posture; - const positions: string[] = Array.isArray(context?.positions) ? context.positions : []; const permissions: string[] = Array.isArray(context?.permissions) ? context.permissions : []; return deriveAdminPosture({ - isPlatformAdmin: - context?.hasPlatformAdminGrant === true || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN), + isPlatformAdmin: context?.hasPlatformAdminGrant === true, isTenantAdmin: ORGANIZATION_ADMIN_GRANTS.some((n) => permissions.includes(n)), }); } diff --git a/packages/plugins/plugin-security/src/explain-positions-name-authority.test.ts b/packages/plugins/plugin-security/src/explain-positions-name-authority.test.ts new file mode 100644 index 0000000000..d86b686c69 --- /dev/null +++ b/packages/plugins/plugin-security/src/explain-positions-name-authority.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15981] A `sys_user_position` row SPELLING `platform_admin` does not make the + * explain panel REPORT `PLATFORM_ADMIN`. + * + * ## The defect this pins, and how it differs from its three siblings + * + * `derivePosture` read a NAME as platform evidence: + * + * isPlatformAdmin: + * context?.hasPlatformAdminGrant === true || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN), + * + * ⚠️ MEASURED, and the reason this site's arms are shaped differently from the + * other three in #15981: this read sits BEHIND an early return — + * `if (isAuthzPosture(context?.posture)) return context.posture;` — and + * `buildContextForUser`, the explain API's own principal builder, always + * attaches `posture` for an authenticated principal. So on the SHIPPING path + * the name-read was already unreachable, and the D4 row changed nothing. That + * half is asserted below (`the shipping path was already gated`) and it was + * GREEN before this change as well as after: it is a regression guard, not + * evidence that anything was repaired here. + * + * What WAS reachable is the fallback branch itself, on a context carrying + * `positions` with no `posture` — the shape the doc block names ("a HAND-BUILT + * context: tests, an internal caller assembling `{ userId, positions, + * permissions }` itself"). There the name alone produced `PLATFORM_ADMIN`. + * That is a MISREPORT rather than an enforcement bypass — this engine explains, + * it does not admit — but it is a misreport in the one tool an administrator + * uses to check whether someone is a platform operator, so it answers + * "reassuringly wrong" exactly when someone is looking. + * + * The fallback's remaining evidence is `hasPlatformAdminGrant`, which + * `buildContextForUser` sets from `grants.posture === 'PLATFORM_ADMIN'` — the + * rung, byte-for-byte what `hasPlatformAdminStanding` returns. + * + * ## POPULATION OF THIS PIN — stated because a pin proves only what it covers + * + * Covers: `explainAccess`'s reported `principal.posture` for (a) contexts built + * by the REAL `buildContextForUser` over inserted rows, in the `name-only` and + * `genuine` shapes, and (b) hand-built contexts exercising the fallback branch + * directly — the name alone, the rung alone, and both together. + * + * Does NOT cover: the TENANT_ADMIN arm, the guest/EXTERNAL floor, or the + * record-grained layer attribution — those are `explain-engine.test.ts`'s + * population, and its passing is NOT evidence about this one. In particular it + * does not cover any ENFORCEMENT decision: no arm here admits or refuses + * anything, because this engine reports. + */ + +import { describe, it, expect } from 'vitest'; +import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { hasPlatformAdminStanding } from '@objectstack/core'; +import { PermissionEvaluator } from './permission-evaluator'; +import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine'; + +const ADMIN = PermissionSetSchema.parse({ + name: 'admin_full_access', + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true } }, + systemPermissions: ['manage_users'], +}); +const PRIVATE_SCHEMA = { name: 'leave_request', sharingModel: 'private' }; + +/** The base object-level deps — the posture value is independent of the record-grained ones. */ +function makeDeps(): ExplainEngineDeps { + return { + ql: { getSchema: () => PRIVATE_SCHEMA }, + resolveSets: async () => [ADMIN], + evaluator: new PermissionEvaluator(), + getObjectSecurityMeta: async () => ({ + isPrivate: false, + requiredPermissions: { all: [], read: [], create: [], update: [], delete: [] }, + fieldRequiredPermissions: {}, + }), + requiredCaps: (meta: any, op: string) => { + const bucket = op === 'find' ? 'read' : op === 'insert' ? 'create' : op; + return [...(meta.all ?? []), ...((meta as any)[bucket] ?? [])]; + }, + computeRlsFilter: async () => null, + getFieldMask: () => ({}), + getPartialMaskRules: async () => ({}), + baselinePermissionSets: ['member_default'], + }; +} + +/** `posture` is surfaced whenever a `recordId` is supplied. */ +const postureOf = async (context: any): Promise => { + const d = await explainAccess(makeDeps(), { + object: 'leave_request', operation: 'read', context, recordId: 'r1', + }); + return d.principal.posture; +}; + +const USER = 'usr_subject'; +const HOME_ORG = 'org_home'; +const PS_ADMIN = 'ps_admin_full_access'; + +/** + * A `where`-HONOURING ObjectQL double — load-bearing, not tidiness, for the + * reason `explain-engine.test.ts` states at its own `buildContextForUser` + * fixtures: the resolver delegates filtering to the engine, so a fake that + * ignored `where` would report grants nobody holds. Top-level `$` combinators + * are refused rather than approximated. + */ +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 { + getSchema: () => PRIVATE_SCHEMA, + 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; + }, + }; +} + +function authzTables(shape: 'name-only' | 'genuine') { + return { + sys_user: [{ id: USER, email: 'subject@example.com', email_verified: true }], + sys_member: [{ organization_id: HOME_ORG, user_id: USER, role: 'member' }], + sys_user_position: + shape === 'name-only' + ? [{ user_id: USER, position: BUILTIN_IDENTITY_PLATFORM_ADMIN, organization_id: null }] + : [], + sys_position: + shape === 'name-only' + ? [{ id: 'pos_pa', name: BUILTIN_IDENTITY_PLATFORM_ADMIN, label: 'Platform Admin', active: true }] + : [], + sys_position_permission_set: [], + sys_user_permission_set: + shape === 'genuine' + ? [{ user_id: USER, permission_set_id: PS_ADMIN, organization_id: null }] + : [], + sys_permission_set: [{ id: PS_ADMIN, name: ADMIN_FULL_ACCESS, active: true }], + }; +} + +describe('[#15981] the SHIPPING explain path was already gated — a regression guard, not a repair', () => { + it('a D4 row spelling the built-in name is REPORTED as MEMBER, and the rung agrees', async () => { + const ql = makeAuthzQl(authzTables('name-only')); + const context = await buildContextForUser(ql as any, USER); + + // The premise: the name really is on the array the panel prints. + expect(context.positions, JSON.stringify(context.positions)).toContain( + BUILTIN_IDENTITY_PLATFORM_ADMIN, + ); + // …and the reported rung is the capability answer, not the name. + expect(context.posture).toBe('MEMBER'); + expect(context.hasPlatformAdminGrant).toBe(false); + expect(await hasPlatformAdminStanding(ql as any, USER)).toBe(false); + expect(await postureOf(context)).toBe('MEMBER'); + }); + + it('CONTROL — a genuine unscoped admin_full_access grant is still reported as PLATFORM_ADMIN', async () => { + const ql = makeAuthzQl(authzTables('genuine')); + const context = await buildContextForUser(ql as any, USER); + + expect(context.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(context.hasPlatformAdminGrant).toBe(true); + expect(await hasPlatformAdminStanding(ql as any, USER)).toBe(true); + expect(await postureOf(context)).toBe('PLATFORM_ADMIN'); + }); + + it('the two shapes are INDISTINGUISHABLE by name and separable only by the rung', async () => { + const nameOnly = makeAuthzQl(authzTables('name-only')); + const genuine = makeAuthzQl(authzTables('genuine')); + const a = await buildContextForUser(nameOnly as any, USER); + const b = await buildContextForUser(genuine as any, USER); + + expect(a.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(b.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect([a.posture, b.posture]).toEqual(['MEMBER', 'PLATFORM_ADMIN']); + }); +}); + +describe('[#15981] the FALLBACK branch — reachable on a posture-less context, and it no longer reads the name', () => { + const base = { userId: 'a1', tenantId: 'org1', permissions: [] }; + + it('THREE-WAY AGREEMENT — the name says yes; the report and the rung both say no, and agree', async () => { + // No `posture` key: the early return is skipped and the fallback runs. This + // is the branch the shipping builder cannot produce, and the only one this + // change actually moves. + const context = { ...base, positions: [BUILTIN_IDENTITY_PLATFORM_ADMIN, 'everyone'] }; + const ql = makeAuthzQl(authzTables('name-only')); + + const nameRead = context.positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + const reported = await postureOf(context); + const rung = await hasPlatformAdminStanding(ql as any, USER); + + expect({ nameRead, reported, rung }).toEqual({ + nameRead: true, reported: 'MEMBER', rung: false, + }); + }); + + it('the unscoped-grant flag alone STILL yields PLATFORM_ADMIN — the evidence that survives', async () => { + // `buildContextForUser` sets this from `grants.posture === 'PLATFORM_ADMIN'`, + // so the fallback's remaining input is the rung itself. + expect( + await postureOf({ ...base, positions: ['everyone'], hasPlatformAdminGrant: true }), + ).toBe('PLATFORM_ADMIN'); + }); + + it('the grant flag wins even when the name is absent, and the name loses even when the flag is false', async () => { + expect( + await postureOf({ ...base, positions: ['everyone'], hasPlatformAdminGrant: true }), + ).toBe('PLATFORM_ADMIN'); + expect( + await postureOf({ + ...base, + positions: [BUILTIN_IDENTITY_PLATFORM_ADMIN, 'everyone'], + hasPlatformAdminGrant: false, + }), + ).toBe('MEMBER'); + }); + + it('an explicit ctx.posture still wins over both — the early return is untouched', async () => { + expect( + await postureOf({ ...base, positions: ['everyone'], posture: 'PLATFORM_ADMIN' }), + ).toBe('PLATFORM_ADMIN'); + expect( + await postureOf({ + ...base, + positions: [BUILTIN_IDENTITY_PLATFORM_ADMIN, 'everyone'], + posture: 'MEMBER', + }), + ).toBe('MEMBER'); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-positions-name-authority.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule-positions-name-authority.test.ts new file mode 100644 index 0000000000..fbf56421a1 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/sharing-rule-positions-name-authority.test.ts @@ -0,0 +1,288 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15981] A `sys_user_position` row SPELLING `platform_admin` confers no + * platform authority on the sharing-rule surface. + * + * ## The defect this pins + * + * `hasPlatformAuthority` derived PLATFORM standing from a NAME: + * + * const positions = Array.isArray(context?.positions) ? context.positions : []; + * return positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + * + * `sys_user_position` is `apiEnabled` and its `position` values are + * unconstrained, so a tenant can mint an ADR-0057 D4 row spelling that exact + * built-in name. `resolveUserAuthzGrants` §4 pushes the row's `position` + * straight into `grants.positions`, and `resolveAuthzContext` copies that array + * onto `ExecutionContext.positions` verbatim. The capability rung + * (`grants.posture`, §6d) is derived from the unscoped `admin_full_access` + * grant and nothing else, so it stays `MEMBER` — the two answers genuinely + * disagree, and the name-read took the wrong one. + * + * ⭐ WHAT THE ESCALATION BUYS, driven rather than argued (see the arms below): + * `manage_sharing` is an ORG-scoped capability (ADR-0111 D6) that an ordinary + * tenant admin may grant. Holding it with no organization resolved is refused + * by `assertResolvableAdminScope` precisely because an unscoped answer "would + * expose every tenant's rules". The D4 name-read was the bypass: it satisfied + * that gate, `adminOrgScope` then returned the UNFILTERED `where`, and + * `listRules` answered with every organization's rows. The same read also + * carried `deleteRule` past `assertCanDeletePlatformGlobalRule`. + * + * ## POPULATION OF THIS PIN — stated because a pin proves only what it covers + * + * Covers: an ORG-LESS caller holding `manage_sharing`, in two shapes — a D4 + * row spelling the built-in name with no capability grant behind it + * (`name-only`), and a genuine unscoped `admin_full_access` grant + * (`genuine`, the control that proves the fix did not simply deny everyone). + * Both shapes are built by inserting rows and resolving them through the REAL + * `resolveUserAuthzGrants`, so the `positions` / `posture` disagreement under + * test is produced by the shipping resolver rather than hand-asserted. + * + * Does NOT cover: the `manage_platform_settings` spelling of platform + * authority (untouched by this change and asserted only as still-admitting + * below), tenant-SCOPED callers (a caller with an organization never reaches + * `assertResolvableAdminScope`'s refusal at all), the ADR-0091 validity window, + * or the ADR-0049 `active` flags. Those are other suites' populations, and + * their passing is NOT evidence about this one. + */ + +import { describe, it, expect } from 'vitest'; +import { BUILTIN_IDENTITY_PLATFORM_ADMIN } from '@objectstack/spec/identity'; +import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { hasPlatformAdminStanding, resolveUserAuthzGrants } from '@objectstack/core'; +import { SharingRuleService } from './sharing-rule-service.js'; + +const USER = 'usr_subject'; +const OTHER_ORG = 'org_victim'; +const HOME_ORG = 'org_home'; +const PS_SHARING = 'ps_sharing_admin'; +const PS_ADMIN = 'ps_admin_full_access'; + +/** + * A minimal ObjectQL double for the AUTHZ resolver, copied in shape from + * `resolve-authz-context.platform-admin-config.test.ts`'s `makeQl` — including + * its refusal of top-level `$` combinators. The resolver issues none on this + * path, so a matcher that silently read `$or` as a field name would leave the + * suite asserting on an empty result with nothing erroring. + */ +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; + }, + }; +} + +/** The permission set that carries the ORG-scoped `manage_sharing` capability. */ +const sharingAdminSet = { + id: PS_SHARING, + name: 'sharing_admin', + system_permissions: ['manage_sharing'], + active: true, +}; + +function authzTables(shape: 'name-only' | 'genuine') { + const userPositions = + shape === 'name-only' + ? [ + // Exactly what a tenant admin can write through the `apiEnabled` + // `sys_user_position` surface: a row whose NAME is the built-in. + { user_id: USER, position: BUILTIN_IDENTITY_PLATFORM_ADMIN, organization_id: null }, + ] + : []; + const userSets: Array> = [ + // The ORG-scoped capability both shapes hold — the precondition, not the + // axis under test. Scoped to `HOME_ORG`, so it can never be mistaken for + // the unscoped grant that confers standing. + { user_id: USER, permission_set_id: PS_SHARING, organization_id: HOME_ORG }, + ]; + if (shape === 'genuine') { + userSets.push({ user_id: USER, permission_set_id: PS_ADMIN, organization_id: null }); + } + return { + sys_user: [{ id: USER, email: 'subject@example.com', email_verified: true }], + sys_member: [{ organization_id: HOME_ORG, user_id: USER, role: 'member' }], + sys_user_position: userPositions, + // An ACTIVE catalogue row for the minted position, so ADR-0049's + // deactivated-position filter cannot be what carries the arm. + sys_position: + shape === 'name-only' + ? [{ id: 'pos_pa', name: BUILTIN_IDENTITY_PLATFORM_ADMIN, label: 'Platform Admin', active: true }] + : [], + sys_position_permission_set: [], + sys_user_permission_set: userSets, + sys_permission_set: [ + sharingAdminSet, + { id: PS_ADMIN, name: ADMIN_FULL_ACCESS, active: true }, + ], + }; +} + +/** + * Resolve one principal through the REAL resolver and hand back both the + * context the transports would build and the rung, so every assertion below is + * about resolver output rather than a hand-written array. + * + * The caller is deliberately ORG-LESS (`tenantId` omitted): that is the shape + * `assertResolvableAdminScope` exists to refuse. + */ +async function resolve(shape: 'name-only' | 'genuine') { + const ql = makeAuthzQl(authzTables(shape)); + const grants = await resolveUserAuthzGrants(ql as any, USER); + const context = { + userId: USER, + positions: grants.positions, + permissions: grants.permissions, + systemPermissions: grants.systemPermissions, + ...(grants.posture ? { posture: grants.posture } : {}), + } as ExecutionContext; + return { context, grants, rung: await hasPlatformAdminStanding(ql as any, USER) }; +} + +/** Two tenants' rules plus one platform-global row, so a cross-tenant read is VISIBLE. */ +const RULE_ROWS = [ + { id: 'srule_home', name: 'home_rule', object_name: 'account', organization_id: HOME_ORG, active: true }, + { id: 'srule_victim', name: 'victim_rule', object_name: 'account', organization_id: OTHER_ORG, active: true }, + { id: 'srule_global', name: 'global_rule', object_name: 'account', organization_id: null, active: true }, +]; + +/** + * A sharing-engine double that RECORDS its reads. It refuses top-level `$` + * combinators for the same reason the authz double does; on this pin's + * org-less arms `adminOrgScope` never produces one, so a `$or` reaching it is + * itself a finding rather than something to teach the double. + */ +function makeSharingEngine() { + const finds: Array<{ object: string; where: any }> = []; + const deletes: Array<{ object: string; id: unknown }> = []; + const matches = (row: Record, where: any): boolean => + Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake engine: unsupported operator ${k}`); + return row[k] === v; + }); + return { + finds, + deletes, + async find(object: string, opts: any) { + finds.push({ object, where: opts?.where }); + if (object !== 'sys_sharing_rule') return []; + const rows = RULE_ROWS.filter((r) => matches(r, opts?.where)); + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + }, + async delete(object: string, id: unknown) { + deletes.push({ object, id }); + }, + }; +} + +function makeService(engine: ReturnType) { + return new SharingRuleService({ + engine: engine as any, + sharing: { revoke: async () => {} } as any, + logger: { warn: () => {} }, + }); +} + +describe('[#15981] a D4 `sys_user_position` row spelling `platform_admin` confers NO platform authority', () => { + it('the name IS in positions[] while the rung says MEMBER — the premise, without which the rest is vacuous', async () => { + const { context, grants, rung } = await resolve('name-only'); + + expect(context.positions, JSON.stringify(context.positions)).toContain( + BUILTIN_IDENTITY_PLATFORM_ADMIN, + ); + expect(grants.posture).not.toBe('PLATFORM_ADMIN'); + expect(rung).toBe(false); + // The org-scoped capability really is held — so a refusal below is about + // platform authority, not about a caller who could never manage rules. + expect(context.systemPermissions).toContain('manage_sharing'); + }); + + it('THREE-WAY AGREEMENT — the name says yes; the site gate and the rung both say no, and agree', async () => { + const { context, rung } = await resolve('name-only'); + const engine = makeSharingEngine(); + const service = makeService(engine); + + const nameRead = (context.positions ?? []).includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + // The site's own gate, read through the public verb it guards. + const gate = await service + .listRules({}, context) + .then(() => true) + .catch(() => false); + + expect({ nameRead, gate, rung }).toEqual({ nameRead: true, gate: false, rung: false }); + }); + + it('`listRules` refuses instead of answering with every organization’s rules', async () => { + const { context } = await resolve('name-only'); + const engine = makeSharingEngine(); + const service = makeService(engine); + + await expect(service.listRules({}, context)).rejects.toThrow(/PERMISSION_DENIED/); + // Refused BEFORE the read, so no other tenant's rows were ever fetched — + // the half a throw alone would not establish. + expect(engine.finds).toEqual([]); + }); + + it('`deleteRule` refuses to destroy a platform-global rule', async () => { + const { context } = await resolve('name-only'); + const engine = makeSharingEngine(); + const service = makeService(engine); + + await expect(service.deleteRule('global_rule', context)).rejects.toThrow(/PERMISSION_DENIED/); + expect(engine.deletes).toEqual([]); + }); +}); + +describe('[#15981] CONTROL — a genuine unscoped admin_full_access grant still admits', () => { + it('all three answers are TRUE and agree, and the cross-tenant read is served', async () => { + const { context, rung } = await resolve('genuine'); + const engine = makeSharingEngine(); + const service = makeService(engine); + + const nameRead = (context.positions ?? []).includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + const rows = await service.listRules({}, context); + expect({ nameRead, gate: true, rung }).toEqual({ nameRead: true, gate: true, rung: true }); + // A real platform operator still reads across tenants — the functional + // half, which a fix that simply denied everyone would break. + expect(rows.map((r) => r.id).sort()).toEqual(['srule_global', 'srule_home', 'srule_victim']); + }); + + it('the two shapes are INDISTINGUISHABLE by name and separable only by the rung', async () => { + const escalation = await resolve('name-only'); + const genuine = await resolve('genuine'); + + // Identical on the axis a name-reading predicate would consult … + expect(escalation.context.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(genuine.context.positions).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + // … and opposite on the axis that actually decides. + expect(escalation.rung).toBe(false); + expect(genuine.rung).toBe(true); + }); +}); + +describe('[#15981] the OTHER spelling of platform authority is untouched', () => { + it('`manage_platform_settings` still admits an org-less caller', async () => { + const engine = makeSharingEngine(); + const service = makeService(engine); + // A hand-built context on purpose: this arm pins the capability spelling + // this change does NOT touch, and it reaches contexts no resolver builds + // (ADR-0068 D2's second channel, per `hasPlatformAuthority`'s doc block). + const context = { + userId: 'usr_ops', + positions: [], + permissions: [], + systemPermissions: ['manage_sharing', 'manage_platform_settings'], + } as ExecutionContext; + + await expect(service.listRules({}, context)).resolves.toHaveLength(3); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index dafa6fddf4..be016b7e73 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -12,10 +12,11 @@ import type { // has declared for every one of these context parameters since #6523 (the // #6206 ruling: no per-site subset contracts). import type { ExecutionContext } from '@objectstack/spec/kernel'; -// [#7795] The built-in platform-operator position (ADR-0068 D2) — one of the -// two spellings of platform authority the ruling names; see -// {@link SharingRuleService.assertCanDeletePlatformGlobalRule}. -import { BUILTIN_IDENTITY_PLATFORM_ADMIN } from '@objectstack/spec/identity'; +// [#15981] The built-in platform-operator position NAME is deliberately NOT +// imported here any more. It used to spell the second half of +// {@link SharingRuleService.hasPlatformAuthority}, and reading it as authority +// became an escalation channel once `positions[]` started carrying ADR-0057 D4 +// `sys_user_position` names; that predicate now reads the ADR-0095 rung. // [#8710] The ONE predicate for `sys_position.active` / `sys_permission_set.active` // (#8613). Reused rather than re-spelled: two notions of "is this row active" // — one honouring the 1/0 and 'false' storage shapes, one not — is how the @@ -316,18 +317,32 @@ export class SharingRuleService implements ISharingRuleService { * accepting either is the fail-safe reading: they are two independent * channels by which the same unscoped `admin_full_access` grant reaches an * `ExecutionContext` (a `scope: 'platform'` capability on - * `systemPermissions`; the ADR-0068 D2 built-in position on `positions`), - * and a hand-built context may carry only one. + * `systemPermissions`; the ADR-0095 posture RUNG). * * It is asked TWICE now — once to authorize destroying a platform-global * rule, once to decide whether an org-less caller may read across tenants — * so it is one predicate rather than two spellings that can drift apart. + * + * ⛔ [#15981] The second spelling is the RUNG, never + * `positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)`. The two stopped + * being the same channel when `positions[]` became the security axis: it + * carries ADR-0057 D4 `sys_user_position` names, that table is `apiEnabled` + * with unconstrained `position` values, and `resolveUserAuthzGrants` §4 + * pushes a minted row's name straight onto the array. `resolve-authz-context.ts` + * states the rule at `hasPlatformAdminStanding`, and + * `grants.posture === 'PLATFORM_ADMIN'` is byte-for-byte what that predicate + * returns — the unscoped `admin_full_access` evidence, which is what this + * doc block meant by the built-in position all along. + * + * Driven, not argued: with the minted row present and only the ORG-scoped + * `manage_sharing` capability held, the old read admitted an org-less caller + * to every tenant's rules through {@link listRules}. See + * `sharing-rule-positions-name-authority.test.ts`. */ private hasPlatformAuthority(context: ExecutionContext): boolean { const caps = Array.isArray(context?.systemPermissions) ? context.systemPermissions : []; if (caps.includes('manage_platform_settings')) return true; - const positions = Array.isArray(context?.positions) ? context.positions : []; - return positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + return context?.posture === 'PLATFORM_ADMIN'; } /** diff --git a/packages/plugins/plugin-sharing/src/sharing-rule.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule.test.ts index 22d33a9ad0..8185bcf464 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule.test.ts @@ -1377,10 +1377,34 @@ describe('[#7795] deleting a platform-global rule requires platform authority', userId: 'ops', organizationId: 'org1', systemPermissions: ['manage_sharing', 'manage_platform_settings'], } as any; - /** Platform authority, spelling 2: the built-in POSITION (ADR-0068 D2). */ - const PLATFORM_ADMIN_POSITION = { + /** + * Platform authority, spelling 2: the ADR-0095 posture RUNG. + * + * [#15981] This fixture used to spell the second channel as the built-in + * POSITION NAME alone, on the reading that the name is only ever projected + * from the unscoped `admin_full_access` grant. `positions[]` is now the + * security axis and also carries ADR-0057 D4 `sys_user_position` names, so + * the name no longer implies the grant — the rung does. A genuine operator's + * resolved context carries BOTH (the projection is still emitted), which is + * why this keeps `positions` as well: dropping it would make the fixture + * unrepresentative in the opposite direction. + */ + const PLATFORM_ADMIN_RUNG = { userId: 'root', organizationId: 'org1', systemPermissions: ['manage_sharing'], positions: ['platform_admin'], + posture: 'PLATFORM_ADMIN', + } as any; + /** + * [#15981] The ESCALATION shape: the same name with no rung behind it — what + * a tenant can mint through the `apiEnabled` `sys_user_position` surface. + * Refused. Driven end-to-end through the real resolver in + * `sharing-rule-positions-name-authority.test.ts`; this arm keeps the + * boundary visible in the suite that owns the gate. + */ + const PLATFORM_ADMIN_NAME_ONLY = { + userId: 'impostor', organizationId: 'org1', + systemPermissions: ['manage_sharing'], positions: ['platform_admin'], + posture: 'MEMBER', } as any; /** No `manage_sharing` at all — must still be refused by the OLDER gate. */ const MALLORY = { userId: 'mallory', organizationId: 'org1', systemPermissions: [] } as any; @@ -1511,12 +1535,19 @@ describe('[#7795] deleting a platform-global rule requires platform authority', expect(grantsOf(seededId)).toHaveLength(0); }); - it('the platform_admin position authorizes the delete', async () => { - await expect(rules.deleteRule(seededId, PLATFORM_ADMIN_POSITION)).resolves.toBeUndefined(); + it('the PLATFORM_ADMIN posture rung authorizes the delete', async () => { + await expect(rules.deleteRule(seededId, PLATFORM_ADMIN_RUNG)).resolves.toBeUndefined(); expect(engine._tables.sys_sharing_rule.find((r) => r.id === seededId)).toBeUndefined(); expect(grantsOf(seededId)).toHaveLength(0); }); + it('[#15981] the built-in position NAME without the rung does NOT', async () => { + expect(await refusalCodeOf(rules.deleteRule(seededId, PLATFORM_ADMIN_NAME_ONLY))) + .toBe('PERMISSION_DENIED'); + // The row survives: a refusal that still destroyed the rule would be none. + expect(engine._tables.sys_sharing_rule.find((r) => r.id === seededId)).toBeTruthy(); + }); + it('a system context (boot seeding, hooks, backfills) still deletes', async () => { await expect(rules.deleteRule(seededId, BOOT)).resolves.toBeUndefined(); expect(engine._tables.sys_sharing_rule.find((r) => r.id === seededId)).toBeUndefined(); @@ -1620,9 +1651,19 @@ describe('[#8158] a non-system caller with NO organization does not get the syst const ORG_LESS_PLATFORM_CAP = { userId: 'ops', systemPermissions: ['manage_sharing', 'manage_platform_settings'], } as any; - /** Platform authority with no active org, spelling 2: the ADR-0068 D2 position. */ - const ORG_LESS_PLATFORM_POSITION = { + /** + * Platform authority with no active org, spelling 2: the ADR-0095 rung. + * [#15981] Was the built-in position NAME alone — see the migration note on + * `PLATFORM_ADMIN_RUNG` above for why a name is no longer that evidence. + */ + const ORG_LESS_PLATFORM_RUNG = { userId: 'root', systemPermissions: ['manage_sharing'], positions: ['platform_admin'], + posture: 'PLATFORM_ADMIN', + } as any; + /** [#15981] The escalation shape: the minted name, no rung. Refused. */ + const ORG_LESS_NAME_ONLY = { + userId: 'impostor', systemPermissions: ['manage_sharing'], positions: ['platform_admin'], + posture: 'MEMBER', } as any; /** No capability at all — the OLDER gate must still fire first. */ const ORG_LESS_NOBODY = { userId: 'nobody', systemPermissions: [] } as any; @@ -1788,10 +1829,18 @@ describe('[#8158] a non-system caller with NO organization does not get the syst // bootstrapped, so refusing it would be a functional regression. expect(namesOf(await rules.listRules({}, ORG_LESS_PLATFORM_CAP))) .toEqual(['org1_rule', 'org2_rule', SEEDED].sort()); - expect(namesOf(await rules.listRules({}, ORG_LESS_PLATFORM_POSITION))) + expect(namesOf(await rules.listRules({}, ORG_LESS_PLATFORM_RUNG))) .toEqual(['org1_rule', 'org2_rule', SEEDED].sort()); expect((await rules.getRule(org2RuleId, ORG_LESS_PLATFORM_CAP))?.name).toBe('org2_rule'); - expect((await rules.getRule('org2_rule', ORG_LESS_PLATFORM_POSITION))?.id).toBe(org2RuleId); + expect((await rules.getRule('org2_rule', ORG_LESS_PLATFORM_RUNG))?.id).toBe(org2RuleId); + }); + + it('[#15981] an org-less caller holding only the minted NAME reads nothing', async () => { + // The whole point of the #8158 gate: `manage_sharing` is ORG-scoped, so an + // org-less holder of it must not answer unscoped. The name-read was the + // bypass — this is the arm that keeps it closed. + await expect(rules.listRules({}, ORG_LESS_NAME_ONLY)).rejects.toThrow(/PERMISSION_DENIED/); + await expect(rules.getRule(org2RuleId, ORG_LESS_NAME_ONLY)).rejects.toThrow(/PERMISSION_DENIED/); }); // ── the anti-vacuity control: scoped ≠ single-tenant ───────────────── diff --git a/packages/runtime/src/domains/action-activation-posture-gate.test.ts b/packages/runtime/src/domains/action-activation-posture-gate.test.ts index 0e10c4df8d..80ea2ec775 100644 --- a/packages/runtime/src/domains/action-activation-posture-gate.test.ts +++ b/packages/runtime/src/domains/action-activation-posture-gate.test.ts @@ -124,10 +124,36 @@ const PLATFORM_OPERATOR = (): HttpProtocolContext => ({ positions: ['platform_admin'], permissions: ['admin_full_access'], systemPermissions: ['manage_metadata'], + // [#15981] The ADR-0095 rung, which is what the gate now reads. A real + // operator's resolved context carries it AND the projected position name; + // this fixture used to carry only the NAME, and a name stopped being + // platform evidence when `positions[]` became the security axis (it also + // carries ADR-0057 D4 `sys_user_position` names, mintable by a tenant). + posture: 'PLATFORM_ADMIN', organizationId: null, }, } as unknown as HttpProtocolContext); +/** + * [#15981] The ESCALATION shape: a tenant org admin who ALSO holds a + * `sys_user_position` row spelling the built-in name. `sys_user_position` is + * `apiEnabled` with unconstrained `position` values, so a tenant can mint this + * for themselves; the rung stays `TENANT_ADMIN`, because it is derived from the + * unscoped `admin_full_access` grant they do not hold. Must be REFUSED. + */ +const MINTED_NAME_ONLY = (): HttpProtocolContext => ({ + request: {}, + environmentId: 'platform', + executionContext: { + userId: 'u_impostor', + positions: ['platform_admin', 'org_owner'], + permissions: ['organization_admin'], + systemPermissions: ['manage_metadata'], + posture: 'TENANT_ADMIN', + organizationId: 'org_northwind', + }, +} as unknown as HttpProtocolContext); + /** Engine self-invocation — never settable from the wire. */ const SYSTEM = (): HttpProtocolContext => ({ request: {}, @@ -221,6 +247,18 @@ describe('ADR-0126 §5 — the action activation write is operator-gated in wall expect(h.setActionActive).toHaveBeenCalled(); }); + it('[#15981] REFUSES a tenant admin holding only a MINTED `platform_admin` name', async () => { + const h = boot(posture); + + const r = await flip(h, MINTED_NAME_ONLY()); + + expect(statusOf(r)).toBe(403); + expect(codeOf(r)).toBe('PERMISSION_DENIED'); + // Refused BEFORE the install-wide write — the half a status + // assertion alone would not establish. + expect(h.setActionActive).not.toHaveBeenCalled(); + }); + it('ALLOWS engine self-invocation', async () => { const h = boot(posture); diff --git a/packages/runtime/src/domains/activation-gate-positions-name-authority.test.ts b/packages/runtime/src/domains/activation-gate-positions-name-authority.test.ts new file mode 100644 index 0000000000..ee6e926999 --- /dev/null +++ b/packages/runtime/src/domains/activation-gate-positions-name-authority.test.ts @@ -0,0 +1,254 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#15981] A `sys_user_position` row SPELLING `platform_admin` confers no +// operator authority on the ADR-0126 §5 activation gate. +// +// ## The defect this pins +// +// `refuseUngrantedActivationWrite` derived operator standing from a NAME: +// +// const positions: string[] = Array.isArray(ec?.positions) ? ec.positions : []; +// if (positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)) return undefined; +// +// The gate's own doc block argued for reading the position on the grounds that +// it is "unscoped, sourced from the unscoped `admin_full_access` grant". That +// premise stopped holding when `positions[]` became the security axis: it now +// also carries ADR-0057 D4 `sys_user_position` names, and that table is +// `apiEnabled` with unconstrained `position` values. So a tenant could mint a +// row spelling the built-in name, `resolveUserAuthzGrants` §4 pushed it into +// `grants.positions`, and this gate opened. +// +// ⭐ WHAT THE ESCALATION BUYS, driven rather than argued: this gate is the ONLY +// thing standing between a tenant org admin and the install-wide activation +// row under a walled posture. It is #10243 exactly — a tenant org owner +// switching a shipped flow off ENVIRONMENT-WIDE — except that ADR-0126 made +// the row DURABLE, so the same leak now survives a cold boot. The arms below +// drive the real `POST /automation/:name/toggle` route and assert on whether +// `toggleFlow` was entered, which is the write itself. +// +// ## POPULATION OF THIS PIN — stated because a pin proves only what it covers +// +// Covers: the `group` and `isolated` (wall-enforcing) postures, on the +// automation toggle door, for a tenant org admin who already holds +// `manage_metadata` — so the tier-above gate passes and THIS gate is the only +// thing left. Three shapes: a D4 row spelling the built-in name with no +// capability grant (`name-only`), a genuine unscoped `admin_full_access` grant +// (`genuine`), and a plain tenant admin (`plain`, the floor). All three +// contexts are produced by inserting rows and resolving them through the REAL +// `resolveUserAuthzGrants`, so `positions` / `posture` come from the shipping +// resolver rather than being hand-asserted. +// +// Does NOT cover: the `single` posture (where the gate is inert by design and +// this change cannot reach it), the actions door (same gate, same call — +// covered by `action-activation-posture-gate.test.ts`'s own population), the +// `manage_metadata` tier above, or engine self-invocation. Those are other +// suites' populations and their passing is NOT evidence about this one. + +import { describe, it, expect, vi } from 'vitest'; +import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import { hasPlatformAdminStanding, resolveUserAuthzGrants } from '@objectstack/core'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import type { HttpProtocolContext } from '../http-dispatcher.js'; + +const FLOW = 'vendor_lead_router'; +const DEFINITION = { name: FLOW, label: 'Vendor Lead Router', type: 'autolaunched', nodes: [], edges: [] }; + +const TENANT_ORG = 'org_northwind'; +const ACTOR = 'usr_tenant_admin'; +const PS_ADMIN = 'ps_admin_full_access'; +const PS_METADATA = 'ps_metadata_author'; + +interface Harness { + dispatcher: HttpDispatcher; + toggleFlow: ReturnType; +} + +/** A dispatcher whose `tenancy` service reports the given posture. */ +function boot(posture: 'group' | 'isolated'): Harness { + const toggleFlow = vi.fn(async () => undefined); + const services: Record = { + automation: { + handlerReady: true, + toggleFlow, + getFlow: vi.fn(async (name: string) => (name === FLOW ? DEFINITION : undefined)), + }, + tenancy: { posture }, + }; + const resolve = (name: string): unknown => services[name]; + const kernel = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + return { dispatcher: new HttpDispatcher(kernel as never), toggleFlow }; +} + +/** + * 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 = 'name-only' | 'genuine' | 'plain'; + +function authzTables(shape: Shape) { + const userSets: Array> = [ + // The ORG-scoped authoring capability every shape holds, so the + // `manage_metadata` tier above always passes and this gate is the only + // thing under test. Scoped to the org, never unscoped. + { user_id: ACTOR, permission_set_id: PS_METADATA, organization_id: TENANT_ORG }, + ]; + if (shape === 'genuine') { + userSets.push({ user_id: ACTOR, permission_set_id: PS_ADMIN, organization_id: null }); + } + return { + sys_user: [{ id: ACTOR, email: 'tenant-admin@example.com', email_verified: true }], + sys_member: [{ organization_id: TENANT_ORG, user_id: ACTOR, role: 'owner' }], + sys_user_position: + shape === 'name-only' + ? [ + // Exactly what a tenant admin can write through the + // `apiEnabled` `sys_user_position` surface. + { user_id: ACTOR, position: BUILTIN_IDENTITY_PLATFORM_ADMIN, organization_id: null }, + ] + : [], + // An ACTIVE catalogue row, so ADR-0049's deactivated-position filter is + // not what carries the arm. + sys_position: + shape === 'name-only' + ? [{ id: 'pos_pa', name: BUILTIN_IDENTITY_PLATFORM_ADMIN, label: 'Platform Admin', active: true }] + : [], + sys_position_permission_set: [], + sys_user_permission_set: userSets, + sys_permission_set: [ + { id: PS_METADATA, name: 'metadata_author', system_permissions: ['manage_metadata'], active: true }, + { id: PS_ADMIN, name: ADMIN_FULL_ACCESS, active: true }, + ], + }; +} + +/** Resolve one principal through the REAL resolver into the protocol context. */ +async function resolve(shape: Shape) { + const ql = makeAuthzQl(authzTables(shape)); + const grants = await resolveUserAuthzGrants(ql as any, ACTOR, { tenantId: TENANT_ORG }); + const context = { + request: {}, + executionContext: { + userId: ACTOR, + tenantId: TENANT_ORG, + organizationId: TENANT_ORG, + positions: grants.positions, + permissions: grants.permissions, + systemPermissions: grants.systemPermissions, + ...(grants.posture ? { posture: grants.posture } : {}), + }, + } as HttpProtocolContext; + return { context, grants, rung: await hasPlatformAdminStanding(ql as any, ACTOR) }; +} + +const statusOf = (response: unknown): unknown => (response as any)?.status; +const codeOf = (response: unknown): unknown => { + const r = response as any; + return r?.body?.error?.code ?? r?.body?.error?.details?.code; +}; +const messageOf = (response: unknown): string => String((response as any)?.body?.error?.message ?? ''); + +const toggle = (h: Harness, ctx: HttpProtocolContext) => + h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: false }, ctx, undefined); + +for (const posture of ['group', 'isolated'] as const) { + describe(`[#15981] \`${posture}\` — a D4 row spelling \`platform_admin\` does NOT open the install-wide switch`, () => { + it('the name IS in positions[] while the rung says TENANT_ADMIN — the premise, without which the rest is vacuous', async () => { + const { context, grants, rung } = await resolve('name-only'); + const ec = (context as any).executionContext; + + expect(ec.positions, JSON.stringify(ec.positions)).toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(grants.posture).not.toBe('PLATFORM_ADMIN'); + expect(rung).toBe(false); + // The tier above really does pass, so a refusal below is this + // gate's and not `manage_metadata`'s. + expect(ec.systemPermissions).toContain('manage_metadata'); + }); + + it('THREE-WAY AGREEMENT — the name says yes; the site gate and the rung both say no, and agree', async () => { + const { context, rung } = await resolve('name-only'); + const h = boot(posture); + const ec = (context as any).executionContext; + + const nameRead = (ec.positions ?? []).includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + const { response } = await toggle(h, context); + const gate = statusOf(response) === 200; + + expect({ nameRead, gate, rung }).toEqual({ nameRead: true, gate: false, rung: false }); + }); + + it('REFUSES 403 PERMISSION_DENIED and never enters toggleFlow', async () => { + const { context } = await resolve('name-only'); + const h = boot(posture); + + const { response } = await toggle(h, context); + + expect(statusOf(response)).toBe(403); + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + // The load-bearing assertion: refused BEFORE the write. A gate that + // wrote the install-wide row and then refused would satisfy the two + // above and still be #10243. + expect(h.toggleFlow).not.toHaveBeenCalled(); + }); + + it('the refusal still says nothing about the caller’s own positions (#7450)', async () => { + const { context } = await resolve('name-only'); + const h = boot(posture); + + const message = messageOf((await toggle(h, context)).response); + + expect(message).toContain(posture); + expect(message).toContain('ADR-0126 §5'); + expect(message).not.toContain(BUILTIN_IDENTITY_PLATFORM_ADMIN); + expect(message).not.toContain('org_owner'); + }); + + it('answers the same as a PLAIN tenant admin — the minted row buys nothing', async () => { + const nameOnly = await resolve('name-only'); + const plain = await resolve('plain'); + + const hName = boot(posture); + const hPlain = boot(posture); + const viaName = statusOf((await toggle(hName, nameOnly.context)).response); + const viaPlain = statusOf((await toggle(hPlain, plain.context)).response); + + // The floor: if this stops being 403, the comparison measures nothing. + expect(viaPlain).toBe(403); + expect(viaName).toBe(viaPlain); + expect(hName.toggleFlow).not.toHaveBeenCalled(); + }); + + it('CONTROL — a genuine unscoped admin_full_access grant still flips the switch', async () => { + const { context, rung } = await resolve('genuine'); + const h = boot(posture); + const ec = (context as any).executionContext; + + const nameRead = (ec.positions ?? []).includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); + const { response } = await toggle(h, context); + + expect({ nameRead, gate: statusOf(response) === 200, rung }).toEqual({ + nameRead: true, gate: true, rung: true, + }); + expect(h.toggleFlow).toHaveBeenCalledWith(FLOW, false); + }); + }); +} diff --git a/packages/runtime/src/domains/activation-gate.ts b/packages/runtime/src/domains/activation-gate.ts index d24e3a7631..693ee3a7a2 100644 --- a/packages/runtime/src/domains/activation-gate.ts +++ b/packages/runtime/src/domains/activation-gate.ts @@ -32,19 +32,37 @@ * writable by tenants would be the same leak WITH persistence, which is * strictly worse than what was measured. * - * ## Why the operator test is a POSITION and not a capability + * ## Why the operator test is the POSTURE RUNG and not a capability * * ADR-0126 §5 says "the platform-operator capability"; the platform's actual - * operator identity is the ADR-0068 D2 built-in `platform_admin` POSITION, - * documented verbatim as "Platform operator (SaaS admin). NOT a tenant user - * role", unscoped, sourced from the unscoped `admin_full_access` grant. No - * capability in `PLATFORM_CAPABILITIES` carries that meaning: `manage_metadata` - * is the one the tier above already requires, and a tenant org admin can hold - * it — so spelling this gate as a capability check would either re-ask the - * question already answered or invent a capability name, which would be a - * `packages/spec` change ADR-0126 §9 walls this family out of. The position IS - * the platform's operator concept; this gate reads it rather than minting a - * synonym. + * operator identity is the ADR-0068 D2 platform operator — "Platform operator + * (SaaS admin). NOT a tenant user role", unscoped, sourced from the unscoped + * `admin_full_access` grant. No capability in `PLATFORM_CAPABILITIES` carries + * that meaning: `manage_metadata` is the one the tier above already requires, + * and a tenant org admin can hold it — so spelling this gate as a capability + * check would either re-ask the question already answered or invent a + * capability name, which would be a `packages/spec` change ADR-0126 §9 walls + * this family out of. + * + * ⛔ [#15981] What this gate reads is the ADR-0095 D2/D3 posture RUNG + * (`posture === 'PLATFORM_ADMIN'`), NEVER + * `positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)`. It used to read the + * name, on the reasoning above that the built-in position IS "sourced from the + * unscoped `admin_full_access` grant" — a premise that stopped holding when + * `positions[]` became the security axis. That array now also carries ADR-0057 + * D4 `sys_user_position` names; `sys_user_position` is `apiEnabled` with + * unconstrained `position` values, so a tenant could mint a row spelling that + * very built-in and `resolveUserAuthzGrants` §4 would push it onto the array. + * The rung is derived from the unscoped-grant evidence and nothing else, so it + * is what the paragraph above always MEANT — and it is byte-for-byte what + * `hasPlatformAdminStanding` returns. `resolve-authz-context.ts` states the + * rule at that predicate; this gate is one of the four sites #15981 found + * ignoring it. + * + * Driven, not argued: with the minted row present, a tenant org admin holding + * only the org-scoped `manage_metadata` capability flipped the install-wide + * switch under both walled postures — #10243 again, now with a DURABLE row. + * See `activation-gate-positions-name-authority.test.ts`. * * ## Fail-open on an ABSENT posture is deliberate, not a gap * @@ -57,11 +75,12 @@ // [ADR-0126 §5] The gate's two inputs: the deployment's EFFECTIVE tenancy // posture (the same resolver `resolve-execution-context.ts` uses, so admission -// and this gate can never disagree) and the built-in identity name that means -// "platform operator, NOT a tenant user role" (ADR-0068 D2). +// and this gate can never disagree) and — since #15981 — the caller's ADR-0095 +// authorization RUNG off the execution context, which is where "platform +// operator, NOT a tenant user role" (ADR-0068 D2) still means that. The +// built-in identity NAME is deliberately no longer imported: see the doc block. import { effectiveTenancyPosture } from '@objectstack/core'; import { postureEnforcesWall } from '@objectstack/spec/security'; -import { BUILTIN_IDENTITY_PLATFORM_ADMIN } from '@objectstack/spec/identity'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps } from '../domain-handler-registry.js'; @@ -145,8 +164,11 @@ export async function refuseUngrantedActivationWrite( } if (!posture || !postureEnforcesWall(posture)) return undefined; - const positions: string[] = Array.isArray(ec?.positions) ? ec.positions : []; - if (positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)) return undefined; + // [ADR-0095 D2/D3 · #15981] The platform-operator test is the posture RUNG. + // See the "Why the operator test is a POSITION" section above for why this + // is the SAME concept that section argues for, read through the one input + // that still means it. + if (ec?.posture === 'PLATFORM_ADMIN') return undefined; // The message names the posture and the sanctioned path — the loud-refusal // shape ADR-0126 §7 asks for throughout — and says nothing about the diff --git a/packages/runtime/src/domains/automation-activation-posture-gate.test.ts b/packages/runtime/src/domains/automation-activation-posture-gate.test.ts index 7ebd09e25c..9f45dd42f2 100644 --- a/packages/runtime/src/domains/automation-activation-posture-gate.test.ts +++ b/packages/runtime/src/domains/automation-activation-posture-gate.test.ts @@ -100,10 +100,35 @@ const PLATFORM_OPERATOR = (): HttpProtocolContext => ({ positions: ['platform_admin'], permissions: ['admin_full_access'], systemPermissions: ['manage_metadata'], + // [#15981] The ADR-0095 rung, which is what the gate now reads. A real + // operator's resolved context carries it AND the projected position name; + // this fixture used to carry only the NAME, and a name stopped being + // platform evidence when `positions[]` became the security axis (it also + // carries ADR-0057 D4 `sys_user_position` names, mintable by a tenant). + posture: 'PLATFORM_ADMIN', organizationId: null, }, } as HttpProtocolContext); +/** + * [#15981] The ESCALATION shape: a tenant org admin who ALSO holds a + * `sys_user_position` row spelling the built-in name. `sys_user_position` is + * `apiEnabled` with unconstrained `position` values, so a tenant can mint this + * for themselves; the rung stays `TENANT_ADMIN`, because it is derived from the + * unscoped `admin_full_access` grant they do not hold. Must be REFUSED. + */ +const MINTED_NAME_ONLY = (): HttpProtocolContext => ({ + request: {}, + executionContext: { + userId: 'u_impostor', + positions: ['platform_admin', 'org_owner'], + permissions: ['organization_admin'], + systemPermissions: ['manage_metadata'], + posture: 'TENANT_ADMIN', + organizationId: 'org_northwind', + }, +} as HttpProtocolContext); + /** Engine self-invocation — never settable from the wire. */ const SYSTEM = (): HttpProtocolContext => ({ request: {}, @@ -197,6 +222,18 @@ describe('ADR-0126 §5 — the activation write is operator-gated in walled post expect(h.toggleFlow).toHaveBeenCalledWith(FLOW, false); }); + it('[#15981] REFUSES a tenant admin holding only a MINTED `platform_admin` name', async () => { + const h = boot(posture); + + const { response } = await toggle(h, MINTED_NAME_ONLY()); + + expect(statusOf(response)).toBe(403); + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + // Refused BEFORE the install-wide write — the half a status + // assertion alone would not establish. + expect(h.toggleFlow).not.toHaveBeenCalled(); + }); + it('ALLOWS engine self-invocation', async () => { const h = boot(posture); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 81ef3511f3..38f5b095b8 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2126,6 +2126,16 @@ "verb": "delete", "pinned": 1 }, + { + "file": "packages/plugins/plugin-approvals/src/approval-positions-name-authority.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-positions-name-authority.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts", "verb": "update", From a8d29e2a327d976ecc6be6358a8e3a5b3b549a21 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 03:18:36 +0000 Subject: [PATCH 2/2] chore(docs): regenerate the system-context census from the merged tree The os-regen driver merged `content/docs/permissions/system-context.mdx` without a text merge, so the page had to be re-derived from the merged tree rather than from either side. Regenerated with the gate's own generator; the fixpoint holds (a second run rewrites 0 anchors) and `check:system-context-census` exits 0. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- content/docs/permissions/system-context.mdx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d72a894c8d..1a78541625 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1565`, `:1594`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1651`, `:1680`), and neither can an action body (`packages/runtime/src/domains/actions.ts:414`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:250` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1597` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1683` | ### 2. Write pipeline and data integrity @@ -136,7 +136,7 @@ The largest single consumer — **17 of the 105 sites**. | 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1528` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1189` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:459`, `:513`, `:517`, `:590`, `:620` | -| 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | +| 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:66` | | 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:279`, `:518` | ### 4. Approvals, reports, attachments, comments, knowledge @@ -158,7 +158,7 @@ The largest single consumer — **17 of the 105 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5145`, `:6571`, `:6819`, `:7250`, `:7443` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5231`, `:6657`, `:6905`, `:7336`, `:7529` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:535`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -167,7 +167,7 @@ The largest single consumer — **17 of the 105 sites**. | 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:157`, `:211` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | | 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | -| 59 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `email-template-provenance.ts:59`, `webhook-provenance.ts:50` | +| 59 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `email-template-provenance.ts:77`, `webhook-provenance.ts:68` | | 60 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `runtime-identity.ts:279`, called from `builtin/crud-nodes.ts:319` | | 61 | Inbox caller refusal names `isSystem` as what was carried | service-messaging | Get: nothing — the refusal still fires. The flag only shapes the diagnostic, because privilege is not an authorization subject | `inbox-caller.ts:148` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1590` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1565`, `:1594`; `domains/actions.ts:414` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1651`, `:1680`; `domains/actions.ts:414` | ---