SSRF helper validates a hostname then lets httpx re-resolve it (tsk-6uymvv)

changelog.d/tsk-6uymvv-ssrf-dns-pinning.md

@@ -0,0 +1,14 @@
+- Security: the SSRF guard now pins each outbound connection to the address it
+  validated. Fetches of user-supplied URLs (browser proxy, extract, download,
+  Library web ingest, Knowledge article ingest, peer handshake delivery,
+  UnifiedPush) go through a guarded client whose connections resolve and check
+  the hostname once and connect to that answer, so a low-TTL nameserver can no
+  longer answer public to the check and 127.0.0.1 to the connection. TLS
+  verification is unchanged and still validates the original hostname.
+- Fix: Library web ingest now reuses a single guarded client across an entire
+  redirect chain instead of building and tearing down a fresh one (new
+  connection pool, SSL context, pinned backend) on every hop.
+- Fix: Knowledge article ingest now rejects a caller-supplied `fetch_client`
+  that is an `httpx.AsyncClient` but was not built by `guarded_async_client`,
+  instead of silently accepting an unguarded client and bypassing the SSRF
+  pin.

tests/routes/desktop_browser/test_ssrf.py

@@ -191,3 +191,124 @@ def test_rejects_when_only_ipv6_resolves_to_private(self):
         ):
             with pytest.raises(SsrfBlockedError):
                 validate_url_or_raise("http://dual-stack.test/")
+
+
+class TestPinnedTransport:
+    """The guarded client must connect to the address the guard checked."""
+
+    def test_validate_returns_the_addresses_it_approved(self):
+        from tinyagentos.routes.desktop_browser.ssrf import validate_url_or_raise
+
+        with patch(
+            "tinyagentos.routes.desktop_browser.ssrf.socket.getaddrinfo",
+            return_value=[
+                (2, 1, 6, "", ("93.184.216.34", 0)),
+                (2, 1, 6, "", ("93.184.216.35", 0)),
+            ],
+        ):
+            addrs = validate_url_or_raise("http://example.com/")
+
+        # Resolver order is preserved: the first answer is the one a pinned
+        # connection uses, so it must not come back through a set.
+        assert addrs == ["93.184.216.34", "93.184.216.35"]
+
+    def test_client_pins_the_connection_to_the_checked_address(self):
+        """The pin sits on the pool's network backend, below the request URL."""
+        from tinyagentos.routes.desktop_browser.ssrf import (
+            _PinnedResolutionBackend,
+            guarded_async_client,
+        )
+
+        client = guarded_async_client()
+        backend = client._transport._pool._network_backend
+        assert isinstance(backend, _PinnedResolutionBackend)
+
+    def test_pinned_backend_hands_the_socket_a_checked_literal(self):
+        """connect_tcp resolves once, validates, and connects to that answer."""
+        import asyncio
+        from unittest.mock import AsyncMock
+
+        from tinyagentos.routes.desktop_browser.ssrf import _PinnedResolutionBackend
+
+        inner = AsyncMock()
+        backend = _PinnedResolutionBackend(inner)
+
+        with patch(
+            "tinyagentos.routes.desktop_browser.ssrf.socket.getaddrinfo",
+            return_value=[(2, 1, 6, "", ("93.184.216.34", 0))],
+        ):
+            asyncio.run(backend.connect_tcp("example.com", 443, timeout=5.0))
+
+        args, kwargs = inner.connect_tcp.await_args
+        assert args[0] == "93.184.216.34"
+        assert args[1] == 443
+        assert kwargs["timeout"] == 5.0
+
+    def test_pinned_backend_refuses_a_blocked_answer(self):
+        import asyncio
+
+        from tinyagentos.routes.desktop_browser.ssrf import (
+            SsrfBlockedError,
+            _PinnedResolutionBackend,
+        )
+        from unittest.mock import AsyncMock
+
+        inner = AsyncMock()
+        backend = _PinnedResolutionBackend(inner)
+
+        with patch(
+            "tinyagentos.routes.desktop_browser.ssrf.socket.getaddrinfo",
+            return_value=[(2, 1, 6, "", ("127.0.0.1", 0))],
+        ):
+            with pytest.raises(SsrfBlockedError):
+                asyncio.run(backend.connect_tcp("rebind.test", 80))
+
+        inner.connect_tcp.assert_not_awaited()
+
+    def test_allow_private_reaches_the_pin(self):
+        """A LAN-facing caller pins too — it just permits RFC1918."""
+        import asyncio
+        from unittest.mock import AsyncMock
+
+        from tinyagentos.routes.desktop_browser.ssrf import _PinnedResolutionBackend
+
+        inner = AsyncMock()
+        backend = _PinnedResolutionBackend(inner, allow_private=True)
+
+        with patch(
+            "tinyagentos.routes.desktop_browser.ssrf.socket.getaddrinfo",
+            return_value=[(2, 1, 6, "", ("192.168.1.50", 0))],
+        ):
+            asyncio.run(backend.connect_tcp("nas.example.com", 80))
+
+        assert inner.connect_tcp.await_args[0][0] == "192.168.1.50"
+
+    def test_tls_verification_stays_on_and_the_url_keeps_the_hostname(self):
+        """Pinning below the URL is what keeps certificate checks meaningful."""
+        import ssl
+
+        from tinyagentos.routes.desktop_browser.ssrf import guarded_async_client
+
+        client = guarded_async_client()
+        request = client.build_request("GET", "https://example.com/page")
+        # The URL is untouched, so httpcore derives SNI and the cert hostname
+        # from the real name rather than from a pinned literal.
+        assert request.url.host == "example.com"
+        assert request.headers["host"] == "example.com"
+
+        ssl_context = client._transport._pool._ssl_context
+        assert ssl_context.verify_mode == ssl.CERT_REQUIRED
+        assert ssl_context.check_hostname is True
+
+    def test_unix_sockets_are_refused(self):
+        import asyncio
+        from unittest.mock import AsyncMock
+
+        from tinyagentos.routes.desktop_browser.ssrf import (
+            SsrfBlockedError,
+            _PinnedResolutionBackend,
+        )
+
+        backend = _PinnedResolutionBackend(AsyncMock())
+        with pytest.raises(SsrfBlockedError):
+            asyncio.run(backend.connect_unix_socket("/run/taos.sock"))

