fix(mcp,runtime)!: the MCP data tools refuse to filter, sort, group or aggregate on an internal field, with the data door's own 400 INVALID_FIELD

.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
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no metadata an author writes changes meaning: a query position the REST data door already refuses is now refused by the MCP data tools too, with the door's own refusal. The fix for a caller is to query another field, which the refusal says; there is nothing for a migration to rewrite and no tombstone. -->
+
+**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`.

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<string, unknown>) => {
+    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<unknown>) => {
+  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();
+    }
+  });
+});

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<unknown>;
   /** The by-name read without the serving decorations. */
   getMetaItemForExecution?(request: { type: string; name: string }): Promise<unknown>;
+  /**
+   * 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<string, unknown>): 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<string, unknown>,
+): 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<string, unknown> = {};
       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