Skip to content

Commit 1ad1ccd

Browse files
waleedlatif1waleedlatif1
committed
fix(mcp): pinned fetch dispatch through global fetch so Response passes instanceof checks
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).
1 parent f477faa commit 1ad1ccd

2 files changed

Lines changed: 32 additions & 22 deletions

File tree

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { createLogger } from '@sim/logger'
66
import { toError } from '@sim/utils/errors'
77
import { omit } from '@sim/utils/object'
88
import * as ipaddr from 'ipaddr.js'
9-
import { Agent, type RequestInit as UndiciRequestInit, fetch as undiciFetch } from 'undici'
9+
import { Agent } from 'undici'
1010
import { isHosted, isPrivateDatabaseHostsAllowed } from '@/lib/core/config/env-flags'
1111
import { type ValidationResult, validateExternalUrl } from '@/lib/core/security/input-validation'
1212
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
@@ -433,18 +433,21 @@ export function createPinnedLookup(resolvedIP: string): LookupFunction {
433433
*
434434
* The `Agent` is captured for the lifetime of the returned function, so repeated
435435
* calls (e.g. a provider tool loop) reuse its keep-alive connections.
436+
*
437+
* Dispatches through the global `fetch` (passing `dispatcher` as an
438+
* undici-specific extension) rather than a separately imported `undici` copy of
439+
* `fetch`, so the returned `Response` is the same class reference every other
440+
* consumer's `instanceof Response` check expects. A separately imported undici
441+
* `Response` fails that check across module instances — the caller then treats
442+
* the whole `Response` object as if it were already-read body text.
436443
*/
437444
export function createPinnedFetch(resolvedIP: string): typeof fetch {
438445
const dispatcher = new Agent({ connect: { lookup: createPinnedLookup(resolvedIP) } })
439446

440447
const pinned = async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
441-
// double-cast-allowed: DOM RequestInfo/URL and undici fetch input types differ but are structurally compatible at runtime (Node's global fetch IS undici)
442-
const undiciInput = input as unknown as Parameters<typeof undiciFetch>[0]
443-
// double-cast-allowed: DOM RequestInit and undici RequestInit are structurally compatible at runtime but the TS types differ
444-
const undiciInit: UndiciRequestInit = { ...(init as unknown as UndiciRequestInit), dispatcher }
445-
const response = await undiciFetch(undiciInput, undiciInit)
446-
// double-cast-allowed: undici Response and DOM Response are structurally compatible at runtime
447-
return response as unknown as Response
448+
// `dispatcher` is an undici-specific fetch option Node's global fetch forwards at
449+
// runtime, but the DOM RequestInit type doesn't declare it.
450+
return fetch(input, { ...init, dispatcher } as RequestInit)
448451
}
449452

450453
return pinned

‎apps/sim/lib/core/security/pinned-fetch.server.test.ts‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
* @vitest-environment node
33
*/
44
import { envFlagsMock } from '@sim/testing'
5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockAgent, mockUndiciFetch, capturedAgentOptions, agentCloses } = vi.hoisted(() => {
7+
const { mockAgent, mockGlobalFetch, capturedAgentOptions, agentCloses } = vi.hoisted(() => {
88
const capturedAgentOptions: unknown[] = []
99
const agentCloses: unknown[] = []
1010
class MockAgent {
@@ -18,13 +18,13 @@ const { mockAgent, mockUndiciFetch, capturedAgentOptions, agentCloses } = vi.hoi
1818
}
1919
return {
2020
mockAgent: MockAgent,
21-
mockUndiciFetch: vi.fn(),
21+
mockGlobalFetch: vi.fn(),
2222
capturedAgentOptions,
2323
agentCloses,
2424
}
2525
})
2626

27-
vi.mock('undici', () => ({ Agent: mockAgent, fetch: mockUndiciFetch }))
27+
vi.mock('undici', () => ({ Agent: mockAgent }))
2828
vi.mock('@/lib/core/config/env-flags', () => envFlagsMock)
2929

3030
import { createPinnedFetch } from '@/lib/core/security/input-validation.server'
@@ -37,7 +37,14 @@ describe('createPinnedFetch', () => {
3737
vi.clearAllMocks()
3838
capturedAgentOptions.length = 0
3939
agentCloses.length = 0
40-
mockUndiciFetch.mockResolvedValue(new Response('ok'))
40+
mockGlobalFetch.mockResolvedValue(new Response('ok'))
41+
// Dispatches through the platform's global fetch (see input-validation.server.ts) so its
42+
// returned Response stays instanceof the same Response every consumer checks against.
43+
vi.stubGlobal('fetch', mockGlobalFetch)
44+
})
45+
46+
afterEach(() => {
47+
vi.unstubAllGlobals()
4148
})
4249

4350
it('builds an undici Agent whose pinned lookup always resolves to the validated IP', async () => {
@@ -75,8 +82,8 @@ describe('createPinnedFetch', () => {
7582
signal: controller.signal,
7683
})
7784

78-
expect(mockUndiciFetch).toHaveBeenCalledTimes(1)
79-
const [url, init] = mockUndiciFetch.mock.calls[0]
85+
expect(mockGlobalFetch).toHaveBeenCalledTimes(1)
86+
const [url, init] = mockGlobalFetch.mock.calls[0]
8087
expect(url).toBe('https://myresource.openai.azure.com/openai/v1/responses')
8188
const typedInit = init as RequestInit & { dispatcher?: unknown }
8289
expect(typedInit.dispatcher).toBeInstanceOf(mockAgent)
@@ -89,7 +96,7 @@ describe('createPinnedFetch', () => {
8996
it('handles an undefined init by still attaching the dispatcher', async () => {
9097
const pinned = createPinnedFetch('203.0.113.10')
9198
await pinned('https://example.com')
92-
const init = mockUndiciFetch.mock.calls[0][1] as { dispatcher?: unknown }
99+
const init = mockGlobalFetch.mock.calls[0][1] as { dispatcher?: unknown }
93100
expect(init.dispatcher).toBeInstanceOf(mockAgent)
94101
})
95102

@@ -99,8 +106,8 @@ describe('createPinnedFetch', () => {
99106
await pinned('https://example.com/b')
100107

101108
expect(capturedAgentOptions).toHaveLength(1)
102-
const d1 = (mockUndiciFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher
103-
const d2 = (mockUndiciFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher
109+
const d1 = (mockGlobalFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher
110+
const d2 = (mockGlobalFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher
104111
expect(d1).toBe(d2)
105112
})
106113

@@ -111,13 +118,13 @@ describe('createPinnedFetch', () => {
111118
await b('https://example.com/b')
112119

113120
expect(capturedAgentOptions).toHaveLength(2)
114-
const d1 = (mockUndiciFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher
115-
const d2 = (mockUndiciFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher
121+
const d1 = (mockGlobalFetch.mock.calls[0][1] as { dispatcher: unknown }).dispatcher
122+
const d2 = (mockGlobalFetch.mock.calls[1][1] as { dispatcher: unknown }).dispatcher
116123
expect(d1).not.toBe(d2)
117124
})
118125

119-
it('returns the response produced by undici fetch', async () => {
120-
mockUndiciFetch.mockResolvedValueOnce(new Response('pong', { status: 201 }))
126+
it('returns the response produced by the global fetch', async () => {
127+
mockGlobalFetch.mockResolvedValueOnce(new Response('pong', { status: 201 }))
121128
const pinned = createPinnedFetch('203.0.113.10')
122129
const response = await pinned('https://example.com')
123130
expect(response.status).toBe(201)

0 commit comments

Comments
 (0)