Keep client referrer attribution free of private URL data

routstr/core/middleware.py

@@ -2,6 +2,7 @@ import time
 import uuid
 from contextvars import ContextVar
 from typing import Callable
+from urllib.parse import urlsplit
 
 from fastapi import Request, Response
 from starlette.datastructures import Headers
@@ -39,6 +40,15 @@ def client_app_from_headers(headers: Headers) -> str:
         if raw is None:
             continue
         cleaned = "".join(ch for ch in raw if ch.isprintable()).strip()
+        if header in ("http-referer", "referer"):
+            try:
+                url = urlsplit(cleaned)
+                if url.scheme not in ("http", "https") or not url.hostname:
+                    continue
+            except ValueError:
+                continue
+            # Attribution needs the origin, not credentials or private page URLs.
+            cleaned = f"{url.scheme}://{url.netloc.rsplit('@', 1)[-1]}"
         if cleaned:
             return cleaned[:_CLIENT_APP_MAX_LENGTH]
     return UNKNOWN_CLIENT_APP

tests/unit/test_client_app_logging.py

@@ -1,9 +1,10 @@
 """Tests for client-app identification in request logging."""
 
+import asyncio
 import logging
 
 import pytest
-from fastapi import FastAPI
+from fastapi import FastAPI, Request, Response
 from fastapi.testclient import TestClient
 from starlette.datastructures import Headers
 
@@ -66,6 +67,27 @@ def test_client_app_from_headers(headers: dict[str, str], expected: str) -> None
     assert client_app_from_headers(Headers(headers)) == expected
 
 
+@pytest.mark.parametrize("header", ["http-referer", "referer"])
+@pytest.mark.parametrize(
+    ("url", "expected"),
+    [
+        (
+            "https://alice:password@app.example:8443/private/chat?token=secret#access_token=secret",
+            "https://app.example:8443",
+        ),
+        ("http://[::1]:3000/chat?key=secret", "http://[::1]:3000"),
+        ("https://app.example/" + "a" * 200, "https://app.example"),
+        ("https://[invalid", "curl/8.4.0"),
+        ("/private/chat?token=secret", "curl/8.4.0"),
+        ("javascript:secret", "curl/8.4.0"),
+        ("https:///private", "curl/8.4.0"),
+    ],
+)
+def test_referrer_only_identifies_origin(header: str, url: str, expected: str) -> None:
+    headers = Headers({header: url, "user-agent": "curl/8.4.0"})
+    assert client_app_from_headers(headers) == expected
+
+
 def test_value_is_truncated_to_120_chars() -> None:
     assert client_app_from_headers(Headers({"x-title": "a" * 500})) == "a" * 120
 
@@ -86,6 +108,69 @@ def test_filter_reads_context_variable() -> None:
         client_app_context.reset(token)
 
 
+@pytest.mark.parametrize("fail", [False, True])
+async def test_context_is_restored_after_request(fail: bool) -> None:
+    middleware = LoggingMiddleware(FastAPI())
+    request = Request(
+        {
+            "type": "http",
+            "method": "GET",
+            "path": "/test",
+            "query_string": b"",
+            "headers": [],
+        }
+    )
+
+    async def call_next(request: Request) -> Response:
+        assert client_app_context.get() == UNKNOWN_CLIENT_APP
+        if fail:
+            raise RuntimeError("handler failed")
+        return Response()
+
+    token = client_app_context.set("outer")
+    try:
+        if fail:
+            with pytest.raises(RuntimeError, match="handler failed"):
+                await middleware.dispatch(request, call_next)
+        else:
+            await middleware.dispatch(request, call_next)
+        assert client_app_context.get() == "outer"
+    finally:
+        client_app_context.reset(token)
+
+
+async def test_concurrent_requests_keep_their_own_client_app() -> None:
+    middleware = LoggingMiddleware(FastAPI())
+    ready = asyncio.Event()
+    apps: list[str] = []
+
+    async def call_next(request: Request) -> Response:
+        apps.append(request.headers["x-title"])
+        if len(apps) == 2:
+            ready.set()
+        await asyncio.wait_for(ready.wait(), timeout=5)
+        assert client_app_context.get() == request.headers["x-title"]
+        return Response()
+
+    await asyncio.gather(
+        *(
+            middleware.dispatch(
+                Request(
+                    {
+                        "type": "http",
+                        "method": "GET",
+                        "path": "/test",
+                        "query_string": b"",
+                        "headers": [(b"x-title", app)],
+                    }
+                ),
+                call_next,
+            )
+            for app in (b"Goose", b"Pi")
+        )
+    )
+
+
 def test_filter_defaults_to_unknown_outside_request_context() -> None:
     record = _record()
     assert ClientAppFilter().filter(record) is True