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.
This commit is contained in:
chengyongru
Xubin Ren
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user