Skip to content

[Fix] Approval control no longer gates Roomote-internal MCP servers - #3101

Open
roomote-roomote[bot] wants to merge 6 commits into
developfrom
roomote/internal-mcp-approval-exempt
Open

roomote-roomote[bot] wants to merge 6 commits into
developfrom
roomote/internal-mcp-approval-exempt

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

​Opened on behalf of @daniel-lxs. Follow up by mentioning @roomote-roomote, in the web UI, or in Telegram.

Related issue

Related to floating product feedback from an internal owner; no linked issue. (If required to close this PR, please have a maintainer rename.)

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

What changed

Per-tool integration approvals excluded Roomote's own MCP servers: the internal classification (INTERNAL_MCP_SERVER_IDS: roomote, _roomote_http_integrations, gbrain) now labels those servers approval-exempt, so they are no longer governed by the shared policy resolver, compile into no native ask/deny rules, return no blocks at the integration-boundary proxy, and no longer fall through the SDK's task-approval recorder. The admin listing, personal policy setters (single and bulk), session override setter, and pending/override GET route all reject or filter internal ids, and the settings UI hides approval controls for them.

A native permission key cannot name which server half it came from, so a key an actual internal tool flattens to (for example a custom gbrain_get server's page tool colliding with the Brain's own get_page) is left ungated in both compilers: enforcing it would also hold the exempt internal tool. The drop is exact — a custom server whose id merely starts with an internal server's name keeps its rules. The Brain's agent-facing tool set is the existing static allowlist (GBRAIN_READ_TOOL_NAMES, now in @roomote/types next to BRAIN_MCP_ID); _roomote_http_integrations is structurally collision-free because its flattened keys start with _, which no governable server name can produce; roomote is not mounted in task runs. The integration proxy remains the authoritative per-integration boundary for task runs.

New custom MCP servers may not take a name starting with an internal server prefix (roomote_, gbrain_): a flattened-key collision requires exactly that prefix, so no external tool's ask policy can become permanently unapprovable through a native-key collision. Legacy rows with such names stay editable (names are immutable) but are never mounted into tasks — their tools cannot gate an exempt internal tool and their own policies stay inert. The Fast addRemoteCustomMcpForFast creation path enforces the same prefix guard, so agents cannot create rows that would silently never mount.

Auto mode (#3084) also honors the exemption: internal servers get no <server>_* wildcard ask rule.

External built-ins and custom MCPs keep exactly the same approval behavior they had; this only removes approval control from genuine Roomote-internal infrastructure (do not exempt Notion/Linear/Granola-like in-process catalog integrations).

How it was tested

Regression tests added:

  • @roomote/types: resolver never governs internal ids; task-rule compiler skips them and drops only the exact native keys of actual internal tools; isInternalMcpServer predicate coverage; structural invariant that the HTTP integrations broker cannot collide; create-only name rejection and legacy-name update acceptance; no Auto wildcard for internal servers.
  • @roomote/api: resolveProxyToolApprovalBlocks returns no blocks, no Auto default, and no shadow assessment for internal ids.
  • @roomote/cloud-agents: fast-agent rule builder emits no rules for internal ids and drops rules for keys shared with internal tools.
  • @roomote/sdk: requestTaskToolApproval returns not_required for internal ids; legacy internal-prefixed custom servers never mount; the Fast creation path rejects internal-prefixed names before any fetch or write.

Suites run: full @roomote/types (1172), sdk mcp-connections (50), sdk task-tool-approvals (13), sdk add-remote-custom-mcp (39), API enforcement (11), web integration-tool-policies and custom-mcp-servers commands (24). CI Test check passes at a697dcd.

Screenshots

Creating a custom MCP server named gbrain_get is rejected with the internal-prefix validation message

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

@roomote-community

roomote-community Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

No code issues found. See task

  • Internal MCP exemption still loses on an OpenCode permission-key collision (packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts:129; task compiler also affected).
  • Task compiler disables approvals for unrelated custom MCP servers whose ids start with an internal-server prefix (packages/types/src/integration-tool-approvals.ts:301).
  • Exact native-key collision leaves an external tool's ask policy permanently unapprovable (packages/types/src/integration-tool-approvals.ts:341; Fast compiler also affected).
  • Existing internal-prefixed custom MCP rows remain mounted and permanently unapprovable under ask (packages/types/src/custom-mcp-servers.ts:242).
  • Fast remote-MCP creation now enforces the internal-prefix guard before probing or writing (packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.ts:799).

Reviewed a697dcd

@roomote-roomote

roomote-roomote Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

All five review issues are fixed; latest commit a697dcd8:

  1. Native permission keys shared with internal tools are left ungated in both compilers.
  2. The task compiler's drop is exact, using the Brain's actual tool allowlist (GBRAIN_READ_TOOL_NAMES, now in @roomote/types).
  3. New custom server names starting with an internal server prefix are rejected at create time (customMcpServerCreateInputSchema).
  4. Legacy rows with such names stay editable but are excluded at mount time in both the remote and stdio paths.
  5. The Fast addRemoteCustomMcpForFast creation path now enforces the same prefix guard before parsing, so agents cannot create rows the mount-time exclusion would silently drop.

The Test CI check and the Roomote code review check both pass on a697dcd. All five review threads resolved.

@roomote-community

Copy link
Copy Markdown
Contributor

I saw the mention, but I need your GitHub account linked to Roomote before issue and PR comments can start work here. Link it from Settings -> Linked Accounts and then mention me again. If you'd like an invite to join the community Roomote, please send MrUbens a message on Discord: http://discord.gg/roomote

Comment thread packages/types/src/integration-tool-approvals.ts Outdated
Comment thread packages/types/src/integration-tool-approvals.ts
Comment thread packages/types/src/custom-mcp-servers.ts Outdated
Comment thread packages/types/src/custom-mcp-servers.ts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant