fix(mcp-store): connect to validated IPs for upstream MCP calls - #95848
Open
AUTHENSOR wants to merge 1 commit into
Open
fix(mcp-store): connect to validated IPs for upstream MCP calls#95848AUTHENSOR wants to merge 1 commit into
AUTHENSOR wants to merge 1 commit into
Conversation
The MCP store validated each upstream URL (check_mcp_url_policy) and then opened the connection with a plain httpx.Client, which re-resolved the hostname at connect time. A DNS record that answers the validator with a public IP and the connector with an internal one slipped the org's upstream credential past the check. Add ValidatingPinnedTransport, which runs validate_url_and_pin_ips per request at connect time and rewrites the connection to the validated IP, mirroring pinned_requests used elsewhere.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Adding an MCP server means the store fetches from a member-supplied URL. Before connecting,
check_mcp_url_policyvalidates the URL and resolves the hostname — then the connection itself is opened by a plainhttpx.Client, which resolves the hostname a second time.Two resolutions of the same attacker-controlled hostname is a DNS-rebinding window. A DNS record can answer the validator with a public IP and answer the actual connect with
127.0.0.1, a cluster-internal address, or a cloud metadata IP.When that happens, the proxy POSTs the org's upstream credential to the internal host and streams the response back. From a pre-fix repro of this chain, the internal listener captured:
The repo already documents this exact requirement in
posthog/security/pinned_requests.py:validate_url_and_pin_ipsalone leaves a TOCTOU window, and callers that open a connection afterwards must use the returned IPs viaPinnedIPAdapterinstead of re-resolving DNS. Teams webhook delivery, CIMD, and business_knowledge do this. The MCP store (proxy.py, and the same shape twice in tools.py) did not.Changes
ValidatingPinnedTransportinproducts/mcp_store/backend/pinned_transport.py: anhttpx.HTTPTransportsubclass that runs the repo's ownvalidate_url_and_pin_ipsat connect time, for every request through the client, and rewrites the connection to the validated IP so httpcore never re-resolves the hostname.SSRFBlockedErrormaps to a 400{"error": "URL not allowed: …"}in the proxy andToolsFetchError/ToolCallErrorin tools, matching the existing error surface.Hostheader keeps the original netloc andsni_hostnamekeeps TLS verification on the original hostname.select_pinned_ip(the same chooser the requests-side machinery uses). An empty pin set (dev-mode SSRF bypass) passes through untouched, and the operator's internal allowlist is honored, both matchingPinnedIPAdapter's posture.proxy_mcp_request,fetch_upstream_tools,call_upstream_tool. The transport is passed where thehttpx.Clientis constructed, so existing tests' patch targets and timeout-kwarg assertions stay intact. Thecheck_mcp_url_policypreflights are kept as fast-fail defense in depth.presentation/views.py,presentation/agent_views.py, and gateway tasks are unaffected.How did you test this code?
test_pinned_transport.py, mirroring the conventions ofposthog/security/test/test_pinned_requests.py: boundary patches ofvalidate_url_and_pin_ips, host-rewrite/Host/SNI assertions for both IPv4 and IPv6, and a live-socket test asserting the connection lands on the pinned IP.ssrf_blocked_at_connectrow intest_proxy.py's parameterized error-mapping test.ruff checkandruff formatpass on the touched files at the repo-pinned version.Automatic notifications
Docs update
None. This is an internal hardening change with no user-facing behavior change for allowed URLs.
🤖 Agent context
Autonomy: Fully autonomous
Found during an authorized security review of the MCP store's outbound request path; this PR ships only the fix. The key decision was pinning at connect time inside an
httpx.HTTPTransportrather than pre-resolving at the call sites, so validation cannot be raced by a second resolution, and every redirect hop is covered without new per-call-site state.