fix(worker): three correctness fixes found in review

- Validate drinkId before getReviewSummary to prevent an injection
  path through the resource-name segments (INVALID_RESOURCE_NAME 400)
- Reject unknown updateMask fields rather than silently ignoring them
  (UNKNOWN_FIELD_MASK 400), matching AIP-134 contract guarantees
- Eliminate post-write readRow() in upsert: compute finalStarRating /
  finalRecommend before writing and build the response from those values,
  removing one DB round trip and closing a TOCTOU race where a concurrent
  DELETE between write and re-read caused a non-null assertion crash

Test: adds UNKNOWN_FIELD_MASK case; all 86 tests pass

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C

Author: claude

cloudflare-worker/reviews.ts

@@ -217,7 +217,17 @@ export async function handleReviews(
 
   // isSummary
   if (segments.length === 4) {
-    return getReviewSummary({ db, bucket, festivalId, drinkId: segments[3], corsHeaders });
+    const drinkId = segments[3];
+    if (!isValidId(drinkId)) {
+      return errorResponse(
+        400,
+        "INVALID_ARGUMENT",
+        "Invalid resource name",
+        "INVALID_RESOURCE_NAME",
+        corsHeaders,
+      );
+    }
+    return getReviewSummary({ db, bucket, festivalId, drinkId, corsHeaders });
   }
   return listReviewSummaries({ db, bucket, festivalId, url, corsHeaders });
 }
@@ -311,12 +321,24 @@ async function upsertReview(request: Request, ctx: ReviewCtx): Promise<Response>
   }
 
   // Parse updateMask: comma-separated field names. Absent/empty = all provided fields.
+  const KNOWN_FIELDS = new Set(["starRating", "wouldRecommend"]);
   const patch = body as Record<string, unknown>;
   const maskRaw = patch.updateMask;
-  const mask: Set<string> | null =
-    typeof maskRaw === "string" && maskRaw.length > 0
-      ? new Set(maskRaw.split(",").map((s) => s.trim()))
-      : null;
+  let mask: Set<string> | null = null;
+  if (typeof maskRaw === "string" && maskRaw.length > 0) {
+    const fields = maskRaw.split(",").map((s) => s.trim());
+    const unknown = fields.filter((f) => !KNOWN_FIELDS.has(f));
+    if (unknown.length > 0) {
+      return errorResponse(
+        400,
+        "INVALID_ARGUMENT",
+        `Unknown updateMask field(s): ${unknown.join(", ")}`,
+        "UNKNOWN_FIELD_MASK",
+        corsHeaders,
+      );
+    }
+    mask = new Set(fields);
+  }
 
   const updateStar = mask === null ? "starRating" in patch : mask.has("starRating");
   const updateRec = mask === null ? "wouldRecommend" in patch : mask.has("wouldRecommend");
@@ -364,37 +386,36 @@ async function upsertReview(request: Request, ctx: ReviewCtx): Promise<Response>
   const existing = await readRow(db, bucket, festivalId, drinkId, deviceId);
   const now = Date.now();
 
+  // Compute the final column values upfront so we can build the response
+  // without a second DB read — avoids a round trip and the race where a
+  // concurrent DELETE between write and re-read would make row! throw.
+  const finalStarRating = updateStar ? (starRating ?? null) : (existing?.star_rating ?? null);
+  const finalRecommend = updateRec ? (recommend ?? null) : (existing?.recommend ?? null);
+
   if (existing) {
     await db
       .prepare(
         "UPDATE reviews SET star_rating = ?, recommend = ?, updated_at = ? " +
           "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?",
       )
-      .bind(
-        updateStar ? starRating : existing.star_rating,
-        updateRec ? recommend : existing.recommend,
-        now,
-        bucket, festivalId, drinkId, deviceId,
-      )
+      .bind(finalStarRating, finalRecommend, now, bucket, festivalId, drinkId, deviceId)
       .run();
   } else {
     await db
       .prepare(
         "INSERT INTO reviews (bucket, festival_id, drink_id, device_id, star_rating, recommend, updated_at) " +
           "VALUES (?, ?, ?, ?, ?, ?, ?)",
       )
-      .bind(
-        bucket, festivalId, drinkId, deviceId,
-        updateStar ? (starRating ?? null) : null,
-        updateRec ? (recommend ?? null) : null,
-        now,
-      )
+      .bind(bucket, festivalId, drinkId, deviceId, finalStarRating, finalRecommend, now)
       .run();
   }
 
-  const row = await readRow(db, bucket, festivalId, drinkId, deviceId);
   return jsonResponse<Review>(
-    serializeReview(reviewName(festivalId, drinkId), row!),
+    serializeReview(reviewName(festivalId, drinkId), {
+      star_rating: finalStarRating,
+      recommend: finalRecommend,
+      updated_at: now,
+    }),
     200,
     corsHeaders,
   );

cloudflare-worker/test/reviews.test.js

@@ -134,6 +134,17 @@ describe("reviews — PATCH (upsert)", () => {
     expect(data.averageRating).toBe(5);
   });
 
+  it("rejects an unknown updateMask field with a structured error", async () => {
+    const response = await patch("cbf2025", "beer-1", {
+      starRating: 3,
+      updateMask: "starRating,bogusField",
+    });
+    expect(response.status).toBe(400);
+    const { error } = await response.json();
+    expect(error.status).toBe("INVALID_ARGUMENT");
+    expect(error.details[0].reason).toBe("UNKNOWN_FIELD_MASK");
+  });
+
   it("rejects an out-of-range starRating with a structured error", async () => {
     const response = await patch("cbf2025", "beer-1", { starRating: 9 });
     expect(response.status).toBe(400);