diff --git a/.changeset/30771-platform-admin-action-gate.md b/.changeset/30771-platform-admin-action-gate.md new file mode 100644 index 00000000000..ebe88a1fe40 --- /dev/null +++ b/.changeset/30771-platform-admin-action-gate.md @@ -0,0 +1,24 @@ +--- +'@objectstack/platform-objects': patch +'@objectstack/runtime': patch +'@objectstack/mcp': patch +--- + +fix(platform-objects)!: the platform-admin actions declare their gate, so MCP `list_actions` no longer lists them to a caller who cannot run them, and both action doors refuse that caller themselves + +Clause-②: yes (narrowing) +Metadata-protocol: none + + + +**BREAKING.** The 15 first-party actions whose door runs the platform-admin gate (on `sys_user`: `ban_user`, `unban_user`, `unlock_user`, `create_user`, `set_user_password`, `impersonate_user`, `set_user_manager`, `list_platform_admins`; on `sys_member`: `add_member`; on `sys_oauth_application`: `disable_oauth_application`, `enable_oauth_application`; on `sys_sso_provider`: `register_sso_provider`, `register_saml_provider`, `request_domain_verification`, `verify_domain`) now declare `requiredPermissions`: `['manage_users']` for the account and membership doors, `['manage_platform_settings']` for the OAuth application and SSO doors. It is the existing ADR-0066 D4 key, read by one runtime check for MCP `list_actions`, MCP `run_action` and `POST /api/v1/actions/:object/:action`. A platform administrator holds both capabilities on either way of becoming one (the `admin_full_access` grant or `OS_PLATFORM_OWNER_EMAIL`), so nothing changes for them. + +What changes, before and now: + +- **MCP `list_actions`.** Before: every caller was listed all of these actions (a plain member saw `ban_user` and `set_user_manager`). Now: a caller without the capability is not listed them. +- **The refusal at `POST /actions/:object/:action` and MCP `run_action`.** Before: the door forwarded the call and the target answered 403 `PERMISSION_DENIED` "Admin role required". Now: nothing reaches the target. `POST /actions/...` answers 403 itself, with the capability sentence (`Action 'ban_user' on 'sys_user' requires capability [manage_users] — caller is missing [manage_users]`), and MCP `run_action` answers the same sentence as `PERMISSION_DENIED` with status 403, whether it is called by `actionName` alone or with `objectName`. +- **A caller whose only admin standing is the legacy better-auth `role === 'admin'` scalar** (no `admin_full_access` grant, not in `OS_PLATFORM_OWNER_EMAIL`). Before: the action doors forwarded the call and the target admitted it. Now: the action doors refuse it 403, because that caller holds no `manage_users` / `manage_platform_settings`. The target endpoints themselves (`/api/v1/auth/admin/*`) are unchanged and still admit it; give the account the `admin_full_access` permission set, or list its email in `OS_PLATFORM_OWNER_EMAIL`, to keep it on the action doors. + +**MCP `run_action` refusal codes (`@objectstack/runtime`, `@objectstack/mcp`).** The capability gate's refusal at MCP `run_action` used to be a plain message; it now carries `code: 'PERMISSION_DENIED'` and `status: 403`, the REST door's answer, for every action with `requiredPermissions`. With `objectName` on a `sys_*` object, the MCP tools admit an action only where `list_actions` lists it, and answered every action missing from the listing with the system-object sentence (`… is not exposed via MCP …`). An action that IS exposed but whose gate refuses the caller now gets the gate's refusal there too, through a new optional `McpActionBridge.capabilityRefusal(name, objectName)` the runtime bridge implements; an action not exposed to AI keeps the system-object sentence, and a bridge without the member behaves as before. + +What does not change: the target endpoints judge the platform-admin standing as before, the console shows and hides these buttons from their `visible` standing as before, and an action on another object is unaffected. diff --git a/packages/mcp/src/mcp-action-system-object-exposure.test.ts b/packages/mcp/src/mcp-action-system-object-exposure.test.ts index 81a83101d01..0520157aa85 100644 --- a/packages/mcp/src/mcp-action-system-object-exposure.test.ts +++ b/packages/mcp/src/mcp-action-system-object-exposure.test.ts @@ -131,3 +131,46 @@ describe('[#27681] run_action: a sys_* action runs only where list_actions lists expect(bridge.runs).toEqual(['complete_task']); }); }); + +describe('[#30771] run_action: a sys_* action left out for the caller\'s capability answers the gate, not the system-object sentence', () => { + const GATE = "Action 'ban_user' on 'sys_user' requires capability [manage_users] — caller is missing [manage_users]"; + /** The bridge's answer for an exposed action whose declared gate refuses this caller. */ + const withCapabilityRefusal = (listing: McpActionSummary[]) => { + const bridge = makeBridge(listing); + const asked: Array<[string, string]> = []; + return Object.assign(bridge, { + asked, + async capabilityRefusal(name: string, objectName: string) { + asked.push([name, objectName]); + return name === 'ban_user' ? { code: 'PERMISSION_DENIED', status: 403, message: GATE } : undefined; + }, + }); + }; + + it('answers the bridge\'s capability refusal (403, PERMISSION_DENIED, the gate\'s sentence) and runs nothing', async () => { + const bridge = withCapabilityRefusal([APP]); + const json = await call(bridge, toolsCall(7, 'run_action', { actionName: 'ban_user', objectName: SYS, recordId: 'u1' })); + expect(json.result.isError).toBe(true); + const body = JSON.parse(json.result.content[0].text); + expect(body.error).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, message: GATE }); + expect(String(json.result.content[0].text)).not.toMatch(/is not exposed via MCP/); + expect(bridge.asked).toEqual([['ban_user', SYS]]); + expect(bridge.runs).toEqual([]); + }); + + it('CONTROL — an action the bridge answers no capability refusal for keeps the system-object sentence', async () => { + const bridge = withCapabilityRefusal([APP]); + const json = await call(bridge, toolsCall(8, 'run_action', { actionName: 'unban_user', objectName: SYS, recordId: 'u1' })); + expect(json.result.isError).toBe(true); + expect(String(json.result.content[0].text)).toMatch(/system object "sys_user" is not exposed via MCP/); + expect(bridge.runs).toEqual([]); + }); + + it('CONTROL — a listed action never asks: the listing admits it as before', async () => { + const bridge = withCapabilityRefusal([EXPOSED]); + const json = await call(bridge, toolsCall(9, 'run_action', { actionName: 'ban_user', objectName: SYS, recordId: 'u1' })); + expect(json.result.isError, JSON.stringify(json.result)).toBeFalsy(); + expect(bridge.asked).toEqual([]); + expect(bridge.runs).toEqual(['ban_user']); + }); +}); diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index 755a5bc9018..32a2027192e 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -315,6 +315,18 @@ export interface McpActionBridge { name: string, input: { objectName?: string; recordId?: string; params?: Record } & AIActionConfirmation, ): Promise; + /** + * [#30771] The capability refusal `runAction` would throw for an action on + * `objectName` that {@link listActions} leaves out because the caller lacks + * its declared `requiredPermissions` — `{ code, status, message }`, the same + * values the by-name call throws. `undefined` for anything else (not + * declared, not exposed to AI), so `run_action`'s scoped `sys_*` pre-screen + * keeps its system-object refusal there. Runs nothing. + * + * OPTIONAL: a bridge without it keeps the system-object refusal for every + * `sys_*` action its listing leaves out, as before. + */ + capabilityRefusal?(name: string, objectName: string): Promise; /** * [#15705] Continue a run that `runAction` answered with `status: 'paused'` * and a `screen`, submitting that screen's field values as `values`. @@ -1783,7 +1795,15 @@ export function registerActionTools( const admitted = listed.some( (a) => a.name === actionName && a.objectName === objectName && isListableAction(a), ); - if (!admitted) return errorResult(systemObjectActionRefusal(actionName, objectName)); + if (!admitted) { + // [#30771] An exposed action the caller's declared gate refuses is + // left out of the listing too: answer that gate's refusal, the one + // the by-name call gets, not the system-object sentence. Nothing + // runs either way. + const refusal = await bridge.capabilityRefusal?.(actionName, objectName); + if (refusal) return errorResultWithCode(Object.assign(new Error(refusal.message), refusal)); + return errorResult(systemObjectActionRefusal(actionName, objectName)); + } } const result = await bridge.runAction(actionName, { objectName, diff --git a/packages/platform-objects/src/identity/platform-admin-action-gate.ts b/packages/platform-objects/src/identity/platform-admin-action-gate.ts new file mode 100644 index 00000000000..0dd52730aaf --- /dev/null +++ b/packages/platform-objects/src/identity/platform-admin-action-gate.ts @@ -0,0 +1,40 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The declared gate of a first-party action whose door runs the ADR-0068 + * platform-admin gate (#30771, ruling A on #30126 row 16). + * + * ## Why the action declares it + * + * Such a door answers every caller below the ADR-0095 D3 `PLATFORM_ADMIN` rung + * 403. Its action used to carry that fact only in `visible`, which is display + * only: no door reads it. So MCP `list_actions` listed `ban_user` and + * `set_user_manager` to a plain member, and the platform action door + * (`POST /actions/:object/:action`, MCP `run_action`) forwarded the call for the + * target to refuse. Declaring the gate in `requiredPermissions` (ADR-0066 D4, + * an existing key) puts it where both doors already look: the runtime's ONE + * reader, `actionPermissionError`, refuses the call with 403 at the door and + * leaves the action out of `list_actions`, from this one declaration. + * + * ## Why a capability stands for the rung + * + * The rung is not a key an action can name, but the rung implies these + * capabilities by construction: both anchors that confer it (the stored + * `admin_full_access` grant and the deployment's `OS_PLATFORM_OWNER_EMAIL` + * list) hand the holder `ADMIN_FULL_ACCESS_CAPABILITIES.systemPermissions`, + * and both names below are catalog `scope: 'platform'` capabilities that + * `organization_admin` withholds. So the declaration never hides the action from + * a caller its door admits on the rung, and the door still judges the rung + * itself: a holder of the capability without the rung is listed and refused by + * the target, as before. + * + * Pinned, for every action on such a door, by + * `platform-admin-affordance-standing.test.ts` (both halves: the `visible` + * standing and this declaration). + */ + +/** Doors that administer accounts and memberships across the platform. */ +export const PLATFORM_ADMIN_USERS_GATE: readonly string[] = Object.freeze(['manage_users']); + +/** Doors that administer platform-wide settings: OAuth applications, SSO providers. */ +export const PLATFORM_ADMIN_SETTINGS_GATE: readonly string[] = Object.freeze(['manage_platform_settings']); diff --git a/packages/platform-objects/src/identity/sys-member.object.ts b/packages/platform-objects/src/identity/sys-member.object.ts index fb6a89d34d0..ab0d3441c65 100644 --- a/packages/platform-objects/src/identity/sys-member.object.ts +++ b/packages/platform-objects/src/identity/sys-member.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PLATFORM_ADMIN_USERS_GATE } from './platform-admin-action-gate.js'; import { F } from '@objectstack/spec/shared'; import { BUILTIN_MEMBERSHIP_ROLE_OPTIONS, @@ -177,6 +178,8 @@ export const SysMember = ObjectSchema.create({ locations: ['list_toolbar'], type: 'api', target: '/api/v1/auth/organization/add-member', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Offered only to the one standing the door admits (ADR-0068 D4): // `current_user.isPlatformAdmin` is the ADR-0095 D3 PLATFORM_ADMIN // posture rung — what the session payload emits and what the mount's diff --git a/packages/platform-objects/src/identity/sys-oauth-application.object.ts b/packages/platform-objects/src/identity/sys-oauth-application.object.ts index 37a63b9fd65..4f653b1eec9 100644 --- a/packages/platform-objects/src/identity/sys-oauth-application.object.ts +++ b/packages/platform-objects/src/identity/sys-oauth-application.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PLATFORM_ADMIN_SETTINGS_GATE } from './platform-admin-action-gate.js'; /** * sys_oauth_application — Registered OAuth/OIDC client application @@ -98,6 +99,8 @@ export const SysOauthApplication = ObjectSchema.create({ type: 'api', method: 'POST', target: '/api/v1/auth/admin/oauth2/toggle-disabled', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_SETTINGS_GATE], requiresFeature: 'oidcProvider', // The confirm question rides `description`, not `confirmText`: this action // collects params, and the console action runner chains confirmation THEN @@ -131,6 +134,8 @@ export const SysOauthApplication = ObjectSchema.create({ type: 'api', method: 'POST', target: '/api/v1/auth/admin/oauth2/toggle-disabled', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_SETTINGS_GATE], requiresFeature: 'oidcProvider', // Confirm question on `description` — one dialog, not two (#7278/#7309). description: 'Re-enable this OAuth application? Token issuance, authorization, and introspection will resume immediately.', diff --git a/packages/platform-objects/src/identity/sys-sso-provider.object.ts b/packages/platform-objects/src/identity/sys-sso-provider.object.ts index e54390e673c..700701b40d7 100644 --- a/packages/platform-objects/src/identity/sys-sso-provider.object.ts +++ b/packages/platform-objects/src/identity/sys-sso-provider.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PLATFORM_ADMIN_SETTINGS_GATE } from './platform-admin-action-gate.js'; /** * sys_sso_provider — Registered external SSO identity provider (OIDC / SAML) @@ -101,6 +102,8 @@ export const SysSsoProvider = ObjectSchema.create({ // /sso/register would drop clientId/clientSecret (top-level → Zod-stripped) // and persist an unusable `oidc_config = null` provider. target: '/api/v1/auth/admin/sso/register', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_SETTINGS_GATE], // Platform-admin standing (ADR-0068 D4) — see the actions header above. visible: 'current_user.isPlatformAdmin == true', refreshAfter: true, @@ -143,6 +146,8 @@ export const SysSsoProvider = ObjectSchema.create({ // re-dispatches to /sso/register (admin gate runs). The response returns // the SP ACS + metadata URLs to configure on the IdP. target: '/api/v1/auth/admin/sso/register-saml', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_SETTINGS_GATE], // Platform-admin standing (ADR-0068 D4) — see the actions header above. visible: 'current_user.isPlatformAdmin == true', refreshAfter: true, @@ -175,6 +180,8 @@ export const SysSsoProvider = ObjectSchema.create({ // feature is OFF the bridge returns a clear "not enabled for this // environment" error instead of a bare 404. target: '/api/v1/auth/admin/sso/request-domain-verification', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_SETTINGS_GATE], // Platform-admin standing (ADR-0068 D4) — see the actions header above. visible: 'current_user.isPlatformAdmin == true', params: [ @@ -211,6 +218,8 @@ export const SysSsoProvider = ObjectSchema.create({ // on success. Routed through the env bridge, which maps @better-auth/sso's // empty 204 / 502 into a clear success/error toast. target: '/api/v1/auth/admin/sso/verify-domain', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_SETTINGS_GATE], // Platform-admin standing (ADR-0068 D4) — see the actions header above. visible: 'current_user.isPlatformAdmin == true', successMessage: 'Domain ownership verified', diff --git a/packages/platform-objects/src/identity/sys-user.object.ts b/packages/platform-objects/src/identity/sys-user.object.ts index 3b82d6a2c1e..dfbd4f3fe75 100644 --- a/packages/platform-objects/src/identity/sys-user.object.ts +++ b/packages/platform-objects/src/identity/sys-user.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PLATFORM_ADMIN_USERS_GATE } from './platform-admin-action-gate.js'; import { INVITATION_ROLE_OPTIONS } from '@objectstack/spec/identity'; import { INVITE_USER_OUTCOME_MESSAGES } from './invite-outcome-messages.js'; import { MEMBERSHIP_ROLE_PARAM_HELP } from './membership-role-help.js'; @@ -191,6 +192,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_item', 'record_header'], type: 'api', target: '/api/v1/auth/admin/ban-user', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Platform-admin standing (ADR-0068 D4), then a user not banned yet — // see the block header above. A null `banned` reads as not banned. visible: 'current_user.isPlatformAdmin == true && has(record.banned) && record.banned != true', @@ -228,6 +231,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_item', 'record_header'], type: 'api', target: '/api/v1/auth/admin/unban-user', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Platform-admin standing (ADR-0068 D4), then a banned user — see the // block header above. visible: 'current_user.isPlatformAdmin == true && has(record.banned) && record.banned == true', @@ -255,6 +260,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_item', 'record_header'], type: 'api', target: '/api/v1/auth/admin/unlock-user', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Platform-admin standing (ADR-0068 D4), then an account that can hold // a lock. `unlockUser` clears TWO stages: this row's `locked_until` // (the password check) and `sys_two_factor.locked_until` (the second @@ -294,6 +301,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_toolbar'], type: 'api', target: '/api/v1/auth/admin/create-user', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Platform-admin standing (ADR-0068 D4) — see the block header above — // and hidden while cloud-managed (`features.cloudManaged`, hide-when-on): // a managed environment's users come from the identity provider. @@ -407,6 +416,8 @@ export const SysUser = ObjectSchema.create({ // legacy role scalar), can mint a temporary password, and stamps // must_change_password. target: '/api/v1/auth/admin/set-user-password', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Platform-admin standing (ADR-0068 D4) — see the block header above — // and hidden while cloud-managed (`features.cloudManaged`, hide-when-on). // Console only: the server keeps answering it on an `env_native` row, so @@ -472,6 +483,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_item', 'record_header'], type: 'api', target: '/api/v1/auth/admin/impersonate-user', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], // Platform-admin standing (ADR-0068 D4) — see the block header above. visible: 'current_user.isPlatformAdmin == true', requiresFeature: 'admin', @@ -522,6 +535,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_item', 'record_header'], type: 'api', target: '/api/v1/auth/admin/set-user-manager', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], recordIdParam: 'userId', successMessage: 'Manager updated', refreshAfter: true, @@ -601,6 +616,8 @@ export const SysUser = ObjectSchema.create({ locations: ['list_toolbar'], type: 'api', target: '/api/v1/auth/admin/list-platform-admins', + // The door's platform-admin gate, declared where both action doors read it (#30771). + requiredPermissions: [...PLATFORM_ADMIN_USERS_GATE], visible: 'current_user.isPlatformAdmin == true', // A read: the Users list behind the dialog is not reloaded. refreshAfter: false, diff --git a/packages/platform-objects/src/platform-admin-affordance-standing.test.ts b/packages/platform-objects/src/platform-admin-affordance-standing.test.ts index c3c207205a1..e4e1239ce14 100644 --- a/packages/platform-objects/src/platform-admin-affordance-standing.test.ts +++ b/packages/platform-objects/src/platform-admin-affordance-standing.test.ts @@ -65,6 +65,8 @@ import { readdirSync, statSync } from 'node:fs'; import { join, relative, resolve } from 'node:path'; import { beforeAll, describe, expect, it } from 'vitest'; import { celEngine } from '@objectstack/formula'; +import { ADMIN_FULL_ACCESS_CAPABILITIES } from '@objectstack/spec/identity'; +import { PLATFORM_CAPABILITIES } from '@objectstack/spec/security'; /** The standing, spelled as the ruling fixed it. */ const STANDING = 'current_user.isPlatformAdmin == true'; @@ -128,6 +130,8 @@ interface FirstPartyAction { target: string; /** The served (parsed) predicate's CEL source, or undefined when none is declared. */ source: string | undefined; + /** The declared ADR-0066 D4 gate both action doors read, or undefined when none is declared. */ + requiredPermissions: unknown; } function sourceOf(raw: unknown): string | undefined { @@ -156,6 +160,7 @@ async function loadActions(): Promise<{ files: number; actions: FirstPartyAction file: relative(REPO_ROOT, file), target: action.target, source: sourceOf(action.visible), + requiredPermissions: action.requiredPermissions, }); } } @@ -300,6 +305,37 @@ describe('platform-admin-gated doors: every first-party action offers itself to expect(leaks).toEqual([]); }); + it('each member declares its gate where both action doors read it: a platform capability the rung always carries (#30771)', () => { + // `visible` is display only, so the doors never read it. The gate both the + // platform action door (`POST /actions/...`, MCP `run_action`) and MCP + // `list_actions` read is `requiredPermissions`, through the runtime's one + // reader `actionPermissionError`. Each name must be one the PLATFORM_ADMIN + // rung carries on both of its anchors (`ADMIN_FULL_ACCESS_CAPABILITIES`) and + // a catalog `scope: 'platform'` capability, so the declaration never hides + // an action from a caller its door admits on the rung. + const carried = new Set(ADMIN_FULL_ACCESS_CAPABILITIES.systemPermissions ?? []); + const platformScoped = new Set(PLATFORM_CAPABILITIES.filter((c) => c.scope === 'platform').map((c) => c.name)); + // Positive control: the two names the declarations use are both. + for (const name of ['manage_users', 'manage_platform_settings']) { + expect(carried.has(name), name).toBe(true); + expect(platformScoped.has(name), name).toBe(true); + } + const wrong: string[] = []; + for (const a of family) { + const declared = a.requiredPermissions; + if (!Array.isArray(declared) || declared.length === 0) { + wrong.push(`${a.site} → ${a.target} (${a.file}) declares no requiredPermissions`); + continue; + } + for (const name of declared) { + if (typeof name !== 'string' || !carried.has(name) || !platformScoped.has(name)) { + wrong.push(`${a.site} declares ${JSON.stringify(name)}, which the rung does not always carry as a platform capability`); + } + } + } + expect(wrong, 'spread PLATFORM_ADMIN_USERS_GATE or PLATFORM_ADMIN_SETTINGS_GATE (identity/platform-admin-action-gate.ts)').toEqual([]); + }); + it('the platform admin is still offered every member — the standing is a gate, not an always-false wrapper', () => { const hidden = family .filter((a) => a.source !== undefined) diff --git a/packages/qa/dogfood/test/api-action-door-forward.dogfood.test.ts b/packages/qa/dogfood/test/api-action-door-forward.dogfood.test.ts index 2282ffafaa4..b344ffff89b 100644 --- a/packages/qa/dogfood/test/api-action-door-forward.dogfood.test.ts +++ b/packages/qa/dogfood/test/api-action-door-forward.dogfood.test.ts @@ -231,10 +231,14 @@ describe('[#27506] a same-origin api action is dispatched through the action doo await expect(stack.signIn(TARGET_EMAIL, TARGET_PASS)).rejects.toThrow(/signIn failed/); }, 120_000); - it('the target still authorizes: a plain member is refused by ban-user\'s own 403; anonymous meets the 401 floor', async () => { + it('a plain member is refused 403 at the door by ban_user\'s declared gate (#30771); anonymous meets the 401 floor', async () => { const member = await stack.signUp(MEMBER_EMAIL, 'Member-Pass-123'); const refused = await call({ token: member }, 'POST', '/actions/sys_user/ban_user', { recordId: targetUserId }); expect(refused.status, refused.text).toBe(403); + // The door's own sentence (`requiredPermissions`, ADR-0066 D4), so ban-user + // is never called; the target still judges the rung for every caller the + // door admits (the admin cases above). + expect(String(refused.json?.error?.message)).toContain('requires capability [manage_users]'); expect(await isBanned()).toBe(false); const anonymous = await call(null, 'POST', '/actions/sys_user/ban_user', { recordId: targetUserId }); diff --git a/packages/qa/dogfood/test/mcp-system-object-action-exposure.dogfood.test.ts b/packages/qa/dogfood/test/mcp-system-object-action-exposure.dogfood.test.ts index 2224c4a2026..b155b05e5ce 100644 --- a/packages/qa/dogfood/test/mcp-system-object-action-exposure.dogfood.test.ts +++ b/packages/qa/dogfood/test/mcp-system-object-action-exposure.dogfood.test.ts @@ -40,9 +40,10 @@ // until #28367 the agent's forward was refused by name (#28178). Now the // forward carries a credential the server mints for that one forward, and the // identity mount shows better-auth a request-scoped view of the delegating -// user: the agent's E call and its confirmed C call are SERVED, and a plain -// member's agent is refused by the identity route itself (the intersection at -// the target). ⛔ No session is minted for the agent, and its token never +// user: the agent's E call and its confirmed C call are SERVED. A plain +// member's agent no longer reaches the target for `ban_user`: the action +// declares its gate (#30771), so it is not listed and the door refuses it. +// ⛔ No session is minted for the agent, and its token never // reaches the target. // // `OS_SCIM_ENABLED=true` precedes the boot because `/admin/*` 501s without @@ -315,16 +316,41 @@ describe('[#27681, #27900] sys_user identity actions over MCP: what each bucket\ expect((await userRow(adminUserId))?.name).toBe('Renamed By Session'); }, 120_000); - it('the intersection at the target: a plain member\'s agent, confirmed, passes the action door and is refused by the identity route itself; nobody is banned', async () => { - const refused = await tool(memberAgent, 'run_action', { + it('a plain member\'s agent is not listed ban_user and is refused at the door, scoped or by name; nobody is banned (#30771)', async () => { + // `ban_user` declares its gate (`requiredPermissions: ['manage_users']`), + // which the member's agent does not hold, so `list_actions` leaves it out. + // Before #30771 it was listed, passed the action door and was refused by + // the identity route itself. + const onUser = await listedOnUser(memberAgent); + expect(onUser.map((a) => a.name)).not.toContain('ban_user'); + + // Scoped (objectName) and by name answer the same refusal: the declared + // gate's sentence with the REST door's 403. The scoped call is not told the + // action is unexposed, because it is exposed; the caller's grant is why. + const scoped = await tool(memberAgent, 'run_action', { actionName: 'ban_user', objectName: 'sys_user', recordId: agentTargetUserId, params: { banReason: 'member agent' }, confirm: true, }); - expect(refused.isError, refused.text).toBe(true); - expect(refused.payload?.error?.status, refused.text).toBe(403); - expect(messageOf(refused)).not.toMatch(/OAuth access token|scope/); + const byName = await tool(memberAgent, 'run_action', { + actionName: 'ban_user', recordId: agentTargetUserId, params: { banReason: 'member agent' }, confirm: true, + }); + for (const refused of [scoped, byName]) { + expect(refused.isError, refused.text).toBe(true); + expect(refused.payload?.error, refused.text).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(messageOf(refused)).toContain('requires capability [manage_users]'); + expect(messageOf(refused)).not.toContain('is not exposed via MCP'); + } + expect(messageOf(scoped)).toBe(messageOf(byName)); expect(await isBanned(agentTargetUserId)).toBe(false); }, 120_000); + it('CONTROL: an action genuinely not exposed to AI keeps the system-object sentence for the member\'s agent (#30771)', async () => { + // `impersonate_user` declares a gate too, but no `ai` block: its absence + // from the listing is exposure, so the sentence stays. + const scoped = await tool(memberAgent, 'run_action', { actionName: 'impersonate_user', objectName: 'sys_user', recordId: agentTargetUserId }); + expect(scoped.isError).toBe(true); + expect(messageOf(scoped)).toContain('system object "sys_user" is not exposed via MCP'); + }, 120_000); + it('C: the agent\'s unconfirmed ban_user is refused 428 before any forward; confirmed, it is served as the agent (#28367) and bans the user', async () => { const unconfirmed = await tool(agent, 'run_action', { actionName: 'ban_user', objectName: 'sys_user', recordId: agentTargetUserId, params: { banReason: 'agent' }, diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 202ba5a1d00..78a1f1a40a0 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -983,6 +983,30 @@ export function actionPermissionError(_deps: ActionExecutionDeps, actionDef: any ); } +/** + * [#30771] {@link actionPermissionError} as the ADR-0112 refusal the MCP + * `run_action` door throws: `403`, the standard code for it, and the gate's + * own sentence — the status `POST /actions/...` answers for the same + * declaration. `null` when the gate admits the caller. One reading of one + * declaration, so the by-name call and the MCP tool layer's scoped pre-screen + * (`McpActionBridge.capabilityRefusal`) answer a gated action alike. + */ +export interface ActionPermissionRefusal { + code: string; + status: 403; + message: string; +} + +export function actionPermissionRefusal( + deps: ActionExecutionDeps, + actionDef: any, + ec: any, + objectName?: string, +): ActionPermissionRefusal | null { + const message = actionPermissionError(deps, actionDef, ec, objectName); + return message ? { code: standardErrorCodeForHttpStatus(403), status: 403, message } : null; +} + /** * [ADR-0126 §8 item 2] The activation refusal a DISABLED packaged action is * answered with — `409 ACTION_DISABLED`. @@ -4420,9 +4444,12 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps, const exposureError = actionAiExposureError(deps, action, objectName); if (exposureError) throw new Error(exposureError); - // ADR-0066 D4 capability gate — same declaration the REST route enforces. - const gateError = actionPermissionError(deps, action, ec, objectName); - if (gateError) throw new Error(gateError); + // ADR-0066 D4 capability gate — same declaration the REST route enforces, + // thrown with the REST door's `403` and its standard code (#30771). + const gateRefusal = actionPermissionRefusal(deps, action, ec, objectName); + if (gateRefusal) { + throw Object.assign(new Error(gateRefusal.message), { code: gateRefusal.code, status: gateRefusal.status }); + } // [ADR-0126 §8 item 2] ACTIVATION CONSULT — door 2 of 2. A packaged action // the installation switched off is refused here, before the param contract diff --git a/packages/runtime/src/domains/api-action-create-user-email-param.test.ts b/packages/runtime/src/domains/api-action-create-user-email-param.test.ts index 23a9f7a97cf..2e66f945ec5 100644 --- a/packages/runtime/src/domains/api-action-create-user-email-param.test.ts +++ b/packages/runtime/src/domains/api-action-create-user-email-param.test.ts @@ -24,10 +24,13 @@ // sign-in off, pinned in plugin-auth), and the doors relay its answer whole. import { describe, it, expect, vi } from 'vitest'; +// Paid at collection, not inside a clocked case: the dispatcher's reach dynamic-imports it (check:test-source-alias). +import '@objectstack/metadata'; import { HttpDispatcher } from '../http-dispatcher.js'; import type { HttpProtocolContext } from '../http-dispatcher.js'; import { SysUser } from '@objectstack/platform-objects/identity'; +import { ADMIN_FULL_ACCESS_CAPABILITIES } from '@objectstack/spec/identity'; import { ActionActivationProjection } from '@objectstack/objectql'; const OBJECT = 'sys_user'; @@ -79,7 +82,9 @@ const CTX = (): HttpProtocolContext => ({ }, }, environmentId: 'platform', - executionContext: { userId: 'u_admin', tenantId: 'org_1', systemPermissions: [] }, + // [#30771] The capabilities the platform-admin rung carries: `create_user` + // declares `requiredPermissions: ['manage_users']`, which both doors check. + executionContext: { userId: 'u_admin', tenantId: 'org_1', systemPermissions: [...(ADMIN_FULL_ACCESS_CAPABILITIES.systemPermissions ?? [])] }, } as unknown as HttpProtocolContext); const rest = (h: ReturnType, params: Record) => diff --git a/packages/runtime/src/domains/mcp.ts b/packages/runtime/src/domains/mcp.ts index abf84dcdb49..065eaed0be7 100644 --- a/packages/runtime/src/domains/mcp.ts +++ b/packages/runtime/src/domains/mcp.ts @@ -908,6 +908,23 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon } return out; }, + // [#30771] Why the scoped `run_action` pre-screen (`@objectstack/mcp`) + // finds a `sys_*` action missing from this caller's listing when the + // reason is the caller's capability: an action its author exposed whose + // declared gate refuses this caller answers the gate's own refusal, the + // one `runAction` throws by name. Anything else (not declared there, not + // exposed) answers `undefined`, so the tool keeps its system-object + // sentence. Reads the declarations `listActions` reads, runs nothing. + capabilityRefusal: async (name: string, objectName: string) => { + const declarations = await actionExec.collectActionDeclarations( + deps, await getMeta(), await readServedObjects(), await readServedActions(), + ); + const found = declarations.find((d) => d.objectName === objectName && d.action?.name === name); + if (!found) return undefined; + if (actionExec.systemObjectActionExposureError(deps, found.action, objectName)) return undefined; + if (actionExec.actionAiExposureError(deps, found.action)) return undefined; + return actionExec.actionPermissionRefusal(deps, found.action, ec, objectName) ?? undefined; + }, // [#15942] `confirm` rides through UNTOUCHED. The bridge forwards the // whole request object, so widening the type here is the whole change: // the MCP door grew the member in the same change that enforces it, and diff --git a/packages/runtime/src/platform-admin-action-gate.test.ts b/packages/runtime/src/platform-admin-action-gate.test.ts new file mode 100644 index 00000000000..b5109dcca6a --- /dev/null +++ b/packages/runtime/src/platform-admin-action-gate.test.ts @@ -0,0 +1,210 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30771 · ruling A on #30126 row 16] A platform-admin action declares its + * gate once, and both action doors read that one declaration. + * + * Before: `ban_user`, `set_user_manager` and the rest of the platform-admin + * family carried their gate only in `visible` (display only), so MCP + * `list_actions` listed them to a plain member and the platform action door + * forwarded the call for the target to refuse. Now each declares + * `requiredPermissions` (ADR-0066 D4), and the runtime's one reader, + * `actionPermissionError`, answers both doors from it. + * + * Driven over the REAL `sys_user` declaration (`@objectstack/platform-objects`) + * through the one production `McpActionBridge` and the REST + * `POST /actions/:object/:action` door, with the forward target faked so the + * test can see whether the call ever left the door. + * + * What it pins: + * - a plain member is not listed `ban_user` / `set_user_manager`; a platform + * admin (the capabilities the rung carries) is; + * - a plain member's call is refused 403 at both doors and never forwarded; + * an admin's is forwarded, so the target still judges the rung as before; + * - the listing and both run doors agree, action by action. + */ + +import { describe, it, expect, vi } from 'vitest'; +// Paid at collection, not inside a clocked case: the dispatcher's reach dynamic-imports it (check:test-source-alias). +import '@objectstack/metadata'; + +import { SysUser } from '@objectstack/platform-objects/identity'; +import { ADMIN_FULL_ACCESS_CAPABILITIES } from '@objectstack/spec/identity'; + +import { HttpDispatcher } from './http-dispatcher.js'; +import type { HttpProtocolContext } from './http-dispatcher.js'; +import { actionPermissionError } from './action-execution.js'; + +const OBJECT = 'sys_user'; +const ADMIN_CAPS: string[] = [...(ADMIN_FULL_ACCESS_CAPABILITIES.systemPermissions ?? [])]; + +function boot() { + const request = vi.fn(async () => new Response(JSON.stringify({ success: true, data: { ok: true } }), { + status: 200, headers: [['content-type', 'application/json']], + })); + const server = { getRawApp: () => ({ request }) }; + const served: Record = { [OBJECT]: SysUser }; + const ql: any = { + executeAction: vi.fn(async () => ({ ran: 'script' })), + listRegisteredActions: () => [], + getSchema: (name: string) => served[name], + registry: { getObject: (name: string) => served[name], getItem: () => undefined }, + find: vi.fn(async (_object: string, q: any) => + q?.where?.id === 'u_target' ? [{ id: 'u_target', email: 'target@example.test', banned: false, source: 'local' }] : []), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + isActionEnabled: () => true, + }; + const metadata: any = { + load: vi.fn(async () => null), + loadDiagnosed: vi.fn(async () => ({ data: null, degraded: false, errors: [] })), + loadMany: vi.fn(async () => []), + listObjects: vi.fn(async () => Object.values(served)), + getObject: vi.fn(async (name?: string) => (name ? served[name] : undefined)), + }; + const resolve = (n: string) => + n === 'objectql' || n === 'data' ? ql + : n === 'metadata' ? metadata + : n === 'http.server' || n === 'http-server' ? server + : null; + const kernel: any = { getService: resolve, getServiceAsync: async (n: string) => resolve(n), context: { getService: resolve } }; + return { dispatcher: new HttpDispatcher(kernel) as any, request }; +} + +/** A signed-in human holding `caps`, over a session cookie. */ +const CTX = (userId: string, caps: string[]): HttpProtocolContext => ({ + request: { + headers: { + host: 'app.example.test', + 'x-forwarded-proto': 'https', + cookie: 'better-auth.session_token=abc', + 'content-type': 'application/json', + }, + }, + environmentId: 'platform', + executionContext: { userId, tenantId: 'org_1', permissions: [], systemPermissions: caps, principalKind: 'human' }, +} as unknown as HttpProtocolContext); + +const MEMBER = () => CTX('u_member', []); +const ADMIN = () => CTX('u_admin', ADMIN_CAPS); + +async function listedNames(ctx: HttpProtocolContext): Promise { + const { dispatcher } = boot(); + const listed: any[] = await dispatcher.buildMcpBridge(ctx).listActions(); + return listed.filter((a) => a.objectName === OBJECT).map((a) => a.name); +} + +async function refusalOf(p: Promise): Promise { + try { + await p; + } catch (e) { + return e; + } + throw new Error('expected a refusal, the call ran'); +} + +const PINNED = ['ban_user', 'set_user_manager'] as const; + +/** The params each pinned action declares, so a refusal is the gate's and not the param contract's. */ +const PARAMS: Record<(typeof PINNED)[number], Record> = { + ban_user: { banReason: 'spam' }, + set_user_manager: { managerId: 'u_admin' }, +}; + +describe('[#30771] the declaration: the platform-admin actions on sys_user declare their gate', () => { + it('ban_user and set_user_manager declare a capability the rung carries (positive control for every block below)', () => { + for (const name of PINNED) { + const action = (SysUser as any).actions.find((a: any) => a.name === name); + expect(action, name).toBeDefined(); + expect(action.requiredPermissions, name).toEqual(['manage_users']); + expect(ADMIN_CAPS).toContain('manage_users'); + } + }); +}); + +describe('[#30771] MCP list_actions lists only what the caller can run', () => { + it('a plain member is not listed ban_user or set_user_manager', async () => { + const names = await listedNames(MEMBER()); + // Control: the listing is live for this caller (an ungated identity action is still there). + expect(names.length).toBeGreaterThan(0); + for (const name of PINNED) expect(names, name).not.toContain(name); + }); + + it('a platform admin (the capabilities the rung carries) is listed both', async () => { + const names = await listedNames(ADMIN()); + for (const name of PINNED) expect(names, name).toContain(name); + }); +}); + +describe('[#30771] a hidden action is still refused when called', () => { + for (const name of PINNED) { + it(`REST POST /actions/sys_user/${name}: the member is refused 403 at the door, nothing forwarded; the admin is forwarded`, async () => { + const member = boot(); + const refused = await member.dispatcher.handleActions(`/${OBJECT}/${name}`, 'POST', { recordId: 'u_target', params: PARAMS[name] }, MEMBER()); + expect(refused?.response?.status).toBe(403); + expect(String(refused?.response?.body?.error?.message)).toContain('requires capability [manage_users]'); + expect(member.request).not.toHaveBeenCalled(); + + const admin = boot(); + await admin.dispatcher.handleActions(`/${OBJECT}/${name}`, 'POST', { recordId: 'u_target', params: PARAMS[name] }, ADMIN()); + // The door no longer answers for the admin: the target judges the rung, as before. + expect(admin.request).toHaveBeenCalledTimes(1); + }); + + it(`MCP run_action ${name}: the member is refused with the same gate, nothing forwarded`, async () => { + const { dispatcher, request } = boot(); + const err = await refusalOf(dispatcher.buildMcpBridge(MEMBER()).runAction(name, { + objectName: OBJECT, recordId: 'u_target', params: PARAMS[name], + })); + expect(String(err?.message)).toContain('requires capability [manage_users]'); + // The REST door's status and its standard code (ADR-0112). + expect(err).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(request).not.toHaveBeenCalled(); + }); + } +}); + +describe('[#30771] capabilityRefusal: why the scoped MCP pre-screen finds an action missing', () => { + it('answers the gate\'s refusal for an exposed action the member cannot run — the one run_action throws by name', async () => { + const { dispatcher, request } = boot(); + const bridge = dispatcher.buildMcpBridge(MEMBER()); + for (const name of PINNED) { + const refusal = await bridge.capabilityRefusal(name, OBJECT); + expect(refusal, name).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + const thrown = await refusalOf(bridge.runAction(name, { recordId: 'u_target', params: PARAMS[name] })); + expect(refusal.message, name).toBe(thrown.message); + } + expect(request).not.toHaveBeenCalled(); + }); + + it('answers nothing for the admin, for an action not exposed to AI, and for one not declared there', async () => { + const { dispatcher } = boot(); + expect(await dispatcher.buildMcpBridge(ADMIN()).capabilityRefusal('ban_user', OBJECT)).toBeUndefined(); + const member = dispatcher.buildMcpBridge(MEMBER()); + // CONTROL: `impersonate_user` declares a gate but no `ai` block, so its + // absence from the listing is exposure, and the tool keeps the + // system-object sentence for it. + const impersonate = (SysUser as any).actions.find((a: any) => a.name === 'impersonate_user'); + expect(impersonate.requiredPermissions).toEqual(['manage_users']); + expect(impersonate.ai?.exposed).not.toBe(true); + expect(await member.capabilityRefusal('impersonate_user', OBJECT)).toBeUndefined(); + expect(await member.capabilityRefusal('no_such_action', OBJECT)).toBeUndefined(); + }); +}); + +describe('[#30771] the listing and the run doors read one declaration', () => { + it('for every sys_user action and both callers, list_actions omits it exactly when the shared gate refuses it', async () => { + const deps: any = {}; + for (const [who, ctx] of [['member', MEMBER()], ['admin', ADMIN()]] as const) { + const names = new Set(await listedNames(ctx)); + for (const action of (SysUser as any).actions as any[]) { + const gated = actionPermissionError(deps, action, (ctx as any).executionContext, OBJECT) !== null; + if (gated) expect(names.has(action.name), `${who}: ${action.name} is gated but listed`).toBe(false); + } + // Every listed action passes the gate that the REST door runs first. + for (const name of names) { + const action = (SysUser as any).actions.find((a: any) => a.name === name); + expect(actionPermissionError(deps, action, (ctx as any).executionContext, OBJECT), `${who}: ${name}`).toBeNull(); + } + } + }); +});