billing: org monthly spend-limit TOCTOU — concurrent preflights can overshoot a hard cap by N× request cost

[rohith500]

Summary

The org monthly spend-limit check (WithOrgMonthlySpendCap middleware →
CheckOrgMonthlySpend) is a plain check-then-act with no guard against
concurrent requests. Under real concurrency, N simultaneous requests can
all read the same pre-debit spend total, all pass the limit check, and all
debit — allowing total spend to exceed a configured "hard cap" by a
multiple of the limit itself, not by a bounded, single-request amount.

Reproduced independently three times against real Postgres and real
production code (this investigation, an independent second pass, and
directly on a maintainer-reproducible local stack) — see below.

Why this is a bug, not an accepted soft cap

• Migration 0037 names org_monthly_limit_usd_micros a "hard cap" on
  total inference spend per UTC month, "independent of prepaid
  balance". No soft / best-effort / advisory wording in the migration
  or in PRs feat(billing): meter per-engineer and per-org monthly spend #768 / feat(billing): enforce per-engineer and per-org monthly spend limits #769.
• Customer-facing 402 copy implies a hard stop:
  "Your organization has reached its monthly Weave Router spend limit. An org admin can raise the limit, or it resets next month."
  Multiplying past that limit under concurrency breaks that promise.
• Soft/singular overshoot ("bounded by in-flight request cost") was
  documented in feat(billing): enforce per-engineer and per-org monthly spend limits #769 for organic post-response metering. That claim does
  not cover unbounded N-concurrent overshoot, and no source accepts that
  tradeoff for monthly caps.

Root cause

Check (internal/server/middleware/org_monthly_spend_cap.go):

result, err := svc.CheckOrgMonthlySpend(c.Request.Context(), orgID)
// ...
if result.LimitReached() {
    // 402
}
c.Next()

CheckOrgMonthlySpend → Repo.GetOrgMonthlySpendAndLimit is a plain
SELECT — no FOR UPDATE, no serializable isolation, no advisory lock.
LimitReached() is a simple spent >= limit comparison against the value
read at check time; it doesn't account for the current request's own cost
either, so there's already a small expected single-request overshoot by
design — the bug here is the multiplicative overshoot under concurrency,
not that number.

Write (db/queries/billing.sql, DebitOrgCredits): a single CTE that
atomically decrements organization_credit_balance, inserts a ledger row,
and (gated on the balance row existing) upserts organization_monthly_spend.
The query's own comment explicitly documents this exact tradeoff for the
prepaid-balance case:

No balance >= amount guard: concurrent requests can both pass the
preflight balance check and both debit... The min-balance threshold on
the middleware bounds the typical dip.

That comment is scoped to the prepaid balance dip (bounded by the
middleware's min-balance / overdraft-floor machinery). It says nothing about
the monthly cap case — and for the monthly cap, there is no equivalent
bound. Once N concurrent requests all clear the preflight check, all N
debits land, and the final spend is starting_spend + N × cost_per_request,
unbounded by N.

Middleware ordering (internal/server/server.go): on inference routes,
billing middleware is appended as
WithBalanceCheck → WithAPIKeySpendCap → WithOrgMonthlySpendCap.
The monthly-cap gate only runs for requests that already passed the prepaid
balance check (or override/subscription short-circuit). Exposure is therefore
highest for orgs with sufficient prepaid balance / override but a tight
monthly hard cap.

Sibling gates (checkUserMonthlySpendLimit, WithAPIKeySpendCap) share the
same unlocked check-then-act shape against counters bumped in the same
DebitOrgCredits statement.

What #769 actually claimed (verified exact text)

"Spend is metered post-response, so overshoot is bounded by in-flight
request cost."

No dollar figure, no N-concurrent analysis — a singular/organic framing
only. This claim is false under any real concurrency: overshoot is bounded
by N × in-flight request cost, where N is not bounded by anything in this
code path.

(The ~$6.75 used in the fixture below is not from #769 — it is
auditor-chosen catalog math: 1M input @ $3/MTok + 250K output @ $15/MTok.)

Reproduction (three independent runs, real Postgres, real code)

Fixture: $1.00 org monthly cap, $0.90 already spent this month, N=20
concurrent calls to the real CheckOrgMonthlySpend → DebitForInference
path, each costing $6.75 (1M input tokens @ $3/M + 250K output tokens @
$15/M).

Run	Preflights passed	Final spend	Overshoot
Original audit	20/20	$135.90	$134.90 (~20x)
Independent second pass	20/20	$135.90	$134.90 (~20x)
Local reproduction (this filing)	13/20	$88.65	$87.65 (~88x on the $1 cap)

The exact number varies run to run (timing-dependent — how many goroutines
land inside the race window before any debit commits), but the pattern is
consistent and reproduces every time: multiple requests clear the
preflight check before any debit lands, and every one that clears it
debits in full regardless of how many others already did.

Why this isn't the same as #478 or #564

• billing: no balance floor or concurrency guard — adversarial burst can drive balance arbitrarily negative with tokens already served #478 is the prepaid credit balance TOCTOU (organization_credit_balance,
  balance_check.go) — a different table, different product surface, and
  (per the query comment above) an acknowledged, intentionally-bounded
  dip, not a hard cap. This issue is about organization_spend_limits /
  organization_monthly_spend — a feature explicitly documented (migration
  0037) as a hard cap, with no equivalent "this is expected to dip"
  language anywhere.
• RotateAPIKey silently drops spend_cap_usd_micros on the replacement key — no API or UI path exists to restore it #564 is about key rotation resetting a spend counter — an admin-path
  bug, unrelated to this request-path race.
• No open issue or PR tracks concurrent overshoot on the spend-limit
  tables specifically.

Real-world plausibility

No per-org or per-key inference concurrency limiter exists anywhere in this
codebase. The only relevant limit found (MaxConns=6) is a database
connection pool size, not a request-concurrency cap. Sub-agent parallelism
(Explore/Task dispatch) is a normal, designed usage pattern for this
router — concurrent requests from a single org/session are not a contrived
edge case.

One caveat: if the prepaid WithBalanceCheck middleware 402s first (org
has insufficient prepaid balance), this spend-limit path never runs, so
exposure is highest for orgs with sufficient prepaid balance/override but a
tight monthly cap they intend as a hard ceiling.

Suggested fix

#478's circuit-breaker approach (bounding concurrent in-flight requests
against a resource with a hard limit) applies cleanly in spirit, but the
implementation will differ: the monthly-spend table has a monthly reset and
the per-key table has a lifetime cap, so whatever mechanism is chosen
(advisory lock keyed on org+month, SELECT ... FOR UPDATE on the spend
row, or an in-process semaphore per org) needs to account for those reset/
lifetime semantics rather than reusing #478's fix verbatim.

Duplicate check

Searched all open/closed issues and PRs for spend-limit concurrency
overshoot specifically (not the prepaid-balance TOCTOU, which is #478).
None found.