From ac5a818030ea069eb2745dbfd6a6acbab78b0b16 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 01:13:51 +0000 Subject: [PATCH] fix(mcp): apply the ADR-0049 exposure gate on the stdio MCP bridge (#8083) Both MCP transports register the same tools from the same McpDataBridge, but the two hosts implement that bridge over different seams: HTTP runs every data verb through callData, which gates on the object's declared apiEnabled / apiMethods before dispatch, while the stdio bridge went straight to the engine and applied no gate. One declaration, two transports, two answers. This is a surface-area declaration leak, not an authorization bypass -- the gate is a surface-area control by api-exposure.ts's own ADR note, and every stdio call passed the engine's CRUD/FLS/RLS before this change and after it. What was leaking is the author's exposure declaration. The decision is not re-implemented here: it comes from the spec's single source of truth (resolveEffectiveApiMethods / isApiOperationAllowed), the same helpers checkApiExposure (runtime) and apiAccessDenialFromEnable (rest) delegate to, so each surface owns only its envelope. Refusals carry the same two machine codes REST answers with. Gated verbs are exactly the six buildMcpBridge routes through callData; list_objects / describe_object stay ungated because the HTTP bridge answers both off the metadata service, and gating them would be a fresh divergence pointing the other way. The isSystem bypass, the fail-open on unresolvable metadata and the flat-shape fallback are matched to the HTTP path deliberately. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016pY4Xb2iDecfDtT3CWoiTW --- .changeset/mcp-stdio-exposure-gate.md | 43 +++ .../src/stdio-data-bridge.exposure.test.ts | 356 ++++++++++++++++++ packages/mcp/src/stdio-data-bridge.ts | 161 +++++++- 3 files changed, 555 insertions(+), 5 deletions(-) create mode 100644 .changeset/mcp-stdio-exposure-gate.md create mode 100644 packages/mcp/src/stdio-data-bridge.exposure.test.ts diff --git a/.changeset/mcp-stdio-exposure-gate.md b/.changeset/mcp-stdio-exposure-gate.md new file mode 100644 index 0000000000..93db768106 --- /dev/null +++ b/.changeset/mcp-stdio-exposure-gate.md @@ -0,0 +1,43 @@ +--- +"@objectstack/mcp": patch +--- + +fix(mcp): the stdio transport honours the ADR-0049 `apiEnabled` / `apiMethods` exposure declaration (#8083) + +An object that declares `enable.apiEnabled: false` — or narrows `enable.apiMethods` — +is telling the platform which data operations it exposes over the API. That +declaration was honoured on the MCP **HTTP** surface and ignored on the MCP +**stdio** surface: same product, same tool names, same key, different answer. + +**This is a surface-area declaration leak, not an authorization bypass.** The gate +is a surface-area control by `api-exposure.ts`'s own ADR note, and every stdio call +passed the ObjectQL security middleware (CRUD / FLS / RLS) before this change and +after it. What was leaking is the author's *exposure declaration*, not the data +guard. + +The two MCP hosts implement the same `McpDataBridge` over different seams — HTTP +through `callData`, which gates before dispatch; stdio straight onto the engine, +which did not. The stdio bridge now applies the same gate, and takes its decision +from the same single source of truth both existing enforcement points already +delegate to (the spec's `resolveEffectiveApiMethods` / `isApiOperationAllowed`), so +the three-state whitelist, the action-to-operation mapping and the derived verbs +resolve identically on all three surfaces. + +Gated verbs are exactly the six the HTTP bridge routes through `callData`: +`query_records`, `get_record`, `create_record`, `update_record`, `delete_record` +and `aggregate_records`. `list_objects` / `describe_object` stay ungated, because +the HTTP bridge answers both straight off the metadata service — a schema read +refused on stdio and served on HTTP would be the same divergence pointing the +other way. + +Refusals carry the same machine codes the REST surface answers with: +`OBJECT_API_DISABLED` (the object is hidden) and `OBJECT_API_METHOD_NOT_ALLOWED` +(the operation is outside the whitelist, with the effective operation set +attached). Three behaviours are matched to the HTTP path deliberately: a system +context bypasses the gate, unresolvable metadata **fails open** to the schema +defaults, and the flat legacy definition shape is read when there is no nested +`enable` block. + +Unaffected: the remaining known divergences between the two MCP bridges (the +protocol layer's ingress `readonly` strip, its existence probes, its spec-shaped +receipts and `expand` / `select`) are unchanged and still filed as follow-up work. diff --git a/packages/mcp/src/stdio-data-bridge.exposure.test.ts b/packages/mcp/src/stdio-data-bridge.exposure.test.ts new file mode 100644 index 0000000000..ba596d72f1 --- /dev/null +++ b/packages/mcp/src/stdio-data-bridge.exposure.test.ts @@ -0,0 +1,356 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8083 — transport parity for the ADR-0049 object exposure gate. + * + * The defect: both MCP transports register the SAME tools from the same + * `McpDataBridge` (#8034 made that structural), but the two hosts implement + * that bridge over different seams — HTTP through `callData`, which gates on + * the object's declared `apiEnabled` / `apiMethods`, and stdio straight onto + * the engine, which did not. One declaration, two transports, two answers. + * + * **What was leaking is the author's DECLARATION, not the data guard.** The + * gate is a surface-area control by `api-exposure.ts`'s own ADR note, and every + * stdio call passed the engine's CRUD/FLS/RLS before this change and after it. + * These tests are graded accordingly: they assert an exposure verdict, never a + * data-authorization one. + * + * ## How the parity claim is pinned from inside `packages/mcp` + * + * The HTTP verdict function is `checkApiExposure` + * (`packages/runtime/src/api-exposure.ts`), and `packages/mcp` neither depends + * on `@objectstack/runtime` nor may read its sources (that would be exactly the + * cross-package test input `check:cross-package-test-inputs` exists to catch). + * So the parity is pinned the way it is actually reviewable: the declaration → + * verdict table below is the SAME table `packages/runtime/src/api-exposure.test.ts` + * pins the HTTP side against, case for case — `apiEnabled: false` → 404, + * a whitelist miss → 405, `[]` → deny-all, `find`/`query` → `list`, + * `aggregate` as a list-class read, an unmapped action ungated, and the nested + * `enable` block winning over the flat shape. If either surface moves off the + * shared derivation, the two tables stop agreeing. + * + * The half that is NOT a hand-copied table is {@link GATED_ACTIONS}: it is + * asserted structurally against the spec's own action map, so a typo in an + * action word cannot silently degrade a verb to the ungated pass-through. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { DATA_ACTION_TO_API_OPERATION } from '@objectstack/spec/data'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; +import { createStdioDataBridge, GATED_ACTIONS, type McpExposureError } from './stdio-data-bridge.js'; + +// --------------------------------------------------------------------------- +// Doubles +// --------------------------------------------------------------------------- + +/** + * The engine double. `insert` / `update` / `delete` are bare `vi.fn()` — the + * idiom `__tests__/plugin.test.ts` already uses here — because every write + * assertion below is that they were **never reached**: the gate refuses before + * dispatch, so a double that modelled the engine's write semantics would be + * modelling a call that must not happen. + */ +function makeEngine(rows: Array> = [{ id: 'r1', title: 'row' }]) { + return { + find: vi.fn(async () => rows), + findOne: vi.fn(async () => rows[0] ?? null), + insert: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + count: vi.fn(async () => rows.length), + aggregate: vi.fn(async () => [{ n: rows.length }]), + }; +} + +/** A metadata service serving ONE object definition (or a thrown read). */ +function makeMetadata(def: unknown, opts?: { throws?: boolean }) { + return { + listObjects: vi.fn(async () => [{ name: 'task', label: 'Task', fields: {} }]), + getObject: vi.fn(async () => { + if (opts?.throws) throw new Error('metadata service is down'); + return def; + }), + get: vi.fn(async () => null), + list: vi.fn(async () => []), + exists: vi.fn(async () => true), + getRegisteredTypes: vi.fn(async () => ['object']), + register: vi.fn(), + unregister: vi.fn(), + }; +} + +const PRINCIPAL = { userId: 'u1', isSystem: false } as unknown as ExecutionContext; +const SYSTEM = { userId: 'u1', isSystem: true } as unknown as ExecutionContext; + +function makeBridge( + def: unknown, + opts?: { throws?: boolean; context?: ExecutionContext; rows?: Array> }, +) { + const engine = makeEngine(opts?.rows); + const metadataService = makeMetadata(def, { throws: opts?.throws }); + const bridge = createStdioDataBridge({ + engine: engine as unknown as IDataEngine, + metadataService: metadataService as unknown as IMetadataService, + resolvePrincipal: async () => opts?.context ?? PRINCIPAL, + }); + return { bridge, engine, metadataService }; +} + +/** Run a bridge verb by name with arguments that satisfy every signature. */ +function invoke(bridge: ReturnType['bridge'], method: string): Promise { + switch (method) { + case 'query': + return bridge.query('task', { limit: 5 }) as Promise; + case 'get': + return bridge.get('task', 'r1') as Promise; + case 'create': + return bridge.create('task', { title: 'x' }) as Promise; + case 'update': + return bridge.update('task', 'r1', { title: 'x' }) as Promise; + case 'remove': + return bridge.remove('task', 'r1') as Promise; + case 'aggregate': + return bridge.aggregate!('task', { + aggregations: [{ function: 'count', field: 'id', alias: 'n' }], + }) as Promise; + default: + throw new Error(`no such bridge verb: ${method}`); + } +} + +/** + * Assert an exposure refusal by its ENVELOPE (ADR-0112), not by the fact that + * something threw: a bridge that threw a bare `Error` for an unrelated reason + * would satisfy `.toThrow()` while the gate stayed missing. + */ +async function expectRefusal( + run: () => Promise, + expected: { code: string; status: number }, +): Promise { + const err = (await run().then( + () => null, + (e: unknown) => e, + )) as McpExposureError | null; + expect(err, 'the call resolved — no exposure refusal was raised').toBeTruthy(); + expect(err!.code).toBe(expected.code); + expect(err!.status).toBe(expected.status); + return err!; +} + +const ALL_GATED = Object.keys(GATED_ACTIONS) as Array; + +// --------------------------------------------------------------------------- +// The structural half — no hand-copied table +// --------------------------------------------------------------------------- + +describe('#8083 the gated verb set is the HTTP one', () => { + it('gates exactly the six methods `buildMcpBridge` routes through callData', () => { + // `listObjects` / `describeObject` are absent BY DESIGN: the HTTP bridge + // answers both straight off the metadata service, so gating them here + // would be a fresh divergence in the opposite direction. + expect(ALL_GATED.sort()).toEqual( + ['aggregate', 'create', 'get', 'query', 'remove', 'update'].sort(), + ); + }); + + it('spells every action in a way the spec map actually recognises', () => { + // The failure this catches is silent: `DATA_ACTION_TO_API_OPERATION[action] + // ?? action` passes an unrecognised word straight through as an ungated + // custom action. A typo here would not throw — it would just stop gating + // that verb, which is the bug this card exists to close. + for (const [method, action] of Object.entries(GATED_ACTIONS)) { + expect( + DATA_ACTION_TO_API_OPERATION[action], + `bridge.${method} gates on "${action}", which the spec's action map does not define`, + ).toBeTruthy(); + } + }); + + it('maps `remove` onto the `delete` action word, as callData receives it', () => { + // The one entry whose bridge name and action word differ. + expect(GATED_ACTIONS.remove).toBe('delete'); + expect(DATA_ACTION_TO_API_OPERATION.delete).toBe('delete'); + }); +}); + +// --------------------------------------------------------------------------- +// The declaration → verdict table (mirrors runtime/src/api-exposure.test.ts) +// --------------------------------------------------------------------------- + +describe('#8083 apiEnabled: false hides the object on stdio (404)', () => { + it.each(ALL_GATED)('refuses %s', async (method) => { + const { bridge, engine } = makeBridge({ name: 'task', enable: { apiEnabled: false } }); + + const err = await expectRefusal(() => invoke(bridge, method), { + code: 'OBJECT_API_DISABLED', + status: 404, + }); + expect(err.message).toContain('task'); + + // Refused BEFORE dispatch — the engine was never asked anything. + expect(engine.find).not.toHaveBeenCalled(); + expect(engine.insert).not.toHaveBeenCalled(); + expect(engine.update).not.toHaveBeenCalled(); + expect(engine.delete).not.toHaveBeenCalled(); + expect(engine.aggregate).not.toHaveBeenCalled(); + }); + + it('still serves the schema reads the HTTP bridge serves ungated', async () => { + // The other direction of parity: `describe_object` / `list_objects` answer + // off the metadata service on HTTP with no gate, so a hidden object's + // SCHEMA stays readable on stdio too. Divergence in either direction is + // the same defect. + const { bridge } = makeBridge({ name: 'task', label: 'Task', enable: { apiEnabled: false } }); + + await expect(bridge.describeObject('task')).resolves.toMatchObject({ name: 'task' }); + await expect(bridge.listObjects()).resolves.toHaveLength(1); + }); +}); + +describe('#8083 apiMethods whitelist on stdio', () => { + const readOnly = { name: 'task', enable: { apiMethods: ['list', 'get'] } }; + + it('allows the whitelisted reads (query → list, get → get)', async () => { + const { bridge, engine } = makeBridge(readOnly); + + await expect(bridge.query('task', { limit: 5 })).resolves.toMatchObject({ object: 'task' }); + await expect(bridge.get('task', 'r1')).resolves.toMatchObject({ id: 'r1' }); + expect(engine.find).toHaveBeenCalledTimes(2); + }); + + it.each([ + ['create', 'create'], + ['update', 'update'], + ['remove', 'delete'], + ] as const)('refuses %s (405) and names the effective set', async (method, operation) => { + const { bridge, engine } = makeBridge(readOnly); + + const err = await expectRefusal(() => invoke(bridge, method), { + code: 'OBJECT_API_METHOD_NOT_ALLOWED', + status: 405, + }); + expect(err.message).toContain(operation); + // The EFFECTIVE operation set, as REST's 405 body carries it — the + // whitelist plus its derived reads, never the raw declaration. + expect(err.allowedOperations).toContain('get'); + expect(err.allowedOperations).toContain('list'); + expect(err.allowedOperations).not.toContain(operation); + + expect(engine.insert).not.toHaveBeenCalled(); + expect(engine.update).not.toHaveBeenCalled(); + expect(engine.delete).not.toHaveBeenCalled(); + }); + + it('gates aggregate as a list-class read', async () => { + // An object whose whitelist excludes `list` must not leak row statistics + // through GROUP BY either — the derivation, not a special case here. + const listed = makeBridge({ name: 'task', enable: { apiMethods: ['list'] } }); + await expect( + listed.bridge.aggregate!('task', { + aggregations: [{ function: 'count', field: 'id', alias: 'n' }], + }), + ).resolves.toBeDefined(); + + const getOnly = makeBridge({ name: 'task', enable: { apiMethods: ['get'] } }); + await expectRefusal(() => invoke(getOnly.bridge, 'aggregate'), { + code: 'OBJECT_API_METHOD_NOT_ALLOWED', + status: 405, + }); + expect(getOnly.engine.aggregate).not.toHaveBeenCalled(); + }); + + it('an empty whitelist is deny-all', async () => { + const { bridge } = makeBridge({ name: 'task', enable: { apiMethods: [] } }); + + for (const method of ALL_GATED) { + await expectRefusal(() => invoke(bridge, method), { + code: 'OBJECT_API_METHOD_NOT_ALLOWED', + status: 405, + }); + } + }); + + it('an absent whitelist is unrestricted', async () => { + const { bridge, engine } = makeBridge({ name: 'task', enable: { apiEnabled: true } }); + + await expect(bridge.create('task', { title: 'x' })).resolves.toMatchObject({ object: 'task' }); + expect(engine.insert).toHaveBeenCalledTimes(1); + }); +}); + +describe('#8083 shapes the HTTP gate reads that stdio must read too', () => { + it('reads the flat legacy shape when there is no nested enable block', async () => { + // `checkApiExposure` falls back to the flat top level for legacy/test + // doubles. Reading only the nested shape here would re-open this very + // divergence one shape down: gated on HTTP, ungated on stdio. + const { bridge } = makeBridge({ name: 'task', apiEnabled: false }); + + await expectRefusal(() => invoke(bridge, 'get'), { + code: 'OBJECT_API_DISABLED', + status: 404, + }); + }); + + it('lets the nested enable block win over the flat shape', async () => { + const { bridge, engine } = makeBridge({ name: 'task', apiEnabled: false, enable: {} }); + + await expect(bridge.get('task', 'r1')).resolves.toMatchObject({ id: 'r1' }); + expect(engine.find).toHaveBeenCalledTimes(1); + }); +}); + +describe('#8083 the fail-open and bypass behaviours match the HTTP path', () => { + it('falls open when the metadata read throws', async () => { + // `callData` wraps its metadata read in `catch { def = undefined }` and + // `checkApiExposure(undefined, …)` allows. Failing CLOSED here would be a + // divergence in the other direction — #3545's reasoning holds because the + // engine's CRUD/FLS/RLS still runs on the call. + const { bridge, engine } = makeBridge(null, { throws: true }); + + await expect(bridge.query('task', { limit: 5 })).resolves.toMatchObject({ object: 'task' }); + expect(engine.find).toHaveBeenCalledTimes(1); + }); + + it('falls open when the object does not resolve', async () => { + const { bridge, engine } = makeBridge(null); + + await expect(bridge.create('task', { title: 'x' })).resolves.toMatchObject({ object: 'task' }); + expect(engine.insert).toHaveBeenCalledTimes(1); + }); + + it('bypasses for a system context, as callData does', async () => { + // These flags govern API *exposure*, not internal engine self-writes. + const { bridge, engine, metadataService } = makeBridge( + { name: 'task', enable: { apiEnabled: false } }, + { context: SYSTEM }, + ); + + await expect(bridge.create('task', { title: 'x' })).resolves.toMatchObject({ object: 'task' }); + expect(engine.insert).toHaveBeenCalledTimes(1); + // Bypassed outright — not "read the metadata then allow". + expect(metadataService.getObject).not.toHaveBeenCalled(); + }); +}); + +describe('#8083 the gate runs before the existence probe', () => { + it.each(['update', 'remove'] as const)( + '%s refuses a hidden object without telling the caller whether the row exists', + async (method) => { + // Both verbs probe with `findById` and answer `recordNotFound` on a miss. + // Gating AFTER that probe would answer "no such record" for one id and + // succeed for another — an existence oracle on an object the author + // declared unexposed. + const { bridge, engine } = makeBridge({ name: 'task', enable: { apiEnabled: false } }, { + rows: [], + }); + + const err = await expectRefusal(() => invoke(bridge, method), { + code: 'OBJECT_API_DISABLED', + status: 404, + }); + expect(err.message).not.toMatch(/not found/i); + expect(engine.find).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/packages/mcp/src/stdio-data-bridge.ts b/packages/mcp/src/stdio-data-bridge.ts index d65c8fa63a..e2471d3a27 100644 --- a/packages/mcp/src/stdio-data-bridge.ts +++ b/packages/mcp/src/stdio-data-bridge.ts @@ -32,21 +32,37 @@ * tool call here is bounded exactly like the same identity over REST. This file * decides no policy — if it ever appears to, that is a bug in this file. * + * ## The ADR-0049 exposure gate (#8083) + * + * Every data verb below is gated on the object's declared `apiEnabled` / + * `apiMethods` before it dispatches, exactly as `callData` gates the HTTP + * bridge. This is a SURFACE-AREA control, not the authorization boundary (see + * `api-exposure.ts`'s own ADR note) — CRUD/FLS/RLS ran on this transport before + * and after. What was leaking was the AUTHOR'S DECLARATION: the same + * `apiEnabled: false` was honoured on MCP over HTTP and ignored on MCP over + * stdio. See {@link GATED_ACTIONS} for which verbs are gated and why that set + * is exactly the HTTP one. + * * ## Known divergences from the HTTP bridge (deliberate, filed, not security) * * `callData` prefers the `protocol` service (metadata-protocol) and falls back * to the engine; this bridge is engine-only. So the HTTP tools additionally get * that layer's ingress `readonly` strip, its existence probes, its spec-shaped - * receipts and `expand`/`select`, and the ADR-0049 `apiEnabled` / `apiMethods` - * exposure gate `callData` applies before dispatch. None of those is the - * authorization boundary — the exposure gate is a SURFACE-AREA control by its - * own ADR note, and every call here still passes the engine's CRUD/FLS/RLS — - * but the two transports should not differ at all, and unifying them behind one + * receipts and `expand`/`select`. None of those is the authorization boundary + * — every call here still passes the engine's CRUD/FLS/RLS — but the two + * transports should not differ at all, and unifying them behind one * transport-neutral data seam is filed as follow-up work rather than forked * here (route-ownership rule 1: a mirrored copy of `callData` would be a second * implementation that drifts). */ +import { + resolveEffectiveApiMethods, + isApiOperationAllowed, + effectiveOperationsArray, + DATA_ACTION_TO_API_OPERATION, + type EnableLike, +} from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { EngineAggregateOptions } from '@objectstack/spec/data'; import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; @@ -119,6 +135,128 @@ function recordNotFound(object: string, id: string): Error { return new Error(`Record "${id}" not found in "${object}"`); } +/** + * Bridge method → the `callData` action name the HTTP bridge gates it under. + * + * This table IS the parity claim, so it is data rather than six literals spread + * through the verbs below: `buildMcpBridge` (`packages/runtime/src/domains/mcp.ts`) + * routes exactly these six methods through `callData`, which gates on exactly + * these six action words — `remove` reaching it as `'delete'`, the only entry + * whose two names differ. `listObjects` / `describeObject` are deliberately + * ABSENT: the HTTP bridge answers both straight off the metadata service + * without touching `callData`, so gating them here would be a NEW divergence + * pointing the other way (a schema read refused on stdio and served on HTTP). + */ +export const GATED_ACTIONS = { + query: 'query', + get: 'get', + create: 'create', + update: 'update', + remove: 'delete', + aggregate: 'aggregate', +} as const; + +/** + * ADR-0112 machine codes for the two exposure refusals — the SAME pair REST's + * `apiAccessDenialFromEnable` answers with, so one declaration reads as one + * code on every surface that enforces it. + */ +const OBJECT_API_DISABLED = 'OBJECT_API_DISABLED'; +const OBJECT_API_METHOD_NOT_ALLOWED = 'OBJECT_API_METHOD_NOT_ALLOWED'; + +/** An exposure refusal: an `Error` (so the tool layer reads `.message`) carrying the envelope. */ +export interface McpExposureError extends Error { + /** ADR-0112 machine code. */ + code: string; + /** 404 (object hidden) or 405 (operation not in the whitelist). */ + status: number; + /** The effective operation set — present on a 405, as REST's `allowed` is. */ + allowedOperations?: string[]; +} + +function exposureError( + message: string, + code: string, + status: number, + allowedOperations?: string[], +): McpExposureError { + const err = new Error(message) as McpExposureError; + err.code = code; + err.status = status; + if (allowedOperations) err.allowedOperations = allowedOperations; + return err; +} + +/** + * The ADR-0049 object exposure gate, applied before a data verb dispatches + * (#8083). Throws {@link McpExposureError} when the object's own declaration + * does not expose `action`; returns normally when it does. + * + * The DECISION is not re-implemented here — it comes from the spec's single + * source of truth (`resolveEffectiveApiMethods` / `isApiOperationAllowed`), + * the same functions `checkApiExposure` (runtime, the HTTP/MCP path) and + * `apiAccessDenialFromEnable` (rest) delegate to. Each surface owns only its + * own envelope; the three-state whitelist, the action→operation mapping and + * the derived verbs resolve identically on all three. + * + * Three behaviours are matched to the HTTP path deliberately, not by accident: + * + * - **`isSystem` bypasses.** These flags govern API *exposure*, so an internal + * engine self-write is not subject to them (`callData`'s first condition). + * - **Unresolvable metadata FAILS OPEN.** A thrown or empty `getObject` falls + * back to the schema defaults (`apiEnabled` true, no whitelist), matching + * `callData`'s `catch { def = undefined }` and `checkApiExposure`'s + * `if (!def) return { allowed: true }`. The fail-open is safe for the reason + * ADR/#3545 records: this is surface area, and the engine's CRUD/FLS/RLS + * still runs on the call regardless of the outcome here. + * - **The flat shape is still read.** `getObject()` returns the flags nested + * under `.enable`, but `checkApiExposure` falls back to a flat top level for + * legacy/test doubles. Reading only the nested shape here would let a flat + * definition be gated on HTTP and ungated on stdio — the very divergence + * this function closes, re-opened one shape down. + */ +async function enforceApiExposure( + metadataService: IMetadataService, + object: string, + action: string, + context: ExecutionContext, +): Promise { + if (context?.isSystem) return; + + let def: ObjectDef | null | undefined; + try { + def = (await metadataService.getObject(object)) as ObjectDef | null | undefined; + } catch { + def = undefined; // fall open to the schema defaults + } + if (!def) return; + + const enable = ( + def.enable && typeof def.enable === 'object' ? def.enable : def + ) as unknown as EnableLike; + + if (enable.apiEnabled === false) { + throw exposureError( + `Object '${object}' is not exposed via the API`, + OBJECT_API_DISABLED, + 404, + ); + } + + const eff = resolveEffectiveApiMethods(enable); + if (eff.mode === 'unrestricted') return; + + const operation = DATA_ACTION_TO_API_OPERATION[action] ?? action; + if (isApiOperationAllowed(eff, operation)) return; + + throw exposureError( + `API operation '${operation}' is not allowed on object '${object}'`, + OBJECT_API_METHOD_NOT_ALLOWED, + 405, + effectiveOperationsArray(eff), + ); +} + /** * Build the stdio transport's principal-bound data bridge. * @@ -163,6 +301,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async query(object, opts) { const context = await resolvePrincipal(); + await enforceApiExposure(metadataService, object, GATED_ACTIONS.query, context); const query: Record = {}; if (opts?.where) query.where = opts.where; if (opts?.fields) query.fields = opts.fields; @@ -175,6 +314,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async get(object, id) { const context = await resolvePrincipal(); + await enforceApiExposure(metadataService, object, GATED_ACTIONS.get, context); // `null` rather than a throw: `get_record` owns the not-found wording on // this path and already branches on a nullish record. return await findById(engine, object, id, context); @@ -182,6 +322,7 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async create(object, data) { const context = await resolvePrincipal(); + await enforceApiExposure(metadataService, object, GATED_ACTIONS.create, context); const written = (await engine.insert(object, data, { context })) as | Record | undefined; @@ -191,6 +332,10 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async update(object, id, data) { const context = await resolvePrincipal(); + // Before the existence probe, not after: `recordNotFound` vs. a hit is an + // observable difference, so gating second would answer "that id names no + // row" for an object the author declared unexposed. + await enforceApiExposure(metadataService, object, GATED_ACTIONS.update, context); const existing = await findById(engine, object, id, context); if (!existing) throw recordNotFound(object, id); await engine.update(object, data, { where: { id }, context }); @@ -199,6 +344,8 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge async remove(object, id) { const context = await resolvePrincipal(); + // Gated before the probe, for the reason `update` states. + await enforceApiExposure(metadataService, object, GATED_ACTIONS.remove, context); const existing = await findById(engine, object, id, context); if (!existing) throw recordNotFound(object, id); await engine.delete(object, { where: { id }, context }); @@ -210,6 +357,10 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge if (typeof engine.aggregate === 'function') { bridge.aggregate = async (object, opts) => { const context = await resolvePrincipal(); + // `aggregate` is a list-class read: an object whose whitelist excludes + // `list` must not leak row statistics through GROUP BY either. The + // derivation lives in the spec helpers, so that holds here for free. + await enforceApiExposure(metadataService, object, GATED_ACTIONS.aggregate, context); // Two casts, one cause: `McpDataBridge.aggregate` declares a WIDER input // than `EngineAggregateOptions` accepts, and the HTTP path never noticed // because it reaches the engine through `callData`'s untyped `params`.