security: SSRF guardrails for server-side connector fetches - #26
Conversation
Port the fork's SSRF hardening for MCP connector egress into the upstream layout: - Extract private/reserved IP classification into lib/privateIp.ts and fix IPv6 gaps: fe80::/10 link-local matching (the /^fe[89ab]:/ regex only matched the hextet "fe8:" and let fe80::1 through), hex-form IPv4-mapped addresses (::ffff:a00:1), NAT64 (64:ff9b::/96) and 6to4 (2002::/16) embedded IPv4 ranges. - Strip brackets from IPv6 literals in validateRemoteMcpUrl so [::1] et al. are classified by the private-IP guard instead of falling through to DNS lookup. - Route all OAuth egress (metadata fetch, discovery probes, dynamic client registration, token refresh) through guardedFetch so every outbound MCP request gets the same HTTPS-only / blocked-host / private-IP / no-redirect checks; the discovery probes were previously raw, unvalidated fetches. - Add SSRF regression tests (run atop the test-harness PR) and exclude test files from the tsc production build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
Restore the fork's pinnedGuardAgent verbatim: guardedFetch now routes
through a per-request undici Agent whose connect-time DNS lookup runs
the private-IP guard and returns only validated addresses, so the
address we validate is the address we connect to — closing the
DNS-rebinding/TOCTOU window between the pre-fetch validation lookup and
the socket's own resolution. Adds the undici runtime dependency the
fork ships for exactly this purpose ("undici": "^6.27.0"), and restores
the dispatcher assertions in the SSRF test so it matches the fork's
byte for byte.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
SummaryWhen a user adds an MCP connector, the server makes outbound HTTP requests to whatever URL the connector points at — including the OAuth discovery, registration, and token-refresh calls. Without guardrails, a malicious connector URL can turn our own server into a proxy: it can make the backend fetch internal-network addresses or cloud metadata endpoints (e.g. Changes
WhyConnector URLs are user-supplied and therefore attacker-influenced; every gap above is a way to aim the backend's own network position at things only the backend can reach. This shrinks that security surface with pure validation logic on the existing egress path — zero new services and zero hosting cost. It adds exactly one dependency: undici — the engine behind Node's own fetch — pinned for connect-time DNS validation; used by the fork in production for exactly this purpose. Testing
ProvenanceAll changes are mechanical ports of code in amal66/mike@origin/main (commit b3166dd):
🤖 Generated with Claude Code |
Summary
When a user adds an MCP connector, the server makes outbound HTTP requests to whatever URL the connector points at — including the OAuth discovery, registration, and token-refresh calls. Without guardrails, a malicious connector URL can turn our own server into a proxy: it can make the backend fetch internal-network addresses or cloud metadata endpoints (e.g.
169.254.169.254,metadata.google.internal), which on most hosts hand out credentials that unlock the whole environment. This PR closes the remaining holes in that guard: the OAuth discovery probes previously went out with no validation at all, IPv6 literals likehttps://[::1]/slipped past the IP classifier, several IPv6 encodings of internal IPv4 addresses (link-localfe80::1, hex-form mapped addresses like::ffff:a00:1, NAT64, 6to4) were not recognized as private, and a DNS-rebinding attacker could pass the pre-fetch check with a public address and then serve a private one to the actual connection.Changes
backend/src/lib/privateIp.ts: shared private/reserved IP classification, extracted from the inline helpers inbackend/src/lib/mcp/client.ts, with hardened IPv6 handling:fe80::/10link-local check — the previous/^fe[89ab]:/regex only matched the unrelated hextetfe8:and letfe80::1through::ffff:a00:1), not just the dotted form (::ffff:10.0.0.1)64:ff9b::/96) and 6to4 (2002::/16) addresses whose embedded IPv4 is privatebackend/src/lib/mcp/client.ts:guardedFetchnow pins the connection via a per-request undici dispatcher (pinnedGuardAgent) whose connect-time DNS lookup runs the private-IP guard and returns only validated addresses — the address we validate is the address we connect to, so there is no second, unguarded resolution for a DNS-rebinding attacker to race (TOCTOU). Host header and TLS SNI are untouched, so HTTPS still verifies normallybackend/src/lib/mcp/client.ts:validateRemoteMcpUrlnow strips the brackets URL parsing adds around IPv6 literals ([::1]) so they are classified by the private-IP guard instead of falling through to a DNS lookup; inline IP helpers replaced by the shared modulebackend/src/lib/mcp/oauth.ts: all four outbound OAuth call sites (metadatafetchJson, both discovery probes, dynamic client registration, token refresh) now go throughguardedFetch, so every outbound MCP request gets the same HTTPS-only / blocked-host / private-IP / connect-time-pinned / no-auto-redirect checks — the discovery probes were previously raw, unvalidatedfetchcalls, and token refresh did not disable redirectsbackend/src/lib/mcp/__tests__/client.ssrf.test.ts: 9 regression tests covering HTTPS enforcement, metadata/localhost host blocking, private IPv4/IPv6 literals, DNS resolution to private addresses, mixed public+private record sets, credential/fragment stripping, dispatcher pinning, and redirect disablingbackend/package.json/package-lock.json: addsundici@^6.27.0(lockfile regenerated by npm)backend/tsconfig.json: excludes*.test.ts/__tests__/from the productiontscbuildWhy
Connector URLs are user-supplied and therefore attacker-influenced; every gap above is a way to aim the backend's own network position at things only the backend can reach. This shrinks that security surface with pure validation logic on the existing egress path — zero new services and zero hosting cost. It adds exactly one dependency: undici — the engine behind Node's own fetch — pinned for connect-time DNS validation; used by the fork in production for exactly this purpose.
Testing
npm install && npm run buildinbackend/on this branch as committed: passes (tsc, no errors)upstream-pr/test-harness), which this branch deliberately does not include; with that branch merged locally:npx vitest run src/lib/mcp/__tests__/client.ssrf.test.ts— 9/9 passednpx vitest run(full backend suite) — 2 files, 21/21 passedProvenance
All changes are mechanical ports of code in amal66/mike@origin/main (commit b3166dd):
pinnedGuardAgent,guardedFetch, the IPv6-bracket handling, the OAuth call-site changes, and the test file are verbatim from the fork (apps/api/src/lib/mcp/client.ts,oauth.ts,__tests__/client.ssrf.test.ts),privateIp.tsfromapps/api/src/lib/privateIp.ts, the tsconfig excludes and theundicidependency line from the fork'sapps/apiconfig. Exceptions:privateIp.tsdoc comment and the classifier-relocation comment inclient.tsno longer mention the fork's OpenAI base-URL validation, which does not exist upstream.🤖 Generated with Claude Code
https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC