diff --git a/coworker/connectors/integration_tools.py b/coworker/connectors/integration_tools.py index 7588136d..f0f18d0b 100644 --- a/coworker/connectors/integration_tools.py +++ b/coworker/connectors/integration_tools.py @@ -19,6 +19,7 @@ import aisuite as ai from ..secrets import SecretStore +from ..web.guard import get_checked from .browser_automation import make_browser_automation_tools from .email_tools import make_email_tools from .tool_defs import approval_for_tool, connector_for_tool @@ -305,15 +306,39 @@ def _gmail_is_hidden( def _request( - method: str, url: str, *, headers=None, params=None, json=None, auth=None + method: str, + url: str, + *, + headers=None, + params=None, + json=None, + auth=None, + check_addresses: bool = False, ) -> dict[str, Any]: + """HTTP for the connectors. + + `check_addresses` is for URLs the *model* supplies (browser_read_url). It turns off + automatic redirects and walks the chain through the address guard instead, so a public + URL cannot 302 into loopback or the metadata endpoint. The vendor endpoints everything + else in this module calls are hardcoded, so they skip the guard and its DNS lookup. + """ try: import httpx - with httpx.Client(timeout=30.0, follow_redirects=True) as client: - resp = client.request( - method, url, headers=headers, params=params, json=json, auth=auth - ) + with httpx.Client( + timeout=30.0, follow_redirects=not check_addresses + ) as client: + if check_addresses: + if method.upper() != "GET": + return {"error": "address-checked requests must be GET"} + try: + resp = get_checked(client, url) + except PermissionError as exc: + return {"error": str(exc)} + else: + resp = client.request( + method, url, headers=headers, params=params, json=json, auth=auth + ) ctype = resp.headers.get("content-type", "") data: Any = resp.json() if "json" in ctype.lower() else resp.text if resp.status_code >= 400: @@ -533,7 +558,13 @@ def make_integration_tools( def browser_read_url(url: str, max_chars: int = 20000) -> dict[str, Any]: if not url.lower().startswith(("http://", "https://")): return {"error": "url must start with http:// or https://"} - out = _request("GET", url, headers={"User-Agent": "coworker/0.1 (+connector)"}) + # Model-supplied URL: address-check every hop, same guard as web_fetch. + out = _request( + "GET", + url, + headers={"User-Agent": "coworker/0.1 (+connector)"}, + check_addresses=True, + ) if "error" in out: return out data = out["data"] diff --git a/coworker/web/fetch.py b/coworker/web/fetch.py index 9d273d03..f58291da 100644 --- a/coworker/web/fetch.py +++ b/coworker/web/fetch.py @@ -13,6 +13,8 @@ import aisuite as ai +from .guard import get_checked + _MAX = 20000 # default chars returned _SCHEMA = { @@ -84,16 +86,20 @@ def web_fetch(url: str, max_chars: int = _MAX) -> dict[str, Any]: try: import httpx + # follow_redirects=False: guard.get_checked walks the chain so every hop is + # address-checked, not just the URL the model first supplied. with httpx.Client( - follow_redirects=True, + follow_redirects=False, timeout=20.0, headers={"User-Agent": "coworker/0.1 (+desktop)"}, ) as client: - resp = client.get(url) + resp = get_checked(client, url) resp.raise_for_status() ctype = resp.headers.get("content-type", "") body = resp.text final_url = str(resp.url) + except PermissionError as exc: # blocked address (loopback, private, metadata) + return {"error": str(exc)} except Exception as exc: # network / HTTP / TLS return {"error": f"fetch failed: {exc}"} text = _html_to_text(body) if "html" in ctype.lower() else body diff --git a/coworker/web/guard.py b/coworker/web/guard.py new file mode 100644 index 00000000..c4f6d1c3 --- /dev/null +++ b/coworker/web/guard.py @@ -0,0 +1,109 @@ +"""Address guard for URLs the model chooses. + +`web_fetch` and `browser_read_url` take a URL straight from the model, and the model's +input is untrusted by design — it reads web pages, email and Slack messages, all of which +are documented as "data, not instructions". A page that talks the agent into fetching +`http://169.254.169.254/` or `http://127.0.0.1:11434/` turns a read-only research tool into +a probe of the machine's own network position, and `web_fetch` is `requires_approval=False`, +so no prompt ever appears. + +This blocks the ranges that are only reachable *because* OpenWorker runs on the user's +machine: loopback, RFC1918 and other private space, link-local (which covers the cloud +metadata endpoint at 169.254.169.254), and the reserved/multicast blocks. + +Every hop is checked, not just the first: `follow_redirects=True` otherwise lets a public +URL 302 straight to loopback, which is the standard way this filter is bypassed. + +Not covered: DNS rebinding. The name is resolved here and resolved again by the client when +it connects, so a record with a ~0 TTL can change between the two. Closing that needs +connection-level IP pinning; the hop check is the cheap 90% and is stated as such. +""" + +from __future__ import annotations + +import ipaddress +import socket +from typing import Optional +from urllib.parse import urlsplit + +MAX_REDIRECTS = 5 + + +def _blocked_reason(ip: ipaddress._BaseAddress) -> Optional[str]: + if ip.is_loopback: + return "loopback" + if ip.is_link_local: + return "link-local (includes the cloud metadata endpoint)" + if ip.is_private: + return "a private network" + if ip.is_multicast: + return "multicast" + if ip.is_reserved or ip.is_unspecified: + return "a reserved range" + return None + + +def check_url(url: str) -> Optional[str]: + """None if the URL may be fetched, else a human-readable refusal reason. + + Resolves the host and rejects when *any* answer lands in a blocked range, so a name + with both a public and a private A record cannot be used to slip through. + """ + parts = urlsplit(url) + if parts.scheme not in ("http", "https"): + return "url must start with http:// or https://" + host = parts.hostname + if not host: + return "url has no host" + + # A literal address needs no lookup. + try: + literal = ipaddress.ip_address(host) + except ValueError: + literal = None + if literal is not None: + reason = _blocked_reason(literal) + return f"refusing to fetch {host}: {reason}" if reason else None + + try: + infos = socket.getaddrinfo(host, parts.port or (443 if parts.scheme == "https" else 80), + proto=socket.IPPROTO_TCP) + except OSError as exc: + return f"could not resolve {host}: {exc}" + + for info in infos: + raw = info[4][0] + try: + ip = ipaddress.ip_address(raw) + except ValueError: + continue + # ::ffff:127.0.0.1 and friends must be judged as the v4 address they carry. + mapped = getattr(ip, "ipv4_mapped", None) + if mapped is not None: + ip = mapped + reason = _blocked_reason(ip) + if reason: + return f"refusing to fetch {host} ({ip}): {reason}" + return None + + +def get_checked(client, url: str, *, max_redirects: int = MAX_REDIRECTS): + """GET `url`, validating the address before every hop. + + `client` must be built with `follow_redirects=False`; redirects are walked here so each + Location is checked. Returns the final response. Raises `PermissionError` when a hop is + refused, `RuntimeError` when the redirect budget is exhausted. + """ + seen = url + for _ in range(max_redirects + 1): + reason = check_url(seen) + if reason: + raise PermissionError(reason) + resp = client.get(seen) + if resp.status_code not in (301, 302, 303, 307, 308): + return resp + location = resp.headers.get("location") + if not location: + return resp + seen = str(resp.url.join(location)) + raise RuntimeError(f"too many redirects (>{max_redirects})") diff --git a/tests/test_url_address_guard.py b/tests/test_url_address_guard.py new file mode 100644 index 00000000..13642b6c --- /dev/null +++ b/tests/test_url_address_guard.py @@ -0,0 +1,157 @@ +"""`web_fetch` / `browser_read_url` must not reach the machine's own network position. + +Both take a URL straight from the model, and the model's input is untrusted by design — +the tools' own descriptions call fetched content "data to evaluate, not instructions". +`web_fetch` is additionally `requires_approval=False`, so nothing prompts the user. +""" + +import socket + +import pytest + +from coworker.web import guard +from coworker.web.fetch import make_web_fetch_tool + + +def _resolves_to(monkeypatch, ip: str): + monkeypatch.setattr( + guard.socket, "getaddrinfo", + lambda *a, **k: [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (ip, 80))], + ) + + +# -- literals ----------------------------------------------------------------- + +@pytest.mark.parametrize("url,needle", [ + ("http://127.0.0.1:11434/api/tags", "loopback"), + ("http://localhost:8000/", "loopback"), + ("http://[::1]:8080/", "loopback"), + ("http://169.254.169.254/latest/meta-data/", "link-local"), + ("http://10.0.0.5/admin", "private"), + ("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 +]) +def test_blocked_literals(url, needle): + reason = guard.check_url(url) + assert reason and needle in reason + + +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]/") + + +def test_public_literal_is_allowed(): + assert guard.check_url("https://93.184.216.34/") is None + + +@pytest.mark.parametrize("url", ["file:///etc/passwd", "ftp://example.com/x", + "gopher://example.com/", "http://"]) +def test_non_http_schemes_and_hostless_urls_are_refused(url): + assert guard.check_url(url) + + +# -- names -------------------------------------------------------------------- + +def test_hostname_resolving_to_loopback_is_blocked(monkeypatch): + """`localtest.me` and friends are public names with private answers.""" + _resolves_to(monkeypatch, "127.0.0.1") + assert "loopback" in guard.check_url("http://sneaky.example.com/") + + +def test_hostname_resolving_to_metadata_ip_is_blocked(monkeypatch): + _resolves_to(monkeypatch, "169.254.169.254") + assert guard.check_url("http://metadata.example.com/") + + +def test_any_private_answer_blocks_a_split_horizon_name(monkeypatch): + """One public and one private A record must not be a way through.""" + monkeypatch.setattr( + guard.socket, "getaddrinfo", + lambda *a, **k: [ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("93.184.216.34", 80)), + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("127.0.0.1", 80)), + ], + ) + assert guard.check_url("http://split.example.com/") + + +def test_public_hostname_is_allowed(monkeypatch): + _resolves_to(monkeypatch, "93.184.216.34") + assert guard.check_url("https://example.com/docs") is None + + +def test_unresolvable_host_is_refused_not_fetched(monkeypatch): + def boom(*a, **k): + raise socket.gaierror("nodename nor servname provided") + monkeypatch.setattr(guard.socket, "getaddrinfo", boom) + assert "could not resolve" in guard.check_url("http://nope.invalid/") + + +# -- redirects ---------------------------------------------------------------- + +class _Resp: + def __init__(self, status=200, location=None, url="https://example.com/"): + self.status_code = status + self.headers = {"location": location} if location else {} + self.url = _Url(url) + self.text = "body" + + def raise_for_status(self): + pass + + +class _Url(str): + def join(self, other): + return other + + +class _Client: + """Records what was actually requested, so a blocked hop is provably not fetched.""" + + def __init__(self, script): + self.script = script + self.requested = [] + + def get(self, url): + self.requested.append(url) + return self.script.pop(0) + + +def test_redirect_into_loopback_is_blocked_before_the_second_request(monkeypatch): + _resolves_to(monkeypatch, "93.184.216.34") + client = _Client([_Resp(302, location="http://127.0.0.1:11434/api/tags")]) + with pytest.raises(PermissionError, match="loopback"): + guard.get_checked(client, "https://example.com/start") + assert client.requested == ["https://example.com/start"], ( + "the redirect target must never be requested" + ) + + +def test_allowed_redirect_chain_is_followed(monkeypatch): + _resolves_to(monkeypatch, "93.184.216.34") + client = _Client([_Resp(302, location="https://example.com/b"), _Resp(200)]) + resp = guard.get_checked(client, "https://example.com/a") + assert resp.status_code == 200 + assert client.requested == ["https://example.com/a", "https://example.com/b"] + + +def test_redirect_loop_is_bounded(monkeypatch): + _resolves_to(monkeypatch, "93.184.216.34") + client = _Client([_Resp(302, location="https://example.com/loop")] * 50) + with pytest.raises(RuntimeError, match="too many redirects"): + guard.get_checked(client, "https://example.com/loop") + + +# -- the tool ----------------------------------------------------------------- + +def test_web_fetch_returns_the_refusal_as_a_tool_error(monkeypatch): + _resolves_to(monkeypatch, "127.0.0.1") + out = make_web_fetch_tool()("http://sneaky.example.com/") + assert "loopback" in out["error"] + assert "text" not in out + + +def test_web_fetch_still_rejects_non_http_schemes(): + assert "http" in make_web_fetch_tool()("file:///etc/passwd")["error"]