Skip to content

fix(mcp): pinned fetch dispatches through global fetch so Response passes instanceof checks#5620

Open
waleedlatif1 wants to merge 1 commit into
stagingfrom
worktree-fix-mcp-oauth-error-response
Open

fix(mcp): pinned fetch dispatches through global fetch so Response passes instanceof checks#5620
waleedlatif1 wants to merge 1 commit into
stagingfrom
worktree-fix-mcp-oauth-error-response

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes the "Invalid OAuth error response: SyntaxError: JSON Parse error: Unexpected identifier "object". Raw body: [object Response]" toast shown when authorizing an MCP server whose OAuth error response body isn't a clean spec-compliant JSON object
  • Root cause: createPinnedFetch (used for DNS-pinned SSRF-guarded MCP OAuth requests) called a separately-imported undici package's fetch, returning a Response from that module's own copy of the Response class — a distinct constructor reference from Node's global Response, even though Node's global fetch is itself undici-backed
  • The MCP SDK's OAuth error parser does input instanceof Response ? await input.text() : input to decide whether to read the response body. Against the separately-imported undici Response, this check fails, so the raw Response object itself gets treated as the body text — stringifying to the literal "[object Response]" in the error message, which then also fails a JSON.parse attempt
  • Fix: dispatch through the global fetch instead (passing the same undici Agent as its dispatcher option — Node's fetch forwards this to undici at runtime), so the returned Response is the same class reference every consumer's instanceof Response check expects
  • Verified directly with a standalone repro script: instanceof Response is false via the old undici-imported fetch and true via the global fetch with a dispatcher — confirmed the exact bug and the fix

Type of Change

  • Bug fix

Testing

  • Standalone repro script against real undici/global fetch confirmed both the bug and the fix
  • Updated pinned-fetch.server.test.ts to mock the global fetch instead of undici's now-unused fetch export (the old mock target was silently unused, so those assertions were hitting real network calls)
  • All existing tests touching createPinnedFetch pass (21/21), plus provider tests that consume it (25/25)
  • bun run check:api-validation passes (this change removes 3 double-casts, adds 0)

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…es 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).
@vercel

vercel Bot commented Jul 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 12, 2026 4:34am

Request Review

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Small change in the SSRF-pinned fetch path with the same Agent/dispatcher semantics; tests were updated to match and reduce accidental real-network calls.

Overview
createPinnedFetch still pins outbound connections with the same undici Agent and DNS lookup, but it now calls global fetch with dispatcher in the init instead of undici's separately imported fetch.

That fixes MCP OAuth (and any SDK that does input instanceof Response) treating error bodies as the literal "[object Response]" when the pinned client returned a Response from a different module copy of the class. IP pinning behavior is unchanged; only which fetch/Response constructor is used.

Tests stub global fetch instead of mocking an unused undici fetch export, so assertions no longer hit the real network.

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

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

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.

@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes MCP pinned fetch responses by routing through the platform fetch path. The main changes are:

  • createPinnedFetch now calls global fetch with the pinned undici dispatcher.
  • The separate undici.fetch import and related type casts were removed.
  • The pinned fetch tests now stub global fetch instead of the unused undici fetch export.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The server package targets Node, where the global fetch path can use the undici dispatcher.
  • The updated tests now mock the fetch binding that createPinnedFetch actually calls.

Important Files Changed

Filename Overview
apps/sim/lib/core/security/input-validation.server.ts Routes pinned requests through global fetch while preserving the pinned undici dispatcher.
apps/sim/lib/core/security/pinned-fetch.server.test.ts Updates the test mock to target the global fetch path used by the changed implementation.

Reviews (1): Last reviewed commit: "fix(mcp): pinned fetch dispatch through ..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant