Repository navigation
feat(mcp): add MCP server settings with project-trust gating - #893
Closed
YaoShenyaoge wants to merge 1 commit into
Closed
YaoShenyaoge wants to merge 1 commit into
YaoShenyaoge wants to merge 1 commit into
Conversation
Pi has no built-in MCP support: the runtime comes from an extension that reads mcp.json. This adds a settings section that edits the same two files the CLI and that extension read, so a single runtime serves the terminal, Pi Web, and any other host instead of a second, app-local implementation. - lib/mcp-config-store.ts: read/write global and project mcp.json with atomic writes, unknown keys preserved, and per-entry validation - app/api/mcp/route.ts: GET/POST with the standard request guards; project-scope writes require project trust, mirroring the plugin route - components/McpConfig.tsx: settings section listing configured servers plus the live mcp_* tools the session reports - ADR 0004 records why the surface is config-only and why project servers are gated: a project mcp.json spawns a local process, which the extension-level project-trust gate does not cover Also exports PROJECT_SECTIONS from lib/settings-navigation so the settings panel no longer keeps its own copy of the project-section list.
Owner
|
Thanks @YaoShenyaoge, and especially for the project-trust analysis. I'm closing this because pi 0.99 now supports MCP natively. It reads the same For Pi Web, the remaining work is upgrading to 0.99 and wiring |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
A settings section for MCP servers, editing the two files the
piCLI and themcp-bridgeextension already read:<agentDir>/mcp.json(global)<cwd>/.pi/mcp.json(project), overriding global entries by namePi has no built-in MCP support; the runtime comes from an extension. So this PR
deliberately only edits config, and the running session applies changes on
reload (
reloadre-emitssession_start, which is where the extension re-readsthe file). One runtime then serves the terminal, Pi Web, and any other host,
instead of a second implementation that would only work inside this app.
The panel also shows which
mcp_*tools the session actually has registered, so"did my server start?" is answerable from the UI.
Security: why project servers are gated
While reading the plugin and trust code I noticed a gap worth calling out.
projectTrustReloadOptionscorrectly keeps a project's.pi/extensionsfromexecuting (issue #236). But that gate covers extension code loading, not
config file reads. An MCP server definition is a
commandthat gets spawnedas a local process, so a globally installed MCP extension reading
<cwd>/.pi/mcp.jsonbypasses the gate: clone a hostile repo, open it in Pi Web,and the repository's command runs.
This PR closes it on both sides:
app/api/mcp/route.tsrefuses project-scope writes while the project isuntrusted (403), so the UI cannot be used to plant a payload either.
isProjectTrusted()isfalse, so an untrusted repository stays dormant even with a global install.
Global config is never gated. Because override is resolved per entry, a project
entry that overrides a global server becomes project-origin and is therefore
gated too — fail closed.
Reasoning is recorded in
docs/adr/0004-mcp-server-config-surface.md.Files
lib/mcp-config-store.tsapp/api/mcp/route.tsGET ?cwd=andPOST {action,cwd,scope,name,config}; standard guards (isApiRequestAllowed403,hasJsonContentType415, allowed-root 403); project-scope writes require project trustcomponents/McpConfig.tsxlib/settings-navigation.tsmcpsection and exportsPROJECT_SECTIONS—SettingsPanelhad its own copy of that list, now sharedcomponents/SettingsPanel.tsxlib/i18n/messages/{en,zh-CN,zh-TW}.tsdocs/adr/0004-...mdVerification
lib/mcp-config-store.test.mjs(16),app/api/mcp/route.test.mjs(17)npm test: 1102 tests / 1083 pass / 14 fail. The 14 failures are pre-existingand identical to a clean
HEADworktree (compared test-by-test); they areWindows-environment issues (symlink EPERM, unix sockets, PTY, ripgrep)
npm run lint: 0 problemstsc --noEmit: cleanDeliberate non-goals
No "test connection" endpoint. That would let an HTTP request spawn an arbitrary
command, widening the surface this PR is trying to keep narrow; live tool names
from the session cover the same need read-only.
Open question
Two behaviours I did not touch, because they look like your call rather than
mine:
withExtensionToolsforce-adds every non-builtin tool, so all MCP tools arealways active — there is no way to disable an individual one, and a server
with 30 tools adds 30 tool definitions to every request.
validateSessionToolSelectionrejects non-builtin names, so an MCP toolselection cannot be persisted across reloads.
Both sit in ADR-0002 territory. Happy to follow up if you want either changed;
I would rather ask than refactor your tool-selection policy in an unrelated PR.