fix(mcp): don't leak non-OAuth errors; clearCache covers disabled servers

Author: waleedlatif1

apps/sim/app/api/mcp/oauth/start/route.test.ts

@@ -134,6 +134,23 @@ describe('MCP OAuth start route', () => {
     expect(body.error).toBe('OAuth authorization already in progress for this server')
     expect(mockMcpAuth).not.toHaveBeenCalled()
   })
+
+  it('does not leak non-OAuth internal error details to the client', async () => {
+    mcpOauthMockFns.mockGetOrCreateOauthRow.mockRejectedValueOnce(
+      new Error('connect ECONNREFUSED 10.0.0.5:5432 (internal-db-host)')
+    )
+    const request = new NextRequest(
+      'http://localhost:3000/api/mcp/oauth/start?workspaceId=workspace-1&serverId=server-1'
+    )
+
+    const response = await GET(request)
+    const body = await response.json()
+
+    expect(response.status).toBe(500)
+    expect(body.error).toBe('Failed to start OAuth flow')
+    expect(body.error).not.toContain('ECONNREFUSED')
+    expect(body.error).not.toContain('internal-db-host')
+  })
 })
 
 describe('surfaceOauthError', () => {

apps/sim/app/api/mcp/oauth/start/route.ts

@@ -150,7 +150,11 @@ export const GET = withRouteHandler(
       }
     } catch (error) {
       logger.error('Error starting MCP OAuth flow:', error)
-      return createMcpErrorResponse(toError(error), surfaceOauthError(error), 500)
+      // Only surface OAuth-flow errors verbatim; everything else (DB, decryption,
+      // network) gets a generic message to avoid leaking internal details.
+      const userMessage =
+        error instanceof OAuthError ? surfaceOauthError(error) : 'Failed to start OAuth flow'
+      return createMcpErrorResponse(toError(error), userMessage, 500)
     }
   })
 )

apps/sim/lib/mcp/service.ts

@@ -673,13 +673,17 @@ class McpService {
   async clearCache(workspaceId?: string): Promise<void> {
     try {
       if (workspaceId) {
-        const servers = await this.getWorkspaceServers(workspaceId)
+        // Enumerate without enabled/deletedAt filters so disabled or soft-deleted
+        // servers still get their cache keys dropped (hard-deleted rows are gone
+        // from the table and their keys expire naturally via TTL).
+        const rows = await db
+          .select({ id: mcpServers.id })
+          .from(mcpServers)
+          .where(eq(mcpServers.workspaceId, workspaceId))
         await Promise.allSettled(
-          servers.map((s) => this.cacheAdapter.delete(serverCacheKey(workspaceId, s.id)))
-        )
-        logger.debug(
-          `Cleared MCP tool cache for workspace ${workspaceId} (${servers.length} servers)`
+          rows.map((r) => this.cacheAdapter.delete(serverCacheKey(workspaceId, r.id)))
         )
+        logger.debug(`Cleared MCP tool cache for workspace ${workspaceId} (${rows.length} servers)`)
       } else {
         await this.cacheAdapter.clear()
         logger.debug('Cleared all MCP tool cache')