fix(mcp): replace single-IP pinning with validate-at-connect SSRF guard by waleedlatif1 · Pull Request #5823 · simstudioai/sim

waleedlatif1 · 2026-07-21T22:54:47Z

Summary

Replaces the MCP transport + OAuth guard's single-IP pinning with the standard validate-at-connect SSRF pattern (LibreChat getSSRFConnect): DNS resolves normally, and every socket connect filters the resolved addresses against the private/reserved blocklist. No reference MCP client pins IPs; pinning welded each attempt to one address with no fallback and is the prime suspect in the headers-then-zero-body stalls (failed first attempt → instant retry via resolver rotation, 50/50 rotation proven).
Adversarially security-reviewed before shipping (dedicated red-team pass with live undici tests). Verdict: the hostname rebinding/TOCTOU window is genuinely closed (proven), IPv6 embedded-private forms all blocked (NAT64/6to4/Teredo/v4-mapped verified against isPrivateOrReservedIP). Two findings — both closed in this PR:
1. IP-literal redirect bypass (shared by the old pin, not a regression): Node skips a custom lookup for numeric hosts, so a 3xx to http://169.254.169.254/ dialed directly. Redirects are now followed manually with per-hop validation (protocol + IP-literal checks, 5-hop cap).
2. Cross-origin header retention: undici keeps custom headers (e.g. x-api-key) across cross-origin redirects. Custom headers are now dropped on any cross-origin hop.
Policy gating unchanged: guard activates exactly where the pin did (validateMcpServerSsrf non-null); allowlist mode / localhost-on-self-hosted remain unguarded; OAuth legs keep per-hop up-front validation. Non-MCP pinned-fetch consumers (LLM providers, A2A, probe) untouched.

Type of Change

Bug fix / security hardening

Testing

397 tests green across the MCP + security suites, including new coverage: rebinding (public→private flip blocked on second connect), mixed private+public DNS answers filtered, redirect to metadata IP-literal blocked, bracketed IPv6 literal blocked, non-http(s) protocols blocked, hop cap, 303-POST→GET and 307 semantics, cross-origin header stripping, same-origin header retention.
tsc + biome clean.

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)

Swaps the MCP transport + OAuth guard from pin-one-resolved-IP to the standard pattern (LibreChat getSSRFConnect): DNS resolves normally and EVERY socket connect filters the resolved addresses against the private/reserved blocklist, closing the validate-then-trust window a rebind could race and removing the non-standard mechanism implicated in the headers-then-no-body stalls (no reference MCP client pins IPs; a pinned attempt welded to a bad flow cannot escape, while retries rotate via resolver round-robin and succeed). Adversarially reviewed before shipping; both findings closed here: - Redirects are now followed manually with per-hop validation: an IP-literal redirect target (which bypasses ANY connect-time lookup - Node skips the custom lookup for numeric hosts) is checked explicitly, closing a metadata- endpoint redirect hole the old pin also had. - Custom request headers are dropped on cross-origin hops, so a redirect to a second attacker host cannot harvest configured auth headers. Policy gating unchanged: the guard activates exactly where the pin did (validateMcpServerSsrf non-null; allowlist mode / localhost-on-self-hosted stay unguarded). Non-MCP consumers of the pinned fetch are untouched. 397 tests incl. new rebinding, mixed-answer, IP-literal-redirect, and hop-cap coverage.

vercel · 2026-07-21T22:54:51Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 21, 2026 11:52pm

cursor · 2026-07-21T22:54:57Z

PR Summary

High Risk
Changes security-critical outbound networking for user-controlled MCP hosts, including redirect and header semantics; regressions could break connections or weaken SSRF defenses.

Overview
Replaces single-IP pinning for MCP transport and OAuth outbound HTTP with a validate-at-connect SSRF model: DNS resolves normally, and each socket connect filters addresses against the private/reserved blocklist via createSsrfGuardedLookup, keeping multiple public addresses for fallback instead of welding to one IP.

Adds createSsrfGuardedFetchWithDispatcher and followRedirectsGuarded so redirects are followed manually with per-hop checks—blocking IP-literal targets (which bypass custom lookup), non-http(s) protocols, a 5-hop cap, cross-origin custom header stripping, and refusal to forward a request body on cross-origin 307/308.

