diff --git a/internal/api/handlers_internal.go b/internal/api/handlers_internal.go index a9c96c8..4534934 100644 --- a/internal/api/handlers_internal.go +++ b/internal/api/handlers_internal.go @@ -232,7 +232,9 @@ func (a *API) handleInternalClaim(w http.ResponseWriter, r *http.Request) { return } - // ② quota gate, evaluated before the ownership write (mirrors handleClaim). + // ② quota gate, evaluated before the ownership write (mirrors handleClaim). It + // shares handleClaim's quota TOCTOU KNOWN-LIMITATION — see QuotaAvailable (audit + // #4, ENV-blocked). ok, err := a.Repo.QuotaAvailable(r.Context(), userID) if err != nil { writeError(w, r, err) diff --git a/internal/api/handlers_user.go b/internal/api/handlers_user.go index febab50..a38bbbe 100644 --- a/internal/api/handlers_user.go +++ b/internal/api/handlers_user.go @@ -120,7 +120,9 @@ func (a *API) handleClaim(w http.ResponseWriter, r *http.Request) { return } - // ② quota gate, evaluated before the ownership write + // ② quota gate, evaluated before the ownership write. This gate and ③ are two + // separate statements, not one transaction — see the quota TOCTOU KNOWN-LIMITATION + // on QuotaAvailable (audit #4, ENV-blocked: needs real Postgres to close/verify). ok, err := a.Repo.QuotaAvailable(r.Context(), p.UserID) if err != nil { writeError(w, r, err) diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index ad74d3d..9fdbef4 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -191,6 +191,22 @@ func (p *PGRepo) RedeemPlayerBindCode(ctx context.Context, newUserID, code strin // QuotaAvailable treats a missing quota row or a NULL max_servers as unlimited; // otherwise it compares the live owned-server count against the cap (spec §9.3). +// +// KNOWN-LIMITATION (audit #4, quota TOCTOU): this check and ClaimServer are two +// separate statements, not one transaction, so the count read here is not serialized +// against a concurrent claim's UPDATE. Two claims by the same user for two DIFFERENT +// ownerless servers can both read count < max_servers (under READ COMMITTED neither +// sees the other's uncommitted UPDATE) and both succeed, leaving the user one server +// over quota. Severity is low: it over-provisions the quota by a small margin under a +// deliberate concurrent burst — it is NOT an authorization, ownership, or isolation +// break (each server is still claimed atomically via UPDATE ... WHERE owner_id IS +// NULL, so two users never share one server). Closing it needs Postgres transaction +// semantics: wrap the count and a conditional UPDATE (gated on count < max_servers) in +// one tx under pg_advisory_xact_lock(hashtext(user_id)) — or SERIALIZABLE with a retry +// loop — folding the gate out of the two handlers (handleClaim and the internal UUID +// claim) into a single repo method. That is INTEGRATION-dependent: it is verifiable +// only against a real Postgres, not the hermetic fakeRepo suite, so it is documented +// here rather than patched blind. func (p *PGRepo) QuotaAvailable(ctx context.Context, userID string) (bool, error) { var maxServers sql.NullInt64 switch err := p.db.QueryRowContext(ctx,