[v0.8 MCP 5] enforce project MCP trust boundaries - #1337
Conversation
Reconstruct the unique net delta from PR #1263, excluding propagation merges.
Register every supported MCP child command and describe declaration tests as offline previews.
jonaowen
left a comment
There was a problem hiding this comment.
This security boundary is POSIX-only but is being merged as cross-platform product behavior. supportsRetainedDirectoryFd() unconditionally returns false on win32, so every project trust policy creates an empty invalid authority and all project mcp ... --project commands fail closed on Windows. There is no Windows test or user-facing capability contract. If project declarations are intended unsupported on Windows, that must be explicit in scope/docs/help and tested; otherwise implement a reparse-safe retained-handle boundary.
There is also an FD lifecycle leak independent of platform. releaseMcpProjectTrustBinding() only marks/deletes the binding record; it never closes the retained root FD. Authority FDs are closed only by a FinalizationRegistry registered on the authority, but every released admission’s private AdmissionPair remains in admissionPairs and strongly retains the authority. releasedAdmissions is itself a WeakSet and does not sever that value. Repeated CLI/service compositions can therefore accumulate one retained FD per allowed directory until GC behavior and the side maps happen to release the admission keys; explicit release does not actually release the authority resource.
Add deterministic authority disposal/ref-count ownership that closes each retained descriptor exactly once after the last binding/composition release (including failed construction), and behavioral FD-count/EBADF tests across repeated admit/release cycles, concurrent bindings, duplicate release, and authority GC. Do not use finalization as the normal ownership path.
Finally, the public mcp test command is materially misleading: it performs no test and returns exactly the same offline preview as preview. The help calls it “test” while only the summary says preview. Remove/rename it or make the public usage unambiguously preview; do not create a compatibility spelling before there is a released compatibility obligation.
|
Fresh independent review confirms the above and adds one public SDK contract blocker: The reviewer otherwise found the global-before-project ordering, descriptor-relative helper, signal-masked lock acquisition, and declaration-only inert staging directionally sound. This verdict does not import the separate #1338 transport blockers. |
Replacement scope
This PR reconstructs and supersedes the unique implementation delta reviewed in #1263 without rewriting that historical branch. The original PR remains the immutable discussion record: #1263
core02-host-request-dispatcherv080/mcp-split-m5-project-trusta3ef6b6697d867a6740eaf8e8592ca85487b4f8677b188b92dc91365cb2bc41bdb46a50669d104a8) is the shared foundation. For reconstructed deltas it is a proven tree-compatible base, not an ancestry claim about the historical PR stack.Validation
tsgo --noEmit: passNo original PR was retargeted, closed, merged, or otherwise mutated.
Note
High Risk
Changes security-sensitive project filesystem access, trust policy, and MCP wiring at CLI/SDK startup; misconfiguration or stale bindings fail closed by omitting project declarations rather than surfacing errors in some paths.
Overview
Introduces declarative MCP endpoint records (name, HTTP(S) URL, enabled) separate from legacy
mcpServers, plus a newprime-agent mcpcommand family for list/inspect/preview/test/add/enable/disable/remove on user or--projectscope. Commands only mutate declarations, redact sensitive output, andtestreturns an offline initialize probe—no runtime or auth.Project scope is gated by a global
mcpProjectTrustPolicy(allowed directories + revision). The CLI loads global settings only before admitting project work; denied or malformed policy never opens project settings. Admitted access uses opaque admission tokens, a pinned-directory trust authority (retained root FD + binding validation), and descriptor-relative reads/writes of.prime/agent/settings.jsonvia a bounded Python helper.Session/SDK startup composes admission before full
SettingsManagerproject load, builds an immutable runtime declaration snapshot (user first; project merged only when admission stays valid; name/endpoint collisions drop the whole project contribution), and passes it toMcpManagerviagetDeclarationSnapshot()while host handlers still use global-only legacy servers. InjectedMcpManagerinstances are left untouched.Reviewed by Cursor Bugbot for commit 15f003a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Enforce MCP project trust boundaries with admission-gated declaration snapshots
McpProjectTrustAuthoritythat pins project directories via retained file descriptors and issues opaque, revocable bindings; bindings are validated before and after use to detect identity changes (ABA, symlink swaps, root replacement).ProjectMcpDeclarationAdmissionas a capability token gating all project MCP declaration reads and writes; admission is derived from a global trust policy and released after session initialization.McpRuntimeDeclarationSnapshotcombining user and (admission-gated) project declarations, with collision detection, deterministic code-point ordering, and a SHA-256 revision digest;McpManagerexposes this viagetDeclarationSnapshot().ProjectSettingsOpenatclass that reads/writes.prime/agent/settings.jsonthrough a sandboxed stdlib-only Python helper over a dirfd, enforcing size/time limits and cooperative locking without exposing project paths.mcpcommand group (list,inspect,preview,test,add,enable,disable,remove) that enforces project trust policy for--projectscope and outputs redacted JSON.Macroscope summarized 15f003a.