From 28ef63a10cdb7323f0aee461ffaeddd9b302d596 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Wed, 12 Aug 2026 17:36:43 -0700 Subject: [PATCH] fix: revalidate frontend assets across builds --- scripts/asset_cache_e2e.py | 458 +++++++++++++++++++++++++++++++++ scripts/update-vendored-js.sh | 4 +- tests/test_api_versioning.py | 4 + tests/test_console.py | 163 ++++++++++++ tests/test_coordinator_page.py | 26 +- tests/test_web_helpers.py | 145 +++++++++++ turnstone/console/server.py | 108 +++++--- turnstone/core/web_helpers.py | 109 +++++++- turnstone/server.py | 14 +- 9 files changed, 980 insertions(+), 51 deletions(-) create mode 100755 scripts/asset_cache_e2e.py diff --git a/scripts/asset_cache_e2e.py b/scripts/asset_cache_e2e.py new file mode 100755 index 00000000..ed5fc3d0 --- /dev/null +++ b/scripts/asset_cache_e2e.py @@ -0,0 +1,458 @@ +#!/usr/bin/env python3 +"""Browser regression for the two-layer frontend cache contract. + +This harness serves a versioned entry module which imports the real, +unversioned ``shared/interactive.js`` module. It loads an old pane build in a +real Chrome profile, switches the server to a same-version replacement whose +pane has the same byte length and mtime, then performs a normal browser reload +without clearing or disabling the HTTP cache. + +The old fixture advertises ``user_turn=0&tool_turn=0`` in its EventSource URL; +the current source advertises both capabilities as ``1``. A passing run +therefore proves both layers of the contract: + +* the package-versioned entry URL revalidates and may return 304; and +* its unversioned transitive pane import revalidates by content and returns the + current bytes rather than surviving from the prior build. + +The page also loads representative KaTeX, Highlight.js, and HLS.js assets. +Their installed version directories are discovered from ``shared_static`` at +runtime, so a normal vendor-version bump requires no harness edit; reload must +reuse them under the immutable policy. + +Usage:: + + uv run python scripts/asset_cache_e2e.py +""" + +from __future__ import annotations + +import contextlib +import json +import os +import shutil +import socket +import sys +import tempfile +import threading +import time +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from recovery_e2e import CDP, _find_chrome, _launch_chrome, _page_ws_url # noqa: E402 + +from turnstone import __version__ # noqa: E402 +from turnstone.core.web_helpers import ( # noqa: E402 + RevalidatingStaticFiles, + version_html, +) + +_PAGE_HTML = """ + + + + ASSET-CACHE-PENDING + + + + +
+ + + +""" + +_BOOT_JS = """import { InteractivePane } from "/shared/interactive.js"; + +const fixtureWorkstreamId = "00000000-0000-0000-0000-000000000001"; +const pane = new InteractivePane(fixtureWorkstreamId, { base: "" }); +document.getElementById("pane").appendChild(pane.el); +pane.connectSSE(fixtureWorkstreamId); +const url = window.__assetCacheUrls.at(-1) || ""; +const generation = url.includes("user_turn=1") && url.includes("tool_turn=1") + ? "CURRENT" + : url.includes("user_turn=0") && url.includes("tool_turn=0") + ? "OLD" + : "FAILED-CAPABILITIES"; +window.__assetCacheResult = { generation, url }; +document.title = "ASSET-CACHE-" + generation; +""" + + +@dataclass +class CacheState: + phase: str = "old" + requests: list[dict[str, Any]] = field(default_factory=list) + lock: threading.Lock = field(default_factory=threading.Lock) + + def record(self, item: dict[str, Any]) -> None: + with self.lock: + self.requests.append(item) + + def matching(self, phase: str, path: str) -> list[dict[str, Any]]: + with self.lock: + return [ + item for item in self.requests if item["phase"] == phase and item["path"] == path + ] + + +class SwitchingStaticFiles: + """Select one immutable build snapshot at request dispatch time.""" + + def __init__(self, state: CacheState, old_dir: Path, current_dir: Path) -> None: + self._state = state + self._apps = { + "old": RevalidatingStaticFiles(directory=str(old_dir)), + "current": RevalidatingStaticFiles(directory=str(current_dir)), + } + + async def __call__(self, scope: Any, receive: Any, send: Any) -> None: + await self._apps[self._state.phase](scope, receive, send) + + +class RecordingApp: + """Record static HTTP validators and status without perturbing streaming.""" + + def __init__(self, app: Any, state: CacheState) -> None: + self._app = app + self._state = state + + async def __call__(self, scope: Any, receive: Any, send: Any) -> None: + path = str(scope.get("path", "")) + if scope.get("type") != "http" or not path.startswith(("/static/", "/shared/")): + await self._app(scope, receive, send) + return + + phase = self._state.phase + request_headers = { + key.decode("latin-1").lower(): value.decode("latin-1") + for key, value in scope.get("headers", []) + } + + async def record_send(message: dict[str, Any]) -> None: + if message["type"] == "http.response.start": + response_headers = { + key.decode("latin-1").lower(): value.decode("latin-1") + for key, value in message.get("headers", []) + } + self._state.record( + { + "phase": phase, + "path": path, + "query": scope.get("query_string", b"").decode("latin-1"), + "if_none_match": request_headers.get("if-none-match"), + "status": message["status"], + "etag": response_headers.get("etag"), + "cache_control": response_headers.get("cache-control"), + } + ) + await send(message) + + await self._app(scope, receive, record_send) + + +def _old_interactive(current: bytes) -> bytes: + old = current + for needle, replacement in ( + (b'"user_turn=1"', b'"user_turn=0"'), + (b'"&tool_turn=1"', b'"&tool_turn=0"'), + ): + if old.count(needle) != 1: + raise RuntimeError(f"expected exactly one {needle.decode()} capability literal") + old = old.replace(needle, replacement, 1) + if len(old) != len(current): + raise AssertionError("old and current pane fixtures must have equal byte length") + return old + + +def _discover_vendor_assets(source_shared: Path) -> tuple[str, ...]: + selected = ( + ("katex", "katex.min.css"), + ("katex", "katex.min.js"), + ("hljs", "highlight.min.js"), + ("hls", "hls.min.js"), + ) + paths = [] + for library, filename in selected: + matches = sorted(source_shared.glob(f"{library}-*/{filename}")) + if not matches: + raise RuntimeError(f"no vendored {library} asset named {filename} was found") + paths.extend(f"/shared/{match.relative_to(source_shared).as_posix()}" for match in matches) + return tuple(paths) + + +def _vendor_tags(vendor_paths: tuple[str, ...]) -> str: + tags = [] + for path in vendor_paths: + if path.endswith(".css"): + tags.append(f'') + else: + tags.append(f'') + return "\n ".join(tags) + + +def _prepare_builds(scratch: Path) -> tuple[Path, Path, Path, tuple[str, ...]]: + import turnstone + + package_dir = Path(turnstone.__file__).resolve().parent + source_shared = package_dir / "shared_static" + vendor_paths = _discover_vendor_assets(source_shared) + old_shared = scratch / "old" / "shared" + current_shared = scratch / "current" / "shared" + static_dir = scratch / "static" + shutil.copytree(source_shared, old_shared) + shutil.copytree(source_shared, current_shared) + static_dir.mkdir() + (static_dir / "asset_cache_boot.js").write_text(_BOOT_JS, encoding="utf-8") + + current_asset = current_shared / "interactive.js" + old_asset = old_shared / "interactive.js" + current = current_asset.read_bytes() + old_asset.write_bytes(_old_interactive(current)) + + # Reproduce the metadata collision which defeated Starlette's default ETag. + fixed_mtime_ns = 1_700_000_000_123_456_789 + for asset in (old_asset, current_asset): + os.utime(asset, ns=(fixed_mtime_ns, fixed_mtime_ns)) + old_stat = old_asset.stat() + current_stat = current_asset.stat() + if (old_stat.st_size, old_stat.st_mtime_ns) != ( + current_stat.st_size, + current_stat.st_mtime_ns, + ): + raise AssertionError("pane fixture size/mtime collision was not preserved") + return old_shared, current_shared, static_dir, vendor_paths + + +def _make_app( + state: CacheState, + old_dir: Path, + current_dir: Path, + static_dir: Path, + vendor_paths: tuple[str, ...], +) -> Any: + from starlette.applications import Starlette + from starlette.responses import HTMLResponse + from starlette.routing import Mount, Route + + async def page(_request: Any) -> HTMLResponse: + page_html = _PAGE_HTML.replace("", _vendor_tags(vendor_paths)) + return HTMLResponse( + version_html(page_html), + headers={"Cache-Control": "no-store"}, + ) + + app = Starlette( + routes=[ + Route("/asset-cache-e2e", page), + Mount( + "/static", + app=RevalidatingStaticFiles(directory=str(static_dir)), + ), + Mount( + "/shared", + app=SwitchingStaticFiles(state, old_dir, current_dir), + ), + ] + ) + + return RecordingApp(app, state) + + +def _start_server(app: Any) -> tuple[Any, threading.Thread, socket.socket, str]: + import uvicorn + + sock = socket.socket() + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + sock.bind(("127.0.0.1", 0)) + sock.listen(128) + port = int(sock.getsockname()[1]) + server = uvicorn.Server( + uvicorn.Config(app, host="127.0.0.1", port=port, log_level="warning", lifespan="off") + ) + thread = threading.Thread( + target=server.run, + kwargs={"sockets": [sock]}, + name="asset-cache-e2e-server", + daemon=True, + ) + thread.start() + deadline = time.monotonic() + 10 + while not server.started and thread.is_alive() and time.monotonic() < deadline: + time.sleep(0.05) + if not server.started: + server.should_exit = True + thread.join(timeout=2) + sock.close() + raise RuntimeError("asset cache test server did not start") + return server, thread, sock, f"http://127.0.0.1:{port}" + + +def _wait_for_generation(cdp: CDP, expected: str, timeout: float = 20) -> dict[str, str]: + deadline = time.monotonic() + timeout + last_title = "" + while time.monotonic() < deadline: + last_title = cdp.title() + result = cdp.evaluate("window.__assetCacheResult || null") + if isinstance(result, dict) and result.get("generation") == expected: + return {"generation": str(result["generation"]), "url": str(result["url"])} + if last_title.startswith("ASSET-CACHE-FAILED"): + raise RuntimeError(last_title) + time.sleep(0.1) + raise TimeoutError(f"expected {expected}, last title was {last_title!r}") + + +def _one(state: CacheState, phase: str, path: str) -> dict[str, Any]: + requests = state.matching(phase, path) + if len(requests) != 1: + raise AssertionError(f"expected one {phase} request for {path}, got {requests!r}") + return requests[0] + + +def _verify_trace(state: CacheState, vendor_paths: tuple[str, ...]) -> tuple[str, list[str]]: + entry_path = "/static/asset_cache_boot.js" + pane_path = "/shared/interactive.js" + old_entry = _one(state, "old", entry_path) + current_entry = _one(state, "current", entry_path) + old_pane = _one(state, "old", pane_path) + current_pane = _one(state, "current", pane_path) + + expected_query = f"v={__version__}" + if old_entry["query"] != expected_query or current_entry["query"] != expected_query: + raise AssertionError("entry URL did not retain the same package version across builds") + if old_entry["status"] != 200 or old_pane["status"] != 200: + raise AssertionError("old build did not populate the browser cache") + if current_entry["status"] != 304 or not current_entry["if_none_match"]: + raise AssertionError(f"versioned entry did not revalidate to 304: {current_entry!r}") + if current_pane["status"] != 200 or not current_pane["if_none_match"]: + raise AssertionError( + f"transitive pane did not revalidate to current bytes: {current_pane!r}" + ) + if old_pane["etag"] == current_pane["etag"]: + raise AssertionError("content-derived pane validators did not change") + if current_pane["cache_control"] != "no-cache": + raise AssertionError("transitive pane lost its revalidation policy") + + vendor_trace = [] + immutable = "public, max-age=31536000, immutable" + for path in vendor_paths: + old_vendor = _one(state, "old", path) + if old_vendor["query"] or old_vendor["status"] != 200: + raise AssertionError(f"versioned vendor URL was rewritten or failed: {old_vendor!r}") + if old_vendor["cache_control"] != immutable: + raise AssertionError(f"versioned vendor asset was not immutable: {old_vendor!r}") + revisits = state.matching("current", path) + if revisits: + if len(revisits) != 1 or revisits[0]["status"] not in (200, 304): + raise AssertionError(f"unexpected vendor reload trace: {revisits!r}") + if revisits[0]["cache_control"] != immutable: + raise AssertionError(f"vendor reload lost immutable policy: {revisits[0]!r}") + vendor_trace.append(f"{path}: revisited-{revisits[0]['status']}") + else: + vendor_trace.append(f"{path}: cache-hit") + verdict = f"ASSET-CACHE-READY-entry304-pane200-user1-tool1-vendor{len(vendor_paths)}" + return verdict, vendor_trace + + +def _stop_process(proc: Any) -> None: + if proc.poll() is not None: + return + proc.terminate() + with contextlib.suppress(Exception): + proc.wait(timeout=5) + if proc.poll() is None: + proc.kill() + with contextlib.suppress(Exception): + proc.wait(timeout=2) + + +def main() -> int: + chrome = _find_chrome() + if not chrome: + print("ASSET-CACHE-FAILED-no-chrome") + return 2 + + with tempfile.TemporaryDirectory(prefix="turnstone-asset-cache-e2e-") as raw_scratch: + scratch = Path(raw_scratch) + old_dir, current_dir, static_dir, vendor_paths = _prepare_builds(scratch) + state = CacheState() + server, server_thread, sock, base_url = _start_server( + _make_app(state, old_dir, current_dir, static_dir, vendor_paths) + ) + chrome_proc = None + cdp = None + try: + chrome_proc, cdp_port = _launch_chrome(chrome, scratch / "chrome-profile") + cdp = CDP(_page_ws_url(cdp_port)) + cdp.cmd("Page.enable") + cdp.cmd("Runtime.enable") + cdp.cmd("Network.enable") + cdp.cmd("Page.navigate", {"url": f"{base_url}/asset-cache-e2e"}) + old_result = _wait_for_generation(cdp, "OLD") + + state.phase = "current" + cdp.cmd("Page.reload", {"ignoreCache": False}) + current_result = _wait_for_generation(cdp, "CURRENT") + + if "user_turn=0" not in old_result["url"] or "tool_turn=0" not in old_result["url"]: + raise AssertionError(f"old pane did not expose old capabilities: {old_result!r}") + if ( + "user_turn=1" not in current_result["url"] + or "tool_turn=1" not in current_result["url"] + ): + raise AssertionError( + f"reloaded pane did not expose current capabilities: {current_result!r}" + ) + + verdict, vendor_trace = _verify_trace(state, vendor_paths) + cdp.evaluate(f"document.title = {json.dumps(verdict)}") + print(verdict) + print(f" old EventSource: {old_result['url']}") + print(f" current EventSource: {current_result['url']}") + for item in vendor_trace: + print(f" vendor: {item}") + return 0 + except Exception as exc: + print(f"ASSET-CACHE-FAILED-{type(exc).__name__}: {exc}") + return 1 + finally: + if cdp is not None: + cdp.close() + if chrome_proc is not None: + _stop_process(chrome_proc) + server.should_exit = True + server_thread.join(timeout=10) + with contextlib.suppress(OSError): + sock.close() + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/update-vendored-js.sh b/scripts/update-vendored-js.sh index a5c8bb49..fe09d2e0 100755 --- a/scripts/update-vendored-js.sh +++ b/scripts/update-vendored-js.sh @@ -185,8 +185,8 @@ esac echo "" echo "NOTE: If you added a NEW library (not just updating a version), also update" -echo " the _ASSET_RE regex in turnstone/core/web_helpers.py — its negative lookahead" -echo " skips vendored directories to avoid double-versioning static asset URLs." +echo " _VERSIONED_VENDOR_DIR in turnstone/core/web_helpers.py — it controls both" +echo " HTML version rewriting and immutable static-response caching." echo "" echo "Verify the update:" echo " git diff --stat" diff --git a/tests/test_api_versioning.py b/tests/test_api_versioning.py index 44e18af4..fb88e2ec 100644 --- a/tests/test_api_versioning.py +++ b/tests/test_api_versioning.py @@ -91,6 +91,8 @@ class TestServerVersioning: def test_shared_static_unversioned(self, client): resp = client.get("/shared/base.css") assert resp.status_code == 200 + assert resp.headers["cache-control"] == "no-cache" + assert resp.headers["etag"] class TestConsoleVersioning: @@ -147,3 +149,5 @@ class TestConsoleVersioning: resp = client.get("/static/app.js") body = resp.text assert "/v1/api/cluster" in body + assert resp.headers["cache-control"] == "no-cache" + assert resp.headers["etag"] diff --git a/tests/test_console.py b/tests/test_console.py index 2bbb5a0f..706a4073 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -1587,6 +1587,168 @@ class TestConsoleProxy: mock_collector.get_node_detail.return_value = None resp = client.get("/node/unknown/static/app.js") assert resp.status_code == 404 + assert resp.headers["cache-control"] == "no-store" + + @pytest.mark.parametrize("mount", ["static", "shared"]) + @pytest.mark.parametrize( + "suffix", + [ + "%2e%2e/%2e%2e/v1/api/workstreams/private/history", + "%2E%2E/%2E%2E/v1/api/workstreams/private/history", + "%2e%2e%2f%2e%2e%2fv1%2fapi%2fworkstreams", + "%5c..%5c..%5cv1%5capi%5cworkstreams", + ], + ) + def test_proxy_static_rejects_encoded_traversal_before_upstream(self, mount, suffix): + from starlette.applications import Starlette + from starlette.routing import Route + from starlette.testclient import TestClient + + from turnstone.console import server as csrv + + handler = csrv.proxy_static if mount == "static" else csrv.proxy_shared_static + app = Starlette( + routes=[Route(f"/node/{{node_id}}/{mount}/{{path:path}}", endpoint=handler)] + ) + app.state.proxy_client = MagicMock() + + with TestClient(app) as client: + resp = client.get(f"/node/node-a/{mount}/katex-0.18.4/{suffix}") + + assert resp.status_code == 400 + assert resp.headers["cache-control"] == "no-store" + app.state.proxy_client.get.assert_not_called() + + def test_proxy_static_percent_encodes_each_valid_path_segment(self, monkeypatch): + from types import SimpleNamespace + + import httpx + + from turnstone.console import server as csrv + + upstream = httpx.Response( + 200, + content=b"asset", + request=httpx.Request("GET", "http://n:1/static/nested/asset"), + ) + calls = [] + + async def _mock_get(url, *, headers): + calls.append((url, headers)) + return upstream + + proxy_client = MagicMock(spec=httpx.AsyncClient) + proxy_client.get = MagicMock(side_effect=_mock_get) + request = SimpleNamespace( + app=SimpleNamespace(state=SimpleNamespace(proxy_client=proxy_client)), + path_params={"node_id": "node-a", "path": "nested/asset name?#.js"}, + headers={}, + ) + monkeypatch.setattr(csrv, "_proxy_auth_headers", lambda request: {}) + monkeypatch.setattr(csrv, "_get_server_url", lambda request, node_id: "http://n:1") + + resp = asyncio.run(csrv.proxy_static(request)) + + assert calls == [("http://n:1/static/nested/asset%20name%3F%23.js", {})] + assert resp.status_code == 200 + assert resp.headers["cache-control"] == "no-cache" + + @pytest.mark.parametrize( + ("handler_name", "mount", "path"), + [ + ("proxy_static", "static", "app.js"), + ("proxy_shared_static", "shared", "interactive.js"), + ], + ) + def test_proxy_static_forwards_conditional_request_and_validators( + self, monkeypatch, handler_name, mount, path + ): + from types import SimpleNamespace + + import httpx + + from turnstone.console import server as csrv + + upstream = httpx.Response( + 304, + headers={ + "etag": '"current-build"', + "last-modified": "Wed, 12 Aug 2026 12:00:00 GMT", + }, + request=httpx.Request("GET", f"http://n:1/{mount}/{path}"), + ) + calls = [] + + async def _mock_get(url, *, headers): + calls.append((url, headers)) + return upstream + + proxy_client = MagicMock(spec=httpx.AsyncClient) + proxy_client.get = MagicMock(side_effect=_mock_get) + request = SimpleNamespace( + app=SimpleNamespace(state=SimpleNamespace(proxy_client=proxy_client)), + path_params={"node_id": "node-a", "path": path}, + headers={"if-none-match": '"current-build"'}, + ) + monkeypatch.setattr(csrv, "_proxy_auth_headers", lambda request: {"X-Auth": "test"}) + monkeypatch.setattr(csrv, "_get_server_url", lambda request, node_id: "http://n:1") + + resp = asyncio.run(getattr(csrv, handler_name)(request)) + + assert calls == [ + ( + f"http://n:1/{mount}/{path}", + {"X-Auth": "test", "if-none-match": '"current-build"'}, + ) + ] + assert resp.status_code == 304 + assert resp.headers["cache-control"] == "no-cache" + assert resp.headers["etag"] == '"current-build"' + assert resp.headers["last-modified"] == "Wed, 12 Aug 2026 12:00:00 GMT" + + @pytest.mark.parametrize("status_code", [404, 500]) + def test_proxy_vendor_static_error_is_never_immutable(self, status_code): + import httpx + + from turnstone.console.server import _proxy_static_response + + upstream = httpx.Response( + status_code, + content=b"transient failure", + request=httpx.Request("GET", "http://n:1/shared/katex-0.18.4/missing.css"), + ) + + resp = _proxy_static_response(upstream, "katex-0.18.4/missing.css") + + assert resp.status_code == status_code + assert resp.headers["cache-control"] == "no-store" + + @pytest.mark.parametrize( + ("upstream_policy", "expected"), + [ + ("no-store", "no-store"), + ("private, max-age=600", "private, max-age=600"), + ("public, max-age=0, must-revalidate", "public, max-age=0, must-revalidate"), + ("public, max-age=60", "public, max-age=60"), + ], + ) + def test_proxy_vendor_static_preserves_stricter_upstream_policy( + self, upstream_policy, expected + ): + import httpx + + from turnstone.console.server import _proxy_static_response + + upstream = httpx.Response( + 200, + content=b"asset", + headers={"cache-control": upstream_policy}, + request=httpx.Request("GET", "http://n:1/shared/katex-0.18.4/katex.js"), + ) + + resp = _proxy_static_response(upstream, "katex-0.18.4/katex.js") + + assert resp.headers["cache-control"] == expected def test_proxy_api_unknown_node_returns_404(self, client, mock_collector): mock_collector.get_node_detail.return_value = None @@ -2447,6 +2609,7 @@ class TestProxySharedStatic: client = TestClient(app, raise_server_exceptions=False, headers=_TEST_AUTH_HEADERS) resp = client.get("/node/unknown/shared/base.css") assert resp.status_code == 404 + assert resp.headers["cache-control"] == "no-store" client.close() diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index fd821b1b..cd7f3ad0 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -26,6 +26,8 @@ def client(): def test_valid_ws_id_injects_data_attr(client): + from turnstone import __version__ + ws_id = "a" * 32 resp = client.get(f"/coordinator/{ws_id}") assert resp.status_code == 200 @@ -35,9 +37,27 @@ def test_valid_ws_id_injects_data_attr(client): assert f'data-ws-id="{ws_id}"' in body # Template placeholder is fully substituted. assert "{{WS_ID}}" not in body - # Sanity: the shared static imports are wired. - assert "/shared/base.css" in body - assert "/static/coordinator/coordinator.js" in body + # First-party tags are versioned; version-named vendor assets stay stable. + assert f"/shared/base.css?v={__version__}" in body + assert f"/static/coordinator/coordinator.css?v={__version__}" in body + assert "/shared/katex-0.18.4/katex.min.css?v=" not in body + # Inline module imports are outside version_html's src/href boundary. The + # static route's no-cache policy makes this URL revalidate on every reload. + assert 'from "/static/coordinator/coordinator.js"' in body + + +def test_coordinator_page_revalidates_with_etag(client): + ws_id = "b" * 32 + first = client.get(f"/coordinator/{ws_id}") + assert first.headers["cache-control"] == "no-cache" + assert first.headers["etag"] + + unchanged = client.get( + f"/coordinator/{ws_id}", headers={"If-None-Match": first.headers["etag"]} + ) + assert unchanged.status_code == 304 + assert unchanged.headers["cache-control"] == "no-cache" + assert unchanged.headers["etag"] == first.headers["etag"] def test_non_hex_ws_id_returns_400(client): diff --git a/tests/test_web_helpers.py b/tests/test_web_helpers.py index f207df1f..b38de9a9 100644 --- a/tests/test_web_helpers.py +++ b/tests/test_web_helpers.py @@ -2,6 +2,10 @@ from __future__ import annotations +import os + +import pytest + class TestVersionHtml: def test_app_css_gets_version(self): @@ -54,6 +58,13 @@ class TestVersionHtml: result = version_html(html) assert result == html # unchanged + def test_vendor_prefix_lookalike_is_still_versioned(self): + from turnstone.core.web_helpers import version_html + + html = '' + result = version_html(html) + assert "/shared/hls-2-player.js?v=" in result + def test_external_urls_not_modified(self): from turnstone.core.web_helpers import version_html @@ -116,6 +127,140 @@ class TestVersionHtml: assert result == html # unchanged — already has query string +class TestRevalidatingStaticFiles: + def test_same_versioned_url_revalidates_after_asset_changes(self, tmp_path): + from starlette.applications import Starlette + from starlette.routing import Mount + from starlette.testclient import TestClient + + from turnstone import __version__ + from turnstone.core.web_helpers import RevalidatingStaticFiles + + asset = tmp_path / "app.js" + old_body = b"export const generation = 'old';" + new_body = b"export const generation = 'new';" + assert len(old_body) == len(new_body) + asset.write_bytes(old_body) + original_stat = asset.stat() + app = Starlette( + routes=[Mount("/static", app=RevalidatingStaticFiles(directory=str(tmp_path)))] + ) + + with TestClient(app) as client: + url = f"/static/app.js?v={__version__}" + first = client.get(url) + assert first.status_code == 200 + assert first.headers["cache-control"] == "no-cache" + assert "last-modified" not in first.headers + old_etag = first.headers["etag"] + + asset.write_bytes(new_body) + os.utime( + asset, + ns=(original_stat.st_atime_ns, original_stat.st_mtime_ns), + ) + changed_stat = asset.stat() + assert changed_stat.st_size == original_stat.st_size + assert changed_stat.st_mtime_ns == original_stat.st_mtime_ns + + changed = client.get(url, headers={"If-None-Match": old_etag}) + assert changed.status_code == 200 + assert changed.content == new_body + assert changed.headers["etag"] != old_etag + assert changed.headers["cache-control"] == "no-cache" + assert "last-modified" not in changed.headers + + stale_date_only = client.get( + url, + headers={"If-Modified-Since": "Wed, 31 Dec 9999 23:59:59 GMT"}, + ) + assert stale_date_only.status_code == 200 + assert stale_date_only.content == new_body + + unchanged = client.get(url, headers={"If-None-Match": changed.headers["etag"]}) + assert unchanged.status_code == 304 + assert unchanged.headers["cache-control"] == "no-cache" + + def test_version_named_vendor_asset_is_immutable(self, tmp_path): + from starlette.applications import Starlette + from starlette.routing import Mount + from starlette.testclient import TestClient + + from turnstone.core.web_helpers import RevalidatingStaticFiles + + vendor_dir = tmp_path / "katex-0.18.4" + vendor_dir.mkdir() + (vendor_dir / "katex.min.css").write_text(".katex {}", encoding="utf-8") + app = Starlette( + routes=[Mount("/shared", app=RevalidatingStaticFiles(directory=str(tmp_path)))] + ) + + with TestClient(app) as client: + resp = client.get("/shared/katex-0.18.4/katex.min.css") + + assert resp.status_code == 200 + assert resp.headers["cache-control"] == "public, max-age=31536000, immutable" + assert resp.headers["etag"] + + def test_missing_asset_is_not_cached(self, tmp_path): + from starlette.applications import Starlette + from starlette.routing import Mount + from starlette.testclient import TestClient + + from turnstone.core.web_helpers import RevalidatingStaticFiles + + app = Starlette( + routes=[Mount("/shared", app=RevalidatingStaticFiles(directory=str(tmp_path)))] + ) + + with TestClient(app) as client: + resp = client.get("/shared/not-deployed-yet.js") + + assert resp.status_code == 404 + assert resp.headers["cache-control"] == "no-store" + + +class TestStaticAssetCacheControl: + @pytest.mark.parametrize( + ("path", "expected"), + [ + ("interactive.js", "no-cache"), + ("hls-2-player.js", "no-cache"), + ("katex-0.18.4/katex.min.css", "public, max-age=31536000, immutable"), + ("hljs-11.11.1/highlight.min.js", "public, max-age=31536000, immutable"), + ("katex-0.18.4/../private.json", "no-store"), + (r"katex-0.18.4\..\private.json", "no-store"), + ("nested//asset.js", "no-store"), + ], + ) + def test_policy_requires_a_canonical_exact_vendor_path(self, path, expected): + from turnstone.core.web_helpers import static_asset_cache_control + + assert static_asset_cache_control(path) == expected + + def test_every_packaged_versioned_vendor_directory_uses_the_shared_policy(self): + import re + from pathlib import Path + + import turnstone + from turnstone.core.web_helpers import static_asset_cache_control, version_html + + shared_dir = Path(turnstone.__file__).resolve().parent / "shared_static" + versioned_dir = re.compile(r"^[a-z][a-z0-9_-]*-\d+(?:\.\d+)+$") + vendor_dirs = sorted( + path.name + for path in shared_dir.iterdir() + if path.is_dir() and versioned_dir.fullmatch(path.name) + ) + assert vendor_dirs + + for directory in vendor_dirs: + asset_path = f"{directory}/asset.js" + assert static_asset_cache_control(asset_path) == "public, max-age=31536000, immutable" + html = f'' + assert version_html(html) == html + + class TestLatin1SafeFilename: """Content-Disposition filename sanitizer — must yield a value that is both latin-1 encodable (Starlette) and control-char free (h11).""" diff --git a/turnstone/console/server.py b/turnstone/console/server.py index 60f03fff..21b98fd1 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -14,6 +14,7 @@ import argparse import asyncio import contextlib import functools +import hashlib import json import logging import math @@ -38,7 +39,6 @@ from starlette.background import BackgroundTask from starlette.middleware import Middleware from starlette.responses import HTMLResponse, JSONResponse, Response, StreamingResponse from starlette.routing import Mount, Route -from starlette.staticfiles import StaticFiles from turnstone.api.console_spec import build_console_spec from turnstone.api.docs import make_docs_handler, make_openapi_handler @@ -110,8 +110,12 @@ from turnstone.core.skill_field_validation import SKILL_RUNTIME_CONFIG_FIELDS from turnstone.core.skill_kind import SkillKind from turnstone.core.skill_parser import MAX_SKILL_DESCRIPTION_LEN from turnstone.core.web_helpers import ( + RevalidatingStaticFiles, + is_safe_static_asset_path, read_json_or_400, require_storage_or_503, + static_asset_cache_control, + version_html, ) from turnstone.core.workstream import ( Workstream, @@ -147,10 +151,6 @@ _HTML_ETAG = "" def _load_static() -> None: - import hashlib - - from turnstone.core.web_helpers import version_html - global _HTML, _HTML_ETAG _HTML = version_html((_STATIC_DIR / "index.html").read_text(encoding="utf-8")) _HTML_ETAG = '"' + hashlib.md5(_HTML.encode()).hexdigest()[:16] + '"' # noqa: S324 @@ -3032,52 +3032,66 @@ async def proxy_index(request: Request) -> Response: return JSONResponse({"error": "Node unreachable"}, status_code=502) -async def proxy_static(request: Request) -> Response: - """GET /node/{node_id}/static/{path} — proxy static files.""" +def _proxy_static_request_headers(request: Request) -> dict[str, str]: + """Build upstream headers for a cache-aware static asset request.""" + headers = _proxy_auth_headers(request) + for name in ("if-none-match", "if-modified-since"): + value = request.headers.get(name) + if value: + headers[name] = value + return headers + + +def _proxy_static_response(resp: httpx.Response, path: str) -> Response: + """Preserve upstream validators and apply the local static cache policy.""" + cache_control = "no-store" + if resp.status_code in (200, 304): + cache_control = resp.headers.get("cache-control") or static_asset_cache_control(path) + headers = {"Cache-Control": cache_control} + for name in ("content-type", "etag", "last-modified"): + value = resp.headers.get(name) + if value: + headers[name] = value + return Response(content=resp.content, status_code=resp.status_code, headers=headers) + + +def _static_proxy_error(message: str, status_code: int) -> JSONResponse: + return JSONResponse( + {"error": message}, status_code=status_code, headers={"Cache-Control": "no-store"} + ) + + +async def _proxy_static_mount(request: Request, mount: str) -> Response: + """Proxy one validated static mount without URL-normalization ambiguity.""" node_id = request.path_params["node_id"] path = request.path_params["path"] + if not is_safe_static_asset_path(path): + return _static_proxy_error("Invalid static asset path", 400) server_url = _get_server_url(request, node_id) if not server_url: - return JSONResponse({"error": "Node not found"}, status_code=404) + return _static_proxy_error("Node not found", 404) + encoded_path = "/".join(urllib.parse.quote(segment, safe="") for segment in path.split("/")) client: httpx.AsyncClient = request.app.state.proxy_client try: resp = await client.get( - f"{server_url}/static/{path}", - headers=_proxy_auth_headers(request), - ) - return Response( - content=resp.content, - status_code=resp.status_code, - media_type=resp.headers.get("content-type", "application/octet-stream"), + f"{server_url}/{mount}/{encoded_path}", + headers=_proxy_static_request_headers(request), ) + return _proxy_static_response(resp, path) except httpx.HTTPError as exc: - log.debug("Proxy static error for %s/%s: %s", node_id, path, exc) - return JSONResponse({"error": "Node unreachable"}, status_code=502) + log.debug("Proxy %s error for %s/%s: %s", mount, node_id, path, exc) + return _static_proxy_error("Node unreachable", 502) + + +async def proxy_static(request: Request) -> Response: + """GET /node/{node_id}/static/{path} — proxy static files.""" + return await _proxy_static_mount(request, "static") async def proxy_shared_static(request: Request) -> Response: """GET /node/{node_id}/shared/{path} — proxy shared static files.""" - node_id = request.path_params["node_id"] - path = request.path_params["path"] - server_url = _get_server_url(request, node_id) - if not server_url: - return JSONResponse({"error": "Node not found"}, status_code=404) - - client: httpx.AsyncClient = request.app.state.proxy_client - try: - resp = await client.get( - f"{server_url}/shared/{path}", - headers=_proxy_auth_headers(request), - ) - return Response( - content=resp.content, - status_code=resp.status_code, - media_type=resp.headers.get("content-type", "application/octet-stream"), - ) - except httpx.HTTPError as exc: - log.debug("Proxy shared static error for %s/%s: %s", node_id, path, exc) - return JSONResponse({"error": "Node unreachable"}, status_code=502) + return await _proxy_static_mount(request, "shared") # Auth endpoints the console handles locally instead of forwarding to @@ -3965,14 +3979,18 @@ async def coordinator_page(request: Request) -> Response: if not template_path.is_file(): return JSONResponse({"error": "coordinator UI template missing"}, status_code=500) try: - body = template_path.read_text(encoding="utf-8") + body = version_html(template_path.read_text(encoding="utf-8")) except OSError: return JSONResponse({"error": "failed to read coordinator UI template"}, status_code=500) # Inject the ws_id as an HTML attribute. ws_id passed the # ``_VALID_WS_ID_RE`` gate above (hex only) so there's nothing # to HTML-escape; leave the replacement simple. body = body.replace("{{WS_ID}}", ws_id) - return Response(body, media_type="text/html; charset=utf-8") + etag = '"' + hashlib.md5(body.encode()).hexdigest()[:16] + '"' # noqa: S324 + headers = {"Cache-Control": "no-cache", "ETag": etag} + if request.headers.get("If-None-Match") == etag: + return Response(status_code=304, headers=headers) + return HTMLResponse(body, headers=headers) _CHILDREN_PAGE_LIMIT = 200 @@ -16471,8 +16489,16 @@ def create_app( Route("/metrics", console_metrics_endpoint), Route("/openapi.json", _openapi_handler), Route("/docs", _docs_handler), - Mount("/static", app=StaticFiles(directory=str(_STATIC_DIR)), name="static"), - Mount("/shared", app=StaticFiles(directory=str(_SHARED_DIR)), name="shared"), + Mount( + "/static", + app=RevalidatingStaticFiles(directory=str(_STATIC_DIR)), + name="static", + ), + Mount( + "/shared", + app=RevalidatingStaticFiles(directory=str(_SHARED_DIR)), + name="shared", + ), # Coordinator one-pane UI — the route serves a single # index.html template with the ws_id injected via data-ws-id # so coordinator.js can pull it without an extra round-trip. diff --git a/turnstone/core/web_helpers.py b/turnstone/core/web_helpers.py index b3bc5a37..828dae5c 100644 --- a/turnstone/core/web_helpers.py +++ b/turnstone/core/web_helpers.py @@ -3,15 +3,23 @@ from __future__ import annotations import contextlib +import hashlib import json import os import re +from functools import cached_property from typing import TYPE_CHECKING, Any +from starlette.datastructures import Headers +from starlette.exceptions import HTTPException +from starlette.responses import FileResponse, PlainTextResponse, Response +from starlette.staticfiles import StaticFiles + if TYPE_CHECKING: from starlette.middleware import Middleware from starlette.requests import Request from starlette.responses import JSONResponse + from starlette.types import Scope def latin1_safe_filename(name: str, *, fallback: str = "attachment") -> str: @@ -463,6 +471,103 @@ def cors_middleware(origins: list[str]) -> Middleware: # Static asset cache-busting # --------------------------------------------------------------------------- +# Keep this directory definition shared by the HTML rewriter and static response +# policy. These directories already carry their library version in the URL, +# so their contents can be cached immutably; every other first-party asset must +# revalidate because ES-module imports do not inherit the entry module's ?v=. +_VERSIONED_VENDOR_DIR = r"(?:katex|hljs|hls|mermaid)-\d+(?:\.\d+)+" +_VERSIONED_VENDOR_PATH_RE = re.compile(rf"^{_VERSIONED_VENDOR_DIR}/") + + +def is_safe_static_asset_path(path: str) -> bool: + """Return whether *path* is a canonical mount-relative asset path. + + Starlette decodes percent escapes before populating ``{path:path}``, and + HTTPX normalizes dot segments when it builds an outbound URL. Reject the + ambiguous forms before a console proxy can leave its static mount. + """ + return ( + bool(path) + and "\\" not in path + and all(segment not in {"", ".", ".."} for segment in path.split("/")) + ) + + +def static_asset_cache_control(path: str) -> str: + """Return the cache policy for an asset path relative to its mount.""" + if not is_safe_static_asset_path(path): + return "no-store" + if _VERSIONED_VENDOR_PATH_RE.match(path): + return "public, max-age=31536000, immutable" + return "no-cache" + + +class RevalidatingStaticFiles(StaticFiles): + """StaticFiles with correctness-first caching for first-party assets. + + First-party files use a content-derived ETag and no Last-Modified header. + ``no-cache`` therefore revalidates by content on reload, even when two + builds share a package version, byte length, and filesystem timestamp. + Version-named vendor directories retain immutable caching. + """ + + @cached_property + def _content_etags(self) -> dict[str, tuple[tuple[int, int, int], str]]: + """Lazily allocate the bounded per-static-tree validator cache.""" + return {} + + def _content_etag( + self, + full_path: str | os.PathLike[str], + stat_result: os.stat_result, + ) -> str: + path = os.fspath(full_path) + signature = (stat_result.st_mtime_ns, stat_result.st_ctime_ns, stat_result.st_size) + cached = self._content_etags.get(path) + if cached is not None and cached[0] == signature: + return cached[1] + with open(path, "rb") as asset: + digest = hashlib.file_digest(asset, "sha256").hexdigest() + etag = f'"sha256-{digest}"' + self._content_etags[path] = (signature, etag) + return etag + + def file_response( + self, + full_path: str | os.PathLike[str], + stat_result: os.stat_result, + scope: Scope, + status_code: int = 200, + ) -> Response: + asset_path = self.get_path(scope) + if static_asset_cache_control(asset_path) != "no-cache": + return super().file_response(full_path, stat_result, scope, status_code) + + request_headers = Headers(scope=scope) + response = FileResponse(full_path, status_code=status_code, stat_result=stat_result) + etag = self._content_etag(full_path, stat_result) + response.headers["ETag"] = etag + del response.headers["Last-Modified"] + if self.is_not_modified(response.headers, request_headers): + return Response(status_code=304, headers={"ETag": etag}) + return response + + async def get_response(self, path: str, scope: Scope) -> Response: + try: + response = await super().get_response(path, scope) + except HTTPException as exc: + if exc.status_code != 404: + raise + return PlainTextResponse( + "Not Found", status_code=404, headers={"Cache-Control": "no-store"} + ) + cache_control = ( + "no-store" if response.status_code >= 400 else static_asset_cache_control(path) + ) + response.headers["Cache-Control"] = cache_control + return response + + # Matches src="/static/..." and href="/shared/..." (and vice-versa) but skips # vendored libraries whose directory names already contain a version number # (e.g. katex-0.16.44/, hljs-11.11.1/) and URLs that already have a query @@ -470,7 +575,7 @@ def cors_middleware(origins: list[str]) -> Middleware: _ASSET_RE = re.compile( r'(?P(?:src|href)=")' r"(?P/(?:static|shared)/)" - r"(?!(?:katex|hljs|hls|mermaid)-\d)" + rf"(?!{_VERSIONED_VENDOR_DIR}/)" r'(?P[^"?]+)"' ) @@ -480,7 +585,7 @@ def version_html(html: str) -> str: Vendored libraries with version-bearing directory names are skipped. URLs that already contain a query string are left unchanged. - Called once at startup when loading HTML into memory. + Safe to call either at startup or while rendering a dynamic template. """ from turnstone import __version__ diff --git a/turnstone/server.py b/turnstone/server.py index efb838f2..45c9ca27 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -42,7 +42,6 @@ from starlette.middleware import Middleware from starlette.requests import Request from starlette.responses import HTMLResponse, JSONResponse, Response, StreamingResponse from starlette.routing import Mount, Route -from starlette.staticfiles import StaticFiles from turnstone import __version__ from turnstone.api.docs import make_docs_handler, make_openapi_handler @@ -114,6 +113,7 @@ from turnstone.core.session_ui_base import ( ) from turnstone.core.tools import TOOLS # noqa: F401 — available for introspection from turnstone.core.trajectory import final_assistant_text +from turnstone.core.web_helpers import RevalidatingStaticFiles from turnstone.core.web_helpers import version_html as _version_html from turnstone.core.workstream import ( Workstream, @@ -5802,8 +5802,16 @@ def create_app( Route("/metrics", metrics_endpoint), Route("/openapi.json", _openapi_handler), Route("/docs", _docs_handler), - Mount("/static", app=StaticFiles(directory=str(_STATIC_DIR)), name="static"), - Mount("/shared", app=StaticFiles(directory=str(_SHARED_DIR)), name="shared"), + Mount( + "/static", + app=RevalidatingStaticFiles(directory=str(_STATIC_DIR)), + name="static", + ), + Mount( + "/shared", + app=RevalidatingStaticFiles(directory=str(_SHARED_DIR)), + name="shared", + ), ], middleware=_build_middleware(cors_origins), lifespan=_lifespan,