Skip to content

Commit 72f9fc9

Browse files
committed
fix(mcp): handle URLSearchParams OAuth bodies and copy stream chunks
- toUndiciRequestBody serializes a URLSearchParams body (the MCP SDK's OAuth token/refresh exchange sends one); undici.request rejects it otherwise. - Default content-type application/x-www-form-urlencoded when the caller didn't set one (fetch parity). - nodeReadableToWebStream enqueues a copy (new Uint8Array(chunk)) not a view, so undici recycling the pooled source buffer can't corrupt queued chunks. - Tests for both.
1 parent d2e037c commit 72f9fc9

2 files changed

Lines changed: 68 additions & 3 deletions

File tree

apps/sim/lib/core/security/guarded-request-fetch.server.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,54 @@ describe('createSsrfGuardedFetchWithDispatcher (undici.request backed)', () => {
132132
expect(Buffer.from(options.body).toString()).toBe('payload')
133133
})
134134

135+
it('serializes a URLSearchParams body and defaults the form content-type (OAuth token exchange)', async () => {
136+
mockUndiciRequest.mockResolvedValueOnce(undiciReply(200, {}, byteStream('{}')))
137+
const { fetch } = createSsrfGuardedFetchWithDispatcher()
138+
139+
await fetch('https://auth.example.com/token', {
140+
method: 'POST',
141+
body: new URLSearchParams({ grant_type: 'refresh_token', refresh_token: 'r-1' }),
142+
})
143+
144+
const [, options] = mockUndiciRequest.mock.calls[0]
145+
expect(options.body).toBe('grant_type=refresh_token&refresh_token=r-1')
146+
expect(options.headers['content-type']).toBe('application/x-www-form-urlencoded;charset=UTF-8')
147+
})
148+
149+
it('does not override an explicit content-type on a URLSearchParams body', async () => {
150+
mockUndiciRequest.mockResolvedValueOnce(undiciReply(200, {}, byteStream('{}')))
151+
const { fetch } = createSsrfGuardedFetchWithDispatcher()
152+
153+
await fetch('https://auth.example.com/token', {
154+
method: 'POST',
155+
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
156+
body: new URLSearchParams({ grant_type: 'authorization_code' }),
157+
})
158+
159+
const [, options] = mockUndiciRequest.mock.calls[0]
160+
expect(options.body).toBe('grant_type=authorization_code')
161+
expect(options.headers['Content-Type']).toBe('application/x-www-form-urlencoded')
162+
expect(options.headers['content-type']).toBeUndefined()
163+
})
164+
165+
it('copies each chunk so a recycled source buffer cannot corrupt queued data', async () => {
166+
const source = new Readable({ read() {} })
167+
mockUndiciRequest.mockResolvedValueOnce(undiciReply(200, {}, source))
168+
const { fetch } = createSsrfGuardedFetchWithDispatcher()
169+
170+
const response = await fetch('https://mcp.example.com/stream', { method: 'GET' })
171+
const reader = response.body!.getReader()
172+
173+
// Emit a chunk, then mutate the SAME backing buffer (as undici's pool reuse would).
174+
const buf = Buffer.from('AB')
175+
source.push(buf)
176+
const first = await reader.read()
177+
buf[0] = 0x00 // corrupt the source buffer after the chunk was enqueued
178+
expect(Buffer.from(first.value!).toString()).toBe('AB') // copy is unaffected
179+
source.push(null)
180+
await reader.read()
181+
})
182+
135183
it('rejects the reader when the source is destroyed without an error (abort/reset)', async () => {
136184
const source = new Readable({ read() {} }) // stays open, never pushes
137185
mockUndiciRequest.mockResolvedValueOnce(undiciReply(200, {}, source))

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,9 @@ function toUndiciRequestBody(
660660
body: UndiciRequestInit['body']
661661
): string | Buffer | Uint8Array | Readable | undefined {
662662
if (body == null) return undefined
663+
// fetch accepts URLSearchParams (form-encoded) and undici.request does not — the MCP SDK's
664+
// OAuth token/refresh exchange sends one. Serialize it to its wire form.
665+
if (body instanceof URLSearchParams) return body.toString()
663666
if (body instanceof ArrayBuffer) return Buffer.from(body)
664667
if (ArrayBuffer.isView(body) && !(body instanceof Uint8Array)) {
665668
return Buffer.from(body.buffer, body.byteOffset, body.byteLength)
@@ -689,7 +692,10 @@ function nodeReadableToWebStream(nodeStream: Readable): ReadableStream<Uint8Arra
689692
start(controller) {
690693
nodeStream.on('data', (chunk: Buffer) => {
691694
try {
692-
controller.enqueue(new Uint8Array(chunk.buffer, chunk.byteOffset, chunk.byteLength))
695+
// Copy, not a view: undici may recycle the pooled buffer backing `chunk` after
696+
// this handler returns, which would corrupt a chunk still queued for a slow
697+
// consumer. `new Uint8Array(chunk)` allocates a fresh backing buffer.
698+
controller.enqueue(new Uint8Array(chunk))
693699
} catch {
694700
// Controller already closed (consumer cancelled) — stop the source, drop the chunk.
695701
nodeStream.destroy()
@@ -775,10 +781,21 @@ async function undiciRequestAsResponse(
775781

776782
const method = (effectiveInit.method ?? 'GET').toUpperCase()
777783
const canHaveBody = method !== 'GET' && method !== 'HEAD'
784+
const requestHeaders = toUndiciRequestHeaders(effectiveInit.headers) ?? {}
785+
const requestBody = canHaveBody ? toUndiciRequestBody(effectiveInit.body) : undefined
786+
// fetch auto-adds a form content-type for a URLSearchParams body; preserve that parity
787+
// when the caller didn't set one (the MCP SDK does set it explicitly, but not every caller).
788+
if (
789+
canHaveBody &&
790+
effectiveInit.body instanceof URLSearchParams &&
791+
!Object.keys(requestHeaders).some((key) => key.toLowerCase() === 'content-type')
792+
) {
793+
requestHeaders['content-type'] = 'application/x-www-form-urlencoded;charset=UTF-8'
794+
}
778795
const { statusCode, headers, body } = await undiciRequest(url, {
779796
method: method as Dispatcher.HttpMethod,
780-
headers: toUndiciRequestHeaders(effectiveInit.headers),
781-
body: canHaveBody ? toUndiciRequestBody(effectiveInit.body) : undefined,
797+
headers: requestHeaders,
798+
body: requestBody,
782799
signal: effectiveInit.signal ?? undefined,
783800
dispatcher,
784801
// No `maxRedirections`: request() does not auto-follow by default, so the caller's

0 commit comments

Comments
 (0)