security: make money side effects atomic with their guards (ledger integrity)#94
Merged
Conversation
…tegrity) Closes three transaction-atomicity findings from the 2026-07-05 review whose shared root cause was a money side effect committing in a different transaction than the marker guarding it: - Webhook (F2/F5/F6): claim + credit/tier/refund now commit in one db.immediate transaction (commit=False threaded through mark_stripe_event/add_ledger/ set_org_tier); a crash between claim and credit no longer loses the credit, and concurrent refund/dispute events for one charge can't double-reverse. Rollback on error replaces the manual unmark. - /v1/usage idempotency (#4): the response/status is stored inside the same db.immediate block as the debits, so a crash can't leave committed events behind a reclaimable NULL-status claim that a retry would re-record (double-debit). - Hard-stop (#14): decided in integer micro-dollars, not float USD. 263 tests pass (+5: webhook apply-failure rollback, hard-stop micro boundary, idempotent atomic-store + replay-no-double-debit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ledger atomicity — HIGH #4/#5 + MED #6 + LOW #14 (2026-07-05 security review)
The most important PR for billing correctness. Shared root cause of #4/#5/#6: a
money side effect committed in a different transaction than the marker
guarding it, leaving a crash window.
mark_stripe_eventcommitted the event claimseparately from the credit
add_ledger. A crash between them + Stripe's retry→ event seen as "duplicate", credit never applied (silent credit loss);
concurrent refund/dispute events for one charge could also double-reverse.
handle_webhook_eventnow wraps the claim + all appliers in onedb.immediate(
BEGIN IMMEDIATE) transaction,commit=Falsethreaded throughmark_stripe_event/add_ledger/set_org_tier. Any error rolls back the wholething (claim included) → clean retry; the manual
unmarkis no longer needed./v1/usageidempotency double-debit — the batch committed but thekey's status flipped in a later separate commit; a crash in between left
committed events behind a NULL-status claim that the 120s reclaim deleted,
letting a retry re-record the batch. The response is now stored inside the same
db.immediateblock as the debits (extracted_usage_responsehelper).(
get_balance_micros/usd_to_micros), not float USD.Tests
263 pass (
pytest tests/ -q), +5 new intest_txn_atomicity.py: webhookapply-failure rolls back the claim + balance (then retry credits once), duplicate
event credits once, hard-stop micro boundary (drains to exactly 0 then blocks),
idempotent response stored with non-NULL status, retry replays without re-debit.
No behavior change to the existing 258 (webhook-reversal + money-cluster suites
still green).