Phase 5 hygiene: image ownership, real deletes, scan resilience

Images were readable by any signed-in user, not just their owner. Keys
are now namespaced per user in both shapes the app writes, the three
bar/ objects that predated that were migrated under the owner's prefix,
and the image route enforces the prefix - answering 404 rather than 403
so it does not confirm someone else's key exists. Barcode-mirrored
images now write under the user prefix too.

deleteImage existed but was never called, so deleting a drink, bar item
or scan removed the row and left the file in storage forever, still
fetchable. All three now clean up through one shared helper, after the
database work and best-effort, so a storage hiccup cannot block a delete
the user asked for or leave the row behind.

Menu scans are processed detached from the request, so every deploy
abandoned anything in flight and left the row PROCESSING forever with a
spinner that never resolved. They are now reaped lazily when a user
lists their scans - no scheduler needed. Concurrent extractions are
capped at two per process: each is a vision call with a four minute
timeout that costs real money, and nothing else bounded them.

MenuItem gains userId. Ownership was only ever transitive via scanId,
which held because nothing queries MenuItem directly, but left any
future direct query an IDOR with nothing to stop it.

Restore was correctly scoped but unbounded, so a crafted file could
create unlimited rows in one transaction against shared Postgres - self
harm with one user, denial of service with several. Capped, and imageUrl
from the CSV now goes through the same validation the API enforces
instead of reaching the column unchecked.

Members can delete their own account and data. Once the app holds other
people's history, including Rating.location, that is the minimum.

Registration rate limiting took the first x-forwarded-for hop, which the
client controls; it now takes the last, which our proxy appends.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/lib/backup.ts

@@ -1,5 +1,6 @@
 import { objectsToCsv } from "@/lib/csv"
 import { prisma } from "@/lib/prisma"
+import { imageUrlSchema } from "@/lib/validators"
 import type {
   Drink,
   Rating,
@@ -466,11 +467,42 @@ type DrinkType = "BEER" | "WINE" | "COCKTAIL" | "SPIRIT" | "OTHER"
 type BarItemCategory = "SPIRITS" | "LIQUEURS" | "MIXERS" | "BITTERS" | "GARNISHES" | "TOOLS"
 type BarItemQuantity = "FULL" | "HALF" | "LOW" | "EMPTY"
 
+/**
+ * Upper bound on what one restore may write.
+ *
+ * The restore is correctly scoped to the caller, but nothing bounded its size: a
+ * crafted file could create an unlimited number of rows in a single transaction
+ * against shared Postgres. Self-harm with one user; a denial of service once other
+ * people share the database.
+ */
+const MAX_RESTORE_ROWS = 20000
+
+/**
+ * imageUrl comes straight from an uploaded CSV and never passed through the schema
+ * the API enforces, so a crafted file could put an arbitrary string in the column.
+ */
+function safeImageUrl(value: string | null | undefined): string | null {
+  if (!value) return null
+  return imageUrlSchema.safeParse(value).success ? value : null
+}
+
 export async function executeRestore(
   userId: string,
   data: ParsedBackupData,
   mode: RestoreMode
 ): Promise<RestoreSummary> {
+  const totalRows =
+    data.drinks.length +
+    data.ratings.length +
+    data.wishlistItems.length +
+    data.sharedLists.length +
+    data.barItems.length
+  if (totalRows > MAX_RESTORE_ROWS) {
+    throw new Error(
+      `That backup has ${totalRows} rows, more than the ${MAX_RESTORE_ROWS} allowed in one restore.`
+    )
+  }
+
   return await prisma.$transaction(
     async (tx) => {
       const summary: RestoreSummary = {
@@ -553,7 +585,7 @@ export async function executeRestore(
               region: drink.region ?? null,
               abv: drink.abv ?? null,
               description: drink.description ?? null,
-              imageUrl: drink.imageUrl ?? null,
+              imageUrl: safeImageUrl(drink.imageUrl),
             },
           })
           drinkIdMap.set(drink._originalId, created.id)
@@ -577,7 +609,7 @@ export async function executeRestore(
                   region: drink.region ?? null,
                   abv: drink.abv ?? null,
                   description: drink.description ?? null,
-                  imageUrl: drink.imageUrl ?? null,
+                  imageUrl: safeImageUrl(drink.imageUrl),
                 },
               })
               summary.drinks.updated++
@@ -595,7 +627,7 @@ export async function executeRestore(
                 region: drink.region ?? null,
                 abv: drink.abv ?? null,
                 description: drink.description ?? null,
-                imageUrl: drink.imageUrl ?? null,
+                imageUrl: safeImageUrl(drink.imageUrl),
               },
             })
             drinkIdMap.set(drink._originalId, created.id)

