fix(mcp): bound OAuth discovery/DCR/token fetches with a timeout

The MCP SDK issues OAuth discovery, dynamic client registration, and token
exchange with a bare fetch and no AbortSignal (only the JSON-RPC layer gets the
SDK's request timeout), and undici's default headers/body timeouts are 5 min. A
slow or unresponsive authorization server therefore left /oauth/start pending for
minutes — the browser stuck on "Connecting…" forever.

Bound each guarded OAuth/revocation leg with a 30s AbortSignal.timeout, composed
with any caller signal so cancellation still works. Only our own deadline is
relabeled to an McpError; caller aborts and other failures propagate unchanged.
Scoped to createSsrfGuardedMcpFetch (OAuth/revoke/probe) — the live transport's
timeouts are untouched.

Author: waleedlatif1

apps/sim/lib/mcp/pinned-fetch.test.ts

@@ -32,9 +32,62 @@ describe('createSsrfGuardedMcpFetch', () => {
 
     expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith('https://attacker.example/revoke')
     expect(mockCreatePinnedFetch).toHaveBeenCalledWith('203.0.113.10')
-    expect(sentinelFetch).toHaveBeenCalledWith('https://attacker.example/revoke', {
-      method: 'POST',
-    })
+    expect(sentinelFetch).toHaveBeenCalledWith(
+      'https://attacker.example/revoke',
+      expect.objectContaining({ method: 'POST', signal: expect.any(AbortSignal) })
+    )
+  })
+
+  it('attaches an abort signal to every guarded request even without a caller signal', async () => {
+    mockValidateMcpServerSsrf.mockResolvedValue('203.0.113.10')
+    const fetchLike = createSsrfGuardedMcpFetch()
+    await fetchLike('https://attacker.example/discover')
+
+    const [, init] = sentinelFetch.mock.calls[0]
+    expect(init.signal).toBeInstanceOf(AbortSignal)
+  })
+
+  it('surfaces an McpError when a request exceeds the deadline', async () => {
+    mockValidateMcpServerSsrf.mockResolvedValue('203.0.113.10')
+    // Hang until the guard's own deadline aborts the request.
+    sentinelFetch.mockImplementation(
+      (_url: string, init: RequestInit) =>
+        new Promise((_resolve, reject) => {
+          const signal = init.signal
+          if (signal?.aborted) {
+            reject(signal.reason)
+            return
+          }
+          signal?.addEventListener('abort', () => reject(signal.reason), { once: true })
+        })
+    )
+    const fetchLike = createSsrfGuardedMcpFetch(5)
+
+    await expect(fetchLike('https://slow.example/token', { method: 'POST' })).rejects.toThrow(
+      /timed out after 5ms/
+    )
+  })
+
+  it('propagates a caller-initiated abort unchanged (composed with the deadline)', async () => {
+    mockValidateMcpServerSsrf.mockResolvedValue('203.0.113.10')
+    sentinelFetch.mockImplementation(
+      (_url: string, init: RequestInit) =>
+        new Promise((_resolve, reject) => {
+          const signal = init.signal
+          if (signal?.aborted) {
+            reject(signal.reason)
+            return
+          }
+          signal?.addEventListener('abort', () => reject(signal.reason), { once: true })
+        })
+    )
+    const controller = new AbortController()
+    // Long deadline so the caller's abort — not the timeout — is what settles the request.
+    const fetchLike = createSsrfGuardedMcpFetch(60_000)
+    const pending = fetchLike('https://slow.example/token', { signal: controller.signal })
+    controller.abort(new Error('caller cancelled'))
+
+    await expect(pending).rejects.toThrow('caller cancelled')
   })
 
   it('rejects URLs that resolve to blocked IPs without issuing the request', async () => {

apps/sim/lib/mcp/pinned-fetch.ts

@@ -4,6 +4,7 @@ import {
   createPinnedFetchWithDispatcher,
 } from '@/lib/core/security/input-validation.server'
 import { validateMcpServerSsrf } from '@/lib/mcp/domain-check'
+import { McpError } from '@/lib/mcp/types'
 
 /** Pinned fetch for the live MCP transport, plus a handle to release its sockets. */
 export interface PinnedMcpFetch {
@@ -31,6 +32,20 @@ export function createPinnedMcpFetch(resolvedIP: string): PinnedMcpFetch {
   return { fetch: pinnedFetch, close: () => dispatcher.destroy() }
 }
 
+/**
+ * Per-request deadline for guarded MCP OAuth / RFC 7009 revocation HTTP calls.
+ *
+ * The MCP SDK issues OAuth discovery, dynamic client registration, and token
+ * exchange with a bare `fetch` and no `AbortSignal` — only the JSON-RPC message
+ * layer gets the SDK's request timeout. Combined with undici's 5-minute default
+ * headers/body timeouts, a slow or unresponsive authorization server leaves the
+ * request (and the browser the user is waiting on during `/oauth/start`) pending
+ * for minutes. Bounding each leg turns that into a fast, actionable failure. 30s
+ * mirrors `MCP_CLIENT_CONSTANTS.DEFAULT_CONNECTION_TIMEOUT` and leaves wide
+ * headroom over a healthy server, which completes each leg in well under a second.
+ */
+const OAUTH_FETCH_TIMEOUT_MS = 30_000
+
 /**
  * Builds a `FetchLike` that validates every outbound request URL against the
  * MCP SSRF policy before issuing it, then pins the connection to the resolved
@@ -42,19 +57,35 @@ export function createPinnedMcpFetch(resolvedIP: string): PinnedMcpFetch {
  * per request and rejects private/reserved/loopback targets (honoring
  * `ALLOWED_MCP_DOMAINS` and self-hosted localhost rules).
  *
- * Note: a caller-provided `AbortSignal` in `init` only bounds the HTTP request,
- * not the validation DNS lookup — Node's `dns.lookup` does not accept a signal,
- * so a hanging resolution can extend the overall call past the caller's timeout
- * by up to the OS DNS timeout. Acceptable here because all consumers are
- * best-effort, non-blocking flows (OAuth discovery and RFC 7009 revocation).
+ * Each request is bounded by a `timeoutMs` deadline via `AbortSignal.timeout`,
+ * composed with any caller-provided signal so cancellation still works. Only our
+ * own deadline is relabeled to an `McpError`; a caller abort or any other failure
+ * propagates unchanged.
+ *
+ * Note: the deadline only bounds the HTTP request, not the validation DNS lookup
+ * — Node's `dns.lookup` does not accept a signal, so a hanging resolution can
+ * extend the overall call by up to the OS DNS timeout. Acceptable here because
+ * all consumers are best-effort authorization/revocation flows.
  *
+ * @param timeoutMs Per-request deadline in ms (defaults to 30s; override for tests).
  * @throws McpSsrfError if a request URL resolves to a blocked IP address
+ * @throws McpError if a request exceeds `timeoutMs`
  */
-export function createSsrfGuardedMcpFetch(): FetchLike {
+export function createSsrfGuardedMcpFetch(timeoutMs: number = OAUTH_FETCH_TIMEOUT_MS): FetchLike {
   return (async (url, init) => {
     const target = typeof url === 'string' ? url : url.href
     const resolvedIP = await validateMcpServerSsrf(target)
     const pinnedFetch: FetchLike = resolvedIP ? createPinnedFetch(resolvedIP) : globalThis.fetch
-    return pinnedFetch(url, init)
+    const timeoutSignal = AbortSignal.timeout(timeoutMs)
+    const signal = init?.signal ? AbortSignal.any([init.signal, timeoutSignal]) : timeoutSignal
+    try {
+      return await pinnedFetch(url, { ...init, signal })
+    } catch (error) {
+      if (timeoutSignal.aborted && !init?.signal?.aborted) {
+        const host = URL.canParse(target) ? new URL(target).host : target
+        throw new McpError(`MCP authorization request to ${host} timed out after ${timeoutMs}ms`)
+      }
+      throw error
+    }
   }) satisfies FetchLike
 }