Keep platform cookies away from external upstreams

enlace/proxy.py

@@ -8,7 +8,69 @@
 app is actually instantiated, so the dependency remains optional.
 """
 
-from typing import Optional
+from typing import Callable, Iterable, Optional
+
+#: Cookies the platform itself sets on its own origin -- enlace_auth's defaults
+#: (``AuthConfig.session_cookie_name``, ``CSRFMiddleware`` cookie name, and the
+#: per-app ``shared_auth_<app>`` cookies). Keep in sync with enlace_auth. They
+#: are credentials for *this* platform and must never reach an upstream that
+#: is not part of it.
+PLATFORM_COOKIE_NAMES = ("enlace_session", "enlace_csrf")
+PLATFORM_COOKIE_PREFIXES = ("shared_auth_",)
+
+#: Request headers carrying platform credentials, withheld from external
+#: upstreams (the CSRF double-submit token pairs with ``enlace_csrf``).
+EXTERNAL_DROP_REQUEST_HEADERS = ("x-csrf-token",)
+
+#: Response headers an external upstream may not send on the platform origin:
+#: ``Clear-Site-Data`` could wipe the platform's cookies/storage, and
+#: ``Service-Worker-Allowed`` could let a script under the app's prefix
+#: register a service worker controlling the whole origin.
+EXTERNAL_DROP_RESPONSE_HEADERS = ("clear-site-data", "service-worker-allowed")
+
+CookieFilter = Callable[[str], bool]  # cookie name -> forward it?
+
+
+def platform_cookie_filter(
+    *,
+    names: Iterable[str] = PLATFORM_COOKIE_NAMES,
+    prefixes: Iterable[str] = PLATFORM_COOKIE_PREFIXES,
+) -> CookieFilter:
+    """Return a filter that keeps every cookie except the platform's own.
+
+    Used for ``mode="external"`` apps: an upstream on another host has no
+    business seeing a visitor's platform session, and must not be able to set
+    (overwrite) one on the platform's origin either.
+    """
+    names = frozenset(names)
+    prefixes = tuple(prefixes)
+    return lambda name: not (name in names or name.startswith(prefixes))
+
+
+def _filter_cookie_header(value: str, keep: CookieFilter) -> str:
+    """Drop the cookies *keep* rejects from a request ``Cookie`` header value."""
+    parts = []
+    for part in value.split(";"):
+        name = part.split("=", 1)[0].strip()
+        if name and keep(name):
+            parts.append(part.strip())
+    return "; ".join(parts)
+
+
+def _set_cookie_allowed(value: str, keep: CookieFilter) -> bool:
+    """Whether a ``Set-Cookie`` header value may pass *keep*.
+
+    A nameless cookie (``=enlace_session=x`` or a bare value) is refused
+    outright: browsers send such a cookie as its bare value, so
+    ``=enlace_session=x`` reaches the server as ``enlace_session=x`` and would
+    shadow the real one.
+    """
+    pair = value.split(";", 1)[0]
+    if "=" not in pair:
+        return False
+    name = pair.split("=", 1)[0].strip()
+    return bool(name) and keep(name)
+
 
 # Default per-request timeout (seconds) for proxied requests. Bounds a hung
 # upstream so a stuck app can't tie up the gateway indefinitely.
@@ -34,18 +96,39 @@ def _request_timeout(accept: str, base: float) -> Optional[dict]:
     return None
 
 
-def make_proxy_app(*, upstream: str, strip_prefix: str = ""):
+def make_proxy_app(
+    *,
+    upstream: str,
+    strip_prefix: str = "",
+    cookie_filter: Optional[CookieFilter] = None,
+    drop_request_headers: Iterable[str] = (),
+    drop_response_headers: Iterable[str] = (),
+):
     """Create an ASGI app that proxies requests to *upstream*.
 
     Args:
         upstream: Base URL of the upstream server (e.g. ``http://127.0.0.1:9100``).
         strip_prefix: Route prefix to strip before forwarding
             (e.g. ``/api/blog`` → upstream receives ``/``).
+        cookie_filter: ``name -> bool``; when given, request cookies it
+            rejects are not forwarded and upstream ``Set-Cookie`` headers
+            naming them are dropped. ``None`` (the default) forwards all
+            cookies, which suits a local process app that is part of the
+            platform. See :func:`platform_cookie_filter`.
+        drop_request_headers / drop_response_headers: header names
+            (case-insensitive) never forwarded upstream / never passed back
+            to the client. See ``EXTERNAL_DROP_*`` for what external apps use.
 
     Returns:
         An ASGI callable.
     """
-    return _HttpxProxy(upstream=upstream, strip_prefix=strip_prefix)
+    return _HttpxProxy(
+        upstream=upstream,
+        strip_prefix=strip_prefix,
+        cookie_filter=cookie_filter,
+        drop_request_headers=drop_request_headers,
+        drop_response_headers=drop_response_headers,
+    )
 
 
 class _HttpxProxy:
@@ -57,7 +140,17 @@ def __init__(
         upstream: str,
         strip_prefix: str = "",
         timeout: float = _DEFAULT_TIMEOUT_S,
+        cookie_filter: Optional[CookieFilter] = None,
+        drop_request_headers: Iterable[str] = (),
+        drop_response_headers: Iterable[str] = (),
     ):
+        self.cookie_filter = cookie_filter
+        self._drop_request = {"host", "transfer-encoding", "connection"} | {
+            h.lower() for h in drop_request_headers
+        }
+        self._drop_response = {"transfer-encoding", "connection", "keep-alive"} | {
+            h.lower() for h in drop_response_headers
+        }
         self.upstream = upstream.rstrip("/")
         self.strip_prefix = strip_prefix
         self.timeout = timeout
@@ -112,9 +205,16 @@ async def __call__(self, scope, receive, send):
         headers = {}
         for key, value in scope.get("headers", []):
             name = key.decode("latin-1").lower()
-            if name in ("host", "transfer-encoding", "connection"):
+            if name in self._drop_request:
                 continue
-            headers[name] = value.decode("latin-1")
+            decoded = value.decode("latin-1")
+            if name == "cookie" and self.cookie_filter is not None:
+                decoded = _filter_cookie_header(decoded, self.cookie_filter)
+                if not decoded:
+                    continue
+                if "cookie" in headers:  # HTTP/2 may split cookies (RFC 9113)
+                    decoded = f"{headers['cookie']}; {decoded}"
+            headers[name] = decoded
 
         import httpx
 
@@ -143,7 +243,12 @@ async def __call__(self, scope, receive, send):
             response_headers = [
                 (k.encode("latin-1"), v.encode("latin-1"))
                 for k, v in response.headers.multi_items()
-                if k.lower() not in ("transfer-encoding", "connection", "keep-alive")
+                if k.lower() not in self._drop_response
+                and not (
+                    k.lower() == "set-cookie"
+                    and self.cookie_filter is not None
+                    and not _set_cookie_allowed(v, self.cookie_filter)
+                )
             ]
 
             await send(

enlace/strategies.py

@@ -352,6 +352,19 @@ def make_lifecycle(self, app, platform):
         )
 
 
+def _platform_cookie_names(platform) -> tuple[str, ...]:
+    """The platform's own cookie names, honouring a custom session cookie name.
+
+    ``[auth]`` is opaque to enlace core (enlace_auth owns its schema), so this
+    reads only the one key that renames a platform cookie.
+    """
+    from enlace.proxy import PLATFORM_COOKIE_NAMES
+
+    auth = getattr(platform, "auth", None) or {}
+    custom = auth.get("session_cookie_name") if isinstance(auth, dict) else None
+    return PLATFORM_COOKIE_NAMES + ((custom,) if custom else ())
+
+
 class ExternalStrategy(BackendStrategy):
     """Route to a pre-existing service at a known URL; no lifecycle."""
 
@@ -369,9 +382,24 @@ def validate(self, app):
     def make_asgi(self, app, platform):
         if not app.upstream_url:
             return None
-        from enlace.proxy import make_proxy_app
+        from enlace.proxy import (
+            EXTERNAL_DROP_REQUEST_HEADERS,
+            EXTERNAL_DROP_RESPONSE_HEADERS,
+            make_proxy_app,
+            platform_cookie_filter,
+        )
 
-        return make_proxy_app(upstream=app.upstream_url, strip_prefix=app.route_prefix)
+        # An external upstream is someone else's server: it must neither see
+        # the visitor's platform credentials nor set them on our origin.
+        return make_proxy_app(
+            upstream=app.upstream_url,
+            strip_prefix=app.route_prefix,
+            cookie_filter=platform_cookie_filter(
+                names=_platform_cookie_names(platform)
+            ),
+            drop_request_headers=EXTERNAL_DROP_REQUEST_HEADERS,
+            drop_response_headers=EXTERNAL_DROP_RESPONSE_HEADERS,
+        )
 
 
 class StaticStrategy(BackendStrategy):

enlace/tests/test_proxy_cookies.py

@@ -0,0 +1,145 @@
+"""An external upstream never sees, nor sets, the platform's own cookies.
+
+``mode="external"`` proxies to a server outside the platform. Forwarding the
+visitor's ``Cookie`` header verbatim would hand it their platform session; an
+upstream ``Set-Cookie`` for a platform cookie name would overwrite it on the
+platform's origin.
+"""
+
+from __future__ import annotations
+
+import httpx
+import pytest
+from starlette.applications import Starlette
+from starlette.routing import Mount
+from starlette.testclient import TestClient
+
+from enlace.base import AppConfig, PlatformConfig
+from enlace.proxy import _HttpxProxy, make_proxy_app, platform_cookie_filter
+from enlace.strategies import ExternalStrategy
+
+
+def _echo_transport(seen: dict):
+    def handler(request: httpx.Request) -> httpx.Response:
+        seen["cookie"] = request.headers.get("cookie")
+        seen["authorization"] = request.headers.get("authorization")
+        seen["x-csrf-token"] = request.headers.get("x-csrf-token")
+        return httpx.Response(
+            200,
+            headers=[
+                ("set-cookie", "enlace_session=forged; Path=/"),
+                ("set-cookie", "shared_auth_vault=forged; Path=/"),
+                ("set-cookie", "space_pref=dark; Path=/typola"),
+                ("set-cookie", "=enlace_session=forged; Path=/auth"),
+                ("set-cookie", " =enlace_csrf=forged"),
+                ("set-cookie", "nameless-value; Path=/"),
+                ("clear-site-data", '"cookies"'),
+                ("service-worker-allowed", "/"),
+                ("x-upstream", "kept"),
+            ],
+            json={"ok": True},
+        )
+
+    return httpx.MockTransport(handler)
+
+
+def _client(proxy: _HttpxProxy, seen: dict) -> TestClient:
+    async def _get_client():
+        return httpx.AsyncClient(transport=_echo_transport(seen))
+
+    proxy._get_client = _get_client
+    return TestClient(Starlette(routes=[Mount("/typola", app=proxy)]))
+
+
+VISITOR_COOKIES = (
+    "enlace_session=SECRET-SESSION; enlace_csrf=SECRET-CSRF; "
+    "shared_auth_vault=SECRET-SHARED; space_pref=light"
+)
+
+
+def test_filtered_proxy_forwards_only_non_platform_cookies():
+    seen: dict = {}
+    proxy = make_proxy_app(
+        upstream="https://space.example",
+        strip_prefix="/typola",
+        cookie_filter=platform_cookie_filter(),
+    )
+    r = _client(proxy, seen).get("/typola/x", headers={"Cookie": VISITOR_COOKIES})
+    assert r.status_code == 200
+    assert seen["cookie"] == "space_pref=light"
+    assert "SECRET" not in (seen["cookie"] or "")
+    set_cookies = r.headers.get_list("set-cookie")
+    assert set_cookies == ["space_pref=dark; Path=/typola"]
+
+
+def test_filtered_proxy_drops_cookie_header_when_nothing_is_left():
+    seen: dict = {}
+    proxy = make_proxy_app(
+        upstream="https://space.example",
+        strip_prefix="/typola",
+        cookie_filter=platform_cookie_filter(),
+    )
+    _client(proxy, seen).get("/typola/x", headers={"Cookie": "enlace_session=SECRET"})
+    assert seen["cookie"] is None
+
+
+def test_unfiltered_proxy_is_unchanged():
+    """Process-mode (local, part of the platform) keeps forwarding everything."""
+    seen: dict = {}
+    proxy = make_proxy_app(upstream="http://127.0.0.1:9", strip_prefix="/typola")
+    r = _client(proxy, seen).get("/typola/x", headers={"Cookie": VISITOR_COOKIES})
+    assert seen["cookie"] == VISITOR_COOKIES
+    assert len(r.headers.get_list("set-cookie")) == 6
+    assert r.headers.get("clear-site-data") == '"cookies"'
+
+
+def test_external_strategy_isolates_upstream(tmp_path):
+    """Through ExternalStrategy: no platform credentials out, no takeover in."""
+    seen: dict = {}
+    app = AppConfig(
+        name="typola",
+        route_prefix="/typola",
+        app_type="asgi_app",
+        mode="external",
+        upstream_url="https://space.example",
+    )
+    proxy = ExternalStrategy().make_asgi(app, PlatformConfig(apps_dir=tmp_path))
+    r = _client(proxy, seen).get(
+        "/typola/x",
+        headers={
+            "Cookie": VISITOR_COOKIES,
+            "X-CSRF-Token": "SECRET-CSRF",
+            "Authorization": "Bearer upstream-own-token",
+        },
+    )
+    assert seen["cookie"] == "space_pref=light"
+    assert seen["x-csrf-token"] is None
+    assert seen["authorization"] == "Bearer upstream-own-token"
+    assert r.headers.get_list("set-cookie") == ["space_pref=dark; Path=/typola"]
+    assert "clear-site-data" not in r.headers
+    assert "service-worker-allowed" not in r.headers
+    assert r.headers["x-upstream"] == "kept"
+
+
+@pytest.mark.parametrize(
+    "auth, extra_name",
+    [({}, None), ({"session_cookie_name": "my_sess"}, "my_sess")],
+)
+def test_external_strategy_applies_the_platform_filter(tmp_path, auth, extra_name):
+    platform = PlatformConfig(apps_dir=tmp_path, auth=auth)
+    app = AppConfig(
+        name="typola",
+        route_prefix="/typola",
+        app_type="asgi_app",
+        mode="external",
+        upstream_url="https://space.example",
+    )
+    proxy = ExternalStrategy().make_asgi(app, platform)
+    keep = proxy.cookie_filter
+    assert keep is not None
+    assert not keep("enlace_session")
+    assert not keep("enlace_csrf")
+    assert not keep("shared_auth_anything")
+    assert keep("space_pref")
+    if extra_name:
+        assert not keep(extra_name)