From 19ad24d70cc2138f003461452ebe72c80fa72ae1 Mon Sep 17 00:00:00 2001 From: Leo Kettmeir Date: Thu, 27 Aug 2026 14:14:56 +0000 Subject: [PATCH] fix: make the lb's bucket cache purgeable on publish (#1541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1455. ### The bug The lb keys R2-backed responses under a namespace of their own, because `/@scope/...` is served as *either* a module file (bucket) or an HTML page (frontend) depending on request headers, and the two must not cross-serve. That namespace was a synthetic host: `https://bucket-cache.jsr.internal/`. Cloudflare's purge-by-URL API only accepts URLs inside the zone, so every publish-time purge of `https://jsr.io/@scope/pkg/meta.json` missed the key the manifest was actually cached under. The result is exactly what was reported: R2 had the regenerated `meta.json`, a cache-busting query string returned the correct `latest`, but the edge kept serving the previous copy until its TTL lapsed. `_meta.json` and the version page were live the whole time, while Deno's resolver — which reads `meta.json` — could not see the new version, and further publishes did not dislodge it. The npm version manifest on `npm.jsr.io` had the same gap. (#1456 capped how *long* that lasts by dropping SWR and shortening `s-maxage`; this makes the purge itself land, so a publish is visible immediately rather than after the TTL.) ### The fix Namespace bucket entries under a reserved path on the **public** origin — `https://jsr.io/__bucket-cache/@scope/pkg/meta.json` — instead of a synthetic host. The host is still part of the key, so the module and npm buckets stay distinct, and the purge now has a URL inside the zone that it can target. `route` rejects inbound requests for the reserved prefix before any backend, cache read, or cache write, so the namespace cannot be reached — or poisoned — from outside. The API mirrors the prefix (`s3_paths::BUCKET_CACHE_PREFIX`, kept in sync with `lb/proxy.ts`) and purges both the public and the namespaced form of each mutable manifest. The public URL still matters there: a browser navigation to the same URL is served, and cached, by the frontend. Changing the key also orphans the current entries, so deploying this clears any manifest that is stuck right now. ### Tests - `s3_paths`: both purge helpers return the public URL *and* the namespaced key. - `lb/proxy_test.ts`: bucket cache keys live on the public origin under the prefix, and the module vs. npm buckets stay keyed apart. - `lb/main_test.ts`: `route` 404s the reserved namespace on all three hosts without touching a backend, while a merely similar-looking path routes normally. `cargo clippy --all-targets` clean; `deno test` in `lb/` passes (43 tests). --- api/src/api/package.rs | 16 +++---- api/src/external/cloudflare.rs | 13 +++-- api/src/publish.rs | 8 ++-- api/src/s3_paths.rs | 87 +++++++++++++++++++++++++++++----- api/src/tasks.rs | 4 +- lb/main.ts | 20 +++++++- lb/main_test.ts | 47 +++++++++++++++++- lb/proxy.ts | 40 ++++++++++++---- lb/proxy_test.ts | 36 +++++++++++++- 9 files changed, 229 insertions(+), 42 deletions(-) diff --git a/api/src/api/package.rs b/api/src/api/package.rs index a4209841d..edffe9154 100644 --- a/api/src/api/package.rs +++ b/api/src/api/package.rs @@ -709,11 +709,11 @@ async fn update_description( .await?; cache_purge - .purge(vec![crate::s3_paths::npm_version_manifest_url( + .purge(crate::s3_paths::npm_version_manifest_purge_urls( npm_url, scope, &package.name, - )]) + )) .await; Ok(package) @@ -824,12 +824,12 @@ async fn upload_package_manifests( ) .await?; - cache_purge - .purge(vec![ - crate::s3_paths::package_metadata_url(registry_url, scope, package), - crate::s3_paths::npm_version_manifest_url(npm_url, scope, package), - ]) - .await; + let mut purge_urls = + crate::s3_paths::package_metadata_purge_urls(registry_url, scope, package); + purge_urls.extend(crate::s3_paths::npm_version_manifest_purge_urls( + npm_url, scope, package, + )); + cache_purge.purge(purge_urls).await; Ok(()) } diff --git a/api/src/external/cloudflare.rs b/api/src/external/cloudflare.rs index 7a3b89ad3..4a31f10c9 100644 --- a/api/src/external/cloudflare.rs +++ b/api/src/external/cloudflare.rs @@ -140,8 +140,13 @@ pub struct CachePurge(pub Option); impl CachePurge { /// Purge `urls` if a client is configured. Errors are logged inside /// `purge_urls` and converted into `Ok(())` here, since callers want - /// best-effort behaviour (the manifests have `stale-while-revalidate` - /// as their durability net). + /// best-effort behaviour (the manifests' short `s-maxage` is their + /// durability net — see `CACHE_CONTROL_MANIFEST`). + /// + /// Note that a bucket-backed object is not cached under its public URL: + /// build its URLs with `s3_paths::package_metadata_purge_urls` / + /// `npm_version_manifest_purge_urls`, which also cover the lb's namespaced + /// cache key. Purging the public URL alone is a no-op for those. pub async fn purge(&self, urls: Vec) { let Some(client) = &self.0 else { return; @@ -158,8 +163,8 @@ impl CachePurgeClient { /// Purge a set of fully-qualified URLs from the Cloudflare zone cache. /// /// Errors are logged and returned — callers should treat purge as - /// best-effort and not fail the publish on a purge failure (the - /// `stale-while-revalidate` window on the manifests is the safety net). + /// best-effort and not fail the publish on a purge failure (the short + /// `s-maxage` on the manifests is the safety net). #[instrument(name = "cloudflare.purge_cache", skip(self, urls), err)] pub async fn purge_urls( &self, diff --git a/api/src/publish.rs b/api/src/publish.rs index 33220576d..1b7c02f22 100644 --- a/api/src/publish.rs +++ b/api/src/publish.rs @@ -497,11 +497,11 @@ async fn upload_package_manifest( ) .await?; - let mut purge_urls = vec![crate::s3_paths::package_metadata_url( + let mut purge_urls = crate::s3_paths::package_metadata_purge_urls( registry_url, &publishing_task.package_scope, &publishing_task.package_name, - )]; + ); purge_urls.extend(crate::s3_paths::package_api_cache_urls( registry_url, &publishing_task.package_scope, @@ -546,11 +546,11 @@ async fn upload_npm_version_manifest( .await?; cache_purge - .purge(vec![crate::s3_paths::npm_version_manifest_url( + .purge(crate::s3_paths::npm_version_manifest_purge_urls( npm_url, &publishing_task.package_scope, &publishing_task.package_name, - )]) + )) .await; Ok(()) diff --git a/api/src/s3_paths.rs b/api/src/s3_paths.rs index 2137a2ad8..01e3876bb 100644 --- a/api/src/s3_paths.rs +++ b/api/src/s3_paths.rs @@ -70,30 +70,56 @@ pub fn npm_version_manifest_path( format!("{npm_mapped_package_name}") } -/// Public URL of the package-level `meta.json` that the registry serves -/// to `deno install` / browser module resolution. Pass `registry_url` -/// as `https://jsr.io/` (must end with a slash). -pub fn package_metadata_url( +/// Path prefix the lb Worker namespaces its bucket (R2) cache entries under, so +/// that a `/@scope/...` URL cached as a module file cannot collide with the same +/// URL cached as an HTML page. Must stay in sync with `BUCKET_CACHE_PREFIX` in +/// `lb/proxy.ts`. +/// +/// Bucket-backed responses are therefore *never* cached under their public URL, +/// and purging only the public URL leaves the served copy in place — which is +/// how a freshly regenerated `meta.json` stayed invisible to Deno's resolver +/// until its edge TTL lapsed. Every purge of a bucket-backed manifest must +/// cover both forms; `bucket_backed_purge_urls` builds the pair. +const BUCKET_CACHE_PREFIX: &str = "__bucket-cache"; + +/// The URLs a bucket-backed object at `path` (relative to `base`, which must end +/// with a slash) is cached under: the public URL, plus the lb's namespaced +/// bucket cache key. Both are purged, since a browser navigation to the same +/// public URL is served — and cached — by the frontend instead. +fn bucket_backed_purge_urls(base: &url::Url, path: &str) -> Vec { + vec![ + format!("{base}{path}"), + format!("{base}{BUCKET_CACHE_PREFIX}/{path}"), + ] +} + +/// Cache URLs of the package-level `meta.json` that the registry serves to +/// `deno install` / browser module resolution, to purge when it is regenerated. +/// Pass `registry_url` as `https://jsr.io/` (must end with a slash). +pub fn package_metadata_purge_urls( registry_url: &url::Url, scope: &ScopeName, package_name: &PackageName, -) -> String { - format!("{registry_url}@{scope}/{package_name}/meta.json") +) -> Vec { + bucket_backed_purge_urls( + registry_url, + &format!("@{scope}/{package_name}/meta.json"), + ) } -/// Public URL of the npm version manifest the registry serves to -/// `npm install` / `pnpm install` / etc. Pass `npm_url` as -/// `https://npm.jsr.io/` (must end with a slash). -pub fn npm_version_manifest_url( +/// Cache URLs of the npm version manifest the registry serves to +/// `npm install` / `pnpm install` / etc., to purge when it is regenerated. Pass +/// `npm_url` as `https://npm.jsr.io/` (must end with a slash). +pub fn npm_version_manifest_purge_urls( npm_url: &url::Url, scope: &ScopeName, package_name: &PackageName, -) -> String { +) -> Vec { let npm_mapped_package_name = NpmMappedJsrPackageName { scope, package: package_name, }; - format!("{npm_url}{npm_mapped_package_name}") + bucket_backed_purge_urls(npm_url, &format!("{npm_mapped_package_name}")) } /// Base URL of the public API host (`https://api.jsr.io/`), derived from the @@ -197,6 +223,43 @@ mod tests { assert!(urls.contains(&"https://api.jsr.io/api/scopes/std".into())); } + // A bucket-backed manifest is cached by the lb under its namespaced key, not + // its public URL, so the purge must cover both — purging only the public URL + // leaves the copy the resolver actually reads in place (jsr-io/jsr#1455). + #[test] + fn package_metadata_purge_covers_the_bucket_cache_key() { + let registry_url = url::Url::parse("https://jsr.io/").unwrap(); + let scope = ScopeName::try_from("std").unwrap(); + let package = PackageName::try_from("fs").unwrap(); + let urls = + super::package_metadata_purge_urls(®istry_url, &scope, &package); + + assert_eq!( + urls, + vec![ + "https://jsr.io/@std/fs/meta.json".to_string(), + "https://jsr.io/__bucket-cache/@std/fs/meta.json".to_string(), + ] + ); + } + + #[test] + fn npm_version_manifest_purge_covers_the_bucket_cache_key() { + let npm_url = url::Url::parse("https://npm.jsr.io/").unwrap(); + let scope = ScopeName::try_from("std").unwrap(); + let package = PackageName::try_from("yaml").unwrap(); + let urls = + super::npm_version_manifest_purge_urls(&npm_url, &scope, &package); + + assert_eq!( + urls, + vec![ + "https://npm.jsr.io/@jsr/std__yaml".to_string(), + "https://npm.jsr.io/__bucket-cache/@jsr/std__yaml".to_string(), + ] + ); + } + #[test] fn version_metadata_is_correct() { let crazy = "= v 1.2.3-pre.other+build.test"; diff --git a/api/src/tasks.rs b/api/src/tasks.rs index 33eff5d87..05e8e4fb7 100644 --- a/api/src/tasks.rs +++ b/api/src/tasks.rs @@ -293,9 +293,9 @@ pub async fn npm_tarball_build_handler( .await?; cache_purge - .purge(vec![crate::s3_paths::npm_version_manifest_url( + .purge(crate::s3_paths::npm_version_manifest_purge_urls( &npm_url, &job.scope, &job.name, - )]) + )) .await; Ok(()) diff --git a/lb/main.ts b/lb/main.ts index c37bc66a0..8881eda3b 100644 --- a/lb/main.ts +++ b/lb/main.ts @@ -1,7 +1,12 @@ // Copyright 2024 the JSR authors. All rights reserved. MIT license. import type { WorkerEnv } from "./types.ts"; -import { type ExecutionCtx, proxyToBackend, proxyToR2 } from "./proxy.ts"; +import { + type ExecutionCtx, + isBucketCachePath, + proxyToBackend, + proxyToR2, +} from "./proxy.ts"; import { handleCORSPreflight, isCORSPreflight, @@ -48,6 +53,19 @@ export async function route( const url = new URL(request.url); const hostname = url.hostname.toLowerCase(); + // Reserved namespace the lb keys its own bucket cache entries under (see + // BUCKET_CACHE_PREFIX). Nothing is served from here; rejecting it up front — + // before any backend, and before any cache read or write — keeps a crafted + // request from planting a response under, or reading, a bucket cache key. + if (isBucketCachePath(url.pathname)) { + return new Response("404 - Not Found", { + status: 404, + headers: { + "Content-Type": "text/plain", + }, + }); + } + if (hostname === env.API_DOMAIN) { return await handleAPIRequest(request, env, true, ctx); } else if (hostname === env.NPM_DOMAIN) { diff --git a/lb/main_test.ts b/lb/main_test.ts index a469f9fa3..592c237c6 100644 --- a/lb/main_test.ts +++ b/lb/main_test.ts @@ -1,7 +1,8 @@ // Copyright 2024 the JSR authors. All rights reserved. MIT license. import { assertEquals } from "@std/assert"; -import { isDocsDiffSourceRoute } from "./main.ts"; +import { isDocsDiffSourceRoute, route } from "./main.ts"; +import type { PartialBucket, WorkerEnv } from "./types.ts"; Deno.test("isDocsDiffSourceRoute matches doc pages", () => { for ( @@ -62,3 +63,47 @@ Deno.test("isDocsDiffSourceRoute ignores other routes", () => { assertEquals(isDocsDiffSourceRoute(path), false, path); } }); + +// The lb keys its bucket cache entries under a reserved path prefix on the +// public origin (see BUCKET_CACHE_PREFIX). Nothing is served from there, and +// `route` must reject such requests before reaching a backend — otherwise a +// crafted request could plant a frontend response under a bucket cache key. +Deno.test("route rejects the reserved bucket-cache namespace", async () => { + const unreachable = (what: string) => () => { + throw new Error(`${what} must not be consulted`); + }; + const bucket = { + get: unreachable("bucket"), + head: unreachable("bucket"), + } as unknown as PartialBucket; + const env: WorkerEnv = { + REGISTRY_API_URL: "https://api.invalid/", + FRONTEND: { fetch: unreachable("frontend") } as unknown as Fetcher, + ROOT_DOMAIN: "jsr.io", + API_DOMAIN: "api.jsr.io", + NPM_DOMAIN: "npm.jsr.io", + NPM_BUCKET: bucket, + MODULES_BUCKET: bucket, + }; + + for ( + const url of [ + "https://jsr.io/__bucket-cache", + "https://jsr.io/__bucket-cache/@scope/pkg/meta.json", + "https://npm.jsr.io/__bucket-cache/@jsr/scope__pkg", + "https://api.jsr.io/__bucket-cache/api/scopes/scope", + ] + ) { + const res = await route(new Request(url), env); + assertEquals(res.status, 404, url); + } + + // A path that merely starts with the same characters is a normal route: it + // still reaches the frontend, which this env makes throw — surfacing as the + // proxy's 502 rather than the 404 above. + const res = await route( + new Request("https://jsr.io/__bucket-cache-not-really"), + env, + ); + assertEquals(res.status, 502); +}); diff --git a/lb/proxy.ts b/lb/proxy.ts index 262461286..7d2dc4af8 100644 --- a/lb/proxy.ts +++ b/lb/proxy.ts @@ -39,17 +39,41 @@ async function persistCacheWrite( } } -// Cache key for a bucket (R2) response. `caches.default` is shared across all -// backends, and a `/@scope/...` URL is served as EITHER a module file (bucket, -// JSON) or an HTML page (frontend) depending on request headers — keying both -// on the raw URL cross-serves HTML for module files (and vice versa). Bucket -// entries are namespaced under a synthetic, non-routable host (which no real -// request can ever target, so it can't be poisoned) keyed by the original host -// + path so module and npm buckets also stay distinct. +// Path prefix that namespaces bucket (R2) cache entries. `caches.default` is +// shared across all backends, and a `/@scope/...` URL is served as EITHER a +// module file (bucket, JSON) or an HTML page (frontend) depending on request +// headers — keying both on the raw URL cross-serves HTML for module files (and +// vice versa), so bucket entries need a namespace of their own. +// +// That namespace is a reserved path on the PUBLIC origin, not a synthetic host. +// Cloudflare's purge-by-URL API only accepts URLs inside the zone, so entries +// keyed under an out-of-zone host — as they were, under +// `bucket-cache.jsr.internal` — were unreachable by every publish-time purge: +// a regenerated `meta.json` sat in R2 while the edge kept serving the previous +// one until its TTL lapsed, leaving a just-published version invisible to +// Deno's resolver even though its version page and `_meta.json` were already +// live. Keying on the public origin keeps the module and npm buckets distinct +// (the host is part of the key) while giving the purge a URL it can target. +// +// The API mirrors this prefix in `s3_paths::BUCKET_CACHE_PREFIX` and purges +// both the public and the namespaced form of every mutable manifest; the two +// constants must stay in sync. +export const BUCKET_CACHE_PREFIX = "__bucket-cache"; + +// True for the reserved bucket-cache namespace. No real resource lives there, +// and `route` rejects such requests before any backend or cache is consulted, +// so a crafted request can neither read nor populate a bucket cache entry. +export function isBucketCachePath(path: string): boolean { + return path === `/${BUCKET_CACHE_PREFIX}` || + path.startsWith(`/${BUCKET_CACHE_PREFIX}/`); +} + +// Cache key for a bucket (R2) response: the public URL, moved under the +// reserved namespace above. function bucketCacheKey(rawUrl: string): Request { const u = new URL(rawUrl); return new Request( - `https://bucket-cache.jsr.internal/${u.host}${u.pathname}${u.search}`, + `${u.origin}/${BUCKET_CACHE_PREFIX}${u.pathname}${u.search}`, { method: "GET" }, ); } diff --git a/lb/proxy_test.ts b/lb/proxy_test.ts index a815f88bc..00e1761e6 100644 --- a/lb/proxy_test.ts +++ b/lb/proxy_test.ts @@ -179,6 +179,8 @@ Deno.test("proxyToR2 cache hit returns a fresh, mutable response", async () => { Deno.test("proxyToR2 namespaces cache keys away from the raw URL", async () => { // The frontend caches `/@scope/...` navigations under the raw URL; bucket // responses for the same URL must use a distinct key to avoid cross-serving. + // The key stays on the public origin so the publish-time Cloudflare purge — + // which only accepts in-zone URLs — can actually evict it (jsr-io/jsr#1455). const matchKeys: string[] = []; const putKeys: string[] = []; (globalThis as any).caches = { @@ -206,7 +208,7 @@ Deno.test("proxyToR2 namespaces cache keys away from the raw URL", async () => { assertEquals(res.status, 200); assertEquals( matchKeys[0], - "https://bucket-cache.jsr.internal/npm.jsr.io/@jsr/std__yaml", + "https://npm.jsr.io/__bucket-cache/@jsr/std__yaml", ); assertEquals(putKeys[0], matchKeys[0]); // match and put use the same key } finally { @@ -214,6 +216,36 @@ Deno.test("proxyToR2 namespaces cache keys away from the raw URL", async () => { } }); +Deno.test("proxyToR2 keys the module bucket per host", async () => { + // The bucket cache key is namespaced by path, so the host must still keep the + // module and npm buckets apart — `/@jsr/std__yaml` is a valid path on both. + const matchKeys: string[] = []; + (globalThis as any).caches = { + default: { + match: (req: Request) => { + matchKeys.push(req.url); + return Promise.resolve(undefined); + }, + put: () => Promise.resolve(), + }, + }; + + try { + const bucket = createFakeBucket({ + "@jsr/std__yaml": { body: "{}", contentType: "application/json" }, + }); + await proxyToR2(new Request("https://jsr.io/@jsr/std__yaml"), bucket); + await proxyToR2(new Request("https://npm.jsr.io/@jsr/std__yaml"), bucket); + + assertEquals(matchKeys, [ + "https://jsr.io/__bucket-cache/@jsr/std__yaml", + "https://npm.jsr.io/__bucket-cache/@jsr/std__yaml", + ]); + } finally { + (globalThis as any).caches = { default: undefined }; + } +}); + // --- proxyToR2 fallback registry tests --- /** @@ -528,7 +560,7 @@ Deno.test("proxyToR2 caches a fallback response the fallback marked cacheable", assertEquals(res.status, 200); assertEquals(putKeys, [ - "https://bucket-cache.jsr.internal/jsr.io/@std/yaml/meta.json", + "https://jsr.io/__bucket-cache/@std/yaml/meta.json", ]); } finally { restore();