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

Author: TheodoreSpeaks

apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts

@@ -261,6 +261,29 @@ describe('MCP server lifecycle orchestration', () => {
     expect(mockRevokeOauthTokens).not.toHaveBeenCalled()
   })
 
+  it('refuses a non-admin save when the URL changed after it was checked', async () => {
+    dbChainMockFns.limit.mockResolvedValueOnce([
+      {
+        url: 'https://example.com/mcp',
+        authType: 'headers',
+        headers: {},
+        oauthClientId: null,
+        oauthClientSecret: null,
+      },
+    ])
+    dbChainMockFns.returning.mockResolvedValueOnce([])
+
+    const result = await performUpdateMcpServer({
+      workspaceId: 'workspace-1',
+      userId: 'user-1',
+      serverId: 'server-1',
+      allowDestinationChange: false,
+      url: 'https://example.com/mcp?token=rotated',
+    })
+
+    expect(result).toMatchObject({ success: false, errorCode: 'conflict' })
+  })
+
   it('lets an admin point an existing server at a different host', async () => {
     dbChainMockFns.limit.mockResolvedValueOnce([
       {

apps/sim/lib/mcp/orchestration/server-lifecycle.ts

@@ -435,11 +435,12 @@ export async function updateMcpServer(
 
     if (!currentServer) return { success: false, error: 'Server not found', errorCode: 'not_found' }
 
+    const checkedUrl =
+      !params.allowDestinationChange && params.url !== undefined ? currentServer.url : null
     if (
-      !params.allowDestinationChange &&
+      checkedUrl &&
       params.url !== undefined &&
-      currentServer.url &&
-      !isSameMcpServerDestination(currentServer.url, params.url)
+      !isSameMcpServerDestination(checkedUrl, params.url)
     ) {
       return {
         success: false,
@@ -505,7 +506,8 @@ export async function updateMcpServer(
           and(
             eq(mcpServers.id, params.serverId),
             eq(mcpServers.workspaceId, params.workspaceId),
-            isNull(mcpServers.deletedAt)
+            isNull(mcpServers.deletedAt),
+            checkedUrl ? eq(mcpServers.url, checkedUrl) : undefined
           )
         )
         .returning()
@@ -518,7 +520,15 @@ export async function updateMcpServer(
       return updated
     })
 
-    if (!server) return { success: false, error: 'Server not found', errorCode: 'not_found' }
+    if (!server) {
+      return checkedUrl
+        ? {
+            success: false,
+            error: 'The MCP server URL changed while saving; reload and try again',
+            errorCode: 'conflict',
+          }
+        : { success: false, error: 'Server not found', errorCode: 'not_found' }
+    }
 
     const shouldClearCache =
       urlChanged ||

apps/sim/lib/mcp/utils.test.ts

@@ -10,6 +10,7 @@ import {
   generateManagedMcpConnectionId,
   generateMcpServerId,
   isManagedMcpConnectionId,
+  isSameMcpServerDestination,
   parseMcpToolId,
   parseMcpToolTarget,
 } from './utils'
@@ -49,6 +50,21 @@ describe('generateMcpServerId', () => {
   })
 })
 
+describe('isSameMcpServerDestination', () => {
+  it('ignores only the query string and fragment', () => {
+    const url = 'https://mcp.example.com/mcp'
+    expect(isSameMcpServerDestination(url, `${url}?token=abc#x`)).toBe(true)
+    expect(isSameMcpServerDestination(url, 'https://MCP.example.com/mcp')).toBe(true)
+  })
+
+  it('treats a different host, path case, or trailing slash as a new destination', () => {
+    const url = 'https://mcp.example.com/mcp'
+    expect(isSameMcpServerDestination(url, 'https://other.example.com/mcp')).toBe(false)
+    expect(isSameMcpServerDestination(url, 'https://mcp.example.com/MCP')).toBe(false)
+    expect(isSameMcpServerDestination(url, `${url}/`)).toBe(false)
+  })
+})
+
 describe('categorizeError', () => {
   it.concurrent('returns 401 for McpOauthAuthorizationRequiredError via instanceof', () => {
     const error = new McpOauthAuthorizationRequiredError('mcp-a', 'A')

apps/sim/lib/mcp/utils.ts

@@ -244,11 +244,18 @@ export function generateMcpServerId(workspaceId: string, url: string): string {
 }
 
 /**
- * Whether two URLs name the same MCP server destination — the origin and path
- * its id is derived from. Only the query string and fragment may differ.
+ * Whether two URLs name the same MCP server destination: the same origin and
+ * exact path. Only the query string and fragment may differ — paths can be
+ * case-sensitive, so this is stricter than the id hash.
  */
 export function isSameMcpServerDestination(a: string, b: string): boolean {
-  return normalizeUrlForHashing(a) === normalizeUrlForHashing(b)
+  try {
+    const parsedA = new URL(a)
+    const parsedB = new URL(b)
+    return parsedA.origin === parsedB.origin && parsedA.pathname === parsedB.pathname
+  } catch {
+    return a === b
+  }
 }
 
 /**