From 31a71d6cd5cca2a2d6a844cccdedb59c00b37e1b Mon Sep 17 00:00:00 2001 From: shixi-li Date: Wed, 5 Aug 2026 20:59:03 +0800 Subject: [PATCH] fix(web): keep credential-bearing URLs away from the remote Jina reader --- docs/configuration.md | 8 + docs/guides/configure-web-search.md | 4 + nanobot/agent/tools/web.py | 44 ++++- tests/tools/test_web_fetch_jina_privacy.py | 181 +++++++++++++++++++++ 4 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 tests/tools/test_web_fetch_jina_privacy.py diff --git a/docs/configuration.md b/docs/configuration.md index 75b9cc451..0a86c6e29 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -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 diff --git a/docs/guides/configure-web-search.md b/docs/guides/configure-web-search.md index 333ee4e03..9d370302e 100644 --- a/docs/guides/configure-web-search.md +++ b/docs/guides/configure-web-search.md @@ -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. diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index f3d6caafb..f492dd0f7 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -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 diff --git a/tests/tools/test_web_fetch_jina_privacy.py b/tests/tools/test_web_fetch_jina_privacy.py new file mode 100644 index 000000000..8f5449842 --- /dev/null +++ b/tests/tools/test_web_fetch_jina_privacy.py @@ -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 = "T

ok

" + 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)