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

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,