docs(api): record the quota-claim TOCTOU as a KNOWN-LIMITATION (audit #4)
QuotaAvailable and ClaimServer run as two separate statements, so the count read is not serialized against a concurrent claim's UPDATE: two claims by one user for two different ownerless servers can both pass the gate and both succeed, leaving the user one server over quota. It is low severity — quota over-provisioning under a deliberate burst, not an authorization, ownership, or isolation break, since each server is still claimed atomically via UPDATE ... WHERE owner_id IS NULL. Closing it requires Postgres transaction semantics (advisory-xact-lock on the user, or SERIALIZABLE with retry) folding the gate into a single repo method — verifiable only against a real Postgres, not the hermetic fakeRepo suite. Documented at QuotaAvailable with back-references from the two claim gates (handleClaim and the internal UUID claim) rather than patched blind.
This commit is contained in:
3 files changed
+22
-2
No files matched your search
@@ -232,7 +232,9 @@ func (a *API) handleInternalClaim(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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)
|
ok, err := a.Repo.QuotaAvailable(r.Context(), userID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeError(w, r, err)
|
writeError(w, r, err)
|
||||||
|
|||||||
@@ -120,7 +120,9 @@ func (a *API) handleClaim(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
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)
|
ok, err := a.Repo.QuotaAvailable(r.Context(), p.UserID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeError(w, r, err)
|
writeError(w, r, err)
|
||||||
|
|||||||
@@ -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;
|
// 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).
|
// 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) {
|
func (p *PGRepo) QuotaAvailable(ctx context.Context, userID string) (bool, error) {
|
||||||
var maxServers sql.NullInt64
|
var maxServers sql.NullInt64
|
||||||
switch err := p.db.QueryRowContext(ctx,
|
switch err := p.db.QueryRowContext(ctx,
|
||||||
|
|||||||
Reference in new issue
Block a user