From 36f647ec9ca5e48fc3ee57c88cb4fa06ac53abe2 Mon Sep 17 00:00:00 2001
From: Anthony <anthony@profullstack.com>
Date: Sat, 1 Aug 2026 03:21:14 +0000
Subject: [PATCH] fix(entitlements): never let promo redemption throw into the
 grant response
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#110 dropped the try/catch around recordPromoRedemption to make the
duplicate guard atomic. Both callers grant the entitlement BEFORE
recording the redemption and neither handles errors:

  - app/api/payments/status/route.ts:58
  - app/api/payments/create-checkout/route.ts:59

So a transient DB failure in the INSERT now 500s a response that has
already granted the purchase — and the status route's payload carries
grant.apiKeyPlaintext, which is shown exactly once. The user would lose
their API key to that 500.

Restore the catch (log + return false) while keeping the atomic
INSERT OR IGNORE guard and the capped increment. The recorded flag is
tracked outside the try so a failed counter bump still reports the
redemption row that did land.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 apps/web/lib/entitlements.test.ts | 19 +++++++++++++++++
 apps/web/lib/entitlements.ts      | 35 +++++++++++++++++++++----------
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/apps/web/lib/entitlements.test.ts b/apps/web/lib/entitlements.test.ts
index 6e65896..bc50ad9 100644
--- a/apps/web/lib/entitlements.test.ts
+++ b/apps/web/lib/entitlements.test.ts
@@ -62,4 +62,23 @@ describe("recordPromoRedemption (atomic duplicate guard, #92)", () => {
     expect(ok).toBe(false);
     expect(execute).not.toHaveBeenCalled();
   });
+
+  // The grant (and its one-time API key) is already issued by the time we get
+  // here, so a DB blip must not escape and 500 the caller's response.
+  it("swallows a failing INSERT and returns false", async () => {
+    const err = vi.spyOn(console, "error").mockImplementation(() => {});
+    execute.mockRejectedValueOnce(new Error("db unreachable"));
+    await expect(recordPromoRedemption("SUMMER25", "user_1")).resolves.toBe(false);
+    expect(err).toHaveBeenCalled();
+    err.mockRestore();
+  });
+
+  it("swallows a failing uses increment but still reports the recorded row", async () => {
+    const err = vi.spyOn(console, "error").mockImplementation(() => {});
+    execute.mockResolvedValueOnce({ rows: [], rowsAffected: 1 }); // INSERT OR IGNORE landed
+    execute.mockRejectedValueOnce(new Error("db unreachable")); // UPDATE uses blew up
+    await expect(recordPromoRedemption("SUMMER25", "user_1")).resolves.toBe(true);
+    expect(err).toHaveBeenCalled();
+    err.mockRestore();
+  });
 });
diff --git a/apps/web/lib/entitlements.ts b/apps/web/lib/entitlements.ts
index b02c40e..a6a0796 100644
--- a/apps/web/lib/entitlements.ts
+++ b/apps/web/lib/entitlements.ts
@@ -148,21 +148,34 @@ export async function quotePromo(
  * single atomic statement instead of the separate check-then-record pair in
  * quotePromo()/recordPromoRedemption() (fixes #92, TOCTOU). The composite PK
  * (code, user_id) makes the INSERT itself the source of truth.
+ *
+ * Never throws. Callers grant the entitlement BEFORE recording the redemption,
+ * and the grant response carries the one-time API key plaintext — letting a DB
+ * blip escape here would 500 that response and lose the key for good. Log and
+ * report "not newly recorded" instead.
  */
 export async function recordPromoRedemption(codeRaw: string, userId: string): Promise<boolean> {
   const code = codeRaw.trim().toUpperCase();
   if (!code) return false;
-  const res = await sqlClient.execute({
-    sql: "INSERT OR IGNORE INTO promo_redemptions (code, user_id) VALUES (?, ?)",
-    args: [code, userId],
-  });
-  if (Number(res.rowsAffected) !== 1) return false; // already recorded
-  // Atomic capped increment (also covered by #106): never over-redeem past max_uses.
-  await sqlClient.execute({
-    sql: "UPDATE promo_codes SET uses = uses + 1 WHERE code = ? AND (max_uses IS NULL OR uses < max_uses)",
-    args: [code],
-  });
-  return true;
+  // Tracked outside the try so a failed counter bump still reports the
+  // redemption that did land, rather than claiming it never happened.
+  let recorded = false;
+  try {
+    const res = await sqlClient.execute({
+      sql: "INSERT OR IGNORE INTO promo_redemptions (code, user_id) VALUES (?, ?)",
+      args: [code, userId],
+    });
+    if (Number(res.rowsAffected) !== 1) return false; // already recorded
+    recorded = true;
+    // Atomic capped increment (also covered by #106): never over-redeem past max_uses.
+    await sqlClient.execute({
+      sql: "UPDATE promo_codes SET uses = uses + 1 WHERE code = ? AND (max_uses IS NULL OR uses < max_uses)",
+      args: [code],
+    });
+  } catch (err) {
+    console.error(`[promo] failed to record redemption of ${code}:`, (err as Error).message);
+  }
+  return recorded;
 }
 
 /** Apply the entitlement for a purpose directly (used for 100%-off comps). */