tests/test_knowledge_ingest.py

@@ -82,6 +82,7 @@ async def pipeline(store, mock_http):
     p = IngestPipeline(
         store=store,
         http_client=mock_http,
+        fetch_client=mock_http,
         notifications=notif,
         category_engine=cat_engine,
         qmd_base_url="",  # QMD disabled for unit tests
@@ -200,6 +201,7 @@ async def test_summarise_called_when_llm_url_set(store):
     pipeline = IngestPipeline(
         store=store,
         http_client=mock_http,
+        fetch_client=mock_http,
         notifications=notif,
         category_engine=cat_engine,
         qmd_base_url="",  # disable embed for this test
@@ -240,6 +242,7 @@ async def test_embed_called_when_qmd_url_set(store):
     pipeline = IngestPipeline(
         store=store,
         http_client=mock_http,
+        fetch_client=mock_http,
         notifications=notif,
         category_engine=cat_engine,
         qmd_base_url="http://localhost:7832",
@@ -284,6 +287,7 @@ async def test_semaphore_custom_max_concurrent(store, mock_http):
     p = IngestPipeline(
         store=store,
         http_client=mock_http,
+        fetch_client=mock_http,
         notifications=notif,
         category_engine=cat_engine,
         max_concurrent=2,
@@ -301,6 +305,7 @@ async def test_max_concurrent_zero_raises(store, mock_http):
         IngestPipeline(
             store=store,
             http_client=mock_http,
+            fetch_client=mock_http,
             notifications=notif,
             category_engine=cat_engine,
             max_concurrent=0,
@@ -332,6 +337,7 @@ async def counting_run(self, item_id: str) -> None:
     p = IngestPipeline(
         store=store,
         http_client=mock_http,
+        fetch_client=mock_http,
         notifications=notif,
         category_engine=cat_engine,
         max_concurrent=2,
@@ -373,6 +379,7 @@ async def test_categories_from_caller_are_preserved(store):
     pipeline = IngestPipeline(
         store=store,
         http_client=mock_http,
+        fetch_client=mock_http,
         notifications=notif,
         category_engine=cat_engine,
         qmd_base_url="",
@@ -413,3 +420,36 @@ async def test_download_article_blocks_internal_url(pipeline, store):
     # The guard raises before any HTTP call to the internal address.
     for call in pipeline._http_client.get.await_args_list:
         assert "127.0.0.1" not in str(call)
+
+
+@pytest.mark.asyncio
+async def test_fetch_client_must_be_guarded(store, mock_http):
+    """A caller-supplied fetch_client that IS an httpx.AsyncClient must carry
+    the SSRF-pinned transport. Otherwise a caller handing in a plain,
+    unguarded client (e.g. a shared app-wide client) would silently bypass
+    the guard for every article fetch through this pipeline."""
+    import httpx
+
+    def _handler(request):
+        # Should never actually be reached — the type check must fire first.
+        return httpx.Response(
+            200,
+            headers={"content-type": "text/html"},
+            text="<html><body><p>Should never be read.</p></body></html>",
+        )
+
+    notif = AsyncMock()
+    cat_engine = AsyncMock()
+    unguarded_client = httpx.AsyncClient(transport=httpx.MockTransport(_handler))
+    pipeline = IngestPipeline(
+        store=store,
+        http_client=mock_http,
+        fetch_client=unguarded_client,
+        notifications=notif,
+        category_engine=cat_engine,
+    )
+    try:
+        with pytest.raises(TypeError, match="guarded_async_client"):
+            await pipeline._download_article("https://example.com/a", "", {})
+    finally:
+        await unguarded_client.aclose()

tests/test_library.py

@@ -709,6 +709,60 @@ async def test_web_redirect_hop_validated(self, lib_store, storage_dir):
         # Both hops should have been validated
         assert mock_validate.call_count == 2
 
+    @pytest.mark.asyncio
+    async def test_web_redirect_reuses_single_guarded_client(self, lib_store, storage_dir):
+        """One `guarded_async_client()` call must serve every hop of a
+        multi-hop redirect chain — not a fresh client (pool, SSL context,
+        pinned backend) built and torn down on each hop."""
+        from unittest.mock import patch, MagicMock, AsyncMock
+        import tinyagentos.routes.desktop_browser.ssrf as ssrf_mod
+
+        html = "<html><p>Final hop content, long enough to pass the readability minimum threshold for extraction.</p></html>"
+        item_id = await lib_store.create_item(
+            kind="url:web",
+            source_url="https://safe.example.com/start",
+        )
+        item = await lib_store.get_item(item_id)
+        proc = WebProcessor(lib_store, storage_dir)
+
+        # Two redirects then a final 200: 3 hops total.
+        mock_resp1 = MagicMock()
+        mock_resp1.status_code = 302
+        mock_resp1.headers = {"location": "https://safe.example.com/hop1"}
+        mock_resp1.is_redirect = True
+
+        mock_resp2 = MagicMock()
+        mock_resp2.status_code = 302
+        mock_resp2.headers = {"location": "https://safe.example.com/final"}
+        mock_resp2.is_redirect = True
+
+        mock_resp3 = _mock_httpx_response(html, 200)
+
+        mock_client = MagicMock()
+        mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+        mock_client.__aexit__ = AsyncMock(return_value=None)
+        mock_client.stream = MagicMock(
+            side_effect=[
+                _mock_stream_ctx(mock_resp1),
+                _mock_stream_ctx(mock_resp2),
+                _mock_stream_ctx(mock_resp3),
+            ]
+        )
+
+        counting = MagicMock(side_effect=lambda *a, **kw: mock_client)
+
+        with (
+            patch.object(ssrf_mod, "guarded_async_client", counting),
+            patch.object(ssrf_mod, "validate_url_or_raise"),
+        ):
+            await proc.process(item)
+
+        assert counting.call_count == 1, (
+            f"guarded_async_client entered {counting.call_count} times "
+            "for a 3-hop fetch — it must be entered exactly once and reused "
+            "across every redirect hop"
+        )
+
     @pytest.mark.asyncio
     async def test_web_size_cap(self, lib_store, storage_dir):
         """Responses exceeding the size cap raise ValueError."""