From dc438352e5d34b8c5353606d4e8e302c8303be11 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 17:30:09 +0000 Subject: [PATCH 1/8] harden(data): changing an API key through the data door requires a signed-in session A write to sys_api_key through the generic data doors (RestServer and the runtime dispatcher's data faces) from a request an API key authenticated is refused 403 PERMISSION_DENIED; reads, session callers and system-context writes are unchanged. One rule in @objectstack/core, asked at each door's per-object gate. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../api-key-management-credential.test.ts | 44 ++++ .../security/api-key-management-credential.ts | 86 ++++++++ packages/core/src/security/index.ts | 9 + ...data-door-session-required.dogfood.test.ts | 201 ++++++++++++++++++ packages/rest/src/rest-server.ts | 39 +++- packages/runtime/src/action-execution.ts | 18 ++ ...data-api-key-management-credential.test.ts | 120 +++++++++++ 7 files changed, 514 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/security/api-key-management-credential.test.ts create mode 100644 packages/core/src/security/api-key-management-credential.ts create mode 100644 packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts create mode 100644 packages/runtime/src/domains/data-api-key-management-credential.test.ts diff --git a/packages/core/src/security/api-key-management-credential.test.ts b/packages/core/src/security/api-key-management-credential.test.ts new file mode 100644 index 00000000000..1e35908204c --- /dev/null +++ b/packages/core/src/security/api-key-management-credential.test.ts @@ -0,0 +1,44 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + apiKeyManagementCredentialRefusal, + API_KEY_MANAGEMENT_READ_OPERATIONS, +} from './api-key-management-credential.js'; + +/** + * The one rule both data doors ask: a write to `sys_api_key` from a principal + * an API key admitted is refused; everything else keeps today's answer. + */ +describe('apiKeyManagementCredentialRefusal', () => { + const WRITES = ['create', 'update', 'delete', 'bulk', 'import']; + + it.each(WRITES)('refuses `%s` on sys_api_key from an API-key-admitted caller, on the ADR-0112 envelope', (operation) => { + const refusal = apiKeyManagementCredentialRefusal({ object: 'sys_api_key', operation, credential: 'api_key' }); + expect(refusal).toBeInstanceOf(Error); + expect(refusal).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, statusCode: 403, object: 'sys_api_key' }); + }); + + it('refuses a write verb it has never heard of: only the declared reads pass', () => { + const refusal = apiKeyManagementCredentialRefusal({ object: 'sys_api_key', operation: 'upsert', credential: 'api_key' }); + expect(refusal).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + }); + + it.each([...API_KEY_MANAGEMENT_READ_OPERATIONS])('leaves the read `%s` alone', (operation) => { + expect(apiKeyManagementCredentialRefusal({ object: 'sys_api_key', operation, credential: 'api_key' })).toBeUndefined(); + }); + + it.each(WRITES)('leaves `%s` alone for a session-admitted caller', (operation) => { + expect(apiKeyManagementCredentialRefusal({ object: 'sys_api_key', operation, credential: 'session' })).toBeUndefined(); + }); + + it('leaves a caller whose door was told no credential alone (positive rule)', () => { + expect(apiKeyManagementCredentialRefusal({ object: 'sys_api_key', operation: 'update', credential: undefined })).toBeUndefined(); + }); + + it('leaves every other object alone for an API-key-admitted caller', () => { + for (const object of ['crm_account', 'sys_user', 'sys_session', undefined, '']) { + expect(apiKeyManagementCredentialRefusal({ object, operation: 'update', credential: 'api_key' })).toBeUndefined(); + } + }); +}); diff --git a/packages/core/src/security/api-key-management-credential.ts b/packages/core/src/security/api-key-management-credential.ts new file mode 100644 index 00000000000..25d0ae2970f --- /dev/null +++ b/packages/core/src/security/api-key-management-credential.ts @@ -0,0 +1,86 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Managing API keys is a signed-in-session act, on every data door. + * + * An API key acts as its owner for data, automation and MCP. Managing the + * owner's credentials is not part of that: `POST /api/v1/keys` takes a mint + * only from a principal a signed-in session admitted, and this module is the + * same answer for the generic data doors that reach the `sys_api_key` rows + * themselves (`PATCH /api/v1/data/sys_api_key/:id` and every sibling route + * that writes through the per-object gate). A write to `sys_api_key` from a + * principal an API key admitted is refused `403 PERMISSION_DENIED`, and + * nothing is written; reads are unchanged. + * + * ## Why at the transport and not in an ObjectQL hook + * + * Which credential admitted the caller is decided once, by the shared resolver + * (`resolveAuthzContext`'s `credential`), and handed to the door that called + * it (`resolveRequestExecutionContext`'s `onAdmission`). The + * `ExecutionContext` envelope does not carry it, so an ObjectQL hook cannot + * ask it. Each door asks this function at its per-object gate, the one every + * data route on that door passes: the runtime dispatcher's + * `refuseUnexposedDataAction` and `RestServer.enforceApiAccess`. + * + * ## The rule + * + * - Positive on the credential: refused only when the door was told an API + * key admitted the caller. A session caller, and a caller whose door was + * told nothing (an embedder handing the dispatcher its own context), keep + * today's answer. + * - Every operation that is not a read is refused, so a write verb added to + * a door later is covered from the day it lands. + * - A system-context write never reaches a door's gate as a caller, and the + * gates skip the system context before asking: the mint, the + * `last_used_at` stamp and the forced reset's revoke are untouched. + */ + +import { SystemObjectName } from '@objectstack/spec/system'; + +/** + * The data-door operations that only read. Every other operation on + * `sys_api_key` (`create`, `update`, `delete`, `bulk`, `import`, and any verb a + * door adds later) is a write for this rule. + */ +export const API_KEY_MANAGEMENT_READ_OPERATIONS: ReadonlySet = new Set([ + 'get', + 'list', + 'query', + 'find', + 'aggregate', + 'history', + 'export', +]); + +/** The located refusal: names the road (sign in, the API Keys page) and nothing about the key presented. */ +export const API_KEY_MANAGEMENT_SESSION_REQUIRED_MESSAGE = + 'PERMISSION_DENIED: Changing an API key requires a signed-in session. Sign in and manage the key from the ' + + 'API Keys page; a request authenticated with an API key cannot revoke, restore or change API keys.'; + +/** What a door hands the rule: the object addressed, the door's operation name, and the admitting credential. */ +export interface ApiKeyManagementCredentialInput { + object: unknown; + operation: string; + /** The credential the door's `onAdmission` was told, or `undefined` when it was told none. */ + credential: 'api_key' | 'session' | undefined; +} + +/** + * The refusal for a `sys_api_key` write from a principal an API key admitted, + * or `undefined` when the rule does not apply. Thrown by the door, it renders + * on the envelope the data doors already map (`code`, `status`, `object`), + * the one the `sys_api_key` restore guard's refusal uses. + */ +export function apiKeyManagementCredentialRefusal( + input: ApiKeyManagementCredentialInput, +): (Error & { code: 'PERMISSION_DENIED'; status: 403; statusCode: 403; object: string }) | undefined { + if (input.credential !== 'api_key') return undefined; + if (input.object !== SystemObjectName.API_KEY) return undefined; + if (API_KEY_MANAGEMENT_READ_OPERATIONS.has(input.operation)) return undefined; + return Object.assign(new Error(API_KEY_MANAGEMENT_SESSION_REQUIRED_MESSAGE), { + code: 'PERMISSION_DENIED' as const, + status: 403 as const, + statusCode: 403 as const, + object: SystemObjectName.API_KEY as string, + }); +} diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index 08759adf3ce..d4cbc82e65d 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -364,6 +364,15 @@ export { type UserAuthGateSource, } from './auth-gate.js'; +// Managing API keys is a signed-in-session act on every data door: the one +// rule the runtime dispatcher's and RestServer's per-object gates ask. +export { + apiKeyManagementCredentialRefusal, + API_KEY_MANAGEMENT_READ_OPERATIONS, + API_KEY_MANAGEMENT_SESSION_REQUIRED_MESSAGE, + type ApiKeyManagementCredentialInput, +} from './api-key-management-credential.js'; + // #2567 — the single anonymous-deny decision shared by every HTTP seam. export { shouldDenyAnonymous, diff --git a/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts b/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts new file mode 100644 index 00000000000..61c8febd8d5 --- /dev/null +++ b/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts @@ -0,0 +1,201 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Changing an API key through the generic data door requires a signed-in + * session, as minting one through `/api/v1/keys` does. + * + * An API key acts as its owner for data, automation and MCP; managing the + * owner's keys stays with the owner's own sign-in. The `sys_api_key` rows are + * reachable through `/api/v1/data/sys_api_key`, so that door gives the same + * answer: a write to `sys_api_key` from a request an API key authenticated is + * refused `403 PERMISSION_DENIED`, nothing is written, and reads are + * unchanged. The rule is `@objectstack/core`'s + * `apiKeyManagementCredentialRefusal`, asked at each data door's per-object + * gate (RestServer's `enforceApiAccess` serves this composition; the runtime + * dispatcher's twin is pinned in `packages/runtime`). + * + * Persona: a plain member (`stack.signUp`), so every case runs on the owner + * path the API Keys list renders, never the administrator's. + * + * Every refusal asserts status AND `code` (ADR-0112) AND the row afterwards: + * a 403 that still wrote is the defect wearing a refusal. + * + * Controls, which must keep today's answer: + * - the owner's SESSION revokes and restores a key from the list (200); + * - a key revoked with the forced-reset reason stays administrator-only for + * the owner's session (the restore guard, 403); + * - the mint, and the `last_used_at` stamp the key-admission seam writes, + * are system-context writes and still land. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { type VerifyStack } from '@objectstack/verify'; +import { bootShowcase } from './showcase-boot.js'; + +const SYS = { context: { isSystem: true } }; + +describe('sys_api_key on the data door: changing a key requires a signed-in session', () => { + let stack: VerifyStack; + let memberToken: string; + + const MEMBER_EMAIL = 'apikey.datadoor.session@verify.test'; + + /** Mint through the one mint path, as the member's session. */ + const mintKey = async (name: string): Promise<{ id: string; raw: string }> => { + const res = await stack.apiAs(memberToken, 'POST', '/keys', { name }); + expect(res.status, await res.clone().text()).toBe(201); + const body: any = await res.json(); + return { id: String(body.data.id), raw: String(body.data.key) }; + }; + + /** A request authenticated by the key alone: no cookie, no bearer. */ + const asKey = (raw: string, method: string, path: string, body?: unknown) => + stack.api(path, { + method, + headers: { 'x-api-key': raw, ...(body !== undefined ? { 'content-type': 'application/json' } : {}) }, + ...(body !== undefined ? { body: JSON.stringify(body) } : {}), + }); + + /** Does this key still authenticate? Asked on an object that is not the one under test. */ + const keyAuthenticates = async (raw: string): Promise => { + const res = await stack.api('/data/showcase_task?$top=1', { headers: { 'x-api-key': raw } }); + if (res.status === 200) return true; + if (res.status === 403) { + const body: any = await res.json(); + expect(body?.error?.code, 'a 403 on the key probe must be the key\'s own revocation').toBe('API_KEY_REVOKED'); + return false; + } + expect(res.status).toBe(401); + return false; + }; + + /** The row, read back by the owner's session. */ + const readKey = async (id: string) => { + const res = await stack.apiAs(memberToken, 'GET', `/data/sys_api_key/${id}`); + expect(res.status, await res.clone().text()).toBe(200); + const body: any = await res.json(); + return (body.record ?? body.data ?? {}) as Record; + }; + + const expectRefused = async (res: Response, status: number, code: string) => { + const text = await res.clone().text(); + expect(res.status, text).toBe(status); + const body: any = await res.json(); + expect(body.code ?? body.error?.code, text).toBe(code); + }; + + beforeAll(async () => { + stack = await bootShowcase({}); + await stack.signIn(); + memberToken = await stack.signUp(MEMBER_EMAIL); + }, 180_000); + + afterAll(async () => { await stack?.stop?.(); }); + + // ── refused for a request an API key authenticated ─────────────────────── + + it('refuses restoring a sibling key the owner revoked from the list — 403 PERMISSION_DENIED, key stays revoked', async () => { + const caller = await mintKey('dd-caller-restore'); + const sibling = await mintKey('dd-sibling-revoked'); + const off = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${sibling.id}`, { revoked: true }); + expect(off.status).toBe(200); + expect(await keyAuthenticates(sibling.raw)).toBe(false); + + await expectRefused( + await asKey(caller.raw, 'PATCH', `/data/sys_api_key/${sibling.id}`, { revoked: false }), + 403, 'PERMISSION_DENIED', + ); + expect((await readKey(sibling.id)).revoked).toBe(true); + expect(await keyAuthenticates(sibling.raw)).toBe(false); + }); + + it('refuses revoking a live sibling key — 403 PERMISSION_DENIED, key still authenticates', async () => { + const caller = await mintKey('dd-caller-revoke'); + const sibling = await mintKey('dd-sibling-live'); + expect(await keyAuthenticates(sibling.raw)).toBe(true); + + await expectRefused( + await asKey(caller.raw, 'PATCH', `/data/sys_api_key/${sibling.id}`, { revoked: true }), + 403, 'PERMISSION_DENIED', + ); + expect((await readKey(sibling.id)).revoked).toBe(false); + expect(await keyAuthenticates(sibling.raw)).toBe(true); + }); + + it('refuses the key revoking itself too — 403 PERMISSION_DENIED, key still authenticates', async () => { + const caller = await mintKey('dd-caller-self'); + await expectRefused( + await asKey(caller.raw, 'PATCH', `/data/sys_api_key/${caller.id}`, { revoked: true }), + 403, 'PERMISSION_DENIED', + ); + expect((await readKey(caller.id)).revoked).toBe(false); + expect(await keyAuthenticates(caller.raw)).toBe(true); + }); + + it('refuses a rename — 403 PERMISSION_DENIED, name unchanged', async () => { + const caller = await mintKey('dd-caller-rename'); + const sibling = await mintKey('dd-sibling-name'); + await expectRefused( + await asKey(caller.raw, 'PATCH', `/data/sys_api_key/${sibling.id}`, { name: 'renamed' }), + 403, 'PERMISSION_DENIED', + ); + expect((await readKey(sibling.id)).name).toBe('dd-sibling-name'); + }); + + it('create and delete keep the method gate\'s 405, ahead of the credential rule', async () => { + const caller = await mintKey('dd-caller-method'); + const sibling = await mintKey('dd-sibling-method'); + await expectRefused(await asKey(caller.raw, 'POST', '/data/sys_api_key', { name: 'forged' }), 405, 'OBJECT_API_METHOD_NOT_ALLOWED'); + await expectRefused(await asKey(caller.raw, 'DELETE', `/data/sys_api_key/${sibling.id}`), 405, 'OBJECT_API_METHOD_NOT_ALLOWED'); + expect(await keyAuthenticates(sibling.raw)).toBe(true); + }); + + it('reads are unchanged: the key lists and reads its owner\'s keys, and the stored hash is never served', async () => { + const caller = await mintKey('dd-caller-read'); + const list = await asKey(caller.raw, 'GET', '/data/sys_api_key'); + expect(list.status, await list.clone().text()).toBe(200); + const records: any[] = ((await list.json()) as any).records ?? []; + expect(records.some((r) => r.id === caller.id)).toBe(true); + for (const r of records) expect(Object.prototype.hasOwnProperty.call(r, 'key')).toBe(false); + + const one = await asKey(caller.raw, 'GET', `/data/sys_api_key/${caller.id}`); + expect(one.status).toBe(200); + }); + + // ── controls: unchanged for the session and the system ──────────────────── + + it('[control] the owner\'s session still revokes and restores a key from the list (200)', async () => { + const key = await mintKey('dd-control-session'); + const off = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${key.id}`, { revoked: true }); + expect(off.status, await off.clone().text()).toBe(200); + expect(await keyAuthenticates(key.raw)).toBe(false); + + const on = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${key.id}`, { revoked: false }); + expect(on.status, await on.clone().text()).toBe(200); + expect(await keyAuthenticates(key.raw)).toBe(true); + }); + + it('[control] a key the forced reset revoked stays administrator-only for the owner\'s session (403 PERMISSION_DENIED)', async () => { + const key = await mintKey('dd-control-forced-reset'); + // The stored state the forced reset writes (`forced-reset-revocation.ts`): + // `revoked: true` with its reason, written in the system context. + const ql: any = await stack.kernel.getServiceAsync('objectql'); + await ql.update('sys_api_key', { id: key.id, revoked: true, revoked_reason: 'admin_password_reset' }, SYS); + expect(await keyAuthenticates(key.raw)).toBe(false); + + await expectRefused( + await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${key.id}`, { revoked: false }), + 403, 'PERMISSION_DENIED', + ); + const row = await readKey(key.id); + expect(row.revoked).toBe(true); + expect(row.revoked_reason).toBe('admin_password_reset'); + }); + + it('[control] the mint and the key-admission `last_used_at` stamp, both system-context writes, still land', async () => { + const key = await mintKey('dd-control-system'); + expect((await readKey(key.id)).last_used_at ?? null).toBeNull(); + expect(await keyAuthenticates(key.raw)).toBe(true); + expect((await readKey(key.id)).last_used_at).toBeTruthy(); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index d70b11fea43..d77de8a4a9e 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -25,6 +25,10 @@ import { // [#23367] The API-key lifecycle's branded refusal. isApiKeyLifecycleRefusalError, type ApiKeyLifecycleRefusalError, shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS, + // Managing API keys is a signed-in-session act: the one rule + // `enforceApiAccess` asks beside the exposure decision, shared with the + // runtime dispatcher's per-object gate. + apiKeyManagementCredentialRefusal, // `isAudienceBindingSuggestionStatus` and `unknownAudienceBindingSuggestionStatusMessage`, // the suggested-binding `?status=` vocabulary (#7678), moved with `registerSecurityEndpoints`' // body to `security-routes.ts` (#23050 slice 6). @@ -2695,6 +2699,26 @@ export class RestServer { res.status(denial.status).json(denial.body); return true; } + // A write to `sys_api_key` from a principal an API key admitted is + // refused `403 PERMISSION_DENIED`: changing a key is a signed-in-session + // act, the answer `/keys` gives (`apiKeyManagementCredentialRefusal`, + // `@objectstack/core`, whose docblock carries the rule). Asked after the + // exposure verdict, so an operation the object does not expose keeps its + // `405`, and here because every data route of this server passes this + // gate. The context is the request's memoized one, already resolved by + // the route before it asked this gate; `__admittedBy` is the credential + // the shared resolver told `computeExecCtx`. + const context = await this.resolveExecCtx(environmentId, req); + const credentialRefusal = apiKeyManagementCredentialRefusal({ + object: objectName, + operation, + credential: context?.__admittedBy, + }); + if (credentialRefusal) { + const mapped = mapDataError(credentialRefusal, objectName); + res.status(mapped.status).json(mapped.body); + return true; + } return false; } @@ -3339,6 +3363,7 @@ export class RestServer { // role/permission/RLS aggregation, the localization and the envelope // are the composition's; each parameter below is this door's answer // from before the composition existed (its docblock tables them). + let admittedBy: 'api_key' | 'session' | undefined; const base = await resolveRequestExecutionContext({ headers, ql, @@ -3395,9 +3420,10 @@ export class RestServer { // scopes cannot ride onto REST — so `principalKind: 'agent'`, // `onBehalfOf` and `oauthScopes` are not representable here. oauth: undefined, - // The dispatcher's admission channel. This door enforces the gate - // off the envelope, at `enforceAuth`. - onAdmission: undefined, + // Which credential admitted the caller, kept on THIS request for + // `enforceApiAccess`'s API-key management rule. The ADR-0069 gate + // is still enforced off the envelope, at `enforceAuth`. + onAdmission: (admission) => { admittedBy = admission.credential; }, }); if (!base) return undefined; @@ -3425,6 +3451,13 @@ export class RestServer { // behind it. Same status as `__authEnvironmentId`: not an // `ExecutionContext` field, nothing downstream branches on it. __environmentId: environmentId, + // Internal: which credential admitted this caller (`'api_key'` | + // `'session'`), as the shared resolver told this door through + // `onAdmission` above. An authorization input at ONE reader, + // `enforceApiAccess` (managing API keys is a signed-in-session + // act, `apiKeyManagementCredentialRefusal`); not an + // `ExecutionContext` field, nothing downstream branches on it. + __admittedBy: admittedBy, } as any; // [#2408 / #3361] Open the per-request `Server-Timing` disclosure gate diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index df33d88ceb4..37d96279e54 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -40,6 +40,9 @@ import { relationConditionSites } from '@objectstack/core'; // [#25653] The app-automation elevation mark an action body's context carries // — see `buildActionExecutionContext`. import { markAppAutomationElevation } from '@objectstack/core'; +// Managing API keys is a signed-in-session act: the one rule this door's +// per-object gate asks beside the exposure decision, shared with `RestServer`. +import { apiKeyManagementCredentialRefusal } from '@objectstack/core'; // [#9446] The ONE #9378 status table. Imported rather than re-read here: this // door's blanket `FLOW_FAILED` was the second of three readings of one engine // result, and a second definition of the rule is what let the doors diverge. @@ -485,6 +488,21 @@ export async function refuseUnexposedDataAction(deps: ActionExecutionDeps, if (!gate.allowed) { throw Object.assign(new Error(gate.reason ?? 'API access denied'), exposureRefusalFields(gate), { object }); } + // A write to `sys_api_key` from a principal an API key admitted is refused + // `403 PERMISSION_DENIED`: changing a key is a signed-in-session act, the + // answer `/keys` gives (`@objectstack/core` `apiKeyManagementCredentialRefusal`, + // whose docblock carries the rule). Asked after the exposure verdict, so an + // operation the object does not expose keeps its `405`, and on this gate + // because every data face of this door passes it: `/data`, the `/mcp` + // bridge and the action doors, through {@link callData}. The credential is + // the one the identity step was told (`HttpProtocolContext.admission`); + // a context that carries none keeps today's answer. + const credentialRefusal = apiKeyManagementCredentialRefusal({ + object, + operation: action, + credential: requestContext?.admission?.credential, + }); + if (credentialRefusal) throw credentialRefusal; } export async function callData(deps: ActionExecutionDeps, diff --git a/packages/runtime/src/domains/data-api-key-management-credential.test.ts b/packages/runtime/src/domains/data-api-key-management-credential.test.ts new file mode 100644 index 00000000000..02b9d08e365 --- /dev/null +++ b/packages/runtime/src/domains/data-api-key-management-credential.test.ts @@ -0,0 +1,120 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The runtime dispatcher's data faces refuse a write to `sys_api_key` from a +// principal an API key admitted: changing a key is a signed-in-session act, the +// answer `/keys` gives. The rule is `@objectstack/core`'s +// `apiKeyManagementCredentialRefusal`, asked at the per-object gate every data +// face of this door passes (`refuseUnexposedDataAction`, reached through +// `callData`), and the credential is the one the dispatcher's identity step +// wrote to `HttpProtocolContext.admission`. +// +// Both halves are pinned per case: the refusal (thrown with `code` + `status`, +// rendered by the dispatcher's own `errorFromThrown` on the ADR-0112 envelope) +// and that the protocol was never asked; and, for every caller the rule leaves +// alone, the write the protocol actually received. + +import { describe, it, expect, vi } from 'vitest'; +import { handleDataRequest } from './data.js'; +import { callData } from '../action-execution.js'; +import { HttpDispatcher } from '../http-dispatcher.js'; +import type { DomainHandlerDeps } from '../domain-handler-registry.js'; + +/** The dispatcher's OWN exits, borrowed off a real dispatcher (see `data-path-object.test.ts`). */ +const realDomainDeps = (() => { + const dispatcher: any = new HttpDispatcher({ context: { getService: () => null } } as any); + const domainDeps: DomainHandlerDeps = dispatcher.domainDeps; + return { + success: domainDeps.success, + error: domainDeps.error, + routeNotFound: domainDeps.routeNotFound, + errorFromThrown: domainDeps.errorFromThrown, + }; +})(); + +function setup(admission: { credential: 'api_key' | 'session'; authGate: undefined } | undefined, opts: { isSystem?: boolean } = {}) { + const updateData = vi.fn(async (req: any) => ({ object: req.object, id: req.id, record: { id: req.id, ...req.data } })); + const findData = vi.fn(async (req: any) => ({ object: req.object, records: [], total: 0 })); + const protocol = { updateData, findData }; + const metadata = { getObject: async () => undefined }; + const deps: any = { + resolveService: async (_ctx: any, name: string) => + name === 'protocol' ? protocol : name === 'metadata' ? metadata : null, + getService: () => null, + getObjectQL: async () => null, + getRequestKernelService: async () => null, + isMultiTenantHost: () => false, + ...realDomainDeps, + resolveActiveOrganizationId: async () => undefined, + announceKernelEvent: async () => {}, + }; + const executionContext: any = { userId: 'u1', ...(opts.isSystem ? { isSystem: true } : {}) }; + const context: any = { dataDriver: undefined, environmentId: undefined, executionContext, ...(admission ? { admission } : {}) }; + return { deps, context, updateData, findData }; +} + +const API_KEY = { credential: 'api_key' as const, authGate: undefined }; +const SESSION = { credential: 'session' as const, authGate: undefined }; + +describe('dispatcher data faces: changing an API key requires a signed-in session', () => { + it('refuses PATCH sys_api_key from an API-key-admitted caller with 403 PERMISSION_DENIED, and never asks the protocol', async () => { + const { deps, context, updateData } = setup(API_KEY); + let thrown: any; + try { + await handleDataRequest(deps, 'sys_api_key/k1', 'PATCH', { revoked: false }, {}, context); + } catch (e) { + thrown = e; + } + expect(thrown).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: 'sys_api_key' }); + expect(updateData).not.toHaveBeenCalled(); + + // The envelope this door renders for it, through its own exit. + const rendered: any = realDomainDeps.errorFromThrown(thrown, 500); + expect(rendered.status).toBe(403); + expect(rendered.body.success).toBe(false); + expect(rendered.body.error.code).toBe('PERMISSION_DENIED'); + expect(rendered.body.error.httpStatus).toBe(403); + }); + + it('refuses the same write through callData, the seam the /mcp bridge and the action doors ask', async () => { + const { deps, context, updateData } = setup(API_KEY); + await expect( + callData(deps, context, 'update', { object: 'sys_api_key', id: 'k1', data: { revoked: true } }, + undefined, undefined, context.executionContext), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: 'sys_api_key' }); + expect(updateData).not.toHaveBeenCalled(); + }); + + it('serves the same PATCH for a session-admitted caller, with the patch it sent', async () => { + const { deps, context, updateData } = setup(SESSION); + const res: any = await handleDataRequest(deps, 'sys_api_key/k1', 'PATCH', { revoked: false }, {}, context); + expect(res.response.status).toBe(200); + expect(updateData).toHaveBeenCalledTimes(1); + expect(updateData.mock.calls[0][0]).toMatchObject({ object: 'sys_api_key', id: 'k1', data: { revoked: false } }); + }); + + it('serves it for a context whose admission this door was never told (the rule is positive on the credential)', async () => { + const { deps, context, updateData } = setup(undefined); + const res: any = await handleDataRequest(deps, 'sys_api_key/k1', 'PATCH', { revoked: true }, {}, context); + expect(res.response.status).toBe(200); + expect(updateData).toHaveBeenCalledTimes(1); + }); + + it('leaves a system-context write alone whatever the admission says', async () => { + const { deps, context, updateData } = setup(API_KEY, { isSystem: true }); + await callData(deps, context, 'update', { object: 'sys_api_key', id: 'k1', data: { last_used_at: 'now' } }, + undefined, undefined, context.executionContext); + expect(updateData).toHaveBeenCalledTimes(1); + }); + + it('leaves reads of sys_api_key, and writes to every other object, alone for an API-key-admitted caller', async () => { + const { deps, context, updateData, findData } = setup(API_KEY); + const list: any = await handleDataRequest(deps, 'sys_api_key', 'GET', undefined, {}, context); + expect(list.response.status).toBe(200); + expect(findData).toHaveBeenCalledTimes(1); + + const other: any = await handleDataRequest(deps, 'crm_account/a1', 'PATCH', { name: 'x' }, {}, context); + expect(other.response.status).toBe(200); + expect(updateData).toHaveBeenCalledTimes(1); + expect(updateData.mock.calls[0][0].object).toBe('crm_account'); + }); +}); From b5da53d041f53793df2fdf52d36c90795bc2b1c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 17:37:03 +0000 Subject: [PATCH 2/8] chore(changeset): declare the data-door API-key management narrowing Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../harden-api-key-data-door-session.md | 26 +++++++++++++++++++ ...data-door-session-required.dogfood.test.ts | 8 +++--- 2 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 .changeset/harden-api-key-data-door-session.md diff --git a/.changeset/harden-api-key-data-door-session.md b/.changeset/harden-api-key-data-door-session.md new file mode 100644 index 00000000000..769b483bcf9 --- /dev/null +++ b/.changeset/harden-api-key-data-door-session.md @@ -0,0 +1,26 @@ +--- +'@objectstack/core': patch +'@objectstack/rest': patch +'@objectstack/runtime': patch +--- + +harden(data): changing an API key through the data door requires a signed-in session + +Clause-②: no (narrowing) +Metadata-protocol: none +Skills-docs: none — no file under `skills/**` names `sys_api_key` or teaches managing a key with an API key, and the docs teach key revocation and rotation from the API Keys list (`content/docs/ai/connect-mcp.mdx`, `content/docs/api/error-catalog.mdx`), a signed-in surface; every taught use stays true. + + + +**BREAKING** (an accept-set narrowing). + +Managing API keys is a signed-in-session act on every data door, as minting one through `POST /api/v1/keys` is. An API key keeps acting as its owner for data, automation and MCP. + +**FROM.** The generic data door judged a write to `sys_api_key` by the row and column rules alone, whichever credential authenticated the request. + +**TO.** A write to `sys_api_key` from a request an API key authenticated is refused `403 PERMISSION_DENIED`, and nothing is written. The message tells the caller to sign in and manage the key from the API Keys page. + +- **Who is affected.** Only a script or integration that changed `sys_api_key` rows through `/api/v1/data/sys_api_key` while authenticated with an API key. **Fix:** make the change signed in, from the API Keys list (Account → Developer), or with a session cookie or session bearer token. +- **What does not change.** A signed-in session manages its own keys from the list as before, and the existing rule on who restores a key a forced password reset revoked is unchanged. Reading `sys_api_key` is unchanged. System-context writes (the mint, the last-used stamp, the forced reset) are untouched. An operation the object does not expose keeps its `405 OBJECT_API_METHOD_NOT_ALLOWED`. +- **Where it is decided.** One rule, `apiKeyManagementCredentialRefusal` (new export of `@objectstack/core`), asked at each data door's per-object gate: `RestServer`'s data routes and the runtime dispatcher's data faces. It reads which credential authenticated the request from the shared request resolver (`onAdmission`); the `ExecutionContext` shape is unchanged. +- **Embedders calling `HttpDispatcher` data handlers directly.** The rule is positive on the credential: a context with no `admission` keeps today's answer. diff --git a/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts b/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts index 61c8febd8d5..100273ee722 100644 --- a/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts +++ b/packages/qa/dogfood/test/api-key-data-door-session-required.dogfood.test.ts @@ -94,7 +94,7 @@ describe('sys_api_key on the data door: changing a key requires a signed-in sess // ── refused for a request an API key authenticated ─────────────────────── - it('refuses restoring a sibling key the owner revoked from the list — 403 PERMISSION_DENIED, key stays revoked', async () => { + it('refuses `revoked: false` on a revoked key — 403 PERMISSION_DENIED, row unchanged', async () => { const caller = await mintKey('dd-caller-restore'); const sibling = await mintKey('dd-sibling-revoked'); const off = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${sibling.id}`, { revoked: true }); @@ -109,7 +109,7 @@ describe('sys_api_key on the data door: changing a key requires a signed-in sess expect(await keyAuthenticates(sibling.raw)).toBe(false); }); - it('refuses revoking a live sibling key — 403 PERMISSION_DENIED, key still authenticates', async () => { + it('refuses `revoked: true` on another key — 403 PERMISSION_DENIED, row unchanged', async () => { const caller = await mintKey('dd-caller-revoke'); const sibling = await mintKey('dd-sibling-live'); expect(await keyAuthenticates(sibling.raw)).toBe(true); @@ -122,7 +122,7 @@ describe('sys_api_key on the data door: changing a key requires a signed-in sess expect(await keyAuthenticates(sibling.raw)).toBe(true); }); - it('refuses the key revoking itself too — 403 PERMISSION_DENIED, key still authenticates', async () => { + it('refuses `revoked: true` on the calling key — 403 PERMISSION_DENIED, row unchanged', async () => { const caller = await mintKey('dd-caller-self'); await expectRefused( await asKey(caller.raw, 'PATCH', `/data/sys_api_key/${caller.id}`, { revoked: true }), @@ -132,7 +132,7 @@ describe('sys_api_key on the data door: changing a key requires a signed-in sess expect(await keyAuthenticates(caller.raw)).toBe(true); }); - it('refuses a rename — 403 PERMISSION_DENIED, name unchanged', async () => { + it('refuses a `name` change — 403 PERMISSION_DENIED, row unchanged', async () => { const caller = await mintKey('dd-caller-rename'); const sibling = await mintKey('dd-sibling-name'); await expectRefused( From fbacecb9ed8800224fc9b5564f989dc488624995 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 17:49:05 +0000 Subject: [PATCH 3/8] harden(rest): hand the route context to enforceApiAccess; ask the rule per op on the cross-object batch The credential rule reads the route's own resolved context instead of a second resolve, and the cross-object batch asks it per op after its exposure verdict. Pins the fourth post-assembly internal key. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- packages/rest/src/batch-routes.ts | 26 +++- packages/rest/src/crud-routes.ts | 12 +- packages/rest/src/data-action-routes.ts | 14 +- ...rest-api-key-management-credential.test.ts | 144 ++++++++++++++++++ .../src/rest-exec-ctx-principal-kind.test.ts | 8 + packages/rest/src/rest-server.ts | 17 ++- 6 files changed, 198 insertions(+), 23 deletions(-) create mode 100644 packages/rest/src/rest-api-key-management-credential.test.ts diff --git a/packages/rest/src/batch-routes.ts b/packages/rest/src/batch-routes.ts index c2ad0e6dac7..0707bb5c3fb 100644 --- a/packages/rest/src/batch-routes.ts +++ b/packages/rest/src/batch-routes.ts @@ -53,6 +53,10 @@ import type { DroppedFieldsEvent } from '@objectstack/spec/data'; // The write-response rules (credential-class mask, `internal` omit) for // the cross-object batch's direct-engine update arm. import { omitInternalFieldsFromWriteResponse } from '@objectstack/core'; +// Managing API keys is a signed-in-session act: the rule every other data +// route of this server asks at `enforceApiAccess`, asked per op here because +// this route judges each op's object itself. +import { apiKeyManagementCredentialRefusal } from '@objectstack/core'; // The engine's own by-id predicates, which `ObjectQL.update` / `.delete` // dispatch on — the cross-object batch's up-front id check asks them. import { resolveEngineDeleteDispatch, resolveEngineUpdateDispatch } from '@objectstack/metadata-core'; @@ -296,6 +300,20 @@ export function registerBatchEndpoints(this: BatchRouteHost, basePath: string): } } + // A write to `sys_api_key` from a request an API key + // authenticated is refused `403 PERMISSION_DENIED`, as on every + // single-object route (`enforceApiAccess`): after the exposure + // verdict above, so an op the object does not expose keeps its + // `405`, and before the transaction opens. + for (const op of ops) { + const credentialRefusal = apiKeyManagementCredentialRefusal({ + object: op.object, + operation: 'bulk', + credential: (context as { __admittedBy?: 'api_key' | 'session' } | undefined)?.__admittedBy, + }); + if (credentialRefusal) { handleRouteError(res, credentialRefusal, op.object); return; } + } + // Resolve `{ $ref: }` values against results collected // so far. A ref MUST point at an earlier create whose id is known; // anything else is a 400 (never a silent null FK). @@ -430,7 +448,7 @@ export function registerBatchEndpoints(this: BatchRouteHost, basePath: string): if (this.enforceAuth(req, res, context)) return; // [#3391] bulk ∧ child(body.operation) — the object must grant // the `bulk` primitive AND the batched write kind. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: req.body?.operation })) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', context, { bulkChild: req.body?.operation })) return; // [#3899] Validate against the declared contract // (`BatchUpdateRequestSchema`, catalog `requestSchema`) — // this route used to hand the body straight to the @@ -485,7 +503,7 @@ export function registerBatchEndpoints(this: BatchRouteHost, basePath: string): const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; // [#3391] bulk ∧ create — createMany requires the `bulk` primitive. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'create' })) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', context, { bulkChild: 'create' })) return; // [#3899] Body IS the records array on this route (what // `client.data.createMany` posts). Validate the assembled // protocol request (`CreateManyDataRequestSchema`, catalog @@ -538,7 +556,7 @@ export function registerBatchEndpoints(this: BatchRouteHost, basePath: string): const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; // [#3391] bulk ∧ update — updateMany requires the `bulk` primitive. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'update' })) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', context, { bulkChild: 'update' })) return; // [#3933] Validate against the spec contract, and write the // PATH object last. The body used to be spread over // `object: req.params.object`, so `{"object":"other", …}` @@ -594,7 +612,7 @@ export function registerBatchEndpoints(this: BatchRouteHost, basePath: string): const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; // [#3391] bulk ∧ delete — deleteMany requires the `bulk` primitive. - if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', { bulkChild: 'delete' })) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'bulk', context, { bulkChild: 'delete' })) return; // [#3897] Validate against the spec contract instead of // splatting the raw body into the protocol request. Zod // object schemas STRIP unknown keys, which is what makes diff --git a/packages/rest/src/crud-routes.ts b/packages/rest/src/crud-routes.ts index d6ded1489fc..eeb698e3e1f 100644 --- a/packages/rest/src/crud-routes.ts +++ b/packages/rest/src/crud-routes.ts @@ -121,7 +121,7 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo const p = await this.resolveProtocol(environmentId, req); const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - if (await this.enforceApiAccess(req, res, p, environmentId, 'list')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'list', context)) return; // [#6877] Still NOT arity-gated as a whole, for the // reason it never was: this route hands the WHOLE query // record to `findData`, whose normalizer @@ -198,7 +198,7 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo // says the downstream was built for it. const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - if (await this.enforceApiAccess(req, res, p, environmentId, 'get')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'get', context)) return; // [#7606] Closed parameter set, AFTER the capability // gates for the same reason #7527 put it there: which // parameters a route understands is information, and a @@ -259,7 +259,7 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo const p = await this.resolveProtocol(environmentId, req); const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - if (await this.enforceApiAccess(req, res, p, environmentId, 'create')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'create', context)) return; // [#3899] The wire body IS the record. Validate the // assembled protocol request (`CreateDataRequestSchema`, // catalog `requestSchema`) so a non-record body — an @@ -332,7 +332,7 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo const p = await this.resolveProtocol(environmentId, req); const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - if (await this.enforceApiAccess(req, res, p, environmentId, 'list')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'list', context)) return; // [#3899] Validate the QueryAST body against the declared // contract (`FindDataRequestSchema`, catalog `requestSchema`). // A malformed body used to be forwarded as-is — and since a @@ -432,7 +432,7 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo bodyVersion != null ? 'The body\'s `expectedVersion`' : 'The `If-Match` header', ); if (versionRefusal) throw validationFailure(versionRefusal, []); - if (await this.enforceApiAccess(req, res, p, environmentId, 'update')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'update', context)) return; // [#3899] Same gate as create: the wire body is the bare // field patch (`expectedVersion` already stripped above), // validated as the assembled `UpdateDataRequestSchema` @@ -533,7 +533,7 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo queryVersion != null ? 'The `expectedVersion` query parameter' : 'The `If-Match` header', ); if (versionRefusal) throw validationFailure(versionRefusal, []); - if (await this.enforceApiAccess(req, res, p, environmentId, 'delete')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'delete', context)) return; const deleteRequest: ServerScopedDataRequest = { object: req.params.object, id: req.params.id, diff --git a/packages/rest/src/data-action-routes.ts b/packages/rest/src/data-action-routes.ts index 11546c5099b..9726fc26493 100644 --- a/packages/rest/src/data-action-routes.ts +++ b/packages/rest/src/data-action-routes.ts @@ -307,7 +307,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: const p = await this.resolveProtocol(environmentId, req); const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - if (await this.enforceApiAccess(req, res, p, environmentId, 'create')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'create', context)) return; const cloneData = (p as any).cloneData; if (typeof cloneData !== 'function') { res.status(501).json({ code: 'NOT_IMPLEMENTED', error: 'Clone not supported by this protocol' }); @@ -360,7 +360,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: const p = await this.resolveProtocol(environmentId, req); const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - if (await this.enforceApiAccess(req, res, p, environmentId, 'get')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'get', context)) return; // Closed and empty: the record is the path, and nothing else is read. if (refuseUnknownQueryParams(req, res, RECORD_FIELD_ACCESS_PARAMS)) return; const object = String(req.params.object); @@ -426,7 +426,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: res.status(400).json({ code: 'INVALID_REQUEST', error: 'object is required' }); return; } - if (await this.enforceApiAccess(req, res, p, environmentId, 'import')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'import', context)) return; const body = req.body ?? {}; // [#24348] Who may take the object's declared controls off this @@ -464,7 +464,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: // mode is known, re-gate precisely: insert→create, update→update, // upsert→create∧update. Catches e.g. an object that grants only // `create` receiving an update-mode import. - if (await this.enforceApiAccess(req, res, p, environmentId, 'import', { writeMode })) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'import', context, { writeMode })) return; // Delegate the per-row coercion + upsert loop to the shared // runner (also used by the async import-job worker). @@ -546,7 +546,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: res.status(400).json({ code: 'INVALID_REQUEST', error: 'object is required' }); return; } - if (await this.enforceApiAccess(req, res, p, environmentId, 'import')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'import', context)) return; // [#24348] The synchronous door's control opt-out gate, asked at // the same point: before the body is parsed and before a job row @@ -574,7 +574,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: // [#3391] Import gate — stage 2 (precise), see the synchronous // route: now that writeMode is resolved, re-gate precisely // (insert→create, update→update, upsert→create∧update). - if (await this.enforceApiAccess(req, res, p, environmentId, 'import', { writeMode: prepared.writeMode })) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'import', context, { writeMode: prepared.writeMode })) return; const jobId = newImportJobId(); const createdAt = new Date().toISOString(); @@ -1027,7 +1027,7 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: if (readTemplateMode({ template: req.query?.template }).kind === 'template') { if (await this.enforceImportTemplateGates(req, res, p, environmentId, objectName, context)) return; } else { - if (await this.enforceApiAccess(req, res, p, environmentId, 'export')) return; + if (await this.enforceApiAccess(req, res, p, environmentId, 'export', context)) return; // [#3544] …then the USER-level one. The object may expose // export while THIS caller's permission sets deny it. if (await this.enforceExportPermission(req, res, environmentId, objectName, context)) return; diff --git a/packages/rest/src/rest-api-key-management-credential.test.ts b/packages/rest/src/rest-api-key-management-credential.test.ts new file mode 100644 index 00000000000..60799ed5404 --- /dev/null +++ b/packages/rest/src/rest-api-key-management-credential.test.ts @@ -0,0 +1,144 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// RestServer's data routes refuse a write to `sys_api_key` from a request an +// API key authenticated: changing a key is a signed-in-session act, the answer +// `/keys` gives. The rule is `@objectstack/core`'s +// `apiKeyManagementCredentialRefusal`, asked at `enforceApiAccess` (every +// single-object data route) and per op on the cross-object `POST /batch`, off +// the context's `__admittedBy` (the credential the shared resolver told +// `computeExecCtx`). +// +// The fixture's `sys_api_key` declares MORE methods than the shipped object +// (bulk included) on purpose: the shipped object's own `apiMethods` answer the +// bulk routes `405` before this rule is asked, so only a wider declaration +// reaches the rule there. Both halves are pinned per case: the refusal (status +// + `code`) with the protocol never asked, and the served write's arguments. + +import { describe, it, expect, vi } from 'vitest'; +import { RestServer } from './rest-server'; + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.setHeader = vi.fn(); res.header = vi.fn(() => res); res.write = vi.fn(); res.end = vi.fn(); + return res; +} + +function setup(admittedBy: 'api_key' | 'session' | undefined, sysApiKeyMethods = ['get', 'list', 'update', 'bulk']) { + const updateData = vi.fn(async (req: any) => ({ object: req.object, id: req.id, record: { id: req.id, ...req.data } })); + const updateManyData = vi.fn().mockResolvedValue({ + success: true, operation: 'update', total: 1, succeeded: 1, failed: 0, results: [], + }); + const findData = vi.fn().mockResolvedValue({ object: 'sys_api_key', records: [], total: 0 }); + const items = [ + { name: 'sys_api_key', enable: { apiMethods: sysApiKeyMethods } }, + { name: 'open' }, + ]; + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue(items), + getMetaItem: vi.fn(async (req: any) => ({ type: req?.type, name: req?.name, item: items.find((o) => o.name === req?.name) })), + findData, + updateData, + updateManyData, + }; + const transaction = vi.fn(async (fn: any) => fn({})); + const engine: any = { transaction, update: vi.fn(async () => ({})), insert: vi.fn(), delete: vi.fn() }; + const rest = new RestServer( + createMockServer() as any, protocol, { api: { requireAuth: false } } as any, + undefined, undefined, undefined, undefined, + (async () => engine) as any, + ); + (rest as any).resolveExecCtx = async () => ({ userId: 'u1', ...(admittedBy ? { __admittedBy: admittedBy } : {}) }); + rest.registerRoutes(); + const call = async (method: string, path: string, params: Record, body?: unknown) => { + const route = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + if (!route) throw new Error(`route not registered: ${method} ${path}`); + const res = makeRes(); + await route.handler({ method, params, query: {}, headers: {}, body } as any, res); + return res; + }; + return { call, updateData, updateManyData, findData, transaction }; +} + +const BY_ID = '/api/v1/data/:object/:id'; +const LIST = '/api/v1/data/:object'; +const UPDATE_MANY = '/api/v1/data/:object/updateMany'; +const BATCH = '/api/v1/batch'; + +describe('RestServer data routes: changing an API key requires a signed-in session', () => { + it('refuses PATCH sys_api_key from an API-key-admitted request — 403 PERMISSION_DENIED, protocol never asked', async () => { + const { call, updateData } = setup('api_key'); + const res = await call('PATCH', BY_ID, { object: 'sys_api_key', id: 'k1' }, { revoked: false }); + expect(res.statusCode).toBe(403); + expect(res.body.code).toBe('PERMISSION_DENIED'); + expect(res.body.object).toBe('sys_api_key'); + expect(updateData).not.toHaveBeenCalled(); + }); + + it('serves the same PATCH for a session-admitted request, with the patch it sent', async () => { + const { call, updateData } = setup('session'); + const res = await call('PATCH', BY_ID, { object: 'sys_api_key', id: 'k1' }, { revoked: false }); + expect(res.statusCode).toBe(200); + expect(updateData).toHaveBeenCalledTimes(1); + expect(updateData.mock.calls[0][0]).toMatchObject({ object: 'sys_api_key', id: 'k1', data: { revoked: false } }); + }); + + it('serves it for a context that names no credential (the rule is positive on the credential)', async () => { + const { call, updateData } = setup(undefined); + const res = await call('PATCH', BY_ID, { object: 'sys_api_key', id: 'k1' }, { revoked: true }); + expect(res.statusCode).toBe(200); + expect(updateData).toHaveBeenCalledTimes(1); + }); + + it('leaves reads of sys_api_key, and writes to other objects, alone for an API-key-admitted request', async () => { + const { call, updateData, findData } = setup('api_key'); + expect((await call('GET', LIST, { object: 'sys_api_key' })).statusCode).toBe(200); + expect(findData).toHaveBeenCalledTimes(1); + const other = await call('PATCH', BY_ID, { object: 'open', id: 'o1' }, { name: 'x' }); + expect(other.statusCode).toBe(200); + expect(updateData.mock.calls[0][0].object).toBe('open'); + }); + + it('an operation the object does not expose keeps its 405, ahead of the credential rule', async () => { + const { call, updateData } = setup('api_key', ['get', 'list']); + const res = await call('PATCH', BY_ID, { object: 'sys_api_key', id: 'k1' }, { revoked: false }); + expect(res.statusCode).toBe(405); + expect(res.body.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED'); + expect(updateData).not.toHaveBeenCalled(); + }); + + it('refuses the per-object bulk route the same way', async () => { + const { call, updateManyData } = setup('api_key'); + const res = await call('POST', UPDATE_MANY, { object: 'sys_api_key' }, { records: [{ id: 'k1', data: { revoked: true } }] }); + expect(res.statusCode).toBe(403); + expect(res.body.code).toBe('PERMISSION_DENIED'); + expect(updateManyData).not.toHaveBeenCalled(); + }); + + it('refuses a cross-object batch naming sys_api_key before the transaction opens, and serves one that does not', async () => { + const refused = setup('api_key'); + const res = await refused.call('POST', BATCH, {}, { + operations: [{ object: 'sys_api_key', action: 'update', id: 'k1', data: { revoked: true } }], + }); + expect(res.statusCode).toBe(403); + expect(res.body.code ?? res.body.error?.code).toBe('PERMISSION_DENIED'); + expect(refused.transaction).not.toHaveBeenCalled(); + + const served = setup('api_key'); + const ok = await served.call('POST', BATCH, {}, { + operations: [{ object: 'open', action: 'update', id: 'o1', data: { name: 'x' } }], + }); + expect(ok.statusCode, JSON.stringify(ok.body)).toBe(200); + expect(served.transaction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/rest/src/rest-exec-ctx-principal-kind.test.ts b/packages/rest/src/rest-exec-ctx-principal-kind.test.ts index ae99033f829..92446ae17ea 100644 --- a/packages/rest/src/rest-exec-ctx-principal-kind.test.ts +++ b/packages/rest/src/rest-exec-ctx-principal-kind.test.ts @@ -323,7 +323,15 @@ describe('#6216 — the REST face assembles through the SHARED assembler, output // in `rest-server.ts`), which refuses a credential validated in a // different environment. Same status as `__authEnvironmentId`. '__environmentId', + // The FOURTH post-assembly internal key: which credential admitted + // the caller (`'session'` here), as the shared resolver told this + // door through `onAdmission`. Read by ONE reader inside + // `rest-server.ts`, `enforceApiAccess` (changing an API key is a + // signed-in-session act, `apiKeyManagementCredentialRefusal`); same + // status as `__authEnvironmentId`: not an `ExecutionContext` field. + '__admittedBy', ])); + expect(ctx.__admittedBy).toBe('session'); }); it('still WITHHOLDS accessToken — the named per-face divergence, not an omission', async () => { diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index d77de8a4a9e..ae64f8d73c8 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -2678,6 +2678,13 @@ export class RestServer { p: RestProtocol, environmentId: string | undefined, operation: string, + /** + * The request's resolved context, as the route already holds it (the + * one it handed `enforceAuth`). Required, so a route cannot ask this + * gate without it: the API-key management rule below reads which + * credential admitted the caller off it (`__admittedBy`). + */ + context: unknown, opts?: ApiAccessOpts, ): Promise { const objectName = req?.params?.object; @@ -2705,14 +2712,12 @@ export class RestServer { // `@objectstack/core`, whose docblock carries the rule). Asked after the // exposure verdict, so an operation the object does not expose keeps its // `405`, and here because every data route of this server passes this - // gate. The context is the request's memoized one, already resolved by - // the route before it asked this gate; `__admittedBy` is the credential - // the shared resolver told `computeExecCtx`. - const context = await this.resolveExecCtx(environmentId, req); + // gate. `__admittedBy` is the credential the shared resolver told + // `computeExecCtx`, on the context the route hands in. const credentialRefusal = apiKeyManagementCredentialRefusal({ object: objectName, operation, - credential: context?.__admittedBy, + credential: (context as { __admittedBy?: 'api_key' | 'session' } | undefined)?.__admittedBy, }); if (credentialRefusal) { const mapped = mapDataError(credentialRefusal, objectName); @@ -2871,7 +2876,7 @@ export class RestServer { objectName: string, context: any, ): Promise { - if (await this.enforceApiAccess(req, res, p, environmentId, 'import')) return true; + if (await this.enforceApiAccess(req, res, p, environmentId, 'import', context)) return true; const security = await this.resolveSecurityService(environmentId, req); if (!security || typeof security.explain !== 'function') return false; let allowed: boolean; From 5215b0774defea77a619a92766e140224afa301e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 18:10:23 +0000 Subject: [PATCH 4/8] test(runtime): set aside and pin REST's admitted-credential key in the one-context conformance Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../request-execution-context.conformance.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/security/request-execution-context.conformance.test.ts b/packages/runtime/src/security/request-execution-context.conformance.test.ts index 9d878de72e2..7ade1e76e37 100644 --- a/packages/runtime/src/security/request-execution-context.conformance.test.ts +++ b/packages/runtime/src/security/request-execution-context.conformance.test.ts @@ -30,7 +30,7 @@ * never who the caller is. * * A door's own additions after the function returns (REST's `__kernel`, - * `__authEnvironmentId` and `__environmentId`, see its `drive`) are not the composition and are + * `__authEnvironmentId`, `__environmentId` and `__admittedBy`, see its `drive`) are not the composition and are * set aside before the comparison, by name. * * A projection can drop a parameter's whole effect: the settings context @@ -344,10 +344,16 @@ async function driveRest(w: World, headers: Record): Promise Date: Thu, 29 Oct 2026 18:38:00 +0000 Subject: [PATCH 5/8] test(rest): open the batch fake's update with the engine's dispatch assertion Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../src/rest-api-key-management-credential.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/rest/src/rest-api-key-management-credential.test.ts b/packages/rest/src/rest-api-key-management-credential.test.ts index 60799ed5404..1d6b2fcd8c9 100644 --- a/packages/rest/src/rest-api-key-management-credential.test.ts +++ b/packages/rest/src/rest-api-key-management-credential.test.ts @@ -15,6 +15,7 @@ // + `code`) with the protocol never asked, and the served write's arguments. import { describe, it, expect, vi } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { RestServer } from './rest-server'; function createMockServer() { @@ -52,7 +53,14 @@ function setup(admittedBy: 'api_key' | 'session' | undefined, sysApiKeyMethods = updateManyData, }; const transaction = vi.fn(async (fn: any) => fn({})); - const engine: any = { transaction, update: vi.fn(async () => ({})), insert: vi.fn(), delete: vi.fn() }; + // The cross-object batch's engine: its update refuses what a real engine refuses. + const engine: any = { + transaction, + update: vi.fn(async (_object: string, data: any, options?: any) => { + assertEngineUpdateDispatch(data, options); + return { ...data }; + }), + }; const rest = new RestServer( createMockServer() as any, protocol, { api: { requireAuth: false } } as any, undefined, undefined, undefined, undefined, From c9771407b5ee8c24eedc8d3b11e7c06dd0a30a8e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 19:24:32 +0000 Subject: [PATCH 6/8] harden(mcp): ask the API-key management rule on the stdio transport's write arms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stdio bridge's create, update and remove ask apiKeyManagementCredentialRefusal beside the exposure gate, with the transport's API-key credential. The changeset declares the widened core entry (Clause-② yes) and names the stdio door. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../harden-api-key-data-door-session.md | 15 ++-- .../security/api-key-management-credential.ts | 5 +- .../src/stdio-data-bridge.exposure.test.ts | 78 +++++++++++++++++++ packages/mcp/src/stdio-data-bridge.ts | 26 +++++++ 4 files changed, 116 insertions(+), 8 deletions(-) diff --git a/.changeset/harden-api-key-data-door-session.md b/.changeset/harden-api-key-data-door-session.md index 769b483bcf9..4b5095d178c 100644 --- a/.changeset/harden-api-key-data-door-session.md +++ b/.changeset/harden-api-key-data-door-session.md @@ -2,25 +2,26 @@ '@objectstack/core': patch '@objectstack/rest': patch '@objectstack/runtime': patch +'@objectstack/mcp': patch --- harden(data): changing an API key through the data door requires a signed-in session -Clause-②: no (narrowing) +Clause-②: yes (narrowing) Metadata-protocol: none Skills-docs: none — no file under `skills/**` names `sys_api_key` or teaches managing a key with an API key, and the docs teach key revocation and rotation from the API Keys list (`content/docs/ai/connect-mcp.mdx`, `content/docs/api/error-catalog.mdx`), a signed-in surface; every taught use stays true. - + **BREAKING** (an accept-set narrowing). Managing API keys is a signed-in-session act on every data door, as minting one through `POST /api/v1/keys` is. An API key keeps acting as its owner for data, automation and MCP. -**FROM.** The generic data door judged a write to `sys_api_key` by the row and column rules alone, whichever credential authenticated the request. +**FROM.** The data doors judged a write to `sys_api_key` by the row and column rules alone, whichever credential authenticated the caller. -**TO.** A write to `sys_api_key` from a request an API key authenticated is refused `403 PERMISSION_DENIED`, and nothing is written. The message tells the caller to sign in and manage the key from the API Keys page. +**TO.** A write to `sys_api_key` from a caller an API key authenticated is refused `403 PERMISSION_DENIED`, and nothing is written, on the REST data routes (`/api/v1/data/sys_api_key`, the bulk routes and the cross-object `POST /api/v1/batch`), on the runtime dispatcher's data faces (`/data`, the `/mcp` HTTP tools, the action doors) and on the stdio MCP transport's `create_record`, `update_record` and `delete_record` tools, whose principal is always the configured `OS_MCP_STDIO_API_KEY`. The message tells the caller to sign in and manage the key from the API Keys page. -- **Who is affected.** Only a script or integration that changed `sys_api_key` rows through `/api/v1/data/sys_api_key` while authenticated with an API key. **Fix:** make the change signed in, from the API Keys list (Account → Developer), or with a session cookie or session bearer token. -- **What does not change.** A signed-in session manages its own keys from the list as before, and the existing rule on who restores a key a forced password reset revoked is unchanged. Reading `sys_api_key` is unchanged. System-context writes (the mint, the last-used stamp, the forced reset) are untouched. An operation the object does not expose keeps its `405 OBJECT_API_METHOD_NOT_ALLOWED`. -- **Where it is decided.** One rule, `apiKeyManagementCredentialRefusal` (new export of `@objectstack/core`), asked at each data door's per-object gate: `RestServer`'s data routes and the runtime dispatcher's data faces. It reads which credential authenticated the request from the shared request resolver (`onAdmission`); the `ExecutionContext` shape is unchanged. +- **Who is affected.** Only a script, integration or agent that changed `sys_api_key` rows through one of those doors while authenticated with an API key. **Fix:** make the change signed in, from the API Keys list (Account → Developer), or with a session cookie or session bearer token. +- **What does not change.** A signed-in session manages its own keys from the list as before, and the existing rule on who restores a key a forced password reset revoked is unchanged. Reading `sys_api_key` is unchanged on every door. System-context writes (the mint, the last-used stamp, the forced reset) are untouched. An operation the object does not expose keeps its `405 OBJECT_API_METHOD_NOT_ALLOWED`. +- **Where it is decided.** One rule, `apiKeyManagementCredentialRefusal`, added to `@objectstack/core`'s entry with `API_KEY_MANAGEMENT_READ_OPERATIONS`, `API_KEY_MANAGEMENT_SESSION_REQUIRED_MESSAGE` and the type `ApiKeyManagementCredentialInput`, asked beside each door's exposure gate. The REST and dispatcher doors read which credential authenticated the request from the shared request resolver (`onAdmission`); the stdio transport's credential is its API key by construction. The `ExecutionContext` shape is unchanged. - **Embedders calling `HttpDispatcher` data handlers directly.** The rule is positive on the credential: a context with no `admission` keeps today's answer. diff --git a/packages/core/src/security/api-key-management-credential.ts b/packages/core/src/security/api-key-management-credential.ts index 25d0ae2970f..ccc9949610a 100644 --- a/packages/core/src/security/api-key-management-credential.ts +++ b/packages/core/src/security/api-key-management-credential.ts @@ -20,7 +20,10 @@ * `ExecutionContext` envelope does not carry it, so an ObjectQL hook cannot * ask it. Each door asks this function at its per-object gate, the one every * data route on that door passes: the runtime dispatcher's - * `refuseUnexposedDataAction` and `RestServer.enforceApiAccess`. + * `refuseUnexposedDataAction`, `RestServer.enforceApiAccess` (and per op on + * its cross-object batch), and the stdio MCP transport's write arms + * (`@objectstack/mcp` `stdio-data-bridge.ts`), whose principal an API key + * admits by construction (ADR-0101). * * ## The rule * diff --git a/packages/mcp/src/stdio-data-bridge.exposure.test.ts b/packages/mcp/src/stdio-data-bridge.exposure.test.ts index 8f0d5c0bc05..a9e30665472 100644 --- a/packages/mcp/src/stdio-data-bridge.exposure.test.ts +++ b/packages/mcp/src/stdio-data-bridge.exposure.test.ts @@ -495,3 +495,81 @@ describe('#15416 the stdio refusal never names an operation it also allows', () } }); }); + +// --------------------------------------------------------------------------- +// Beside the exposure gate: changing an API key requires a signed-in session +// --------------------------------------------------------------------------- + +/** + * This transport's principal is admitted by `OS_MCP_STDIO_API_KEY` alone + * (ADR-0101), so its write arms ask `apiKeyManagementCredentialRefusal` + * (`@objectstack/core`) with the API-key credential, after the exposure gate + * and before the engine. Refusals are asserted by envelope (`code` + `status`) + * with the engine never asked; every served case asserts the engine call. + */ +describe('stdio data bridge: a sys_api_key write is refused 403 PERMISSION_DENIED', () => { + /** The shipped declaration's method set, and an unrestricted one that lets every write verb reach the rule. */ + const SHIPPED = { name: 'sys_api_key', enable: { apiMethods: ['get', 'list', 'update'] } }; + const UNRESTRICTED = { name: 'sys_api_key' }; + + const expectCredentialRefusal = async (run: () => Promise) => { + let thrown: any; + try { + await run(); + } catch (e) { + thrown = e; + } + expect(thrown, 'the write must be refused').toBeInstanceOf(Error); + expect(thrown).toMatchObject({ code: 'PERMISSION_DENIED', status: 403, object: 'sys_api_key' }); + }; + + it('refuses update on the shipped declaration, and never asks the engine', async () => { + const { bridge, engine } = makeBridge(SHIPPED); + await expectCredentialRefusal(() => bridge.update('sys_api_key', 'k1', { revoked: false })); + expect(engine.findOne).not.toHaveBeenCalled(); + expect(engine.update).not.toHaveBeenCalled(); + }); + + it('refuses create, update and remove when every verb is exposed, and never asks the engine', async () => { + const { bridge, engine } = makeBridge(UNRESTRICTED); + await expectCredentialRefusal(() => bridge.create('sys_api_key', { name: 'x' })); + await expectCredentialRefusal(() => bridge.update('sys_api_key', 'k1', { revoked: true })); + await expectCredentialRefusal(() => bridge.remove('sys_api_key', 'k1')); + expect(engine.insert).not.toHaveBeenCalled(); + expect(engine.findOne).not.toHaveBeenCalled(); + expect(engine.update).not.toHaveBeenCalled(); + expect(engine.delete).not.toHaveBeenCalled(); + }); + + it('an operation the object does not expose keeps its 405, ahead of the rule', async () => { + const { bridge, engine } = makeBridge(SHIPPED); + for (const run of [() => bridge.create('sys_api_key', { name: 'x' }), () => bridge.remove('sys_api_key', 'k1')]) { + let thrown: any; + try { await run(); } catch (e) { thrown = e; } + expect(thrown).toMatchObject({ code: 'OBJECT_API_METHOD_NOT_ALLOWED', status: 405 }); + } + expect(engine.insert).not.toHaveBeenCalled(); + expect(engine.delete).not.toHaveBeenCalled(); + }); + + it('serves reads of sys_api_key', async () => { + const { bridge, engine } = makeBridge(SHIPPED); + await bridge.query('sys_api_key', { limit: 5 }); + await bridge.get('sys_api_key', 'r1'); + expect(engine.find).toHaveBeenCalledTimes(1); + expect(engine.findOne).toHaveBeenCalledTimes(1); + }); + + it('serves a write to another object', async () => { + const { bridge, engine } = makeBridge({ name: 'task' }); + await bridge.update('task', 'r1', { title: 'x' }); + expect(engine.update).toHaveBeenCalledTimes(1); + expect(engine.update.mock.calls[0][0]).toBe('task'); + }); + + it('leaves a system context alone, as the exposure gate does', async () => { + const { bridge, engine } = makeBridge(UNRESTRICTED, { context: SYSTEM }); + await bridge.update('sys_api_key', 'r1', { revoked: true }); + expect(engine.update).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/mcp/src/stdio-data-bridge.ts b/packages/mcp/src/stdio-data-bridge.ts index 956357a9eee..54d5d3918a7 100644 --- a/packages/mcp/src/stdio-data-bridge.ts +++ b/packages/mcp/src/stdio-data-bridge.ts @@ -121,6 +121,10 @@ import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts' // and `@objectstack/core` is the lowest package that carries the factory, so // there is no reason to add a second import path to the same function. import { recordNotFoundError } from '@objectstack/core'; +// Managing API keys is a signed-in-session act: the one rule every data door +// asks beside its exposure gate (`RestServer.enforceApiAccess`, the runtime +// dispatcher's `refuseUnexposedDataAction`), asked here by the write arms. +import { apiKeyManagementCredentialRefusal } from '@objectstack/core'; import type { McpDataBridge, McpObjectSummary } from './mcp-http-tools.js'; import { metaItemsArray, type McpObjectView } from './mcp-server-runtime.js'; import { @@ -852,6 +856,25 @@ export function serveStoredMetadataRows( return rows.map((row) => serveStoredMetadataRow(object, row, opts)); } +/** + * The API-key management rule on this transport: a write to `sys_api_key` is + * refused `403 PERMISSION_DENIED` (`apiKeyManagementCredentialRefusal`, + * `@objectstack/core`, whose docblock carries the rule), thrown as the error + * the tool layer renders, like the exposure refusals beside it. + * + * The credential is `'api_key'` by construction: this transport admits its + * principal from `OS_MCP_STDIO_API_KEY` and from nothing else (ADR-0101, + * `resolveStdioExecutionContext` in `plugin.ts`), and the envelope carries no + * credential to read. A system context skips it, as the exposure gate does. + * Asked after the exposure gate, so an operation the object does not expose + * keeps its `405`, and before the engine is asked anything. + */ +function enforceApiKeyManagementCredential(object: string, action: string, context: ExecutionContext): void { + if (context?.isSystem) return; + const refusal = apiKeyManagementCredentialRefusal({ object, operation: action, credential: 'api_key' }); + if (refusal) throw refusal; +} + /** * Build the stdio transport's principal-bound data bridge. * @@ -965,6 +988,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async create(object, data) { const context = await resolvePrincipal(); await enforceApiExposure(reads, object, GATED_ACTIONS.create, context); + enforceApiKeyManagementCredential(object, GATED_ACTIONS.create, context); const written = (await engine.insert(object, data, { context })) as | Record | undefined; @@ -988,6 +1012,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge // miss is an observable difference, so gating second would answer "that // id names no row" for an object the author declared unexposed. await enforceApiExposure(reads, object, GATED_ACTIONS.update, context); + enforceApiKeyManagementCredential(object, GATED_ACTIONS.update, context); const existing = await findById(engine, object, id, context); // The "this id names no row" refusal, raised BEFORE the write is // attempted — a write path that answered success for an id that matched @@ -1018,6 +1043,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge const context = await resolvePrincipal(); // Gated before the probe, for the reason `update` states. await enforceApiExposure(reads, object, GATED_ACTIONS.remove, context); + enforceApiKeyManagementCredential(object, GATED_ACTIONS.remove, context); const existing = await findById(engine, object, id, context); // Same shared envelope as `update`, above. if (!existing) throw recordNotFoundError(object, id); From 9bb853f27e97b433b2d93da9015daec91a2b4999 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 19:31:47 +0000 Subject: [PATCH 7/8] test(mcp): assert the stdio write refusals against the engine read the bridge uses Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../mcp/src/stdio-data-bridge.exposure.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/mcp/src/stdio-data-bridge.exposure.test.ts b/packages/mcp/src/stdio-data-bridge.exposure.test.ts index a9e30665472..744d585f052 100644 --- a/packages/mcp/src/stdio-data-bridge.exposure.test.ts +++ b/packages/mcp/src/stdio-data-bridge.exposure.test.ts @@ -526,7 +526,7 @@ describe('stdio data bridge: a sys_api_key write is refused 403 PERMISSION_DENIE it('refuses update on the shipped declaration, and never asks the engine', async () => { const { bridge, engine } = makeBridge(SHIPPED); await expectCredentialRefusal(() => bridge.update('sys_api_key', 'k1', { revoked: false })); - expect(engine.findOne).not.toHaveBeenCalled(); + expect(engine.find).not.toHaveBeenCalled(); expect(engine.update).not.toHaveBeenCalled(); }); @@ -536,7 +536,7 @@ describe('stdio data bridge: a sys_api_key write is refused 403 PERMISSION_DENIE await expectCredentialRefusal(() => bridge.update('sys_api_key', 'k1', { revoked: true })); await expectCredentialRefusal(() => bridge.remove('sys_api_key', 'k1')); expect(engine.insert).not.toHaveBeenCalled(); - expect(engine.findOne).not.toHaveBeenCalled(); + expect(engine.find).not.toHaveBeenCalled(); expect(engine.update).not.toHaveBeenCalled(); expect(engine.delete).not.toHaveBeenCalled(); }); @@ -554,10 +554,12 @@ describe('stdio data bridge: a sys_api_key write is refused 403 PERMISSION_DENIE it('serves reads of sys_api_key', async () => { const { bridge, engine } = makeBridge(SHIPPED); - await bridge.query('sys_api_key', { limit: 5 }); - await bridge.get('sys_api_key', 'r1'); - expect(engine.find).toHaveBeenCalledTimes(1); - expect(engine.findOne).toHaveBeenCalledTimes(1); + const listed: any = await bridge.query('sys_api_key', { limit: 5 }); + const one: any = await bridge.get('sys_api_key', 'r1'); + expect(listed.records).toHaveLength(1); + expect(one?.id).toBe('r1'); + // Both reads reach the engine (the by-id read goes through `find` too). + expect(engine.find).toHaveBeenCalledTimes(2); }); it('serves a write to another object', async () => { From 853277e1dfa7f78ad3c249d32bede7e0a9f30240 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 19:34:45 +0000 Subject: [PATCH 8/8] harden(mcp): the stdio write-arm rule reads no elevation flag The stdio principal is never the system context, so the API-key management check on its write arms asks the rule unconditionally. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude --- .../mcp/src/stdio-data-bridge.exposure.test.ts | 6 ------ packages/mcp/src/stdio-data-bridge.ts | 16 ++++++++-------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/mcp/src/stdio-data-bridge.exposure.test.ts b/packages/mcp/src/stdio-data-bridge.exposure.test.ts index 744d585f052..5dff404126c 100644 --- a/packages/mcp/src/stdio-data-bridge.exposure.test.ts +++ b/packages/mcp/src/stdio-data-bridge.exposure.test.ts @@ -568,10 +568,4 @@ describe('stdio data bridge: a sys_api_key write is refused 403 PERMISSION_DENIE expect(engine.update).toHaveBeenCalledTimes(1); expect(engine.update.mock.calls[0][0]).toBe('task'); }); - - it('leaves a system context alone, as the exposure gate does', async () => { - const { bridge, engine } = makeBridge(UNRESTRICTED, { context: SYSTEM }); - await bridge.update('sys_api_key', 'r1', { revoked: true }); - expect(engine.update).toHaveBeenCalledTimes(1); - }); }); diff --git a/packages/mcp/src/stdio-data-bridge.ts b/packages/mcp/src/stdio-data-bridge.ts index 54d5d3918a7..6aaa0d9e507 100644 --- a/packages/mcp/src/stdio-data-bridge.ts +++ b/packages/mcp/src/stdio-data-bridge.ts @@ -865,12 +865,12 @@ export function serveStoredMetadataRows( * The credential is `'api_key'` by construction: this transport admits its * principal from `OS_MCP_STDIO_API_KEY` and from nothing else (ADR-0101, * `resolveStdioExecutionContext` in `plugin.ts`), and the envelope carries no - * credential to read. A system context skips it, as the exposure gate does. - * Asked after the exposure gate, so an operation the object does not expose - * keeps its `405`, and before the engine is asked anything. + * credential to read. That principal is never the system context, so the rule + * reads no elevation flag: it has none to grant. Asked after the exposure + * gate, so an operation the object does not expose keeps its `405`, and before + * the engine is asked anything. */ -function enforceApiKeyManagementCredential(object: string, action: string, context: ExecutionContext): void { - if (context?.isSystem) return; +function enforceApiKeyManagementCredential(object: string, action: string): void { const refusal = apiKeyManagementCredentialRefusal({ object, operation: action, credential: 'api_key' }); if (refusal) throw refusal; } @@ -988,7 +988,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async create(object, data) { const context = await resolvePrincipal(); await enforceApiExposure(reads, object, GATED_ACTIONS.create, context); - enforceApiKeyManagementCredential(object, GATED_ACTIONS.create, context); + enforceApiKeyManagementCredential(object, GATED_ACTIONS.create); const written = (await engine.insert(object, data, { context })) as | Record | undefined; @@ -1012,7 +1012,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge // miss is an observable difference, so gating second would answer "that // id names no row" for an object the author declared unexposed. await enforceApiExposure(reads, object, GATED_ACTIONS.update, context); - enforceApiKeyManagementCredential(object, GATED_ACTIONS.update, context); + enforceApiKeyManagementCredential(object, GATED_ACTIONS.update); const existing = await findById(engine, object, id, context); // The "this id names no row" refusal, raised BEFORE the write is // attempted — a write path that answered success for an id that matched @@ -1043,7 +1043,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge const context = await resolvePrincipal(); // Gated before the probe, for the reason `update` states. await enforceApiExposure(reads, object, GATED_ACTIONS.remove, context); - enforceApiKeyManagementCredential(object, GATED_ACTIONS.remove, context); + enforceApiKeyManagementCredential(object, GATED_ACTIONS.remove); const existing = await findById(engine, object, id, context); // Same shared envelope as `update`, above. if (!existing) throw recordNotFoundError(object, id);