From c5e053f83b107f9bb375a38b8c86458e7094bf87 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Tue, 7 Jul 2026 14:07:18 +0800 Subject: [PATCH] fix: pin validated DNS for SSRF-safe fetches maintainer edit: keep MCP HTTP SSRF checks strict, pin validated DNS for direct web_fetch and HTTP/SSE MCP requests, preserve explicit and environment proxy compatibility, and cover the proxy/redirect/rebinding cases with tests. --- docs/configuration.md | 7 +- nanobot/agent/tools/mcp.py | 43 ++++++--- nanobot/agent/tools/web.py | 50 +++++++--- nanobot/security/network.py | 68 ++++++++++++- tests/security/test_security_network.py | 22 +++++ tests/tools/test_mcp_probe.py | 61 ++++++++++++ tests/tools/test_mcp_tool.py | 97 +++++++++++++++++++ tests/tools/test_web_fetch_security.py | 122 +++++++++++++++++++++++- 8 files changed, 436 insertions(+), 34 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 59ae4818..35e15704 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1600,18 +1600,21 @@ nanobot uses a shared SSRF guard for built-in web fetches and HTTP/SSE MCP conne Keep whitelist entries as narrow as possible, such as a single host CIDR (`192.168.1.50/32`). The whitelist is global for the shared SSRF guard; it is not limited to one tool or one MCP server. +HTTP/SSE MCP connections use the same process-wide proxy environment behavior as `web_fetch`: proxied targets use the configured proxy, and URLs excluded by `NO_PROXY` remain DNS-pinned direct connections. + > [!TIP] -> Use `proxy` in `tools.web` to route all web requests (search + fetch) through a proxy: +> Use `proxy` in `tools.web` to route web requests through a proxy: > ```json > { "tools": { "web": { "proxy": "http://127.0.0.1:7890" } } } > ``` +> `web_fetch` applies DNS pinning for direct connections. When an explicit `tools.web.proxy` or a process-wide proxy environment variable applies to the target URL, nanobot still validates the requested URL locally, but DNS resolution for the outbound fetch happens at the proxy; configure only trusted proxies. URLs excluded by `NO_PROXY` keep the DNS-pinned direct path unless `tools.web.proxy` is configured. ### `tools.web` | Option | Type | Default | Description | |--------|------|---------|-------------| | `enable` | boolean | `true` | Enable or disable all built-in web tools (`web_search` + `web_fetch`) | -| `proxy` | string or null | `null` | Proxy for all web requests, for example `http://127.0.0.1:7890` | +| `proxy` | string or null | `null` | Proxy for web requests, for example `http://127.0.0.1:7890`. `web_fetch` DNS pinning applies only to direct connections; proxied fetches rely on the configured proxy as the trusted network exit. | | `userAgent` | string or null | `null` | User-Agent header for all web requests. If null, a browser one will be used | ### Web Search diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index b4b07dd6..53193a0d 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -25,6 +25,8 @@ from nanobot.bus.events import ( ) from nanobot.security.network import ( PinnedDNSAsyncTransport, + env_proxy_applies_to_url, + httpx_env_proxy_mounts, resolve_url_target, validate_url_target, ) @@ -179,21 +181,24 @@ async def _probe_http_url(url: str, timeout: float = 3.0) -> bool: port = parsed.port if not port: port = 443 if parsed.scheme == "https" else 80 - ok, _, resolved_ips = resolve_url_target(url, allow_loopback=True) + ok, _, resolved_ips = resolve_url_target(url) if not ok: return False - try: - target_host = resolved_ips[0] if resolved_ips else host - reader, writer = await asyncio.wait_for( - asyncio.open_connection(target_host, port), - timeout=timeout, - ) - writer.close() - with suppress(OSError, asyncio.TimeoutError): - await asyncio.wait_for(writer.wait_closed(), timeout=0.2) + if env_proxy_applies_to_url(url): return True - except (OSError, asyncio.TimeoutError): - return False + for target_host in resolved_ips or (host,): + try: + _reader, writer = await asyncio.wait_for( + asyncio.open_connection(target_host, port), + timeout=timeout, + ) + writer.close() + with suppress(OSError, asyncio.TimeoutError): + await asyncio.wait_for(writer.wait_closed(), timeout=0.2) + return True + except (OSError, asyncio.TimeoutError): + continue + return False def _redact_url(url: str) -> str: @@ -215,9 +220,17 @@ def _redact_url(url: str) -> str: return "" +def _pinned_transport_kwargs() -> dict[str, object]: + kwargs: dict[str, object] = {"transport": PinnedDNSAsyncTransport()} + mounts = httpx_env_proxy_mounts() + if mounts: + kwargs["mounts"] = mounts + return kwargs + + async def _validate_mcp_request_url(request: httpx.Request) -> None: """Validate each outgoing MCP HTTP request, including redirect targets.""" - ok, error = validate_url_target(str(request.url), allow_loopback=True) + ok, error = validate_url_target(str(request.url)) if not ok: raise httpx.RequestError( f"Blocked unsafe MCP URL {_redact_url(str(request.url))} ({error})", @@ -884,7 +897,7 @@ async def connect_mcp_servers( follow_redirects=True, timeout=timeout, auth=auth, - transport=PinnedDNSAsyncTransport(allow_loopback=True), + **_pinned_transport_kwargs(), ) read, write = await server_stack.enter_async_context( @@ -902,7 +915,7 @@ async def connect_mcp_servers( event_hooks={"request": [_validate_mcp_request_url]}, follow_redirects=True, timeout=httpx.Timeout(30.0, connect=10.0), - transport=PinnedDNSAsyncTransport(allow_loopback=True), + **_pinned_transport_kwargs(), ) ) read, write, _ = await server_stack.enter_async_context( diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index 3f76e986..bcae4bbd 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -124,6 +124,26 @@ def _pinned_dns_transport() -> httpx.AsyncBaseTransport: return PinnedDNSAsyncTransport() +def _fetch_client_kwargs(proxy: str | None, timeout: float) -> dict[str, Any]: + from nanobot.security.network import httpx_env_proxy_mounts + + kwargs: dict[str, Any] = {"timeout": timeout} + if proxy: + kwargs["proxy"] = proxy + else: + kwargs["transport"] = _pinned_dns_transport() + mounts = httpx_env_proxy_mounts() + if mounts: + kwargs["mounts"] = mounts + return kwargs + + +def _unsafe_url_request_error(exc: BaseException) -> str | None: + from nanobot.security.network import UnsafeURLRequestError + + return str(exc) if isinstance(exc, UnsafeURLRequestError) else None + + async def _get_with_safe_redirects( client: httpx.AsyncClient, url: str, @@ -136,7 +156,13 @@ async def _get_with_safe_redirects( if not is_valid: return None, f"Redirect blocked: {error_msg}" - response = await client.get(current_url, headers=headers, follow_redirects=False) + try: + response = await client.get(current_url, headers=headers, follow_redirects=False) + except httpx.RequestError as exc: + unsafe_error = _unsafe_url_request_error(exc) + if unsafe_error is not None: + return None, f"Redirect blocked: {unsafe_error}" + raise is_redirect = 300 <= response.status_code < 400 if not is_redirect: return response, None @@ -175,7 +201,13 @@ async def _stream_with_safe_redirects( headers=headers, follow_redirects=False, ) - response = await stream.__aenter__() + try: + response = await stream.__aenter__() + except httpx.RequestError as exc: + unsafe_error = _unsafe_url_request_error(exc) + if unsafe_error is not None: + return None, None, f"Redirect blocked: {unsafe_error}" + raise is_redirect = 300 <= response.status_code < 400 if not is_redirect: return response, stream, None @@ -963,17 +995,11 @@ class WebFetchTool(Tool): is_valid, error_msg = _validate_url_safe(url) if not is_valid: return json.dumps({"error": f"URL validation failed: {error_msg}", "url": url}, ensure_ascii=False) - if self.proxy: - return json.dumps({ - "error": "web_fetch proxy is incompatible with DNS-pinned SSRF protection", - "url": url, - }, ensure_ascii=False) # Detect and fetch images directly to avoid Jina's textual image captioning try: async with httpx.AsyncClient( - transport=_pinned_dns_transport(), - timeout=15.0, + **_fetch_client_kwargs(self.proxy, 15.0), ) as client: r, stream, redirect_error = await _stream_with_safe_redirects( client, @@ -995,6 +1021,9 @@ class WebFetchTool(Tool): if stream is not None: await stream.__aexit__(None, None, None) except Exception as e: + unsafe_error = _unsafe_url_request_error(e) + if unsafe_error is not None: + return json.dumps({"error": f"URL validation failed: {unsafe_error}", "url": url}, ensure_ascii=False) logger.debug("Pre-fetch image detection failed for {}: {}", url, e) result = None @@ -1044,8 +1073,7 @@ class WebFetchTool(Tool): """Local fallback using readability-lxml.""" try: async with httpx.AsyncClient( - timeout=30.0, - transport=_pinned_dns_transport(), + **_fetch_client_kwargs(self.proxy, 30.0), ) as client: r, redirect_error = await _get_with_safe_redirects( client, diff --git a/nanobot/security/network.py b/nanobot/security/network.py index 3a31e97e..9cc2e9ca 100644 --- a/nanobot/security/network.py +++ b/nanobot/security/network.py @@ -8,6 +8,7 @@ import re import socket from contextlib import contextmanager, suppress from urllib.parse import urlparse +from urllib.request import getproxies, proxy_bypass import httpx @@ -25,7 +26,6 @@ _BLOCKED_NETWORKS = [ ] _URL_RE = re.compile(r"https?://[^\s\"'`;|<>]+", re.IGNORECASE) - _allowed_networks: list[ipaddress.IPv4Network | ipaddress.IPv6Network] = [] @@ -113,6 +113,66 @@ def validate_url_target(url: str, *, allow_loopback: bool = False) -> tuple[bool return ok, error +def env_proxy_applies_to_url(url: str) -> bool: + """Return True when process proxy settings would proxy this URL.""" + try: + parsed = urlparse(url) + except Exception: + return False + if parsed.scheme not in {"http", "https"} or not parsed.hostname: + return False + + proxies = getproxies() + proxy_url = proxies.get(parsed.scheme) or proxies.get("all") + if not proxy_url: + return False + + host = parsed.hostname + if parsed.port is not None: + host = f"[{host}]:{parsed.port}" if ":" in host else f"{host}:{parsed.port}" + return not proxy_bypass(host) + + +def httpx_env_proxy_mounts() -> dict[str, httpx.AsyncBaseTransport | None]: + """Build HTTPX proxy mounts while leaving direct routes to the base transport.""" + proxies = getproxies() + mounts: dict[str, httpx.AsyncBaseTransport | None] = {} + for scheme in ("http", "https", "all"): + proxy_url = proxies.get(scheme) + if proxy_url: + if "://" not in proxy_url: + proxy_url = f"http://{proxy_url}" + mounts[f"{scheme}://"] = httpx.AsyncHTTPTransport(proxy=httpx.Proxy(proxy_url)) + + if not mounts: + return {} + + no_proxy = proxies.get("no", "") + if no_proxy == "*": + return {} + for entry in no_proxy.split(","): + pattern = _no_proxy_mount_pattern(entry.strip()) + if pattern: + mounts[pattern] = None + return mounts + + +def _no_proxy_mount_pattern(hostname: str) -> str | None: + if not hostname: + return None + if "://" in hostname: + return hostname + + unbracketed = hostname.strip("[]") + with suppress(ValueError): + addr = ipaddress.ip_address(unbracketed) + return f"all://[{addr}]" if addr.version == 6 else f"all://{addr}" + + if hostname.lower() == "localhost": + return "all://localhost" + return f"all://*{hostname}" + + @contextmanager def pin_resolved_url_dns(url: str, resolved_ips: tuple[str, ...]): """Pin DNS lookups for the URL hostname to previously validated IPs. @@ -152,6 +212,10 @@ def pin_resolved_url_dns(url: str, resolved_ips: tuple[str, ...]): socket.getaddrinfo = original_getaddrinfo +class UnsafeURLRequestError(httpx.RequestError): + """Raised when an outgoing request is rejected by URL safety validation.""" + + class PinnedDNSAsyncTransport(httpx.AsyncBaseTransport): """HTTPX transport that pins each request to the IPs validated for its URL.""" @@ -170,7 +234,7 @@ class PinnedDNSAsyncTransport(httpx.AsyncBaseTransport): url = str(request.url) ok, error, resolved_ips = resolve_url_target(url, allow_loopback=self._allow_loopback) if not ok: - raise httpx.RequestError(error, request=request) + raise UnsafeURLRequestError(error, request=request) async with self._resolver_lock: with pin_resolved_url_dns(url, resolved_ips): return await self._inner.handle_async_request(request) diff --git a/tests/security/test_security_network.py b/tests/security/test_security_network.py index 9584fb20..09ccbcf1 100644 --- a/tests/security/test_security_network.py +++ b/tests/security/test_security_network.py @@ -11,11 +11,21 @@ import pytest from nanobot.security.network import ( configure_ssrf_whitelist, contains_internal_url, + env_proxy_applies_to_url, + httpx_env_proxy_mounts, pin_resolved_url_dns, resolve_url_target, validate_url_target, ) +_PROXY_ENV_VARS = ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy") + + +@pytest.fixture(autouse=True) +def _clear_proxy_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in (*_PROXY_ENV_VARS, "NO_PROXY", "no_proxy"): + monkeypatch.delenv(name, raising=False) + def _fake_resolve(host: str, results: list[str]): """Return a getaddrinfo mock that maps the given host to fake IP results.""" @@ -184,6 +194,18 @@ def test_allows_normal_https(): assert ok +def test_env_proxy_helpers_respect_no_proxy(monkeypatch): + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "localhost,127.0.0.1,::1") + + assert env_proxy_applies_to_url("https://example.com/page") + assert not env_proxy_applies_to_url("http://localhost:8765/mcp") + + mounts = httpx_env_proxy_mounts() + assert any(transport is None for transport in mounts.values()) + assert any(transport is not None for transport in mounts.values()) + + # --------------------------------------------------------------------------- # contains_internal_url — shell command scanning # --------------------------------------------------------------------------- diff --git a/tests/tools/test_mcp_probe.py b/tests/tools/test_mcp_probe.py index b814ad80..20e30719 100644 --- a/tests/tools/test_mcp_probe.py +++ b/tests/tools/test_mcp_probe.py @@ -9,6 +9,16 @@ import pytest from nanobot.agent.tools.mcp import _probe_http_url, connect_mcp_servers from nanobot.agent.tools.registry import ToolRegistry +from nanobot.security.network import configure_ssrf_whitelist + +_PROXY_ENV_VARS = ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy") + + +@pytest.fixture(autouse=True) +def _clear_proxy_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in (*_PROXY_ENV_VARS, "NO_PROXY", "no_proxy"): + monkeypatch.delenv(name, raising=False) + # --------------------------------------------------------------------------- # _probe_http_url unit tests @@ -23,9 +33,11 @@ async def test_probe_returns_true_for_open_port(tmp_path): server = await asyncio.start_server(_close_connection, "127.0.0.1", 0) port = server.sockets[0].getsockname()[1] + configure_ssrf_whitelist(["127.0.0.1/32"]) try: assert await _probe_http_url(f"http://127.0.0.1:{port}/mcp") is True finally: + configure_ssrf_whitelist([]) server.close() await server.wait_closed() @@ -51,6 +63,55 @@ async def test_probe_rejects_public_name_resolving_to_loopback(): assert await _probe_http_url("http://example.com:8765/mcp") is False +@pytest.mark.asyncio +async def test_probe_skips_direct_tcp_when_global_proxy_env_is_set(monkeypatch): + def _resolver(hostname, port, family=0, type_=0): + return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))] + + async def _open_connection(*args, **kwargs): + raise AssertionError("global proxy env should skip direct TCP probe") + + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "localhost,127.0.0.1,::1") + monkeypatch.setattr("nanobot.agent.tools.mcp.asyncio.open_connection", _open_connection) + + with patch("nanobot.security.network.socket.getaddrinfo", _resolver): + assert await _probe_http_url("https://mcp.example.com/mcp") is True + + +@pytest.mark.asyncio +async def test_probe_tries_next_validated_ip_when_first_is_unreachable(monkeypatch): + attempts: list[tuple[str, int]] = [] + + class FakeWriter: + def close(self): + return None + + async def wait_closed(self): + return None + + def _resolver(hostname, port, family=0, type_=0): + return [ + (socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0)), + (socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.35", 0)), + ] + + async def _open_connection(host: str, port: int): + attempts.append((host, port)) + if host == "93.184.216.34": + raise OSError("first address unreachable") + return object(), FakeWriter() + + monkeypatch.setattr("nanobot.security.network.socket.getaddrinfo", _resolver) + monkeypatch.setattr("nanobot.agent.tools.mcp.asyncio.open_connection", _open_connection) + + assert await _probe_http_url("http://mcp.example:8765/mcp") is True + assert attempts == [ + ("93.184.216.34", 8765), + ("93.184.216.35", 8765), + ] + + # --------------------------------------------------------------------------- # connect_mcp_servers skips unreachable HTTP servers # --------------------------------------------------------------------------- diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 819550b7..e81a9d04 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -23,6 +23,8 @@ from nanobot.agent.tools.mcp import ( from nanobot.agent.tools.registry import ToolRegistry, is_tool_error_result from nanobot.config.schema import MCPServerConfig +_PROXY_ENV_VARS = ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy") + class _FakeTextContent: def __init__(self, text: str) -> None: @@ -50,6 +52,12 @@ def fake_mcp_runtime() -> dict[str, object | None]: return {"session": None} +@pytest.fixture(autouse=True) +def _clear_proxy_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in (*_PROXY_ENV_VARS, "NO_PROXY", "no_proxy"): + monkeypatch.delenv(name, raising=False) + + @pytest.fixture(autouse=True) def _fake_mcp_module( monkeypatch: pytest.MonkeyPatch, fake_mcp_runtime: dict[str, object | None] @@ -712,6 +720,8 @@ async def test_connect_mcp_servers_logs_stdio_pollution_hint( @pytest.mark.parametrize( "config", [ + MCPServerConfig(url="http://127.0.0.1:9/sse"), + MCPServerConfig(type="streamableHttp", url="http://127.0.0.1:9/mcp"), MCPServerConfig(url="http://169.254.169.254/sse"), MCPServerConfig(type="streamableHttp", url="http://169.254.169.254/mcp"), ], @@ -742,6 +752,93 @@ async def test_connect_mcp_servers_rejects_unsafe_http_urls_before_probe( assert any("blocked unsafe URL" in warning for warning in warnings) +@pytest.mark.asyncio +async def test_validate_mcp_request_url_rejects_loopback_without_whitelist() -> None: + from nanobot.security.network import configure_ssrf_whitelist + + configure_ssrf_whitelist([]) + request = httpx.Request("GET", "http://127.0.0.1/private") + + with pytest.raises(httpx.RequestError, match="Blocked unsafe MCP URL"): + await mcp_mod._validate_mcp_request_url(request) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "config", + [ + MCPServerConfig(type="sse", url="https://mcp.example.com/sse"), + MCPServerConfig(type="streamableHttp", url="https://mcp.example.com/mcp"), + ], +) +async def test_connect_mcp_servers_env_proxy_adds_proxy_mounts_and_keeps_pinned_transport( + config: MCPServerConfig, + fake_mcp_runtime: dict[str, object | None], + monkeypatch: pytest.MonkeyPatch, +) -> None: + fake_mcp_runtime["session"] = _make_fake_session(["demo"]) + client_kwargs: list[dict[str, object]] = [] + + async def _reachable(_url: str) -> bool: + return True + + def _validate(_url: str) -> tuple[bool, str]: + return True, "" + + class FakeAsyncClient: + def __init__(self, *args: object, **kwargs: object) -> None: + client_kwargs.append(kwargs) + + async def __aenter__(self) -> object: + return self + + async def __aexit__(self, exc_type: object, exc: object, tb: object) -> bool: + return False + + @asynccontextmanager + async def _capturing_sse_client(_url: str, httpx_client_factory=None): + assert httpx_client_factory is not None + async with httpx_client_factory(): + pass + yield object(), object() + + @asynccontextmanager + async def _capturing_streamable_http_client(_url: str, http_client=None): + assert http_client is not None + yield object(), object(), object() + + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "localhost,127.0.0.1,::1") + monkeypatch.setattr(mcp_mod, "validate_url_target", _validate) + monkeypatch.setattr(mcp_mod, "_probe_http_url", _reachable) + monkeypatch.setattr(mcp_mod.httpx, "AsyncClient", FakeAsyncClient) + monkeypatch.setattr(sys.modules["mcp.client.sse"], "sse_client", _capturing_sse_client) + monkeypatch.setattr( + sys.modules["mcp.client.streamable_http"], + "streamable_http_client", + _capturing_streamable_http_client, + ) + + registry = ToolRegistry() + stacks = await connect_mcp_servers({"remote": config}, registry) + for stack in stacks.values(): + await stack.aclose() + + assert client_kwargs + assert all("transport" in kwargs for kwargs in client_kwargs) + assert all("mounts" in kwargs for kwargs in client_kwargs) + + +def test_mcp_http_clients_no_proxy_env_keeps_pinned_direct_route(monkeypatch): + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "mcp.example.com") + + kwargs = mcp_mod._pinned_transport_kwargs() + + assert "transport" in kwargs + assert any(transport is None for transport in kwargs["mounts"].values()) + + @pytest.mark.asyncio @pytest.mark.parametrize( ("config", "expected_transport"), diff --git a/tests/tools/test_web_fetch_security.py b/tests/tools/test_web_fetch_security.py index 71a45ab4..ab9bbe9f 100644 --- a/tests/tools/test_web_fetch_security.py +++ b/tests/tools/test_web_fetch_security.py @@ -21,6 +21,13 @@ from nanobot.security.workspace_access import ( ) _REAL_GETADDRINFO = socket.getaddrinfo +_PROXY_ENV_VARS = ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy") + + +@pytest.fixture(autouse=True) +def _clear_proxy_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in (*_PROXY_ENV_VARS, "NO_PROXY", "no_proxy"): + monkeypatch.delenv(name, raising=False) def _fake_resolve_private(hostname, port, family=0, type_=0): @@ -31,6 +38,49 @@ def _fake_resolve_public(hostname, port, family=0, type_=0): return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))] +def _patch_web_fetch_fake_client(monkeypatch: pytest.MonkeyPatch) -> list[dict]: + client_kwargs: list[dict] = [] + + class FakeStreamResponse: + status_code = 200 + headers = {"content-type": "text/html"} + url = "https://example.com/page" + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + class FakeJinaResponse: + status_code = 200 + + def raise_for_status(self): + return None + + def json(self): + return {"data": {"title": "Example", "content": "Hello", "url": "https://example.com/page"}} + + class FakeClient: + def __init__(self, *args, **kwargs): + client_kwargs.append(kwargs) + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + def stream(self, method, url, headers=None, **kwargs): + return FakeStreamResponse() + + async def get(self, url, headers=None, **kwargs): + return FakeJinaResponse() + + monkeypatch.setattr(web_module.httpx, "AsyncClient", FakeClient) + return client_kwargs + + @pytest.mark.asyncio async def test_web_fetch_blocks_private_ip(): tool = WebFetchTool() @@ -148,16 +198,80 @@ async def test_safe_redirect_requests_use_independent_pinned_dns_concurrently(mo @pytest.mark.asyncio -async def test_web_fetch_rejects_proxy_because_upstream_dns_cannot_be_pinned(): - tool = WebFetchTool(proxy="http://proxy.example:8080") +async def test_web_fetch_proxy_remains_supported(monkeypatch): + tool = WebFetchTool(proxy="http://config-proxy.example:7890") + client_kwargs = _patch_web_fetch_fake_client(monkeypatch) + + monkeypatch.setenv("HTTPS_PROXY", "http://env-proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "example.com") with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public): result = await tool.execute(url="https://example.com/page") + data = json.loads(result) + assert data["extractor"] == "jina" + assert all(kwargs["proxy"] == "http://config-proxy.example:7890" for kwargs in client_kwargs) + assert all("mounts" not in kwargs for kwargs in client_kwargs) + assert all("transport" not in kwargs for kwargs in client_kwargs) + + +@pytest.mark.asyncio +async def test_web_fetch_env_proxy_adds_proxy_mounts_and_keeps_pinned_transport(monkeypatch): + tool = WebFetchTool() + client_kwargs = _patch_web_fetch_fake_client(monkeypatch) + + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "localhost,127.0.0.1,::1") + + with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public): + result = await tool.execute(url="https://example.com/page") + + data = json.loads(result) + assert data["extractor"] == "jina" + fetch_kwargs = [kwargs for kwargs in client_kwargs if kwargs.get("timeout") == 15.0] + assert fetch_kwargs + assert all("transport" in kwargs for kwargs in fetch_kwargs) + assert all("mounts" in kwargs for kwargs in fetch_kwargs) + + +def test_web_fetch_no_proxy_env_keeps_pinned_direct_route(monkeypatch): + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.example:8080") + monkeypatch.setenv("NO_PROXY", "example.com") + + kwargs = web_module._fetch_client_kwargs(None, 15.0) + + assert "transport" in kwargs + assert any(transport is None for transport in kwargs["mounts"].values()) + + +@pytest.mark.asyncio +async def test_web_fetch_does_not_fallback_after_pinned_dns_rebind_rejection(monkeypatch): + calls = {"evil.example": 0} + + def _rebinding_resolver(hostname, port, family=0, type_=0, proto=0, flags=0): + host = str(hostname).rstrip(".").lower() + calls[host] += 1 + ip = "93.184.216.34" if calls[host] <= 2 else "169.254.169.254" + return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", (ip, 0))] + + tool = WebFetchTool() + + async def _unexpected_jina(*args, **kwargs): + raise AssertionError("Jina fallback should not run after an SSRF rejection") + + async def _unexpected_readability(*args, **kwargs): + raise AssertionError("Readability fallback should not run after an SSRF rejection") + + monkeypatch.setattr(tool, "_fetch_jina", _unexpected_jina) + monkeypatch.setattr(tool, "_fetch_readability", _unexpected_readability) + + with patch("nanobot.security.network.socket.getaddrinfo", _rebinding_resolver): + result = await tool.execute(url="http://evil.example/page") + data = json.loads(result) assert "error" in data - assert "proxy" in data["error"].lower() - assert "dns-pinned" in data["error"].lower() + assert "blocked" in data["error"].lower() + assert calls["evil.example"] == 3 @pytest.mark.asyncio