src/lib/images.ts

@@ -0,0 +1,34 @@
+import { deleteImage } from "@/lib/s3"
+
+const PROXY_PREFIX = "/minio-images/"
+
+/** Storage key behind one of our image URLs, or null if it is not one of ours. */
+export function imageKeyFromUrl(url: string | null | undefined): string | null {
+  if (!url || !url.startsWith(PROXY_PREFIX)) return null
+  const key = url.slice(PROXY_PREFIX.length)
+  return key.length > 0 && !key.includes("..") ? key : null
+}
+
+/**
+ * Delete the objects behind these image URLs, ignoring anything that is not ours.
+ *
+ * Always best effort: storage being unavailable must never fail a delete the user
+ * asked for, and must never leave the row behind either. Call after the database
+ * work, and do not await the result for correctness.
+ */
+export async function deleteImagesByUrl(
+  urls: (string | null | undefined)[]
+): Promise<number> {
+  const keys = urls
+    .map(imageKeyFromUrl)
+    .filter((k): k is string => k !== null)
+
+  if (keys.length === 0) return 0
+
+  const results = await Promise.allSettled(keys.map((k) => deleteImage(k)))
+  const failed = results.filter((r) => r.status === "rejected").length
+  if (failed > 0) {
+    console.warn(`[images] ${failed}/${keys.length} object deletions failed`)
+  }
+  return keys.length - failed
+}

src/lib/scan-health.ts

@@ -0,0 +1,60 @@
+import { prisma } from "@/lib/prisma"
+
+/**
+ * Menu scans are processed detached from the request, so a restart - which every
+ * deploy causes - abandons anything in flight and leaves the row PROCESSING
+ * forever, with a spinner that never resolves.
+ *
+ * There is no job queue to recover them, so instead they are reaped lazily whenever
+ * a user looks at their scans. Cheap, needs no scheduler, and the worst case is a
+ * stuck scan showing as failed a little later than it truly failed.
+ */
+const STUCK_AFTER_MS = 15 * 60 * 1000
+
+export async function reapStuckScans(userId: string): Promise<number> {
+  try {
+    const { count } = await prisma.menuScan.updateMany({
+      where: {
+        userId,
+        status: "PROCESSING",
+        updatedAt: { lt: new Date(Date.now() - STUCK_AFTER_MS) },
+      },
+      data: {
+        status: "FAILED",
+        errorMessage: "Processing was interrupted. Please try scanning again.",
+      },
+    })
+    if (count > 0) {
+      console.warn(`[scan] reaped ${count} stuck scan(s) for ${userId}`)
+    }
+    return count
+  } catch (error) {
+    // Never let housekeeping break a page load.
+    console.warn("[scan] reap failed:", error)
+    return 0
+  }
+}
+
+/**
+ * Caps concurrent menu extractions in this process.
+ *
+ * Each one is a vision call with a 4 minute timeout that costs real money, and the
+ * work is detached from the request so nothing else bounds it. With one user this
+ * never mattered; with several it is the difference between a queue and a stampede.
+ */
+const MAX_CONCURRENT_SCANS = 2
+let inFlight = 0
+
+export function tryAcquireScanSlot(): boolean {
+  if (inFlight >= MAX_CONCURRENT_SCANS) return false
+  inFlight += 1
+  return true
+}
+
+export function releaseScanSlot(): void {
+  inFlight = Math.max(0, inFlight - 1)
+}
+
+export function scansInFlight(): number {
+  return inFlight
+}