mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-13 07:39:15 +03:00
fix(web): keep credential-bearing URLs away from the remote Jina reader
This commit is contained in:
@@ -1921,6 +1921,14 @@ Create a key at [serper.dev](https://serper.dev). You can also set `SERPER_API_K
|
||||
|
||||
nanobot by default uses [Jina Reader](https://jina.ai/reader/), a third-party API, to convert arbitrary pages into Markdown format for easy digestion by the LLM, with a local fallback based on [readability-lxml](https://github.com/buriy/python-readability) if the former fails.
|
||||
|
||||
> [!NOTE]
|
||||
> Using the remote reader means the fetched URL itself is disclosed to the
|
||||
> third-party service. URLs that visibly carry credentials (userinfo, signed-URL
|
||||
> or token-style query parameters) are detected and fetched locally instead, but
|
||||
> secrets embedded in a URL's *path* (for example bot-token or webhook-style
|
||||
> URLs) cannot be reliably detected. Set `useJinaReader: false` if fetched URLs
|
||||
> must never leave the machine.
|
||||
|
||||
If you want to always use the local conversion, you can force it using:
|
||||
|
||||
```json
|
||||
|
||||
@@ -81,6 +81,10 @@ in the WebUI or logs.
|
||||
- Web fetch and HTTP MCP share an SSRF guard.
|
||||
- Private, loopback, link-local, and cloud metadata addresses are blocked by
|
||||
default.
|
||||
- With `useJinaReader` enabled (the default), fetched URLs are disclosed to the
|
||||
remote reader service. Credential-bearing URLs (userinfo or token/signature
|
||||
query parameters) are fetched locally instead; path-embedded secrets cannot
|
||||
be detected, so disable the remote reader when URLs must stay local.
|
||||
- Add `tools.ssrfWhitelist` only for narrow trusted CIDRs.
|
||||
- Do not give public chat users unrestricted web and shell access without
|
||||
review.
|
||||
|
||||
@@ -11,7 +11,7 @@ import os
|
||||
import re
|
||||
from collections.abc import Callable
|
||||
from typing import Any, cast
|
||||
from urllib.parse import quote, urljoin, urlparse
|
||||
from urllib.parse import parse_qsl, quote, urljoin, urlparse
|
||||
|
||||
import httpx
|
||||
from loguru import logger
|
||||
@@ -148,6 +148,33 @@ def _unsafe_url_request_error(exc: BaseException) -> str | None:
|
||||
return str(exc) if isinstance(exc, UnsafeURLRequestError) else None
|
||||
|
||||
|
||||
# Forwarding a URL to the remote Jina reader discloses it to a third party, so
|
||||
# URLs that embed credential material (userinfo, signed-URL parameters, token
|
||||
# or key query values) must never leave the machine. Matching is by parameter
|
||||
# name: over-matching only costs the local readability fallback, while
|
||||
# under-matching leaks a secret.
|
||||
_CREDENTIAL_QUERY_PARAMS = frozenset({
|
||||
"access_token", "apikey", "api_key", "auth", "authorization",
|
||||
"client_secret", "id_token", "key", "password", "passwd", "pwd",
|
||||
"refresh_token", "secret", "sig", "signature", "token",
|
||||
})
|
||||
_CREDENTIAL_QUERY_PREFIXES = ("x-amz-", "x-goog-")
|
||||
|
||||
|
||||
def _url_carries_credentials(url: str) -> bool:
|
||||
try:
|
||||
parsed = urlparse(url)
|
||||
except ValueError:
|
||||
return True
|
||||
if parsed.username is not None or parsed.password is not None:
|
||||
return True
|
||||
for name, _value in parse_qsl(parsed.query, keep_blank_values=True):
|
||||
lowered = name.lower()
|
||||
if lowered in _CREDENTIAL_QUERY_PARAMS or lowered.startswith(_CREDENTIAL_QUERY_PREFIXES):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
async def _get_with_safe_redirects(
|
||||
client: httpx.AsyncClient,
|
||||
url: str,
|
||||
@@ -1082,13 +1109,26 @@ class WebFetchTool(Tool):
|
||||
|
||||
async def _fetch_jina(self, url: str, max_chars: int) -> str | None:
|
||||
"""Try fetching via Jina Reader API. Returns None on failure."""
|
||||
if _url_carries_credentials(url):
|
||||
redacted = urlparse(url)
|
||||
logger.debug(
|
||||
"Skipping Jina Reader for {}://{}{}: URL carries credential material",
|
||||
redacted.scheme,
|
||||
redacted.hostname or "",
|
||||
redacted.path,
|
||||
)
|
||||
return None
|
||||
# httpx already drops the fragment when building the request; strip it
|
||||
# explicitly so client-side-only data (OAuth implicit flows put tokens
|
||||
# there) stays out of this path even if the transport changes.
|
||||
forwarded_url = url.split("#", 1)[0]
|
||||
try:
|
||||
headers = {"Accept": "application/json", "User-Agent": self.user_agent}
|
||||
jina_key = os.environ.get("JINA_API_KEY", "")
|
||||
if jina_key:
|
||||
headers["Authorization"] = f"Bearer {jina_key}"
|
||||
async with httpx.AsyncClient(proxy=self.proxy, timeout=20.0) as client:
|
||||
r = await client.get(f"https://r.jina.ai/{url}", headers=headers)
|
||||
r = await client.get(f"https://r.jina.ai/{forwarded_url}", headers=headers)
|
||||
if r.status_code == 429:
|
||||
logger.debug("Jina Reader rate limited, falling back to readability")
|
||||
return None
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
"""Tests that the Jina Reader path never discloses credential-bearing URLs."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import socket
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.tools import web as web_module
|
||||
from nanobot.agent.tools.web import (
|
||||
WebFetchTool,
|
||||
_url_carries_credentials,
|
||||
)
|
||||
|
||||
|
||||
def _fake_resolve_public(hostname, port, family=0, type_=0):
|
||||
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))]
|
||||
|
||||
|
||||
class _RecordingJinaClient:
|
||||
"""Fake httpx.AsyncClient that records every requested URL."""
|
||||
|
||||
requested: list[str] = []
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *args):
|
||||
return False
|
||||
|
||||
async def get(self, url, **kwargs):
|
||||
_RecordingJinaClient.requested.append(url)
|
||||
|
||||
class _Response:
|
||||
status_code = 200
|
||||
|
||||
def raise_for_status(self):
|
||||
pass
|
||||
|
||||
def json(self):
|
||||
return {"data": {"title": "T", "content": "body", "url": url}}
|
||||
|
||||
return _Response()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def jina_client():
|
||||
_RecordingJinaClient.requested = []
|
||||
with patch("nanobot.agent.tools.web.httpx.AsyncClient", _RecordingJinaClient):
|
||||
yield _RecordingJinaClient
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url",
|
||||
[
|
||||
"https://user:secret@example.com/report",
|
||||
"https://user@example.com/report",
|
||||
"https://example.com/download?token=abc123",
|
||||
"https://example.com/download?access_token=abc123",
|
||||
"https://example.com/doc?Signature=xyz&Expires=1700000000",
|
||||
"https://bucket.s3.amazonaws.com/key?X-Amz-Signature=deadbeef",
|
||||
"https://storage.googleapis.com/o/file?X-Goog-Signature=deadbeef",
|
||||
"https://example.com/blob?sig=sas-token-material",
|
||||
"https://maps.example.com/api?key=AIzaFixture",
|
||||
],
|
||||
)
|
||||
def test_credential_urls_are_detected(url: str) -> None:
|
||||
assert _url_carries_credentials(url) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url",
|
||||
[
|
||||
"https://example.com/",
|
||||
"https://example.com/watch?v=abc123",
|
||||
"https://example.com/search?q=token+design&page=2",
|
||||
"https://example.com/page#section-3",
|
||||
],
|
||||
)
|
||||
def test_plain_urls_are_not_detected(url: str) -> None:
|
||||
assert _url_carries_credentials(url) is False
|
||||
|
||||
|
||||
async def test_jina_is_skipped_for_credential_urls(jina_client) -> None:
|
||||
tool = WebFetchTool()
|
||||
result = await tool._fetch_jina(
|
||||
"https://example.com/download?token=abc123", max_chars=1000
|
||||
)
|
||||
assert result is None
|
||||
assert jina_client.requested == []
|
||||
|
||||
|
||||
async def test_jina_is_skipped_for_userinfo_urls(jina_client) -> None:
|
||||
tool = WebFetchTool()
|
||||
result = await tool._fetch_jina(
|
||||
"https://user:secret@example.com/report", max_chars=1000
|
||||
)
|
||||
assert result is None
|
||||
assert jina_client.requested == []
|
||||
|
||||
|
||||
async def test_jina_still_used_for_plain_urls(jina_client) -> None:
|
||||
tool = WebFetchTool()
|
||||
result = await tool._fetch_jina("https://example.com/watch?v=abc123", max_chars=1000)
|
||||
assert result is not None
|
||||
assert json.loads(result)["extractor"] == "jina"
|
||||
assert jina_client.requested == [
|
||||
"https://r.jina.ai/https://example.com/watch?v=abc123"
|
||||
]
|
||||
|
||||
|
||||
async def test_fragment_is_never_forwarded(jina_client) -> None:
|
||||
tool = WebFetchTool()
|
||||
result = await tool._fetch_jina(
|
||||
"https://example.com/page?q=1#access_token=leaked", max_chars=1000
|
||||
)
|
||||
assert result is not None
|
||||
assert jina_client.requested == ["https://r.jina.ai/https://example.com/page?q=1"]
|
||||
|
||||
|
||||
async def test_execute_fetches_credential_urls_locally(monkeypatch) -> None:
|
||||
"""The tool boundary: a credential URL must use the local extractor and
|
||||
produce zero requests to the remote reader."""
|
||||
|
||||
tool = WebFetchTool()
|
||||
requested: list[str] = []
|
||||
|
||||
class FakeStreamResponse:
|
||||
status_code = 200
|
||||
headers = {"content-type": "text/html"}
|
||||
url = "https://example.com/download"
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
class FakeResponse:
|
||||
status_code = 200
|
||||
url = "https://example.com/download"
|
||||
text = "<html><head><title>T</title></head><body><p>ok</p></body></html>"
|
||||
headers = {"content-type": "text/html"}
|
||||
is_redirect = False
|
||||
|
||||
def raise_for_status(self):
|
||||
return None
|
||||
|
||||
class FakeClient:
|
||||
def __init__(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
def stream(self, method, url, headers=None, **kwargs):
|
||||
requested.append(str(url))
|
||||
return FakeStreamResponse()
|
||||
|
||||
async def get(self, url, headers=None, **kwargs):
|
||||
requested.append(str(url))
|
||||
return FakeResponse()
|
||||
|
||||
monkeypatch.setattr(tool, "_extract_readable_html", lambda html, mode: "ok")
|
||||
monkeypatch.setattr("nanobot.agent.tools.web.httpx.AsyncClient", FakeClient)
|
||||
monkeypatch.setattr(web_module, "_pinned_dns_transport", lambda: object())
|
||||
|
||||
with patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public):
|
||||
result = await tool.execute(url="https://example.com/download?token=abc123")
|
||||
|
||||
data = json.loads(result)
|
||||
assert data["extractor"] == "readability"
|
||||
assert all("r.jina.ai" not in url for url in requested)
|
||||
Reference in New Issue
Block a user