security: SSRF guardrails for server-side connector fetches

⛔ closed · #223 · open-legal-products/mike ← amal66/mike · opened 3mo ago by amal66 · closed 3mo ago · +337-49 across 7 files · ↗ on GitHub

From the PR description

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 like https://[::1]/ slipped past the IP classifier, several IPv6 encodings of internal IPv4 addresses (link-local fe80::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

  • New backend/src/lib/privateIp.ts: shared private/reserved IP classification, extracted from the inline helpers in backend/src/lib/mcp/client.ts, with hardened IPv6 handling:
    • fixes the fe80::/10 link-local check - the previous /^fe[89ab]:/ regex only matched the unrelated hextet fe8: and let fe80::1 through
    • blocks hex-form IPv4-mapped addresses (::ffff:a00:1), not just the dotted form (::ffff:10.0.0.1)
    • blocks NAT64 (64:ff9b::/96) and 6to4 (2002::/16) addresses whose embedded IPv4 is private
    • fails closed: unparseable input is treated as blocked
  • backend/src/lib/mcp/client.ts: guardedFetch now 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 normally
  • backend/src/lib/mcp/client.ts: validateRemoteMcpUrl now 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 module
  • backend/src/lib/mcp/oauth.ts: all four outbound OAuth call sites (metadata fetchJson, both discovery probes, dynamic client registration, token refresh) now go through guardedFetch, 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, unvalidated fetch calls, and token refresh did not disable redirects
  • New backend/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 disabling
  • backend/package.json / package-lock.json: adds undici@^6.27.0 (lockfile regenerated by npm)
  • backend/tsconfig.json: excludes *.test.ts / __tests__/ from the production tsc build

Why

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 build in backend/ on this branch as committed: passes (tsc, no errors)
  • Tests run atop the test-harness PR (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 passed
    • npx vitest run (full backend suite) - 2 files, 21/21 passed

Provenance

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.ts from apps/api/src/lib/privateIp.ts, the tsconfig excludes and the undici dependency line from the fork's apps/api config. Exceptions:

  • Two comments were trimmed where they referenced fork-only code: the privateIp.ts doc comment and the classifier-relocation comment in client.ts no longer mention the fork's OpenAI base-URL validation, which does not exist upstream.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC

Our analysis

Harden MCP connector egress against SSRF — read the full analysis →

Think the analysis missed something the PR description covers?

Capture this PR into my fork

Download a Markdown prompt that tells Claude how to port every commit in this PR into your working tree. Run it via claude -p < capture-pull-223.md from inside the repo you want the changes in.

⬇ Download capture-pull-223.md