From 1ad1ccd06aa536a5e9a348564a64c1f79c6ba1ad Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 11 Jul 2026 21:33:53 -0700 Subject: [PATCH] fix(mcp): pinned fetch dispatch through global fetch so Response passes instanceof checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createPinnedFetch called a separately-imported undici `fetch` (used for DNS-pinned MCP OAuth/SSRF-guarded requests), returning a Response instance from that separate undici module copy. Node's own global fetch is also undici-backed, but a directly imported `undici` package copy is a distinct module instance with its own Response constructor reference — so `instanceof Response` checks against the platform's global Response fail across that boundary. The MCP SDK's OAuth error parser (parseErrorResponse) does exactly that check to decide whether to read the response body as text: `input instanceof Response ? await input.text() : input`. When it failed, `body` became the raw Response object itself, which then stringified to the literal "[object Response]" in the error message and failed a JSON.parse attempt on that non-JSON string — surfacing a confusing "Invalid OAuth error response: ... Raw body: [object Response]" toast on any MCP OAuth error whose response body isn't a clean OAuth-spec JSON error object. Dispatch through the global `fetch` instead (passing the same undici `Agent` as its `dispatcher` option, which Node's fetch forwards to undici at runtime) so the returned Response is the same class every consumer's instanceof check expects. Verified directly: a standalone repro script confirms `instanceof Response` is false via the old undici-imported fetch and true via the global fetch with a dispatcher. Updated pinned-fetch.server.test.ts to mock the global fetch instead of undici's fetch export, matching the new call path (the old mock target being unused meant those assertions were silently hitting real network calls). --- .../core/security/input-validation.server.ts | 19 +++++----- .../core/security/pinned-fetch.server.test.ts | 35 +++++++++++-------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/apps/sim/lib/core/security/input-validation.server.ts b/apps/sim/lib/core/security/input-validation.server.ts index 7f81039622c..320ef6fb8be 100644 --- a/apps/sim/lib/core/security/input-validation.server.ts +++ b/apps/sim/lib/core/security/input-validation.server.ts @@ -6,7 +6,7 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { omit } from '@sim/utils/object' import * as ipaddr from 'ipaddr.js' -import { Agent, type RequestInit as UndiciRequestInit, fetch as undiciFetch } from 'undici' +import { Agent } from 'undici' import { isHosted, isPrivateDatabaseHostsAllowed } from '@/lib/core/config/env-flags' import { type ValidationResult, validateExternalUrl } from '@/lib/core/security/input-validation' import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' @@ -433,18 +433,21 @@ export function createPinnedLookup(resolvedIP: string): LookupFunction { * * The `Agent` is captured for the lifetime of the returned function, so repeated * calls (e.g. a provider tool loop) reuse its keep-alive connections. + * + * Dispatches through the global `fetch` (passing `dispatcher` as an + * undici-specific extension) rather than a separately imported `undici` copy of + * `fetch`, so the returned `Response` is the same class reference every other + * consumer's `instanceof Response` check expects. A separately imported undici + * `Response` fails that check across module instances — the caller then treats + * the whole `Response` object as if it were already-read body text. */ export function createPinnedFetch(resolvedIP: string): typeof fetch { const dispatcher = new Agent({ connect: { lookup: createPinnedLookup(resolvedIP) } }) const pinned = async (input: RequestInfo | URL, init?: RequestInit): Promise => { - // double-cast-allowed: DOM RequestInfo/URL and undici fetch input types differ but are structurally compatible at runtime (Node's global fetch IS undici) - const undiciInput = input as unknown as Parameters[0] - // double-cast-allowed: DOM RequestInit and undici RequestInit are structurally compatible at runtime but the TS types differ - const undiciInit: UndiciRequestInit = { ...(init as unknown as UndiciRequestInit), dispatcher } - const response = await undiciFetch(undiciInput, undiciInit) - // double-cast-allowed: undici Response and DOM Response are structurally compatible at runtime - return response as unknown as Response + // `dispatcher` is an undici-specific fetch option Node's global fetch forwards at + // runtime, but the DOM RequestInit type doesn't declare it. + return fetch(input, { ...init, dispatcher } as RequestInit) } return pinned diff --git a/apps/sim/lib/core/security/pinned-fetch.server.test.ts b/apps/sim/lib/core/security/pinned-fetch.server.test.ts index e07d0d412dd..b9ea2f617dc 100644 --- a/apps/sim/lib/core/security/pinned-fetch.server.test.ts +++ b/apps/sim/lib/core/security/pinned-fetch.server.test.ts @@ -2,9 +2,9 @@ * @vitest-environment node */ import { envFlagsMock } from '@sim/testing' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockAgent, mockUndiciFetch, capturedAgentOptions, agentCloses } = vi.hoisted(() => { +const { mockAgent, mockGlobalFetch, capturedAgentOptions, agentCloses } = vi.hoisted(() => { const capturedAgentOptions: unknown[] = [] const agentCloses: unknown[] = [] class MockAgent { @@ -18,13 +18,13 @@ const { mockAgent, mockUndiciFetch, capturedAgentOptions, agentCloses } = vi.hoi } return { mockAgent: MockAgent, - mockUndiciFetch: vi.fn(), + mockGlobalFetch: vi.fn(), capturedAgentOptions, agentCloses, } }) -vi.mock('undici', () => ({ Agent: mockAgent, fetch: mockUndiciFetch })) +vi.mock('undici', () => ({ Agent: mockAgent })) vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) import { createPinnedFetch } from '@/lib/core/security/input-validation.server' @@ -37,7 +37,14 @@ describe('createPinnedFetch', () => { vi.clearAllMocks() capturedAgentOptions.length = 0 agentCloses.length = 0 - mockUndiciFetch.mockResolvedValue(new Response('ok')) + mockGlobalFetch.mockResolvedValue(new Response('ok')) + // Dispatches through the platform's global fetch (see input-validation.server.ts) so its + // returned Response stays instanceof the same Response every consumer checks against. + vi.stubGlobal('fetch', mockGlobalFetch) + }) + + afterEach(() => { + vi.unstubAllGlobals() }) it('builds an undici Agent whose pinned lookup always resolves to the validated IP', async () => { @@ -75,8 +82,8 @@ describe('createPinnedFetch', () => { signal: controller.signal, }) - expect(mockUndiciFetch).toHaveBeenCalledTimes(1) - const [url, init] = mockUndiciFetch.mock.calls[0] + expect(mockGlobalFetch).toHaveBeenCalledTimes(1) + const [url, init] = mockGlobalFetch.mock.calls[0] expect(url).toBe('https://myresource.openai.azure.com/openai/v1/responses') const typedInit = init as RequestInit & { dispatcher?: unknown } expect(typedInit.dispatcher).toBeInstanceOf(mockAgent) @@ -89,7 +96,7 @@ describe('createPinnedFetch', () => { it('handles an undefined init by still attaching the dispatcher', async () => { const pinned = createPinnedFetch('203.0.113.10') await pinned('https://example.com') - const init = mockUndiciFetch.mock.calls[0][1] as { dispatcher?: unknown } + const init = mockGlobalFetch.mock.calls[0][1] as { dispatcher?: unknown } expect(init.dispatcher).toBeInstanceOf(mockAgent) }) @@ -99,8 +106,8 @@ describe('createPinnedFetch', () => { await pinned('https://example.com/b') expect(capturedAgentOptions).toHaveLength(1) - const d1 = (mockUndiciFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher - const d2 = (mockUndiciFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher + const d1 = (mockGlobalFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher + const d2 = (mockGlobalFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher expect(d1).toBe(d2) }) @@ -111,13 +118,13 @@ describe('createPinnedFetch', () => { await b('https://example.com/b') expect(capturedAgentOptions).toHaveLength(2) - const d1 = (mockUndiciFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher - const d2 = (mockUndiciFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher + const d1 = (mockGlobalFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher + const d2 = (mockGlobalFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher expect(d1).not.toBe(d2) }) - it('returns the response produced by undici fetch', async () => { - mockUndiciFetch.mockResolvedValueOnce(new Response('pong', { status: 201 })) + it('returns the response produced by the global fetch', async () => { + mockGlobalFetch.mockResolvedValueOnce(new Response('pong', { status: 201 })) const pinned = createPinnedFetch('203.0.113.10') const response = await pinned('https://example.com') expect(response.status).toBe(201)