From 46f59d5855d534b6716989eea422104e132474c9 Mon Sep 17 00:00:00 2001 From: Madraka <8898640+Madraka@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:14:26 +0300 Subject: [PATCH] fix(astro-renderer): vary site Worker responses by host Workers Cache keys by path, not hostname, and answers before the Worker runs. Every host routed to one site Worker shared one cache entry per path: a redirect host's 301 (#49) was served on the site's own domain (a loop on cloudhal.com's home page), and RFC-0034's multi-site Workers could serve one site's cached page on another site's host. - Every response from the site Worker carries `Vary: Host` (merged into an existing Vary), so the cache keeps one variant per host. - Redirect-host 301s are `cloudflare-cdn-cache-control: no-store`. - RFC-0034: corrected the "cache key contains the host" assumption. Verified on a test deployment with two custom domains: before, the 301 and the 200 leaked between hosts in both orders; after, each host gets its own answer, and the site's pages are still cached (HIT). Fixes #62 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Madraka <8898640+Madraka@users.noreply.github.com> --- .changeset/cache-vary-host.md | 6 ++++ .../0034-multi-site-serving-and-projects.md | 4 +-- packages/astro-renderer/src/cache-vary.ts | 20 ++++++++++++ .../astro-renderer/src/cloudflare-entry.ts | 31 +++++++++++-------- packages/astro-renderer/src/host-redirect.ts | 2 ++ .../astro-renderer/test/cache-vary.test.ts | 21 +++++++++++++ .../astro-renderer/test/host-redirect.test.ts | 2 ++ 7 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 .changeset/cache-vary-host.md create mode 100644 packages/astro-renderer/src/cache-vary.ts create mode 100644 packages/astro-renderer/test/cache-vary.test.ts diff --git a/.changeset/cache-vary-host.md b/.changeset/cache-vary-host.md new file mode 100644 index 00000000..7269c9f8 --- /dev/null +++ b/.changeset/cache-vary-host.md @@ -0,0 +1,6 @@ +--- +"@cloudhal/astro-renderer": patch +"@cloudhal/cli": patch +--- + +The site Worker's responses carry `Vary: Host`, and redirect-host 301s never enter the edge cache (#62). Workers Cache keys by path, not hostname, so before this every host routed to one site Worker shared a cache entry per path: a `--redirect` host's 301 could be served on the site's own domain (a redirect loop), and the sites of a multi-site Worker could serve each other's cached pages. diff --git a/docs/rfcs/0034-multi-site-serving-and-projects.md b/docs/rfcs/0034-multi-site-serving-and-projects.md index 148e3ba3..786f5dfc 100644 --- a/docs/rfcs/0034-multi-site-serving-and-projects.md +++ b/docs/rfcs/0034-multi-site-serving-and-projects.md @@ -86,7 +86,7 @@ A site in a multi-site installation is as independent as a site in its own insta 3. An unknown host, or one whose site is not in the allow-list, gets a plain 404, cached for 60 s. No other site's content is ever served for it. 4. The Worker's `*.workers.dev` hostname serves the first site in `sites`, marked `noindex`, so a fresh deploy has something to check. - **Per-site state.** `getResolver(siteId)`, media, search, feeds and the analytics and comments settings use the resolved id. The module-level caches become maps keyed by `siteId`. Same-origin checks (RFC-0006 §5.2, RFC-0007 §6.2) read the site's host rows. `CLOUDHAL_SITE_HOSTS` keeps working in single-site mode. -- **Cache.** The edge cache key already contains the host. Every response, 404s included, carries `cloudhal-site-`. In multi-site mode, purges use tags only, never `purgeEverything`, so one site's publish never empties another site's cache. +- **Cache.** Workers Cache keys by path, not host (correction, #62: this RFC first said the key already contained the host). Every site Worker response carries `Vary: Host`, so each host keeps its own cached variant. Every response, 404s included, carries `cloudhal-site-`. In multi-site mode, purges use tags only, never `purgeEverything`, so one site's publish never empties another site's cache. - **Locale domains** (RFC-0033 §4) are host rows with `locale` set; the host decides the locale before the path, as today. ### 5. Domains @@ -174,7 +174,7 @@ What one installation can hold, from Cloudflare's published limits (Workers and - **Rollback:** switch `cloudhal.json` back to `siteId` and redeploy. The tables are ignored in single-site mode. ## Security & policy impact -- **Isolation within an installation:** invariants 1–4. The cache key includes the host, and purge tags are per site. +- **Isolation within an installation:** invariants 1–4. Cached responses vary by host (`Vary: Host`, #62), and purge tags are per site. - **New scopes:** - `site:create`: installation Owner and admins; also used for editions. - `site:delete`: Owner only, typed confirmation, audited. diff --git a/packages/astro-renderer/src/cache-vary.ts b/packages/astro-renderer/src/cache-vary.ts new file mode 100644 index 00000000..4a7887a0 --- /dev/null +++ b/packages/astro-renderer/src/cache-vary.ts @@ -0,0 +1,20 @@ +/** + * Workers Cache keys by path, not hostname, and answers before the Worker runs (#62). Every host + * routed to the site Worker — a custom domain, its workers.dev name, a redirect host (#49), the + * other sites of a multi-site Worker (RFC-0034) — would otherwise share one entry per path. `Vary: + * Host` makes it keep one variant per host. + * + * Worker-safe: no imports. + */ +export function varyByHost(response: Response): Response { + const vary = response.headers.get("vary") ?? ""; + if (vary.trim() === "*" || /(^|,)\s*host\s*(,|$)/i.test(vary)) return response; + // A redirect's or a WebSocket's headers can be immutable; copy them into a fresh response. + const headers = new Headers(response.headers); + headers.set("vary", vary.trim() ? `${vary}, Host` : "Host"); + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + }); +} diff --git a/packages/astro-renderer/src/cloudflare-entry.ts b/packages/astro-renderer/src/cloudflare-entry.ts index 94c351d1..023c74f1 100644 --- a/packages/astro-renderer/src/cloudflare-entry.ts +++ b/packages/astro-renderer/src/cloudflare-entry.ts @@ -54,6 +54,7 @@ import { SiteAnalytics, } from "./analytics.ts"; import { isCloudhalCacheTag } from "./cache-tags.ts"; +import { varyByHost } from "./cache-vary.ts"; import { type CommentsRateLimit, isCommentsPath, SiteComments } from "./comments.ts"; import { SITE_CSP } from "./csp.ts"; import { EMBED_HEADER, withEmbedFrames } from "./embed-client.ts"; @@ -529,21 +530,25 @@ async function serveSite( export function createSiteWorker(options: { siteId?: string } = {}) { const searchLimiter = new SearchLimiter(); + async function route(request: Request, env: Env, ctx: ExecutionContext): Promise { + const url = new URL(request.url); + if (url.pathname === "/__cloudhal/purge") return purge(request, env, options.siteId); + if (url.pathname === "/__cloudhal/features") return features(request, env); + const moved = hostRedirect(request, env); + if (moved) return moved; + // RFC-0034 §4: the hostname picks the site on a multi-site Worker; everything the request + // runs (Astro's render included) then sees that site. + const choice = await siteDispatch(env, options.siteId).choose(request); + if (choice.kind === "unknown") return unknownHost(); + const siteId = choice.siteId; + const run = () => serveSite({ request, env, ctx }, siteId, searchLimiter); + const response = siteId ? await withRequestSite(siteId, run) : await run(); + return choice.noindex ? withNoindex(response) : response; + } return { async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise { - const url = new URL(request.url); - if (url.pathname === "/__cloudhal/purge") return purge(request, env, options.siteId); - if (url.pathname === "/__cloudhal/features") return features(request, env); - const moved = hostRedirect(request, env); - if (moved) return moved; - // RFC-0034 §4: the hostname picks the site on a multi-site Worker; everything the request - // runs (Astro's render included) then sees that site. - const choice = await siteDispatch(env, options.siteId).choose(request); - if (choice.kind === "unknown") return unknownHost(); - const siteId = choice.siteId; - const run = () => serveSite({ request, env, ctx }, siteId, searchLimiter); - const response = siteId ? await withRequestSite(siteId, run) : await run(); - return choice.noindex ? withNoindex(response) : response; + // #62: the edge cache is shared by every host this Worker answers on, unless told to vary. + return varyByHost(await route(request, env, ctx)); }, }; } diff --git a/packages/astro-renderer/src/host-redirect.ts b/packages/astro-renderer/src/host-redirect.ts index c8383c8c..600ced9a 100644 --- a/packages/astro-renderer/src/host-redirect.ts +++ b/packages/astro-renderer/src/host-redirect.ts @@ -27,6 +27,8 @@ export function hostRedirect(request: Request, env: RedirectEnv): Response | und headers: { location: `https://${to}${url.pathname}${url.search}`, "cache-control": "public, max-age=86400", + // Never in the edge cache (#62): it is shared with the hosts this one redirects to. + "cloudflare-cdn-cache-control": "no-store", }, }); } diff --git a/packages/astro-renderer/test/cache-vary.test.ts b/packages/astro-renderer/test/cache-vary.test.ts new file mode 100644 index 00000000..05206483 --- /dev/null +++ b/packages/astro-renderer/test/cache-vary.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from "vitest"; +import { varyByHost } from "../src/cache-vary.ts"; + +describe("varyByHost (#62: the edge cache is keyed by path, not host)", () => { + it("adds Host to Vary, keeping what is there", () => { + expect(varyByHost(new Response("x")).headers.get("vary")).toBe("Host"); + const r = varyByHost(new Response("x", { headers: { vary: "Accept-Encoding" } })); + expect(r.headers.get("vary")).toBe("Accept-Encoding, Host"); + }); + + it("leaves Vary: * and an existing Host alone, and keeps status and body", async () => { + expect(varyByHost(new Response("x", { headers: { vary: "*" } })).headers.get("vary")).toBe("*"); + expect( + varyByHost(new Response("x", { headers: { vary: "accept, host" } })).headers.get("vary"), + ).toBe("accept, host"); + const moved = varyByHost(Response.redirect("https://example.com/a", 301)); + expect(moved.status).toBe(301); + expect(moved.headers.get("location")).toBe("https://example.com/a"); + expect(await varyByHost(new Response("body", { status: 404 })).text()).toBe("body"); + }); +}); diff --git a/packages/astro-renderer/test/host-redirect.test.ts b/packages/astro-renderer/test/host-redirect.test.ts index cabb586d..35ede777 100644 --- a/packages/astro-renderer/test/host-redirect.test.ts +++ b/packages/astro-renderer/test/host-redirect.test.ts @@ -11,6 +11,8 @@ describe("redirect-only hosts (#49)", () => { const r = hostRedirect(new Request("https://WWW.example.com/blog/post?ref=x"), env); expect(r?.status).toBe(301); expect(r?.headers.get("location")).toBe("https://example.com/blog/post?ref=x"); + // #62: never in the edge cache, which every host of the Worker shares + expect(r?.headers.get("cloudflare-cdn-cache-control")).toBe("no-store"); expect(hostRedirect(new Request("http://old.example.org/"), env)?.headers.get("location")).toBe( "https://example.com/", );