fix(api): guard request-derived KV/R2/DO keys against platform length limits

runner/pipeline/o11y-symbolicate.test.mjs

@@ -120,6 +120,29 @@ test("symbolicateResourceLogs leaves a frame unresolved when no map exists (neve
   assert.equal(resolved.scopeLogs[0].logRecords[0].body.stringValue, record.scopeLogs[0].logRecords[0].body.stringValue);
 });
 
+test("symbolicateResourceLogs never asks for a map whose R2 key would exceed 1024 bytes", async () => {
+  // The frame path is browser-supplied; R2 throws on an overlong key, which would surface as a fetch_error.
+  const frameLine = formatStackFrame({
+    filename: `https://demos.handsontable.com/assets/${"a".repeat(1100)}.js`,
+    function: "fn",
+    lineno: 1,
+    colno: 1,
+  });
+  const record = exceptionRecord(["Error: x", frameLine]);
+  const asked = [];
+
+  const [resolved] = await symbolicateResourceLogs([record], {
+    getMap: async (key) => {
+      asked.push(key);
+      if (Buffer.byteLength(key, "utf8") > 1024) throw new Error("The specified object name is not valid. (10020)");
+      return null;
+    },
+  });
+
+  assert.deepEqual(asked, []);
+  assert.equal(resolved.scopeLogs[0].logRecords[0].body.stringValue, record.scopeLogs[0].logRecords[0].body.stringValue);
+});
+
 test("symbolicateResourceLogs never touches a non-exception record", async () => {
   const record = {
     resource: {

runner/pipeline/storage-key-guards.test.mjs

@@ -0,0 +1,234 @@
+// Request-derived KV keys, R2 keys and DO names that would exceed a platform
+// limit must be answered with a clean 4xx (or a miss) before any storage
+// access. The KV fake here throws on a key over 512 bytes the way real Workers
+// KV does, so a route that forgets the guard fails these specs instead of
+// passing on a permissive Map.
+//
+// Run: node --experimental-strip-types --test pipeline/storage-key-guards.test.mjs
+
+import test from "node:test";
+import assert from "node:assert/strict";
+import { register } from "node:module";
+
+import { ctx, demoRow, fakeKV, makeEnv } from "./fixtures/worker-harness.mjs";
+import { setSandboxFactory } from "./fixtures/cloudflare-sandbox-stub.mjs";
+
+register("./fixtures/worker-hooks.mjs", import.meta.url);
+
+const { default: worker } = await import("../workers/api/src/index.ts");
+const { captures } = await import("./fixtures/sentry-cloudflare-stub.mjs");
+const { kvKeyFits, r2KeyFits } = await import("../workers/api/src/storage-key.ts");
+
+const bytes = (s) => Buffer.byteLength(s, "utf8");
+
+/** fakeKV that rejects like the real thing and records every rejected or accepted key. */
+function strictKV() {
+  const inner = fakeKV();
+  const accessed = [];
+  const rejected = [];
+  const guard = (op, key) => {
+    accessed.push(`${op} ${key}`);
+    const n = bytes(key);
+    if (n === 0 || n > 512) {
+      rejected.push(key);
+      throw new Error(`KV ${op} failed: 414 UTF-8 encoded length of ${n} exceeds key length limit of 512.`);
+    }
+  };
+  return {
+    accessed,
+    rejected,
+    inner,
+    async get(key, type) { guard("GET", key); return inner.get(key, type); },
+    async put(key, value, options) { guard("PUT", key); return inner.put(key, value, options); },
+    async delete(key) { guard("DELETE", key); return inner.delete(key); },
+    async list(options) { return inner.list(options); },
+  };
+}
+
+/** R2 fake that throws on an object key over 1024 bytes, like real R2. */
+function strictR2(r2) {
+  const rejected = [];
+  const guard = (key) => {
+    if (bytes(key) > 1024) {
+      rejected.push(key);
+      throw new Error(`R2 get failed: The specified object name is not valid. (10020)`);
+    }
+  };
+  return {
+    rejected,
+    puts: r2.puts,
+    async get(key) { guard(key); return r2.get(key); },
+    async put(key, value, options) { guard(key); return r2.put(key, value, options); },
+    delete: (key) => r2.delete(key),
+    list: (options) => r2.list(options),
+  };
+}
+
+function setup(seedRows = [], seedArtifacts = {}) {
+  const made = makeEnv(seedRows, [], seedArtifacts);
+  const kv = strictKV();
+  made.env.CACHE = kv;
+  const r2 = strictR2(made.env.ARTIFACTS);
+  made.env.ARTIFACTS = r2;
+  return { ...made, kv, r2 };
+}
+
+const call = (env, method, path, init) =>
+  worker.fetch(new Request(`https://demos.handsontable.com${path}`, { method, ...init }), env, ctx);
+
+// ---- unit: bytes, not characters ------------------------------------------
+
+test("key helpers measure UTF-8 bytes, not UTF-16 units", () => {
+  const multibyte = "é".repeat(300); // 300 chars, 600 bytes
+  assert.ok(multibyte.length < 512 && bytes(multibyte) > 512);
+  assert.equal(kvKeyFits(multibyte), false);
+  assert.equal(kvKeyFits("a".repeat(512)), true);
+  assert.equal(kvKeyFits("a".repeat(513)), false);
+  assert.equal(kvKeyFits(""), false);
+  assert.equal(r2KeyFits("a".repeat(1024)), true);
+  assert.equal(r2KeyFits("a".repeat(1025)), false);
+});
+
+// ---- GET /api/versions/exists ----------------------------------------------
+
+test("versions/exists: an overlong v is a 400, with no KV access, no throw and no Sentry capture", async () => {
+  const { env, kv } = setup();
+  const before = captures.length;
+  const res = await call(env, "GET", `/api/versions/exists?v=${"1".repeat(600)}`);
+  assert.equal(res.status, 400);
+  assert.match((await res.json()).error, /valid version/);
+  assert.deepEqual(kv.accessed, [], "the guard must run before any KV access");
+  assert.equal(captures.length, before, "an anonymous malformed request must not reach Sentry");
+});
+
+test("versions/exists: a multibyte v under 512 characters but over 512 bytes is a 400", async () => {
+  const { env, kv } = setup();
+  const v = "é".repeat(300);
+  assert.ok(v.length < 512 && bytes(`version-exists:${v}`) > 512);
+  const before = captures.length;
+  const res = await call(env, "GET", `/api/versions/exists?v=${encodeURIComponent(v)}`);
+  assert.equal(res.status, 400);
+  assert.deepEqual(kv.accessed, []);
+  assert.equal(captures.length, before);
+});
+
+test("versions/exists: a valid version still asks npm once, caches the answer and serves the repeat from KV", async () => {
+  const { env, kv } = setup();
+  const realFetch = globalThis.fetch;
+  const asked = [];
+  globalThis.fetch = async (input) => {
+    if (String(input).startsWith("https://registry.npmjs.org/")) asked.push(String(input));
+    return new Response("{}", { status: 200 });
+  };
+  try {
+    const first = await call(env, "GET", "/api/versions/exists?v=17.0.0-next-abc123");
+    assert.equal(first.status, 200);
+    assert.deepEqual(await first.json(), { exists: true });
+    const second = await call(env, "GET", "/api/versions/exists?v=17.0.0-next-abc123");
+    assert.deepEqual(await second.json(), { exists: true });
+  } finally {
+    globalThis.fetch = realFetch;
+  }
+  assert.deepEqual(asked, ["https://registry.npmjs.org/handsontable/17.0.0-next-abc123"]);
+  assert.deepEqual(kv.rejected, []);
+});
+
+test("versions/exists: an empty v is still a 400", async () => {
+  const { env } = setup();
+  const res = await call(env, "GET", "/api/versions/exists?v=");
+  assert.equal(res.status, 400);
+});
+
+// ---- demo ids (getDemo cache key) -------------------------------------------
+
+test("GET /d/:id with an overlong id is a 404, not a KV throw", async () => {
+  const { env, kv } = setup();
+  const before = captures.length;
+  const res = await call(env, "GET", `/d/${"a".repeat(600)}/`);
+  assert.equal(res.status, 404);
+  assert.deepEqual(kv.rejected, []);
+  assert.equal(captures.length, before);
+});
+
+test("GET /d/:id with a multibyte id under 512 characters but over 512 bytes is a 404", async () => {
+  const { env, kv } = setup();
+  const id = "é".repeat(300);
+  const res = await call(env, "GET", `/d/${encodeURIComponent(id)}/`);
+  assert.equal(res.status, 404);
+  assert.deepEqual(kv.rejected, []);
+});
+
+test("GET /api/demos/:id and /api/demos/:id/source with an overlong id are 404s", async () => {
+  const { env, kv } = setup();
+  const before = captures.length;
+  for (const path of [`/api/demos/${"b".repeat(600)}`, `/api/demos/${"b".repeat(600)}/source`]) {
+    const res = await call(env, "GET", path);
+    assert.equal(res.status, 404, path);
+  }
+  assert.deepEqual(kv.rejected, []);
+  assert.equal(captures.length, before);
+});
+
+test("GET /d/:id for a real demo still serves its index.html", async () => {
+  const { env } = setup([demoRow()], { "demos/abc123/index.html": "<html>ok</html>" });
+  const res = await call(env, "GET", "/d/abc123/");
+  assert.equal(res.status, 200);
+  assert.match(await res.text(), /ok/);
+});
+
+// ---- R2 key from the /d/:id subpath ------------------------------------------
+
+test("GET /d/:id/<overlong path> does not hand R2 a key over 1024 bytes", async () => {
+  const { env, r2 } = setup([demoRow()], { "demos/abc123/index.html": "<html>ok</html>" });
+  const before = captures.length;
+  const res = await call(env, "GET", `/d/abc123/${"p".repeat(1100)}`);
+  assert.equal(res.status, 200, "falls through to the SPA index like any other unknown path");
+  assert.deepEqual(r2.rejected, []);
+  assert.equal(captures.length, before);
+});
+
+// ---- Tier-2 session ids (tombstone/meter KV keys, DO name) -------------------
+
+test("session subroutes with an overlong id are a 400 before any KV or sandbox access", async () => {
+  const { env, kv } = setup();
+  let sandboxTouched = 0;
+  setSandboxFactory(() => { sandboxTouched += 1; return {}; });
+  const id = "s".repeat(600);
+  for (const [method, path, init] of [
+    ["POST", `/api/session/${id}/file`, { body: JSON.stringify({ path: "a.js", contents: "x" }), headers: { "Content-Type": "application/json" } }],
+    ["DELETE", `/api/session/${id}/file?path=a.js`],
+    ["GET", `/api/session/${id}/status`],
+  ]) {
+    const res = await call(env, method, path, init);
+    assert.equal(res.status, 400, `${method} ${path.slice(0, 40)}`);
+  }
+  assert.deepEqual(kv.accessed, [], "no tombstone or meter read for an invalid id");
+  assert.equal(sandboxTouched, 0);
+});
+
+test("DELETE /api/session/:id with an overlong id is a 400 and touches nothing", async () => {
+  const { env, kv } = setup();
+  let sandboxTouched = 0;
+  setSandboxFactory(() => { sandboxTouched += 1; return { destroy: async () => {} }; });
+  const res = await call(env, "DELETE", `/api/session/${"s".repeat(600)}`);
+  assert.equal(res.status, 400);
+  assert.deepEqual(kv.accessed, []);
+  assert.equal(sandboxTouched, 0);
+});
+
+test("POST /api/session with an overlong client-supplied sessionId is a 400 and touches nothing", async () => {
+  const { env, kv } = setup();
+  let sandboxTouched = 0;
+  setSandboxFactory(() => { sandboxTouched += 1; return {}; });
+  const res = await call(env, "POST", "/api/session", {
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify({
+      framework: "react-js",
+      files: { "package.json": "{}" },
+      sessionId: "s".repeat(600),
+    }),
+  });
+  assert.equal(res.status, 400);
+  assert.deepEqual(kv.rejected, []);
+  assert.equal(sandboxTouched, 0);
+});

runner/workers/api/src/index.ts

@@ -45,6 +45,7 @@ import {
 } from "./session-lifecycle.js";
 import { refAmbiguousMessage, refUnknownMessage } from "./session-listing.js";
 import { ImportError, MAX_PAYLOAD_CHARS, importFromUrl, validatePayloadFiles } from "./import-url.js";
+import { VERSION_QUERY_RE, sessionIdFits } from "./storage-key.js";
 import { BuildFailure, buildFailureTags, createDemo, createPendingDemo, demoBuildState, getDemo, getDemoSource, hasCachedBuild, invalidateDemo, isUserBuildError, serveDemoAsset, shortId, updateDemo, userBuildErrorDetail, withEntryScript, type DemoRow } from "./share.js";
 import { BuildJobBase, scheduleSnapshotBuild } from "./snapshot-jobs.js";
 import {
@@ -912,6 +913,10 @@ async function handleNonProxyRequest(request: Request, env: Env, ctx: ExecutionC
         const cfg = BUILD_CONFIG[body.framework];
         if (!dev || !cfg) return json({ error: `Tier-2 not wired for framework: ${body.framework}` }, 400);
         const files = validateFiles(body.files);
+        // The id becomes a KV key suffix and a DO name; an overlong one is a client mistake, not a 500.
+        if (typeof body.sessionId === "string" && body.sessionId.trim() && !sessionIdFits(body.sessionId.trim())) {
+          return json({ error: "invalid session id" }, 400);
+        }
         // `session.start` (contract §5, "all outcomes"): one point per create
         // attempt regardless of how it ends, timed from here.
         const createStartedAt = Date.now();
@@ -1244,6 +1249,10 @@ async function handleNonProxyRequest(request: Request, env: Env, ctx: ExecutionC
       // covered by default instead of each hand-copying the check.
       if (parts[0] === "api" && parts[1] === "session" && parts.length >= 4) {
         const sessionId = parts[2]!;
+        // An overlong id makes the tombstone/meter KV reads throw, which those
+        // helpers swallow as "no marker" / "metered", waving an invented id
+        // through to a sandbox boot.
+        if (!sessionIdFits(sessionId)) return json({ error: "invalid session id" }, 400);
         const gateRequest = { method: request.method, sub: parts[3] };
         const refuse = (verdict: ReturnType<typeof sessionGateVerdict>) =>
           verdict === "noop" ? cors(new Response(null, { status: 204 })) : json({ error: "session closed" }, 410);
@@ -1337,6 +1346,7 @@ async function handleNonProxyRequest(request: Request, env: Env, ctx: ExecutionC
       // discards the response); the admin panel's kill button goes through the
       // same `teardownLiveSession`.
       if (request.method === "DELETE" && parts[0] === "api" && parts[1] === "session" && parts.length === 3) {
+        if (!sessionIdFits(parts[2]!)) return json({ error: "invalid session id" }, 400);
         await teardownLiveSession(env, parts[2]!);
         return cors(new Response(null, { status: 204 }));
       }
@@ -1948,6 +1958,10 @@ async function handleNonProxyRequest(request: Request, env: Env, ctx: ExecutionC
       if (request.method === "GET" && parts[0] === "api" && parts[1] === "versions" && parts[2] === "exists") {
         const v = url.searchParams.get("v")?.trim() ?? "";
         if (!v) return json({ error: "v is required" }, 400);
+        // Shape-check before the KV read: a KV key is capped at 512 bytes, so an
+        // overlong `v` makes `get` throw and an anonymous caller would mint a 500.
+        // Real versions and dist-tags are short ASCII.
+        if (!VERSION_QUERY_RE.test(v)) return json({ error: "v is not a valid version" }, 400);
         const cacheKey = `version-exists:${v}`;
         const cached = await env.CACHE.get(cacheKey, "json");
         if (cached) return cors(cacheableJson(cached));

runner/workers/api/src/share.ts

@@ -18,6 +18,7 @@ import { recordContainerUsage, SESSION_INSTANCE_TYPE } from "./budget.js";
 import { htmlEntryLoadsModule, snapshotBuildCommand } from "./build-command.js";
 import { htMajorFromVersion, injectLiteHtml } from "./monitor-inject.js";
 import { emitPoint } from "./telemetry/points.js";
+import { kvKeyFits, r2KeyFits } from "./storage-key.js";
 
 type SandboxLike = {
   mkdir(path: string, opts?: { recursive?: boolean }): Promise<unknown>;
@@ -723,6 +724,8 @@ export async function updateDemo(
 
 export async function getDemo(env: Env, id: string): Promise<DemoRow | null> {
   const cacheKey = `demo:${id}`;
+  // A KV key is capped at 512 bytes; no minted id is near that, so an overlong one is a miss.
+  if (!kvKeyFits(cacheKey)) return null;
   const cached = await env.CACHE.get(cacheKey, "json");
   if (cached) return cached as DemoRow;
   const row = await env.DB.prepare("SELECT * FROM demos WHERE id = ?").bind(id).first<DemoRow>();
@@ -888,7 +891,9 @@ export async function serveDemoAsset(
     if (isDocRequest) record(404, 0);
     return new Response("Not found", { status: 404 });
   }
-  const candidates = clean === "" ? ["index.html"] : [clean, `${clean}/index.html`, "index.html"];
+  // R2 throws on an object key over 1024 bytes; an overlong path cannot name an object, so skip it.
+  const candidates = (clean === "" ? ["index.html"] : [clean, `${clean}/index.html`, "index.html"])
+    .filter((c) => r2KeyFits(row.r2_prefix + c));
 
   let obj: R2ObjectBodyText | null = null;
   let hitPath = "index.html";

runner/workers/api/src/storage-key.ts

@@ -0,0 +1,36 @@
+// Platform limits on storage keys and object names built from request input.
+// Import-free so pipeline specs can load it directly under strip-types.
+
+const encoder = new TextEncoder();
+
+/** Workers KV rejects a key over 512 UTF-8 bytes (and an empty one) by throwing. */
+export const KV_KEY_MAX_BYTES = 512;
+
+/** R2 rejects an object key over 1024 UTF-8 bytes by throwing. */
+export const R2_KEY_MAX_BYTES = 1024;
+
+/** Longest client-supplied Tier-2 session id we accept; minted ids are ~30 bytes. */
+export const SESSION_ID_MAX_BYTES = 128;
+
+const byteLength = (value: string): number => encoder.encode(value).length;
+
+/** True when `key` is a legal KV key, measured in UTF-8 bytes rather than UTF-16 units. */
+export function kvKeyFits(key: string): boolean {
+  const bytes = byteLength(key);
+  return bytes > 0 && bytes <= KV_KEY_MAX_BYTES;
+}
+
+/** True when `key` is a legal R2 object key. */
+export function r2KeyFits(key: string): boolean {
+  const bytes = byteLength(key);
+  return bytes > 0 && bytes <= R2_KEY_MAX_BYTES;
+}
+
+/** True when a client-supplied session id is short enough to become a KV key suffix and a DO name. */
+export function sessionIdFits(sessionId: string): boolean {
+  const bytes = byteLength(sessionId);
+  return bytes > 0 && bytes <= SESSION_ID_MAX_BYTES;
+}
+
+/** What `/api/versions/exists` accepts: a semver or npm dist-tag shape, ASCII only. */
+export const VERSION_QUERY_RE = /^[0-9A-Za-z][0-9A-Za-z._+-]{0,63}$/;

runner/workers/o11y/src/drain/symbolicate.ts

@@ -63,6 +63,8 @@ function isBabelChunk(filename: string): boolean {
   }
 }
 
+const R2_KEY_MAX_BYTES = 1024;
+
 /** `sourcemaps/<service.version>/<original asset path>.map` (ADR §C.3).
  *  `null` when `filename` is not a parseable URL — a preview-host frame
  *  correctly never resolves to a real map. */
@@ -73,7 +75,9 @@ function mapKeyFor(filename: string, serviceVersion: string): string | null {
     // without this the symbolicator would fetch a nonexistent map and
     // report it as missing.
     if (url.pathname.endsWith("/")) return null;
-    return `sourcemaps/${serviceVersion}${url.pathname}.map`;
+    const key = `sourcemaps/${serviceVersion}${url.pathname}.map`;
+    // R2 throws on a key over 1024 bytes; the frame's path is browser-supplied, so it cannot name a map.
+    return new TextEncoder().encode(key).length > R2_KEY_MAX_BYTES ? null : key;
   } catch {
     return null;
   }