From 341f8b4d935407af920014827ebcffcea37293ac Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:32:37 -0700 Subject: [PATCH] fix: cap, loopback-bypass and share the MCP proxy mounts Review follow-up on the MCP HTTP proxy PR: - Proxy mounts win over transport= for matching URLs, so a bare AsyncHTTPTransport mount bypassed the 10 MiB wire-body cap whenever a proxy applied. Each mount is now wrapped in _make_mcp_body_cap_transport. - Loopback MCP servers (127.0.0.1 / ::1 / localhost) were dialed through HTTP_PROXY unless NO_PROXY covered them; _mcp_proxy_mounts now returns None for is_loopback_host (agent.proxy_bypass rule). - Dropped the fail-open try/except around the proxy transport construction; a proxy httpx cannot build surfaces as the server's connect error. - The content-type preflight client now takes an explicit transport plus the same proxy mounts as the SDK client, so probe and handshake take the same route (no httpx env auto-detection divergence). --- tests/tools/test_mcp_http_proxy.py | 25 ++++++++++++++++++++++++- tools/mcp_tool_transport.py | 28 +++++++++++++++++----------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/tests/tools/test_mcp_http_proxy.py b/tests/tools/test_mcp_http_proxy.py index 1717d9c852..3550de5ba4 100644 --- a/tests/tools/test_mcp_http_proxy.py +++ b/tests/tools/test_mcp_http_proxy.py @@ -44,7 +44,11 @@ def test_proxy_env_becomes_a_mount_and_no_proxy_stays_direct(env_only_proxy, mon monkeypatch.setenv("HTTPS_PROXY", PROXY) mounts = _mcp_proxy_mounts(httpx, URL, True, None) - assert set(mounts) == {"https://"} and isinstance(mounts["https://"], httpx.AsyncHTTPTransport) + # The mount wins over transport= for matching URLs, so it must carry the wire-body cap itself. + assert set(mounts) == {"https://"} and type(mounts["https://"]).__name__ == "_BodyCapTransport" + + monkeypatch.setenv("HTTP_PROXY", PROXY) # loopback is never dialed through a proxy, NO_PROXY or not + assert _mcp_proxy_mounts(httpx, "http://127.0.0.1:5000/mcp", True, None) is None monkeypatch.setenv("NO_PROXY", "mcp.example.com") assert _mcp_proxy_mounts(httpx, URL, True, None) is None @@ -107,3 +111,22 @@ def test_both_client_builders_carry_proxy_mounts_next_to_the_body_cap(env_only_p assert captured["mounts"]["https://"] is not None assert captured["headers"] == {"X-Test": "1"} # SDK passthrough intact assert captured["transport"] is not None + + +def test_preflight_probe_uses_the_same_proxy_mounts_as_the_connect_client(env_only_proxy, monkeypatch): + """The content-type preflight must reach the server the way the SDK client will: explicit mounts + (repo NO_PROXY/loopback rules), not httpx's own env auto-detection.""" + import httpx + + monkeypatch.setenv("HTTPS_PROXY", PROXY) + from tools.mcp_tool import MCPServerTask + + class _Probe(_RecordingClient): + async def head(self, *a, **k): + raise httpx.ConnectError("stub") + + with patch.object(httpx, "AsyncClient", _Probe): + asyncio.run(MCPServerTask("remote")._preflight_content_type(URL, timeout=1.0)) + captured = _Probe.captured + assert type(captured["mounts"]["https://"]).__name__ == "_BodyCapTransport" + assert captured["transport"] is not None # explicit transport: httpx env proxy auto-detection is off diff --git a/tools/mcp_tool_transport.py b/tools/mcp_tool_transport.py index b2896ee17f..ce3c664487 100644 --- a/tools/mcp_tool_transport.py +++ b/tools/mcp_tool_transport.py @@ -10,7 +10,7 @@ import urllib.request from contextlib import asynccontextmanager from typing import Dict, Optional, Set from utils import normalize_proxy_url -from agent.proxy_bypass import should_bypass_proxy +from agent.proxy_bypass import is_loopback_host, should_bypass_proxy from tools.mcp_tool_errors import NonMcpEndpointError, _apply_identity_header, _handshake_rejected_as_modern, _is_streamable_http_rejection, _make_mcp_body_cap_transport, _make_redirect_header_stripper, _resolve_client_cert, _unwrap_exception_group from tools.mcp_tool_lifecycle import _filter_mcp_children, _orphan_stdio_pid_servers, _orphan_stdio_pids, _stdio_pgids, _stdio_pids from tools.mcp_tool_common import _core @@ -54,10 +54,15 @@ def _mcp_proxy_mounts(httpx_mod, url: str, ssl_verify, client_cert, server_name: NO_PROXY goes through ``agent.proxy_bypass.should_bypass_proxy`` — the one matcher the LLM transport and the gateway adapters use (CIDR ranges and ``*.host`` forms the stdlib check does not understand) — plus ``urllib.request.proxy_bypass`` for the OS bypass list - (Windows ``ProxyOverride`` / macOS exceptions). + (Windows ``ProxyOverride`` / macOS exceptions). Loopback is never dialed through a proxy + (``agent.proxy_bypass.is_loopback_host``), NO_PROXY or not. + + A mount wins over ``transport=`` for the URLs it matches, so each proxy transport is wrapped in + the same wire-body cap as the direct one. A proxy the installed httpx cannot build (e.g. + ``socks://`` without socksio) raises here and surfaces as this server's connect error. """ host = urllib.parse.urlsplit(url).hostname or "" - if not host or should_bypass_proxy(url) or urllib.request.proxy_bypass(host): + if not host or is_loopback_host(host) or should_bypass_proxy(url) or urllib.request.proxy_bypass(host): return None proxies = urllib.request.getproxies() mounts: dict = {} @@ -65,12 +70,9 @@ def _mcp_proxy_mounts(httpx_mod, url: str, ssl_verify, client_cert, server_name: proxy_url = normalize_proxy_url(proxies.get(scheme) or proxies.get("all")) if not proxy_url: continue - try: - # verify/cert apply to the CONNECT+TLS leg, so the proxy transport needs its own copy. - mounts[f"{scheme}://"] = httpx_mod.AsyncHTTPTransport( - proxy=proxy_url, verify=ssl_verify, **_present(cert=client_cert)) - except Exception as exc: - logger.warning("MCP server '%s': cannot use %s proxy %s: %s", server_name, scheme, proxy_url, exc) + # verify/cert apply to the CONNECT+TLS leg, so the proxy transport needs its own copy. + mounts[f"{scheme}://"] = _make_mcp_body_cap_transport(httpx_mod, httpx_mod.AsyncHTTPTransport( + proxy=proxy_url, verify=ssl_verify, **_present(cert=client_cert))) return mounts or None @@ -311,9 +313,13 @@ class MCPServerTransportMixin: ct = _content_type_base(resp) return _is_2xx(resp) and bool(ct) and ct not in self._MCP_CONTENT_TYPES probe_headers = dict(headers) if headers else {} + # Same route as the SDK client: TLS on an explicit transport (which also turns off httpx's own + # env proxy auto-detection) plus the repo's proxy mounts, so the probe and the handshake agree. + probe_transport = _httpx.AsyncHTTPTransport(verify=ssl_verify, **_present(cert=client_cert)) try: - async with _httpx.AsyncClient(verify=ssl_verify, follow_redirects=True, timeout=_httpx.Timeout(timeout), - **_present(cert=client_cert)) as client: + async with _httpx.AsyncClient( + follow_redirects=True, timeout=_httpx.Timeout(timeout), transport=probe_transport, + **_present(mounts=_mcp_proxy_mounts(_httpx, url, ssl_verify, client_cert, self.name))) as client: resp = await client.head(url, headers=probe_headers) # cheapest; GET on 405/501 if resp.status_code in (405, 501): resp = await client.get(url, headers=probe_headers)