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();