fix(trust): close all twelve remaining audit findings — no money write from a stale read

CHANGELOG.md

@@ -317,6 +317,66 @@ full release gate; do not backfill a final `v1.2.0` tag.
 
 ### 🔒 Security
 
+- **The child-wallet lifetime spend cap is enforced by the database.**
+  `lifetime_debits + charge_amount <= max_spend` was checked against an earlier
+  read while the increment happened later, so concurrent charges could push a
+  delegated wallet past the ceiling its parent granted. The cap now travels
+  with the debit as its own predicate. (Raised by review on #305 — the balance
+  guard and the cap are separate predicates, and fixing one did not carry the
+  other.)
+- **Every wallet money move is now decided by the database too.** The same
+  read-modify-write shape as the permit fixes below, in the paths that hold
+  actual credits. All were serialized only by `SELECT ... FOR UPDATE`, a
+  silent no-op on SQLite:
+  - `charge` read the balance, checked it, and several statements later wrote
+    back `balance - amount` computed from that read. A charge committing in
+    between was **overwritten**: the ledger recorded the debit but the balance
+    never reflected it, so that request was served for free and the books
+    stopped balancing. The debit is now one guarded UPDATE that repeats
+    `balance >= amount` as its own predicate.
+  - `reclaim_child_wallet` read the child's balance, zeroed the child, and
+    credited the parent by the amount it had read. Two concurrent reclaims
+    each credited the parent in full — **minting credits that never existed**.
+    The child's balance is now claimed by a guarded UPDATE before any of it
+    reaches the parent.
+  - Refund credits (`refund_charge`, and the operator repair in
+    `refund_reconciliation`) wrote absolute totals, so one of two concurrent
+    refunds was lost outright — the customer's money.
+  - The velocity counters `hourly_spent` / `daily_spent` accumulated by
+    read-modify-write. Under-counting there does not skew a metric; it is what
+    the spend cap and the anomaly auto-freeze are measured against, so **both
+    controls silently stopped firing** exactly when spend was most concurrent.
+  - The Stripe cumulative-refund clawback wrote a balance derived from an
+    earlier read, against fiat Stripe had already returned.
+  - `refund_reconciliation` also mutated `permit.spent_credits` directly,
+    outside the module that guarantees every such write is guarded. That
+    guarantee is only as good as its weakest caller.
+  - New `app/db/sql_expressions.py` holds the shared clamped-decrement
+    expression, so the safe form is defined once rather than re-derived.
+- **Every permit budget write is now decided by the database, not by a value
+  this process read earlier.** Three paths in `permits.py` wrote
+  `spent_credits` from a stale read, serialized only by `SELECT ... FOR
+  UPDATE` — which is a silent no-op on SQLite, and nothing in this repository
+  forbids SQLite in production. All three are fixed, and each ships a
+  regression test confirmed to fail against the unfixed source:
+  - `reserve_budget` gated on `status` and the cap but **not on expiry**, so a
+    permit past its `expires_at` kept funding work until some other process
+    happened to flip its status — authority outliving its own deadline. The
+    expiry is now in the statement's predicate.
+  - `reserve_budget` also collapsed every guarded-write failure to
+    `permit_budget_exceeded`, telling an operator to top up a permit that had
+    actually been revoked or had expired. It now classifies status, then
+    expiry, then budget, matching `authorize_and_reserve`.
+  - `release_dispatch_budget_once` decided whether it had already run by
+    reading `budget_released_at` and then writing it. Two callers both saw
+    `NULL`, both passed, and the reservation was released **twice**, leaving
+    the permit under-spent and able to exceed its own cap. The checkpoint is
+    now claimed by a guarded UPDATE before any budget moves.
+  - `reconcile_budgets` recomputed an absolute total from receipts and wrote
+    it back unconditionally, erasing any reservation that landed mid-pass. It
+    now commits only where the stored spend still equals what that pass read,
+    and skips rather than clobbering — optimistic concurrency control, with
+    the permit re-examined on the next pass.
 - **The governed AWI HTTP charge is now keyed to its idempotency record.**
   `awi_http_governance.py` called `money.charge()` with no `operation_key`, so
   the debit had neither the `uq_ledger_wallet_operation_key` constraint nor the

app/db/sql_expressions.py

