Skip to content

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 - #28772

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/harden-mcp-internal-field-query
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/harden-mcp-internal-field-query

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no file under skills/** teaches filtering, sorting, grouping or aggregating on an internal: true field over MCP; content/docs/api/error-catalog.mdx already states that a read naming such a field in an evaluate position is refused INVALID_FIELD, which this diff makes true on the MCP data tools too.

维护者速读

  • 改了什么:MCP 的三个数据入口(stdio 的 query_records / aggregate_records,HTTP /mcp 的 aggregate_records)在交给引擎之前,改为调用 REST 数据入口同一个「internal: true 字段不可求值」闸门(协议服务上的 assertNoInternalFieldEvaluated,改为公开方法,定义仍只有一份);对这类字段做过滤、排序、分组或聚合,统一回 400 INVALID_FIELD,文案与 REST 逐字相同。
  • 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)
  • 风险与回滚:收窄(BREAKING):只影响通过 MCP 对声明了 internal: true 的字段做过滤/排序/分组/聚合的调用;普通字段、行读取(该字段仍不出现在任何行里)、字段权限的 403 均不变;未注册协议服务的宿主照旧。回滚 = revert 本 PR,无数据迁移。
  • 先看:① packages/mcp/src/stdio-data-bridge.ts 的 refuseInternalFieldEvaluation 及其在 query / aggregate 中的两处调用;② packages/runtime/src/action-execution.ts 中 callData 的 aggregate 分支新增段;③ packages/metadata-protocol/src/protocol.ts 中 assertNoInternalFieldEvaluated 去掉 private 及其文档段。
  • 两分钟验证:pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=1 test/mcp-internal-field-evaluate-positions.dogfood.test.ts,预期 32/32 通过(28 个拒绝用例与 REST 的 code/status/message 逐字一致,4 个普通字段对照用例照常返回)。
  • 需要你做的:无,席位落地。

What changes

The data door refuses to EVALUATE a field declared internal: true — a filter, a sort, a group key, an aggregation operand or an aggregation's own filter on it — for every caller, before the engine is asked (assertNoInternalFieldEvaluated, @objectstack/metadata-protocol). Three MCP mouths hand a caller's query to the engine without passing through findData, where the door asks that gate, so they evaluated those positions. Each now asks the door's ONE gate of the protocol service it already resolves, and answers what the door answers, word for word.

  • @objectstack/metadata-protocol — assertNoInternalFieldEvaluated loses private; its docblock names the MCP callers. No other change: one definition, no copy beside it (route-ownership rule 1).
  • @objectstack/mcp, stdio transport — refuseInternalFieldEvaluation asks the protocol's face; query asks it with where and orderBy, aggregate with where, groupBy and aggregations, after the stored-metadata refusals (the door's order) and before the engine. The bridge header's "known divergences" gains this limb. StdioProtocolObjectReads declares the optional face.
  • @objectstack/runtime, HTTP bridge — callData's aggregate arm (the /mcp aggregate_records) asks the same face before engine.aggregate. The query arm already reaches findData, unchanged.
  • Every principal, administrators and system context included, as at the door. The engine's privileged lookups that match on such a field (key, token, share-link verification) call the engine directly and never reach these mouths.
  • Unchanged: ordinary fields; the row strip (the flagged field is still absent from every row); the security middleware's 403 PERMISSION_DENIED for a field the caller may not read; the HTTP bridge's query_records (it already met the gate). A host that registers no protocol service has no data door to agree with and is served as before — see Acceptance notes.

Tests

  • packages/metadata-protocol/src/protocol.data-door-internal-field-positions.test.ts — new block: for each canonical query the MCP mouths can send (filter $eq / $startsWith, sort, aggregate where, group key, operand, aggregation filter, relation condition), the public face throws exactly what findData throws (code, status, object, field, param, message); ordinary fields pass it.
  • packages/runtime/src/action-execution-calldata-aggregate-internal-field.test.ts — callData('aggregate') over the REAL ObjectStackProtocolImplementation as the protocol service: each position × member / admin / system answers findData's own refusal and the engine never aggregates; controls: ordinary fields reach the engine unchanged, and a host with no protocol is served as before.
  • packages/mcp/src/stdio-data-bridge.internal-field-evaluation.test.ts — the bridge asks the face with the call's own arguments before the engine, the refusal reaches the caller unchanged (and renders as the tool's code / status / message envelope), the engine is never asked; no protocol, or one without the face: served as before.
  • packages/qa/dogfood/test/mcp-internal-field-evaluate-positions.dogfood.test.ts — the real stack: showcase with its security and MCPServerPlugin, a runtime object with an internal: true field authored through PUT /meta/object, one stdio MCPServerPlugin per principal started with autoStart and its OS_MCP_STDIO_API_KEY minted through POST /keys. Seven positions × {stdio, HTTP /mcp} × {admin, member}: isError, INVALID_FIELD, 400, and the message equal to what POST /data/:object/query answers for the same body in the same run. Controls per transport and principal: an ordinary field filters (narrowing one row of two), sorts, aggregates and groups; the flagged column is absent from every served row.

Acceptance notes

  • No-protocol hosts. A host that registers no protocol service (registerProtocol: false and no MetadataProtocolPlugin) has no data door, so the gate has no verdict to share there and the MCP tools serve as before; the engine still refuses a group key or an aggregation operand on such a field (rejectCredentialAggregation). Both mouths say so in their comments; this PR does not make that composition refuse. Raised as an open question in the report.
  • Group key / operand. These were already refused on both transports by the engine, in the engine's own words; they now answer the door's sentence first, as REST does. The dogfood pin compares messages, so that change is measured, not assumed.
  • Relation conditions. A nested-relation condition is judged on its target by the same face (the target's own internal fields); the stdio bridge's own relation-target exposure gate still answers first, as before.

Verification

Final head 79470662f — the branch with origin/main (6cbdef1e4) merged in; the merge touches none of this diff's files. Readings are from this worktree (objectstack-ai/objectstack), every exit code captured before any pipe, heavy runs under scripts/pm/os-verify-lock.sh.

Targeted pins — on 7947066, all exit 0
  • @objectstack/metadata-protocol src/protocol.data-door-internal-field-positions.test.ts: Tests 50 passed (50).
  • @objectstack/runtime src/action-execution-calldata-aggregate-internal-field.test.ts: Tests 17 passed (17).
  • @objectstack/mcp src/stdio-data-bridge.internal-field-evaluation.test.ts: Tests 5 passed (5).
  • @objectstack/dogfood test/mcp-internal-field-evaluate-positions.dogfood.test.ts: Tests 32 passed (32).
Touched packages — full suites and typecheck, all exit 0
  • @objectstack/metadata-protocol on 7947066 (the merge brought changes into this package, so it was re-run there): Test Files 292 passed, 4 skipped (296); Tests 21963 passed, 26 skipped (21989); typecheck exit 0.
  • @objectstack/mcp on 8e5daeeca (the merge changed nothing under packages/mcp): Test Files 49 passed (49); Tests 617 passed (617); typecheck (with its test layer) exit 0.
  • @objectstack/runtime on 8e5daeeca (the merge changed nothing under packages/runtime), local project: Test Files 505 passed (505); Tests 8203 passed, 19 skipped (8222); typecheck (with its test layer) exit 0. The repo project is CI's (Test Core).
  • @objectstack/dogfood typecheck exit 0 (8e5daee). ESLint over the six changed TypeScript files on 7947066: exit 0.
Dogfood battery — on 7947066

pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=1, closure built first, under the verify lock: lock VERDICT command-exit 0; Test Files 372 passed, 3 skipped (375); Tests 3026 passed, 28 skipped (3054); no failure.

One-shot ablation — on 7947066, cited here and not kept as a test
  • Mutation, through scripts/ablation-replace.mjs (anchor must hit): the stdio bridge's gate call and callData's gate call each replaced by a console.debug marker — anchor 1 to 0, marker 0 to 1, blob changed, for both files; on-disk counts re-read inside the wrap.
  • @objectstack/mcp and @objectstack/runtime rebuilt; scripts/ablation-dist-preflight.mjs found each marker in the consumed dist/ (2 built files each).
  • Dogfood pin: Tests 22 failed, 10 passed (32). Red: all 14 stdio rows and the 8 HTTP aggregate_records rows — where and the aggregation's own filter were SERVED; group key and operand answered in the engine's words, not the door's. Green: the 6 HTTP query_records rows (they reach findData) and the 4 controls.
  • Restore: each file's blob equals its HEAD blob and git diff HEAD is empty (both checked by the tool and again by the script); both packages rebuilt; --absent preflight: markers absent from every built file, working tree clean.
Gates — derived by dispatch-gates at 7947066
  • node scripts/pm/dispatch-gates.mjs --commands: 66 commands. Run with exit codes recorded; --ran reconciliation: 66 derived, 65 run, 1 NOT-MEASURED, 0 UNRUN.
  • NOT MEASURED: check:dual-build-cjs-loads, reason: exit 3 PREREQUISITE NOT MET — it reads every package's dist/ and this worktree built the dogfood closure only. CI's run is the reading.
  • check:type-check-coverage and check:type-check-debt ran inside the same hold (their closure build nested in it): exit 0, --re-measure 0 raw tsc errors.
  • check:dts-closure first answered exit 1 (@objectstack/metadata-core declarations missing) while that --re-measure was rebuilding the closure in the same hold; re-run on the settled dist/: exit 0, 241 of 241 declared declaration files present.
  • check:slot-lookup was red on the first sweep (an any-typed stub plugin context in the new dogfood file); fixed in 8e5daeeca, green since.
  • Two gate scripts (check-durability-degradation-log-level.mjs, pm/check-half-states.mjs) changed on origin/main after this merge; the local readings are of this tree's copies.
  • CI check-runs: not yet available when this pull was opened.

Generated by Claude Code

…ld evaluation gate before the engine

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
…o the data door's internal-field refusal

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
…member object read on the fixture

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/mcp, @objectstack/metadata-protocol, @objectstack/runtime, touching 6 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/declarative-endpoints.mdx (via callData (symbol, a top-level function))
  • content/docs/concepts/metadata-lifecycle.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/permissions/authorization.mdx (via callData (symbol, a top-level function))
  • content/docs/permissions/system-context.mdx (via callData (symbol, a top-level function))
  • content/docs/protocol/kernel/http-protocol.mdx (via callData (symbol, a top-level function))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 40 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 72533449d3ef11c67594ed05c8a1d4074ef432e0 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 859f8cf0d676571ef50c104a1c6fb8b712778c54 — the merge of head 79470662fc7329acd6106ed74b23ef12a3bf1bd1 into base 72533449d3ef11c67594ed05c8a1d4074ef432e0, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 859f8cf0d676571ef50c104a1c6fb8b712778c54 && git checkout 859f8cf0d676571ef50c104a1c6fb8b712778c54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 72533449d3ef11c67594ed05c8a1d4074ef432e0 79470662fc7329acd6106ed74b23ef12a3bf1bd1 && git checkout -B drift-repro 72533449d3ef11c67594ed05c8a1d4074ef432e0 && git merge --no-ff 79470662fc7329acd6106ed74b23ef12a3bf1bd1

node scripts/docs-audit/affected-docs.mjs --json 72533449d3ef11c67594ed05c8a1d4074ef432e0

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 72533449d3ef11c67594ed05c8a1d4074ef432e0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 79470662fc7329acd6106ed74b23ef12a3bf1bd1
Local-runs: none
Scope: packages/mcp/src/stdio-data-bridge.ts, packages/mcp/src/stdio-data-bridge.internal-field-evaluation.test.ts, packages/metadata-protocol/src/protocol.ts, packages/metadata-protocol/src/protocol.data-door-internal-field-positions.test.ts, packages/runtime/src/action-execution.ts, packages/runtime/src/action-execution-calldata-aggregate-internal-field.test.ts, packages/qa/dogfood/test/mcp-internal-field-evaluate-positions.dogfood.test.ts + packages/metadata-protocol/src/protocol-query-validation.ts (the gate's body: assertNoInternalFieldEvaluated, refuseInternalInFilter, resolveQueryFields), packages/metadata-protocol/src/index.ts, packages/metadata-protocol/package.json (exports map), packages/mcp/src/mcp-http-tools.ts (tool roster and McpDataBridge option shapes), packages/mcp/src/plugin.ts (protocolReads, bridge composition), packages/runtime/src/domains/mcp.ts (buildMcpBridge), packages/objectql/src/credential-evaluation-guard.ts and packages/objectql/src/engine-count-aggregate.ts (the engine's own guards), scripts/check-adr-0087-registration.mjs (the gate the changeset answers), content/docs/api/error-catalog.mdx (the Skills-docs: citation), .changeset/mcp-internal-field-evaluate-positions.md (declaration lines, read as the prose face)

Narrowing face (Clause-②: no (narrowing) on the PR body and the changeset); no published schema and no governed rule text in the tree. Read-only: the diff against origin/main, the cited code at the head, the head's check runs through the API, and the governing text (#22646, #22702, #8497, AGENTS.md route and surface ownership rule 1). Positions and contracts are named; no request shape beyond what the PR's own tests already print.

① Derived judgments

  1. Positions refused, per transport — correct and closed. The one gate's body (protocol-query-validation.ts) judges where (every operator, nested $or, a cross-field comparand, and a nested-relation condition on its target's own flag), filter, orderBy, groupBy, aggregations[i].field, aggregations[i].filter and expand. Read against what each mouth hands it:
    • stdio query_records: { where, orderBy } → filter (any operator) and sort refused; fields is not an evaluate position (the engine's row strip owns it, unchanged); the stdio tool carries no filter alias, expand or search, so nothing it accepts escapes the gate.
    • stdio aggregate_records: { where, groupBy, aggregations } → aggregate filter, group key, operand and an aggregation's own filter refused.
    • HTTP /mcp aggregate_records (callData aggregate arm): the same three keys → the same four positions; timezone is not evaluated.
    • HTTP /mcp query_records: unchanged, already through findData; the dogfood drives it as the parity row.
      The thrown object is the protocol's own (code INVALID_FIELD, status 400, object, field, param, message); the tool layer renders code/status/message, and the dogfood compares the message with what the REST data door answers in the same run, so "same sentence" is measured rather than typed. Judged correct.
  2. Order at the door matches. stdio: exposure → relation-target exposure → stored-body refusal → stored-hash refusal → internal gate → engine. findData: existence gates → body grouping → body predicate → hash → internal gate → engine. Relative order of the stored-metadata family before the internal gate is the door's. The gate's own refuseUnservedRelationTarget re-judges a relation site the stdio exposure gate already answered first; same verdict, no new answer (the PR says so; read as true).
  3. Every other MCP tool read, none evaluates such a field. Roster on both transports (registerObjectTools, mcp-http-tools.ts): list_objects, describe_object, validate_expression, query_records, aggregate_records, get_record, create_record, update_record, delete_record, list_actions, run_action, resume_run, list_remote_tables, import_object, plus the objectstack://objects record resource. get_record, the record resource, update_record, delete_record read or write BY ID (findById / getData / updateData / deleteData); run_action takes a recordId and loads its subject by id; resume_run by run id; validate_expression judges a CEL expression against the schema and reads no row; list_remote_tables / import_object are federation DDL; list_objects / describe_object are metadata. There is no export or search tool on MCP. So the three mouths this PR gates, with the already-gated HTTP query_records, are every MCP position that evaluates a caller's predicate, order, group key or operand. Closed.
  4. Ordinary fields, FLS 403, row strip, system context. Ordinary fields: pinned on both transports and both personas, with a filter that narrows one row of two and a group of two keys. Row strip: the dogfood asserts the flagged column absent from every served row of the controls. FLS: the engine's middleware still runs after the gate, so a field the caller may not read keeps the security middleware's 403 on an ordinary field — unchanged, and the internal-field case answers INVALID_FIELD before FLS exactly as REST does (the door binds every caller, fix(objectql,metadata-protocol): the data door honours a field's internal flag in its filter, sort, group-by, aggregate, $search and $expand positions (#22646) #22702). System context: neither refuseInternalFieldEvaluation nor the callData arm reads the context, matching findData (no carve-out); the runtime pin drives isSystem: true through every position. Note the engine's own rejectCredentialEvaluation does carry a system carve-out, but it keys on secret/password columns (a different flag), not internal, so it neither covers nor contradicts this gate; the engine's rejectCredentialAggregation reads collectInternalReadFields and still refuses a group key or operand on a no-protocol host, in its own words — as the PR states.
  5. assertNoInternalFieldEvaluated made public — one definition kept, additive type-surface exposure, no baseline to regenerate. The body lives once in protocol-query-validation.ts and the class method delegates with .call(host); neither MCP caller copies anything (route and surface ownership rule 1 held). ObjectStackProtocolImplementation is exported from src/index.ts and the package's exports["."] map points at dist/index.d.ts, so the formerly private method is now reachable from the entry type graph: a published, additive widening of @objectstack/metadata-protocol's TypeScript surface. @objectstack/metadata-protocol has no api-surface/ baseline (check:api-surface is packages/spec-scoped), so nothing regenerates, and the spec's MetadataProtocol interface is not widened (not in the diff): both callers duck-type the face. This is a server-side face on an implementation class, not a wire or authoring contract, so it does not move the Clause-② arm; recorded here so the widening is in writing. Judged acceptable at patch.
  6. The changeset's Who sees it paragraph is correct: the built-in objects that declare the flag are sys_*, refused by allowSystemObjects default-off; the dogfood authors a runtime object to reach the position.

② Semver level

Clause-②: no (narrowing) and Metadata-protocol: none, line-initial on the PR body and the changeset; title carries !; patch on @objectstack/mcp, @objectstack/runtime, @objectstack/metadata-protocol. Consistent with AGENTS.md Post-Task Checklist step 3: nothing a metadata author writes changes kind or meaning (none → patch), and a non-protocol breaking change ships patch with its BREAKING banner and ADR-0087 disposition. The precedents cited (#28596, #28688, #28496) declared Metadata-protocol: narrowed because each tightened what an author can WRITE (an inline secret, an undeclared key, a landing item); this PR tightens what an MCP client can QUERY, so none is the right kind and the difference from those precedents is in kind, not an omission. Skills-docs: none — REASON present as the (narrowing) arm owes; its citation verified: content/docs/api/error-catalog.mdx lines 155–161 already state the refusal for every evaluate position and say "query by another field".

ADR-0087: exactly one marker, not-required (no-migration-prescription), in the changeset. Complete without a registry entry: no authorable key, export or config field is removed or renamed, so AGENTS.md's "breaking changesets carry their migration (FROM → TO)" clause does not bind, and the remedy ("filter, sort, group and aggregate on another field") is not a rewrite of anything an author wrote. On the dev's reading of the gate: a bare FROM/TO label is a MENTION since #6967 and refuses nothing by itself; what refuses the exemption is a framed concrete rewrite (an X → Y with code-ish operands under migration framing, a Was/Now table, or an X → delete it). The Before/Now bullets carry no arrow rewrite and no table, so they are not a prescription in either spelling and the exemption stands on its merits, not on wording. The gate's CI job (Check Changeset) is green on this head.

③ Boundary flags

  • Open question (dev): a host with no protocol service — keep (A), fail closed (B) or move the gate below the protocol (C)? Answer: A is acceptable for this PR; not a must-fix; a card is owed. Reasons: (i) the ruling of 2026-11-01 is option a — MCP applies the SAME gate as the REST data door — and on that composition there is no data door: REST's own /data query falls back to callData's ql.find with the caller's where/orderBy and asks no internal gate either (read at the query fallback in action-execution.ts), so MCP is not behind REST there; (ii) B as "refuse the tools without a protocol" is wider than the defect, and B as "judge the schema locally" is a second definition of the gate, which rule 1 forbids; (iii) C — the gate in the engine, keyed on the one flag reader collectInternalReadFields that already lives in @objectstack/objectql and already drives rejectCredentialAggregation — is the structural answer The write-response internal-field tripwire walks the protocol class only — a direct engine write mouth outside it (rest-server's batch ql.update) is covered by the fix but not by the guard #8497 argued for on the write strip ("the boundary, not the class"), and it would also cover a host that supplies a protocol other than ObjectStackProtocolImplementation (both callers duck-type, so such a host is silently ungated too), but it is an engine-wide change outside this ruling. Escalation: the dispatch seat files a card naming the no-protocol / foreign-protocol compositions and the REST fallback beside the MCP mouths, with C as the proposed route, for triage to grade (and to rule whether registerProtocol: false without MetadataProtocolPlugin is a supported production composition at all — ObjectQLPlugin logs that the assembly is "delegated to MetadataProtocolPlugin").
  • Pins non-vacuous: the one-shot ablation in the PR body replaces both gate calls with markers; dogfood 22 red / 10 green, and the partition is arithmetically the matrix (7 positions × 2 personas × 2 transports = 28 refusal rows: 14 stdio + 8 HTTP aggregate_records red, 6 HTTP query_records green because they reach findData; 4 controls green), restore proven by blob equality and an absent-marker preflight. The runtime pin asserts envelope equality against the REAL protocol's findData and that the engine never aggregates; the protocol pin asserts the public face throws what findData throws; the mcp pin asserts the gate is asked with the call's own keys, before the engine, and the envelope renders. Read as sufficient; not re-run here.
  • CI at review time: Check Changeset, Governed Surface Queue Guard, PR-body guards, Lint · ESLint, Dogfood Regression Gate green; Test Core 1–8, Build Core, Type Check · workspace/consumer gates, Lint · repo & tooling gates, Live E2E UI still in progress. All-green is the landing precondition, not this record's; the seat reads them at landing.
  • Author objectstack-fleet[bot]; no relay login to name.

④ Design judgment

On the narrowing itself against the published spec on main: it takes no declared capability away at the consumer — the error catalog already promises INVALID_FIELD for every evaluate position on a field declared internal: true and says such a field "cannot be queried by either" caller, and #22702 (fixing #22646) made that the door's contract, binding every caller. This diff makes the spec's own refusal real on the two MCP transports, by asking the door's one gate rather than copying it. Each Scope: file holds against the code it cites: the stdio bridge's header lists the limb and names the gate; StdioProtocolObjectReads declares the optional face in the same slot the other protocol reads ride; the callData arm asks before engine.aggregate and after the relation-target exposure gate, mirroring the query fallback's order; the protocol's docblock names the MCP callers. No must-fix.

Should-fix (non-blocking):

  1. File the follow-up card from ③ (no-protocol and foreign-protocol compositions; propose the engine-level gate on the one flag reader).
  2. Consider declaring the face on the spec's MetadataProtocol type or a shared QueryEvaluationGate type so callData's protocol.assertNoInternalFieldEvaluated is typed rather than duck-typed on an untyped service — part of the same card, not this PR.

Implemented-by: claude/harden-mcp-internal-field-query
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

ACCEPT → head 79470662fc7329acd6106ed74b23ef12a3bf1bd1 · 2026-11-01T19:01Z

维护者速读

  • 改了什么:REST 数据入口对 internal: true 字段的取值检查(assertNoInternalFieldEvaluated)现在由 MCP 共用。stdio 的 query_records 拦过滤和排序,stdio 与 HTTP /mcp 的 aggregate_records 拦过滤、分组键、聚合字段和聚合自带的过滤。这些位置一旦用到 internal: true 字段,都会被拒,回答与 REST 相同的 400 INVALID_FIELD。检查只有一份定义。普通字段、行字段剥离和 FLS 都不变。
  • 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)。
  • 风险与回滚:只收窄 MCP 的查询接受集:按 internal 字段过滤、排序或聚合的 MCP 调用会被拒,这正是错误目录早已承诺的行为。回滚即 revert 本 PR。
  • 先看:packages/mcp/src/stdio-data-bridge.ts(refuseInternalFieldEvaluation)、packages/runtime/src/action-execution.ts 的 callData aggregate 分支。
  • 两分钟验证:pnpm --filter @objectstack/dogfood exec vitest run test/mcp-internal-field-evaluate-positions.dogfood.test.ts,预期 32/32 通过。
  • 需要你做的:无,席位落地。

席位意见:收。本 PR 落实维护者 2026-11-01 的裁定(a,原话「同意」);达档契约复核 6562332618 PASS,没有必须修改的项。

席位核对

  1. 形态:draft,目标 main;按披露裁决不建卡,正文没有关闭关键词。正文由 dev 在 pr_create 时写入,首行是签名,Clause-②: no (narrowing) 和 Metadata-protocol: none 都在。
  2. 前提实测:ablation 撤掉两处调用后,dogfood 有 22 个用例变红(stdio 14 个、HTTP 聚合 8 个);已经走 findData 的 6 行和 4 个对照组保持绿色。恢复后 blob 与 HEAD 一致,dist 中的标记也已清除。
  3. 门禁:本地派生 66 项,跑了 65 项、exit 0,另 1 项未测(dual-build,前置条件不满足)。完整 dogfood 全套 372 个文件、0 失败。本 head 的 CI 共 49 项:36 success、13 skipped、0 失败。
  4. 契约面:声明为收窄 → 按 CONTRACT_REVIEW_TIER 做完整复核,PASS。复核结论:
    • MCP 工具全部列举过,只有这几处会对字段取值,全部已拦住;
    • 顺序与 findData 一致;
    • 新公开的方法是对已发布类型面的增量扩展,按 patch 发布可以接受。
  5. 范围外去向:没有 protocol 服务、或使用外来 protocol 的组合,以及给检查接口补类型,都是复核的非阻塞建议。属于安全相关,由 PM 私下排队,一次处理一件。
  6. 落地:不属于受管面,801 行,低于 3000 行上限 → pr_ready + automerge_enable。

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 1, 2026 19:02
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 1, 2026 19:02
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit b48eeb1 Nov 1, 2026
50 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/harden-mcp-internal-field-query branch November 1, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant