Close the PWA first-paint gap: ETag revalidation + inlined critical CSS

docs/ASSET_PIPELINE.md

@@ -16,9 +16,9 @@ All of it runs in `scripts/build_assets.mjs`, in the Docker builder stage, befor
 
 2. The 38 JS modules (37 in the runtime import graph — `types.js` is JSDoc-only) get bundled into a single file with esbuild. `minify: true` handles identifier mangling, syntax compression, and whitespace removal, including collapsing the newlines in HTML template strings that esbuild normally leaves alone. Asset references get rewritten, then the whole thing gets content-hashed.
 
-3. The 14 CSS files get concatenated and run through esbuild's CSS transformer. Non-critical styles (everything except `critical.css`) merge into a single `app.css`. Both get content-hashed.
+3. The 14 CSS files get concatenated and run through esbuild's CSS transformer. Non-critical styles (everything except `critical.css`) merge into a single, content-hashed `app.css`. `critical.css` itself is minified but *not* fingerprinted as a file — it's inlined straight into `index.html` (step 4) so first paint (the inline `#loading` screen) doesn't wait on a second network round trip beyond the document itself. Its sha256 is written to `dist/csp.json`.
 
-4. `index.html` gets rewritten: the 13 non-critical `<link>` tags collapse to one, the modulepreload graph (37 entries) drops, every URL swaps to its hashed equivalent, and the HTML gets stripped of comments and collapsed to a single line. The web-app manifest is fingerprinted the same pass (`manifest.webmanifest`, step 4b): its icon refs are rewritten to hashed paths and its `<link rel="manifest">` href updated to match.
+4. `index.html` gets rewritten: the critical.css `<link>` becomes an inline `<style>`, the 13 non-critical `<link>` tags collapse to one, the modulepreload graph (37 entries) drops, every remaining URL swaps to its hashed equivalent, and the HTML gets stripped of comments and collapsed to a single line. The web-app manifest is fingerprinted the same pass (`manifest.webmanifest`, step 4b): its icon refs are rewritten to hashed paths and its `<link rel="manifest">` href updated to match. Since the CSP is `style-src 'self'` with no `unsafe-inline`, the inlined `<style>` only stays policy-compliant because `server/security.py` reads `dist/csp.json` at startup and allows exactly that content via a `'sha256-...'` source — a content-pinned allowance, not a general inline-style exemption.
 
 5. Every text asset (JS, CSS, HTML, SVG, webmanifest) gets a `.gz` sibling at level 9 compression. nginx's `gzip_static` serves the pre-compressed file directly. No CPU cost per request.
 
@@ -79,6 +79,8 @@ On repeat loads, prod assets serve from disk cache in 0ms. Content-hashed filena
 
 Dev appends `?v=<hash>` to every URL at server startup, and the app shell plus every dev-served `/static` response now carries `Cache-Control: no-cache` (`server/security.py`, `main.py`), so the browser revalidates rather than trusting a stale copy. That matters for an installed PWA: a cached HTML shell pointing at old hashed asset URLs can boot a version-skewed module graph and hang on the loading screen. Prod is unaffected — nginx serves the content-hashed `/static` bundles with their own far-future `immutable` cache, and this middleware never runs for them.
 
+The app shell document itself (`/`, `/games/{code}`, `/@{username}`, …) always revalidates (`no-cache`), in both dev and prod — but `server/routes.py` sends an `ETag` on every HTML page response, so a repeat load (an installed PWA relaunching at its `start_url`) revalidates with a cheap `304 Not Modified` instead of re-sending the whole document. Without a validator, `no-cache` alone forces a full re-fetch on every load — on a slow connection, that's real, avoidable latency before the browser even discovers the CSS.
+
 ---
 
 ## Background media

scripts/build_assets.mjs

@@ -13,9 +13,13 @@
 //     string references (e.g. logo-loser.svg used from JS, the font + poster
 //     url() in critical.css) rewritten to the hashed paths before *their* hash
 //     is taken. esbuild does not rewrite string-literal URLs, so we do it.
