Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions apps/sim/lib/core/security/input-validation.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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<Response> => {
// 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<typeof undiciFetch>[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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinned fetch broken under Bun

High Severity

createPinnedFetch now calls the global fetch with an undici Agent as dispatcher. Production starts via Bun (CMD ["bun", "apps/sim/bootstrap.js"]), and Bun’s fetch is known not to honor undici Agent connect options such as custom lookup. The prior undici-imported fetch did apply that pinning. DNS-rebinding SSRF guards for MCP OAuth and Azure provider traffic can therefore fail open while instanceof Response checks pass. Unit tests run under @vitest-environment node, so they do not catch this runtime gap.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1ad1ccd. Configure here.

}

return pinned
Expand Down
35 changes: 21 additions & 14 deletions apps/sim/lib/core/security/pinned-fetch.server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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'
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
})

Expand All @@ -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)
})

Expand All @@ -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)
Expand Down
Loading