fix(mcp): restrict MCP server destination changes to admins (#8629)

* fix(mcp): restrict MCP server destination changes to admins

* fix(mcp): compare exact MCP paths and guard concurrent URL changes

* fix(mcp): treat setting a URL on a URL-less server as a destination change

* fix(mcp): guard re-registration against concurrent URL changes

* fix(mcp): narrow re-registration URL before the guarded update

* chore(mcp): use absolute import in utils test

Author: TheodoreSpeaks

apps/sim/app/api/mcp/servers/[id]/route.ts

@@ -30,7 +30,7 @@ export const PATCH = withRouteHandler(
   )(
     async (
       request: NextRequest,
-      { userId, userName, userEmail, workspaceId, requestId },
+      { userId, userName, userEmail, workspaceId, requestId, permission },
       { params }
     ) => {
       try {
@@ -59,6 +59,7 @@ export const PATCH = withRouteHandler(
           actorName: userName,
           actorEmail: userEmail,
           serverId,
+          allowDestinationChange: permission === 'admin',
           name: body.name,
           description: body.description,
           transport: body.transport,

apps/sim/lib/mcp/application/operations.test.ts

@@ -132,6 +132,7 @@ const EXPECTED_CAPABILITIES: Record<keyof typeof mcpServerOperations, string> =
   register: 'mcp_tools.use',
   update: 'mcp_tools.use',
   reconfigure: 'mcp_tools.use',
+  changeDestination: 'mcp_tools.use',
   delete: 'mcp_tools.use',
   discoverTools: 'mcp_tools.use',
   executeTool: 'mcp_tools.use',

apps/sim/lib/mcp/application/operations.ts

@@ -209,6 +209,15 @@ export const mcpServerOperations = {
     capability: 'mcp_tools.use',
     ...ALL_PRINCIPAL_POLICY,
   }),
+  /** Pointing a server at another host or path; deployed workflows pin servers by id. */
+  changeDestination: defineWorkspaceOperation({
+    id: 'mcp_servers.change_destination',
+    oauthScope: 'api:write',
+    minimumRole: 'admin',
+    workspaceApiKey: 'deny',
+    capability: 'mcp_tools.use',
+    ...HUMAN_PRINCIPAL_POLICY,
+  }),
   delete: defineWorkspaceOperation({
     id: 'mcp_servers.delete',
     oauthScope: 'api:write',

apps/sim/lib/mcp/application/use-cases.test.ts

@@ -12,12 +12,14 @@ import {
   workspaceUploadsMockFns,
 } from '@sim/testing/mocks/workspace-uploads.mock'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { InsufficientWorkspacePermissionsError } from '@/lib/core/application'
 
 const { events, hoisted } = vi.hoisted(() => ({
   events: [] as string[],
   hoisted: {
     idState: vi.fn(),
     create: vi.fn(),
+    update: vi.fn(),
     effects: vi.fn(),
     getServer: vi.fn(),
     listServers: vi.fn(),
@@ -31,7 +33,7 @@ vi.mock('@/lib/mcp/orchestration', () => ({
   applyMcpServerMutationEffects: hoisted.effects,
   createMcpServer: hoisted.create,
   deleteMcpServer: vi.fn(),
-  updateMcpServer: vi.fn(),
+  updateMcpServer: hoisted.update,
 }))
 vi.mock('@/lib/mcp/queries', () => ({
   getMcpServerIdState: hoisted.idState,
@@ -45,6 +47,7 @@ import {
   discoverMcpServerToolsUseCase,
   discoverMcpToolsUseCase,
   getMcpServerUseCase,
+  reconfigureMcpServerUseCase,
 } from '@/lib/mcp/application/use-cases'
 
 const mocks = {
@@ -109,6 +112,46 @@ describe('MCP server application use cases', () => {
     mocks.discoverServerTools.mockResolvedValue([])
   })
 
+  it('refuses a writer pointing a server at a different host before writing', async () => {
+    await expect(
+      reconfigureMcpServerUseCase.execute({
+        principal: { kind: 'session', userId: 'user-1' },
+        input: {
+          workspaceId: workspace.workspaceId,
+          serverId: server.id,
+          url: 'https://other-host.example.com/mcp',
+        },
+      })
+    ).rejects.toBeInstanceOf(InsufficientWorkspacePermissionsError)
+  })
+
+  it('lets a writer change only the query string, and an admin change the host', async () => {
+    mocks.update.mockResolvedValue({ success: true, server, configurationChanged: true })
+
+    await expect(
+      reconfigureMcpServerUseCase.execute({
+        principal: { kind: 'session', userId: 'user-1' },
+        input: {
+          workspaceId: workspace.workspaceId,
+          serverId: server.id,
+          url: `${server.url}?token=rotated`,
+        },
+      })
+    ).resolves.toMatchObject({ server: { id: server.id } })
+
+    mocks.resolvePermission.mockResolvedValue('admin')
+    await expect(
+      reconfigureMcpServerUseCase.execute({
+        principal: { kind: 'session', userId: 'user-1' },
+        input: {
+          workspaceId: workspace.workspaceId,
+          serverId: server.id,
+          url: 'https://new.example.com/mcp',
+        },
+      })
+    ).resolves.toMatchObject({ server: { id: server.id } })
+  })
+
   it('resolves a selected organization server through canonical scope and current permissions', async () => {
     mocks.loadContext.mockResolvedValue({ ...workspace, workspaceOrganizationId: 'org-1' })
     const args = {

apps/sim/lib/mcp/application/use-cases.ts

@@ -2,7 +2,11 @@ import { AuditAction, AuditResourceType } from '@sim/audit'
 import { resolvePrincipalAttribution } from '@sim/auth/principal'
 import { getPostgresErrorCode } from '@sim/utils/errors'
 import type { CursorKey, ListSortOrder } from '@/lib/api/list-query'
-import { defineAuthorizedWorkspaceUseCase, ForbiddenOperationError } from '@/lib/core/application'
+import {
+  authorizeWorkspaceOperation,
+  defineAuthorizedWorkspaceUseCase,
+  ForbiddenOperationError,
+} from '@/lib/core/application'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { sanitizeUrlForLog } from '@/lib/core/utils/logging'
 import {
@@ -37,7 +41,7 @@ import {
 import { mcpService } from '@/lib/mcp/service'
 import { compileMcpToolSchema } from '@/lib/mcp/tool-schema'
 import type { McpAuthType } from '@/lib/mcp/types'
-import { generateMcpServerId } from '@/lib/mcp/utils'
+import { generateMcpServerId, isSameMcpServerDestination } from '@/lib/mcp/utils'
 
 type McpServerTransport = McpServerRow['transport']
 type McpWriteSource = 'api' | 'settings' | 'tool_input'
@@ -386,13 +390,26 @@ async function updateMcpServer(args: {
       'This MCP server is managed from its Credential Group settings'
     )
   }
+  const changesDestination =
+    args.input.url !== undefined &&
+    (!args.context.server.url ||
+      !isSameMcpServerDestination(args.context.server.url, args.input.url))
+  if (changesDestination) {
+    await authorizeWorkspaceOperation(
+      args.principal,
+      mcpServerOperations.changeDestination,
+      args.context,
+      authorizationOptions
+    )
+  }
   const attribution = resolvePrincipalAttribution(args.principal, {
     workspaceBillingOwnerUserId: args.context.billedAccountUserId,
   })
   const result = await updateMcpServerRecord({
     workspaceId: args.context.workspaceId,
     userId: attribution.attributedUserId,
     serverId: args.context.server.id,
+    allowDestinationChange: changesDestination,
     name: args.input.name,
     description: args.input.description,
     transport: args.input.transport,