Repository navigation
Conversation
Serve the same three tools in-process over Streamable HTTP (stateless) and legacy SSE behind a required bearer header, then drive both through McpService: custom headers reach the server, settings stay encrypted, write-capable tools cannot be enabled, read-only calls succeed, and a tool that loses its read-only annotation drops out of enabledTools on the next connect.
The custom fetch handed to the SDK transports is reused for OAuth discovery, registration and token requests, and global fetch follows redirects, so configured headers could leave the configured server. Decide per request URL: only the configured origin gets the headers, never a /.well-known/ discovery document, and a request carrying them is sent with redirect: "manual" and refused on any 3xx. OAuth traffic to a separate authorization server goes through plain fetch. Tests cover a separate OAuth origin (headers reach /mcp only, never discovery or the authorization server) and a redirected request over both Streamable HTTP and SSE (connect fails, the redirect target receives nothing). Docs state that read-only annotations are advisory and that a local command server grants local command execution.
The grant check used the tool list saved at the last connect, so a tool the server had since marked as a write tool, or stopped listing, still ran. call() now lists tools on its own connection first. discover() saves that list and drops grants that no longer apply, and the call is refused before callTool when the grant is gone. A failed listing fails the call. The regression test covers both transports: a tool that turns write- classified and a tool that disappears are refused with nothing dispatched, their grants are dropped, and they stay off when the server lists them as read-only again until the owner enables them.
Author
|
@jerelvelarde This is the focused replacement for #88 that you suggested. |
This branch has not been deployed
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.
Replaces #88, as suggested in #88 (comment). Same feature, rebuilt on current main, with the tool-authorization finding from the October 6 review fixed.
What changed
Owners can add their own MCP servers from Apps → MCP connections: Streamable HTTP, legacy SSE, or a local stdio command. OpenMuse discovers the tools, and the owner enables the ones the server marks read-only. Built-in chat and delegated tasks can then list connections and call enabled tools. Connection settings and OAuth material are encrypted server-side.
Tool authorization is checked against the server's current list, not the saved one. Before,
call()checked the grant against the tool list saved at the last connect and then dispatched on a fresh connection. A tool the server had since re-marked as a write tool kept running. Nowcall()lists tools on its own connection beforecallTool. That list is saved, grants for tools that are missing or no longer read-only are dropped, and the call is refused. The check fails closed: if listing fails, nothing runs, and a dropped grant stays dropped until the owner reconnects and enables the tool again. (apps/server/src/mcp.ts,call())Credential scoping from the first review is kept. Configured headers go only to the configured MCP origin, never to a
/.well-known/document or a separate OAuth origin, and a request that carries them refuses any redirect.Integrated with current main:
@modelcontextprotocol/sdkuses main's^1.30.0(added for Parallel search) instead of the old1.26.0pin. The lockfile is main's, unchanged.read_calendarand the session tests from main sit beside the MCP tools and tests.Verification
a call re-checks the server's current tool list and fails closed on a changed grant, over both Streamable HTTP and SSE: enableechowhile read-only, flip it to write on the server with no reconnect, and call. The call is refused, the server records no dispatch, and the grant is removed. A tool the server stops listing is refused the same way. When both come back read-only they stay off until the owner reconnects and enables them. The test fails on the previous head withMissing expected rejectionand passes with the fix.tests/mcp.test.ts: 5/5 pass, including the separate-OAuth-origin and redirect tests, now on SDK 1.30.pnpm typecheckpasses.pnpm test: 523 passed, 3 skipped, 0 failed.pnpm build:serverandpnpm build:webpass.biome checkpasses on every changed file. Fullpnpm linton my Windows checkout only reports CRLF in files this branch does not touch.Integration limits
docs/MCP.mdsays so, and says that enabling a local stdio server grants it local command execution as the server user.