-//   * critical.css stays a separate <link> (NOT inlined): the CSP is
-//     `style-src 'self'` with no 'unsafe-inline', so an inline <style> would be
-//     a policy violation.
+//   * critical.css is inlined as a <style> in dist/index.html — first paint (the
+//     inline #loading screen) shouldn't depend on a second network round trip on
+//     top of the document itself. The CSP is `style-src 'self'` with no
+//     'unsafe-inline', so this only stays policy-compliant because we pin the
+//     exact inlined bytes: their sha256 goes to dist/csp.json, and
+//     server/security.py adds it to style-src as 'sha256-<hash>' — a narrow,
+//     content-specific allowance, not a general inline-style exemption.
 import esbuild from 'esbuild';
 import { createHash } from 'node:crypto';
 import { gzipSync } from 'node:zlib';
@@ -122,8 +126,8 @@ const indexHtml = readFileSync(join(SRC, 'index.html'), 'utf8');
 // @layer): each asset is fingerprinted on its own, never concatenated into
 // app.css. Declared once here so the bundle-exclusion filters below AND the
 // step-4c hashing loop stay in sync — add a page's assets here and they're both
-// excluded from the bundle and fingerprinted. `critical` is the inline critical
-// sheet, hashed separately in step 4 (css-only, no js).
+// excluded from the bundle and fingerprinted. `critical` is handled separately
+// in step 4 — inlined into index.html, not fingerprinted as its own file.
 const STANDALONE_ASSETS = [
   ['css', 'widget', '.css', 'css'],
   ['js', 'widget', '.js', 'js'],
@@ -147,13 +151,21 @@ const NONCRIT = [...linked, ...unlinked];
   manifest.set('/static/css/app.css', writeHashed('css', 'app', '.css', rewriteRefs(min)));
 }
 
-// ── 4. Minify critical.css, rewrite its url() refs (font + poster), hash ──────
+// ── 4. Minify critical.css, rewrite its url() refs (font + poster) ────────────
+// Not written to a hashed file / not added to `manifest`: it's inlined straight
+// into index.html below (step 5), not linked, so there's no URL to rewrite refs
+// to and no fingerprinted file to serve. Its sha256 goes to dist/csp.json so
+// server/security.py can allow exactly this content in the CSP's style-src.
+let criticalCss;
 {
   const raw = readFileSync(join(SRC, 'css', 'critical.css'), 'utf8');
   const min = (await esbuild.transform(raw, { loader: 'css', minify: true })).code;
-  manifest.set('/static/css/critical.css',
-    writeHashed('css', 'critical', '.css', rewriteRefs(min)));
+  // Flattened to one line so the later whole-document `split('\n').map(trim)`
+  // pass (step 5) can't touch anything inside the <style> tag's text — the CSP
+  // hash below is taken over these exact bytes, so nothing may reshape them.
+  criticalCss = rewriteRefs(min).replace(/[ \t]*\n[ \t]*/g, ' ').trim();
 }
+const criticalCssSha256 = createHash('sha256').update(criticalCss, 'utf8').digest('base64');
 
 // ── 4c. Minify the standalone pages' assets (never part of the app bundle),
 // rewrite refs (wood poster in the CSS), hash. The app resolves the hashed URLs
