fix(vouchers): reserve claims atomically before reward dispatch

src/actions/voucher.test.ts

@@ -0,0 +1,291 @@
+import { drizzle } from "drizzle-orm/mysql-proxy";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const state = vi.hoisted(() => ({
+	userId: "42",
+	count: 0,
+	amount: 50,
+	missing: false,
+	expired: false,
+	max: 1,
+	already: false,
+	failUpdate: false,
+	failCommit: false,
+	queries: [] as Array<{ sql: string; tx: boolean; params: unknown[] }>,
+	events: [] as string[],
+	inTransaction: false,
+	connection: undefined as unknown,
+	deliver: vi.fn(),
+	audit: vi.fn(),
+}));
+vi.mock("@/lib/auth", () => ({
+	auth: async () => ({ user: { id: state.userId } }),
+}));
+vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
+vi.mock("@/lib/rate-limit", () => ({
+	clientIp: async () => "127.0.0.1",
+	rateLimit: async () => ({ ok: true }),
+}));
+vi.mock("@/lib/services/rcon", () => ({ rcon: {} }));
+vi.mock("@/lib/services/send-currency", () => ({
+	currencyDb: {},
+	sendCurrency: state.deliver,
+}));
+vi.mock("@/lib/services/audit", () => ({ logAudit: state.audit }));
+vi.mock("@/lib/db", async () => {
+	const forward =
+		(method: string) =>
+		(...args: unknown[]) =>
+			Reflect.apply(
+				Reflect.get(state.connection as object, method),
+				state.connection,
+				args,
+			);
+	return {
+		...(await import("@/db/schema")),
+		db: {
+			select: forward("select"),
+			insert: forward("insert"),
+			update: forward("update"),
+			transaction: async (run: (tx: unknown) => Promise<unknown>) => {
+				state.events.push("begin");
+				state.inTransaction = true;
+				try {
+					const result = await run(state.connection);
+					if (state.failCommit)
+						throw new Error("commit acknowledgement unavailable");
+					state.events.push("commit");
+					return result;
+				} catch (error) {
+					state.events.push("rollback");
+					throw error;
+				} finally {
+					state.inTransaction = false;
+				}
+			},
+		},
+	};
+});
+
+import { redeem } from "./voucher";
+
+function input() {
+	const form = new FormData();
+	form.set("code", "PROMO");
+	return form;
+}
+beforeEach(() => {
+	state.userId = "42";
+	state.count = 0;
+	state.amount = 50;
+	state.missing = false;
+	state.expired = false;
+	state.max = 1;
+	state.already = false;
+	state.failUpdate = false;
+	state.failCommit = false;
+	state.queries = [];
+	state.events = [];
+	state.inTransaction = false;
+	state.audit.mockReset();
+	state.deliver.mockReset();
+	state.audit.mockImplementation(async () => {
+		state.events.push("audit");
+	});
+	state.deliver.mockImplementation(async () => {
+		state.events.push("deliver");
+		return true;
+	});
+	state.connection = drizzle(async (sql, params, method) => {
+		state.queries.push({ sql, params, tx: state.inTransaction });
+		if (sql.startsWith("update") && state.failUpdate)
+			throw new Error("storage unavailable");
+		if (method !== "all") return { rows: [{ affectedRows: 1, insertId: 9 }] };
+		if (sql.includes("website_used_shop_vouchers"))
+			return { rows: state.already ? [[9]] : [] };
+		return {
+			rows: state.missing
+				? []
+				: [
+						[
+							"7",
+							state.amount,
+							state.max,
+							state.count,
+							state.expired ? "2000-01-01 00:00:00" : null,
+						],
+					],
+		};
+	});
+});
+
+describe("Voucher redemption integrity", () => {
+	it("never dispatches after an uncertain commit acknowledgement", async () => {
+		state.failCommit = true;
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toContain(
+			state.audit.mock.calls[0]?.[0].correlationId,
+		);
+		expect(state.deliver).not.toHaveBeenCalled();
+	});
+	it("locks duplicate claims using the authenticated user and selected voucher", async () => {
+		await redeem(null, input());
+		const query = state.queries.find(
+			(item) =>
+				item.sql.startsWith("select") &&
+				item.sql.includes("website_used_shop_vouchers"),
+		);
+		expect(query?.sql).toMatch(/for update$/);
+		expect(query?.params).toEqual([42, 7n, 1]);
+	});
+	it.each(["missing", "expired"] as const)(
+		"rejects a %s voucher without changing storage",
+		async (kind) => {
+			state[kind] = true;
+			expect((await redeem(null, input()))?.ok).toBe(false);
+			expect(state.queries).toHaveLength(1);
+			expect(state.deliver).not.toHaveBeenCalled();
+		},
+	);
+	it.each([0, -1, 0.5])(
+		"rejects invalid reward amount %s before reserving",
+		async (amount) => {
+			state.amount = amount;
+			expect((await redeem(null, input()))?.ok).toBe(false);
+			expect(state.queries).toHaveLength(1);
+			expect(state.deliver).not.toHaveBeenCalled();
+		},
+	);
+	it("increments the reserved counter and expires the final claim before delivery", async () => {
+		state.count = 4;
+		state.max = 5;
+		await redeem(null, input());
+		const update = state.queries.find((query) =>
+			query.sql.startsWith("update"),
+		);
+		expect(update?.sql).toContain("`use_count` = ?");
+		expect(update?.sql).toContain("`expires_at` = ?");
+		expect(update?.params[0]).toBe(5);
+		expect(state.audit.mock.calls[0]?.[0]).toMatchObject({
+			before: { useCount: 4 },
+			after: { useCount: 5, amount: 50 },
+		});
+	});
+	it("does not expire a voucher with remaining capacity", async () => {
+		state.max = 5;
+		await redeem(null, input());
+		const update = state.queries.find((query) =>
+			query.sql.startsWith("update"),
+		);
+		expect(update?.sql).not.toContain("`expires_at` =");
+	});
+	it("treats false delivery as unconfirmed and keeps the same audit reference", async () => {
+		state.deliver.mockResolvedValueOnce(false);
+		const result = await redeem(null, input());
+		const intent = state.audit.mock.calls[0]?.[0];
+		const outcome = state.audit.mock.calls[1]?.[0];
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toContain(intent.correlationId);
+		expect(outcome).toMatchObject({
+			correlationId: intent.correlationId,
+			outcome: "partial",
+			after: { reward: "unconfirmed" },
+		});
+	});
+	it("preserves the pending intent if delivery and completion audit both fail", async () => {
+		state.deliver.mockRejectedValueOnce(new Error("uncertain delivery"));
+		state.audit
+			.mockResolvedValueOnce(undefined)
+			.mockRejectedValueOnce(new Error("audit unavailable"));
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toContain(
+			state.audit.mock.calls[0]?.[0].correlationId,
+		);
+		expect(state.deliver).toHaveBeenCalledTimes(1);
+	});
+	it("never uses a submitted user id for the reward", async () => {
+		const form = input();
+		form.set("userId", "999");
+		await redeem(null, form);
+		expect(state.deliver).toHaveBeenCalledWith(
+			expect.anything(),
+			42,
+			"credits",
+			50,
+		);
+	});
+	it("reserves usage and audit under lock before sending currency", async () => {
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(true);
+		expect(state.queries[0]?.sql).toMatch(/for update$/);
+		expect(state.queries.every((query) => query.tx)).toBe(true);
+		expect(state.events).toEqual([
+			"begin",
+			"audit",
+			"commit",
+			"deliver",
+			"audit",
+		]);
+		expect(state.audit).toHaveBeenCalledWith(
+			expect.objectContaining({ outcome: "intent", domain: "economy" }),
+			state.connection,
+		);
+	});
+	it("rejects an exhausted unexpired voucher before awarding", async () => {
+		state.count = 1;
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+		expect(state.queries.some((query) => query.sql.startsWith("insert"))).toBe(
+			false,
+		);
+	});
+	it("does not deliver if reservation persistence fails", async () => {
+		state.failUpdate = true;
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+		expect(state.events).toContain("rollback");
+	});
+	it("blocks delivery if the durable intent cannot be stored", async () => {
+		state.audit.mockRejectedValueOnce(new Error("audit unavailable"));
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+		expect(state.events).toContain("rollback");
+	});
+	it("provides a correlated partial result for failed reward delivery", async () => {
+		state.deliver.mockRejectedValueOnce(new Error("transport unavailable"));
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(false);
+		expect(result?.message).toMatch(/reference:/i);
+		expect(state.audit).toHaveBeenLastCalledWith(
+			expect.objectContaining({
+				outcome: "partial",
+				after: expect.objectContaining({ reward: "unconfirmed" }),
+			}),
+		);
+		expect(state.deliver).toHaveBeenCalledTimes(1);
+	});
+	it("does not misreport delivered currency when completion auditing fails", async () => {
+		state.audit
+			.mockResolvedValueOnce(undefined)
+			.mockRejectedValueOnce(new Error("audit unavailable"));
+		const result = await redeem(null, input());
+		expect(result?.ok).toBe(true);
+		expect(result?.message).toMatch(/reference:/i);
+		expect(state.deliver).toHaveBeenCalledTimes(1);
+	});
+	it("rejects a previous claim without delivering again", async () => {
+		state.already = true;
+		expect((await redeem(null, input()))?.ok).toBe(false);
+		expect(state.deliver).not.toHaveBeenCalled();
+	});
+	it.each(["0", "-1", "1.5"])(
+		"rejects invalid session id %s",
+		async (userId) => {
+			state.userId = userId;
+			expect((await redeem(null, input()))?.ok).toBe(false);
+			expect(state.queries).toEqual([]);
+		},
+	);
+});

