diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index 99baf91f..b4b07dd6 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -25,7 +25,6 @@ from nanobot.bus.events import ( ) from nanobot.security.network import ( PinnedDNSAsyncTransport, - pin_resolved_url_dns, resolve_url_target, validate_url_target, ) @@ -184,11 +183,11 @@ async def _probe_http_url(url: str, timeout: float = 3.0) -> bool: if not ok: return False try: - with pin_resolved_url_dns(url, resolved_ips): - reader, writer = await asyncio.wait_for( - asyncio.open_connection(host, port), - timeout=timeout, - ) + 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) diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index b8b7f971..3f76e986 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -118,10 +118,10 @@ def _resolve_url_safe(url: str) -> tuple[bool, str, tuple[str, ...]]: return resolve_url_target(url) -def _pinned_dns_transport(proxy: str | None = None) -> httpx.AsyncBaseTransport: +def _pinned_dns_transport() -> httpx.AsyncBaseTransport: from nanobot.security.network import PinnedDNSAsyncTransport - return PinnedDNSAsyncTransport(proxy=proxy) + return PinnedDNSAsyncTransport() async def _get_with_safe_redirects( @@ -963,11 +963,16 @@ 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(self.proxy), + transport=_pinned_dns_transport(), timeout=15.0, ) as client: r, stream, redirect_error = await _stream_with_safe_redirects( @@ -1040,7 +1045,7 @@ class WebFetchTool(Tool): try: async with httpx.AsyncClient( timeout=30.0, - transport=_pinned_dns_transport(self.proxy), + transport=_pinned_dns_transport(), ) as client: r, redirect_error = await _get_with_safe_redirects( client, diff --git a/nanobot/security/network.py b/nanobot/security/network.py index 3cb7df3c..3a31e97e 100644 --- a/nanobot/security/network.py +++ b/nanobot/security/network.py @@ -161,11 +161,10 @@ class PinnedDNSAsyncTransport(httpx.AsyncBaseTransport): self, *, allow_loopback: bool = False, - proxy: httpx.ProxyTypes | None = None, inner: httpx.AsyncBaseTransport | None = None, ) -> None: self._allow_loopback = allow_loopback - self._inner = inner or httpx.AsyncHTTPTransport(proxy=proxy) + self._inner = inner or httpx.AsyncHTTPTransport() async def handle_async_request(self, request: httpx.Request) -> httpx.Response: url = str(request.url) diff --git a/tests/tools/test_mcp_probe.py b/tests/tools/test_mcp_probe.py index 818895a7..b814ad80 100644 --- a/tests/tools/test_mcp_probe.py +++ b/tests/tools/test_mcp_probe.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio +import socket from unittest.mock import MagicMock, patch import pytest @@ -41,6 +42,15 @@ async def test_probe_uses_default_port_for_http(): assert await _probe_http_url("http://unreachable-host.test/mcp") is False +@pytest.mark.asyncio +async def test_probe_rejects_public_name_resolving_to_loopback(): + def _resolver(hostname, port, family=0, type_=0): + return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.1", 0))] + + with patch("nanobot.security.network.socket.getaddrinfo", _resolver): + assert await _probe_http_url("http://example.com:8765/mcp") is False + + # --------------------------------------------------------------------------- # connect_mcp_servers skips unreachable HTTP servers # --------------------------------------------------------------------------- diff --git a/tests/tools/test_web_fetch_security.py b/tests/tools/test_web_fetch_security.py index 7523f81b..71a45ab4 100644 --- a/tests/tools/test_web_fetch_security.py +++ b/tests/tools/test_web_fetch_security.py @@ -147,6 +147,19 @@ async def test_safe_redirect_requests_use_independent_pinned_dns_concurrently(mo assert calls == {"a.example": 2, "b.example": 2} +@pytest.mark.asyncio +async def test_web_fetch_rejects_proxy_because_upstream_dns_cannot_be_pinned(): + tool = WebFetchTool(proxy="http://proxy.example:8080") + + with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public): + result = await tool.execute(url="https://example.com/page") + + data = json.loads(result) + assert "error" in data + assert "proxy" in data["error"].lower() + assert "dns-pinned" in data["error"].lower() + + @pytest.mark.asyncio async def test_web_fetch_can_skip_jina_and_use_custom_user_agent(monkeypatch): tool = WebFetchTool(