mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-13 07:39:15 +03:00
fix(web): keep credential redirects away from Jina
This commit is contained in:
+30
-10
@@ -218,13 +218,14 @@ async def _stream_with_safe_redirects(
|
|||||||
client: httpx.AsyncClient,
|
client: httpx.AsyncClient,
|
||||||
url: str,
|
url: str,
|
||||||
headers: dict[str, str] | None = None,
|
headers: dict[str, str] | None = None,
|
||||||
) -> tuple[httpx.Response | None, Any | None, str | None]:
|
) -> tuple[httpx.Response | None, Any | None, str | None, bool]:
|
||||||
"""Open a streamed response while validating every redirect target first."""
|
"""Open a streamed response while validating every redirect target first."""
|
||||||
current_url = url
|
current_url = url
|
||||||
|
chain_carries_credentials = _url_carries_credentials(url)
|
||||||
for _ in range(MAX_REDIRECTS + 1):
|
for _ in range(MAX_REDIRECTS + 1):
|
||||||
is_valid, error_msg, _ = _resolve_url_safe(current_url)
|
is_valid, error_msg, _ = _resolve_url_safe(current_url)
|
||||||
if not is_valid:
|
if not is_valid:
|
||||||
return None, None, f"Redirect blocked: {error_msg}"
|
return None, None, f"Redirect blocked: {error_msg}", chain_carries_credentials
|
||||||
|
|
||||||
stream = client.stream(
|
stream = client.stream(
|
||||||
"GET",
|
"GET",
|
||||||
@@ -237,26 +238,39 @@ async def _stream_with_safe_redirects(
|
|||||||
except httpx.RequestError as exc:
|
except httpx.RequestError as exc:
|
||||||
unsafe_error = _unsafe_url_request_error(exc)
|
unsafe_error = _unsafe_url_request_error(exc)
|
||||||
if unsafe_error is not None:
|
if unsafe_error is not None:
|
||||||
return None, None, f"Redirect blocked: {unsafe_error}"
|
return (
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
f"Redirect blocked: {unsafe_error}",
|
||||||
|
chain_carries_credentials,
|
||||||
|
)
|
||||||
raise
|
raise
|
||||||
is_redirect = 300 <= response.status_code < 400
|
is_redirect = 300 <= response.status_code < 400
|
||||||
if not is_redirect:
|
if not is_redirect:
|
||||||
return response, stream, None
|
return response, stream, None, chain_carries_credentials
|
||||||
|
|
||||||
location = response.headers.get("location")
|
location = response.headers.get("location")
|
||||||
if not location:
|
if not location:
|
||||||
return response, stream, None
|
return response, stream, None, chain_carries_credentials
|
||||||
|
|
||||||
next_url = urljoin(str(response.url), location)
|
next_url = urljoin(str(response.url), location)
|
||||||
|
chain_carries_credentials = (
|
||||||
|
chain_carries_credentials or _url_carries_credentials(next_url)
|
||||||
|
)
|
||||||
is_valid, error_msg = _validate_url_safe(next_url)
|
is_valid, error_msg = _validate_url_safe(next_url)
|
||||||
if not is_valid:
|
if not is_valid:
|
||||||
await stream.__aexit__(None, None, None)
|
await stream.__aexit__(None, None, None)
|
||||||
return None, None, f"Redirect blocked: {error_msg}"
|
return None, None, f"Redirect blocked: {error_msg}", chain_carries_credentials
|
||||||
|
|
||||||
await stream.__aexit__(None, None, None)
|
await stream.__aexit__(None, None, None)
|
||||||
current_url = next_url
|
current_url = next_url
|
||||||
|
|
||||||
return None, None, f"Too many redirects: exceeded limit of {MAX_REDIRECTS}"
|
return (
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
f"Too many redirects: exceeded limit of {MAX_REDIRECTS}",
|
||||||
|
chain_carries_credentials,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _format_results(query: str, items: list[dict[str, Any]], n: int) -> str:
|
def _format_results(query: str, items: list[dict[str, Any]], n: int) -> str:
|
||||||
@@ -1070,20 +1084,26 @@ class WebFetchTool(Tool):
|
|||||||
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)
|
||||||
|
|
||||||
# Detect and fetch images directly to avoid Jina's textual image captioning
|
# Detect and fetch images directly to avoid Jina's textual image captioning.
|
||||||
|
# This local preflight also proves that no credential-bearing URL occurs
|
||||||
|
# in the redirect chain before the original URL may be sent to Jina.
|
||||||
|
jina_remote_safe = False
|
||||||
try:
|
try:
|
||||||
async with httpx.AsyncClient(
|
async with httpx.AsyncClient(
|
||||||
**_fetch_client_kwargs(self.proxy, 15.0),
|
**_fetch_client_kwargs(self.proxy, 15.0),
|
||||||
) as client:
|
) as client:
|
||||||
r, stream, redirect_error = await _stream_with_safe_redirects(
|
r, stream, redirect_error, chain_carries_credentials = (
|
||||||
|
await _stream_with_safe_redirects(
|
||||||
client,
|
client,
|
||||||
url,
|
url,
|
||||||
headers={"User-Agent": self.user_agent},
|
headers={"User-Agent": self.user_agent},
|
||||||
)
|
)
|
||||||
|
)
|
||||||
if redirect_error:
|
if redirect_error:
|
||||||
return json.dumps({"error": redirect_error, "url": url}, ensure_ascii=False)
|
return json.dumps({"error": redirect_error, "url": url}, ensure_ascii=False)
|
||||||
if r is None:
|
if r is None:
|
||||||
return json.dumps({"error": "Fetch failed", "url": url}, ensure_ascii=False)
|
return json.dumps({"error": "Fetch failed", "url": url}, ensure_ascii=False)
|
||||||
|
jina_remote_safe = not chain_carries_credentials
|
||||||
|
|
||||||
try:
|
try:
|
||||||
ctype = r.headers.get("content-type", "")
|
ctype = r.headers.get("content-type", "")
|
||||||
@@ -1101,7 +1121,7 @@ class WebFetchTool(Tool):
|
|||||||
logger.debug("Pre-fetch image detection failed for {}: {}", url, e)
|
logger.debug("Pre-fetch image detection failed for {}: {}", url, e)
|
||||||
|
|
||||||
result = None
|
result = None
|
||||||
if self.config.use_jina_reader:
|
if self.config.use_jina_reader and jina_remote_safe:
|
||||||
result = await self._fetch_jina(url, max_chars)
|
result = await self._fetch_jina(url, max_chars)
|
||||||
if result is None:
|
if result is None:
|
||||||
result = await self._fetch_readability(url, extract_mode, max_chars)
|
result = await self._fetch_readability(url, extract_mode, max_chars)
|
||||||
|
|||||||
@@ -179,3 +179,68 @@ async def test_execute_fetches_credential_urls_locally(monkeypatch) -> None:
|
|||||||
data = json.loads(result)
|
data = json.loads(result)
|
||||||
assert data["extractor"] == "readability"
|
assert data["extractor"] == "readability"
|
||||||
assert all("r.jina.ai" not in url for url in requested)
|
assert all("r.jina.ai" not in url for url in requested)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_execute_does_not_send_redirected_credential_url_to_jina(monkeypatch) -> None:
|
||||||
|
"""A plain short URL that redirects through a signed URL must stay local."""
|
||||||
|
|
||||||
|
tool = WebFetchTool()
|
||||||
|
requested: list[str] = []
|
||||||
|
short_url = "https://example.com/short"
|
||||||
|
signed_url = "https://cdn.example.com/file?token=secret"
|
||||||
|
|
||||||
|
class FakeStreamResponse:
|
||||||
|
def __init__(self, url: str):
|
||||||
|
self.url = url
|
||||||
|
self.status_code = 302 if url == short_url else 200
|
||||||
|
self.headers = (
|
||||||
|
{"location": signed_url}
|
||||||
|
if url == short_url
|
||||||
|
else {"content-type": "text/html"}
|
||||||
|
)
|
||||||
|
|
||||||
|
async def __aenter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
async def __aexit__(self, exc_type, exc, tb):
|
||||||
|
return False
|
||||||
|
|
||||||
|
class FakeResponse:
|
||||||
|
status_code = 200
|
||||||
|
url = signed_url
|
||||||
|
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(str(url))
|
||||||
|
|
||||||
|
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=short_url)
|
||||||
|
|
||||||
|
data = json.loads(result)
|
||||||
|
assert data["extractor"] == "readability"
|
||||||
|
assert signed_url in requested
|
||||||
|
assert all("r.jina.ai" not in url for url in requested)
|
||||||
|
|||||||
Reference in New Issue
Block a user