src/actions/voucher.ts

@@ -1,30 +1,18 @@
 "use server";
 
-import { and, eq, sql } from "drizzle-orm";
+import { randomUUID } from "node:crypto";
+import { and, eq } from "drizzle-orm";
 import { revalidatePath } from "next/cache";
 import { auth } from "@/lib/auth";
 import { db, WebsiteShopVouchers, WebsiteUsedShopVouchers } from "@/lib/db";
 import { clientIp, rateLimit } from "@/lib/rate-limit";
+import { type AuditEntry, logAudit } from "@/lib/services/audit";
 import { rcon } from "@/lib/services/rcon";
 import { currencyDb, sendCurrency } from "@/lib/services/send-currency";
 
-/** Feedback returned to the <RedeemForm/> client component via useActionState. */
 export type RedeemState = { ok: boolean; message: string } | null;
 
-/**
- * Redeem a shop voucher for the SIGNED-IN user. Faithful to AtomCMS's
- * ShopVoucherController:
- *   - the user id is re-read from the session (auth()), NEVER from FormData,
- *     so a crafted form cannot redeem on another account;
- *   - a code that is missing or expired is rejected;
- *   - each voucher may be redeemed once per user (website_used_shop_vouchers);
- *   - on success the reward `amount` is granted, the used-row is inserted,
- *     use_count is incremented, and the voucher is expired once max_uses is hit.
- *
- * The reward is delivered through sendCurrency({ rcon, db: currencyDb }); the
- * voucher schema carries a single `amount`, granted as the website credits
- * wallet currency.
- */
+/** Reserve each claim atomically before attempting a non-replayable reward. */
 export async function redeem(
 	_prev: RedeemState,
 	formData: FormData,
@@ -33,15 +21,13 @@ export async function redeem(
 	if (!session?.user?.id) {
 		return { ok: false, message: "You must be signed in to redeem a voucher." };
 	}
-
 	const userId = Number(session.user.id);
-	if (!Number.isFinite(userId)) {
+	if (!Number.isSafeInteger(userId) || userId <= 0) {
 		return {
 			ok: false,
 			message: "Your session is invalid. Please sign in again.",
 		};
 	}
-
 	await clientIp();
 	if (!(await rateLimit(`voucher-redeem:${userId}`, 5, 60_000)).ok) {
 		return {
@@ -49,132 +35,140 @@ export async function redeem(
 			message: "You're redeeming too fast. Please wait a moment and try again.",
 		};
 	}
-
-	const code = String(formData.get("code") ?? "")
-		.normalize("NFC")
-		.trim();
-	if (!code) {
-		return { ok: false, message: "Please enter a voucher code." };
+	const rawCode = formData.get("code");
+	const code =
+		typeof rawCode === "string" ? rawCode.normalize("NFC").trim() : "";
+	if (!code || code.length > 255) {
+		return { ok: false, message: "Please enter a valid voucher code." };
 	}
 
-	// Look up the code (website_shop_vouchers.code is unique).
-	let voucher: {
-		id: bigint;
-		amount: number;
-		maxUses: number;
-		useCount: number;
-		expiresAt: Date | null;
-	} | null;
+	const correlationId = randomUUID();
+	let claim: { amount: number; audit: AuditEntry } | { rejection: string };
 	try {
-		const [row] = await db
-			.select({
-				id: WebsiteShopVouchers.id,
-				amount: WebsiteShopVouchers.amount,
-				maxUses: WebsiteShopVouchers.maxUses,
-				useCount: WebsiteShopVouchers.useCount,
-				expiresAt: WebsiteShopVouchers.expiresAt,
-			})
-			.from(WebsiteShopVouchers)
-			.where(eq(WebsiteShopVouchers.code, code))
-			.limit(1);
-		voucher = row ?? null;
-	} catch {
-		return {
-			ok: false,
-			message: "We couldn't reach the server. Please try again.",
-		};
-	}
-
-	// Not found OR already expired -> generic "no active voucher" (matches AtomCMS).
-	if (
-		!voucher ||
-		(voucher.expiresAt && voucher.expiresAt.getTime() <= Date.now())
-	) {
-		return {
-			ok: false,
-			message: "No active voucher with the given code was found.",
-		};
-	}
-
-	// One redemption per user.
-	try {
-		const [already] = await db
-			.select({ id: WebsiteUsedShopVouchers.id })
-			.from(WebsiteUsedShopVouchers)
-			.where(
-				and(
-					eq(WebsiteUsedShopVouchers.userId, userId),
-					eq(WebsiteUsedShopVouchers.voucherId, voucher.id),
-				),
-			)
-			.limit(1);
-		if (already) {
-			return { ok: false, message: "You can only use each shop voucher once." };
-		}
-	} catch {
-		return {
-			ok: false,
-			message: "We couldn't reach the server. Please try again.",
-		};
-	}
-
-	// Record the redemption first so a successful grant can never be double-claimed.
-	try {
-		await db.insert(WebsiteUsedShopVouchers).values({
-			userId,
-			voucherId: voucher.id,
+		claim = await db.transaction(async (transaction) => {
+			const [voucher] = await transaction
+				.select({
+					id: WebsiteShopVouchers.id,
+					amount: WebsiteShopVouchers.amount,
+					maxUses: WebsiteShopVouchers.maxUses,
+					useCount: WebsiteShopVouchers.useCount,
+					expiresAt: WebsiteShopVouchers.expiresAt,
+				})
+				.from(WebsiteShopVouchers)
+				.where(eq(WebsiteShopVouchers.code, code))
+				.limit(1)
+				.for("update");
+			const now = new Date();
+			if (
+				!voucher ||
+				(voucher.expiresAt && voucher.expiresAt.getTime() <= now.getTime()) ||
+				!Number.isSafeInteger(voucher.maxUses) ||
+				voucher.maxUses <= 0 ||
+				!Number.isSafeInteger(voucher.useCount) ||
+				voucher.useCount < 0 ||
+				voucher.useCount >= voucher.maxUses ||
+				!Number.isSafeInteger(voucher.amount) ||
+				voucher.amount <= 0
+			) {
+				return {
+					rejection: "No active voucher with the given code was found.",
+				};
+			}
+			// A locking read avoids a stale repeatable-read snapshot after waiting
+			// for another claim on the same voucher. All claims lock voucher first.
+			const [already] = await transaction
+				.select({ id: WebsiteUsedShopVouchers.id })
+				.from(WebsiteUsedShopVouchers)
+				.where(
+					and(
+						eq(WebsiteUsedShopVouchers.userId, userId),
+						eq(WebsiteUsedShopVouchers.voucherId, voucher.id),
+					),
+				)
+				.limit(1)
+				.for("update");
+			if (already)
+				return { rejection: "You can only use each shop voucher once." };
+			await transaction.insert(WebsiteUsedShopVouchers).values({
+				userId,
+				voucherId: voucher.id,
+			});
+			const useCount = voucher.useCount + 1;
+			await transaction
+				.update(WebsiteShopVouchers)
+				.set({
+					useCount,
+					...(useCount >= voucher.maxUses ? { expiresAt: now } : {}),
+					updatedAt: now,
+				})
+				.where(eq(WebsiteShopVouchers.id, voucher.id));
+			const audit: AuditEntry = {
+				userId,
+				action: "economy.voucher.redeem",
+				target: "ShopVoucher",
+				domain: "economy",
+				correlationId,
+				before: {
+					voucherId: voucher.id.toString(),
+					useCount: voucher.useCount,
+				},
+				after: {
+					voucherId: voucher.id.toString(),
+					userId,
+					amount: voucher.amount,
+					useCount,
+					reward: "pending",
+				},
+			};
+			await logAudit({ ...audit, outcome: "intent" }, transaction);
+			return { amount: voucher.amount, audit };
 		});
 	} catch {
-		// Most likely a race (another tab redeemed it) — treat as already used.
-		return { ok: false, message: "You can only use each shop voucher once." };
+		// A commit acknowledgement can itself be uncertain. Never send a reward
+		// after any reservation error; the durable intent supports investigation.
+		return {
+			ok: false,
+			message: `We could not confirm your voucher reservation. Contact staff if needed. Reference: ${correlationId}`,
+		};
 	}
+	if ("rejection" in claim) return { ok: false, message: claim.rejection };
 
-	// Grant the reward. The voucher carries a single amount, delivered as credits.
+	let delivered = false;
 	try {
-		await sendCurrency(
+		delivered = await sendCurrency(
 			{ rcon, db: currencyDb },
 			userId,
 			"credits",
-			voucher.amount,
+			claim.amount,
 		);
 	} catch {
-		// sendCurrency already falls back to a direct DB write; if it still throws,
-		// the used-row stands and the balance simply wasn't credited — surface that.
+		// An increment may already have been dispatched. Keep the reservation,
+		// expose its reference, and do not automatically replay or refund it.
+	}
+	let auditSaved = true;
+	try {
+		await logAudit({
+			...claim.audit,
+			outcome: delivered ? "success" : "partial",
+			after: {
+				...claim.audit.after,
+				reward: delivered ? "dispatched" : "unconfirmed",
+			},
+		});
+	} catch {
+		auditSaved = false;
+	}
+	revalidatePath("/redeem");
+	if (!delivered) {
 		return {
 			ok: false,
-			message:
-				"Your voucher was accepted but the reward could not be delivered. Contact staff.",
+			message: `Your voucher is reserved, but reward delivery could not be confirmed. Contact staff. Reference: ${correlationId}`,
 		};
 	}
-
-	// Bump use_count and expire the voucher once the cap is reached.
-	try {
-		await db
-			.update(WebsiteShopVouchers)
-			.set({ useCount: sql`${WebsiteShopVouchers.useCount} + 1` })
-			.where(eq(WebsiteShopVouchers.id, voucher.id));
-		const [updated] = await db
-			.select({
-				maxUses: WebsiteShopVouchers.maxUses,
-				useCount: WebsiteShopVouchers.useCount,
-			})
-			.from(WebsiteShopVouchers)
-			.where(eq(WebsiteShopVouchers.id, voucher.id))
-			.limit(1);
-		if (updated?.maxUses && updated.useCount >= updated.maxUses) {
-			await db
-				.update(WebsiteShopVouchers)
-				.set({ expiresAt: new Date() })
-				.where(eq(WebsiteShopVouchers.id, voucher.id));
-		}
-	} catch {
-		// Reward already delivered; the counter bump is best-effort.
-	}
-
-	revalidatePath("/redeem");
-
 	return {
 		ok: true,
-		message: `Success! Your balance has been increased by ${voucher.amount.toLocaleString()} credits.`,
+		message: auditSaved
+			? `Your voucher was accepted and the ${claim.amount.toLocaleString()} credit reward was sent.`
+			: `Your voucher reward was sent, but its audit could not be completed. Reference: ${correlationId}`,
 	};
 }