fix(mcp): pinned fetch dispatches through global fetch so Response passes instanceof checks#5620
fix(mcp): pinned fetch dispatches through global fetch so Response passes instanceof checks#5620waleedlatif1 wants to merge 1 commit into
Conversation
…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).
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview That fixes MCP OAuth (and any SDK that does Tests stub global Reviewed by Cursor Bugbot for commit 1ad1ccd. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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) |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 1ad1ccd. Configure here.
Greptile SummaryThis PR fixes MCP pinned fetch responses by routing through the platform fetch path. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (1): Last reviewed commit: "fix(mcp): pinned fetch dispatch through ..." | Re-trigger Greptile |


Summary
createPinnedFetch(used for DNS-pinned SSRF-guarded MCP OAuth requests) called a separately-importedundicipackage'sfetch, returning aResponsefrom that module's own copy of theResponseclass — a distinct constructor reference from Node's globalResponse, even though Node's global fetch is itself undici-backedinput instanceof Response ? await input.text() : inputto decide whether to read the response body. Against the separately-imported undici Response, this check fails, so the rawResponseobject itself gets treated as the body text — stringifying to the literal"[object Response]"in the error message, which then also fails aJSON.parseattemptfetchinstead (passing the same undiciAgentas itsdispatcheroption — Node's fetch forwards this to undici at runtime), so the returnedResponseis the same class reference every consumer'sinstanceof Responsecheck expectsinstanceof Responseisfalsevia the old undici-imported fetch andtruevia the global fetch with a dispatcher — confirmed the exact bug and the fixType of Change
Testing
undici/global fetch confirmed both the bug and the fixpinned-fetch.server.test.tsto mock the global fetch instead of undici's now-unusedfetchexport (the old mock target was silently unused, so those assertions were hitting real network calls)createPinnedFetchpass (21/21), plus provider tests that consume it (25/25)bun run check:api-validationpasses (this change removes 3 double-casts, adds 0)Checklist