fix: atomic promo redemption duplicate guard (fixes #92)

apps/web/lib/entitlements.test.ts

@@ -1,16 +1,17 @@
-import { describe, expect, it, vi } from "vitest";
+import { describe, expect, it, vi, beforeEach } from "vitest";
 
 vi.mock("server-only", () => ({}));
+// vi.mock factories are hoisted above top-level consts — create the mock fn
+// via vi.hoisted() to avoid a temporal-dead-zone ReferenceError.
+const { execute } = vi.hoisted(() => ({ execute: vi.fn() }));
 vi.mock("./db", () => ({
-  sqlClient: {
-    execute: vi.fn(),
-  },
+  sqlClient: { execute },
 }));
 vi.mock("@aiornot/db", () => ({
   newId: () => "key_test",
 }));
 
-import { normalizeApiKeyLabel } from "./entitlements";
+import { normalizeApiKeyLabel, recordPromoRedemption } from "./entitlements";
 
 describe("normalizeApiKeyLabel", () => {
   it("trims and collapses labels before storing them", () => {
@@ -26,3 +27,39 @@ describe("normalizeApiKeyLabel", () => {
     expect(normalizeApiKeyLabel("a".repeat(80))).toHaveLength(60);
   });
 });
+
+describe("recordPromoRedemption (atomic duplicate guard, #92)", () => {
+  beforeEach(() => {
+    execute.mockReset();
+  });
+
+  it("records a new redemption and bumps uses", async () => {
+    execute.mockResolvedValueOnce({ rows: [], rowsAffected: 1 }); // INSERT OR IGNORE
+    execute.mockResolvedValueOnce({ rows: [], rowsAffected: 1 }); // UPDATE uses
+    const ok = await recordPromoRedemption("summer25", "user_1");
+    expect(ok).toBe(true);
+    expect(execute).toHaveBeenCalledTimes(2);
+    expect(String(execute.mock.calls[0][0].sql)).toContain("INSERT OR IGNORE");
+    expect(String(execute.mock.calls[1][0].sql)).toContain("uses < max_uses");
+  });
+
+  it("normalizes the code to uppercase", async () => {
+    execute.mockResolvedValueOnce({ rows: [], rowsAffected: 1 });
+    execute.mockResolvedValueOnce({ rows: [], rowsAffected: 1 });
+    await recordPromoRedemption("  summer25 ", "user_1");
+    expect(execute.mock.calls[0][0].args).toEqual(["SUMMER25", "user_1"]);
+  });
+
+  it("returns false and skips the increment when the row already exists", async () => {
+    execute.mockResolvedValueOnce({ rows: [], rowsAffected: 0 }); // duplicate — INSERT OR IGNORE no-op
+    const ok = await recordPromoRedemption("SUMMER25", "user_1");
+    expect(ok).toBe(false);
+    expect(execute).toHaveBeenCalledTimes(1); // no UPDATE issued
+  });
+
+  it("returns false for a blank code", async () => {
+    const ok = await recordPromoRedemption("   ", "user_1");
+    expect(ok).toBe(false);
+    expect(execute).not.toHaveBeenCalled();
+  });
+});

apps/web/lib/entitlements.ts

@@ -140,16 +140,29 @@ export async function quotePromo(
   return { ok: true, code, percentOff, baseUsd, finalUsd, free: finalUsd <= 0 };
 }
 
-/** Record a promo redemption (idempotent). Call once the discount is actually granted. */
-export async function recordPromoRedemption(codeRaw: string, userId: string): Promise<void> {
+/**
+ * Record a promo redemption. Returns true if this call newly recorded the
+ * redemption, false if it was already recorded for this user+code.
+ *
+ * Uses INSERT OR IGNORE + a rowsAffected check so the duplicate guard is a
+ * 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.
+ */
+export async function recordPromoRedemption(codeRaw: string, userId: string): Promise<boolean> {
   const code = codeRaw.trim().toUpperCase();
-  if (!code) return;
-  try {
-    await sqlClient.execute({ sql: "INSERT INTO promo_redemptions (code, user_id) VALUES (?, ?)", args: [code, userId] });
-    await sqlClient.execute({ sql: "UPDATE promo_codes SET uses = uses + 1 WHERE code = ?", args: [code] });
-  } catch {
-    /* already recorded — composite PK makes this a no-op */
-  }
+  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;
 }
 
 /** Apply the entitlement for a purpose directly (used for 100%-off comps). */