fix(invites): close single-use invite max_uses TOCTOU (H1)
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
624a708d8c
commit
2ee929658e
@@ -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<string, unknown> = {
|
||||
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<string, unknown> = {
|
||||
eq() {
|
||||
return builder
|
||||
},
|
||||
maybeSingle() {
|
||||
return Promise.resolve({ data: { uses_count: row.uses_count } })
|
||||
},
|
||||
}
|
||||
return builder
|
||||
},
|
||||
}
|
||||
},
|
||||
_row: row,
|
||||
}
|
||||
return client as unknown as Parameters<typeof claimInviteUse>[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
|
||||
})
|
||||
})
|
||||
+22
-9
@@ -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 }
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import type { createAdminClient } from '@/lib/supabase/admin'
|
||||
|
||||
type Admin = ReturnType<typeof createAdminClient>
|
||||
|
||||
/**
|
||||
* 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<number | null> {
|
||||
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
|
||||
}
|
||||
Vendored
+1
-1
@@ -1,6 +1,6 @@
|
||||
/// <reference types="next" />
|
||||
/// <reference types="next/image-types/global" />
|
||||
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.
|
||||
|
||||
Reference in New Issue
Block a user