From b097554842960959e85655843c8279cb5514b7f0 Mon Sep 17 00:00:00 2001
From: hedhoud <74668966+hedhoud@users.noreply.github.com>
Date: Tue, 26 May 2026 15:20:04 +0200
Subject: [PATCH] fix(websearch): harden SSRF guard in ContentFetcher (#383)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Three vectors were unguarded:
1. Only loopback addresses were blocked — private RFC 1918 ranges,
link-local (169.254.x.x), CGNAT (100.64.x.x), and other reserved
networks were passed through.
2. Decimal-integer-encoded IPv4 addresses (e.g. 2130706433 == 127.0.0.1)
were not caught because ipaddress.ip_address() only parses dotted
decimal; the integer form is parsed by a second int() coercion.
3. follow_redirects=True meant a legitimate initial URL could silently
redirect to a private address after the initial check passed.
Fix: replace _is_loopback_url with _is_safe_url (module-level) that
checks all private/reserved/non-global flags explicitly, handles
decimal-integer IPs, and blocks non-HTTP(S) schemes. Redirect following
is now manual (follow_redirects=False) so every hop is validated before
the next request is sent.
Tests: 39 cases covering private ranges, decimal encoding, non-HTTP
schemes, redirect-to-private-IP, safe redirect following, and redirect
chain length cap.
---
.../components/websearch/content_fetcher.py | 136 ++++++++++++++----
.../websearch/test_content_fetcher.py | 126 +++++++++++++++-
2 files changed, 231 insertions(+), 31 deletions(-)
diff --git a/openrag/components/websearch/content_fetcher.py b/openrag/components/websearch/content_fetcher.py
index 3e3ffc10a..27fa9d033 100644
--- a/openrag/components/websearch/content_fetcher.py
+++ b/openrag/components/websearch/content_fetcher.py
@@ -1,6 +1,6 @@
import asyncio
import ipaddress
-from urllib.parse import urlparse
+from urllib.parse import urljoin, urlparse
import httpx
import lxml.html
@@ -18,6 +18,72 @@
_USER_AGENT = "Mozilla/5.0 (compatible; OpenRAG/1.0; +https://github.com/linagora/openrag)"
+_MAX_REDIRECTS = 10
+
+
+def _is_blocked_address(addr: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool:
+ """True for any IP that a server-side fetcher must not contact.
+
+ Checks all private/reserved/non-global flags explicitly so the guard is
+ correct across Python minor releases (``is_global`` semantics changed
+ between 3.10 and 3.11 for CGNAT and some multicast ranges).
+ """
+ return (
+ addr.is_loopback
+ or addr.is_private
+ or addr.is_link_local
+ or addr.is_reserved
+ or addr.is_unspecified
+ or addr.is_multicast
+ or not addr.is_global
+ )
+
+
+def _is_safe_url(url: str) -> bool:
+ """Return True only if *url* is safe for a server-side fetch.
+
+ Blocks:
+ - Non-HTTP(S) schemes
+ - ``localhost`` hostname
+ - IPv4/IPv6 literals in private, loopback, link-local, or reserved ranges
+ - Decimal-integer-encoded IPv4 addresses (e.g. ``2130706433`` == ``127.0.0.1``)
+
+ Regular hostnames pass through; per-hop redirect validation (in
+ :meth:`ContentFetcher._fetch_single`) re-checks every redirect target,
+ covering the case where a public hostname redirects to a private address.
+ """
+ try:
+ parsed = urlparse(url)
+ except Exception:
+ return False
+
+ if parsed.scheme not in ("http", "https"):
+ return False
+
+ host = parsed.hostname
+ if not host:
+ return False
+
+ if host.lower() == "localhost":
+ return False
+
+ # Dotted-decimal or IPv6 literal (e.g. "127.0.0.1", "::1", "10.0.0.1")
+ try:
+ return not _is_blocked_address(ipaddress.ip_address(host))
+ except ValueError:
+ pass
+
+ # Decimal-integer form (e.g. 2130706433 → 127.0.0.1).
+ # ipaddress.ip_address(int) interprets the value as a packed IPv4 address,
+ # matching how glibc's resolver (and therefore httpx) handles such hostnames.
+ try:
+ return not _is_blocked_address(ipaddress.ip_address(int(host)))
+ except (ValueError, TypeError):
+ pass
+
+ # Regular hostname — passes initial check; every redirect hop is re-validated.
+ return True
+
class ContentFetcher:
"""Fetch and extract text content from web search result URLs."""
@@ -46,29 +112,44 @@ def _truncate(self, text: str) -> str:
truncated = truncated[:last_space]
return truncated.rstrip() + " [...]"
- @staticmethod
- def _is_loopback_url(url: str) -> bool:
- host = urlparse(url).hostname or ""
- if host == "localhost":
- return True
- try:
- return not ipaddress.ip_address(host).is_global
- except ValueError:
- return False # Regular hostname, let it through
-
async def _fetch_single(self, client: httpx.AsyncClient, url: str) -> str | None:
- """Fetch a single URL and extract text. Returns None on any failure."""
- # Guard against SSRF: URLs come from the search provider, but a compromised
- # or misbehaving provider could return loopback addresses targeting internal services.
- if self._is_loopback_url(url):
- logger.warning("Blocked loopback URL in web search results", url=url)
- return None
- try:
- response = await asyncio.wait_for(
- client.get(url, follow_redirects=True),
- timeout=self.timeout,
- )
- response.raise_for_status()
+ """Fetch a single URL and extract text. Returns None on any failure.
+
+ Redirects are followed manually (``follow_redirects=False``) so every
+ hop is validated by :func:`_is_safe_url` before the request is sent.
+ This prevents a legitimate initial URL from redirecting to a private
+ address after the initial check passes.
+ """
+ current_url = url
+ for _ in range(_MAX_REDIRECTS + 1):
+ if not _is_safe_url(current_url):
+ logger.warning("Blocked unsafe URL in web search results", url=current_url)
+ return None
+
+ try:
+ response = await asyncio.wait_for(
+ client.get(current_url, follow_redirects=False),
+ timeout=self.timeout,
+ )
+ except TimeoutError:
+ logger.debug("Content fetch timed out", url=current_url)
+ return None
+ except Exception as e:
+ logger.debug("Content fetch failed", url=current_url, error=str(e))
+ return None
+
+ if response.is_redirect:
+ location = response.headers.get("location", "")
+ if not location:
+ return None
+ current_url = urljoin(current_url, location)
+ continue
+
+ try:
+ response.raise_for_status()
+ except Exception as e:
+ logger.debug("Content fetch failed", url=current_url, error=str(e))
+ return None
content_type = response.headers.get("content-type", "")
if "text/html" not in content_type and "text/plain" not in content_type:
@@ -78,7 +159,6 @@ async def _fetch_single(self, client: httpx.AsyncClient, url: str) -> str | None
if not html.strip():
return None
- # Strip boilerplate elements (nav, footer, etc.) before conversion
try:
tree = lxml.html.fromstring(html)
for tag in _BOILERPLATE_TAGS:
@@ -95,12 +175,8 @@ async def _fetch_single(self, client: httpx.AsyncClient, url: str) -> str | None
return self._truncate(text)
- except TimeoutError:
- logger.debug("Content fetch timed out", url=url)
- return None
- except Exception as e:
- logger.debug("Content fetch failed", url=url, error=str(e))
- return None
+ logger.debug("Too many redirects, giving up", url=url)
+ return None
async def enrich(self, results: list[WebResult]) -> list[WebResult]:
"""Fetch content for the top N results in parallel. Mutates results in place."""
diff --git a/openrag/components/websearch/test_content_fetcher.py b/openrag/components/websearch/test_content_fetcher.py
index e171c8e13..5ab74f81a 100644
--- a/openrag/components/websearch/test_content_fetcher.py
+++ b/openrag/components/websearch/test_content_fetcher.py
@@ -3,7 +3,7 @@
import httpx
import pytest
from components.websearch.base import WebResult
-from components.websearch.content_fetcher import ContentFetcher
+from components.websearch.content_fetcher import ContentFetcher, _is_safe_url
@pytest.fixture
@@ -15,6 +15,63 @@ def _make_result(url="https://example.com", snippet="short snippet"):
return WebResult(title="Test", url=url, snippet=snippet)
+# ---------------------------------------------------------------------------
+# _is_safe_url — unit tests (no network, no client)
+# ---------------------------------------------------------------------------
+
+
+class TestIsSafeUrl:
+ @pytest.mark.parametrize(
+ "url",
+ [
+ "http://localhost/secret",
+ "http://127.0.0.1/admin",
+ "http://127.0.0.42/x",
+ "http://[::1]/admin",
+ "http://10.0.0.1/internal",
+ "http://192.168.1.1/router",
+ "http://169.254.169.254/metadata", # AWS/cloud metadata service
+ "http://0.0.0.0/x",
+ "http://100.64.0.1/cgnat", # RFC 6598 shared address space
+ "http://198.18.0.1/bench", # Benchmarking range
+ ],
+ )
+ def test_blocks_private_and_reserved_addresses(self, url):
+ assert _is_safe_url(url) is False
+
+ def test_blocks_decimal_encoded_loopback(self):
+ """2130706433 is 127.0.0.1 in decimal integer form."""
+ assert _is_safe_url("http://2130706433/secret") is False
+
+ def test_blocks_decimal_encoded_private(self):
+ """167772161 is 10.0.0.1 in decimal integer form."""
+ assert _is_safe_url("http://167772161/secret") is False
+
+ @pytest.mark.parametrize(
+ "url",
+ [
+ "file:///etc/passwd",
+ "ftp://example.com/x",
+ "data:text/html,hi
",
+ "javascript:alert(1)",
+ ],
+ )
+ def test_blocks_non_http_schemes(self, url):
+ assert _is_safe_url(url) is False
+
+ def test_allows_public_ip(self):
+ # 93.184.216.34 is example.com — globally routable
+ assert _is_safe_url("http://93.184.216.34/page") is True
+
+ def test_allows_regular_hostname(self):
+ assert _is_safe_url("https://example.com/page") is True
+
+
+# ---------------------------------------------------------------------------
+# _fetch_single — integration tests with mock transport
+# ---------------------------------------------------------------------------
+
+
class TestFetchSingleURL:
@pytest.mark.asyncio
async def test_extracts_text_from_html(self, fetcher):
@@ -78,6 +135,73 @@ async def mock_handler(request):
assert text is None
+ @pytest.mark.asyncio
+ async def test_blocks_redirect_to_private_ip(self, fetcher):
+ """A response that redirects to a private IP must be blocked after the initial fetch."""
+ calls: list[str] = []
+
+ async def redirecting_handler(request):
+ calls.append(str(request.url))
+ if len(calls) == 1:
+ return httpx.Response(
+ 302,
+ headers={"location": "http://192.168.1.1/secret"},
+ )
+ return httpx.Response(200, text="SSRF data")
+
+ transport = httpx.MockTransport(redirecting_handler)
+ async with httpx.AsyncClient(transport=transport) as client:
+ # 93.184.216.34 is example.com — passes the initial check
+ text = await fetcher._fetch_single(client, "http://93.184.216.34/page")
+
+ assert text is None
+ # The redirect target (192.168.1.1) must never have been contacted
+ assert len(calls) == 1
+
+ @pytest.mark.asyncio
+ async def test_follows_safe_redirect(self, fetcher):
+ """A redirect to another public URL must be followed and its content returned."""
+ calls: list[str] = []
+
+ async def handler(request):
+ calls.append(str(request.url))
+ if len(calls) == 1:
+ return httpx.Response(
+ 301,
+ headers={"location": "http://93.184.216.34/final"},
+ )
+ return httpx.Response(
+ 200,
+ text="Final page content.
",
+ )
+
+ transport = httpx.MockTransport(handler)
+ async with httpx.AsyncClient(transport=transport) as client:
+ text = await fetcher._fetch_single(client, "http://93.184.216.34/start")
+
+ assert text is not None
+ assert "Final page content" in text
+ assert len(calls) == 2
+
+ @pytest.mark.asyncio
+ async def test_stops_after_max_redirects(self, fetcher):
+ """Redirect chains that exceed _MAX_REDIRECTS are aborted."""
+ hop = 0
+
+ async def infinite_redirect(request):
+ nonlocal hop
+ hop += 1
+ return httpx.Response(
+ 302,
+ headers={"location": f"http://93.184.216.34/hop{hop}"},
+ )
+
+ transport = httpx.MockTransport(infinite_redirect)
+ async with httpx.AsyncClient(transport=transport) as client:
+ text = await fetcher._fetch_single(client, "http://93.184.216.34/start")
+
+ assert text is None
+
@pytest.mark.asyncio
async def test_strips_boilerplate_html(self, fetcher):
html = """