diff --git a/src/agentic_cli/tools/webfetch/fetcher.py b/src/agentic_cli/tools/webfetch/fetcher.py index a5a8a98..bde8ed9 100644 --- a/src/agentic_cli/tools/webfetch/fetcher.py +++ b/src/agentic_cli/tools/webfetch/fetcher.py @@ -2,20 +2,19 @@ from __future__ import annotations -import socket import time from dataclasses import dataclass from urllib.parse import urljoin, urlparse import httpx -from agentic_cli.tools.webfetch.validator import URLValidator +from agentic_cli.tools.webfetch.validator import URLValidator, BlockedAddressError from agentic_cli.tools.webfetch.robots import RobotsTxtChecker +from agentic_cli.tools.webfetch.transport import PinnedTransport @dataclass class RedirectInfo: - """Information about a cross-host redirect.""" from_url: str to_url: str to_host: str @@ -23,7 +22,6 @@ class RedirectInfo: @dataclass class FetchResult: - """Result of a content fetch operation.""" success: bool content: str | bytes | None = None content_type: str | None = None @@ -35,7 +33,6 @@ class FetchResult: @dataclass class CachedResponse: - """Cached HTTP response.""" content: str | bytes content_type: str timestamp: float @@ -43,186 +40,119 @@ class CachedResponse: class ContentFetcher: - """Fetches web content with validation, caching, and redirect handling.""" + """Fetches web content with SSRF-safe pinning, caching, and redirect handling.""" + + MAX_REDIRECTS = 5 def __init__( self, validator: URLValidator, robots_checker: RobotsTxtChecker, + transport: PinnedTransport, cache_ttl_seconds: int = 900, max_content_bytes: int = 102400, max_pdf_bytes: int = 5242880, ) -> None: self._validator = validator self._robots = robots_checker + self._transport = transport self._cache_ttl = cache_ttl_seconds self._max_content_bytes = max_content_bytes self._max_pdf_bytes = max_pdf_bytes self._cache: dict[str, CachedResponse] = {} - MAX_REDIRECTS = 5 - async def fetch(self, url: str, timeout: int = 30) -> FetchResult: """Fetch content from a URL. - Redirects are followed manually so each Location is revalidated by - the SSRF guard before the next request is issued — using - ``follow_redirects=True`` would let httpx contact intermediate hosts - (e.g., 169.254.169.254) before we get a chance to inspect them. - - Args: - url: The URL to fetch. - timeout: Request timeout in seconds. - - Returns: - FetchResult with content or error information. + Redirects are followed manually so each Location is revalidated before + the next request; the PinnedTransport resolves+validates+pins every hop + (and the robots fetch), connecting only to globally-routable IPs. """ - # Check cache first cached = self._get_cached(url) if cached is not None: return FetchResult( - success=True, - content=cached.content, - content_type=cached.content_type, - truncated=cached.truncated, - from_cache=True, + success=True, content=cached.content, content_type=cached.content_type, + truncated=cached.truncated, from_cache=True, ) - # Validate the originally requested URL validation = self._validator.validate(url) if not validation.valid: return FetchResult(success=False, error=validation.error) - # Check robots.txt for the originally requested URL if not await self._robots.can_fetch(url): return FetchResult(success=False, error=f"Blocked by robots.txt for {url}") original_url = url current_url = url - history: list[str] = [] - response: httpx.Response | None = None try: - async with httpx.AsyncClient(follow_redirects=False) as client: + async with httpx.AsyncClient(transport=self._transport, follow_redirects=False) as client: for _ in range(self.MAX_REDIRECTS + 1): - response = await client.get(current_url, timeout=timeout) - - # Post-fetch IP revalidation (mitigates DNS rebinding) - final_host = urlparse(str(response.url)).hostname - if final_host: - try: - post_ip = socket.gethostbyname(final_host) - ip_check = self._validator.validate_ip(post_ip) - if not ip_check.valid: + async with client.stream("GET", current_url, timeout=timeout) as response: + status = response.status_code + headers = response.headers + content_type = headers.get("content-type", "text/html") + is_pdf = "application/pdf" in content_type.lower() + cap = self._max_pdf_bytes if is_pdf else self._max_content_bytes + encoding = response.charset_encoding or "utf-8" + buf = bytearray() + truncated = False + async for chunk in response.aiter_bytes(): + buf.extend(chunk) + if len(buf) > cap: + del buf[cap:] + truncated = True + break + + if status in (301, 302, 303, 307, 308): + location = headers.get("location") + if location: + next_url = urljoin(current_url, location) + next_validation = self._validator.validate(next_url) + if not next_validation.valid: return FetchResult( success=False, - error=f"DNS rebinding detected: {ip_check.error}", + error=f"Redirect to disallowed URL blocked: {next_validation.error}", ) - except socket.gaierror: - pass - - if response.status_code not in (301, 302, 303, 307, 308): - break - - location = response.headers.get("location") - if not location: - break - - next_url = urljoin(str(response.url), location) - next_validation = self._validator.validate(next_url) - if not next_validation.valid: - return FetchResult( - success=False, - error=( - f"Redirect to disallowed URL blocked: " - f"{next_validation.error}" - ), - ) - - # Cross-host redirects must be reported BEFORE issuing - # the next GET — the user's permission grant covers the - # original host only, so contacting another origin is a - # side effect they did not approve. - next_host = urlparse(next_url).netloc - original_host = urlparse(original_url).netloc - if next_host.lower() != original_host.lower(): - return FetchResult( - success=False, - redirect=RedirectInfo( - from_url=original_url, - to_url=next_url, - to_host=next_host, - ), - error=f"Cross-host redirect to {next_host}", - ) - - # Same-host redirect: the new path may itself be - # disallowed by robots.txt even though the original - # path was not. - if not await self._robots.can_fetch(next_url): - return FetchResult( - success=False, - error=f"Blocked by robots.txt for {next_url}", - ) - - history.append(current_url) - current_url = next_url - else: + next_host = urlparse(next_url).netloc + original_host = urlparse(original_url).netloc + if next_host.lower() != original_host.lower(): + return FetchResult( + success=False, + redirect=RedirectInfo(original_url, next_url, next_host), + error=f"Cross-host redirect to {next_host}", + ) + if not await self._robots.can_fetch(next_url): + return FetchResult(success=False, error=f"Blocked by robots.txt for {next_url}") + current_url = next_url + continue + + # Final response (non-redirect, or redirect without Location). + if is_pdf: + content: str | bytes = bytes(buf) + else: + content = buf.decode(encoding, errors="replace") + if truncated: + content += f"\n\n[Content truncated at {cap} bytes]" + self._cache[original_url] = CachedResponse( + content=content, content_type=content_type, + timestamp=time.time(), truncated=truncated, + ) return FetchResult( - success=False, - error=f"Too many redirects (max {self.MAX_REDIRECTS})", + success=True, content=content, content_type=content_type, + truncated=truncated, from_cache=False, ) + else: + return FetchResult(success=False, error=f"Too many redirects (max {self.MAX_REDIRECTS})") + except BlockedAddressError as e: + return FetchResult(success=False, error=f"Blocked (SSRF): {e}") except httpx.TimeoutException: return FetchResult(success=False, error=f"Request timeout after {timeout}s") except httpx.RequestError as e: return FetchResult(success=False, error=f"Request failed: {e}") - assert response is not None # loop runs at least once - - content_type = response.headers.get("content-type", "text/html") - - # Use bytes for PDF, text for everything else - is_pdf = "application/pdf" in content_type.lower() - if is_pdf: - content: str | bytes = response.content - truncated = False - if len(content) > self._max_pdf_bytes: - content = content[:self._max_pdf_bytes] - truncated = True - else: - content = response.text - truncated = False - if len(content) > self._max_content_bytes: - content = content[:self._max_content_bytes] - content += f"\n\n[Content truncated at {self._max_content_bytes} bytes]" - truncated = True - - # Cache the response under the originally requested URL - self._cache[original_url] = CachedResponse( - content=content, - content_type=content_type, - timestamp=time.time(), - truncated=truncated, - ) - - return FetchResult( - success=True, - content=content, - content_type=content_type, - truncated=truncated, - from_cache=False, - ) - def _get_cached(self, url: str) -> CachedResponse | None: - """Get cached response if available and not expired. - - Args: - url: The URL to look up. - - Returns: - CachedResponse if found and valid, None otherwise. - """ if url not in self._cache: return None cached = self._cache[url] @@ -232,5 +162,4 @@ def _get_cached(self, url: str) -> CachedResponse | None: return cached def clear_cache(self) -> None: - """Clear the response cache.""" self._cache.clear() diff --git a/src/agentic_cli/tools/webfetch/robots.py b/src/agentic_cli/tools/webfetch/robots.py index c66c396..6c0862c 100644 --- a/src/agentic_cli/tools/webfetch/robots.py +++ b/src/agentic_cli/tools/webfetch/robots.py @@ -9,67 +9,42 @@ class RobotsTxtChecker: - """Checks robots.txt compliance for URLs. - - Fetches and caches robots.txt per domain, then checks if our - user agent is allowed to access specific paths. - """ + """Checks robots.txt compliance, fetching robots.txt through the shared + SSRF-safe PinnedTransport (so the robots request is resolved-validated-pinned + like every other webfetch request).""" USER_AGENT = "AgenticCLI/1.0" + _MAX_ROBOTS_BYTES = 512 * 1024 - def __init__(self) -> None: - """Initialize the checker with empty cache.""" + def __init__(self, transport: httpx.AsyncBaseTransport | None = None) -> None: + self._transport = transport self._cache: dict[str, RobotFileParser | None] = {} async def can_fetch(self, url: str) -> bool: - """Check if URL can be fetched according to robots.txt. - - Args: - url: The URL to check. - - Returns: - True if allowed (or on error), False if explicitly disallowed. - """ parsed = urlparse(url) domain = f"{parsed.scheme}://{parsed.netloc}" - - # Get or fetch robots.txt for this domain if domain not in self._cache: self._cache[domain] = await self._fetch_robots(domain) - parser = self._cache[domain] if parser is None: - # No robots.txt or error fetching - be permissive - return True - + return True # no robots.txt / fetch error → permissive return parser.can_fetch(self.USER_AGENT, url) async def _fetch_robots(self, domain: str) -> RobotFileParser | None: - """Fetch and parse robots.txt for a domain. - - Args: - domain: The domain (scheme + netloc) to fetch robots.txt for. - - Returns: - Parsed RobotFileParser, or None if not found/error. - """ robots_url = f"{domain}/robots.txt" - try: - async with httpx.AsyncClient() as client: - response = await client.get(robots_url, timeout=10.0) - - if response.status_code != 200: - return None - - parser = RobotFileParser() - parser.parse(response.text.splitlines()) - return parser - + async with httpx.AsyncClient(transport=self._transport) as client: + async with client.stream("GET", robots_url, timeout=10.0) as response: + if response.status_code != 200: + return None + buf = bytearray() + async for chunk in response.aiter_bytes(): + buf.extend(chunk) + if len(buf) > self._MAX_ROBOTS_BYTES: + break + text = buf.decode(response.charset_encoding or "utf-8", errors="replace") + parser = RobotFileParser() + parser.parse(text.splitlines()) + return parser except Exception: - # Network errors, timeouts, etc. - be permissive - return None - - def clear_cache(self) -> None: - """Clear the robots.txt cache.""" - self._cache.clear() + return None # network / SSRF-block / timeout → permissive diff --git a/src/agentic_cli/tools/webfetch/transport.py b/src/agentic_cli/tools/webfetch/transport.py new file mode 100644 index 0000000..90bd7e4 --- /dev/null +++ b/src/agentic_cli/tools/webfetch/transport.py @@ -0,0 +1,52 @@ +"""SSRF-safe HTTP transport: resolve → validate → pin every request.""" + +from __future__ import annotations + +import httpx + +from agentic_cli.tools.webfetch.validator import URLValidator, BlockedAddressError + + +class PinnedTransport(httpx.AsyncBaseTransport): + """Connect every request to a validated pinned IP. + + Wraps an inner transport (default httpx.AsyncHTTPTransport(); a + httpx.MockTransport in tests). Resolves the host (all A+AAAA), requires all + addresses globally routable, and rewrites the request to connect to the + literal validated IP while preserving the Host header and TLS SNI — so cert + verification stays bound to the hostname and the validated address IS the + connected address (no TOCTOU). Sitting at the transport layer, it covers the + initial fetch, every manually-issued redirect hop, and the robots fetch. + """ + + def __init__( + self, + validator: URLValidator, + inner: httpx.AsyncBaseTransport | None = None, + ) -> None: + self._validator = validator + self._inner = inner if inner is not None else httpx.AsyncHTTPTransport() + + def _pin(self, request: httpx.Request) -> None: + host = request.url.host + port = request.url.port or (443 if request.url.scheme == "https" else 80) + pinned_ip = self._validator.resolve_and_validate(host, port) # raises on block + # The client set the Host header from the original URL before this + # transport runs; rewriting url.host to the IP does not touch it. Keep + # Host + TLS SNI bound to the hostname. + request.url = request.url.copy_with(host=pinned_ip) + request.headers.setdefault("Host", host if port in (80, 443) else f"{host}:{port}") + request.extensions["sni_hostname"] = host + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + self._pin(request) + return await self._inner.handle_async_request(request) + + async def aclose(self) -> None: + # No-op: this transport is SHARED and long-lived (one per fetcher, used + # by both the fetcher and the robots checker). Each webfetch call opens + # a short-lived httpx.AsyncClient(transport=self) and closes it on exit, + # which would otherwise tear down the shared inner pool under a + # concurrent call. The inner pool persists for the fetcher's lifetime + # (which also enables connection keep-alive across fetches). + return None diff --git a/src/agentic_cli/tools/webfetch/validator.py b/src/agentic_cli/tools/webfetch/validator.py index f78d1ce..840e987 100644 --- a/src/agentic_cli/tools/webfetch/validator.py +++ b/src/agentic_cli/tools/webfetch/validator.py @@ -7,6 +7,14 @@ from dataclasses import dataclass from urllib.parse import urlparse +import httpx + + +class BlockedAddressError(httpx.RequestError): + """A host resolves to a non-global / blocked address (or cannot be + resolved). Subclasses httpx.RequestError so the fetcher's existing + ``except httpx.RequestError`` maps it to a FetchResult error.""" + @dataclass class ValidationResult: @@ -17,137 +25,118 @@ class ValidationResult: resolved_ip: str | None = None +# Ranges ipaddress.is_global marks global but that are SSRF-risky. +_SUPPLEMENTARY_BLOCKED = [ + ipaddress.ip_network("64:ff9b::/96"), # NAT64 well-known prefix (embeds v4) + ipaddress.ip_network("64:ff9b:1::/48"), # NAT64 local-use prefix + ipaddress.ip_network("192.88.99.0/24"), # 6to4 relay anycast (deprecated) +] + + +def _ip_is_safe(ip_obj: ipaddress._BaseAddress) -> bool: + """True only if the address is globally routable and not SSRF-risky.""" + if not ip_obj.is_global: + return False + for net in _SUPPLEMENTARY_BLOCKED: + if ip_obj in net: + return False + mapped = getattr(ip_obj, "ipv4_mapped", None) + if mapped is not None and not _ip_is_safe(mapped): + return False + return True + + +def _as_ip_literal(host: str) -> ipaddress._BaseAddress | None: + """Return the IP if host is an IP literal (brackets stripped), else None.""" + try: + return ipaddress.ip_address(host.strip("[]")) + except ValueError: + return None + + class URLValidator: - """Validates URLs for security (SSRF protection) and policy compliance. + """Validates URLs for SSRF protection and policy compliance. - Checks: - - Allowed schemes (http, https only) - - Private/internal IP addresses blocked - - Configurable domain blocklist with wildcard support + ``validate`` performs non-DNS policy (scheme, blocked domains, IP-literal + safety). Authoritative resolution + pinning happens in + ``resolve_and_validate`` (used by PinnedTransport), which resolves every + A+AAAA and requires all of them to be globally routable. """ ALLOWED_SCHEMES = {"http", "https"} - BLOCKED_NETWORKS = [ - ipaddress.ip_network("127.0.0.0/8"), # Loopback - ipaddress.ip_network("10.0.0.0/8"), # Private A - ipaddress.ip_network("172.16.0.0/12"), # Private B - ipaddress.ip_network("192.168.0.0/16"), # Private C - ipaddress.ip_network("169.254.0.0/16"), # Link-local - ipaddress.ip_network("::1/128"), # IPv6 loopback - ipaddress.ip_network("fc00::/7"), # IPv6 private - ipaddress.ip_network("fe80::/10"), # IPv6 link-local - ] - def __init__(self, blocked_domains: list[str] | None = None) -> None: - """Initialize validator. - - Args: - blocked_domains: List of domains to block. Supports wildcards (*.example.com). - """ self.blocked_domains = blocked_domains or [] def validate(self, url: str) -> ValidationResult: - """Validate a URL for fetching. + """Scheme + blocked-domain + hostname-present + IP-literal safety. - Args: - url: The URL to validate. - - Returns: - ValidationResult with valid=True if OK, or valid=False with error message. + Does NOT resolve DNS — hostname resolution is done (once) by the + transport's resolve_and_validate, which pins the connection. """ - # Parse URL try: parsed = urlparse(url) except Exception as e: return ValidationResult(valid=False, error=f"Malformed URL: {e}") - # Check scheme if parsed.scheme not in self.ALLOWED_SCHEMES: return ValidationResult( valid=False, error=f"Scheme '{parsed.scheme}' not allowed. Use http or https.", ) - # Check hostname exists hostname = parsed.hostname if not hostname: return ValidationResult(valid=False, error="URL must have a hostname") - # Check blocked domains if self._is_domain_blocked(hostname): - return ValidationResult( - valid=False, - error=f"Domain '{hostname}' is blocked by policy", - ) - - # Resolve hostname and check for private IPs - try: - ip_str = socket.gethostbyname(hostname) - ip = ipaddress.ip_address(ip_str) + return ValidationResult(valid=False, error=f"Domain '{hostname}' is blocked by policy") - for network in self.BLOCKED_NETWORKS: - if ip in network: - return ValidationResult( - valid=False, - error=f"Private/internal IP address blocked: {ip_str}", - resolved_ip=ip_str, - ) - - return ValidationResult(valid=True, resolved_ip=ip_str) - - except socket.gaierror as e: + literal = _as_ip_literal(hostname) + if literal is not None and not _ip_is_safe(literal): return ValidationResult( valid=False, - error=f"Could not resolve hostname '{hostname}': {e}", + error=f"Private/internal IP address blocked: {literal}", + resolved_ip=str(literal), ) - def validate_ip(self, ip_str: str) -> ValidationResult: - """Validate an IP address against blocked networks. + return ValidationResult(valid=True) - Args: - ip_str: The IP address string to validate. + def resolve_and_validate(self, host: str, port: int) -> str: + """Resolve every A+AAAA for host and return one validated pinned IP. - Returns: - ValidationResult with valid=True if OK, or valid=False if blocked. + Rejects the whole host (BlockedAddressError) if ANY resolved address is + unsafe, so a split-horizon / rebinding resolver cannot smuggle a private + address alongside a public one. An IP-literal host is validated directly. """ - try: - ip = ipaddress.ip_address(ip_str) - except ValueError as e: - return ValidationResult(valid=False, error=f"Invalid IP address: {e}") - - for network in self.BLOCKED_NETWORKS: - if ip in network: - return ValidationResult( - valid=False, - error=f"Private/internal IP address blocked: {ip_str}", - resolved_ip=ip_str, - ) + literal = _as_ip_literal(host) + if literal is not None: + if not _ip_is_safe(literal): + raise BlockedAddressError(f"blocked non-global address: {host}") + return str(literal) - return ValidationResult(valid=True, resolved_ip=ip_str) + try: + infos = socket.getaddrinfo(host, port, type=socket.SOCK_STREAM) + except socket.gaierror as exc: + raise BlockedAddressError(f"could not resolve {host}: {exc}") from exc + + # sockaddr[0] is the IP; strip any IPv6 zone id ("fe80::1%eth0"). + ips = [ipaddress.ip_address(info[4][0].split("%")[0]) for info in infos] + if not ips: + raise BlockedAddressError(f"no addresses for {host}") + for ip_obj in ips: + if not _ip_is_safe(ip_obj): + raise BlockedAddressError(f"blocked non-global address {ip_obj} for {host}") + return str(ips[0]) def _is_domain_blocked(self, hostname: str) -> bool: - """Check if hostname matches any blocked domain pattern. - - Args: - hostname: The hostname to check. - - Returns: - True if blocked, False otherwise. - """ hostname_lower = hostname.lower() - for pattern in self.blocked_domains: pattern_lower = pattern.lower() - - # Exact match if hostname_lower == pattern_lower: return True - - # Wildcard match (*.example.com matches sub.example.com) if pattern_lower.startswith("*."): suffix = pattern_lower[1:] # .example.com if hostname_lower.endswith(suffix) and hostname_lower != pattern_lower[2:]: return True - return False diff --git a/src/agentic_cli/tools/webfetch_tool.py b/src/agentic_cli/tools/webfetch_tool.py index b37fa07..8369425 100644 --- a/src/agentic_cli/tools/webfetch_tool.py +++ b/src/agentic_cli/tools/webfetch_tool.py @@ -18,6 +18,7 @@ HTMLToMarkdown, build_summarize_prompt, ) +from agentic_cli.tools.webfetch.transport import PinnedTransport from agentic_cli.workflow.service_registry import require_service, LLM_SUMMARIZER @@ -60,11 +61,13 @@ def get_or_create_fetcher(settings=None) -> ContentFetcher: return _fetcher validator = URLValidator(blocked_domains=settings.webfetch_blocked_domains) - robots_checker = RobotsTxtChecker() + transport = PinnedTransport(validator) + robots_checker = RobotsTxtChecker(transport=transport) _fetcher = ContentFetcher( validator=validator, robots_checker=robots_checker, + transport=transport, cache_ttl_seconds=settings.webfetch_cache_ttl_seconds, max_content_bytes=settings.webfetch_max_content_bytes, max_pdf_bytes=settings.webfetch_max_pdf_bytes, diff --git a/tests/test_webfetch.py b/tests/test_webfetch.py index a90267c..d9d0f6b 100644 --- a/tests/test_webfetch.py +++ b/tests/test_webfetch.py @@ -1,12 +1,36 @@ """Tests for webfetch tool.""" import ipaddress +import socket +import httpx import pytest +from unittest.mock import AsyncMock, patch from agentic_cli.config import BaseSettings +def _pinned_fetcher(monkeypatch, handler, *, resolves_to="93.184.216.34", **kw): + """Build a ContentFetcher whose transport is a PinnedTransport over a + MockTransport(handler); getaddrinfo is stubbed so pinning succeeds.""" + from agentic_cli.tools.webfetch.validator import URLValidator + from agentic_cli.tools.webfetch.transport import PinnedTransport + from agentic_cli.tools.webfetch.robots import RobotsTxtChecker + from agentic_cli.tools.webfetch.fetcher import ContentFetcher + + def _gai(host, port, *a, **k): + return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (resolves_to, port))] + monkeypatch.setattr(socket, "getaddrinfo", _gai) + + validator = URLValidator() + transport = PinnedTransport(validator, inner=httpx.MockTransport(handler)) + robots = RobotsTxtChecker() # robots.can_fetch is patched in these tests, so + # its transport is irrelevant here (wired in Task 4) + return ContentFetcher( + validator=validator, robots_checker=robots, transport=transport, **kw + ) + + class TestWebFetchSettings: """Tests for webfetch settings fields.""" @@ -61,11 +85,10 @@ def test_invalid_scheme_file(self, validator): result = validator.validate("file:///etc/passwd") assert result.valid is False - def test_localhost_blocked(self, validator): - """Test localhost is blocked.""" - result = validator.validate("http://localhost/api") + def test_localhost_ip_literal_blocked(self, validator): + """A loopback IP literal is blocked by validate() without DNS.""" + result = validator.validate("http://127.0.0.1/api") assert result.valid is False - assert "private" in result.error.lower() or "blocked" in result.error.lower() def test_127_0_0_1_blocked(self, validator): """Test 127.0.0.1 is blocked.""" @@ -120,370 +143,183 @@ def test_malformed_url(self, validator): assert result.valid is False -class TestValidateIP: - """Tests for URLValidator.validate_ip() (C3: extracted IP check).""" - - @pytest.fixture - def validator(self): - from agentic_cli.tools.webfetch.validator import URLValidator - return URLValidator(blocked_domains=[]) - - def test_validate_ip_blocks_private(self, validator): - """Private IPs are blocked.""" - for ip in ("127.0.0.1", "10.0.0.1", "192.168.1.1", "172.16.0.1"): - result = validator.validate_ip(ip) - assert result.valid is False, f"{ip} should be blocked" - assert "blocked" in result.error.lower() - - def test_validate_ip_allows_public(self, validator): - """Public IPs pass validation.""" - result = validator.validate_ip("8.8.8.8") - assert result.valid is True - assert result.resolved_ip == "8.8.8.8" - - def test_validate_ip_invalid_string(self, validator): - """Invalid IP string returns error.""" - result = validator.validate_ip("not-an-ip") - assert result.valid is False - assert "invalid" in result.error.lower() - - -class TestPostFetchRevalidation: - """Tests for DNS rebinding protection in ContentFetcher (C3).""" - - @pytest.mark.asyncio - async def test_post_fetch_revalidation_blocks_rebind(self): - """If DNS resolves to private IP post-fetch, the response is rejected.""" - from agentic_cli.tools.webfetch.fetcher import ContentFetcher - from agentic_cli.tools.webfetch.validator import URLValidator, ValidationResult - from agentic_cli.tools.webfetch.robots import RobotsTxtChecker - from unittest.mock import AsyncMock, patch, MagicMock - - fetcher = ContentFetcher( - validator=URLValidator(blocked_domains=[]), - robots_checker=RobotsTxtChecker(), - ) - - # Mock validator.validate to allow pre-fetch (public IP) - with patch.object(fetcher._validator, "validate") as mock_validate: - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = "evil content" - mock_response.headers = {"content-type": "text/html"} - mock_response.history = [] - mock_response.url = "https://example.com/page" - mock_get.return_value = mock_response - - # Post-fetch DNS resolves to private IP (rebinding attack) - with patch("agentic_cli.tools.webfetch.fetcher.socket.gethostbyname") as mock_dns: - mock_dns.return_value = "127.0.0.1" - result = await fetcher.fetch("https://example.com/page") - - assert result.success is False - assert "rebinding" in result.error.lower() - - -from unittest.mock import AsyncMock, patch - - class TestRobotsTxtChecker: - """Tests for robots.txt compliance.""" - - @pytest.fixture - def checker(self): + def _checker(self, monkeypatch, handler, resolves_to="93.184.216.34"): + from agentic_cli.tools.webfetch.validator import URLValidator + from agentic_cli.tools.webfetch.transport import PinnedTransport from agentic_cli.tools.webfetch.robots import RobotsTxtChecker - return RobotsTxtChecker() + def _gai(host, port, *a, **k): + return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (resolves_to, port))] + monkeypatch.setattr(socket, "getaddrinfo", _gai) + transport = PinnedTransport(URLValidator(), inner=httpx.MockTransport(handler)) + return RobotsTxtChecker(transport=transport) @pytest.mark.asyncio - async def test_allowed_when_no_robots_txt(self, checker): - """Test URL is allowed when robots.txt doesn't exist.""" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_get.return_value.status_code = 404 - result = await checker.can_fetch("https://example.com/page") - assert result is True + async def test_allowed_when_no_robots_txt(self, monkeypatch): + checker = self._checker(monkeypatch, lambda req: httpx.Response(404)) + assert await checker.can_fetch("https://example.com/page") is True @pytest.mark.asyncio - async def test_allowed_when_robots_txt_error(self, checker): - """Test URL is allowed when robots.txt fetch fails.""" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_get.side_effect = Exception("Network error") - result = await checker.can_fetch("https://example.com/page") - assert result is True # Permissive on error + async def test_blocked_by_robots_txt(self, monkeypatch): + robots = "User-agent: *\nDisallow: /private/\n" + checker = self._checker(monkeypatch, lambda req: httpx.Response(200, text=robots)) + assert await checker.can_fetch("https://example.com/private/secret") is False @pytest.mark.asyncio - async def test_blocked_by_robots_txt(self, checker): - """Test URL is blocked when robots.txt disallows it.""" - robots_content = """ -User-agent: * -Disallow: /private/ -""" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = robots_content - mock_get.return_value = mock_response - result = await checker.can_fetch("https://example.com/private/secret") - assert result is False + async def test_allowed_by_robots_txt(self, monkeypatch): + robots = "User-agent: *\nDisallow: /private/\n" + checker = self._checker(monkeypatch, lambda req: httpx.Response(200, text=robots)) + assert await checker.can_fetch("https://example.com/public/page") is True @pytest.mark.asyncio - async def test_allowed_by_robots_txt(self, checker): - """Test URL is allowed when robots.txt permits it.""" - robots_content = """ -User-agent: * -Disallow: /private/ -""" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = robots_content - mock_get.return_value = mock_response - result = await checker.can_fetch("https://example.com/public/page") - assert result is True - - @pytest.mark.asyncio - async def test_robots_txt_cached(self, checker): - """Test robots.txt is cached per domain.""" - robots_content = "User-agent: *\nAllow: /" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = robots_content - mock_get.return_value = mock_response - - await checker.can_fetch("https://example.com/page1") - await checker.can_fetch("https://example.com/page2") - - # Should only fetch robots.txt once - assert mock_get.call_count == 1 - - -import time -import httpx + async def test_robots_txt_cached(self, monkeypatch): + calls = {"n": 0} + def handler(req): + calls["n"] += 1 + return httpx.Response(200, text="User-agent: *\nAllow: /\n") + checker = self._checker(monkeypatch, handler) + await checker.can_fetch("https://example.com/page1") + await checker.can_fetch("https://example.com/page2") + assert calls["n"] == 1 # one robots.txt fetch per domain class TestContentFetcher: - """Tests for content fetching with caching and redirect handling.""" - - @pytest.fixture - def fetcher(self): - from agentic_cli.tools.webfetch.fetcher import ContentFetcher - from agentic_cli.tools.webfetch.validator import URLValidator - from agentic_cli.tools.webfetch.robots import RobotsTxtChecker - return ContentFetcher( - validator=URLValidator(blocked_domains=[]), - robots_checker=RobotsTxtChecker(), - cache_ttl_seconds=900, - max_content_bytes=102400, - ) - @pytest.mark.asyncio - async def test_fetch_success(self, fetcher): - """Test successful fetch.""" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = "Content" - mock_response.headers = {"content-type": "text/html"} - mock_response.history = [] - mock_response.url = "https://example.com/page" - mock_get.return_value = mock_response - - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - result = await fetcher.fetch("https://example.com/page") - + async def test_fetch_success(self, monkeypatch): + fetcher = _pinned_fetcher( + monkeypatch, + lambda req: httpx.Response(200, text="Content", + headers={"content-type": "text/html"}), + ) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://example.com/page") assert result.success is True assert "Content" in result.content @pytest.mark.asyncio - async def test_fetch_blocked_by_validator(self, fetcher): - """Test fetch blocked by URL validator.""" + async def test_fetch_pins_to_validated_ip(self, monkeypatch): + seen = {} + def handler(req): + seen["host"] = req.url.host + seen["host_header"] = req.headers.get("Host") + return httpx.Response(200, text="ok", headers={"content-type": "text/html"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + await fetcher.fetch("https://example.com/page") + assert seen["host"] == "93.184.216.34" # connected to pinned IP + assert seen["host_header"] == "example.com" # Host preserved + + @pytest.mark.asyncio + async def test_fetch_blocked_by_validator_ip_literal(self, monkeypatch): + fetcher = _pinned_fetcher(monkeypatch, lambda req: httpx.Response(200)) result = await fetcher.fetch("http://127.0.0.1/internal") assert result.success is False - assert "blocked" in result.error.lower() or "private" in result.error.lower() + assert result.error @pytest.mark.asyncio - async def test_fetch_blocked_by_robots(self, fetcher): - """Test fetch blocked by robots.txt.""" - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = False - with patch.object(fetcher._validator, "validate") as mock_validate: - from agentic_cli.tools.webfetch.validator import ValidationResult - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - result = await fetcher.fetch("https://example.com/private/page") - + async def test_fetch_blocked_when_host_resolves_private(self, monkeypatch): + fetcher = _pinned_fetcher(monkeypatch, lambda req: httpx.Response(200), + resolves_to="10.0.0.5") + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://intranet.test/secret") assert result.success is False - assert "robots" in result.error.lower() + assert "block" in (result.error or "").lower() @pytest.mark.asyncio - async def test_fetch_cross_host_redirect_blocks_before_second_request(self, fetcher): - """A 3xx redirect to a different host must be reported as a cross-host - redirect WITHOUT issuing the next GET. The user's permission grant - covers the original host only, so contacting another origin is an - unapproved side effect.""" - redirect_response = AsyncMock() - redirect_response.status_code = 302 - redirect_response.headers = {"location": "https://other.com/page"} - redirect_response.url = httpx.URL("https://example.com/page") - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_get.return_value = redirect_response - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - with patch.object(fetcher._validator, "validate") as mock_validate: - from agentic_cli.tools.webfetch.validator import ValidationResult - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - result = await fetcher.fetch("https://example.com/page") - + async def test_fetch_blocked_by_robots(self, monkeypatch): + fetcher = _pinned_fetcher(monkeypatch, lambda req: httpx.Response(200, text="x")) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=False): + result = await fetcher.fetch("https://example.com/private/page") assert result.success is False - assert result.redirect is not None - assert result.redirect.to_host == "other.com" - # Only the original GET should have been issued. - assert mock_get.call_count == 1 + assert "robots" in result.error.lower() @pytest.mark.asyncio - async def test_fetch_same_host_redirect_blocked_by_robots(self, fetcher): - """A same-host redirect whose target is disallowed by robots.txt - must not be fetched, even though the original URL was allowed.""" - redirect_response = AsyncMock() - redirect_response.status_code = 302 - redirect_response.headers = {"location": "/private/page"} - redirect_response.url = httpx.URL("https://example.com/public") - - robots_calls: list[str] = [] - + async def test_same_host_redirect_blocked_by_robots(self, monkeypatch): + def handler(req): + if req.url.path == "/public": + return httpx.Response(302, headers={"location": "/private/page"}) + return httpx.Response(200, text="x", headers={"content-type": "text/html"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + robots_calls = [] async def fake_can_fetch(u): robots_calls.append(u) return "/private" not in u + with patch.object(fetcher._robots, "can_fetch", side_effect=fake_can_fetch): + result = await fetcher.fetch("https://example.com/public") + assert result.success is False + assert "robots" in result.error.lower() + assert any("/private" in u for u in robots_calls) - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_get.return_value = redirect_response - with patch.object(fetcher._robots, "can_fetch", side_effect=fake_can_fetch): - with patch.object(fetcher._validator, "validate") as mock_validate: - from agentic_cli.tools.webfetch.validator import ValidationResult - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - result = await fetcher.fetch("https://example.com/public") - + @pytest.mark.asyncio + async def test_cross_host_redirect_blocks_before_second_request(self, monkeypatch): + calls = {"n": 0} + def handler(req): + calls["n"] += 1 + return httpx.Response(302, headers={"location": "https://other.com/page"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://example.com/page") assert result.success is False - assert "robots" in (result.error or "").lower() - # Only the original GET should have fired; redirected URL was blocked - # before its request. - assert mock_get.call_count == 1 - # robots.txt was consulted for both the original and the redirect target. - assert any("/public" in u for u in robots_calls) - assert any("/private/page" in u for u in robots_calls) + assert result.redirect is not None and result.redirect.to_host == "other.com" + assert calls["n"] == 1 # next GET never issued @pytest.mark.asyncio - async def test_fetch_redirect_to_internal_ip_blocked_before_request(self, fetcher): - """A redirect Location pointing at a private IP is blocked without issuing the next request.""" - redirect_response = AsyncMock() - redirect_response.status_code = 302 - redirect_response.headers = {"location": "http://169.254.169.254/latest/meta-data/"} - redirect_response.url = httpx.URL("https://example.com/page") - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_get.return_value = redirect_response - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - # First validate() call (original URL) passes; the second - # call (redirect target) is the real one — we let the real - # validator catch the private IP. - from agentic_cli.tools.webfetch.validator import ValidationResult, URLValidator - real_validator = URLValidator() - with patch.object( - fetcher._validator, "validate", - side_effect=[ - ValidationResult(valid=True, resolved_ip="1.2.3.4"), - real_validator.validate("http://169.254.169.254/latest/meta-data/"), - ], - ): - result = await fetcher.fetch("https://example.com/page") + async def test_same_host_redirect_followed(self, monkeypatch): + def handler(req): + if req.url.path == "/public": + return httpx.Response(302, headers={"location": "/inner"}) + return httpx.Response(200, text="inner page", headers={"content-type": "text/html"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://example.com/public") + assert result.success is True + assert "inner page" in result.content + @pytest.mark.asyncio + async def test_redirect_to_internal_ip_blocked(self, monkeypatch): + def handler(req): + return httpx.Response(302, headers={"location": "http://169.254.169.254/latest/"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://example.com/page") assert result.success is False assert "redirect" in (result.error or "").lower() - # Only the first GET (the original URL) should have been issued. - assert mock_get.call_count == 1 @pytest.mark.asyncio - async def test_fetch_too_many_redirects(self, fetcher): - """A redirect loop longer than MAX_REDIRECTS is rejected.""" - from agentic_cli.tools.webfetch.fetcher import ContentFetcher - - def make_redirect(i: int): - r = AsyncMock() - r.status_code = 302 - r.headers = {"location": f"https://example.com/hop{i + 1}"} - r.url = httpx.URL(f"https://example.com/hop{i}") - return r - - responses = [make_redirect(i) for i in range(ContentFetcher.MAX_REDIRECTS + 1)] - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_get.side_effect = responses - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - with patch.object(fetcher._validator, "validate") as mock_validate: - from agentic_cli.tools.webfetch.validator import ValidationResult - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - result = await fetcher.fetch("https://example.com/hop0") - + async def test_too_many_redirects(self, monkeypatch): + def handler(req): + n = int(req.url.path.rsplit("hop", 1)[-1]) + return httpx.Response(302, headers={"location": f"https://example.com/hop{n + 1}"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://example.com/hop0") assert result.success is False assert "redirect" in (result.error or "").lower() @pytest.mark.asyncio - async def test_fetch_caching(self, fetcher): - """Test responses are cached.""" - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = "Cached content" - mock_response.headers = {"content-type": "text/html"} - mock_response.history = [] - mock_response.url = "https://example.com/page" - mock_get.return_value = mock_response - - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - with patch.object(fetcher._validator, "validate") as mock_validate: - from agentic_cli.tools.webfetch.validator import ValidationResult - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - - result1 = await fetcher.fetch("https://example.com/page") - result2 = await fetcher.fetch("https://example.com/page") - - assert result1.from_cache is False - assert result2.from_cache is True - assert mock_get.call_count == 1 + async def test_caching(self, monkeypatch): + calls = {"n": 0} + def handler(req): + calls["n"] += 1 + return httpx.Response(200, text="Cached content", headers={"content-type": "text/html"}) + fetcher = _pinned_fetcher(monkeypatch, handler) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + r1 = await fetcher.fetch("https://example.com/page") + r2 = await fetcher.fetch("https://example.com/page") + assert r1.from_cache is False and r2.from_cache is True + assert calls["n"] == 1 @pytest.mark.asyncio - async def test_fetch_content_truncation(self, fetcher): - """Test content is truncated when too large.""" - large_content = "x" * 200000 - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.text = large_content - mock_response.headers = {"content-type": "text/plain"} - mock_response.history = [] - mock_response.url = "https://example.com/large" - mock_get.return_value = mock_response - - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - with patch.object(fetcher._validator, "validate") as mock_validate: - from agentic_cli.tools.webfetch.validator import ValidationResult - mock_validate.return_value = ValidationResult(valid=True, resolved_ip="1.2.3.4") - result = await fetcher.fetch("https://example.com/large") - - assert result.success is True - assert result.truncated is True + async def test_content_truncation_streamed(self, monkeypatch): + big = "x" * 200000 + fetcher = _pinned_fetcher( + monkeypatch, + lambda req: httpx.Response(200, text=big, headers={"content-type": "text/plain"}), + max_content_bytes=102400, + ) + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://example.com/large") + assert result.success is True and result.truncated is True assert len(result.content) <= fetcher._max_content_bytes + 100 @@ -608,69 +444,29 @@ def test_convert_pdf_no_text(self, converter): assert "no extractable text" in result.lower() or "Page" in result @pytest.mark.asyncio - async def test_fetcher_pdf_uses_bytes(self): - """Test fetcher uses response.content (bytes) for PDFs.""" - from agentic_cli.tools.webfetch.fetcher import ContentFetcher - from agentic_cli.tools.webfetch.validator import URLValidator, ValidationResult - from agentic_cli.tools.webfetch.robots import RobotsTxtChecker - - fetcher = ContentFetcher( - validator=URLValidator(blocked_domains=[]), - robots_checker=RobotsTxtChecker(), + async def test_fetcher_pdf_uses_bytes(self, monkeypatch): + pdf = b"%PDF-1.4 fake pdf content" + fetcher = _pinned_fetcher( + monkeypatch, + lambda req: httpx.Response(200, content=pdf, headers={"content-type": "application/pdf"}), ) - - pdf_bytes = b"%PDF-1.4 fake pdf content" - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.content = pdf_bytes - mock_response.text = "garbled text" - mock_response.headers = {"content-type": "application/pdf"} - mock_response.history = [] - mock_response.url = "https://arxiv.org/pdf/2301.00001" - mock_get.return_value = mock_response - - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - result = await fetcher.fetch("https://arxiv.org/pdf/2301.00001") - + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://arxiv.org/pdf/2301.00001") assert result.success is True - assert result.content == pdf_bytes # Should be bytes, not garbled text - assert isinstance(result.content, bytes) + assert result.content == pdf and isinstance(result.content, bytes) @pytest.mark.asyncio - async def test_fetcher_pdf_byte_limit(self): - """Test large PDF is truncated at max_pdf_bytes.""" - from agentic_cli.tools.webfetch.fetcher import ContentFetcher - from agentic_cli.tools.webfetch.validator import URLValidator, ValidationResult - from agentic_cli.tools.webfetch.robots import RobotsTxtChecker - - max_pdf = 1000 - fetcher = ContentFetcher( - validator=URLValidator(blocked_domains=[]), - robots_checker=RobotsTxtChecker(), - max_pdf_bytes=max_pdf, + async def test_fetcher_pdf_byte_limit(self, monkeypatch): + big = b"x" * 5000 + fetcher = _pinned_fetcher( + monkeypatch, + lambda req: httpx.Response(200, content=big, headers={"content-type": "application/pdf"}), + max_pdf_bytes=1000, ) - - large_pdf = b"x" * 5000 - - with patch("httpx.AsyncClient.get", new_callable=AsyncMock) as mock_get: - mock_response = AsyncMock() - mock_response.status_code = 200 - mock_response.content = large_pdf - mock_response.headers = {"content-type": "application/pdf"} - mock_response.history = [] - mock_response.url = "https://arxiv.org/pdf/2301.00001" - mock_get.return_value = mock_response - - with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock) as mock_robots: - mock_robots.return_value = True - result = await fetcher.fetch("https://arxiv.org/pdf/2301.00001") - - assert result.success is True - assert result.truncated is True - assert len(result.content) == max_pdf + with patch.object(fetcher._robots, "can_fetch", new_callable=AsyncMock, return_value=True): + result = await fetcher.fetch("https://arxiv.org/pdf/2301.00001") + assert result.success is True and result.truncated is True + assert len(result.content) == 1000 class TestWebFetchPDFSetting: @@ -1029,3 +825,23 @@ async def _inject_session_messages(self, session_id: str, messages: list[dict], # The manager itself should be the summarizer (has summarize() method) assert manager.llm_summarizer is manager assert hasattr(manager.llm_summarizer, "summarize") + + +class TestFactoryWiring: + def test_get_or_create_fetcher_shares_one_pinned_transport(self): + import agentic_cli.tools.webfetch_tool as wt + from agentic_cli.tools.webfetch.transport import PinnedTransport + from agentic_cli.config import BaseSettings + + orig_fetcher = wt._fetcher + orig_snapshot = wt._fetcher_settings_snapshot + try: + wt._fetcher = None + wt._fetcher_settings_snapshot = None + fetcher = wt.get_or_create_fetcher(BaseSettings()) + assert isinstance(fetcher._transport, PinnedTransport) + assert fetcher._robots._transport is fetcher._transport + assert fetcher._transport._validator is fetcher._validator + finally: + wt._fetcher = orig_fetcher + wt._fetcher_settings_snapshot = orig_snapshot diff --git a/tests/tools/test_webfetch_ssrf.py b/tests/tools/test_webfetch_ssrf.py new file mode 100644 index 0000000..99d1dcd --- /dev/null +++ b/tests/tools/test_webfetch_ssrf.py @@ -0,0 +1,183 @@ +"""P0-5 SSRF tests: resolve-validate-pin. Fully offline — socket.getaddrinfo is +monkeypatched and httpx.MockTransport is the PinnedTransport inner.""" + +import socket + +import httpx +import pytest + +from agentic_cli.tools.webfetch.transport import PinnedTransport +from agentic_cli.tools.webfetch.validator import ( + URLValidator, + BlockedAddressError, + _ip_is_safe, + _as_ip_literal, +) + + +def stub_getaddrinfo(*ips): + """Return a socket.getaddrinfo replacement yielding the given IP strings.""" + def _stub(host, port, *args, **kwargs): + out = [] + for ip in ips: + is6 = ":" in ip + fam = socket.AF_INET6 if is6 else socket.AF_INET + sa = (ip, port, 0, 0) if is6 else (ip, port) + out.append((fam, socket.SOCK_STREAM, 6, "", sa)) + return out + return _stub + + +import ipaddress + + +class TestIpIsSafe: + def test_public_v4_and_v6_safe(self): + assert _ip_is_safe(ipaddress.ip_address("8.8.8.8")) + assert _ip_is_safe(ipaddress.ip_address("2606:4700:4700::1111")) + + @pytest.mark.parametrize("ip", [ + "127.0.0.1", "10.0.0.1", "192.168.1.1", "172.16.0.1", "169.254.169.254", + "100.64.0.1", "192.0.2.1", "198.18.0.1", "240.0.0.1", "0.0.0.1", + "::1", "fc00::1", "fe80::1", + "::ffff:10.0.0.1", # v4-mapped private + "64:ff9b::a00:1", # NAT64 embedding 10.0.0.1 (is_global=True) + "192.88.99.1", # 6to4 relay anycast (deprecated) + ]) + def test_unsafe_addresses(self, ip): + assert not _ip_is_safe(ipaddress.ip_address(ip)) + + +class TestResolveAndValidate: + def test_public_host_returns_ip(self, monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("93.184.216.34")) + assert URLValidator().resolve_and_validate("example.com", 443) == "93.184.216.34" + + def test_private_host_rejected(self, monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("127.0.0.1")) + with pytest.raises(BlockedAddressError): + URLValidator().resolve_and_validate("localhost", 80) + + def test_mixed_public_and_private_rejected(self, monkeypatch): + # split-horizon: a public + a private answer → reject the whole host + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("93.184.216.34", "10.0.0.1")) + with pytest.raises(BlockedAddressError): + URLValidator().resolve_and_validate("rebind.test", 443) + + def test_ipv6_ula_rejected(self, monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("fc00::1")) + with pytest.raises(BlockedAddressError): + URLValidator().resolve_and_validate("v6.test", 443) + + def test_unresolvable_host_raises_blocked(self, monkeypatch): + def boom(*a, **k): + raise socket.gaierror("nope") + monkeypatch.setattr(socket, "getaddrinfo", boom) + with pytest.raises(BlockedAddressError): + URLValidator().resolve_and_validate("nx.test", 443) + + def test_ip_literal_public_ok_private_blocked(self): + assert URLValidator().resolve_and_validate("8.8.8.8", 443) == "8.8.8.8" + with pytest.raises(BlockedAddressError): + URLValidator().resolve_and_validate("169.254.169.254", 80) + + +class TestValidateNoDNS: + def test_ip_literal_private_blocked_without_dns(self): + # validate() must reject a private IP-literal with no DNS call + r = URLValidator().validate("http://127.0.0.1/x") + assert r.valid is False + + def test_public_hostname_passes_policy(self, monkeypatch): + # validate() must NOT resolve DNS — make resolution explode and assert + # a normal hostname still passes policy. + def _boom(*a, **k): + raise AssertionError("validate() must not call DNS") + monkeypatch.setattr(socket, "getaddrinfo", _boom) + monkeypatch.setattr(socket, "gethostbyname", _boom) + assert URLValidator().validate("https://example.com/x").valid is True + + +def mock_pinned_transport(handler, validator=None): + """PinnedTransport whose inner is an httpx.MockTransport(handler).""" + from agentic_cli.tools.webfetch.transport import PinnedTransport + return PinnedTransport(validator or URLValidator(), inner=httpx.MockTransport(handler)) + + +class TestPinnedTransport: + @pytest.mark.asyncio + async def test_pins_to_validated_ip_preserving_host_and_sni(self, monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("93.184.216.34")) + seen = {} + + def handler(request): + seen["url_host"] = request.url.host + seen["host_header"] = request.headers.get("Host") + seen["sni"] = request.extensions.get("sni_hostname") + return httpx.Response(200, text="ok") + + transport = mock_pinned_transport(handler) + async with httpx.AsyncClient(transport=transport) as client: + resp = await client.get("https://example.com/page") + + assert resp.status_code == 200 + assert seen["url_host"] == "93.184.216.34" # connected to the pinned IP + assert seen["host_header"] == "example.com" # Host preserved + assert seen["sni"] == "example.com" # TLS SNI bound to hostname + + @pytest.mark.asyncio + async def test_blocked_host_raises_and_never_calls_inner(self, monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("169.254.169.254")) + called = {"inner": False} + + def handler(request): + called["inner"] = True + return httpx.Response(200) + + transport = mock_pinned_transport(handler) + async with httpx.AsyncClient(transport=transport) as client: + with pytest.raises(BlockedAddressError): + await client.get("http://metadata.test/latest") + assert called["inner"] is False # never connected + + +class TestRobotsThroughTransport: + @pytest.mark.asyncio + async def test_robots_fetch_for_private_host_is_refused_at_transport(self, monkeypatch): + """A host resolving to a private IP: the robots fetch is refused by the + transport (no connection), and can_fetch stays permissive.""" + from agentic_cli.tools.webfetch.robots import RobotsTxtChecker + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("169.254.169.254")) + inner_called = {"n": 0} + def handler(req): + inner_called["n"] += 1 + return httpx.Response(200, text="User-agent: *\nDisallow: /\n") + checker = RobotsTxtChecker(transport=mock_pinned_transport(handler)) + allowed = await checker.can_fetch("http://metadata.test/x") + assert inner_called["n"] == 0 # never connected to the private IP + assert allowed is True # permissive on the (blocked) fetch error + + +class _SpyInner(httpx.MockTransport): + def __init__(self, handler): + super().__init__(handler) + self.closed = False + + async def aclose(self): + self.closed = True + await super().aclose() + + +class TestSharedTransportLifecycle: + @pytest.mark.asyncio + async def test_client_close_does_not_close_shared_inner(self, monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", stub_getaddrinfo("93.184.216.34")) + inner = _SpyInner(lambda req: httpx.Response(200, text="ok")) + transport = PinnedTransport(URLValidator(), inner=inner) + async with httpx.AsyncClient(transport=transport) as client: + await client.get("https://example.com/x") + assert inner.closed is False # shared inner NOT closed by client exit + # ...and the transport is still usable for a subsequent client + async with httpx.AsyncClient(transport=transport) as client2: + r = await client2.get("https://example.com/y") + assert r.status_code == 200