Merge pull request #353 from andrewyng/rpSsrfFollowup

security: block CGNAT range and guard browser_open_url

coworker/connectors/browser_automation.py

@@ -17,6 +17,8 @@ from typing import Any, Callable, Optional
 
 import aisuite as ai
 
+from ..web.guard import check_url
+
 
 def _meta(
     name: str, *, approval: bool = False, capabilities: Optional[list[str]] = None
@@ -332,6 +334,12 @@ def make_browser_automation_tools() -> list[Callable[..., Any]]:
     ) -> dict[str, Any]:
         if not url.lower().startswith(("http://", "https://")):
             return {"error": "url must start with http:// or https://"}
+        # Same address guard as web_fetch. This is approval gated, so it is defense in
+        # depth, not the primary control. It checks the initial model supplied URL only;
+        # redirects that the browser follows internally are not hop checked here.
+        blocked = check_url(url)
+        if blocked:
+            return {"error": blocked}
         return _BROWSER.call(
             "open_url",
             lambda page: (

coworker/web/guard.py

@@ -28,6 +28,11 @@ from urllib.parse import urlsplit
 
 MAX_REDIRECTS = 5
 
+# RFC 6598 shared address space. Python's is_private misses it, but it is carrier grade
+# NAT space and Tailscale hands out internal hosts here (100.64.0.0/10), so a fetch to it
+# is the same "reach the machine's network position" class as RFC1918.
+_CGNAT = ipaddress.ip_network("100.64.0.0/10")
+
 
 def _blocked_reason(ip: ipaddress._BaseAddress) -> Optional[str]:
     if ip.is_loopback:
@@ -36,6 +41,8 @@ def _blocked_reason(ip: ipaddress._BaseAddress) -> Optional[str]:
         return "link-local (includes the cloud metadata endpoint)"
     if ip.is_private:
         return "a private network"
+    if ip.version == 4 and ip in _CGNAT:
+        return "shared address space (CGNAT / RFC 6598)"
     if ip.is_multicast:
         return "multicast"
     if ip.is_reserved or ip.is_unspecified:

tests/test_url_address_guard.py

@@ -31,12 +31,22 @@ def _resolves_to(monkeypatch, ip: str):
     ("http://192.168.1.1/", "private"),
     ("http://172.16.4.4/", "private"),
     ("http://0.0.0.0/", "refusing to fetch"),  # 0.0.0.0/8 lands in is_private first
+    ("http://100.64.0.1/", "CGNAT"),  # RFC 6598 shared space (Tailscale, CGNAT)
+    ("http://100.127.255.254/", "CGNAT"),
 ])
 def test_blocked_literals(url, needle):
     reason = guard.check_url(url)
     assert reason and needle in reason
 
 
+def test_cgnat_neighbours_still_allowed(monkeypatch):
+    """100.64.0.0/10 is blocked, but the adjacent public 100.63/100.128 space is not."""
+    _resolves_to(monkeypatch, "100.63.255.255")
+    assert guard.check_url("http://below.example/") is None
+    _resolves_to(monkeypatch, "100.128.0.0")
+    assert guard.check_url("http://above.example/") is None
+
+
 def test_ipv4_mapped_ipv6_loopback_is_blocked():
     """::ffff:127.0.0.1 must be judged as the v4 address it carries."""
     assert guard.check_url("http://[::ffff:127.0.0.1]/")
@@ -155,3 +165,14 @@ def test_web_fetch_returns_the_refusal_as_a_tool_error(monkeypatch):
 
 def test_web_fetch_still_rejects_non_http_schemes():
     assert "http" in make_web_fetch_tool()("file:///etc/passwd")["error"]
+
+
+def test_browser_open_url_is_guarded_and_never_launches(monkeypatch):
+    """The Playwright browser_open_url is approval gated, but the address guard still
+    refuses a blocked URL before the browser is touched (defense in depth)."""
+    from coworker.connectors.browser_automation import make_browser_automation_tools
+
+    open_url = {t.__name__: t for t in make_browser_automation_tools()}["browser_open_url"]
+    out = open_url("http://169.254.169.254/latest/meta-data/")
+    assert "link-local" in out["error"]
+    assert out.get("ok") is None