McpClient and createSsrfGuardedMcpFetch now use the guarded connector for public resolutions; self-hosted private/loopback resolutions keep createPinnedPrivateMcpFetch / legacy pin so permitted local aliases still connect. LLM and other pinned-fetch consumers are unchanged.

^{Reviewed by Cursor Bugbot for commit 8ada731. Configure here.}

greptile-apps · 2026-07-21T22:57:22Z

Greptile Summary

This PR replaces single-address MCP pinning with connection-time SSRF checks. The main changes are:

Filters every DNS result before each guarded socket connection.
Follows redirects manually with protocol, address, and hop validation.
Removes sensitive headers and blocks request bodies on cross-origin redirects.
Preserves private-address pinning for permitted self-hosted deployments.
Adds coverage for rebinding, redirects, request options, and transport cleanup.

Confidence Score: 5/5

This looks safe to merge.

The Request-input fix preserves method, headers, body, and signal.
Bodyless redirect transitions remove the relevant entity headers.
No blocking issue remains in the changed paths reviewed.

Important Files Changed

Filename	Overview
apps/sim/lib/core/security/input-validation.server.ts	Adds connection-time DNS filtering, guarded redirect handling, and preservation of Request options.
apps/sim/lib/mcp/pinned-fetch.ts	Uses guarded connections for public addresses while retaining private-address pinning for permitted self-hosted targets.
apps/sim/lib/mcp/client.ts	Selects the guarded or private-pinned transport and retains idempotent Agent cleanup.
apps/sim/lib/core/security/ssrf-guarded-lookup.test.ts	Covers rebinding, mixed DNS answers, redirect validation, redirect semantics, and cross-origin protections.

_{Reviews (5): Last reviewed commit: "fix(mcp): pin (not skip) self-hosted pri..." | Re-trigger Greptile}

…lower - Restore validateUrlWithDNS's prefer-IPv4 resolution (a stale working-tree hunk accidentally reverted #5798 for the still-pinned non-MCP consumers). - Validate the INITIAL url's IP-literal in followRedirectsGuarded, not just redirect hops, so the exported guard is self-contained. - Drop entity headers (content-length/type/encoding) when a 301/302/303 switches a POST to a bodyless GET, which undici would otherwise reject. - Lift method/headers/body/signal from a Request input instead of silently downgrading a guarded POST Request to a bare GET.

waleedlatif1 · 2026-07-21T23:12:28Z

@greptile

waleedlatif1 · 2026-07-21T23:12:29Z

@cursor review

…e throw paths - Add the missing double-cast-allowed annotation on the sanitized-headers cast (strict boundary audit). - Cancel the redirect response body before the hop-cap / blocked-target throws so those paths can't leave a socket checked out on the long-lived Agent.

waleedlatif1 · 2026-07-21T23:26:23Z

@greptile

waleedlatif1 · 2026-07-21T23:26:24Z

@cursor review

… parity) A DNS alias resolving to loopback/private is only reachable on self-hosted, where the policy explicitly permits it; the guarded lookup would filter the address and strand the connect where the old pin connected. Both MCP gates (transport + OAuth guard) now route private/loopback resolutions over the unguarded path, same as the localhost carve-out. Test IPs moved off RFC-5737 TEST-NET (which is correctly classified reserved).

waleedlatif1 · 2026-07-21T23:37:42Z

@greptile

waleedlatif1 · 2026-07-21T23:37:43Z

@cursor review

…s-origin body forwards - The private/loopback carve-out now keeps the LEGACY PIN to the validated address instead of falling back to unguarded fetch — preserving both the old behavior and its anti-rebinding property for self-hosted DNS aliases. - Cross-origin redirect hops now also refuse to forward a request body (307/308 preserve method+body; post-pin those redirects really dial the new origin, so an open redirect could exfiltrate OAuth client secrets). Bodyless cross-origin redirects still follow. Tests for both.

waleedlatif1 · 2026-07-21T23:46:06Z

@greptile

waleedlatif1 · 2026-07-21T23:46:08Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 8ada731. Configure here.}

vercelBottemporarily deployed to Preview July 21, 2026 22:54 Inactive