Complete MCP v2 extension management and security - #2870
Complete MCP v2 extension management and security#2870Hillary Mutisya (hillary-mutisya) wants to merge 4 commits into
Conversation
Complete the MCP v2 integration by making MCP servers fully managed, secure, dynamic TypeAgent extensions. - unify installed-agent and MCP runtime sources across all hosts - persist normalized MCP configs with stable identity, provenance, trust, enabled state, tool policy, and credential references - extend @Package install, list, update, and uninstall with MCP support, previews, confirmation, ambiguity handling, and management commands - add local MCP config imports and an MCP Registry v0.1 source with pagination, incremental caching, exact version resolution, remote HTTP, and verified npm package materialization - safely track and remove owned package paths without deleting shared or unrelated files - add credential-store and OAuth/PKCE integration points without persisting plaintext secrets - enforce transport, command, domain, registry, package, and per-tool policies before installation, connection, and invocation - require confirmation for potentially mutating MCP tools and replace Copilot's unconditional permission approval - add sanitized, bounded audit logging for MCP lifecycle and tool activity - validate MCP arguments and structured results using bounded JSON Schema 2020-12 validation with external references disabled - preserve tool metadata under stable server-config/tool identities - subscribe to tools/list_changed and atomically refresh schemas across all connected sessions through the dispatcher reload path - add extensive coverage for installation transactions, registry behavior, security, OAuth, lifecycle fan-out, validation, and catalog refresh Durable secret persistence and browser-based OAuth interaction remain host-injected interfaces. Default implementations use environment/session credentials and fail explicitly when durable secure storage is unavailable.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| function shouldPromptForTool( |
There was a problem hiding this comment.
The decision for when a tool should be prompting should probably be left to the User's explicit approval or the calling host. We probably shouldn't decide solely based on the MCP Server's self-reporting because :
- The server we're calling could hypothetically be malicious and falsely report its safety
- The MCP server could just be incorrectly labeled leading to unexpected destructive behavior
It's probably better to use the annotations to be able to only increase the restrictiveness of the gate but not reduce it. Shifting the standard gating decision to the host/user will help improve our security stance.
| if ( | ||
| configured === "allow" || | ||
| !shouldPromptForTool(config, tool) || | ||
| sessionApprovals.get(sessionId)?.has(toolId) |
There was a problem hiding this comment.
This is kind of a low priority/unlikely scenario in the life of a single session but something to consider :
I see that we check the sessionApprovals for the existence of the toolId in the session. However, there is a potential scenario where a MCP server could update their tool and the behavior changes, but it's still approved. Ideally, it'll need to reacquire approval, but there isn't a great way of doing that right now.
I do see below that we check for "toolsChanged", in terms of updates to the catalog. It may be worth considering if we can track something similar for tool versioning to refresh approval requirements, but it's more of a nice todo in the future.
| }); | ||
| } | ||
| await mutation.addProvider(next); | ||
| await replaceUserConfig(config, issuingController); |
There was a problem hiding this comment.
A potential edgecase here is that replaceUserConfig can commit the config successfully, but the following services.audit.write() can still fail for artbirary reasons (like running out of space, permissions, locks, etc).
If the write() fails, it'll throw an exception in the parent tryCatch during runMcpInstall, triggering cleanup() which deletes the MCP package files.
This can lead to an awkward situation where the MCP server appears in the list, but it won't actually function because the package files have been cleaned up.
You may want to make the audit best effort but not trigger the cleanup rollback. Alternatively, the rollback behavior should probably remove the config as well. Either option seem reasonable but would be good to clean up.
3 participants
Complete the MCP v2 integration by making MCP servers fully managed, secure, dynamic TypeAgent extensions.
Durable secret persistence and browser-based OAuth interaction remain host-injected interfaces. Default implementations use environment/session credentials and fail explicitly when durable secure storage is unavailable.