From 393ceac1b05042562dfe69cb63bce53e29b0ceae Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 08:06:58 +0000 Subject: [PATCH 1/4] [Fix] Approval control no longer gates Roomote-internal MCP servers --- .../tool-approval-enforcement.test.ts | 14 ++++++++ .../handlers/mcp/tool-approval-enforcement.ts | 6 ++++ .../integration-tool-approvals/route.ts | 16 ++++++++- .../IntegrationToolApprovalControls.tsx | 9 ++++- .../integration-tool-policies/index.ts | 35 ++++++++++++++++--- .../fast-agent-tool-approvals.test.ts | 25 +++++++++++++ .../fast-agent/fast-agent-tool-approvals.ts | 4 +++ .../lib/__tests__/task-tool-approvals.test.ts | 13 +++++++ .../sdk/src/server/lib/task-tool-approvals.ts | 6 ++++ .../src/__tests__/custom-mcp-servers.test.ts | 15 ++++++++ ...integration-tool-policy-strictness.test.ts | 13 ++++--- .../task-integration-tool-approvals.test.ts | 20 +++++++++++ packages/types/src/custom-mcp-servers.ts | 22 ++++++++++-- .../types/src/integration-tool-approvals.ts | 10 ++++++ 14 files changed, 195 insertions(+), 13 deletions(-) diff --git a/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts b/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts index d5838dd2ba..db603a74d0 100644 --- a/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/tool-approval-enforcement.test.ts @@ -129,6 +129,20 @@ describe('resolveProxyToolApprovalBlocks', () => { expect(mockUser).not.toHaveBeenCalled(); }); + it('never blocks Roomote-internal MCP servers', async () => { + for (const integrationId of ['roomote', 'gbrain']) { + const blocks = await resolveProxyToolApprovalBlocks({ + integrationId, + tokenType: 'run', + resolveActingUserId: async () => 'user-1', + resolveTaskId: async () => 'task-1', + }); + expect(blocks.size).toBe(0); + expect(mockDeployment).not.toHaveBeenCalled(); + expect(mockUser).not.toHaveBeenCalled(); + } + }); + it('holds an auto tool for a task run exactly like an ask tool', async () => { mockDeployment.mockResolvedValue([policy('linear', 'save_issue', 'auto')]); const task = await resolveProxyToolApprovalBlocks({ diff --git a/apps/api/src/handlers/mcp/tool-approval-enforcement.ts b/apps/api/src/handlers/mcp/tool-approval-enforcement.ts index b74f7e6fd2..4f4652f804 100644 --- a/apps/api/src/handlers/mcp/tool-approval-enforcement.ts +++ b/apps/api/src/handlers/mcp/tool-approval-enforcement.ts @@ -10,6 +10,7 @@ import { } from '@roomote/db/server'; import { integrationToolModeAsks, + isInternalMcpServer, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, type IntegrationToolPolicyMode, @@ -55,6 +56,11 @@ export async function resolveProxyToolApprovalBlocks(input: { if (!(await isDeploymentExperimentEnabled('integrationToolApprovals'))) { return blocks; } + // Roomote's own MCP and other internal servers are outside approval + // control entirely; their tools always pass. + if (isInternalMcpServer(input.integrationId)) { + return blocks; + } const actingUserId = input.policyScope === 'deployment' ? null diff --git a/apps/web/src/app/api/sessions/[sessionId]/integration-tool-approvals/route.ts b/apps/web/src/app/api/sessions/[sessionId]/integration-tool-approvals/route.ts index 95851e76ef..9ad44d215e 100644 --- a/apps/web/src/app/api/sessions/[sessionId]/integration-tool-approvals/route.ts +++ b/apps/web/src/app/api/sessions/[sessionId]/integration-tool-approvals/route.ts @@ -11,6 +11,7 @@ import { import { integrationToolApprovalDecisionSchema, integrationToolSessionOverrideUpsertSchema, + isInternalMcpServer, } from '@roomote/types'; import { authorize } from '@/lib/server/auth-context'; @@ -58,7 +59,19 @@ async function handle( listPendingIntegrationToolApprovals(context), listIntegrationToolSessionOverridesForRequester(context), ]); - return NextResponse.json({ pending, sessionOverrides }, { headers }); + // Internal MCPs are outside approval control; never surface asks or + // overrides for them. + return NextResponse.json( + { + pending: pending.filter( + (approval) => !isInternalMcpServer(approval.integrationId), + ), + sessionOverrides: sessionOverrides.filter( + (override) => !isInternalMcpServer(override.integrationId), + ), + }, + { headers }, + ); } // Only configured public authority is trusted, never caller-supplied proxy headers. @@ -93,6 +106,7 @@ async function handle( body.value, ); if (!override.success) return error(400); + if (isInternalMcpServer(override.data.integrationId)) return error(400); await setIntegrationToolSessionOverride(context, override.data); return NextResponse.json({ ok: true }, { status: 200, headers }); } diff --git a/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx b/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx index 5778760dbf..25b24ca78e 100644 --- a/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx +++ b/apps/web/src/components/settings/IntegrationToolApprovalControls.tsx @@ -4,6 +4,7 @@ import { useState, type ReactNode } from 'react'; import { integrationToolPolicyKey, + isInternalMcpServer, type IntegrationToolPolicyMode, } from '@roomote/types'; @@ -269,7 +270,13 @@ export function IntegrationToolApprovalList({ toggleDisabled?: boolean; }) { const experiment = useIntegrationToolApprovalsExperiment(); - const active = experiment.enabled && canManage && integrationId != null; + // Internal MCPs (Roomote's own server, the integrations broker, Brain + // memory) are outside approval control: no approval UI, ever. + const active = + experiment.enabled && + canManage && + integrationId != null && + !isInternalMcpServer(integrationId); const policies = useIntegrationToolPolicies({ enabled: open && active, scope, diff --git a/apps/web/src/trpc/commands/integration-tool-policies/index.ts b/apps/web/src/trpc/commands/integration-tool-policies/index.ts index 17eca089c3..9a03dc5e01 100644 --- a/apps/web/src/trpc/commands/integration-tool-policies/index.ts +++ b/apps/web/src/trpc/commands/integration-tool-policies/index.ts @@ -7,12 +7,29 @@ import { upsertIntegrationToolPolicy, upsertIntegrationToolUserPolicy, } from '@roomote/db/server'; -import type { IntegrationToolPolicyUpsert } from '@roomote/types'; +import { + isInternalMcpServer, + type IntegrationToolPolicyUpsert, +} from '@roomote/types'; import type { UserAuthSuccess } from '@/types'; import { assertAdmin } from '../setup/shared'; +/** + * Internal MCPs (Roomote's own server, the HTTP integrations broker, Brain + * memory) are excluded from approval control; reject attempts to configure + * them and hide any previously stored rows for them. + */ +function assertApprovalManagedIntegrationId(integrationId: string) { + if (isInternalMcpServer(integrationId)) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: `${integrationId} is a Roomote-internal MCP server and is outside approval control.`, + }); + } +} + /** * Experiment-gated (`integrationToolApprovals`) per-tool approval policies. * Deployment-scoped and admin-managed: every session on the deployment runs @@ -22,7 +39,9 @@ export async function listIntegrationToolPoliciesCommand( auth: UserAuthSuccess, ) { assertAdmin(auth); - return listIntegrationToolPolicies(); + return (await listIntegrationToolPolicies()).filter( + (policy) => !isInternalMcpServer(policy.integrationId), + ); } export async function setIntegrationToolPolicyCommand( @@ -30,11 +49,12 @@ export async function setIntegrationToolPolicyCommand( input: IntegrationToolPolicyUpsert, ) { assertAdmin(auth); + assertApprovalManagedIntegrationId(input.integrationId); await upsertIntegrationToolPolicy({ ...input, updatedByUserId: auth.userId, }); - return listIntegrationToolPolicies(); + return listIntegrationToolPoliciesCommand(auth); } const toolApprovalsEnabled = () => @@ -49,7 +69,9 @@ export async function listPersonalIntegrationToolPoliciesCommand( auth: UserAuthSuccess, ) { if (!(await toolApprovalsEnabled())) return []; - return listIntegrationToolUserPolicies(auth.userId); + return (await listIntegrationToolUserPolicies(auth.userId)).filter( + (policy) => !isInternalMcpServer(policy.integrationId), + ); } export async function setPersonalIntegrationToolPolicyCommand( @@ -62,6 +84,9 @@ export async function setPersonalIntegrationToolPolicyCommand( message: 'Tool approvals are not enabled.', }); } + assertApprovalManagedIntegrationId(input.integrationId); await upsertIntegrationToolUserPolicy({ ...input, userId: auth.userId }); - return listIntegrationToolUserPolicies(auth.userId); + return (await listIntegrationToolUserPolicies(auth.userId)).filter( + (policy) => !isInternalMcpServer(policy.integrationId), + ); } diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts index e490efe80d..ab7cf769be 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts @@ -94,6 +94,31 @@ describe('buildIntegrationToolApprovalRules', () => { expect(buildIntegrationToolApprovalRules(integrations, [])).toEqual([]); }); + it('never emits rules for Roomote-internal MCP servers', () => { + const internalIntegrations: FastAgentIntegration[] = [ + { + id: 'roomote', + name: 'Roomote', + description: '', + tools: [ + { name: 'manage_tasks', description: '', inputSchema: {} }, + { name: 'chat_reaction', description: '', inputSchema: {} }, + ], + } as unknown as FastAgentIntegration, + ]; + const rules = buildIntegrationToolApprovalRules(internalIntegrations, [ + { + policyId: 'p1', + integrationId: 'roomote', + toolName: 'manage_tasks', + mode: 'reject', + updatedAt: '', + createdAt: '', + }, + ]); + expect(rules).toEqual([]); + }); + it('never lets distinct integration/tool pairs share one policy entry', () => { // Regression: a delimiter-less composite key makes `a`/`bc` and `ab`/`c` // the same map entry, so one pair's mode would gate the other. diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts index 201d676ee2..e77c7b5db4 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts @@ -20,6 +20,7 @@ import { import { integrationToolModeAsks, integrationToolPolicyKey, + isInternalMcpServer, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, type IntegrationToolApprovalMetadata, @@ -123,6 +124,9 @@ export function buildIntegrationToolApprovalRules( // a block, never let a gated tool run ungated. const actionByKey = new Map(); for (const tool of listMountedIntegrationTools(integrations)) { + // Roomote internal MCPs are outside approval control entirely; their + // tools keep OpenCode's default allow even when a policy row exists. + if (isInternalMcpServer(tool.integrationId)) continue; const key = integrationToolPolicyKey(tool.integrationId, tool.toolName); const policyMode = modeByTool.get(key); const mode = resolveEffectiveIntegrationToolMode({ diff --git a/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts b/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts index 6d77688ffd..9480348e3d 100644 --- a/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts +++ b/packages/sdk/src/server/lib/__tests__/task-tool-approvals.test.ts @@ -114,6 +114,19 @@ describe('resolveTaskIntegrationToolApprovals', () => { }); describe('requestTaskToolApproval', () => { + it('lets Roomote-internal MCP calls run without an approval', async () => { + await expect( + requestTaskToolApproval({ + ...ask, + integrationId: 'roomote', + toolName: 'manage_tasks', + }), + ).resolves.toEqual({ outcome: 'not_required' }); + expect(mocks.sessionForTask).not.toHaveBeenCalled(); + expect(mocks.insert).not.toHaveBeenCalled(); + expect(mocks.insertAuto).not.toHaveBeenCalled(); + }); + it("records the ask on the task's Session for its owner", async () => { await expect(requestTaskToolApproval(ask)).resolves.toEqual({ outcome: 'pending', diff --git a/packages/sdk/src/server/lib/task-tool-approvals.ts b/packages/sdk/src/server/lib/task-tool-approvals.ts index e63209c421..246b82f643 100644 --- a/packages/sdk/src/server/lib/task-tool-approvals.ts +++ b/packages/sdk/src/server/lib/task-tool-approvals.ts @@ -17,6 +17,7 @@ import { import { recordIntegrationToolAutoEvaluationInBackground } from '@roomote/cloud-agents/server/integration-tool-auto-evaluation'; import { compileTaskIntegrationToolApprovals, + isInternalMcpServer, resolveGoverningIntegrationToolPolicies, type IntegrationToolApprovalStatus, type IntegrationToolPolicyScope, @@ -109,6 +110,11 @@ export async function requestTaskToolApproval(input: { if (!(await isDeploymentExperimentEnabled('integrationToolApprovals'))) { return { outcome: 'not_required' }; } + // Internal MCPs never gate: the call runs without recording an approval, + // same as a tool with no governing policy. + if (isInternalMcpServer(input.integrationId)) { + return { outcome: 'not_required' }; + } const session = await resolveTaskApprovalSession(input.runId); if (!session?.ownerUserId) return { outcome: 'unavailable' }; const context = { sessionId: session.sessionId, userId: session.ownerUserId }; diff --git a/packages/types/src/__tests__/custom-mcp-servers.test.ts b/packages/types/src/__tests__/custom-mcp-servers.test.ts index a974eda55b..5741ea7c81 100644 --- a/packages/types/src/__tests__/custom-mcp-servers.test.ts +++ b/packages/types/src/__tests__/custom-mcp-servers.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { RESERVED_CUSTOM_MCP_SERVER_NAMES, customMcpServerInputSchema, + isInternalMcpServer, validateCustomMcpHeaderName, validateCustomMcpServerUrl, } from '../custom-mcp-servers'; @@ -15,6 +16,20 @@ const validServer = { headers: { 'x-api-key': 'secret-value' }, }; +describe('isInternalMcpServer', () => { + it('recognizes Roomote infrastructure and nothing else', () => { + expect(isInternalMcpServer('roomote')).toBe(true); + expect(isInternalMcpServer('_roomote_http_integrations')).toBe(true); + expect(isInternalMcpServer('gbrain')).toBe(true); + // In-process catalog integrations still count as external. + expect(isInternalMcpServer('notion')).toBe(false); + expect(isInternalMcpServer('granola')).toBe(false); + expect(isInternalMcpServer('linear')).toBe(false); + // Memory-category catalog entries are deployment integrations too. + expect(isInternalMcpServer('supermemory')).toBe(false); + }); +}); + describe('customMcpServerInputSchema', () => { it('accepts a valid static-header server', () => { expect(customMcpServerInputSchema.safeParse(validServer).success).toBe( diff --git a/packages/types/src/__tests__/integration-tool-policy-strictness.test.ts b/packages/types/src/__tests__/integration-tool-policy-strictness.test.ts index 21fc66258d..6f555e3a95 100644 --- a/packages/types/src/__tests__/integration-tool-policy-strictness.test.ts +++ b/packages/types/src/__tests__/integration-tool-policy-strictness.test.ts @@ -76,12 +76,17 @@ describe('resolveGoverningIntegrationToolPolicies', () => { expect(resolve()).toEqual(['personal_only', 'shared_only']); }); - it('never lets distinct integration and tool pairs share one entry', () => { + it('never governs Roomote-internal MCP servers', () => { const governing = resolveGoverningIntegrationToolPolicies({ - deploymentPolicies: [policy('a', 'bc', 'ask')], - userPolicies: [policy('ab', 'c', 'reject')], + deploymentPolicies: [ + policy('roomote', 'manage_tasks', 'reject'), + policy('_roomote_http_integrations', 'integration_request', 'ask'), + policy('gbrain', 'query', 'reject'), + policy('linear', 'save_issue', 'reject'), + ], + userPolicies: [policy('roomote', 'manage_tasks', 'ask')], scopeOf: () => undefined, }); - expect(governing).toHaveLength(2); + expect(modes(governing)).toEqual({ save_issue: 'reject' }); }); }); diff --git a/packages/types/src/__tests__/task-integration-tool-approvals.test.ts b/packages/types/src/__tests__/task-integration-tool-approvals.test.ts index 6f5869308d..d54358e26a 100644 --- a/packages/types/src/__tests__/task-integration-tool-approvals.test.ts +++ b/packages/types/src/__tests__/task-integration-tool-approvals.test.ts @@ -87,4 +87,24 @@ describe('compileTaskIntegrationToolApprovals', () => { }); expect(compiled).toEqual({ permission: { a_b_c: 'deny' }, tools: {} }); }); + + it('never emits rules for Roomote-internal MCP servers', () => { + expect( + compileTaskIntegrationToolApprovals({ + serverNames: ['roomote', 'linear'], + policies: [ + policy('roomote', 'manage_tasks', 'reject'), + policy('linear', 'save_issue', 'ask'), + ], + sessionOverrides: [ + { integrationId: 'roomote', toolName: 'manage_tasks', mode: 'ask' }, + ], + }), + ).toEqual({ + permission: { linear_save_issue: 'ask' }, + tools: { + linear_save_issue: { integrationId: 'linear', toolName: 'save_issue' }, + }, + }); + }); }); diff --git a/packages/types/src/custom-mcp-servers.ts b/packages/types/src/custom-mcp-servers.ts index 3ca8b109d2..ce347a43fd 100644 --- a/packages/types/src/custom-mcp-servers.ts +++ b/packages/types/src/custom-mcp-servers.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; import { MCP_INTEGRATIONS } from './mcp-oauth'; +import { BRAIN_MCP_ID } from './brain'; import { PRODUCT_NAME } from './constants'; import { collectReservedEnvReferences } from './reserved-mcp-env-vars'; @@ -39,14 +40,31 @@ export const ROOMOTE_MCP_ID = 'roomote'; // Leading underscore keeps infrastructure outside the valid deployment custom-name namespace. export const HTTP_INTEGRATIONS_MCP_ID = '_roomote_http_integrations'; -export const RESERVED_CUSTOM_MCP_SERVER_NAMES: ReadonlySet = new Set([ +/** + * MCP servers Roomote itself wires into every task: its own MCP, the HTTP + * integrations broker, and the Brain memory store. Approval control never + * applies to them: they are Roomote-internal infrastructure, not deployment + * integrations, and gating their tools would pause or block the product + * itself. External catalog integrations (Notion, Linear, Granola, ...) + * intentionally do not qualify, even when their handler runs in-process + * (`serverMode: 'native'`). + */ +export const INTERNAL_MCP_SERVER_IDS: ReadonlySet = new Set([ ROOMOTE_MCP_ID, HTTP_INTEGRATIONS_MCP_ID, + BRAIN_MCP_ID, +]); + +export function isInternalMcpServer(serverId: string): boolean { + return INTERNAL_MCP_SERVER_IDS.has(serverId); +} + +export const RESERVED_CUSTOM_MCP_SERVER_NAMES: ReadonlySet = new Set([ + ...INTERNAL_MCP_SERVER_IDS, 'github', 'slack', // The Brain is infrastructure rather than a catalog integration, so the // catalog cannot reserve its server name for it. - 'gbrain', ...MCP_INTEGRATIONS.map((integration) => integration.id.toLowerCase()), ]); diff --git a/packages/types/src/integration-tool-approvals.ts b/packages/types/src/integration-tool-approvals.ts index 82f1901ffc..f6de96782d 100644 --- a/packages/types/src/integration-tool-approvals.ts +++ b/packages/types/src/integration-tool-approvals.ts @@ -1,5 +1,7 @@ import { z } from 'zod'; +import { isInternalMcpServer } from './custom-mcp-servers'; + /** * Experiment-gated (`integrationToolApprovals`) per-integration-tool approval * policies and requests for code-mode integration calls in Sessions. @@ -196,6 +198,11 @@ export function resolveGoverningIntegrationToolPolicies< ['personal', input.userPolicies], ] as const) { for (const policy of policies) { + // Internal MCPs (Roomote's own server, the HTTP integrations broker, + // Brain memory) are never governed: approval policy is for deployment + // integrations, and gating product infrastructure would pause the + // product itself. + if (isInternalMcpServer(policy.integrationId)) continue; if ((input.scopeOf(policy.integrationId) ?? layer) !== layer) continue; const key = integrationToolPolicyKey( policy.integrationId, @@ -288,6 +295,9 @@ export function compileTaskIntegrationToolApprovals(input: { ...input.sessionOverrides, ]) { if (!mounted.has(integrationId)) continue; + // Internal MCPs are outside approval control entirely (see + // `resolveGoverningIntegrationToolPolicies`). + if (isInternalMcpServer(integrationId)) continue; const policyKey = integrationToolPolicyKey(integrationId, toolName); const policyMode = policyModes.get(policyKey); const mode = resolveEffectiveIntegrationToolMode({ From a7ee85cb8d05e843d5b60a723de5db39dad11aa2 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:09:23 +0000 Subject: [PATCH 2/4] [Fix] Internal MCP exemption no longer loses on native permission-key collisions --- .../fast-agent-tool-approvals.test.ts | 44 +++++++++++++++++++ .../fast-agent/fast-agent-tool-approvals.ts | 13 ++++++ .../task-integration-tool-approvals.test.ts | 24 ++++++++++ .../types/src/integration-tool-approvals.ts | 11 +++++ 4 files changed, 92 insertions(+) diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts index ab7cf769be..9ec61c2ca7 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-tool-approvals.test.ts @@ -119,6 +119,50 @@ describe('buildIntegrationToolApprovalRules', () => { expect(rules).toEqual([]); }); + it('drops a rule whose native key an internal tool also flattens to', () => { + // A custom `roomote_manage` server's `tasks` tool flattens to + // `roomote_manage_tasks`, the same native key as `roomote` / + // `manage_tasks`. Gating it would also hold the exempt internal tool. + const collidingIntegrations: FastAgentIntegration[] = [ + { + id: 'roomote', + name: 'Roomote', + description: '', + tools: [{ name: 'manage_tasks', description: '', inputSchema: {} }], + } as unknown as FastAgentIntegration, + { + id: 'roomote_manage', + name: 'Roomote Manage', + description: '', + tools: [ + { name: 'tasks', description: '', inputSchema: {} }, + { name: 'status', description: '', inputSchema: {} }, + ], + } as unknown as FastAgentIntegration, + ]; + const rules = buildIntegrationToolApprovalRules(collidingIntegrations, [ + { + policyId: 'p1', + integrationId: 'roomote_manage', + toolName: 'tasks', + mode: 'ask', + updatedAt: '', + createdAt: '', + }, + { + policyId: 'p2', + integrationId: 'roomote_manage', + toolName: 'status', + mode: 'reject', + updatedAt: '', + createdAt: '', + }, + ]); + expect(rules).toEqual([ + { permission: 'roomote_manage_status', pattern: '*', action: 'deny' }, + ]); + }); + it('never lets distinct integration/tool pairs share one policy entry', () => { // Regression: a delimiter-less composite key makes `a`/`bc` and `ab`/`c` // the same map entry, so one pair's mode would gate the other. diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts index e77c7b5db4..59bf5336ef 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-tool-approvals.ts @@ -123,6 +123,16 @@ export function buildIntegrationToolApprovalRules( // restrictive mode among them wins: a collision can only ever add an ask or // a block, never let a gated tool run ungated. const actionByKey = new Map(); + // A native key cannot name which server half it came from, so a key an + // internal tool flattens to (for example a custom `roomote_manage` + // server's `tasks` tool colliding with `roomote` / `manage_tasks`) must + // not gate anything: enforcing it would also hold the internal tool, and + // internal MCPs are exempt. + const internalKeys = new Set( + listMountedIntegrationTools(integrations) + .filter((tool) => isInternalMcpServer(tool.integrationId)) + .map((tool) => tool.key), + ); for (const tool of listMountedIntegrationTools(integrations)) { // Roomote internal MCPs are outside approval control entirely; their // tools keep OpenCode's default allow even when a policy row exists. @@ -148,6 +158,9 @@ export function buildIntegrationToolApprovalRules( actionByKey.set(tool.key, 'ask'); } } + for (const key of internalKeys) { + actionByKey.delete(key); + } const rules: PermissionRuleset = [...actionByKey].map( ([permission, action]) => ({ permission, pattern: '*', action }), ); diff --git a/packages/types/src/__tests__/task-integration-tool-approvals.test.ts b/packages/types/src/__tests__/task-integration-tool-approvals.test.ts index d54358e26a..ccdd7a422e 100644 --- a/packages/types/src/__tests__/task-integration-tool-approvals.test.ts +++ b/packages/types/src/__tests__/task-integration-tool-approvals.test.ts @@ -107,4 +107,28 @@ describe('compileTaskIntegrationToolApprovals', () => { }, }); }); + + it('drops a rule whose native key an internal tool could also flatten to', () => { + // A custom `roomote_manage` server's `tasks` tool flattens to + // `roomote_manage_tasks`, the same native key as `roomote` / + // `manage_tasks`. Gating it would also hold the exempt internal tool, + // so any key the internal server's tools could flatten to is left + // ungated natively; unrelated servers still gate normally. + expect( + compileTaskIntegrationToolApprovals({ + serverNames: ['roomote', 'roomote_manage', 'linear'], + policies: [ + policy('roomote_manage', 'tasks', 'ask'), + policy('roomote_manage', 'status', 'reject'), + policy('linear', 'save_issue', 'ask'), + ], + sessionOverrides: [], + }), + ).toEqual({ + permission: { linear_save_issue: 'ask' }, + tools: { + linear_save_issue: { integrationId: 'linear', toolName: 'save_issue' }, + }, + }); + }); }); diff --git a/packages/types/src/integration-tool-approvals.ts b/packages/types/src/integration-tool-approvals.ts index f6de96782d..c3e153fdd0 100644 --- a/packages/types/src/integration-tool-approvals.ts +++ b/packages/types/src/integration-tool-approvals.ts @@ -290,6 +290,16 @@ export function compileTaskIntegrationToolApprovals(input: { ); const result: TaskIntegrationToolApprovals = { permission: {}, tools: {} }; const ambiguous = new Set(); + // A native key cannot name which server half it came from, so a key an + // internal server's tool could flatten to (for example a custom + // `roomote_manage` server's `tasks` tool colliding with `roomote` / + // `manage_tasks`) must not gate anything: enforcing it would also hold + // the internal tool, and internal MCPs are exempt. + const internalPrefixes = input.serverNames + .filter(isInternalMcpServer) + .map((name) => openCodeMcpToolKey(name, '')); + const isSharedWithInternal = (key: string) => + internalPrefixes.some((prefix) => key.startsWith(prefix)); for (const { integrationId, toolName } of [ ...input.policies, ...input.sessionOverrides, @@ -312,6 +322,7 @@ export function compileTaskIntegrationToolApprovals(input: { : undefined; if (!action) continue; const key = openCodeMcpToolKey(integrationId, toolName); + if (isSharedWithInternal(key)) continue; const known = result.tools[key]; if ( known && From 478cc9f42455b2c27a230de77389bae53b464da6 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:43:02 +0000 Subject: [PATCH 3/4] [Fix] Reserve internal-prefixed custom MCP names so colliding tools stay approvable --- .../__tests__/personal-policies.test.ts | 3 ++- .../src/__tests__/custom-mcp-servers.test.ts | 23 +++++++++++++++++++ packages/types/src/custom-mcp-servers.ts | 22 ++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/apps/web/src/trpc/commands/integration-tool-policies/__tests__/personal-policies.test.ts b/apps/web/src/trpc/commands/integration-tool-policies/__tests__/personal-policies.test.ts index 06d6633e44..b53e12ec3d 100644 --- a/apps/web/src/trpc/commands/integration-tool-policies/__tests__/personal-policies.test.ts +++ b/apps/web/src/trpc/commands/integration-tool-policies/__tests__/personal-policies.test.ts @@ -45,7 +45,8 @@ vi.mock('@roomote/db/server', () => ({ upsertIntegrationToolUserPolicy: vi.fn(), })); -vi.mock('@roomote/types', () => ({ +vi.mock('@roomote/types', async (importOriginal) => ({ + ...(await importOriginal()), getMcpIntegration: mockGetMcpIntegration, })); diff --git a/packages/types/src/__tests__/custom-mcp-servers.test.ts b/packages/types/src/__tests__/custom-mcp-servers.test.ts index 693df80324..2c7c71e181 100644 --- a/packages/types/src/__tests__/custom-mcp-servers.test.ts +++ b/packages/types/src/__tests__/custom-mcp-servers.test.ts @@ -42,6 +42,29 @@ describe('isInternalMcpServer', () => { MCP_INTEGRATIONS.some((integration) => integration.id.startsWith('_')), ).toBe(false); }); + + it('rejects custom names that could collide with internal tool keys', () => { + // OpenCode flattens tools to `_`, so `gbrain_get` / `page` + // would share `gbrain_get_page` with the Brain's own `get_page`. + for (const name of ['gbrain_get', 'roomote_manage']) { + const result = customMcpServerInputSchema.safeParse({ + ...validServer, + name, + }); + expect(result.success).toBe(false); + } + // Merely similar names that cannot collide stay valid. + expect( + customMcpServerInputSchema.safeParse({ + ...validServer, + name: 'gbrain-reports', + }).success, + ).toBe(true); + expect( + customMcpServerInputSchema.safeParse({ ...validServer, name: 'linear_x' }) + .success, + ).toBe(true); + }); }); describe('customMcpServerInputSchema', () => { diff --git a/packages/types/src/custom-mcp-servers.ts b/packages/types/src/custom-mcp-servers.ts index ce347a43fd..8022a8b1f7 100644 --- a/packages/types/src/custom-mcp-servers.ts +++ b/packages/types/src/custom-mcp-servers.ts @@ -59,6 +59,20 @@ export function isInternalMcpServer(serverId: string): boolean { return INTERNAL_MCP_SERVER_IDS.has(serverId); } +/** + * OpenCode flattens every MCP tool to `_`, so a custom server + * whose name starts with an internal server's name plus an underscore can + * produce the same native permission key as an internal tool (for example + * `gbrain_get` / `page` collides with `gbrain` / `get_page`). Native rules + * cannot tell the two apart, which would either gate the exempt internal + * tool or leave the external tool's approval unanswerable, so these names + * are rejected at save time. `_roomote_http_integrations` needs no prefix + * guard: custom names cannot start with an underscore at all. + */ +export function isInternalMcpServerNamePrefix(name: string): boolean { + return [...INTERNAL_MCP_SERVER_IDS].some((id) => name.startsWith(`${id}_`)); +} + export const RESERVED_CUSTOM_MCP_SERVER_NAMES: ReadonlySet = new Set([ ...INTERNAL_MCP_SERVER_IDS, 'github', @@ -223,6 +237,14 @@ export const customMcpServerNameSchema = z (name) => ({ message: `'${name}' is reserved by a built-in ${PRODUCT_NAME} integration.`, }), + ) + .refine( + (name) => !isInternalMcpServerNamePrefix(name), + (name) => ({ + message: + `'${name}' starts with a Roomote-internal MCP server name; agent tool ` + + `permissions could not tell its tools apart from the internal server's.`, + }), ); export const customMcpServerHeadersSchema = z From a697dcd8da317827e0a1406ba69648b1e8f2563e Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:04:57 +0000 Subject: [PATCH 4/4] [Fix] Enforce the internal-name-prefix guard on the Fast remote MCP creation path --- .../server/lib/mcp/add-remote-custom-mcp.test.ts | 14 ++++++++++++++ .../src/server/lib/mcp/add-remote-custom-mcp.ts | 12 +++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.test.ts b/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.test.ts index b8ee399bf2..ecfb9cb0ba 100644 --- a/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.test.ts +++ b/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.test.ts @@ -177,6 +177,20 @@ describe('addRemoteCustomMcpForFast', () => { expect(await db.query.customMcpServers.findMany()).toEqual([]); }); + it('rejects a name starting with an internal server prefix', async () => { + await expect( + addRemoteCustomMcpForFast({ + userId: adminId, + sessionId: crypto.randomUUID(), + name: 'gbrain_get', + url: 'https://mcp.example.com/mcp', + }), + ).rejects.toThrow('starts with a Roomote-internal MCP server name'); + + expect(guardedFetchMock).not.toHaveBeenCalled(); + expect(await db.query.customMcpServers.findMany()).toEqual([]); + }); + describe('for any member, like integration keys', () => { it('shares a server with everyone by default and lets its creator authorize it', async () => { guardedFetchMock.mockImplementation( diff --git a/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.ts b/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.ts index 5f3822b633..1785be18ef 100644 --- a/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.ts +++ b/packages/sdk/src/server/lib/mcp/add-remote-custom-mcp.ts @@ -18,6 +18,7 @@ import { PRODUCT_NAME, customMcpConnectionId, customMcpRemoteServerInputSchema, + isInternalMcpServerNamePrefix, parseMcpJsonRpcPayload, type OAuthClientInformation, type CustomMcpServerVisibility, @@ -796,9 +797,18 @@ export async function addRemoteCustomMcpForFast(input: { const actor = { userId: input.userId, isAdmin: user.role === 'admin' }; const visibility = input.visibility ?? DEFAULT_CUSTOM_MCP_SERVER_VISIBILITY; + const normalizedName = normalizeFastRemoteMcpName(input.name); + // Same guard as the web create path: a name starting with an internal + // server's prefix can collide with internal tools in native permissions, + // which the mount-time exclusion would then silently drop. + if (isInternalMcpServerNamePrefix(normalizedName)) { + throw new Error( + `'${normalizedName}' starts with a Roomote-internal MCP server name; rename it so its tools cannot collide with the internal server's.`, + ); + } const parsed = customMcpRemoteServerInputSchema.parse({ transport: 'remote', - name: normalizeFastRemoteMcpName(input.name), + name: normalizedName, url: input.url, authType: 'none', });