Let a bot file an MCP server switched off

[guylfe]

The user can ask a bot to add an MCP connection. It is saved disabled, the bot cannot enable or test it, and an existing server is left unchanged.

Summary by CodeRabbit

• New Features
  • You can now ask the assistant to add a local or remote MCP server configuration. The assistant can add servers only when you explicitly request it, and they’re saved switched off.
  • Enable a newly added server in settings when you’re ready. The assistant can’t enable or test it, or change existing servers.
  • The assistant can share connection details and the names of configured environment variables or headers without revealing their secret values.
  • Local command servers run on your computer when enabled.

[vercel]

@guylfe is attempting to deploy a commit to the SupaMaus Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: ca0080c6-c7f0-40a2-8d9a-7c08dff8c8f0

📥 Commits

Reviewing files that changed from the base of the PR and between 1ec112a and 58c5383.

📒 Files selected for processing (9)

• server/drivers/agents-call.ts
• server/drivers/agents-catalog-goldens/direct-full.tools-list.json
• server/drivers/agents-catalog-goldens/profiles.json
• server/drivers/agents-catalog-goldens/room-full.tools-list.json
• server/drivers/agents-catalog-wire.test.ts
• server/drivers/agents-catalog.ts
• server/drivers/agents-proxy.test.ts
• server/index.test.ts
• server/index.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds an internal endpoint for bot-submitted MCP server configurations. It validates and stores new servers as disabled, then exposes an agent tool that submits configurations and reports the result.

Changes

MCP server submission

Layer / File(s)	Summary
Validate and add disabled servers server/mcp-registry.ts, server/mcp-registry.test.ts	The registry helper accepts supported server fields and adds parsed servers in the disabled state. Tests cover command and remote configurations, enabled-field rejection, duplicate names, and the server limit.
Add the internal submission endpoint server/index.ts, server/index.test.ts	The endpoint handles update locks, validation failures, and managed-policy refusals. It persists accepted configurations and returns server details. API tests check disabled state, duplicate handling, and secret-value exclusion from responses and listings.
Expose and handle the agent tool server/drivers/agents-catalog.ts, server/drivers/agents-call.ts, server/drivers/agents-catalog-goldens/*, server/drivers/agents-catalog-wire.test.ts, server/drivers/agents-proxy.test.ts, server/drivers/agents-call.test.ts	The catalog defines add_mcp_server and adds it to profiles. callTool submits its arguments to the endpoint and reports server details without including secret values. Related catalog and call tests are updated.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Agent
  participant callTool
  participant InternalMcpEndpoint
  participant MCPRegistry
  participant MCPConfiguration
  Agent->>callTool: invoke add_mcp_server
  callTool->>InternalMcpEndpoint: POST server arguments
  InternalMcpEndpoint->>MCPRegistry: validate and add disabled server
  MCPRegistry-->>InternalMcpEndpoint: server and updated configuration
  InternalMcpEndpoint->>MCPConfiguration: persist updated configuration
  InternalMcpEndpoint-->>callTool: server details or error
  callTool-->>Agent: tool result

Loading

Suggested reviewers: milind-soni

Merge Risk: ⚪ Minimal · up to 58c53

The change lets a bot file an MCP server switched off, with checks that block enabling, overwriting, or exposing secrets. No actionable merge-blocking risk is evident.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 58c53

Connections stay disabled, but the new submission path does not verify that the user authorized each addition. This could allow unwanted entries in shared connection settings. Separate activation, duplicate rejection, and credential-value filtering substantially limit immediate impact.

Retained concerns

• Medium · security · inferred: The explicit-user-request constraint is instruction-only: an active bot turn can persist a new shared MCP entry without request-specific user authorization. Content that induces a bot to invoke the tool could therefore stage an unwanted connection or consume registry capacity. The new path cannot activate or overwrite entries; execution would require a separate activation step.

Security review details

Security Blast Radius

• observed — Writes affect the shared MCP configuration map, not a per-bot proposal store, and are bounded by the 20-server limit and duplicate-name rejection. After separate activation, a server can be inherited by bots without an explicit MCP-server selection; bots with a selection receive only their selected names.

Security Findings and Attack Paths

• inferred — A document, page, or tool result that induces an active bot to invoke add_mcp_server could cross from untrusted content into persistent shared configuration without an explicit user request. The supported outcome is unwanted disabled staging or registry-capacity consumption. Automatic execution and theft of existing stored credentials were not established.

Trust Boundaries and Controls

• observed — Internal authority is an opaque active-turn capability with bot/thread claim binding and capability-kind checks. Standing external-runtime capabilities are separately restricted. These checks establish caller identity, but the new endpoint does not establish request-specific user consent.
• observed — The existing public settings authority is deployment-sensitive: session requests require route scopes, desktop mutations require the desktop-owner token, and shared-service loopback callers are restricted to allowlisted service routes. Owner-trust loopback behavior remains broader, so bot lifecycle restrictions cannot be asserted universally across deployments.

Resilience and Maintainability Implications

• observed — Submission reuses protections against built-in MCP name collisions, harness-owned environment-variable injection, malformed headers, and organization-disallowed targets. Atomic persistence applies mode 0600 to the temporary inode and performs cleanup on write failure.

Hardening Proposals

• proposed — Bind submission to a server-verified, request-specific user authorization covering the proposed name and target, or retain bot submissions in a separate proposal store until authorized acceptance. Preserve the independent disabled-state and activation controls.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description summarizes the core behavior, but it does not address why the change is needed, how it was verified, or the applicable checklist items.	Add the required What changed, Why, and How it was verified sections. Report the test and typecheck results, complete the applicable checklist items, and state that screenshots are not applicable if there are no UI changes.
Docstring Coverage	❓ Inconclusive	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 7 files. (5 skipped: 3…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing a bot to add an MCP server in a disabled state.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 7 files. (5 skipped: 3 unsupported, 2 too large.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}