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 7ab8903d20..33667cdb2e 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 @@ -157,6 +157,23 @@ describe('resolveProxyToolApprovalBlocks', () => { expect(mockAutoState).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.blocks.size).toBe(0); + expect(blocks.defaultBlock).toBeUndefined(); + expect(blocks.shadowDefaultTools).toBe(false); + expect(mockDeployment).not.toHaveBeenCalled(); + expect(mockUser).not.toHaveBeenCalled(); + expect(mockAutoState).not.toHaveBeenCalled(); + } + }); + it("applies the task's session overrides to a task run only", async () => { mockSessionForTask.mockResolvedValue({ id: 'session-1' }); mockOverrides.mockResolvedValue([ diff --git a/apps/api/src/handlers/mcp/gbrain.ts b/apps/api/src/handlers/mcp/gbrain.ts index ea5be0b497..1124bbc74d 100644 --- a/apps/api/src/handlers/mcp/gbrain.ts +++ b/apps/api/src/handlers/mcp/gbrain.ts @@ -2,48 +2,18 @@ import { isBrainEmbeddingAvailable, resolveBrainConnection, } from '@roomote/sdk/server'; +import { GBRAIN_READ_TOOL_NAMES } from '@roomote/types'; import { rerankBrainQueryResult } from './gbrain-rerank'; import { createMcpProxy, McpProxyError } from './proxy-utils'; /** - * Read-only tool allowlist over gbrain's MCP surface, which publishes over a - * hundred tools. The list is filtered on `tools/list` as well as on calls, so - * an agent sees only these and never has to choose against the rest. - * - * `remember` and `forget` are deliberately absent: the agent path is - * structurally incapable of mutation, and memory writes flow only through - * the server-side ingestion pipeline with its own write-only credential. - * - * Deliberately absent for a second reason, that nothing here populates what - * they read: - * - `recall` leads with hot-memory facts saved via `remember`, which this - * deployment never writes. Its page arm duplicates `search`, so exposing it - * only offers a worse `search` with a permanently empty half. - * - `context_pack` and `delta` serve long-lived agents with standing entities - * and heartbeats. Roomote's agents are per-task and start cold. - * - * Keep this list in sync with the instructions in @roomote/types: a tool - * exposed but unexplained is one the agent picks by gbrain's own description, - * which is written for a different product. + * The agent-facing Brain tool set lives in `@roomote/types` next to + * `BRAIN_MCP_ID`, because the approval-rule compilers need the same list to + * tell which native keys are genuinely the Brain's. Re-exported here for the + * proxy's callers and tests. */ -export const GBRAIN_READ_TOOL_NAMES = [ - // Ask. `query` adds multi-query expansion and is the right default when the - // agent does not know the corpus vocabulary; `search` is the cheaper exact - // -token path with no expansion call. - 'query', - 'search', - // Exact, zero-LLM lookup for canonical person cards populated from Roomote - // member identities. Prefer this over broad search for a known person. - 'entity', - // Reason across pages. Expensive and slow, but bounded in tokens, which is - // the only reason to prefer it over reading pages directly. - 'synthesize', - // Browse: without these an agent can only answer questions it already - // knows to ask, and "what do you know?" looks like an empty Brain. - 'list_pages', - 'get_page', -] as const; +export { GBRAIN_READ_TOOL_NAMES }; /** * Brain proxy: fronts the deployment-hosted gbrain HTTP MCP server diff --git a/apps/api/src/handlers/mcp/tool-approval-enforcement.ts b/apps/api/src/handlers/mcp/tool-approval-enforcement.ts index f3147fd73f..cc81f347cf 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 { integrationToolModeIsAutoAssessed, + isInternalMcpServer, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, type IntegrationToolPolicyMode, @@ -73,6 +74,12 @@ export async function resolveProxyToolApprovalBlocks(input: { if (!(await isDeploymentExperimentEnabled('integrationToolApprovals'))) { return result; } + // Roomote's own MCP and other internal servers are outside approval + // control entirely; their tools always pass, with no Auto default or + // shadow assessment either. + if (isInternalMcpServer(input.integrationId)) { + return result; + } const autoState = await resolveIntegrationToolAutoState(); result.shadowDefaultTools = autoState.mode === 'shadow'; if (autoState.mode === 'on' && input.tokenType === 'run') { 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 4d9661a480..8688bdc80a 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'; @@ -252,7 +253,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 showLegacyAvailability = !experiment.enabled && canManage && 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 87c02e3aa0..0e9c4f438e 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/apps/web/src/trpc/commands/integration-tool-policies/index.ts b/apps/web/src/trpc/commands/integration-tool-policies/index.ts index 7b91f17c52..53ead93c18 100644 --- a/apps/web/src/trpc/commands/integration-tool-policies/index.ts +++ b/apps/web/src/trpc/commands/integration-tool-policies/index.ts @@ -20,6 +20,7 @@ import { import { resolveDecisionModel } from '@roomote/cloud-agents/server/typesafe-judgment'; import { getMcpIntegration, + isInternalMcpServer, type IntegrationToolAutoSettings, type IntegrationToolPoliciesUpsert, type IntegrationToolPolicyMode, @@ -30,6 +31,20 @@ 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 @@ -39,7 +54,9 @@ export async function listIntegrationToolPoliciesCommand( auth: UserAuthSuccess, ) { assertAdmin(auth); - return listIntegrationToolPolicies(); + return (await listIntegrationToolPolicies()).filter( + (policy) => !isInternalMcpServer(policy.integrationId), + ); } export async function setIntegrationToolPolicyCommand( @@ -47,12 +64,13 @@ export async function setIntegrationToolPolicyCommand( input: IntegrationToolPolicyUpsert, ) { assertAdmin(auth); + assertApprovalManagedIntegrationId(input.integrationId); await upsertIntegrationToolPolicy({ ...input, updatedByUserId: auth.userId, }); await syncLegacyDisabledTool({ ...input, scope: 'deployment' }); - return listIntegrationToolPolicies(); + return listIntegrationToolPoliciesCommand(auth); } export async function setIntegrationToolPoliciesCommand( @@ -60,12 +78,13 @@ export async function setIntegrationToolPoliciesCommand( input: IntegrationToolPoliciesUpsert, ) { assertAdmin(auth); + assertApprovalManagedIntegrationId(input.integrationId); await upsertIntegrationToolPolicies({ ...input, updatedByUserId: auth.userId, }); await syncLegacyDisabledTools({ ...input, scope: 'deployment' }); - return listIntegrationToolPolicies(); + return listIntegrationToolPoliciesCommand(auth); } const toolApprovalsEnabled = () => @@ -175,7 +194,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( @@ -188,13 +209,14 @@ export async function setPersonalIntegrationToolPolicyCommand( message: 'Tool approvals are not enabled.', }); } + assertApprovalManagedIntegrationId(input.integrationId); await upsertIntegrationToolUserPolicy({ ...input, userId: auth.userId }); await syncLegacyDisabledTool({ ...input, scope: 'personal', userId: auth.userId, }); - return listIntegrationToolUserPolicies(auth.userId); + return listPersonalIntegrationToolPoliciesCommand(auth); } export async function setPersonalIntegrationToolPoliciesCommand( @@ -207,13 +229,14 @@ export async function setPersonalIntegrationToolPoliciesCommand( message: 'Tool approvals are not enabled.', }); } + assertApprovalManagedIntegrationId(input.integrationId); await upsertIntegrationToolUserPolicies({ ...input, userId: auth.userId }); await syncLegacyDisabledTools({ ...input, scope: 'personal', userId: auth.userId, }); - return listIntegrationToolUserPolicies(auth.userId); + return listPersonalIntegrationToolPoliciesCommand(auth); } /** diff --git a/apps/web/src/trpc/routers/_app.ts b/apps/web/src/trpc/routers/_app.ts index 82dadaf128..0841129e8b 100644 --- a/apps/web/src/trpc/routers/_app.ts +++ b/apps/web/src/trpc/routers/_app.ts @@ -26,6 +26,7 @@ import { isSetupModelProviderId, JUDGMENT_MODEL_SELECTIONS, isOpenAiCompatibleProviderId, + customMcpServerCreateInputSchema, customMcpServerInputSchema, customMcpServerVisibilitySchema, isOpenAiRealtimeVoiceId, @@ -1979,7 +1980,7 @@ export const appRouter = createRouter({ create: protectedProcedure .input( - customMcpServerInputSchema.and( + customMcpServerCreateInputSchema.and( z.object({ visibility: customMcpServerVisibilitySchema.optional() }), ), ) 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 a04b032d70..5dcd5cb7e8 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 @@ -105,6 +105,75 @@ 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('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 841a0ead79..9b04299293 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 { integrationToolModeIsAutoAssessed, integrationToolPolicyKey, + isInternalMcpServer, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, type IntegrationToolApprovalMetadata, @@ -133,7 +134,20 @@ 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. + if (isInternalMcpServer(tool.integrationId)) continue; const key = integrationToolPolicyKey(tool.integrationId, tool.toolName); const policyMode = modeByTool.get(key); const mode = resolveEffectiveIntegrationToolMode({ @@ -161,6 +175,9 @@ export function buildIntegrationToolApprovalRules( if (autoAssessed) options.autoToolKeys?.add(key); } } + for (const key of internalKeys) { + actionByKey.delete(key); + } const rules: PermissionRuleset = [...actionByKey].map( ([permission, action]) => ({ permission, pattern: '*', action }), ); 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 fa735bb964..9a9a6e2545 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 @@ -140,6 +140,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 a manual Ask first ask on the task's Session for its owner", async () => { mocks.deploymentPolicies.mockResolvedValue([policy('save_issue', 'ask')]); await expect(requestTaskToolApproval(ask)).resolves.toEqual({ 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', }); diff --git a/packages/sdk/src/server/lib/task-tool-approvals.ts b/packages/sdk/src/server/lib/task-tool-approvals.ts index e4b44ee6b7..2022c239b9 100644 --- a/packages/sdk/src/server/lib/task-tool-approvals.ts +++ b/packages/sdk/src/server/lib/task-tool-approvals.ts @@ -19,8 +19,11 @@ import { resolveIntegrationToolAutoState, } from '@roomote/cloud-agents/server/integration-tool-auto-evaluation'; import { + BRAIN_MCP_ID, compileTaskIntegrationToolApprovals, + GBRAIN_READ_TOOL_NAMES, integrationToolModeIsAutoAssessed, + isInternalMcpServer, resolveEffectiveIntegrationToolMode, resolveGoverningIntegrationToolPolicies, type IntegrationToolApprovalStatus, @@ -109,6 +112,11 @@ export async function resolveTaskIntegrationToolApprovals(input: { policies, sessionOverrides, autoOn: autoState.mode === 'on', + // The Brain's agent-facing tool set is a static allowlist, so the + // compiler can drop exactly the native keys that are genuinely the + // Brain's. `_roomote_http_integrations` needs no list: its flattened + // keys start with `_`, which no governable server name can produce. + internalToolNames: { [BRAIN_MCP_ID]: GBRAIN_READ_TOOL_NAMES }, }); } @@ -137,6 +145,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/sdk/src/server/routers/mcp-connections.test.ts b/packages/sdk/src/server/routers/mcp-connections.test.ts index 75d7bd958f..f6a1896f58 100644 --- a/packages/sdk/src/server/routers/mcp-connections.test.ts +++ b/packages/sdk/src/server/routers/mcp-connections.test.ts @@ -458,6 +458,35 @@ describe('mcpConnectionsRouter.getMcpServerConfigs', () => { expect(connected['intercom']?.url).toContain('/api/mcp/custom/'); }); + it('never mounts a legacy custom server whose name starts with an internal server name', async () => { + mockFindCustomServers.mockResolvedValue([ + { + id: '22222222-2222-4222-8222-222222222222', + name: 'gbrain_get', + url: 'https://shared.example.com/mcp', + stdio: null, + authType: 'none', + updatedAt: new Date('2026-09-16T00:00:00.000Z'), + }, + { + id: '33333333-3333-4333-8333-333333333333', + name: 'intercom', + url: 'https://shared.example.com/mcp', + stdio: null, + authType: 'none', + updatedAt: new Date('2026-09-16T00:00:00.000Z'), + }, + ]); + + const result = await resolveUserMcpServerConfigs({ + userId: 'user-1', + apiBaseUrl: 'https://api.preview.roomote.run', + }); + + expect(result['gbrain_get']).toBeUndefined(); + expect(result['intercom']?.url).toContain('/api/mcp/custom/'); + }); + it('routes the seeded development fixture to the local inert adapter', async () => { mockFindCustomServers.mockResolvedValue([ { diff --git a/packages/sdk/src/server/routers/mcp-connections.ts b/packages/sdk/src/server/routers/mcp-connections.ts index 63a1da1170..6a00b2b296 100644 --- a/packages/sdk/src/server/routers/mcp-connections.ts +++ b/packages/sdk/src/server/routers/mcp-connections.ts @@ -44,6 +44,7 @@ import { isMcpConnectionVercelConfig, isMcpConnectionXConfig, isDeploymentScopedMcpIntegration, + isInternalMcpServerNamePrefix, BRAIN_MCP_ID, BRAIN_PROXY_PATH, CUSTOM_MCP_PROXY_PATH_PREFIX, @@ -413,6 +414,14 @@ export const mcpConnectionsRouter = router({ continue; } + // Same legacy-name exclusion as the remote path above. + if (isInternalMcpServerNamePrefix(row.name)) { + console.warn( + `[getCustomStdioMcpServers] Skipping custom server '${row.name}': name starts with a Roomote-internal MCP server name, so its tools cannot be told apart from the internal server's`, + ); + continue; + } + servers[row.name] = { command: row.stdio.command, ...(row.stdio.args ? { args: row.stdio.args } : {}), @@ -452,6 +461,18 @@ async function buildScopedCustomMcpServerConfigs( continue; } + // Legacy rows whose names collide with an internal server's native tool + // keys (rejected for new servers at create time) are never mounted: + // their flattened keys could gate an exempt internal tool, or leave + // their own `ask` policy permanently unapprovable. The row stays + // manageable in Settings; it just never reaches a task. + if (isInternalMcpServerNamePrefix(row.name)) { + logInfo( + `[getMcpServerConfigs] Skipping custom server '${row.name}': name starts with a Roomote-internal MCP server name, so its tools cannot be told apart from the internal server's`, + ); + continue; + } + if (row.id === demoSeedDevelopmentIntegration.id) { if (Env.APP_ENV !== 'development') continue; servers[row.name] = { diff --git a/packages/types/src/__tests__/custom-mcp-servers.test.ts b/packages/types/src/__tests__/custom-mcp-servers.test.ts index a974eda55b..0cde18e642 100644 --- a/packages/types/src/__tests__/custom-mcp-servers.test.ts +++ b/packages/types/src/__tests__/custom-mcp-servers.test.ts @@ -1,11 +1,16 @@ import { describe, expect, it } from 'vitest'; import { + CUSTOM_MCP_SERVER_NAME_PATTERN, + HTTP_INTEGRATIONS_MCP_ID, RESERVED_CUSTOM_MCP_SERVER_NAMES, + customMcpServerCreateInputSchema, customMcpServerInputSchema, + isInternalMcpServer, validateCustomMcpHeaderName, validateCustomMcpServerUrl, } from '../custom-mcp-servers'; +import { MCP_INTEGRATIONS } from '../mcp-oauth'; const validServer = { transport: 'remote' as const, @@ -15,6 +20,68 @@ 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); + }); + + it('keeps the HTTP integrations broker structurally collision-free', () => { + // Its flattened native keys start with `_`, and no governable server + // name can produce a key like that: custom names reject a leading + // underscore and every catalog id starts alphanumeric. + expect(HTTP_INTEGRATIONS_MCP_ID.startsWith('_')).toBe(true); + expect(CUSTOM_MCP_SERVER_NAME_PATTERN.test('_anything')).toBe(false); + expect( + MCP_INTEGRATIONS.some((integration) => integration.id.startsWith('_')), + ).toBe(false); + }); + + it('rejects new 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 = customMcpServerCreateInputSchema.safeParse({ + ...validServer, + name, + }); + expect(result.success).toBe(false); + } + // Merely similar names that cannot collide stay valid. + expect( + customMcpServerCreateInputSchema.safeParse({ + ...validServer, + name: 'gbrain-reports', + }).success, + ).toBe(true); + expect( + customMcpServerCreateInputSchema.safeParse({ + ...validServer, + name: 'linear_x', + }).success, + ).toBe(true); + }); + + it('lets the update path keep a legacy internal-prefixed name', () => { + // Names are immutable, so an update always carries the existing name; + // rejecting it would lock the row out of every other edit. Legacy rows + // are excluded at mount time instead. + expect( + customMcpServerInputSchema.safeParse({ + ...validServer, + name: 'gbrain_get', + }).success, + ).toBe(true); + }); +}); + 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 37ad3474f3..6beffc61c7 100644 --- a/packages/types/src/__tests__/integration-tool-policy-strictness.test.ts +++ b/packages/types/src/__tests__/integration-tool-policy-strictness.test.ts @@ -71,12 +71,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 71264a871e..a73f74ed7e 100644 --- a/packages/types/src/__tests__/task-integration-tool-approvals.test.ts +++ b/packages/types/src/__tests__/task-integration-tool-approvals.test.ts @@ -102,4 +102,98 @@ describe('compileTaskIntegrationToolApprovals', () => { autoServers: [], }); }); + + 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' }, + }, + autoServers: [], + }); + }); + + it('drops only rules whose native key an actual internal tool flattens to', () => { + // A custom `gbrain_get` server's `page` tool flattens to + // `gbrain_get_page`, the same native key as the Brain's own `get_page`. + // Gating it would also hold the exempt Brain tool, so that one rule is + // dropped — while `gbrain_reports`/`status`, which no Brain tool + // flattens to, keeps its rule. + expect( + compileTaskIntegrationToolApprovals({ + serverNames: ['gbrain', 'gbrain_get', 'gbrain_reports', 'linear'], + policies: [ + policy('gbrain_get', 'page', 'ask'), + policy('gbrain_reports', 'status', 'reject'), + policy('linear', 'save_issue', 'ask'), + ], + sessionOverrides: [], + internalToolNames: { + gbrain: [ + 'query', + 'search', + 'entity', + 'synthesize', + 'list_pages', + 'get_page', + ], + }, + }), + ).toEqual({ + permission: { + gbrain_reports_status: 'deny', + linear_save_issue: 'ask', + }, + tools: { + gbrain_reports_status: { + integrationId: 'gbrain_reports', + toolName: 'status', + }, + linear_save_issue: { integrationId: 'linear', toolName: 'save_issue' }, + }, + autoServers: [], + }); + }); + + it('drops nothing for an internal server whose tool names are unknown', () => { + expect( + compileTaskIntegrationToolApprovals({ + serverNames: ['gbrain', 'gbrain_get'], + policies: [policy('gbrain_get', 'page', 'ask')], + sessionOverrides: [], + }), + ).toEqual({ + permission: { gbrain_get_page: 'ask' }, + tools: { + gbrain_get_page: { integrationId: 'gbrain_get', toolName: 'page' }, + }, + autoServers: [], + }); + }); + + it('never wildcard-gates internal servers while Auto is on', () => { + expect( + compileTaskIntegrationToolApprovals({ + serverNames: ['gbrain', 'linear'], + policies: [], + sessionOverrides: [], + autoOn: true, + }), + ).toEqual({ + permission: { 'linear_*': 'ask' }, + tools: {}, + autoServers: ['linear'], + }); + }); }); diff --git a/packages/types/src/brain.ts b/packages/types/src/brain.ts index f0bd8c1026..32b87ce7eb 100644 --- a/packages/types/src/brain.ts +++ b/packages/types/src/brain.ts @@ -15,6 +15,47 @@ import { z } from 'zod'; export const BRAIN_MCP_ID = 'gbrain'; +/** + * Read-only tool allowlist over gbrain's MCP surface, which publishes over a + * hundred tools. The API proxy filters `tools/list` and calls to exactly + * these, so this is the complete agent-facing tool set — the same list the + * approval-rule compilers use to tell which native keys are genuinely the + * Brain's. + * + * `remember` and `forget` are deliberately absent: the agent path is + * structurally incapable of mutation, and memory writes flow only through + * the server-side ingestion pipeline with its own write-only credential. + * + * Deliberately absent for a second reason, that nothing here populates what + * they read: + * - `recall` leads with hot-memory facts saved via `remember`, which this + * deployment never writes. Its page arm duplicates `search`, so exposing it + * only offers a worse `search` with a permanently empty half. + * - `context_pack` and `delta` serve long-lived agents with standing entities + * and heartbeats. Roomote's agents are per-task and start cold. + * + * Keep this list in sync with the instructions below: a tool exposed but + * unexplained is one the agent picks by gbrain's own description, which is + * written for a different product. + */ +export const GBRAIN_READ_TOOL_NAMES = [ + // Ask. `query` adds multi-query expansion and is the right default when the + // agent does not know the corpus vocabulary; `search` is the cheaper exact + // -token path with no expansion call. + 'query', + 'search', + // Exact, zero-LLM lookup for canonical person cards populated from Roomote + // member identities. Prefer this over broad search for a known person. + 'entity', + // Reason across pages. Expensive and slow, but bounded in tokens, which is + // the only reason to prefer it over reading pages directly. + 'synthesize', + // Browse: without these an agent can only answer questions it already + // knows to ask, and "what do you know?" looks like an empty Brain. + 'list_pages', + 'get_page', +] as const; + export const BRAIN_MCP_DISCLOSURE_INSTRUCTIONS = `When specific information returned by a Brain memory retrieval materially informs your answer or work, naturally tell the user which remembered fact you retrieved and how you used it. Describe the memory in human terms, keep the disclosure incidental, and do not turn the response into tool narration. Do not mention retrieval that did not inform the outcome. Never expose internal memory IDs, page slugs, storage paths, raw metadata, source fields, or other internal provenance.`; /** API proxy mount; shared by SDK config delivery and the worker. */ diff --git a/packages/types/src/custom-mcp-servers.ts b/packages/types/src/custom-mcp-servers.ts index 3ca8b109d2..933fab4254 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,45 @@ 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); +} + +/** + * 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', '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()), ]); @@ -375,3 +407,25 @@ export type CustomMcpStdioServerInput = z.infer< typeof customMcpStdioServerInputSchema >; export type CustomMcpServerInput = z.infer; + +/** + * Create-only name rule, on top of the base input schema: new servers may + * not take a name that could collide with an internal server's native tool + * keys (`isInternalMcpServerNamePrefix`). Updates deliberately use the base + * schema instead: names are immutable, so an update always carries the + * existing name, and rejecting it would lock a legacy row out of every + * other edit. Legacy rows are kept out of tasks by the mount-time exclusion + * in the MCP config resolver, not by blocking edits. + */ +export const customMcpServerCreateInputSchema = + customMcpServerInputSchema.superRefine((server, ctx) => { + if (isInternalMcpServerNamePrefix(server.name)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['name'], + message: + `'${server.name}' starts with a Roomote-internal MCP server name; agent tool ` + + `permissions could not tell its tools apart from the internal server's.`, + }); + } + }); diff --git a/packages/types/src/integration-tool-approvals.ts b/packages/types/src/integration-tool-approvals.ts index 69034a630f..6db88e3205 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. @@ -236,6 +238,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, @@ -322,6 +329,13 @@ function openCodeMcpToolKey(serverName: string, toolName: string): string { * then answered without a card, exactly as in a Session. Two tools that * flatten to one native key cannot be told apart by a native rule, so that * key is denied rather than asked about under the wrong tool's name. + * + * `internalToolNames` names the agent-facing tools of each mounted internal + * MCP server (for example the Brain's read-only allowlist). A native key + * cannot name which server half it came from, so a key an internal tool + * flattens to must not gate anything: enforcing it would also hold the + * exempt internal tool. Only exact keys are dropped — a custom server whose + * id merely starts with an internal server's name keeps its rules. */ export function compileTaskIntegrationToolApprovals(input: { serverNames: string[]; @@ -329,6 +343,7 @@ export function compileTaskIntegrationToolApprovals(input: { sessionOverrides: IntegrationToolSessionOverrideMetadata[]; /** Auto mode on: every default tool asks natively, `_*`. */ autoOn?: boolean; + internalToolNames?: Record; }): TaskIntegrationToolApprovals { const mounted = new Set(input.serverNames); const policyModes = new Map( @@ -350,17 +365,32 @@ export function compileTaskIntegrationToolApprovals(input: { }; if (input.autoOn) { // Wildcards first: a tool's own rule below wins over its server's. + // Internal MCPs are exempt from approval control, so they get no + // wildcard ask either. for (const serverName of input.serverNames) { + if (isInternalMcpServer(serverName)) continue; result.permission[`${openCodeMcpToolKey(serverName, '')}*`] = 'ask'; result.autoServers.push(serverName); } } const ambiguous = new Set(); + const internalKeys = new Set(); + for (const [serverName, toolNames] of Object.entries( + input.internalToolNames ?? {}, + )) { + if (!mounted.has(serverName) || !isInternalMcpServer(serverName)) continue; + for (const toolName of toolNames) { + internalKeys.add(openCodeMcpToolKey(serverName, toolName)); + } + } for (const { integrationId, toolName } of [ ...input.policies, ...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({ @@ -379,6 +409,7 @@ export function compileTaskIntegrationToolApprovals(input: { : undefined; if (!action) continue; const key = openCodeMcpToolKey(integrationId, toolName); + if (internalKeys.has(key)) continue; const known = result.tools[key]; if ( known &&