fix(aggregator): wrap syncGetRecord errors in the CORS + no-store envelope

apps/aggregator/src/routes/xrpc/router.ts

@@ -76,20 +76,36 @@ function applyCorsHeaders(headers: Headers): void {
 }
 
 /**
- * Generic 500 for an unexpected pre-dispatch failure (e.g. a D1 error resolving
- * the labeler policy). Logs the internal error for operators but returns the
- * router's own opaque envelope, so no internal detail (SQL text, stack) reaches
- * the client. Matches the body the router itself produces for a handler throw.
+ * Generic 500 for an unexpected failure (e.g. a D1 error). Logs the internal
+ * error under `context` for operators but returns the router's own opaque
+ * envelope, so no internal detail (SQL text, stack) reaches the client.
+ * Matches the body the router itself produces for a handler throw.
  */
-function internalErrorResponse(err: unknown): Response {
-	console.error("[aggregator] xrpc policy resolution failed", {
+function internalErrorResponse(err: unknown, context: string): Response {
+	console.error(`[aggregator] xrpc ${context} failed`, {
 		error: err instanceof Error ? (err.stack ?? err.message) : String(err),
 	});
 	return new InternalServerError({
 		message: "an exception happened whilst processing this request",
 	}).toResponse();
 }
 
+/**
+ * Wrap an unexpected dispatch failure as an error Response carrying CORS +
+ * `no-store`. An unwrapped throw escapes to workerd's bare 500, dropping both
+ * — and on the cacheable `sync.getRecord` path it would leave a takedown-
+ * relevant error under a public cache header. `XRPCError` keeps its typed
+ * envelope (e.g. a malformed accept-labelers header); anything else is opaque.
+ */
+function wrapDispatchError(err: unknown, context: string): Response {
+	const errorResponse =
+		err instanceof XRPCError ? err.toResponse() : internalErrorResponse(err, context);
+	const headers = new Headers(errorResponse.headers);
+	headers.set("cache-control", NO_STORE);
+	applyCorsHeaders(headers);
+	return new Response(errorResponse.body, { status: errorResponse.status, headers });
+}
+
 /**
  * Dispatch any `/xrpc/*` request. Returns null when the path isn't an
  * XRPC route (caller falls through to other route matching).
@@ -108,7 +124,12 @@ export async function handleXrpc(env: Env, request: Request): Promise<Response |
 	}
 
 	if (url.pathname === SYNC_GET_RECORD_PATH) {
-		const response = await syncGetRecord(env, request);
+		let response: Response;
+		try {
+			response = await syncGetRecord(env, request);
+		} catch (err) {
+			return wrapDispatchError(err, "sync.getRecord");
+		}
 		const headers = new Headers(response.headers);
 		applyCorsHeaders(headers);
 		return new Response(response.body, {
@@ -128,15 +149,7 @@ export async function handleXrpc(env: Env, request: Request): Promise<Response |
 	try {
 		policy = await resolveRequestLabelerPolicy(env, request);
 	} catch (err) {
-		// Both branches take the same wrapper as a normal response so the
-		// CORS + `no-store` cache invariant holds on the error path too — an
-		// unwrapped throw would escape to workerd's bare 500, dropping both
-		// and leaving a takedown-relevant response cacheable.
-		const errorResponse = err instanceof XRPCError ? err.toResponse() : internalErrorResponse(err);
-		const headers = new Headers(errorResponse.headers);
-		headers.set("cache-control", NO_STORE);
-		applyCorsHeaders(headers);
-		return new Response(errorResponse.body, { status: errorResponse.status, headers });
+		return wrapDispatchError(err, "policy resolution");
 	}
 
 	const router = getRouter(env);

apps/aggregator/test/read-api.test.ts

@@ -1293,6 +1293,44 @@ describe("XRPC dispatcher — unexpected policy-resolution failure", () => {
 	});
 });
 
+describe("sync.getRecord — unexpected blob-fetch failure", () => {
+	// A D1 error inside the CAR passthrough (here: `publishers` is gone) must
+	// take the CORS + `no-store` wrapper rather than escape to workerd's bare
+	// 500 — and must NOT inherit the cacheable success header. Capture the
+	// table's schema so the shared beforeEach (`DELETE FROM publishers`) keeps
+	// working after a test drops it.
+	let publishersSchema: string;
+	beforeAll(async () => {
+		const row = await testEnv.DB.prepare(
+			"SELECT sql FROM sqlite_master WHERE type='table' AND name='publishers'",
+		).first<{ sql: string }>();
+		publishersSchema = row!.sql;
+	});
+	afterEach(async () => {
+		const exists = await testEnv.DB.prepare(
+			"SELECT 1 FROM sqlite_master WHERE type='table' AND name='publishers'",
+		).first();
+		if (!exists) await testEnv.DB.prepare(publishersSchema).run();
+	});
+
+	it("wraps a D1 failure in a 500 carrying CORS + no-store and no leaked internals", async () => {
+		// The publisher-profile blob read runs `SELECT record_blob FROM publishers`;
+		// dropping the table makes that throw a non-XRPC D1 error.
+		await testEnv.DB.prepare("DROP TABLE publishers").run();
+
+		const res = await SELF.fetch(
+			`https://test/xrpc/com.atproto.sync.getRecord?did=${DID_A}&collection=${NSID.publisherProfile}&rkey=self`,
+		);
+		expect(res.status).toBe(500);
+		expect(res.headers.get("cache-control")).toBe("private, no-store");
+		expect(res.headers.get("access-control-allow-origin")).toBe("*");
+		const body = (await res.json()) as { error: string; message?: string };
+		expect(body.error).toBe("InternalServerError");
+		// No internal detail (SQL text, table name, stack) leaks to the client.
+		expect(JSON.stringify(body)).not.toMatch(/publishers|no such table|SQL|SELECT/i);
+	});
+});
+
 describe("getPublisherVerification", () => {
 	it("returns the verification claims naming a DID as subject, newest first", async () => {
 		await seedVerification({