From 62a54fe422ac42d728ee0d8f933a52b4157310f0 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:54:39 +0000 Subject: [PATCH 1/2] Keep platform cookies away from external upstreams mode="external" proxies now drop the platform's own cookies (session, CSRF, shared-password) from forwarded requests and drop upstream Set-Cookie headers that would set them on the platform origin. A custom [auth] session_cookie_name is honoured. make_proxy_app gains an optional cookie_filter; process-mode proxies are unchanged. Co-Authored-By: Claude Opus 5 --- enlace/proxy.py | 74 ++++++++++++++++++-- enlace/strategies.py | 27 ++++++- enlace/tests/test_proxy_cookies.py | 109 +++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 enlace/tests/test_proxy_cookies.py diff --git a/enlace/proxy.py b/enlace/proxy.py index bff5281..450cc52 100644 --- a/enlace/proxy.py +++ b/enlace/proxy.py @@ -8,7 +8,47 @@ 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). +#: 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_",) + +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_name(value: str) -> str: + """The cookie name a ``Set-Cookie`` header value sets.""" + return value.split(";", 1)[0].split("=", 1)[0].strip() + # 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 +74,30 @@ 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, +): """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`. Returns: An ASGI callable. """ - return _HttpxProxy(upstream=upstream, strip_prefix=strip_prefix) + return _HttpxProxy( + upstream=upstream, strip_prefix=strip_prefix, cookie_filter=cookie_filter + ) class _HttpxProxy: @@ -57,7 +109,9 @@ def __init__( upstream: str, strip_prefix: str = "", timeout: float = _DEFAULT_TIMEOUT_S, + cookie_filter: Optional[CookieFilter] = None, ): + self.cookie_filter = cookie_filter self.upstream = upstream.rstrip("/") self.strip_prefix = strip_prefix self.timeout = timeout @@ -114,7 +168,14 @@ async def __call__(self, scope, receive, send): name = key.decode("latin-1").lower() if name in ("host", "transfer-encoding", "connection"): 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 @@ -144,6 +205,11 @@ async def __call__(self, scope, receive, send): (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") + and not ( + k.lower() == "set-cookie" + and self.cookie_filter is not None + and not self.cookie_filter(_set_cookie_name(v)) + ) ] await send( diff --git a/enlace/strategies.py b/enlace/strategies.py index 0bfb602..7121aa7 100644 --- a/enlace/strategies.py +++ b/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,17 @@ def validate(self, app): def make_asgi(self, app, platform): if not app.upstream_url: return None - from enlace.proxy import make_proxy_app - - return make_proxy_app(upstream=app.upstream_url, strip_prefix=app.route_prefix) + from enlace.proxy import make_proxy_app, platform_cookie_filter + + # 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) + ), + ) class StaticStrategy(BackendStrategy): diff --git a/enlace/tests/test_proxy_cookies.py b/enlace/tests/test_proxy_cookies.py new file mode 100644 index 0000000..d27896a --- /dev/null +++ b/enlace/tests/test_proxy_cookies.py @@ -0,0 +1,109 @@ +"""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") + 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"), + ], + 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")) == 3 + + +@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) From ce9c6bf89c260b3c3c0ddd09bebc9c023591be72 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:01:21 +0000 Subject: [PATCH 2/2] Address review: nameless Set-Cookie, origin-wide response headers - Drop nameless Set-Cookie values (=enlace_session=x is sent by browsers as enlace_session=x and would shadow the real cookie). - External apps: withhold X-CSRF-Token upstream; drop Clear-Site-Data and Service-Worker-Allowed from upstream responses. - make_proxy_app gains drop_request_headers / drop_response_headers. Co-Authored-By: Claude Opus 5 --- enlace/proxy.py | 59 +++++++++++++++++++++++++----- enlace/strategies.py | 9 ++++- enlace/tests/test_proxy_cookies.py | 38 ++++++++++++++++++- 3 files changed, 94 insertions(+), 12 deletions(-) diff --git a/enlace/proxy.py b/enlace/proxy.py index 450cc52..b441c27 100644 --- a/enlace/proxy.py +++ b/enlace/proxy.py @@ -10,12 +10,24 @@ from typing import Callable, Iterable, Optional -#: Cookies the platform itself sets on its own origin (enlace_auth's defaults). -#: They are credentials for *this* platform and must never reach an upstream -#: that is not part of it. +#: 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_`` 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? @@ -45,9 +57,19 @@ def _filter_cookie_header(value: str, keep: CookieFilter) -> str: return "; ".join(parts) -def _set_cookie_name(value: str) -> str: - """The cookie name a ``Set-Cookie`` header value sets.""" - return value.split(";", 1)[0].split("=", 1)[0].strip() +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 @@ -79,6 +101,8 @@ 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*. @@ -91,12 +115,19 @@ def make_proxy_app( 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, cookie_filter=cookie_filter + upstream=upstream, + strip_prefix=strip_prefix, + cookie_filter=cookie_filter, + drop_request_headers=drop_request_headers, + drop_response_headers=drop_response_headers, ) @@ -110,8 +141,16 @@ def __init__( 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 @@ -166,7 +205,7 @@ 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 decoded = value.decode("latin-1") if name == "cookie" and self.cookie_filter is not None: @@ -204,11 +243,11 @@ 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 self.cookie_filter(_set_cookie_name(v)) + and not _set_cookie_allowed(v, self.cookie_filter) ) ] diff --git a/enlace/strategies.py b/enlace/strategies.py index 7121aa7..bd1e8e6 100644 --- a/enlace/strategies.py +++ b/enlace/strategies.py @@ -382,7 +382,12 @@ def validate(self, app): def make_asgi(self, app, platform): if not app.upstream_url: return None - from enlace.proxy import make_proxy_app, platform_cookie_filter + from enlace.proxy import ( + EXTERNAL_DROP_REQUEST_HEADERS, + EXTERNAL_DROP_RESPONSE_HEADERS, + make_proxy_app, + platform_cookie_filter, + ) # An external upstream is someone else's server: it must neither see # the visitor's platform credentials nor set them on our origin. @@ -392,6 +397,8 @@ def make_asgi(self, app, platform): cookie_filter=platform_cookie_filter( names=_platform_cookie_names(platform) ), + drop_request_headers=EXTERNAL_DROP_REQUEST_HEADERS, + drop_response_headers=EXTERNAL_DROP_RESPONSE_HEADERS, ) diff --git a/enlace/tests/test_proxy_cookies.py b/enlace/tests/test_proxy_cookies.py index d27896a..0244235 100644 --- a/enlace/tests/test_proxy_cookies.py +++ b/enlace/tests/test_proxy_cookies.py @@ -23,12 +23,19 @@ 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}, ) @@ -82,7 +89,36 @@ def test_unfiltered_proxy_is_unchanged(): 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")) == 3 + 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(