fix(server): serve contract webapp HTML with Cache-Control: no-store

.claude/rules/browser-assets.md

@@ -105,3 +105,24 @@ Anything the shell holds — a connection, a timer, a listener, an observer —
 held once per open tab, and users routinely keep several Freenet apps open
 against one origin. Before adding a persistent resource, multiply it by ten
 tabs and check the browser-imposed ceiling.
+
+### 6. Contract HTML is served `no-store`, by route, not by handler
+
+A contract's bundle changes at a stable URL on every republish, so a cached
+HTML document under `/v{1,2}/contract/web/<key>/` pins an open tab to the old
+build (#5323). The guarantee is the `no_store_html(...)` wrapper in
+`client_api/v1.rs` and `v2.rs`, keyed on the HTML `Content-Type`, not any
+handler. A new route that serves contract HTML (or a new API version module)
+must be registered inside it. Nothing will fail if it is not, because
+`contract_html_is_served_no_store_and_assets_are_not` only exercises the
+routes it lists.
+
+Assets are deliberately NOT `no-store`: the reverse proxy in front of
+try.freenet.org (its `@assets` matcher) adds
+`Cache-Control: public, max-age=31536000, immutable` to files directly under
+`assets/` and passes the node's header through alongside, so a `no-store` from
+the node would cancel that in the browser. Unhashed assets therefore stay
+heuristically cacheable through `ServeFile`'s `Last-Modified`; the fix for
+those is conditional requests plus `no-cache` (#5888), not `no-store`. The
+match is by `Content-Type` (`text/html`, `application/xhtml+xml`), so an SVG
+loaded as a document is not covered.

Cargo.toml

@@ -44,7 +44,7 @@ tokio = { version = "1", features = ["full"] }
 tokio-stream = "0.1.18"
 tokio-tungstenite = "0.27.0"
 tokio-util = "0.7"
-tower-http = { version = "0.7", features = ["fs", "trace"] }
+tower-http = { version = "0.7", features = ["fs", "set-header", "trace"] }
 
 # Crypto
 aes-gcm = "0.10"

crates/core/src/server/client_api/v1.rs

@@ -24,8 +24,13 @@ pub(super) fn routes(config: Config) -> Router {
         // and is the modern equivalent of 301 for GETs.
         // (freenet/freenet-core#4019)
         .route("/v1/contract/web/{key}", get(web_root_redirect_v1))
-        .route("/v1/contract/web/{key}/", get(web_home_v1))
-        .route("/v1/contract/web/{key}/{*path}", get(web_subpages_v1))
+        // Everything under the key serves contract content; its HTML documents
+        // must not be cached (#5323). The redirect above stays outside.
+        .merge(no_store_html(
+            Router::new()
+                .route("/v1/contract/web/{key}/", get(web_home_v1))
+                .route("/v1/contract/web/{key}/{*path}", get(web_subpages_v1)),
+        ))
         .with_state(config)
 }
 

crates/core/src/server/client_api/v2.rs

@@ -16,8 +16,13 @@ pub(super) fn routes(config: Config) -> Router {
         .route("/v2", get(home))
         // No-trailing-slash redirect — see v1.rs for the rationale.
         .route("/v2/contract/web/{key}", get(web_root_redirect_v2))
-        .route("/v2/contract/web/{key}/", get(web_home_v2))
-        .route("/v2/contract/web/{key}/{*path}", get(web_subpages_v2))
+        // Everything under the key serves contract content; its HTML documents
+        // must not be cached (#5323). The redirect above stays outside.
+        .merge(no_store_html(
+            Router::new()
+                .route("/v2/contract/web/{key}/", get(web_home_v2))
+                .route("/v2/contract/web/{key}/{*path}", get(web_subpages_v2)),
+        ))
         .with_state(config)
 }
 

crates/core/src/server/path_handlers.rs

@@ -1885,8 +1885,9 @@ fn structural_path_missing_response(
 ///   default where the 500 this replaced was not, so without this a transient
 ///   miss could be remembered by the browser and strand the app once the
 ///   bundle does unpack. `errors.rs` sets `no-store` on its transient answers
-///   for the same reason. (#5323 tracks this route having no cache policy at
-///   all; this is not that fix.)
+///   for the same reason. The route-level layer added for #5323
+///   (`client_api::no_store_html`) covers HTML documents only, so this header
+///   is what keeps the miss uncacheable.
 fn asset_not_found_response() -> axum::response::Response {
     (
         axum::http::StatusCode::NOT_FOUND,

crates/core/tests/playwright/tests/contract-cache-headers.spec.ts

@@ -0,0 +1,94 @@
+import { test, expect, type Response } from "@playwright/test";
+
+// Contract HTML documents must be `Cache-Control: no-store` (freenet-core#5323).
+//
+// A contract's bundle is mutable at a stable URL: `fdev website update` changes
+// what `/v1/contract/web/<key>/` returns while the URL stays the same. A cached
+// document pins an open tab to the superseded build, and the app cannot fix
+// that itself, because the stale document is the one that would have carried
+// the fix. Assets are deliberately left alone (see `no_store_html` in
+// `client_api.rs`).
+//
+// The header is applied by a layer over the contract-content routes in
+// `client_api/v1.rs` and `v2.rs`, keyed on the HTML `Content-Type`. The Rust
+// router test (`contract_html_is_served_no_store_and_assets_are_not` in
+// `client_api.rs`) drives every pipeline, including the JS rewrite branch and
+// `ServeFile` assets, which this fixture never reaches because it contains
+// only HTML. What it cannot see is the router as the harness node composes it,
+// and what an engine actually receives for the documents it loads, including
+// the sandboxed iframe. That is what this spec checks.
+
+const shellUrl = process.env.FREENET_SHELL_URL;
+
+test.beforeAll(() => {
+  if (!shellUrl) {
+    throw new Error(
+      "FREENET_SHELL_URL is not set. These tests are normally driven by the " +
+        "Rust harness (crates/core/tests/playwright_shell.rs), which boots a " +
+        "node, publishes the fixture, and exports the shell URL.",
+    );
+  }
+});
+
+/** `/v1/contract/web/<key>` for the fixture, without the trailing slash. */
+function contractBase(): string {
+  const url = new URL(shellUrl!);
+  const match = url.pathname.match(/^(\/v\d+\/contract\/web\/[^/]+)\//);
+  if (!match) {
+    throw new Error(`could not read a contract key out of ${shellUrl}`);
+  }
+  return `${url.origin}${match[1]}`;
+}
+
+test("the shell and the sandboxed app document are served no-store", async ({
+  page,
+}) => {
+  const base = contractBase();
+  const contractResponses: Response[] = [];
+  page.on("response", (response) => {
+    if (response.url().startsWith(`${base}/`)) {
+      contractResponses.push(response);
+    }
+  });
+
+  await page.goto(`${base}/`);
+  // The iframe's document is the contract's own HTML, served by a different
+  // pipeline from the shell. Waiting for its content means its response has
+  // been recorded too.
+  const app = page.frameLocator("iframe");
+  await expect(app.locator("body")).not.toBeEmpty();
+
+  const isSandbox = (r: Response) =>
+    new URL(r.url()).searchParams.get("__sandbox") === "1";
+  // Only documents are covered; assets keep their caching (#5888). The fixture
+  // is all HTML, so this skips nothing today.
+  const documents: { response: Response; cacheControl?: string }[] = [];
+  for (const response of contractResponses) {
+    // `allHeaders` rather than `headers`: the latter omits some headers in
+    // some engines.
+    const headers = await response.allHeaders();
+    if (/^(text\/html|application\/xhtml\+xml)\b/i.test(headers["content-type"] ?? "")) {
+      documents.push({ response, cacheControl: headers["cache-control"] });
+    }
+  }
+  // Both documents must be among them, or the assertions below could pass
+  // over a set that is missing the pipeline they are meant to cover.
+  expect(documents.some(({ response }) => !isSandbox(response))).toBe(true);
+  expect(documents.some(({ response }) => isSandbox(response))).toBe(true);
+
+  for (const { response, cacheControl } of documents) {
+    expect(
+      cacheControl,
+      `${response.url()} must be no-store, or an open tab keeps running the ` +
+        `old bundle after a republish (#5323)`,
+    ).toBe("no-store");
+  }
+});
+
+test("the no-trailing-slash redirect stays cacheable", async ({ request }) => {
+  // It carries no contract content, and it is a 308 precisely so that browsers
+  // can cache it (#4019). The no-store layer must not reach it.
+  const response = await request.get(contractBase(), { maxRedirects: 0 });
+  expect(response.status()).toBe(308);
+  expect(response.headers()["cache-control"]).toBeUndefined();
+});