mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-02-10 14:55:16 -05:00
312cb0227f
<!-- 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*