Answer stability round 1: 0 🔴, 4 🟡 (0 misread); the serve path streams its hash and answers an unreadable file 500 with a log, cached deployments skip the per-href stat, tests for the pre-context error page and the size half of the memo

src/http/app.test.ts

@@ -451,6 +451,15 @@ test("every /public/ href the host renders is versioned and answers a year-long
   }
 });
 
+test("an error page rendered before the request has a context still versions its stylesheet", async () => {
+  const port = Number(new URL(base).port);
+  const res = await rawGet(port, "//", `localhost:${port}`); // `new URL("//", …)` throws before any context exists
+  assert.equal(res.status, 500);
+  const refs = publicRefs(res.body);
+  assert.ok(refs.length > 0);
+  for (const ref of refs) assert.match(ref, /\?v=[\w-]+$/, ref);
+});
+
 test("the standalone error and consent views version their stylesheets", async () => {
   for (const view of ["403", "404", "500", "503", "error", "oauth-consent"]) {
     const html = await ejs.renderFile(join(viewsDir, `${view}.ejs`), {

src/http/app.ts

@@ -42,7 +42,7 @@ const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..");
 export interface AppOptions {
   appUrl?: string; // canonical public URL (config.appUrl); off-host GET/HEAD visitors are 308'd here. Omitted ⇒ no redirect
   auth?: VerifyOptions; // expected JWT issuer/audience + clock skew (config); used with jwks
-  cache?: boolean; // cache compiled EJS templates (config.cacheTemplates); off ⇒ edits show live
+  cache?: boolean; // cache compiled EJS templates and /public/ asset versions (config.cacheTemplates); off ⇒ edits show live
   csrfSecret?: string; // HMAC key for the double-submit CSRF token (config.csrfSecret); random if omitted
   denylist?: Denylist; // optional instant-revoke; the hot path rejects revoked subjects, admin writes record revokes
   hydra?: HydraAdmin; // Hydra admin client; with kratos enables the OAuth2 login challenge
@@ -98,7 +98,7 @@ export function createApp(options: AppOptions = {}): Server {
   const anyResponseHooks = plugins.some((p) => p.hooks?.onResponse);
   const pluginsDir = options.pluginsDir ?? PLUGINS_DIR;
   const publicDir = options.publicDir ?? join(rootDir, "public");
-  const assets = publicAssets(publicDir, pluginsDir, pluginIds);
+  const assets = publicAssets(publicDir, pluginsDir, pluginIds, { cache });
   const viewsDir = options.viewsDir ?? join(rootDir, "views");
   // Response security headers, fixed at boot (only HSTS depends on the https deployment signal).
   const secHeaderEntries = Object.entries(securityHeaders({ secure: secureCookies }));
@@ -197,7 +197,7 @@ export function createApp(options: AppOptions = {}): Server {
 
       // Before auth: assets don't need a verified user, and the JWT cookie rides every request.
       if (pathname.startsWith("/public/") && (method === "GET" || method === "HEAD")) {
-        await assets.serve(url, req, res, (err) => reqLog.error("static stream error", { error: String(err) }));
+        await assets.serve(url, req, res, (err) => reqLog.error("static file error", { error: String(err) }));
         return;
       }
 

src/http/static.test.ts

@@ -1,5 +1,5 @@
 import assert from "node:assert/strict";
-import { mkdirSync, mkdtempSync, rmSync, statSync, writeFileSync } from "node:fs";
+import { mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from "node:fs";
 import { createServer } from "node:http";
 import type { AddressInfo } from "node:net";
 import { tmpdir } from "node:os";
@@ -17,11 +17,12 @@ mkdirSync(join(publicDir, "css"), { recursive: true });
 mkdirSync(join(pluginsDir, "demo", "public"), { recursive: true });
 writeFileSync(join(publicDir, "css", "a.css"), "body { color: red }");
 writeFileSync(join(pluginsDir, "demo", "public", "app.css"), ".demo {}");
-const assets = publicAssets(publicDir, pluginsDir, new Set(["demo"]));
+const assets = publicAssets(publicDir, pluginsDir, new Set(["demo"]), { cache: false });
 const assetHref = assets.href;
 
+const readErrors: Error[] = [];
 let base = "";
-const server = createServer((req, res) => void assets.serve(new URL(req.url ?? "/", "http://localhost"), req, res, (err) => assert.fail(err)));
+const server = createServer((req, res) => void assets.serve(new URL(req.url ?? "/", "http://localhost"), req, res, (err) => readErrors.push(err)));
 before(() => new Promise<void>((r) => server.listen(0, () => {
   base = `http://localhost:${(server.address() as AddressInfo).port}`;
   r();
@@ -68,6 +69,40 @@ test("assetHref changes when the file's content does", () => {
   assert.notEqual(assetHref("/public/css/changing.css"), first);
 });
 
+test("assetHref changes when the file's size does, even with its mtime restored", () => {
+  const file = join(publicDir, "css", "resized.css");
+  writeFileSync(file, "a {}");
+  const { atime, mtime } = statSync(file);
+  const first = assetHref("/public/css/resized.css");
+  writeFileSync(file, "a { color: blue }");
+  utimesSync(file, atime, mtime);
+  assert.notEqual(assetHref("/public/css/resized.css"), first);
+});
+
+test("with cache on, assetHref keeps a known version; a request naming it after an edit gets the short cache and the new ETag", async () => {
+  const cached = publicAssets(publicDir, pluginsDir, new Set(), { cache: true });
+  const file = join(publicDir, "css", "cached.css");
+  writeFileSync(file, "a {}");
+  const stale = cached.href("/public/css/cached.css");
+  writeFileSync(file, "a { color: green }");
+  assert.equal(cached.href("/public/css/cached.css"), stale);
+
+  const res = await fetch(base + stale);
+  assert.equal(res.status, 200);
+  assert.equal(await res.text(), "a { color: green }");
+  assert.equal(res.headers.get("cache-control"), SHORT);
+  const fresh = assetHref("/public/css/cached.css");
+  assert.notEqual(fresh, stale);
+  assert.equal(res.headers.get("etag"), `"${fresh.split("?v=")[1]}"`);
+});
+
+test("a version hashed while rendering equals the ETag hashed while serving the same bytes", async () => {
+  writeFileSync(join(publicDir, "css", "served-first.css"), "p {}");
+  writeFileSync(join(publicDir, "css", "rendered-first.css"), "p {}");
+  const etag = (await fetch(base + "/public/css/served-first.css")).headers.get("etag");
+  assert.equal(etag, `"${assetHref("/public/css/rendered-first.css").split("?v=")[1]}"`);
+});
+
 test("a URL carrying the current version is cached for a year as immutable", async () => {
   for (const method of ["GET", "HEAD"]) {
     const res = await fetch(base + assetHref("/public/css/a.css"), { method });
@@ -117,13 +152,21 @@ test("If-Modified-Since at or after the file's mtime answers 304; before it, or
   const earlier = new Date(Date.parse(lastModified) - 1000).toUTCString();
   assert.equal((await fetch(base + "/public/css/a.css", { headers: { "if-modified-since": earlier } })).status, 200);
   assert.equal((await fetch(base + "/public/css/a.css", { headers: { "if-modified-since": "not a date" } })).status, 200);
-  // RFC 9110 §13.2.2: If-None-Match, when present, decides alone.
   assert.equal((await fetch(base + "/public/css/a.css", { headers: { "if-modified-since": lastModified, "if-none-match": '"other"' } })).status, 200);
 });
 
-test("unsafe or missing paths still answer 400, 403 and 404", async () => {
+test("unsafe or missing paths answer 400, 403 and 404", async () => {
   assert.equal((await fetch(base + "/public/%E0%A4%A")).status, 400);
   assert.equal((await fetch(base + "/public/..%2f..%2fetc%2fpasswd")).status, 403);
   assert.equal((await fetch(base + "/public/css/missing.css")).status, 404);
+  assert.equal((await fetch(base + "/public/css/a.css/x")).status, 404); // ENOTDIR
   assert.equal((await fetch(base + "/public/css")).status, 404);
+  assert.deepEqual(readErrors, []);
+});
+
+test("a read failure other than a missing file answers 500 and reaches onError", async () => {
+  symlinkSync("loop.css", join(publicDir, "css", "loop.css")); // root reads a mode-000 file, so a symlink loop makes the failure
+  const res = await fetch(base + "/public/css/loop.css");
+  assert.equal(res.status, 500);
+  assert.deepEqual(readErrors.splice(0).map((err) => (err as NodeJS.ErrnoException).code), ["ELOOP"]);
 });

src/http/static.ts

@@ -1,4 +1,4 @@
-import { createHash } from "node:crypto";
+import { createHash, type Hash } from "node:crypto";
 import { createReadStream, readFileSync, statSync, type Stats } from "node:fs";
 import { stat } from "node:fs/promises";
 import type { IncomingMessage, ServerResponse } from "node:http";
@@ -63,24 +63,43 @@ const SHORT = "public, max-age=300";
 // gets a new hash.
 const hashes = new Map<string, { hash: string; mtimeMs: number; size: number }>();
 
-function contentHash(filePath: string, info: Stats): string {
+function memoized(filePath: string, info: Stats): string | undefined {
   const known = hashes.get(filePath);
-  if (known?.mtimeMs === info.mtimeMs && known.size === info.size) return known.hash;
-  const hash = createHash("sha256").update(readFileSync(filePath)).digest("base64url").slice(0, 16);
+  return known?.mtimeMs === info.mtimeMs && known.size === info.size ? known.hash : undefined;
+}
+
+// The one definition of a version. Two readers feed it. A view renders synchronously, so hashFileSync
+// reads the file whole; hashFile streams it, so a large file doesn't stall every other request.
+function remember(filePath: string, info: Stats, sha256: Hash): string {
+  const hash = sha256.digest("base64url").slice(0, 16);
   hashes.set(filePath, { hash, mtimeMs: info.mtimeMs, size: info.size });
   return hash;
 }
 
-// Synchronous because EJS templates are: a view calls it mid-render. The stat per href is the
-// price of picking up an edited file; the file is read and hashed once per version.
-function versioned(href: string, route: StaticRoute): string {
+function hashFileSync(filePath: string, info: Stats): string {
+  return memoized(filePath, info) ?? remember(filePath, info, createHash("sha256").update(readFileSync(filePath)));
+}
+
+async function hashFile(filePath: string, info: Stats): Promise<string> {
+  const known = memoized(filePath, info);
+  if (known !== undefined) return known;
+  const sha256 = createHash("sha256");
+  for await (const chunk of createReadStream(filePath)) sha256.update(chunk);
+  return remember(filePath, info, sha256);
+}
+
+// Without `cache`, a stat per href picks up an edited file. With it, a known version skips the stat:
+// an edit reaches the hrefs once a request for the file hashes it again.
+function versioned(href: string, route: StaticRoute, cache: boolean): string {
   try {
     const filePath = resolveStaticPath(route.dir, decodeURIComponent(route.subPath));
     if (filePath === null) return href;
+    const known = cache ? hashes.get(filePath) : undefined;
+    if (known) return `${href}?v=${known.hash}`;
     const info = statSync(filePath);
-    return info.isFile() ? `${href}?v=${contentHash(filePath, info)}` : href;
+    return info.isFile() ? `${href}?v=${hashFileSync(filePath, info)}` : href;
   } catch {
-    return href; // malformed %-encoding or a missing file: the request answers 400/404 on its own
+    return href; // malformed %-encoding, or a missing or unreadable file: serving it answers the error
   }
 }
 
@@ -122,12 +141,12 @@ async function serveStatic(route: StaticRequest, req: IncomingMessage, res: Serv
   try {
     const info = await stat(filePath);
     if (!info.isFile()) return plain(res, 404, "Not Found");
-    const hash = contentHash(filePath, info);
+    const hash = await hashFile(filePath, info);
     const etag = `"${hash}"`;
     const validators = { "cache-control": route.version === hash ? IMMUTABLE : SHORT, etag, "last-modified": info.mtime.toUTCString() };
     if (notModified(req, etag, info)) return void res.writeHead(304, validators).end();
     res.writeHead(200, { ...validators, "content-length": info.size, "content-type": contentTypeFor(filePath) });
-    if (req.method === "HEAD") return void res.end(); // headers only — skip opening the file
+    if (req.method === "HEAD") return void res.end();
     // Headers are already sent, so a mid-stream read error can't become an HTTP status —
     // log and destroy the response to signal a truncated body, not a hung socket.
     createReadStream(filePath)
@@ -136,21 +155,24 @@ async function serveStatic(route: StaticRequest, req: IncomingMessage, res: Serv
         res.destroy();
       })
       .pipe(res);
-  } catch {
-    plain(res, 404, "Not Found");
+  } catch (err) {
+    const { code } = err as NodeJS.ErrnoException;
+    if (code === "ENOENT" || code === "ENOTDIR") return plain(res, 404, "Not Found");
+    onError(err as Error);
+    plain(res, 500, "Internal Server Error");
   }
 }
 
 export interface PublicAssets {
   href: (href: string) => string; // a /public/ href with its version; any other href unchanged
-  // onError handles a mid-stream read failure (headers already sent).
+  // onError receives every failure to read a file except a missing one, which answers 404.
   serve: (url: URL, req: IncomingMessage, res: ServerResponse, onError: (err: Error) => void) => Promise<void>;
 }
 
-export function publicAssets(publicDir: string, pluginsDir: string, pluginIds: Set<string>): PublicAssets {
+export function publicAssets(publicDir: string, pluginsDir: string, pluginIds: Set<string>, { cache }: { cache: boolean }): PublicAssets {
   const route = (pathname: string): StaticRoute => routePublic(pathname.slice("/public/".length), publicDir, pluginsDir, pluginIds);
   return {
-    href: (href) => (href.startsWith("/public/") && !/[?#]/.test(href) ? versioned(href, route(href)) : href),
+    href: (href) => (href.startsWith("/public/") && !/[?#]/.test(href) ? versioned(href, route(href), cache) : href),
     serve: (url, req, res, onError) => serveStatic({ ...route(url.pathname), version: url.searchParams.get("v") }, req, res, onError),
   };
 }