@@ -0,0 +1,33 @@
+"""Reusable SQL expressions for writes that must not lose a concurrent update.
+
+The money paths in this application mutate counters that several requests can
+touch at once. The unsafe form is always the same: read a column into Python,
+compute a new total, write the total back. That is a read-modify-write, and it
+is serialized only by whatever row lock happens to be held — which is nothing
+at all on SQLite, where ``SELECT ... FOR UPDATE`` parses and does nothing.
+
+Expressing the arithmetic against the *column* instead of against a value this
+process read keeps the whole operation relative, so the database applies it to
+whatever is stored at write time.
+"""
+
+from __future__ import annotations
+
+from decimal import Decimal
+from typing import Any, cast
+
+from sqlalchemy import case
+from sqlalchemy.sql.elements import ColumnElement
+
+
+def clamped_decrement(column: Any, amount: Decimal) -> Any:
+    """``max(0, column - amount)``, evaluated by the database.
+
+    The Python spelling of this clamp (``max(Decimal("0"), col - amount)``)
+    has to read the column first, which reintroduces the lost update the
+    relative form exists to prevent.
+    """
+    return case(
+        (cast(ColumnElement[bool], column - amount < Decimal("0")), Decimal("0")),
+        else_=column - amount,
+    )

app/services/awi_http_governance.py

@@ -29,7 +29,7 @@
     IdempotencyReplay,
     get_idempotency_service,
 )
-from app.services.permits import get_permit_service
+from app.services.permits import PermitError, get_permit_service
 from app.services.receipts import get_receipt_service
 
 logger = logging.getLogger(__name__)
@@ -237,7 +237,18 @@ async def complete_awi_http_governed(
     response_payload: dict[str, Any],
     money: AgentMoney | None = None,
 ) -> dict[str, Any]:
-    """Charge wallet, reserve permit budget, write receipt, complete idempotency."""
+    """Reserve permit budget, charge wallet, write receipt, complete idempotency.
+
+    The order is load-bearing and is stated here in the order the code runs.
+    The reservation is taken **first**, so a later failure compensates by
+    releasing it — with one deliberate exception: on ``ChargeCreditMismatchError``
+    the wallet has already been debited, so the reservation is *retained* rather
+    than released. Releasing it there would free budget the caller has in fact
+    spent.
+    An earlier revision of this line named the charge first; anyone reasoning
+    about crash compensation from that reading would have had the direction of
+    the required rollback backwards.
+    """
     if ctx.replay_response is not None:
         return ctx.replay_response
 
@@ -246,7 +257,45 @@ async def complete_awi_http_governed(
     money = money or get_agent_money()
     permits = get_permit_service()
     idem = get_idempotency_service()
-    await permits.reserve_budget(ctx.permit_id, ctx.credits)
+    try:
+        await permits.reserve_budget(ctx.permit_id, ctx.credits)
+    except PermitError as exc:
+        detail = {
+            "error": exc.reason,
+            "message": exc.reason,
+            "tool": ctx.tool_name,
+        }
+        if exc.reason == "permit_write_contended":
+            # Transient: the guarded write lost to contention and exhausted its
+            # retries. Nothing was reserved and nothing was charged, so the key
+            # must stay usable -- *completing* the record here would freeze a
+            # momentary database conflict into a permanent stored denial that
+            # every retry of that idempotency key replays, long after the
+            # contention cleared. Release the key and answer 503 instead.
+            await idem.abandon(
+                wallet_id=ctx.wallet_id,
+                endpoint=ctx.endpoint,
+                idempotency_key=ctx.idempotency_key,
+            )
+            raise HTTPException(
+                status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+                detail=detail,
+            ) from exc
+        # Terminal: a permit that expired or was revoked between validation and
+        # this reservation is a *denial*, not a server fault. Nothing was
+        # reserved and nothing was charged, so the caller gets the same 403
+        # shape the up-front validation would have produced. Without this the
+        # PermitError escaped uncaught and the route answered 500, which tells
+        # an operator the service is broken when in fact the permit did its job.
+        await abort_awi_http_governed(
+            ctx,
+            status_code=status.HTTP_403_FORBIDDEN,
+            error_payload=detail,
+        )
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail=detail,
+        ) from exc
 
     unit_price = DEFAULT_PRICING[ServiceCategory.AGENT_COMMS][1]
     charge_units = ctx.credits / unit_price if unit_price else Decimal("1")