mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-30 03:00:41 -04:00
fix(backend/credit): prevent double-application of transactions due to race condition (#10672)
<!-- Clearly explain the need for these changes: --> ## 🚨 CRITICAL: Double Transaction Bug **Critical Issue:** Top-up transactions were being applied TWICE to user balances, causing severe accounting errors. **Example:** - User with $160 balance tops up $50 - Expected: $210 balance - Actual: $260 balance (extra $50 incorrectly credited) This compromises the financial integrity of our credit system and requires immediate fix. ### Changes 🏗️ 1. **Added double-checked locking pattern in `_enable_transaction`** (backend/data/credit.py) - Added transaction re-check INSIDE the locked transaction block (lines 294-298) - Prevents race condition when concurrent requests try to activate the same transaction - Ensures transaction can only be activated once, even with webhook retries 2. **Enhanced error messages in Stripe webhook handler** (backend/server/routers/v1.py) - Added detailed error messages for better debugging of webhook failures - Helps identify issues with payload validation or signature verification ### Root Cause Analysis 🔍 **TOCTOU (Time-of-Check to Time-of-Use) Race Condition:** The original code checked `transaction.isActive` outside the database lock. Between this check and acquiring the lock, another concurrent request (webhook retry or duplicate) could enter, causing both to proceed with activation. **Sequence:** 1. Request A: Checks `isActive=False` ✅ 2. Request B: Checks `isActive=False` ✅ (webhook retry) 3. Request A: Acquires lock, activates transaction, adds $50 4. Request B: Waits for lock, then ALSO adds $50 ❌ **Contributing Factors:** - Stripe webhook retry mechanism - `@func_retry` decorator (up to 5 attempts) - No database-level unique constraint on active transactions - Missing atomicity between check and update ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: <!-- Put your test plan here: --> - [x] Verified the double-check prevents duplicate transaction activation - [x] Tested concurrent webhook calls - only one succeeds in activating transaction - [x] Confirmed balance is only incremented once per transaction - [x] Verified idempotency - multiple calls with same transaction_key are safe - [x] All existing credit system tests pass - [x] Tested webhook error handling with invalid payloads/signatures #### For configuration changes: - [x] `.env.example` is updated or already compatible with my changes - [x] `docker-compose.yml` is updated or already compatible with my changes - [x] I have included a list of my configuration changes in the PR description (under **Changes**) *Note: No configuration changes required - this is a code-only fix*
This commit is contained in:
@@ -286,11 +286,17 @@ class UserCreditBase(ABC):
|
||||
transaction = await CreditTransaction.prisma().find_first_or_raise(
|
||||
where={"transactionKey": transaction_key, "userId": user_id}
|
||||
)
|
||||
|
||||
if transaction.isActive:
|
||||
return
|
||||
|
||||
async with db.locked_transaction(f"usr_trx_{user_id}"):
|
||||
|
||||
transaction = await CreditTransaction.prisma().find_first_or_raise(
|
||||
where={"transactionKey": transaction_key, "userId": user_id}
|
||||
)
|
||||
if transaction.isActive:
|
||||
return
|
||||
|
||||
user_balance, _ = await self._get_credits(user_id)
|
||||
await CreditTransaction.prisma().update(
|
||||
where={
|
||||
|
||||
@@ -458,12 +458,16 @@ async def stripe_webhook(request: Request):
|
||||
event = stripe.Webhook.construct_event(
|
||||
payload, sig_header, settings.secrets.stripe_webhook_secret
|
||||
)
|
||||
except ValueError:
|
||||
except ValueError as e:
|
||||
# Invalid payload
|
||||
raise HTTPException(status_code=400)
|
||||
except stripe.SignatureVerificationError:
|
||||
raise HTTPException(
|
||||
status_code=400, detail=f"Invalid payload: {str(e) or type(e).__name__}"
|
||||
)
|
||||
except stripe.SignatureVerificationError as e:
|
||||
# Invalid signature
|
||||
raise HTTPException(status_code=400)
|
||||
raise HTTPException(
|
||||
status_code=400, detail=f"Invalid signature: {str(e) or type(e).__name__}"
|
||||
)
|
||||
|
||||
if (
|
||||
event["type"] == "checkout.session.completed"
|
||||
|
||||
Reference in New Issue
Block a user