fix: add transfer mutex to prevent TOCTOU race in transfer_credits (CWE-367)

[quangtran88]

Summary

Fixes critical CWE-367 TOCTOU race condition in the transfer_credits tool where the balance check and transfer execution were not atomic.

Changes

• Add lightweight promise-based mutex (withTransferLock) to serialize credit transfer operations
• Wrap the balance-check-and-transfer sequence in the lock
• Zero external dependencies — pure promise chaining
• Only serializes transfer operations; other tool calls remain fully concurrent

Security Impact

Before: Concurrent transfer_credits calls could both pass the "max half balance" guard before either transfer executes, allowing an agent to drain its balance beyond the safety limit:

T1: balance = $100, amount = $40 → passes guard (40 < 50) ✓
T2: balance = $100, amount = $40 → passes guard (40 < 50) ✓
T1: transfers $40 → balance now $60
T2: transfers $40 → balance now $20 (violated 50% limit!)

After: Transfer operations are serialized — T2 would see the updated $60 balance and correctly enforce the guard.

Note: Server-side atomic transfers would be the ideal long-term fix. This client-side mutex is a pragmatic defense-in-depth measure.

Closes #177

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

[devin-ai-integration]

🔴 fund_child bypasses transfer mutex, preserving the TOCTOU race the PR aims to fix

The fund_child tool at src/agent/tools.ts:1416-1425 performs the same balance-check-then-transfer pattern as transfer_credits, but does not use withTransferLock. This means concurrent transfer_credits and fund_child calls (or two fund_child calls) can still race, defeating the purpose of the mutex.

Root Cause

The PR adds withTransferLock to transfer_credits (line 884) to serialize the balance check (getCreditsBalance()) and the actual transfer (transferCredits()). However, fund_child at lines 1416-1425 performs the exact same TOCTOU-vulnerable sequence without the lock:

// fund_child — no lock!
const balance = await ctx.conway.getCreditsBalance();  // line 1416
if (amount > balance / 2) { ... }                      // line 1417
const transfer = await ctx.conway.transferCredits(...); // line 1421

If transfer_credits and fund_child are called concurrently (e.g., the agent issues both tool calls in a single turn), both can read the same balance, both pass the "half balance" guard, and both transfers execute — potentially draining more than intended.

Impact: The self-preservation guard ("don't transfer more than half your balance") can be bypassed via concurrent fund_child + transfer_credits calls, which is exactly the class of bug this PR is supposed to fix.

(Refers to lines 1416-1425)

Prompt for agents

In src/agent/tools.ts, the fund_child tool (around lines 1395-1455) performs a balance check followed by a credit transfer without using the withTransferLock mutex. Wrap the balance-check-and-transfer section (lines 1416-1455, from `const balance = await ctx.conway.getCreditsBalance()` through the return statement) inside a `withTransferLock(async () => { ... })` call, similar to how transfer_credits does it at line 884. The early validation checks (child lookup, wallet validation, status check, amount validation) can remain outside the lock since they don't involve the balance race. The return value of the execute function should be `return withTransferLock(async () => { ... })` encompassing the balance check, transfer, transaction recording, funded amount update, lifecycle transition, and return message.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[quangtran88]

Good catch — fund_child had the same TOCTOU-vulnerable pattern.

Fixed in ba94c3c: wrapped the balance-check-through-return sequence in fund_child with withTransferLock, same as transfer_credits. CI is passing (typecheck + tests on Node 20 & 22).