From 2ee929658eb7a46dbdde45cf0ece48c52c8dcb7e Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Sat, 1 Aug 2026 21:16:12 -0700 Subject: [PATCH] fix(invites): close single-use invite max_uses TOCTOU (H1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Redeeming an invite read uses_count, checked the cap in JS, then did a non-atomic uses_count = uses_count + 1 write AFTER creating the membership. Two concurrent redemptions of a max_uses=1 invite both passed the check and both created a member, and the counter under-counted. Fix: claim a use ATOMICALLY before creating the membership via a compare-and-swap on uses_count (lib/invite-claim.ts) — the UPDATE only writes when uses_count still equals the value the cap decision was based on, so of two racing claims exactly one wins. The loser re-reads and retries while capacity remains (multi-use invites unaffected). On a failed membership insert the claim is rolled back so a slot is not burned. Extracted to lib/invite-claim.ts (out of the 'use server' surface, unit-tested). Co-Authored-By: Claude Opus 4.8 (1M context) --- __tests__/invite-claim.test.ts | 92 ++++++++++++++++++++++++++++++++++ lib/auth-actions.ts | 31 ++++++++---- lib/invite-claim.ts | 47 +++++++++++++++++ next-env.d.ts | 2 +- 4 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 __tests__/invite-claim.test.ts create mode 100644 lib/invite-claim.ts diff --git a/__tests__/invite-claim.test.ts b/__tests__/invite-claim.test.ts new file mode 100644 index 0000000..d241059 --- /dev/null +++ b/__tests__/invite-claim.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect } from 'vitest' +import { claimInviteUse } from '@/lib/invite-claim' + +/** + * Builds a mock admin client whose `space_invites` UPDATE ... .eq('uses_count', n) + * behaves like a real compare-and-swap: it only "affects a row" when the guarded + * `uses_count` equals the row's current value, incrementing it on success. This + * lets us simulate concurrent redemptions deterministically. + */ +function makeAdmin(initial: { uses_count: number; max_uses: number | null }) { + const row = { ...initial } + const client = { + from(_table: string) { + return { + // UPDATE path: .update(patch).eq('id',..).eq('uses_count',guard).select('id') + update(patch: { uses_count: number }) { + let guard: number | null = null + const builder: Record = { + eq(col: string, val: number) { + if (col === 'uses_count') guard = val + return builder + }, + select() { + // CAS: apply only if the guard matches the live value. + if (guard === row.uses_count) { + row.uses_count = patch.uses_count + return Promise.resolve({ data: [{ id: 'inv-1' }] }) + } + return Promise.resolve({ data: [] }) + }, + } + return builder + }, + // SELECT path: .select('uses_count').eq('id',..).maybeSingle() + select() { + const builder: Record = { + eq() { + return builder + }, + maybeSingle() { + return Promise.resolve({ data: { uses_count: row.uses_count } }) + }, + } + return builder + }, + } + }, + _row: row, + } + return client as unknown as Parameters[0] & { _row: typeof row } +} + +describe('claimInviteUse (invite max_uses TOCTOU fix)', () => { + it('claims a use when under the cap and returns the new count', async () => { + const admin = makeAdmin({ uses_count: 0, max_uses: 1 }) + const result = await claimInviteUse(admin, 'inv-1', 0, 1) + expect(result).toBe(1) + expect(admin._row.uses_count).toBe(1) + }) + + it('rejects a single-use invite already at cap (no over-issue)', async () => { + const admin = makeAdmin({ uses_count: 1, max_uses: 1 }) + // Second redeemer read uses_count=0 stale, but the row is already at 1. + const result = await claimInviteUse(admin, 'inv-1', 0, 1) + expect(result).toBeNull() + expect(admin._row.uses_count).toBe(1) // unchanged — not incremented past cap + }) + + it('lets the loser of a race retry into remaining multi-use capacity', async () => { + // max_uses=5, another redeemer already bumped it to 1 after our stale read of 0. + const admin = makeAdmin({ uses_count: 1, max_uses: 5 }) + const result = await claimInviteUse(admin, 'inv-1', 0, 5) + expect(result).toBe(2) // re-read to 1, then CAS-claimed to 2 + expect(admin._row.uses_count).toBe(2) + }) + + it('treats an unlimited invite (max_uses null) as always claimable', async () => { + const admin = makeAdmin({ uses_count: 99, max_uses: null }) + const result = await claimInviteUse(admin, 'inv-1', 99, null) + expect(result).toBe(100) + }) + + it('gives up (null) if it cannot win the CAS within the attempt budget', async () => { + // Caller's known count (0) is behind the live row (3), so the first CAS + // guard misses; with a 1-attempt budget it re-reads and then gives up + // rather than looping — proving the retry is bounded, not infinite. + const admin = makeAdmin({ uses_count: 3, max_uses: 10 }) + const result = await claimInviteUse(admin, 'inv-1', 0, 10, 1) + expect(result).toBeNull() + expect(admin._row.uses_count).toBe(3) // untouched + }) +}) diff --git a/lib/auth-actions.ts b/lib/auth-actions.ts index 167d1cc..e386779 100644 --- a/lib/auth-actions.ts +++ b/lib/auth-actions.ts @@ -5,6 +5,7 @@ import { createAdminClient } from '@/lib/supabase/admin' import { redirect } from 'next/navigation' import { checkRateLimit, sanitizeString, sanitizeSlug } from '@/lib/security' import { ACTIVE_STATUSES } from '@/lib/permissions' +import { claimInviteUse } from '@/lib/invite-claim' function generateInviteCode() { // Cryptographically random, and wider (8 chars) so the code space is not @@ -176,6 +177,15 @@ export async function joinSpace(formData: { return { error: 'You are already a member of a space.' } } + // Claim an invite use ATOMICALLY before creating the membership, so a + // single-use invite can never be redeemed twice under concurrent joins. + // (The pre-check above is a fast UX reject; this is the authoritative guard.) + let claimedCount: number | null = null + if (invite) { + claimedCount = await claimInviteUse(admin, invite.id, invite.uses_count, invite.max_uses) + if (claimedCount === null) return { error: 'This invite has reached its use cap.' } + } + const { data: newMember, error: memberErr } = await admin .from('space_members') .insert({ @@ -191,7 +201,18 @@ export async function joinSpace(formData: { .select('id') .single() - if (memberErr) return { error: memberErr.message } + if (memberErr) { + // Roll back the claimed use (best-effort, CAS-guarded) so a failed join + // does not burn an invite slot. + if (invite && claimedCount !== null) { + await admin + .from('space_invites') + .update({ uses_count: claimedCount - 1 }) + .eq('id', invite.id) + .eq('uses_count', claimedCount) + } + return { error: memberErr.message } + } // Retro-link prior anonymous form/waiver submissions in this space to the // new member, but only for a verified email (locked decision). Best-effort: @@ -205,14 +226,6 @@ export async function joinSpace(formData: { .eq('submitter_email', user.email.toLowerCase()) } - // Increment invite usage counter for the multi-code path. - if (invite) { - await admin - .from('space_invites') - .update({ uses_count: invite.uses_count + 1 }) - .eq('id', invite.id) - } - return { success: true } } diff --git a/lib/invite-claim.ts b/lib/invite-claim.ts new file mode 100644 index 0000000..f0ec58b --- /dev/null +++ b/lib/invite-claim.ts @@ -0,0 +1,47 @@ +import type { createAdminClient } from '@/lib/supabase/admin' + +type Admin = ReturnType + +/** + * Atomically claim one use of a multi-code invite via compare-and-swap on + * `uses_count`, closing the check-then-increment TOCTOU where two concurrent + * redemptions of a single-use invite could both succeed. + * + * The UPDATE only writes when `uses_count` still equals the value we based our + * cap decision on, so of two racing claims exactly one wins; the loser sees a + * 0-row result, re-reads, and retries while capacity remains. `uses_count` only + * ever increases, so the CAS has no ABA hazard. + * + * Returns the new `uses_count` on success (the caller stores it to roll the + * claim back if the subsequent membership insert fails), or `null` if the cap + * is reached. + */ +export async function claimInviteUse( + admin: Admin, + inviteId: string, + knownCount: number, + maxUses: number | null, + maxAttempts = 5, +): Promise { + let current = knownCount + for (let attempt = 0; attempt < maxAttempts; attempt++) { + if (maxUses !== null && current >= maxUses) return null + const next = current + 1 + const { data } = await admin + .from('space_invites') + .update({ uses_count: next }) + .eq('id', inviteId) + .eq('uses_count', current) // compare-and-swap guard + .select('id') + if (data && data.length > 0) return next + // Lost the race (or the row changed): re-read and retry while under cap. + const { data: fresh } = await admin + .from('space_invites') + .select('uses_count') + .eq('id', inviteId) + .maybeSingle() + if (!fresh) return null + current = (fresh as { uses_count: number }).uses_count + } + return null +} diff --git a/next-env.d.ts b/next-env.d.ts index c4b7818..9edff1c 100644 --- a/next-env.d.ts +++ b/next-env.d.ts @@ -1,6 +1,6 @@ /// /// -import "./.next/dev/types/routes.d.ts"; +import "./.next/types/routes.d.ts"; // NOTE: This file should not be edited // see https://nextjs.org/docs/app/api-reference/config/typescript for more information.