@@ -178,7 +190,20 @@ for (const [sub, name, ext, loader] of STANDALONE_ASSETS) {
 
 // ── 5. Rewrite index.html ─────────────────────────────────────────────────────
 let html = readFileSync(join(SRC, 'index.html'), 'utf8');
-html = rewriteRefs(html); // images, fonts, critical.css link
+html = rewriteRefs(html); // images, fonts, manifest link
+
+// inline critical.css — first paint (the inline #loading screen) shouldn't
+// wait on a second network round trip beyond the document itself. Its CSP
+// hash (dist/csp.json) is what keeps this compliant with style-src 'self'.
+// Anchored on the comment (like the two collapses below) rather than the
+// literal <link> markup, so it isn't broken by an incidental attribute/
+// formatting change to that tag. Lazy `[^]*?` — unlike the greedy `[^]*`
+// below, this one isn't the last `.css">` in the document, so a greedy match
+// would run past it and swallow the non-critical block that follows too.
+html = html.replace(
+  /  <!-- Critical CSS[^]*?critical\.css">\n/,
+  `  <style>${criticalCss}</style>\n`,
+);
 
 // collapse the 9 non-critical stylesheet links into one bundled link
 html = html.replace(
@@ -199,6 +224,11 @@ writeFileSync(join(DIST, 'index.html'), html);
 writeFileSync(join(DIST, 'manifest.json'),
   JSON.stringify(Object.fromEntries(manifest), null, 1));
 
+// ── 7b. CSP hash for the inlined critical.css <style>, for server/security.py
+// to add to style-src (a content-pinned allowance, not a general 'unsafe-inline').
+writeFileSync(join(DIST, 'csp.json'),
+  JSON.stringify({ 'style-src-sha256': criticalCssSha256 }, null, 1));
+
 // ── 6. Pre-compress text assets for nginx gzip_static ─────────────────────────
 let gz = 0;
 for (const file of walk(DIST)) {
@@ -211,4 +241,4 @@ for (const file of walk(DIST)) {
 console.log(`built dist/: ${manifest.size} fingerprinted assets, ${gz} gzipped`);
 console.log(`  js  -> ${manifest.get('/static/js/app.js')}`);
 console.log(`  css -> ${manifest.get('/static/css/app.css')}`);
-console.log(`  crit-> ${manifest.get('/static/css/critical.css')}`);
+console.log(`  crit-> inlined, sha256-${criticalCssSha256}`);

server/routes.py

@@ -1,4 +1,5 @@
 import asyncio
+import hashlib
 import math
 import re
 from pathlib import Path
@@ -88,9 +89,21 @@ def _require_telemetry() -> None:
         raise HTTPException(status_code=503, detail="telemetry disabled")
 
 
+def _html_response(request: Request, body: str) -> Response:
+    """HTML page response with an ETag, so a repeat load (e.g. reopening an
+    installed PWA at its start_url) can revalidate with a cheap 304 instead of
+    a full re-fetch — the document is always Cache-Control: no-cache (audit:
+    SecurityHeadersMiddleware, to stop a PWA running a version-skewed module
+    graph), so without a validator every load re-sent the whole body."""
+    etag = f'"{hashlib.sha1(body.encode()).hexdigest()[:16]}"'
+    if request.headers.get("if-none-match") == etag:
+        return Response(status_code=304, headers={"ETag": etag})
+    return HTMLResponse(body, headers={"ETag": etag})
+
+
 @router.get("/")
-async def root() -> HTMLResponse:
-    return HTMLResponse(_render_index())
+async def root(request: Request) -> Response:
+    return _html_response(request, _render_index())
 
 
 @router.get("/metrics", dependencies=[Depends(_bearer_guard(METRICS_TOKEN))])
@@ -180,23 +193,23 @@ async def stats_game(game_code: str) -> dict:
 # 404s so this can't shadow favicons or other single-segment asset requests.
 # Declared last so the explicit routes above (/, /metrics, /stats/*) win.
 @router.get("/join")
-async def join_page() -> HTMLResponse:
-    return HTMLResponse(_render_index())
+async def join_page(request: Request) -> Response:
+    return _html_response(request, _render_index())
 
 
 @router.get("/signin")
-async def signin_page() -> HTMLResponse:
-    return HTMLResponse(_render_index())
+async def signin_page(request: Request) -> Response:
+    return _html_response(request, _render_index())
 
 
 @router.get("/welcome")
-async def welcome_page() -> HTMLResponse:
-    return HTMLResponse(_render_index())
+async def welcome_page(request: Request) -> Response:
+    return _html_response(request, _render_index())
 
 
 @router.get("/nearby")
-async def nearby_page() -> HTMLResponse:
-    return HTMLResponse(_render_index())
+async def nearby_page(request: Request) -> Response:
+    return _html_response(request, _render_index())
 
 
 def _valid_coords(lat: float, lon: float) -> bool:
@@ -581,16 +594,16 @@ async def verify_roll(code: str, pid: str, roll_count: int) -> dict:
 
 
 @router.get("/games/{code}")
-async def game_detail_page(code: str) -> HTMLResponse:
-    return HTMLResponse(_render_index())
+async def game_detail_page(code: str, request: Request) -> Response:
+    return _html_response(request, _render_index())
 
 
 # Vanity profile URLs: tensies.app/@username. The @ prefix guarantees no
 # collision with game codes (which are [A-Za-z]{5}).
 @router.get("/@{username}")
-async def profile_vanity(username: str) -> HTMLResponse:
+async def profile_vanity(username: str, request: Request) -> Response:
     if not TELEMETRY_ENABLED:
-        return HTMLResponse(_render_index())
+        return _html_response(request, _render_index())
     try:
         from server.telemetry import store
         async with store.pool().acquire() as con:
@@ -599,7 +612,7 @@ async def profile_vanity(username: str) -> HTMLResponse:
                 username.lower(),
             )
             if user is None:
-                return HTMLResponse(_render_index())
+                return _html_response(request, _render_index())
             stats = await con.fetchrow(
                 "SELECT total_wins, total_games FROM player_stats WHERE user_id = ("
                 "SELECT id::text FROM users WHERE LOWER(username) = $1)",
@@ -621,10 +634,10 @@ async def profile_vanity(username: str) -> HTMLResponse:
             share_description=" ".join(desc_parts),
             canonical_url=f"{base}/@{display}" if base else f"/@{display}",
         )
-        return HTMLResponse(html)
+        return _html_response(request, html)
     except Exception:
         log.exception("profile meta injection failed for @%s", username)
-        return HTMLResponse(_render_index())
+        return _html_response(request, _render_index())
 
 
 # Clean join URLs: GET /<code> serves the SPA, which reads the code from the

server/security.py

@@ -4,12 +4,19 @@
 headers onto every HTTP response, including the index page and /static assets.
 
 The CSP is strict same-origin with no 'unsafe-inline': the frontend has no
-inline scripts, styles, or event handlers and loads every asset from /static,
-so this applies cleanly and turns any future inline-script/innerHTML sink into
-a visible CSP violation. `connect-src 'self'` also covers the same-origin
-WebSocket. `upgrade-insecure-requests` is added only when HSTS is on (i.e. a
-real HTTPS deploy), so plain-http dev isn't forced to upgrade to https.
+inline scripts or event handlers and loads every other asset from /static, so
+this applies cleanly and turns any future inline-script/innerHTML sink into a
+visible CSP violation. The one deliberate exception is prod's inlined
+critical.css <style> (scripts/build_assets.mjs) — allowed via a content-pinned
+`'sha256-...'` source (_critical_style_hash below), not a general style
+exemption, since only that exact byte sequence satisfies the hash.
+`connect-src 'self'` also covers the same-origin WebSocket.
+`upgrade-insecure-requests` is added only when HSTS is on (i.e. a real HTTPS
+deploy), so plain-http dev isn't forced to upgrade to https.
 """
+import json
+from pathlib import Path
+
 from starlette.datastructures import MutableHeaders
 from starlette.requests import HTTPConnection
 
@@ -18,13 +25,15 @@
     CSP_EXTRA_IMG_SRC,
     CSP_EXTRA_SCRIPT_SRC,
     CSP_OVERRIDE,
+    FRONTEND_DIST,
     HSTS_ENABLED,
     HSTS_INCLUDE_SUBDOMAINS,
     HSTS_MAX_AGE,
     HSTS_PRELOAD,
     SECURITY_HEADERS,
     TRUST_PROXY_HEADERS,
     TRUSTED_PROXY_HOPS,
+    log,
 )
 
 
@@ -52,15 +61,33 @@ def _directive(name: str, *sources: str) -> str:
     return " ".join((name, *sources))
 
 
+def _critical_style_hash() -> str | None:
+    """CSP source for the inlined critical.css <style> in prod's dist/index.html
+    (scripts/build_assets.mjs writes its sha256 to dist/csp.json). A narrow,
+    content-pinned allowance — not a general 'unsafe-inline' exemption, since
+    only that exact byte sequence matches the hash. None in dev, where
+    critical.css stays an external <link> and needs no CSP allowance."""
+    if not FRONTEND_DIST:
+        return None
+    try:
+        digest = json.loads((Path(FRONTEND_DIST) / "csp.json").read_text())["style-src-sha256"]
+    except (OSError, ValueError, KeyError):
+        log.error("dist/csp.json unreadable — inlined critical.css will violate "
+                   "the CSP (rebuild dist/ with scripts/build_assets.mjs)")
+        return None
+    return f"'sha256-{digest}'"
+
+
 def build_csp() -> str:
     if CSP_OVERRIDE:
         return CSP_OVERRIDE
     # script-src / connect-src can be extended with extra hosts (e.g. an
     # analytics beacon) via env, without rewriting the whole policy.
+    style_sources = ["'self'", *filter(None, [_critical_style_hash()])]
     directives = [
         "default-src 'self'",
         _directive("script-src", "'self'", *CSP_EXTRA_SCRIPT_SRC),
-        "style-src 'self'",
+        _directive("style-src", *style_sources),
         _directive("img-src", "'self'", "data:", *CSP_EXTRA_IMG_SRC),
         "font-src 'self'",
         _directive("connect-src", "'self'", *CSP_EXTRA_CONNECT_SRC),

tests/assets_test.py

@@ -3,13 +3,17 @@
 Runs the real build (scripts/build_assets.mjs) into dist/ and asserts the
 properties the prod serving path depends on: every reference is fingerprinted
 and resolvable, the document is bundled (no raw module graph), critical.css is
-a separate <link> (never inlined — the CSP forbids inline styles), and every
-text asset has a .gz sibling that round-trips. nginx serves dist/ verbatim, so
-if these hold the prod frontend is internally consistent.
+inlined as a <style> whose sha256 matches dist/csp.json (so server/security.py
+can allow it in the CSP's style-src without a general 'unsafe-inline'), and
+every text asset has a .gz sibling that round-trips. nginx serves dist/
+verbatim, so if these hold the prod frontend is internally consistent.
 
 Run:  python tests/assets_test.py    (requires node; builds dist/ as a side effect)
 """
+import base64
 import gzip
+import hashlib
+import json
 import re
 import subprocess
 import sys
@@ -62,13 +66,24 @@ def main():
           "index.html points at the hashed JS bundle")
     check(re.search(r"/static/css/app-[0-9a-f]{8}\.css", html) is not None,
           "index.html points at the hashed CSS bundle")
-    check(html.count('rel="stylesheet"') == 2,
-          "exactly two stylesheet links (critical + bundle)")
-
-    # ── critical.css stays a separate <link>, never an inline <style> (CSP) ──
-    check("<style" not in html, "no inline <style> (CSP style-src 'self')")
-    check(re.search(r"/static/css/critical-[0-9a-f]{8}\.css", html) is not None,
-          "critical.css served as a fingerprinted <link>")
+    check(html.count('rel="stylesheet"') == 1,
+          "exactly one stylesheet link (the non-critical bundle)")
+    check("/static/css/critical" not in html,
+          "critical.css is not linked as a separate file")
+
+    # ── critical.css is inlined as a <style>, pinned by a CSP hash ──
+    style_match = re.search(r"<style>(.*?)</style>", html, re.S)
+    check(style_match is not None, "index.html has an inline <style> (critical.css)")
+    check(html.count("<style>") == 1, "exactly one inline <style> block")
+
+    csp_path = DIST / "csp.json"
+    check(csp_path.is_file(), "dist/csp.json exists")
+    if style_match and csp_path.is_file():
+        csp = json.loads(csp_path.read_text())
+        expected = csp.get("style-src-sha256")
+        actual = base64.b64encode(hashlib.sha256(style_match.group(1).encode()).digest()).decode()
+        check(expected == actual,
+              "dist/csp.json's style-src-sha256 matches the inlined <style> content")
 
     # ── every /static reference (html + bundles) is hashed and resolvable ──
     refs = set(STATIC_REF.findall(html))