Repository navigation
Commit b48eeb1
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)
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`.
<details><summary>Targeted pins — on 7947066, all exit 0</summary>
- `@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).
</details>
<details><summary>Touched packages — full suites and typecheck, all exit
0</summary>
- `@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.
</details>
<details><summary>Dogfood battery — on 7947066</summary>
`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.
</details>
<details><summary>One-shot ablation — on 7947066, cited here and not
kept as a test</summary>
- 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.
</details>
<details><summary>Gates — derived by dispatch-gates at
7947066</summary>
- `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.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent fc28644 commit b48eeb1
8 files changed
Lines changed: 795 additions & 6 deletions
File tree
- .changeset
- packages
- mcp/src
- metadata-protocol/src
- qa/dogfood/test
- runtime/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
92 | | - | |
93 | | - | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
94 | 106 | | |
95 | 107 | | |
96 | 108 | | |
| |||
136 | 148 | | |
137 | 149 | | |
138 | 150 | | |
139 | | - | |
140 | | - | |
| 151 | + | |
| 152 | + | |
141 | 153 | | |
142 | 154 | | |
143 | 155 | | |
| |||
151 | 163 | | |
152 | 164 | | |
153 | 165 | | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
154 | 174 | | |
155 | 175 | | |
156 | 176 | | |
| |||
551 | 571 | | |
552 | 572 | | |
553 | 573 | | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
554 | 606 | | |
555 | 607 | | |
556 | 608 | | |
| |||
968 | 1020 | | |
969 | 1021 | | |
970 | 1022 | | |
| 1023 | + | |
| 1024 | + | |
| 1025 | + | |
971 | 1026 | | |
972 | 1027 | | |
973 | 1028 | | |
| |||
1091 | 1146 | | |
1092 | 1147 | | |
1093 | 1148 | | |
| 1149 | + | |
| 1150 | + | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
| 1155 | + | |
| 1156 | + | |
1094 | 1157 | | |
1095 | 1158 | | |
1096 | 1159 | | |
| |||
0 commit comments