fix(mcp): bound and tear down one-shot OAuth fetches so auth can't hang (#5789)

The SSRF-guarded fetch used for MCP OAuth (discovery, DCR, token exchange/
refresh, RFC 7009 revocation) created a fresh pinned undici Agent per request
and never tore it down, and returned the live response so the SDK's lazy body
read happened outside any deadline. The MCP SDK sets no timeout on OAuth legs,
so a stalled body read or a leaked keep-alive socket could leave the flow — and
the browser waiting on it — pending indefinitely ("Connecting… forever").

- Buffer the (always-small) OAuth JSON body inside the guard, under the composed
  deadline/caller AbortSignal, then return a detached in-memory Response. undici's
  bodyTimeout only measures idle gaps between chunks and cannot bound a slow-drip
  body; the AbortSignal is the only true wall-clock deadline over the body read.
- Destroy (not close) the per-request pinned Agent on every path so a one-shot
  leg can't strand its keep-alive socket, and teardown itself can't hang.
- Fix the same per-request Agent leak in the auth-type probe.
- Returning a normalized global Response also surfaces provider token-endpoint
  errors cleanly instead of the SDK's opaque parse failure.

Author: waleedlatif1

apps/sim/lib/core/security/input-validation.server.ts

@@ -452,14 +452,20 @@ export function createPinnedFetch(
  * caller with a defined connection lifetime (e.g. a long-lived MCP transport) can
  * tear the Agent down on close instead of waiting for its idle timeout. Closing
  * the Agent is what releases any pooled keep-alive / HTTP/2 sockets it holds.
+ *
+ * `maxResponseSize` caps the (decoded) response body in bytes and makes undici reject
+ * with `UND_ERR_RES_EXCEEDED_MAX_SIZE` once exceeded — a DoS backstop for one-shot
+ * callers reading from a URL taken from untrusted metadata. Omit it (the default) to
+ * leave the response unbounded, which streaming consumers like the MCP transport need.
  */
 export function createPinnedFetchWithDispatcher(
   resolvedIP: string,
-  options?: { allowH2?: boolean }
+  options?: { allowH2?: boolean; maxResponseSize?: number }
 ): { fetch: typeof fetch; dispatcher: Agent } {
   const dispatcher = new Agent({
     allowH2: options?.allowH2 ?? false,
     connect: { lookup: createPinnedLookup(resolvedIP) },
+    ...(options?.maxResponseSize !== undefined ? { maxResponseSize: options.maxResponseSize } : {}),
   })
 
   const pinned = async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {

apps/sim/lib/mcp/oauth/probe.test.ts

@@ -3,20 +3,30 @@
  */
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockCreatePinnedFetch, mockCreateSsrfGuardedMcpFetch, mockPinnedFetch, mockGuardedFetch } =
-  vi.hoisted(() => {
-    const mockPinnedFetch = vi.fn()
-    const mockGuardedFetch = vi.fn()
-    return {
-      mockPinnedFetch,
-      mockGuardedFetch,
-      mockCreatePinnedFetch: vi.fn(() => mockPinnedFetch),
-      mockCreateSsrfGuardedMcpFetch: vi.fn(() => mockGuardedFetch),
-    }
-  })
+const {
+  mockCreatePinnedFetchWithDispatcher,
+  mockCreateSsrfGuardedMcpFetch,
+  mockPinnedFetch,
+  mockGuardedFetch,
+  mockDestroy,
+} = vi.hoisted(() => {
+  const mockPinnedFetch = vi.fn()
+  const mockGuardedFetch = vi.fn()
+  const mockDestroy = vi.fn(() => Promise.resolve())
+  return {
+    mockPinnedFetch,
+    mockGuardedFetch,
+    mockDestroy,
+    mockCreatePinnedFetchWithDispatcher: vi.fn(() => ({
+      fetch: mockPinnedFetch,
+      dispatcher: { destroy: mockDestroy },
+    })),
+    mockCreateSsrfGuardedMcpFetch: vi.fn(() => mockGuardedFetch),
+  }
+})
 
 vi.mock('@/lib/core/security/input-validation.server', () => ({
-  createPinnedFetch: mockCreatePinnedFetch,
+  createPinnedFetchWithDispatcher: mockCreatePinnedFetchWithDispatcher,
 }))
 vi.mock('@/lib/mcp/pinned-fetch', () => ({
   createSsrfGuardedMcpFetch: mockCreateSsrfGuardedMcpFetch,
@@ -48,7 +58,7 @@ describe('detectMcpAuthType — connection pinning (SSRF / DNS-rebinding)', () =
     const authType = await detectMcpAuthType('https://rebind.example.com/mcp', '203.0.113.10')
 
     expect(authType).toBe('none')
-    expect(mockCreatePinnedFetch).toHaveBeenCalledWith('203.0.113.10')
+    expect(mockCreatePinnedFetchWithDispatcher).toHaveBeenCalledWith('203.0.113.10')
     expect(mockCreateSsrfGuardedMcpFetch).not.toHaveBeenCalled()
     expect(mockPinnedFetch).toHaveBeenCalledTimes(1)
     // The unpinned global fetch must never be used — that was the SSRF sink.
@@ -62,7 +72,7 @@ describe('detectMcpAuthType — connection pinning (SSRF / DNS-rebinding)', () =
 
     expect(authType).toBe('none')
     expect(mockCreateSsrfGuardedMcpFetch).toHaveBeenCalledTimes(1)
-    expect(mockCreatePinnedFetch).not.toHaveBeenCalled()
+    expect(mockCreatePinnedFetchWithDispatcher).not.toHaveBeenCalled()
     expect(mockGuardedFetch).toHaveBeenCalledTimes(1)
     expect(globalFetchSpy).not.toHaveBeenCalled()
   })
@@ -88,7 +98,7 @@ describe('detectMcpAuthType — connection pinning (SSRF / DNS-rebinding)', () =
     const authType = await detectMcpAuthType('http://example.com/mcp', '203.0.113.10')
 
     expect(authType).toBe('headers')
-    expect(mockCreatePinnedFetch).not.toHaveBeenCalled()
+    expect(mockCreatePinnedFetchWithDispatcher).not.toHaveBeenCalled()
     expect(mockCreateSsrfGuardedMcpFetch).not.toHaveBeenCalled()
     expect(globalFetchSpy).not.toHaveBeenCalled()
   })

apps/sim/lib/mcp/oauth/probe.ts

@@ -1,7 +1,7 @@
 import { extractWWWAuthenticateParams } from '@modelcontextprotocol/sdk/client/auth.js'
 import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'
 import { createLogger } from '@sim/logger'
-import { createPinnedFetch } from '@/lib/core/security/input-validation.server'
+import { createPinnedFetchWithDispatcher } from '@/lib/core/security/input-validation.server'
 import { isLoopbackHostname } from '@/lib/core/utils/urls'
 import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
 import type { McpAuthType } from '@/lib/mcp/types'
@@ -33,12 +33,17 @@ export async function detectMcpAuthType(
     return 'headers'
   }
 
-  const probeFetch: FetchLike = resolvedIP
-    ? createPinnedFetch(resolvedIP)
-    : createSsrfGuardedMcpFetch()
+  // When the caller pre-validated the IP we pin to it directly and own the Agent's
+  // lifetime; the SSRF-guarded fetch (used when we must validate ourselves) manages its
+  // own per-request Agent teardown internally.
+  const pinned = resolvedIP ? createPinnedFetchWithDispatcher(resolvedIP) : undefined
+  const probeFetch: FetchLike = pinned?.fetch ?? createSsrfGuardedMcpFetch()
 
   const controller = new AbortController()
   const timer = setTimeout(() => controller.abort(), PROBE_TIMEOUT_MS)
+  // Best-effort session cleanup runs after we return the classification; the pinned
+  // Agent must outlive it, so we tear it down once cleanup settles.
+  let sessionClose: Promise<void> = Promise.resolve()
 
   try {
     const res = await probeFetch(url, {
@@ -63,7 +68,7 @@ export async function detectMcpAuthType(
 
     const sessionId = res.headers.get('mcp-session-id')
     if (sessionId) {
-      void closeMcpSession(url, sessionId, probeFetch)
+      sessionClose = closeMcpSession(url, sessionId, probeFetch)
     }
 
     if (res.status === 401) {
@@ -82,6 +87,11 @@ export async function detectMcpAuthType(
     return 'headers'
   } finally {
     clearTimeout(timer)
+    // Destroy (not close) so a hung request can't make teardown itself hang; wait for
+    // the best-effort session-close hop first so it isn't aborted mid-flight.
+    if (pinned) {
+      void sessionClose.finally(() => pinned.dispatcher.destroy().catch(() => {}))
+    }
   }
 }
 

apps/sim/lib/mcp/oauth/revoke.test.ts

@@ -31,7 +31,10 @@ const {
 }))
 
 vi.mock('@/lib/core/security/input-validation.server', () => ({
-  createPinnedFetch: vi.fn(() => mockUndiciFetch),
+  createPinnedFetchWithDispatcher: vi.fn(() => ({
+    fetch: mockUndiciFetch,
+    dispatcher: { destroy: vi.fn(() => Promise.resolve()) },
+  })),
 }))
 vi.mock('@/lib/mcp/domain-check', () => ({
   validateMcpServerSsrf: mockValidateMcpServerSsrf,