The two MCP transports run their data verbs through different seams — stdio skips the ADR-0049 apiEnabled / apiMethods exposure gate that HTTP applies

Found while implementing #8034 (registering the object tools on the stdio MCP transport). Filed rather than fixed there: #8034's scope is "the tools are not registered at all", and closing this properly is a design change in a second package.

What differs

Both MCP transports now register the SAME tools from the same McpDataBridge interface (#8034 makes that structural). What each tool then executes still differs, because the two hosts implement the bridge over different seams:

	HTTP (`/api/v1/mcp`)	stdio (`OS_MCP_STDIO_ENABLED`)
bridge builder	`buildMcpBridge` — `packages/runtime/src/domains/mcp.ts`	`createStdioDataBridge` — `packages/mcp/src/stdio-data-bridge.ts`
data path	`callData` → `protocol` service, falling back to the ObjectQL engine	the ObjectQL engine only
ADR-0049 object exposure gate	applied (`checkApiExposure` at the top of `callData`)	not applied
protocol-layer ingress `readonly` strip, existence probes, spec-shaped receipts, `expand` / `select`	applied when the `protocol` service is registered	not applied

callData cannot simply be reused by the stdio host: its signature is bound to HttpProtocolContext (the request, its resolved kernel, its per-environment data driver), and a long-lived stdio session has no request — it has one identity resolved from OS_MCP_STDIO_API_KEY.

Why it matters

An author who declares enable.apiEnabled: false (or narrows apiMethods) on an object is telling the platform not to expose that object's data operations over the API. That declaration is honoured on the MCP HTTP surface and ignored on the MCP stdio surface — same product, same tool names, same key, different answer.

This is not an authorization bypass, and should not be triaged as one. packages/runtime/src/api-exposure.ts documents the gate as a SURFACE-AREA control rather than the authorization boundary, and its own fail-open decision rests on that: every call still passes the ObjectQL security middleware (CRUD / FLS / RLS) regardless of the outcome. On stdio those middlewares run too — the bridge calls the engine with the key's ExecutionContext on every verb. What leaks is the author's exposure declaration, not the data guard.

Scope note: the gap predates #8034 in a narrower form. The ADR-0101 record resource (objectstack://objects/{objectName}/records/{recordId}) has read rows over ql.find without the exposure gate since #7645. #8034 widens the same seam from one read path to the object-CRUD tool set.

Suggested shape (not prescriptive)

One transport-neutral data seam both MCP hosts call, so there is no second implementation to drift:

A — lift callData off HttpProtocolContext onto a narrow "resolve a service by name, in this scope" interface, and let the stdio host construct that. Largest change; removes the fork outright.
B — have the runtime supply the bridge to the long-lived server (the MCP plugin already triggers mcp:ready), with the stdio identity passed in as a per-call principal provider so ADR-0101 D1 revocation still takes effect on the next call. Needs the tool wiring to move ahead of start(), since registering a tool is what declares the tools capability and the SDK refuses capability registration after a transport attaches.
C — apply just the exposure gate on the stdio bridge by calling the spec's own resolveEffectiveApiMethods / isApiOperationAllowed (the single source of truth checkApiExposure already delegates to). Smallest; closes the declaration leak but leaves the rest of the table divergent.

A parity test in the shape of #8034's transport parity: one bridge, one tool surface — one declaration, both transports, same verdict — is what would keep whichever option lands from regressing.

Source

Extracted from the #8034 implementation. The divergence is recorded in the module docblock of packages/mcp/src/stdio-data-bridge.ts so the next reader of that file is pointed here rather than re-deriving it.

Generated by Claude Code

Triage: domain:cli (both candidate surfaces — packages/mcp/src/stdio-data-bridge.ts and packages/runtime/src/domains/mcp.ts — live in this lane), pm:queue. Type: Bug — an author's enable.apiEnabled / apiMethods declaration is honoured on one MCP transport and ignored on the other; restoring declared = enforced needs no maintainer ruling. The card's own framing is adopted as the triage frame: this is a surface-area declaration leak, not an authorization bypass (CRUD/FLS/RLS run on both transports) — graded accordingly, no security label.

PM-suggested route (optional, measurement wins): option C — apply the exposure gate on the stdio bridge via the spec's own resolveEffectiveApiMethods / isApiOperationAllowed, the single source of truth checkApiExposure already delegates to. Smallest diff, closes the declaration leak, and does not preempt the larger seam-unification question (options A/B), which can become its own card if C's divergence table keeps biting. The parity test shape the card names (one declaration, both transports, same verdict) should be part of acceptance whatever route lands. Note for the claiming seat: check in-flight overlap with the #8034 family before dispatch — same bridge files.

Size/model suggestion: M / opus, mode:subagent.

本评论来自分诊座位 Routine。

Generated by Claude Code

Claim: PM loop round 1
Session: session_016pY4Xb2iDecfDtT3CWoiTW
Branch: claude/issue-8083-mcp-stdio-exposure-gate
Worktree: objectstack-issue-8083
Domain: domain:cli
File surface: packages/mcp/src/stdio-data-bridge.ts + a new parity test in packages/mcp/src/ (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: opus
Serial constraints cleared: packages/mcp — #7279 (a third hand-written ExecutionContext assembly in the stdio MCP plugin) is queued in this lane and touches the same bridge family; deliberately NOT dispatched this round, and it is the next-round candidate that must not overlap. No open PR touches packages/mcp/src/stdio-data-bridge.ts (checked against the open-PR list at claim time); the #8034 family is merged. Batch is package-disjoint from #8132 (packages/types) and #7870 (packages/client).

Premise re-verified on origin/main @ 43f37e1 before dispatch:

checkApiExposure is imported in exactly one production file — packages/runtime/src/action-execution.ts:21, applied at :133 inside callData. Zero hits anywhere under packages/mcp/src, so the stdio bridge does not apply the gate. The card's pointer to domains/mcp.ts names the bridge builder; the gate itself is in callData, which is where the divergence originates.
The divergence is still recorded in the stdio-data-bridge.ts module docblock ("Known divergences from the HTTP bridge"), naming the ADR-0049 apiEnabled / apiMethods gate among them — that docblock is the thing this card retires.

Generated by Claude Code

{
  "issue": 8083,
  "status": "done",
  "branch": "claude/issue-8083-mcp-stdio-exposure-gate",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/8265",
  "premise_still_valid": true,
  "summary": "Premise re-derived on current main (5d7eabc, not the 43f37e1 at dispatch): checkApiExposure still has exactly one production caller (packages/runtime/src/action-execution.ts:21, applied at :133 in callData) and zero hits under packages/mcp/src, so the stdio bridge really did skip the gate. Took the PM's option C: the six data verbs in createStdioDataBridge now call a local enforceApiExposure that takes its decision from the spec's own resolveEffectiveApiMethods / isApiOperationAllowed. Both PM mechanism assumptions held — packages/mcp already depends on @objectstack/spec and the file already imported @objectstack/spec/data, so no new dependency; the helpers are exported from packages/spec/src/data/api-derivation.ts, so nothing in packages/spec changed. Measurement refined the shape in one place worth naming: the repo already has TWO independent wrappers over these same helpers (checkApiExposure in runtime, apiAccessDenialFromEnable in rest), so a third thin per-surface wrapper is the established convention rather than the drift risk route-ownership rule 1 warns about — and REST supplied the canonical machine codes (OBJECT_API_DISABLED 404 / OBJECT_API_METHOD_NOT_ALLOWED 405) which the stdio refusals now reuse. Gated set is exactly the six methods buildMcpBridge routes through callData; listObjects/describeObject stay ungated because HTTP answers those off the metadata service, and gating them would have been a fresh divergence pointing the other way. isSystem bypass, fail-open on unresolvable metadata and the flat-shape fallback were each verified against the HTTP path and matched rather than assumed. The ADR-0049 line is removed from the module docblock's divergence table; the other four divergences are left intact and still point at the follow-up work.",
  "tests": "pnpm --filter '@objectstack/mcp^...' build (dependency closure first, in a fresh worktree) then pnpm --filter @objectstack/mcp test -- --maxWorkers=2 => 'Test Files 14 passed (14) / Tests 162 passed (162)'; pnpm --filter @objectstack/mcp typecheck => clean, no output. New file packages/mcp/src/stdio-data-bridge.exposure.test.ts contributes 24 cases. REVERSE VERIFICATION (fix committed first at ac5a818, direction predicted before running: refusal cases red, structural/fail-open/bypass/allow cases green): with only the six enforceApiExposure call sites removed and the exports kept, that file went 'Tests 14 failed | 10 passed (24)' — exactly the predicted split. One case is worth the PM's attention: the existence-probe test failed with 'expected undefined to be OBJECT_API_DISABLED' because the ungated update/remove path throws a bare recordNotFound Error, so a plain .toThrow() assertion would have stayed GREEN there — which is why every refusal asserts the code AND status envelope per AGENTS.md. Restored with git checkout <branch> -- <path>; git diff empty, i.e. byte-identical to the commit. Gate families run locally, all exit 0: check:cross-package-test-inputs (the one the prompt named; 'OK: 8 package(s) read outside themselves, all declared'), plus five I derived or judged — check:nul-bytes, check:engine-double-contract ('OK — 175 pinned, 133 in the DEBT ledger, 2 exempt'; my engine double uses bare vi.fn() for insert/update/delete, the idiom __tests__/plugin.test.ts already uses, because every write assertion is that they were never reached), check:error-code-casing, check:query-options-erasure, check:type-check-coverage. The last two came from re-deriving against my actual changed paths with node scripts/pm/dispatch-gates.mjs, which surfaced them as convention-triggered by adding a test file — the prompt could not have named them. CI not waited on, per the report-at-draft-PR-time rule.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #8266: the stdio MCP record resource (objectstack://objects/{objectName}/records/{recordId}) reads rows through its own getRecord closure in plugin.ts, not through the bridge, so it still skips the exposure gate after this PR — same declaration leak, different seam, and the one the card's own scope note flags as predating #8034 (ungated since #7645). Filed unassigned and unlabeled for triage, graded explicitly as a declaration leak and not a security issue. NOTE FOR DISPATCH: it collides with #7279's area (getRecord's resolvePrincipal is in that card's blast radius), so per the prompt I did not absorb it, and #8266 says the two should not be in flight simultaneously."
  ]
}

Generated by Claude Code

ACCEPT — PR #8265

Reviewed against the diff, not the report.

Scope: 3 files — packages/mcp/src/stdio-data-bridge.ts, a new stdio-data-bridge.exposure.test.ts, and the changeset. packages/spec untouched (the hard stop held), packages/runtime untouched, and nothing near #7279's area. No breach.

The ruling was respected on the point that mattered: options A/B (unifying the seam) are not attempted, and the framing held all the way into the changeset and test docblock — "surface-area declaration leak, not an authorization bypass", with the tests explicitly graded as asserting an exposure verdict and never a data-authorization one. That framing is the thing that would have been easiest to quietly upgrade into a security fix, and it wasn't.

The decision is delegated, not re-implemented. enforceApiExposure reads resolveEffectiveApiMethods / isApiOperationAllowed from the spec — the same helpers checkApiExposure (runtime) and apiAccessDenialFromEnable (rest) already call. The dev's measurement improved on my dispatch here: I framed a third wrapper as a possible drift risk, and the finding that two such wrappers already exist makes a third the established convention rather than a fork. Reusing REST's OBJECT_API_DISABLED / OBJECT_API_METHOD_NOT_ALLOWED codes follows from that and is right.

Two judgments beyond what I asked for, both correct:

GATED_ACTIONS is data, asserted structurally against the spec's own action map — not six literals sprinkled through the verbs. The failure it catches is the one that matters: DATA_ACTION_TO_API_OPERATION[action] ?? action passes an unrecognised word through as an ungated custom action, so a typo would not throw, it would silently stop gating a verb. That is the same class of defect as the card itself, one layer down, and it is now impossible to reintroduce quietly.
The gate runs before the existence probe in update / remove. Gating after it would answer "that id names no row" for one id and succeed for another — an existence oracle on an object the author declared unexposed. The card did not ask for this and I did not specify it.

Parity is pinned honestly given the package boundary. packages/mcp may not read @objectstack/runtime sources — that is exactly what check:cross-package-test-inputs exists to catch — so the declaration→verdict table mirrors runtime/src/api-exposure.test.ts case for case, and the docblock says plainly that this is a mirrored table. The non-mirrored half is the structural GATED_ACTIONS assertion. listObjects / describeObject stay ungated with the reason recorded (HTTP answers both off the metadata service; gating them would be a fresh divergence pointing the other way), and there is a positive test for that direction too.

Reverse verification is the real thing: with only the six call sites removed, 14 failed / 10 passed — the split predicted before running. The recorded detail is the valuable part: the existence-probe case failed with code: undefined because the ungated path throws a bare recordNotFound, so a plain .toThrow() would have stayed green there. That is precisely why refusals assert code and status, and it is a good argument in a form that survives being read later.

Out-of-scope finding verified filed, not merely described: #8266 exists — the ADR-0101 record resource still reads through its own getRecord closure in plugin.ts and skips the gate, so a hidden object is refused by the get_record tool and still readable through the record resource on the same transport. It carries the same not-a-security-issue grading and flags its own collision with #7279. It landed unlabeled, which is fine: naked cards are exactly what the triage sweep picks up, and routing is triage's to produce, not mine.

Landing: path face clean (no docs/adr/**, no skills/**) → normal route. Ready-flip held until the gate jobs converge.

Generated by Claude Code

Landing update — PR #8265 was removed from the merge queue: MERGE_CONFLICT. Recording it here so the state is on GitHub rather than only in a PM session. The ACCEPT above stands; nothing was found wrong with the work.

Measured cause (origin/main now aca68eb58):

Exactly one file conflicts — packages/mcp/src/stdio-data-bridge.ts, "changed in both" against the merge base. The changeset and the new test file do not conflict.
The commit that moved it is 3f7b4ff4f — "fix(spec): EngineAggregateOptions.groupBy declares the GroupByNodeSchema union the engine already reads ([finding] EngineAggregateOptions.groupBy is declared string[] but the engine also reads { field, dateGranularity } buckets — every correct caller must lie to the type #8032) (fix(spec): EngineAggregateOptions.groupBy declares the GroupByNodeSchema union the engine already reads (#8032) #8236)".

Why this is not a mechanical resolution. #8236 changed the declared types on the aggregate path, and the aggregate arm is exactly where this PR adds its enforceApiExposure call — and where the PR's own comment records "two casts, one cause: McpDataBridge.aggregate declares a WIDER input than EngineAggregateOptions accepts". If #8236 narrowed that, the correct merge is neither side's text but the composition of both intents. Taking one side mechanically is how you get two changes that are each green and wrong together, with neither side's tests able to see it.

Handed back to the implementing dev for a patch round on the same branch (context preserved, same claim, same PR — no new PR, no assignee change). Instructions: merge origin/main, compose both intents rather than picking a side, re-run the full packages/mcp suite and typecheck, and re-derive the gate families against the merged diff, since pre-merge readings do not survive a merge.

The two sibling PRs from this batch merged cleanly (#8263 → #8132, #8267 → #7870), so this is an isolated collision, not a batch problem. I re-queue #8265 once I have read the gate job conclusions on the pushed result — ⛔ not on the dev's local green.

Generated by Claude Code

Patch round: MERGE_CONFLICT resolved by composing both intents. Branch pushed, PR left ready (not re-flipped), assignee and claim comment untouched.

{
  "issue": 8083,
  "status": "done",
  "branch": "claude/issue-8083-mcp-stdio-exposure-gate",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/8265",
  "premise_still_valid": true,
  "summary": "Conflict resolved by merging origin/main (aca68eb58) into the branch — merge, not rebase, so the reviewed commit history is intact; head is now 29802dd0e and PR mergeable_state moved dirty -> blocked. Your read was right that this needed semantic composition, not a side pick: #8236 deleted the exact comment block and the two `as unknown as` casts my aggregate gate call sits on top of, and also deleted the `EngineAggregateOptions` type import plus the `EngineAggregations` alias. Read #8236's diff before resolving rather than inferring from the markers. Only the comment block actually conflicted — git had already auto-merged the call body to #8236's cast-free form and correctly composed the import block (my value import of the spec gate helpers survived, #8236's two deletions applied). Resolution keeps my `enforceApiExposure` call plus its list-class-read comment, then takes #8236's 'No casts' comment and cast-free body verbatim. #8236 made the casts unnecessary, so they are gone — I did not reintroduce them. All six gate call sites intact, gate still before the existence probe on update/remove. Effective diff surface is byte-for-byte the same shape as pre-merge: 3 files, 555 insertions / 5 deletions. ANSWER TO YOUR Q4 — no observable behaviour change on the aggregate arm beyond the gate, and I measured it rather than reasoning from #8236's 'No engine/runtime behavior change' claim: a scratch probe drove the merged bridge with a structured groupBy [{field:'created',dateGranularity:'month',alias:'m'}] and asserted what the engine actually received — groupBy, aggregations and timezone all toEqual the inputs, context present. It passed, then I deleted it (tree confirmed clean, not committed). That is expected, since a TypeScript cast is erased at runtime: #8236's change is type-level, mine is behaviour-level, and they are orthogonal.",
  "tests": "All re-run on the MERGED state, not carried over. pnpm install, then pnpm --filter '@objectstack/mcp^...' build (closure first), then: pnpm --filter @objectstack/mcp typecheck => exit 0, no output — this is the load-bearing one, because #8236 narrowed McpDataBridge.aggregate onto the engine's own slices and my test file passes an aggregations literal [{function:'count',field:'id',alias:'n'}] through it; it still typechecks honestly against the narrowed declaration. pnpm --filter @objectstack/mcp test -- --maxWorkers=2 => 'Test Files 14 passed (14) / Tests 162 passed (162)'. Gates re-derived against the MERGED diff with node scripts/pm/dispatch-gates.mjs, which surfaced five changeset-scoped families my first round never ran — not because of the merge, but because I derived before writing the changeset and omitted its path, so the '.changeset' trigger never fired. My error, worth recording. Full run, all exit 0: check:changeset-gate-self-tests, check:cross-package-test-inputs, check:objectui-changeset, check:changeset-no-major ('Diffing HEAD from aca68eb58 (merge base with origin/main). No major bump'), check:nul-bytes, check:engine-double-contract ('OK — 177 pinned, 133 DEBT, 2 exempt'), check:error-code-casing, check:query-options-erasure ('baseline key set verified against aca68eb: no files added'), check:type-check-coverage. ONE RED, and it is not mine: check:objectui-pin-fresh exits 1 with 'objectui pin is stale — refresh .objectui-sha before releasing (#3340)'. Proven pre-existing rather than assumed — I ran the same script in a detached worktree on clean origin/main and it failed identically (exit 1, same message), and my diff does not touch .objectui-sha (it is bumped by periodic 'chore: bump objectui to <sha>' commits, last a0151e98e). It is triggered on my PR only because touching .changeset pulls it in. Probe worktree removed.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #8266 (previous round, unchanged): the stdio MCP record resource (objectstack://objects/{objectName}/records/{recordId}) reads rows through its own getRecord closure in plugin.ts, not through the bridge, so it still skips the exposure gate. Collides with #7279's area; not absorbed."
  ]
}

Generated by Claude Code

fix(mcp): apply the ADR-0049 exposure gate on the stdio MCP bridge (o…

fix(mcp): the stdio record resource honours the ADR-0049 apiEnabled /…

Navigation Menu

Repository navigation

The two MCP transports run their data verbs through different seams — stdio skips the ADR-0049 `apiEnabled` / `apiMethods` exposure gate that HTTP applies #8083

What differs

Why it matters

Suggested shape (not prescriptive)

Source

ACCEPT — PR #8265

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

The two MCP transports run their data verbs through different seams — stdio skips the ADR-0049 apiEnabled / apiMethods exposure gate that HTTP applies #8083

Description

What differs

Why it matters

Suggested shape (not prescriptive)

Source

Activity

hotlong commented on Aug 12, 2026

os-zhuang commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

ACCEPT — PR #8265

os-zhuang commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

The two MCP transports run their data verbs through different seams — stdio skips the ADR-0049 `apiEnabled` / `apiMethods` exposure gate that HTTP applies #8083