fix(credential-groups): harden managed MCP lifecycle

Author: TheodoreSpeaks

apps/sim/app/api/credential-groups/enroll/[token]/mcp/[mcpServerId]/route.ts

@@ -11,12 +11,15 @@ import {
   enforceCredentialGroupEnrollmentOAuthRateLimit,
   enforcePublicCredentialGroupOAuthStartIpRateLimit,
 } from '@/lib/credential-groups/rate-limit'
+import { makeTimedStep } from '@/lib/mcp/oauth'
 import { createCredentialGroupEnrollmentRedirect } from '@/app/api/credential-groups/enrollment-redirect'
 
 export const dynamic = 'force-dynamic'
 export const runtime = 'nodejs'
 
 const logger = createLogger('CredentialGroupMcpOAuthStartAPI')
+const timedStep = makeTimedStep(logger)
+const MANAGED_MCP_OAUTH_START_TIMEOUT_MS = 26_000
 
 export const GET = withRouteHandler(
   async (
@@ -41,11 +44,16 @@ export const GET = withRouteHandler(
     }
 
     try {
-      const { authorizationUrl } = await startPublicCredentialGroupMcpOAuth.execute({
-        principal,
-        input: { invitationToken: token, mcpServerId },
-        request,
-      })
+      const { authorizationUrl } = await timedStep(
+        'startPublicCredentialGroupMcpOAuth',
+        MANAGED_MCP_OAUTH_START_TIMEOUT_MS,
+        () =>
+          startPublicCredentialGroupMcpOAuth.execute({
+            principal,
+            input: { invitationToken: token, mcpServerId },
+            request,
+          })
+      )
       const response = NextResponse.redirect(authorizationUrl)
       response.headers.set('Cache-Control', 'no-store')
       response.headers.set('Referrer-Policy', 'no-referrer')

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

@@ -16,11 +16,13 @@ const {
   mockCompleteManagedMcpOAuth,
   mockConsumeManagedAttempt,
   mockDiscoverServerTools,
+  mockEnforceCallbackRateLimit,
 } = vi.hoisted(() => ({
   mockAuthenticateEnrollment: vi.fn(),
   mockCompleteManagedMcpOAuth: vi.fn(),
   mockConsumeManagedAttempt: vi.fn(),
   mockDiscoverServerTools: vi.fn(),
+  mockEnforceCallbackRateLimit: vi.fn(),
 }))
 
 vi.mock('@/lib/mcp/oauth', () => mcpOauthMock)
@@ -37,6 +39,9 @@ vi.mock('@/lib/credential-groups/mcp-oauth-state', () => ({
   consumeCredentialGroupMcpOAuthAttempt: mockConsumeManagedAttempt,
   isCredentialGroupMcpOAuthState: (state: string) => state.startsWith('mcp_cg_'),
 }))
+vi.mock('@/lib/credential-groups/rate-limit', () => ({
+  enforcePublicCredentialGroupIpRateLimit: mockEnforceCallbackRateLimit,
+}))
 
 import { GET } from './route'
 
@@ -82,6 +87,7 @@ describe('MCP OAuth callback route', () => {
       connectionId: 'mcp-cg-connection-1',
       mcpServerId: 'server-1',
     })
+    mockEnforceCallbackRateLimit.mockResolvedValue(null)
   })
 
   it('performs the token exchange through the SSRF-guarded mcpAuthGuarded wrapper', async () => {
@@ -151,6 +157,7 @@ describe('MCP OAuth callback route', () => {
 
     const response = await GET(request)
 
+    expect(mockEnforceCallbackRateLimit).toHaveBeenCalledWith(request, 'oauth-callback')
     expect(mockConsumeManagedAttempt).toHaveBeenCalledWith('mcp_cg_state-1')
     expect(mockAuthenticateEnrollment).toHaveBeenCalledWith('invitation-token')
     expect(mockCompleteManagedMcpOAuth).toHaveBeenCalledWith(
@@ -166,4 +173,18 @@ describe('MCP OAuth callback route', () => {
       '/credential-groups/enroll/invitation-token?mcp=connected&mcpServerId=server-1'
     )
   })
+
+  it('rate limits a managed callback before consuming its one-time state', async () => {
+    const limitedResponse = new Response('rate limited', { status: 429 })
+    mockEnforceCallbackRateLimit.mockResolvedValueOnce(limitedResponse)
+    const request = new NextRequest(
+      'http://localhost:3000/api/mcp/oauth/callback?state=mcp_cg_state-1&code=auth-code-1'
+    )
+
+    const response = await GET(request)
+
+    expect(response.status).toBe(429)
+    expect(mockConsumeManagedAttempt).not.toHaveBeenCalled()
+    expect(mockCompleteManagedMcpOAuth).not.toHaveBeenCalled()
+  })
 })

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

@@ -15,6 +15,7 @@ import {
   consumeCredentialGroupMcpOAuthAttempt,
   isCredentialGroupMcpOAuthState,
 } from '@/lib/credential-groups/mcp-oauth-state'
+import { enforcePublicCredentialGroupIpRateLimit } from '@/lib/credential-groups/rate-limit'
 import {
   assertSafeOauthServerUrl,
   clearState,
@@ -125,6 +126,8 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
   const { state, code, error: errorParam } = parsed.data.query
 
   if (state && isCredentialGroupMcpOAuthState(state)) {
+    const limited = await enforcePublicCredentialGroupIpRateLimit(request, 'oauth-callback')
+    if (limited) return limited
     return completeManagedMcpCallback({ request, state, code, error: errorParam })
   }
 

apps/sim/app/credential-groups/enroll/[token]/oauth-toast.test.tsx

@@ -44,7 +44,7 @@ describe('CredentialGroupOAuthToast', () => {
     expect(mockSuccess).toHaveBeenCalledOnce()
     expect(mockSuccess).toHaveBeenCalledWith('Gmail connected successfully.')
     expect(mockSetOAuthStatus).toHaveBeenCalledWith(
-      { connected: null, oauth: null, submitted: null },
+      { connected: null, mcp: null, mcpServerId: null, oauth: null, submitted: null },
       { history: 'replace', scroll: false }
     )
     act(() => root.unmount())
@@ -55,7 +55,7 @@ describe('CredentialGroupOAuthToast', () => {
 
     expect(mockError).toHaveBeenCalledWith('Authorization was canceled.')
     expect(mockSetOAuthStatus).toHaveBeenCalledWith(
-      { connected: null, oauth: null, submitted: null },
+      { connected: null, mcp: null, mcpServerId: null, oauth: null, submitted: null },
       { history: 'replace', scroll: false }
     )
     act(() => root.unmount())
@@ -66,7 +66,7 @@ describe('CredentialGroupOAuthToast', () => {
 
     expect(mockSuccess).toHaveBeenCalledWith('Accounts submitted successfully.')
     expect(mockSetOAuthStatus).toHaveBeenCalledWith(
-      { connected: null, oauth: null, submitted: null },
+      { connected: null, mcp: null, mcpServerId: null, oauth: null, submitted: null },
       { history: 'replace', scroll: false }
     )
     act(() => root.unmount())

apps/sim/app/credential-groups/enroll/[token]/oauth-toast.tsx

@@ -29,7 +29,7 @@ export function CredentialGroupOAuthToast({ message, variant }: CredentialGroupO
     else toast.error(message)
 
     void setOAuthStatus(
-      { connected: null, oauth: null, submitted: null },
+      { connected: null, mcp: null, mcpServerId: null, oauth: null, submitted: null },
       { history: 'replace', scroll: false }
     )
   }, [message, setOAuthStatus, toast, variant])

apps/sim/app/credential-groups/enroll/[token]/search-params.ts

@@ -3,6 +3,8 @@ import { parseAsString } from 'nuqs/server'
 /** One-shot OAuth result signals are nullable because absence means no toast. */
 export const credentialGroupEnrollmentStatusParsers = {
   connected: parseAsString,
+  mcp: parseAsString,
+  mcpServerId: parseAsString,
   oauth: parseAsString,
   submitted: parseAsString,
 } as const

apps/sim/ee/credential-groups/components/credential-group-detail.tsx

@@ -137,7 +137,11 @@ export function CredentialGroupDetail({
     : null
   const configurationReady =
     Boolean(
-      credentialGroup && (credentialGroup.options.length || credentialGroup.mcpServers.length)
+      credentialGroup &&
+        (credentialGroup.options.length ||
+          credentialGroup.mcpServers.some(
+            (server) => server.enabled && server.authType === 'oauth'
+          ))
     ) &&
     credentialGroup?.options.every(
       (option) =>

apps/sim/hooks/queries/mcp.test.tsx

@@ -193,6 +193,24 @@ describe('useMcpToolsQuery', () => {
     hook.unmount()
   })
 
+  it('surfaces a shared server-list failure when the managed catalog is empty', async () => {
+    const serverListError = new Error('server list failed')
+    mockRequestJson.mockImplementation(async (contract) => {
+      if (contract === listMcpServersContract) throw serverListError
+      if (contract === listManagedMcpCatalogContract) return { servers: [], tools: [] }
+      throw new Error('Unexpected MCP request')
+    })
+
+    const hook = renderHookWithClient(() => useMcpToolsQuery(WORKSPACE_ID))
+    await flush()
+
+    expect(hook.getResult().data).toEqual([])
+    expect(hook.getResult().error).toBe(serverListError)
+    expect(hook.getResult().isLoading).toBe(false)
+
+    hook.unmount()
+  })
+
   it('defers detail and form metadata queries while their surfaces are closed', async () => {
     mockRequestJson.mockImplementation(async (contract) => {
       if (contract === listStoredMcpToolsContract || contract === getAllowedMcpDomainsContract) {

apps/sim/hooks/queries/mcp.ts

@@ -190,7 +190,11 @@ function isServerEligibleForDiscovery(server: McpServer, workspaceId: string): b
  */
 export function useMcpToolsQuery(workspaceId: string) {
   const queryClient = useQueryClient()
-  const { data: servers, isLoading: serversLoading } = useMcpServers(workspaceId)
+  const {
+    data: servers,
+    isLoading: serversLoading,
+    error: serversError,
+  } = useMcpServers(workspaceId)
   const managedCatalog = useManagedMcpCatalog(workspaceId)
   // Push is intrinsic to consuming the tools query: every surface that reads tools (settings,
   // tool picker, dynamic args, tool selector, canvas block) gets real-time `list_changed`
@@ -236,10 +240,14 @@ export function useMcpToolsQuery(workspaceId: string) {
 
   return useMemo(() => {
     const tools: McpTool[] = [...(managedCatalog.data?.tools ?? [])]
-    let hasData = Boolean(managedCatalog.data)
+    let hasData = Boolean(managedCatalog.data?.tools.length)
     let anyServerLoading = false
     let firstError: Error | null =
-      managedCatalog.error instanceof Error ? managedCatalog.error : null
+      managedCatalog.error instanceof Error
+        ? managedCatalog.error
+        : serversError instanceof Error
+          ? serversError
+          : null
     const statusById = new Map(
       [...(servers ?? []), ...(managedCatalog.data?.servers ?? [])].map((server) => [
         server.id,
@@ -282,7 +290,7 @@ export function useMcpToolsQuery(workspaceId: string) {
       error: hasData ? null : firstError,
       toolsStateByServer,
     }
-  }, [results, serversLoading, serverIds, servers, managedCatalog])
+  }, [results, serversLoading, serversError, serverIds, servers, managedCatalog])
 }
 
 export function useForceRefreshMcpTools() {

apps/sim/lib/api/contracts/credential-groups.ts

@@ -74,8 +74,10 @@ export const credentialGroupOptionUpdateInputSchema = z.discriminatedUnion('prov
 
 export const credentialGroupMcpServerSchema = z.object({
   id: z.string().min(1).max(128),
-  name: z.string().min(1).max(255),
+  name: z.string().min(1),
   description: z.string().nullable(),
+  authType: z.string().min(1),
+  enabled: z.boolean(),
 })
 
 export const credentialGroupSchema = z.object({