Repository navigation
fix(kyc): a verification session must not lift a control it does not own - #311
Conversation
create_verification_session wrote wallet.status = "pending_kyc"
unconditionally. pending_kyc is a SPENDABLE status -- it is absent from the
non-spendable set every debit path checks -- so an ordinary API call on a
frozen, suspended, or closed wallet silently returned it to a spending state.
Confirmed by reproduction, not by reading:
[A] frozen-before-request -> final status = 'pending_kyc'
[C] operator-suspended -> final status = 'pending_kyc'
[D] closed -> final status = 'pending_kyc'
The velocity anomaly auto-freeze is the sharpest case: it exists to stop a
concurrent spend burst, and starting KYC handed the burst its wallet back. No
race was needed. The wallet is also read BEFORE the Stripe network call and
written after, so the same write clobbered a freeze committed while that call
was in flight.
The status is now claimed by a guarded UPDATE that can only take a status KYC
is entitled to: one that is already spendable, or a suspension KYC itself
imposed. handle_rejected and handle_expired suspend the wallet, and the
success path only reactivates from pending_kyc, so that exception is load
bearing -- without it every rejected sponsor would be stranded in a state they
could never verify out of. A freeze or a closure is not KYC's to lift: those
wallets still get their session and still get their kyc_status updated by the
webhook, but stay exactly as non-spendable as they were.
Four of the seven new tests fail against the unfixed source; the other three
are the remediation and ordinary paths, which pass both before and after and
are there to prove the fix did not buy safety by breaking recovery.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FsXYfTHDLiFNQ24R5JPN7H
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 50 minutes Limit details: You’ve used all 2 included reviews currently available. Your 80 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
KYCService.create_verification_sessionwrotewallet.status = "pending_kyc"unconditionally (app/services/kyc_service.py:161).pending_kycis a spendable status — it is absent from the non-spendable set every debit path checks:So an ordinary API call on a frozen, suspended, or closed wallet silently returned it to a spending state. No race required.
I confirmed it by running it, not by reading it:
The velocity anomaly auto-freeze is the sharpest case. It exists to stop a concurrent spend burst — and starting KYC handed the burst its wallet back. #305 and #308 spent considerable effort making sure a freeze landing mid-charge cannot be outrun by a debit; this let the same freeze be lifted through the front door.
Secondarily, the wallet is read before the Stripe network call and written after, so the same write clobbered a freeze committed while that call was in flight.
The fix
The status is now claimed by a guarded
UPDATEthat can only take a status KYC is entitled to:That second clause is load bearing, not a hedge.
handle_rejectedandhandle_expiredsuspend the wallet themselves (:460,:416), and the success path only reactivates frompending_kyc(:352-353). Without the exception, every sponsor whose verification was rejected would be stranded in a state they could never verify out of — the fix would have bought safety by breaking recovery.A freeze or a closure is not KYC's to lift. Those wallets still get their session, and their
kyc_statusis still updated by the webhook; onlystatusis left alone.Type of Change
Validation
pytest -q— 1740 passed, 32 skippedmake trust-release-gate— passedruff check .clean;mypyclean onkyc_service.pyscripts/check_doc_references.py— 276 references resolveFour of the seven new tests fail against the unfixed source — the three control bypasses and the freeze-during-the-Stripe-call race. The other three (the two remediation paths and the ordinary path) pass both before and after by design: they are there to prove the fix did not achieve safety by breaking recovery.
Risk
One service, one write. No migration. The behavioural change is precisely that a frozen, operator-suspended, or closed wallet no longer becomes spendable by starting KYC.
Worth a reviewer's attention: a caller who relied on KYC clearing a freeze now sees the wallet stay frozen. That reliance was the vulnerability, but if any runbook documents "start KYC to clear a freeze", it needs updating to an explicit operator unfreeze.
Provenance
Found by a subagent during an adversarial audit of the money paths, then reproduced and verified independently before any code was changed.
Generated by Claude Code
Note
Medium Risk
Changes authorization around wallet spendability in the KYC session path—security-adjacent money controls—but scoped to one guarded write with tests; callers who incorrectly used KYC to clear freezes will see different behavior.
Overview
Fixes a wallet-status bypass where
create_verification_sessionalways setpending_kyc, a spendable status, so frozen, operator-suspended, or closed wallets could become spendable again by starting KYC—with no race required.The status transition is now a guarded SQL
UPDATE: only wallets that are already spendable, or suspended withkyc_statusrejected/expired (suspensions KYC itself imposed), move topending_kyc. Frozen, unrelated suspensions, and closed wallets still get a Stripe session and verification row, butstatusstays unchanged; an info log records when the transition is skipped.Race with Stripe: because the wallet was read before the Stripe call and written after, a freeze committed during that call could also be overwritten; the conditional update uses the row at write time so that freeze survives.
New tests in
test_kyc_status_authority.pycover the three non-KYC controls, KYC-owned suspension recovery, the normal active path, and freeze-during-Stripe.Reviewed by Cursor Bugbot for commit c3cdffa. Configure here.