diff --git a/.changeset/mcp-internal-field-evaluate-positions.md b/.changeset/mcp-internal-field-evaluate-positions.md new file mode 100644 index 00000000000..1cf868befef --- /dev/null +++ b/.changeset/mcp-internal-field-evaluate-positions.md @@ -0,0 +1,21 @@ +--- +'@objectstack/mcp': patch +'@objectstack/runtime': patch +'@objectstack/metadata-protocol': patch +--- + +fix(mcp,runtime)!: the MCP data tools refuse to filter, sort, group or aggregate on a field declared `internal: true`, with the data door's own 400 INVALID_FIELD, on the stdio transport and on the HTTP bridge's aggregate_records + +Clause-②: no (narrowing) — the MCP stdio transport's `query_records` and `aggregate_records`, and the HTTP MCP bridge's `aggregate_records`, refuse a query that evaluates a field declared `internal: true`, which they served before. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the MCP data tools (`@objectstack/mcp`, `@objectstack/runtime`), on the gate the data door already applies (`@objectstack/metadata-protocol`). + +- **Before.** A field declared `internal: true` was omitted from every row on every door, and the REST data door (and the HTTP bridge's `query_records`, which reaches it) refused to evaluate one: `400 INVALID_FIELD`. Three MCP mouths hand a caller's query to the engine without reaching that door, and evaluated it: the stdio transport's `query_records` answered a `where` on such a field (`$eq`, `$startsWith`, any operator) and an `orderBy` by it; the stdio transport's `aggregate_records` and the HTTP bridge's `aggregate_records` answered a `where` or an aggregation's own `filter` on it. A group key or an aggregation operand on it was refused by the engine there, in the engine's words. +- **Now.** Each of the three asks the data door's gate (`assertNoInternalFieldEvaluated`, now public on the protocol service) before the engine, for every principal, administrators included, and answers what the door answers for the same query: `400 INVALID_FIELD`, naming the object, the field and the position, in the door's sentence. On MCP the tool result carries that `code`, `status` and message. No row is read. +- **Unchanged.** A query over ordinary fields is served as before; the flagged field is still omitted from every row; a field the caller may not read is still the security middleware's `403 PERMISSION_DENIED`. The engine's privileged lookups that match on such a field (key, token and share-link verification) do not pass through these doors and are untouched. A host that registers no protocol service has no data door for these tools to agree with and serves them as before. +- **Who sees it.** An MCP client that filters, sorts, groups or aggregates on a field its object declares `internal: true`. The built-in objects that declare one are platform `sys_*` objects, which the MCP data tools refuse by default (`allowSystemObjects`); where a host serves them, the same refusal now applies. + +**The remedy.** Filter, sort, group and aggregate on another field. A field that must be queryable is not one to declare `internal: true`. diff --git a/packages/mcp/src/stdio-data-bridge.internal-field-evaluation.test.ts b/packages/mcp/src/stdio-data-bridge.internal-field-evaluation.test.ts new file mode 100644 index 00000000000..6d9b47c3627 --- /dev/null +++ b/packages/mcp/src/stdio-data-bridge.internal-field-evaluation.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The stdio data bridge asks the data door's internal-field gate — the + * `protocol` service's `assertNoInternalFieldEvaluated` — before its engine + * reads anything, on `query` (the call's `where` and `orderBy`) and on + * `aggregate` (`where`, `groupBy`, `aggregations`), and a refusal it throws + * reaches the caller as thrown: the tool renders its `code`, `status` and + * sentence, and the engine is never asked. + * + * The gate itself is the protocol's and is pinned there + * (`@objectstack/metadata-protocol`, the public-face cases beside the data + * door's position enumeration), and the whole path on a real stack in + * `@objectstack/dogfood`. This file pins the bridge's half: that it asks, with + * what, before what — and that a host whose protocol has no such face (or that + * registers no protocol) is served exactly as before. Refusals are asserted on + * the envelope, never on a bare throw. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; +import { createStdioDataBridge, type StdioProtocolObjectReads } from './stdio-data-bridge.js'; +import { registerObjectTools } from './mcp-http-tools.js'; + +const OBJECT = 'ifx_vault'; +const FLAGGED = 'pin'; +const DEF = { name: OBJECT, fields: { name: { type: 'text' }, [FLAGGED]: { type: 'text' } } }; + +/** The door's refusal as the protocol throws it (fields only; the sentence is the protocol's to pin). */ +function doorRefusal(param: string) { + return Object.assign(new Error(`Cannot evaluate '${OBJECT}' by '${FLAGGED}' (${param}): the query was not run.`), { + code: 'INVALID_FIELD', status: 400, httpStatus: 400, object: OBJECT, field: FLAGGED, fields: [FLAGGED], param, + }); +} + +/** A stand-in for the protocol's gate: refuses any query whose JSON names the flagged field. */ +function gate() { + return vi.fn((_object: string, query: Record) => { + if (JSON.stringify(query).includes(`"${FLAGGED}"`)) throw doorRefusal('filter'); + }); +} + +function makeBridge(protocol?: StdioProtocolObjectReads) { + const engine = { + find: vi.fn(async () => [{ id: 'r1', name: 'alpha' }]), + aggregate: vi.fn(async () => [{ n: 1 }]), + }; + const metadataService = { getObject: vi.fn(async (name: string) => (name === OBJECT ? DEF : undefined)) }; + const bridge = createStdioDataBridge({ + engine: engine as unknown as IDataEngine, + metadataService: metadataService as unknown as IMetadataService, + ...(protocol ? { protocol: () => protocol } : {}), + resolvePrincipal: async () => ({ userId: 'u1', isSystem: false }) as unknown as ExecutionContext, + }); + return { bridge, engine }; +} + +const refusalOf = async (run: () => Promise) => { + const err: any = await run().then(() => undefined, (e: unknown) => e); + expect(err, 'the call resolved: the gate\'s refusal did not reach the caller').toBeDefined(); + return { code: err.code, status: err.status, field: err.field, object: err.object }; +}; +const REFUSED = { code: 'INVALID_FIELD', status: 400, field: FLAGGED, object: OBJECT }; + +describe('the stdio bridge asks the protocol\'s internal-field gate before the engine', () => { + it('query: the gate is asked with the call\'s where and orderBy, and its refusal stops the read', async () => { + const assertNoInternalFieldEvaluated = gate(); + const { bridge, engine } = makeBridge({ assertNoInternalFieldEvaluated }); + const where = { [FLAGGED]: { $startsWith: 'x' } }; + const orderBy = [{ field: 'name', order: 'asc' as const }]; + expect(await refusalOf(() => bridge.query(OBJECT, { where, orderBy, limit: 5 }))).toEqual(REFUSED); + expect(assertNoInternalFieldEvaluated).toHaveBeenCalledWith(OBJECT, { where, orderBy }); + expect(engine.find).not.toHaveBeenCalled(); + + const sorted = [{ field: FLAGGED, order: 'desc' as const }]; + expect(await refusalOf(() => bridge.query(OBJECT, { orderBy: sorted }))).toEqual(REFUSED); + expect(engine.find).not.toHaveBeenCalled(); + }); + + it('aggregate: the gate is asked with where, groupBy and aggregations, and its refusal stops the aggregate', async () => { + const assertNoInternalFieldEvaluated = gate(); + const { bridge, engine } = makeBridge({ assertNoInternalFieldEvaluated }); + const bodies = [ + { where: { [FLAGGED]: 'x' }, aggregations: [{ function: 'count' as const, alias: 'n' }] }, + { groupBy: [FLAGGED], aggregations: [{ function: 'count' as const, alias: 'n' }] }, + { aggregations: [{ function: 'max' as const, field: FLAGGED, alias: 'm' }] }, + { aggregations: [{ function: 'count' as const, alias: 'n', filter: { [FLAGGED]: 'x' } }] }, + ]; + for (const body of bodies) { + expect(await refusalOf(() => bridge.aggregate!(OBJECT, body))).toEqual(REFUSED); + expect(assertNoInternalFieldEvaluated).toHaveBeenLastCalledWith(OBJECT, { + where: (body as { where?: unknown }).where, + groupBy: (body as { groupBy?: unknown }).groupBy, + aggregations: body.aggregations, + }); + } + expect(engine.aggregate).not.toHaveBeenCalled(); + }); + + it('CONTROL — a query the gate passes reaches the engine unchanged, on both verbs', async () => { + const assertNoInternalFieldEvaluated = gate(); + const { bridge, engine } = makeBridge({ assertNoInternalFieldEvaluated }); + const where = { name: 'alpha' }; + await bridge.query(OBJECT, { where, orderBy: [{ field: 'name', order: 'asc' }] }); + expect(engine.find).toHaveBeenCalledTimes(1); + expect((engine.find.mock.calls[0] as unknown[])[1]).toMatchObject({ where, orderBy: [{ field: 'name', order: 'asc' }] }); + await bridge.aggregate!(OBJECT, { where, groupBy: ['name'], aggregations: [{ function: 'count', alias: 'n' }] }); + expect(engine.aggregate).toHaveBeenCalledTimes(1); + expect(assertNoInternalFieldEvaluated).toHaveBeenCalledTimes(2); + }); + + it('CONTROL — no protocol, or one without the face: served as before (no door to agree with)', async () => { + for (const protocol of [undefined, {} as StdioProtocolObjectReads]) { + const { bridge, engine } = makeBridge(protocol); + await bridge.query(OBJECT, { where: { [FLAGGED]: 'x' } }); + await bridge.aggregate!(OBJECT, { where: { [FLAGGED]: 'x' }, aggregations: [{ function: 'count', alias: 'n' }] }); + expect(engine.find).toHaveBeenCalledTimes(1); + expect(engine.aggregate).toHaveBeenCalledTimes(1); + } + }); + + it('query_records and aggregate_records render the gate\'s refusal as its envelope', async () => { + const { bridge, engine } = makeBridge({ assertNoInternalFieldEvaluated: gate() }); + const server = new McpServer({ name: 'pin', version: '0' }); + registerObjectTools(server, bridge); + const [clientSide, serverSide] = InMemoryTransport.createLinkedPair(); + await server.connect(serverSide); + const client = new Client({ name: 'pin', version: '0' }); + await client.connect(clientSide); + try { + for (const [name, args] of [ + ['query_records', { objectName: OBJECT, where: { [FLAGGED]: 'x' } }], + ['aggregate_records', { objectName: OBJECT, groupBy: [FLAGGED], aggregations: [{ function: 'count', alias: 'n' }] }], + ] as const) { + const res = (await client.callTool({ name, arguments: args })) as { isError?: boolean; content: Array<{ text: string }> }; + expect(res.isError, name).toBe(true); + const { error } = JSON.parse(res.content[0]!.text) as { error: { code: unknown; status: unknown; message: string } }; + expect({ code: error.code, status: error.status }).toEqual({ code: 'INVALID_FIELD', status: 400 }); + expect(error.message).toBe(doorRefusal('filter').message); + } + expect(engine.find).not.toHaveBeenCalled(); + expect(engine.aggregate).not.toHaveBeenCalled(); + } finally { + await client.close(); + } + }); +}); diff --git a/packages/mcp/src/stdio-data-bridge.ts b/packages/mcp/src/stdio-data-bridge.ts index 6aaa0d9e507..4de8421a5c9 100644 --- a/packages/mcp/src/stdio-data-bridge.ts +++ b/packages/mcp/src/stdio-data-bridge.ts @@ -89,8 +89,20 @@ * this engine-only reader served it as stored — to an administrator's key, a * member being refused by the engine as usual. The family's one projection is * now applied here and the evaluate shapes are refused; see - * {@link storedMetadataBodyRefusal} and {@link serveStoredMetadataRows}. What - * remains of the divergence above is genuinely not security. + * {@link storedMetadataBodyRefusal} and {@link serveStoredMetadataRows}. + * + * ⚠️ A THIRD limb was security as well: the EVALUATE positions of a field + * declared `internal: true`. The engine omits such a field from every row, but + * it still evaluates one named in a filter or a sort, and the protocol's data + * door is where those positions are refused — for every caller, administrators + * included, before the engine is asked. This engine-only bridge evaluated them. + * `query` and `aggregate` now ask that door's ONE gate of the `protocol` + * service before the engine ({@link refuseInternalFieldEvaluation}), so a + * filter, a sort, a group key or an aggregation over such a field answers the + * door's `400 INVALID_FIELD`, word for word, on this transport as on REST and + * on the HTTP bridge. ⛔ Never a copy of that gate here: a second definition is + * the fork this note exists to close. What remains of the divergence above is + * genuinely not security. */ import { omitInternalFieldsFromWriteResponse, relationConditionSites } from '@objectstack/core'; @@ -133,8 +145,8 @@ import { } from './metadata-completeness.js'; /** - * [#25138] The object reads this transport asks of the `protocol` service. - * Every member is optional: the slot is filled by name from a host-owned + * [#25138] The object reads this transport asks of the `protocol` service, + * and the data door's internal-field gate beside them. Every member is optional: the slot is filled by name from a host-owned * registry, and a host may fill it with a partial object (the same duck-typed * probe `plugin.ts` applies to `getMetaItems` for the skill read). * `getMetaItemForExecution` is the server-only by-name face the REST exposure @@ -148,6 +160,14 @@ export interface StdioProtocolObjectReads { getMetaItem?(request: { type: string; name: string }): Promise; /** The by-name read without the serving decorations. */ getMetaItemForExecution?(request: { type: string; name: string }): Promise; + /** + * The data door's evaluate refusal for a field declared `internal: true`: + * throws its `400 INVALID_FIELD` when `query` filters, sorts, groups or + * aggregates on one, and returns otherwise. Not a read, but asked of the + * same service at the same moment, so it rides this slot + * ({@link refuseInternalFieldEvaluation}). + */ + assertNoInternalFieldEvaluated?(object: string, query: Record): void; } /** [#25138] Where this transport reads an object's definition from. */ @@ -548,6 +568,38 @@ async function enforceRelationTargetExposure( } } +/** + * The data door's evaluate refusal for a field declared `internal: true`, + * asked of the `protocol` service before the engine reads anything: the + * protocol's own `assertNoInternalFieldEvaluated`, the gate `findData` asks + * for REST and for the HTTP bridge's `query_records` — ⛔ never a copy of it + * here (route-ownership rule 1). It throws that door's `400 INVALID_FIELD`, + * naming the object, the field and the position in the door's own sentence, + * for a filter (`where`, each aggregation's own `filter`, a nested-relation + * condition judged on its target), a sort, a group key or an aggregation + * operand on such a field — for every principal this transport admits. A + * query over ordinary fields passes untouched. + * + * Asked whatever the caller's context, as the door asks it: the engine's + * privileged lookups that legitimately match on such a field call the engine + * directly and never reach this bridge. + * + * Structural absence answers nothing: on a host that registers no protocol, + * or one whose protocol has no such face, there is no data door whose verdict + * this transport could match, and the call proceeds to the engine as before. + * An object the protocol's registry does not hold is likewise left to the + * engine's existence answer, as the door leaves it. + */ +function refuseInternalFieldEvaluation( + reads: StdioObjectReads, + object: string, + query: Record, +): void { + const protocol = protocolOf(reads); + if (typeof protocol?.assertNoInternalFieldEvaluated !== 'function') return; + protocol.assertNoInternalFieldEvaluated(object, query); +} + // --------------------------------------------------------------------------- // [#21207] The stored-metadata-body family, at this engine-only reader // --------------------------------------------------------------------------- @@ -954,6 +1006,9 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge // [#21207] …and the same for a stored content-hash column. const hashRefusal = storedMetadataHashRefusal(object, { where: opts?.where, orderBy: opts?.orderBy }); if (hashRefusal) throw hashRefusal; + // …and a filter or sort on an `internal: true` field, by the data door's + // own gate, after the stored-metadata refusals as the door orders them. + refuseInternalFieldEvaluation(reads, object, { where: opts?.where, orderBy: opts?.orderBy }); const read = storedMetadataBodyReadFields(object, opts?.fields); const query: Record = {}; if (opts?.where) query.where = opts.where; @@ -1077,6 +1132,14 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge aggregations: opts?.aggregations, }); if (hashRefusal) throw hashRefusal; + // …and a filter (`where` or an aggregation's own), a group key or an + // aggregation operand on an `internal: true` field, by the data door's + // own gate ({@link refuseInternalFieldEvaluation}). + refuseInternalFieldEvaluation(reads, object, { + where: opts?.where, + groupBy: opts?.groupBy, + aggregations: opts?.aggregations, + }); // No casts: `McpDataBridge.aggregate` declares the engine's own // `EngineAggregateOptions` slices since #8032, so the honest call // compiles — the two `as unknown as` casts this line used to carry diff --git a/packages/metadata-protocol/src/protocol.data-door-internal-field-positions.test.ts b/packages/metadata-protocol/src/protocol.data-door-internal-field-positions.test.ts index 55907142a5c..db1917256ae 100644 --- a/packages/metadata-protocol/src/protocol.data-door-internal-field-positions.test.ts +++ b/packages/metadata-protocol/src/protocol.data-door-internal-field-positions.test.ts @@ -251,3 +251,53 @@ describe('[#22646] CONTROL — ordinary fields through a relation and at depth a } } }); + +/** + * The PUBLIC face the MCP data mouths ask (`assertNoInternalFieldEvaluated`): + * the MCP stdio transport's `query_records` / `aggregate_records` and the HTTP + * bridge's `aggregate_records` hand a caller's query to the engine without + * passing through `findData`, so each asks this method of the `protocol` + * service first. It must be the door's own verdict, not a second one: for each + * canonical query those mouths can send, the face throws exactly what + * `findData` throws for the same query (code, status, object, field, param, + * message), and an ordinary-field query passes it without a throw. + */ +describe('the public internal-field face answers exactly what findData answers', () => { + const shapes: Array<[string, Record]> = [ + ['filter ($eq)', { where: { [F]: { $eq: 'h1' } } }], + ['filter ($startsWith)', { where: { [F]: { $startsWith: 'h' } } }], + ['sort (orderBy node)', { orderBy: [{ field: F, order: 'asc' }] }], + ['aggregate where', { where: { [F]: 'h1' }, aggregations: [{ function: 'count', alias: 'n' }] }], + ['group key', { groupBy: [F], aggregations: [{ function: 'count', alias: 'n' }] }], + ['aggregation operand', { aggregations: [{ function: 'max', field: F, alias: 'm' }] }], + ["aggregation's own filter", { aggregations: [{ function: 'count', alias: 'n', filter: { [F]: 'h1' } }] }], + ['relation condition onto the target', { where: { vault_id: { [F]: 'h1' } } }], + ]; + const envelope = (e: any) => ({ code: e?.code, status: e?.status, object: e?.object, field: e?.field, param: e?.param, message: e?.message }); + + for (const [label, query] of shapes) { + it(`${label}: the face throws the door's refusal, field for field`, async () => { + const object = label.startsWith('relation') ? 'pv_holder' : 'pv_vault'; + const { p, find, aggregate } = makeProtocol(); + const door = await refusal(() => p.findData({ object, query: { ...query } })); + let thrown: unknown; + try { p.assertNoInternalFieldEvaluated(object, { ...query }); } catch (e) { thrown = e; } + expect(thrown, 'the face let a position the door refuses through').toBeDefined(); + expect(envelope(thrown)).toEqual(envelope(door)); + expect(envelope(thrown)).toMatchObject({ code: 'INVALID_FIELD', status: 400 }); + expect(find).not.toHaveBeenCalled(); + expect(aggregate).not.toHaveBeenCalled(); + }); + } + + it('CONTROL — ordinary fields in every position pass the face without a throw', () => { + const { p } = makeProtocol(); + expect(() => p.assertNoInternalFieldEvaluated('pv_vault', { + where: { name: { $startsWith: 'a' } }, + orderBy: [{ field: 'name', order: 'asc' }], + groupBy: ['name'], + aggregations: [{ function: 'max', field: 'rank', alias: 'm', filter: { name: 'alpha' } }], + })).not.toThrow(); + expect(() => p.assertNoInternalFieldEvaluated('pv_holder', { where: { vault_id: { name: 'alpha' } } })).not.toThrow(); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index b2b42b025ec..fc3f10a2ba4 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -13288,6 +13288,16 @@ export class ObjectStackProtocolImplementation implements * ever returning the value — is untouched, exactly as #7823 kept the engine's * write results whole while the generic-data-path ingress stripped them. * + * PUBLIC, and asked by the generic data mouths that hand a caller's query + * to the engine without passing through {@link findData}: the MCP stdio + * transport's `query_records` / `aggregate_records` and the HTTP MCP + * bridge's `aggregate_records` arm (`callData`'s `aggregate`). Each asks + * this method on the `protocol` service it already resolves, before the + * engine, so a filter, sort, group key or aggregation over an `internal` + * field gets this door's refusal, word for word, on every transport — + * one definition, never a copy beside it (route-ownership rule 1). The + * engine's privileged consumers above still never reach it. + * * Positions, with the ONE flag reader ({@link collectInternalWriteResponseFields}, * `@objectstack/core`'s cross-package twin of objectql's * `collectInternalReadFields` — identical rule, imported because this package @@ -13327,7 +13337,7 @@ export class ObjectStackProtocolImplementation implements * door). A dotted sort inside an `expand` entry is admitted but orders a * batch the expansion re-keys by id with no `limit`, so it changes no answer. */ - private assertNoInternalFieldEvaluated(object: string, options: Record): void { + assertNoInternalFieldEvaluated(object: string, options: Record): void { // [#25467 cut 3] The body moved to `protocol-query-validation.ts` (see `QueryValidationHost`). const host: ThisParameterType = { groupByFieldNames: (...a) => this.groupByFieldNames(...a), diff --git a/packages/qa/dogfood/test/mcp-internal-field-evaluate-positions.dogfood.test.ts b/packages/qa/dogfood/test/mcp-internal-field-evaluate-positions.dogfood.test.ts new file mode 100644 index 00000000000..1a13acd04e2 --- /dev/null +++ b/packages/qa/dogfood/test/mcp-internal-field-evaluate-positions.dogfood.test.ts @@ -0,0 +1,368 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The MCP data tools refuse to EVALUATE a field declared `internal: true` the + * way the REST data door does, on both transports, for an administrator's + * principal and a member's: a filter (`$eq`, `$startsWith`), a sort, an + * aggregate's `where`, a group key, an aggregation operand and an + * aggregation's own `filter` each answer the door's `400 INVALID_FIELD` with + * the door's own sentence. + * + * ## What is pinned + * + * The data door refuses those positions for every caller + * (`assertNoInternalFieldEvaluated`, `@objectstack/metadata-protocol`), before + * the engine is asked: the engine omits such a field from every row, but it + * still evaluates one named in a predicate or an order. Three MCP mouths hand a + * caller's query to the engine without passing through `findData`, where the + * door asks its gate — the stdio transport's `query_records` and + * `aggregate_records`, and the HTTP bridge's `aggregate_records` — and each now + * asks that same gate of the `protocol` service. The HTTP bridge's + * `query_records` reaches `findData` itself; it is driven here too, as the + * parity row the other three are held to. + * + * Every refusal is compared with what REST answers for the same body — + * `code`, `status` and `message`, read from REST in this run, never a sentence + * typed into this file — so a door that refused with another message (the + * engine's own aggregation refusal, which a group key or an operand met before) + * is told apart from the door's. + * + * ## Controls + * + * The same principals filter, sort, aggregate and group on an ORDINARY field + * of the same object through every mouth, and are served — with a filter that + * narrows (one row of two), so "served" is not "served everything" — and the + * flagged column is still absent from every served row. A door that refused + * every query would fail these. + * + * ## What is real + * + * The showcase stack with its security and `MCPServerPlugin` (the HTTP bridge + * at `/api/v1/mcp`), a runtime object authored through `PUT /meta/object`, and + * one stdio `MCPServerPlugin` per principal started with `autoStart` over the + * same kernel's services, its `OS_MCP_STDIO_API_KEY` a key minted through + * `POST /keys` by that principal. The session is the plugin's own server over + * an in-process transport speaking the JSON-RPC a stdio client speaks (a + * process cannot give a test its stdin), and `start()`'s claim of the real + * stdin/stdout is the one call stubbed. + * + * The negative control — every stdio row and the HTTP aggregate rows served, + * or answered in the engine's words, with the two MCP-side gate calls removed — + * is a one-time ablation cited in the PR, not a permanent case. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { type VerifyStack } from '@objectstack/verify'; +import { MCPServerPlugin, MCPServerRuntime } from '@objectstack/mcp'; +import type { PluginContext } from '@objectstack/core'; +import { bootShowcase } from './showcase-boot.js'; +import { showcaseAppDefaultSecurity } from './showcase-security.js'; +import { catalogPermissionSet } from './catalog-permission-set.js'; + +const OBJECT = 'mcpif_vault'; +const FLAGGED = 'pin'; +const ORDINARY = 'tier'; +const SYS = { context: { isSystem: true } } as const; +const MEMBER_EMAIL = 'mcp-internal-field@verify.test'; +/** Object-level read on the runtime object, the member's only grant on it. */ +const READER_SET = 'mcpif_vault_reader'; + +/** The runtime object: one ordinary column to control with, one flagged `internal: true`. */ +const DEFINITION = { + name: OBJECT, + label: 'MCP Internal Field Vault', + sharingModel: 'public_read_write', + fields: { + name: { type: 'text', label: 'Name' }, + [ORDINARY]: { type: 'text', label: 'Tier' }, + [FLAGGED]: { type: 'text', label: 'Pin', internal: true }, + }, +}; +const ROWS = [ + { name: 'alpha', [ORDINARY]: 'gold', [FLAGGED]: 'fixture-a' }, + { name: 'beta', [ORDINARY]: 'silver', [FLAGGED]: 'fixture-b' }, +]; + +const COUNT = { function: 'count', alias: 'n' }; + +/** + * Each evaluate position: the MCP tool and its arguments, and the REST + * `POST /data/:object/query` body that asks the door the same question. + */ +const POSITIONS: Array<{ label: string; tool: 'query_records' | 'aggregate_records'; args: Record; rest: Record }> = [ + { + label: 'filter ($eq)', + tool: 'query_records', + args: { where: { [FLAGGED]: { $eq: 'fixture-a' } } }, + rest: { where: { [FLAGGED]: { $eq: 'fixture-a' } } }, + }, + { + label: 'filter ($startsWith)', + tool: 'query_records', + args: { where: { [FLAGGED]: { $startsWith: 'fixture' } } }, + rest: { where: { [FLAGGED]: { $startsWith: 'fixture' } } }, + }, + { + label: 'sort', + tool: 'query_records', + args: { orderBy: [{ field: FLAGGED, order: 'asc' }] }, + rest: { orderBy: [{ field: FLAGGED, order: 'asc' }] }, + }, + { + label: 'aggregate where', + tool: 'aggregate_records', + args: { where: { [FLAGGED]: 'fixture-a' }, aggregations: [COUNT] }, + rest: { where: { [FLAGGED]: 'fixture-a' }, aggregations: [COUNT] }, + }, + { + label: 'aggregate group key', + tool: 'aggregate_records', + args: { groupBy: [FLAGGED], aggregations: [COUNT] }, + rest: { groupBy: [FLAGGED], aggregations: [COUNT] }, + }, + { + label: 'aggregation operand', + tool: 'aggregate_records', + args: { aggregations: [{ function: 'max', field: FLAGGED, alias: 'm' }] }, + rest: { aggregations: [{ function: 'max', field: FLAGGED, alias: 'm' }] }, + }, + { + label: "aggregation's own filter", + tool: 'aggregate_records', + args: { aggregations: [{ ...COUNT, filter: { [FLAGGED]: 'fixture-a' } }] }, + rest: { aggregations: [{ ...COUNT, filter: { [FLAGGED]: 'fixture-a' } }] }, + }, +]; + +/** The two engine verbs the fixture writes and reads through, in system context. */ +interface FixtureEngine { + insert(object: string, data: Record, options: typeof SYS): Promise; + findOne(object: string, query: { where: Record } & typeof SYS): Promise<{ id?: string } | null>; +} + +/** What a tool call answered: `isError`, the JSON it returned (or its bare text). */ +interface ToolAnswer { + isError: boolean; + text: string; + payload: any; +} + +type Call = (tool: string, args: Record) => Promise; + +function parse(text: string): any { + try { + return JSON.parse(text); + } catch { + return text; + } +} + +/** An in-process MCP transport: the server's replies are matched back to the request that asked. */ +function inProcessTransport() { + const waiting = new Map void>(); + let nextId = 1; + const transport: any = { + onmessage: undefined, + onclose: undefined, + onerror: undefined, + async start() {}, + async close() { transport.onclose?.(); }, + async send(message: any) { + const resolve = typeof message?.id === 'number' ? waiting.get(message.id) : undefined; + if (resolve) { waiting.delete(message.id); resolve(message); } + }, + }; + const rpc = (method: string, params?: unknown) => new Promise((resolve, reject) => { + const id = nextId++; + const giveUp = setTimeout(() => reject(new Error(`no answer to "${method}" (id ${id}) within 10s`)), 10_000); + waiting.set(id, (frame) => { clearTimeout(giveUp); resolve(frame); }); + transport.onmessage?.({ jsonrpc: '2.0', id, method, ...(params ? { params } : {}) }); + }); + return { transport, rpc, notify: (method: string) => transport.onmessage?.({ jsonrpc: '2.0', method }) }; +} + +describe('MCP data tools refuse an internal field in every evaluate position, as the data door does', () => { + let stack: VerifyStack; + const sessionTokens: Record = {}; + const stdio: Record = {}; + const started: MCPServerPlugin[] = []; + const originalKey = process.env.OS_MCP_STDIO_API_KEY; + + /** HTTP `/mcp` tools/call under a session token. */ + const httpCall = (token: string): Call => async (tool, args) => { + const res = await stack.api('/mcp', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: tool, arguments: { objectName: OBJECT, ...args } } }), + }); + expect(res.status, `MCP transport: ${await res.clone().text()}`).toBe(200); + const rpc: any = await res.json(); + expect(rpc.error, `JSON-RPC error: ${JSON.stringify(rpc.error)}`).toBeUndefined(); + const text: string = rpc.result?.content?.[0]?.text ?? ''; + return { isError: rpc.result?.isError === true, text, payload: parse(text) }; + }; + + /** + * A stdio session bound to `apiKey`: a fresh `MCPServerPlugin` started with + * `autoStart` over this kernel's own services. Registrations stay local, so + * the showcase's own MCP plugin is untouched. + */ + async function stdioSession(apiKey: string): Promise { + const local = new Map(); + const host = { + registerService: (name: string, service: unknown) => { local.set(name, service); }, + getService: (name: string): unknown => (local.has(name) ? local.get(name) : stack.kernel.getService(name)), + replaceService: (name: string, service: unknown) => { local.set(name, service); }, + hook: () => {}, + trigger: async () => {}, + logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }, + getKernel: () => stack.kernel, + }; + // The members the plugin reads; the rest of the kernel's context is not asked here. + const ctx = host as unknown as PluginContext; + process.env.OS_MCP_STDIO_API_KEY = apiKey; + const plugin = new MCPServerPlugin({ autoStart: true }); + try { + await plugin.init(ctx); + const runtime = local.get('mcp') as MCPServerRuntime; + // `start()` would claim this process's real stdin/stdout. + vi.spyOn(runtime, 'start').mockResolvedValue(undefined); + await plugin.start(ctx); + started.push(plugin); + const session = inProcessTransport(); + await runtime.server.connect(session.transport as never); + await session.rpc('initialize', { + protocolVersion: '2025-06-18', + capabilities: {}, + clientInfo: { name: 'mcp-internal-field-evaluate-positions', version: '0.0.0' }, + }); + session.notify('notifications/initialized'); + return async (tool, args) => { + const frame = await session.rpc('tools/call', { name: tool, arguments: { objectName: OBJECT, ...args } }); + expect(frame.error, `tools/call ${tool} failed at the protocol level`).toBeUndefined(); + const text = frame.result.content[0].text as string; + return { isError: frame.result.isError === true, text, payload: parse(text) }; + }; + } finally { + // The key is read once, in `start()`. + if (originalKey === undefined) delete process.env.OS_MCP_STDIO_API_KEY; + else process.env.OS_MCP_STDIO_API_KEY = originalKey; + } + } + + /** `POST /keys` as `token`: the one door a key is minted through. */ + async function mintKey(token: string, name: string): Promise { + const res = await stack.apiAs(token, 'POST', '/keys', { name }); + expect(res.status, await res.clone().text()).toBe(201); + const key = (await res.json())?.data?.key; + expect(typeof key).toBe('string'); + return String(key); + } + + /** + * `POST /data/:object/query` as `persona`. The data door's refusal envelope is + * flat: the code beside the sentence, which rides under `error`. + */ + async function rest(persona: string, body: Record) { + const res = await stack.apiAs(sessionTokens[persona]!, 'POST', `/data/${OBJECT}/query`, body); + const json: any = await res.json(); + return { status: res.status, code: json?.code, message: json?.error, json }; + } + + beforeAll(async () => { + stack = await bootShowcase({ + extraPlugins: [new MCPServerPlugin()], + security: showcaseAppDefaultSecurity(), + }); + sessionTokens.admin = await stack.signIn(); + sessionTokens.member = await stack.signUp(MEMBER_EMAIL); + + const saved = await stack.apiAs(sessionTokens.admin, 'PUT', `/meta/object/${OBJECT}`, DEFINITION); + expect(saved.status, `runtime object authored: ${await saved.clone().text()}`).toBe(200); + const ql = await stack.kernel.getServiceAsync('objectql'); + for (const row of ROWS) await ql.insert(OBJECT, row, SYS); + + // The member reads the object through one authored set and nothing else. + const authored = await stack.apiAs(sessionTokens.admin, 'PUT', `/meta/permission/${READER_SET}`, { + name: READER_SET, + label: 'Scratch: object read on the MCP internal-field fixture', + objects: { [OBJECT]: { allowRead: true } }, + fields: {}, + }); + expect(authored.status, `reader set authored: ${await authored.clone().text()}`).toBeLessThan(300); + const set = await catalogPermissionSet(ql, READER_SET); + expect(set, 'reader set served by the catalog').toBeTruthy(); + const memberId = (await ql.findOne('sys_user', { where: { email: MEMBER_EMAIL }, ...SYS }))?.id; + expect(memberId, 'member provisioned').toBeTruthy(); + await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set: set.name }, SYS); + + for (const persona of ['admin', 'member'] as const) { + stdio[persona] = await stdioSession(await mintKey(sessionTokens[persona]!, `mcp-internal-field-${persona}`)); + } + }, 240_000); + + afterAll(async () => { + for (const plugin of started) await plugin.destroy().catch(() => {}); + if (originalKey === undefined) delete process.env.OS_MCP_STDIO_API_KEY; + else process.env.OS_MCP_STDIO_API_KEY = originalKey; + vi.restoreAllMocks(); + await stack?.stop?.(); + }); + + const transports = { + stdio: (persona: string) => stdio[persona]!, + http: (persona: string) => httpCall(sessionTokens[persona]!), + } as const; + + for (const position of POSITIONS) { + for (const persona of ['admin', 'member'] as const) { + for (const [transport, callFor] of Object.entries(transports)) { + it(`${position.tool} ${position.label} — ${transport}, ${persona}: the door's 400 INVALID_FIELD, word for word`, async () => { + const door = await rest(persona, position.rest); + expect({ status: door.status, code: door.code }, `REST: ${JSON.stringify(door.json)}`).toEqual({ status: 400, code: 'INVALID_FIELD' }); + expect(typeof door.message, `REST: ${JSON.stringify(door.json)}`).toBe('string'); + + const answer = await callFor(persona)(position.tool, position.args); + expect(answer.isError, `${transport} served the position: ${answer.text}`).toBe(true); + expect({ + code: answer.payload?.error?.code, + status: answer.payload?.error?.status, + message: answer.payload?.error?.message, + }).toEqual({ code: 'INVALID_FIELD', status: 400, message: door.message }); + }); + } + } + } + + for (const persona of ['admin', 'member'] as const) { + for (const [transport, callFor] of Object.entries(transports)) { + it(`CONTROL ${transport}, ${persona}: an ordinary field filters, sorts, aggregates and groups; the flagged column stays off every row`, async () => { + const call = callFor(persona); + + const filtered = await call('query_records', { where: { [ORDINARY]: 'gold' }, orderBy: [{ field: ORDINARY, order: 'desc' }] }); + expect(filtered.isError, filtered.text).toBe(false); + expect(filtered.payload.records.map((r: any) => r.name)).toEqual(['alpha']); + + const sorted = await call('query_records', { orderBy: [{ field: ORDINARY, order: 'asc' }] }); + expect(sorted.isError, sorted.text).toBe(false); + expect(sorted.payload.records.map((r: any) => r.name)).toEqual(['alpha', 'beta']); + for (const row of [...filtered.payload.records, ...sorted.payload.records]) expect(row).not.toHaveProperty(FLAGGED); + + const counted = await call('aggregate_records', { where: { [ORDINARY]: 'gold' }, aggregations: [COUNT, { ...COUNT, alias: 'all', filter: { [ORDINARY]: { $ne: 'none' } } }] }); + expect(counted.isError, counted.text).toBe(false); + expect(counted.payload.rows.map((r: any) => ({ n: Number(r.n), all: Number(r.all) }))).toEqual([{ n: 1, all: 1 }]); + + const grouped = await call('aggregate_records', { groupBy: [ORDINARY], aggregations: [COUNT] }); + expect(grouped.isError, grouped.text).toBe(false); + const groups = grouped.payload.rows + .map((r: any) => ({ [ORDINARY]: r[ORDINARY], n: Number(r.n) })) + .sort((a: any, b: any) => String(a[ORDINARY]).localeCompare(String(b[ORDINARY]))); + expect(groups).toEqual([{ [ORDINARY]: 'gold', n: 1 }, { [ORDINARY]: 'silver', n: 1 }]); + }); + } + } +}); diff --git a/packages/runtime/src/action-execution-calldata-aggregate-internal-field.test.ts b/packages/runtime/src/action-execution-calldata-aggregate-internal-field.test.ts new file mode 100644 index 00000000000..6c7d451cc5e --- /dev/null +++ b/packages/runtime/src/action-execution-calldata-aggregate-internal-field.test.ts @@ -0,0 +1,110 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `callData`'s `aggregate` arm — the HTTP MCP bridge's `aggregate_records` — + * hands a caller's `where`, `groupBy` and `aggregations` to the ObjectQL engine + * without passing through the protocol's `findData`, where the data door + * refuses to EVALUATE a field declared `internal: true`. So the arm asks the + * door's own gate (`assertNoInternalFieldEvaluated`) of the `protocol` service + * before the engine aggregates, and throws what the door throws. + * + * Driven against the REAL `ObjectStackProtocolImplementation` registered as the + * `protocol` service, and asserted against it: for each position the refusal is + * the one `findData` itself answers for the same body — code, status, object, + * field, position and message — and the engine never aggregates. Every caller + * meets it, a system context included, as at the door. CONTROLS: an ordinary + * field in every position reaches the engine unchanged, and a host that + * registers no protocol has no door to agree with and is served as before. + */ + +import { describe, expect, it, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { callData, type ActionExecutionDeps } from './action-execution.js'; +import type { HttpProtocolContext } from './http-dispatcher.js'; + +const REQ = { request: {} } as HttpProtocolContext; +const MEMBER = { userId: 'ai_member', isSystem: false, positions: [], permissions: [] } as any; +const ADMIN = { userId: 'ai_admin', isSystem: false, roles: ['admin'], isPlatformAdmin: true, positions: [], permissions: [] } as any; +const SYSTEM = { isSystem: true } as any; + +const F = 'pin'; +const SCHEMAS: Record = { + ai_vault: { + name: 'ai_vault', + label: 'Vault', + fields: { + id: { name: 'id', label: 'ID', type: 'text' }, + name: { name: 'name', label: 'Name', type: 'text' }, + rank: { name: 'rank', label: 'Rank', type: 'number' }, + [F]: { name: F, label: 'Pin', type: 'text', internal: true }, + }, + }, +}; + +function makeHarness(opts: { withProtocol?: boolean } = {}) { + const aggregate = vi.fn(async () => [{ n: 1 }]); + const find = vi.fn(async () => [{ id: 'v1', name: 'alpha', rank: 1 }]); + const engine: any = { registry: { getObject: (n: string) => SCHEMAS[n] }, find, aggregate }; + const protocol = new ObjectStackProtocolImplementation(engine); + const services: Record = { + metadata: { getObject: async (n: string) => SCHEMAS[n] }, + objectql: engine, + ...(opts.withProtocol === false ? {} : { protocol }), + }; + const deps: ActionExecutionDeps = { + resolveService: (async (_ctx: HttpProtocolContext, name: string) => services[name]) as any, + getObjectQL: async () => engine, + }; + return { deps, aggregate, protocol }; +} + +const envelope = (e: any) => ({ code: e?.code, status: e?.status, object: e?.object, field: e?.field, param: e?.param, message: e?.message }); + +const COUNT = { function: 'count', alias: 'n' }; +const POSITIONS: Array<[string, { where?: unknown; groupBy?: unknown[]; aggregations: unknown[] }]> = [ + ['where', { where: { [F]: 'x' }, aggregations: [COUNT] }], + ['where ($startsWith)', { where: { [F]: { $startsWith: 'x' } }, aggregations: [COUNT] }], + ['group key', { groupBy: [F], aggregations: [COUNT] }], + ['aggregation operand', { aggregations: [{ function: 'max', field: F, alias: 'm' }] }], + ["aggregation's own filter", { aggregations: [{ ...COUNT, filter: { [F]: 'x' } }] }], +]; + +describe('callData aggregate asks the data door\'s internal-field gate before the engine aggregates', () => { + for (const [label, body] of POSITIONS) { + for (const [persona, context] of Object.entries({ member: MEMBER, admin: ADMIN, system: SYSTEM })) { + it(`${label} — ${persona}: findData's own refusal, and the engine never aggregates`, async () => { + const h = makeHarness(); + const door = await h.protocol.findData({ object: 'ai_vault', query: { ...body } as any }).then(() => undefined, (e) => e); + expect(envelope(door)).toMatchObject({ code: 'INVALID_FIELD', status: 400, object: 'ai_vault', field: F }); + + const err = await callData(h.deps, REQ, 'aggregate', { object: 'ai_vault', ...body }, undefined, undefined, context) + .then(() => undefined, (e) => e); + expect(err, 'the aggregate arm evaluated a position the door refuses').toBeDefined(); + expect(envelope(err)).toEqual(envelope(door)); + expect(h.aggregate).not.toHaveBeenCalled(); + }); + } + } + + it('CONTROL — ordinary fields in every position reach the engine unchanged', async () => { + const h = makeHarness(); + const body = { + where: { name: 'alpha' }, + groupBy: ['name'], + aggregations: [{ function: 'max', field: 'rank', alias: 'm', filter: { name: 'alpha' } }], + }; + const res = await callData(h.deps, REQ, 'aggregate', { object: 'ai_vault', ...body }, undefined, undefined, MEMBER); + expect(res).toEqual({ object: 'ai_vault', rows: [{ n: 1 }] }); + expect(h.aggregate).toHaveBeenCalledTimes(1); + expect(h.aggregate).toHaveBeenCalledWith('ai_vault', expect.objectContaining(body)); + }); + + it('CONTROL — a host that registers no protocol has no door to agree with, and is served as before', async () => { + const h = makeHarness({ withProtocol: false }); + const res = await callData( + h.deps, REQ, 'aggregate', { object: 'ai_vault', where: { [F]: 'x' }, aggregations: [COUNT] }, undefined, undefined, MEMBER, + ); + expect(res).toEqual({ object: 'ai_vault', rows: [{ n: 1 }] }); + expect(h.aggregate).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index ad1870a54d3..809a09aeee2 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -843,7 +843,7 @@ export async function callData(deps: ActionExecutionDeps, const engine = (await deps.getObjectQL(requestContext, scopeId)) ?? await deps.resolveService(requestContext, 'objectql', scopeId).catch(() => null); if (engine && typeof engine.aggregate === 'function') { - // [#22823] This arm takes no protocol, so it is the engine's door on + // [#22823] This arm runs on no protocol data method, so it is the engine's door on // every assembly: each nested-relation condition's TARGET in `where` // is asked before the engine aggregates, as the query fallback asks // it (the engine lowers relation conditions in `where` only, never @@ -851,6 +851,22 @@ export async function callData(deps: ActionExecutionDeps, if (exposureGated) { await refuseUnservedRelationTargets(deps, requestContext, engine, params.object, params.where, scopeId); } + // The data door's evaluate refusal for a field declared + // `internal: true` — a filter (`where`, an aggregation's own + // `filter`), a group key or an aggregation operand on it. The query + // arm meets it inside `findData`; this arm never reaches `findData`, + // so it asks the protocol's ONE gate itself, before the engine, and + // throws the door's `400 INVALID_FIELD` word for word. ⛔ Never a + // copy of the gate here (route-ownership rule 1). Every caller, as + // at the door; a host with no protocol has no door to agree with + // and is served as before. + if (protocol && typeof protocol.assertNoInternalFieldEvaluated === 'function') { + protocol.assertNoInternalFieldEvaluated(params.object, { + where: params.where, + groupBy: params.groupBy, + aggregations: params.aggregations, + }); + } const rows = await engine.aggregate( params.object, {