fix: reject proxied pinned web fetches
This commit is contained in:
@@ -25,7 +25,6 @@ from nanobot.bus.events import (
|
|||||||
)
|
)
|
||||||
from nanobot.security.network import (
|
from nanobot.security.network import (
|
||||||
PinnedDNSAsyncTransport,
|
PinnedDNSAsyncTransport,
|
||||||
pin_resolved_url_dns,
|
|
||||||
resolve_url_target,
|
resolve_url_target,
|
||||||
validate_url_target,
|
validate_url_target,
|
||||||
)
|
)
|
||||||
@@ -184,11 +183,11 @@ async def _probe_http_url(url: str, timeout: float = 3.0) -> bool:
|
|||||||
if not ok:
|
if not ok:
|
||||||
return False
|
return False
|
||||||
try:
|
try:
|
||||||
with pin_resolved_url_dns(url, resolved_ips):
|
target_host = resolved_ips[0] if resolved_ips else host
|
||||||
reader, writer = await asyncio.wait_for(
|
reader, writer = await asyncio.wait_for(
|
||||||
asyncio.open_connection(host, port),
|
asyncio.open_connection(target_host, port),
|
||||||
timeout=timeout,
|
timeout=timeout,
|
||||||
)
|
)
|
||||||
writer.close()
|
writer.close()
|
||||||
with suppress(OSError, asyncio.TimeoutError):
|
with suppress(OSError, asyncio.TimeoutError):
|
||||||
await asyncio.wait_for(writer.wait_closed(), timeout=0.2)
|
await asyncio.wait_for(writer.wait_closed(), timeout=0.2)
|
||||||
|
|||||||
@@ -118,10 +118,10 @@ def _resolve_url_safe(url: str) -> tuple[bool, str, tuple[str, ...]]:
|
|||||||
return resolve_url_target(url)
|
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
|
from nanobot.security.network import PinnedDNSAsyncTransport
|
||||||
|
|
||||||
return PinnedDNSAsyncTransport(proxy=proxy)
|
return PinnedDNSAsyncTransport()
|
||||||
|
|
||||||
|
|
||||||
async def _get_with_safe_redirects(
|
async def _get_with_safe_redirects(
|
||||||
@@ -963,11 +963,16 @@ class WebFetchTool(Tool):
|
|||||||
is_valid, error_msg = _validate_url_safe(url)
|
is_valid, error_msg = _validate_url_safe(url)
|
||||||
if not is_valid:
|
if not is_valid:
|
||||||
return json.dumps({"error": f"URL validation failed: {error_msg}", "url": url}, ensure_ascii=False)
|
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
|
# Detect and fetch images directly to avoid Jina's textual image captioning
|
||||||
try:
|
try:
|
||||||
async with httpx.AsyncClient(
|
async with httpx.AsyncClient(
|
||||||
transport=_pinned_dns_transport(self.proxy),
|
transport=_pinned_dns_transport(),
|
||||||
timeout=15.0,
|
timeout=15.0,
|
||||||
) as client:
|
) as client:
|
||||||
r, stream, redirect_error = await _stream_with_safe_redirects(
|
r, stream, redirect_error = await _stream_with_safe_redirects(
|
||||||
@@ -1040,7 +1045,7 @@ class WebFetchTool(Tool):
|
|||||||
try:
|
try:
|
||||||
async with httpx.AsyncClient(
|
async with httpx.AsyncClient(
|
||||||
timeout=30.0,
|
timeout=30.0,
|
||||||
transport=_pinned_dns_transport(self.proxy),
|
transport=_pinned_dns_transport(),
|
||||||
) as client:
|
) as client:
|
||||||
r, redirect_error = await _get_with_safe_redirects(
|
r, redirect_error = await _get_with_safe_redirects(
|
||||||
client,
|
client,
|
||||||
|
|||||||
@@ -161,11 +161,10 @@ class PinnedDNSAsyncTransport(httpx.AsyncBaseTransport):
|
|||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
allow_loopback: bool = False,
|
allow_loopback: bool = False,
|
||||||
proxy: httpx.ProxyTypes | None = None,
|
|
||||||
inner: httpx.AsyncBaseTransport | None = None,
|
inner: httpx.AsyncBaseTransport | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
self._allow_loopback = allow_loopback
|
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:
|
async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
|
||||||
url = str(request.url)
|
url = str(request.url)
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import socket
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
import pytest
|
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
|
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
|
# connect_mcp_servers skips unreachable HTTP servers
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -147,6 +147,19 @@ async def test_safe_redirect_requests_use_independent_pinned_dns_concurrently(mo
|
|||||||
assert calls == {"a.example": 2, "b.example": 2}
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_web_fetch_can_skip_jina_and_use_custom_user_agent(monkeypatch):
|
async def test_web_fetch_can_skip_jina_and_use_custom_user_agent(monkeypatch):
|
||||||
tool = WebFetchTool(
|
tool = WebFetchTool(
|
||||||
|
|||||||
Reference in New Issue
Block a user