fix(kyc): stop selling a bank ID check to residences no bank onboards - #2947
Conversation
The six bank entry points derived the Sumsub level from the DESTINATION country, so a UK, Japanese or sanctioned resident tapping "Withdraw → Spain → SEPA" was routed to `bridge-requirements` and walked through a full document check that could only end on a terminal rejection. Nothing on that path consulted the residence they declared at signup, even though the server has been returning `residenceRestrictions` all along. The two guards that exist both read post-hoc signals and so never fire pre-KYC: `isRegionRestricted` needs a Sumsub rejection to have happened, and the capability model's UK block reads `sumsubGeo`, which is null until approval. A restricted resident therefore reads as plain `needs-identity` and gets offered "Unlock now". - useBankRegionIntent keeps the destination in charge — a Brazilian resident really does need the Bridge level for a SEPA destination — and forces ROW only when every residence we know rules bank rails out. It reads `banking` rather than the country code, so it inherits the server's answer and the dual-resident intersection for free. - InitiateKycModal decides the same thing once, next to the outage and region-restricted short-circuits, so a future call site cannot miss it. It yields to `region-unavailable`, whose UK copy is more specific than the new country-neutral pair. - The Get-started checklist stops naming the ID check as the price of a bank rail these residences cannot have, the same ruling the signup residence step already makes for its congrats copy.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code-analysis diffPainscore total: 7064.01 → 7052.99 (-11.02) 🆕 New findings (93)
…and 73 more. ✅ Resolved (92)
…and 72 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
The Bridge onramp page renders without a redux Provider in this suite, so useResidenceRestrictions' useSetupStore threw for all of GROUP 5.
Four branches each re-checked the residence; one resolved variant reads the same and drops the component back under its prior complexity.
🖼 Visual diff — 1 screen moved1 of 60 shots changed · 59 identical · baseline
new screens (1)
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
qr-pay never reads the identity verdict — every state it shows is derived from rail status — so a user Sumsub refused on jurisdiction owns no Manteca rail at all, falls through to REQUIRES_IDENTITY_VERIFICATION and is offered the same verification that just turned them down, indefinitely. It now short-circuits on `isRegionRestricted` above every rail-derived state and renders the one honest ending the drawer and InitiateKycModal already give. Deliberately keyed on the post-hoc Sumsub signal and NOT on residence: the QR pool is residence-agnostic by design (rails.consts.ts — any Sumsub-approved user can pay via the corporate account regardless of where they live), so gating it on `banking` would strip QR pay from Japanese, Algerian, Burundian, Tunisian and UK residents who legitimately have it. Only a jurisdictional document refusal makes the offer dishonest here. The add-money / withdraw country list keeps every entry — the residence sets are advisory, and hiding the list would leave a near-empty screen — but names the restriction once above it instead of letting the user find out three taps later. Mirrors the note Unlock payments already carries.
…C chrome A sibling wrapper meant a second className site in a page file, which the ds-lint ratchet counts. The state belongs in needsKycVerification anyway — the modals there already self-gate on visible.
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Two defect classes from the four prior findings remain: the direct bank-claim path still starts verification for bank-restricted residences, and page presentation still hides the bank-unavailable title.
Findings
-
MAJOR · src/components/Claim/Link/views/BankFlowManager.view.tsx:325 · Block the direct bank-claim verification path
ReceiverKycNeededsubmits through this branch without renderingInitiateKycModal, so a resident withbanking === truestill calls/users/identity.bankRegionIntentonly changes the argument toROW; the backend mapsROWto the general Sumsub level and returns a token, so the SDK still opens for a bank claim that this residence cannot complete. Gate this branch on the residence restriction and render the same bank-unavailable ending before callinghandleInitiateKyc; add a regression test asserting that the SDK handler is not called. -
MINOR · src/components/Kyc/InitiateKycModal.tsx:118 · Show the unavailable title in page presentation
On the add-money bankstep=verifypage, the pre-KYC prop variant isdefault, then this code resolves it tobank-unavailable. Page rendering still computestitleIsGenericfrom the rawvariant, so it suppressesgetTitle()and leaves the genericUnlock paymentsnav title above the unavailable description and CTA. ComputetitleIsGenericfromresolvedVariantand coverpresentation="page"withbanking: true. -
MAJOR · src/components/Claim/Link/views/BankFlowManager.view.tsx:325 · Block the direct bank-claim KYC path
ReceiverKycNeededsubmits through this branch without renderingInitiateKycModal, so a resident withbanking === truestill calls/users/identity.bankRegionIntentonly changes the argument toROW; the backend mapsROWto the general Sumsub level and returns a token, so the SDK still opens for a bank claim that this residence cannot complete. Gate this branch on the residence restriction and render the same bank-unavailable ending before callinghandleInitiateKyc; add a regression test asserting that the SDK handler is not called. -
MINOR · src/components/Kyc/InitiateKycModal.tsx:118 · Show the unavailable title in page mode
On the add-money bankstep=verifypage, the pre-KYC prop variant isdefault, then this code resolves it tobank-unavailable. Page rendering still computestitleIsGenericfrom the rawvariant, so it suppressesgetTitle()and leaves the genericUnlock paymentsnav title above the unavailable description and CTA. ComputetitleIsGenericfromresolvedVariantand coverpresentation="page"withbanking: true. -
MAJOR · src/i18n/app/messages/es-AR.json:1064 · [moonshotai/kimi-k3] es-AR catalog missing kyc.initiate.titleBankUnavailable
InitiateKycModal now calls t('initiate.titleBankUnavailable') for the new bank-unavailable branch, and en, es-419 and pt-BR all add the key — but es-AR only adds descriptionBankUnavailable. next-intl throws on a missing message key at render time, so an es-AR user whose residence restricts banking (exactly the users this PR targets) hits a runtime error instead of the 'Not available in your country' screen. Fix: add "titleBankUnavailable": "No disponible en tu país" (or equivalent) to kyc.initiate in es-AR.json. -
MINOR · src/constants/residence.consts.ts:31 · [claude-opus] Banking-restricted residence list misses Guinea-Bissau
This PR makes BANKING_RESTRICTED_RESIDENCE_ISO2 the authority that decides whether Peanut sells a bank ID check, across six bank entry points plus the InitiateKycModal choke point and useBankRegionIntent (whose docblock says it covers "the sanctioned set, the UK rule, Bridge's banking exclusions"). The list holds DZ, BI, JP, TN.
Product truth names six countries where Bridge onboards but no rail is ever Yes: product/providers/fiat/eligibility.md:463-464 and :1027-1029 — "Algeria, Burundi, China, Guinea-Bissau, Japan, Tunisia", repeated at :1539-1541 and :1966-1967. The code covers five of them (CN sits in RESTRICTED_RESIDENCE_ISO2), and omits GW. eligibility.md:1029 explains exactly how this happens: Bridge's prose note omits Guinea-Bissau while Bridge's own table includes it, and the code mirrored the prose.
The code is the wrong side here. A Guinea-Bissau resident keeps getting the outcome this PR set out to remove: the checklist row still names "Bank transfer or crypto · bank needs a one-time ID check", InitiateKycModal still offers "Unlock now", useBankRegionIntent still returns EU/NA, and the Sumsub run ends on a rail that can never enable.
The gap predates this PR and both repos agree (peanut-api-ts src/kyc/residence-restrictions.ts:53 has the same four), so nothing breaks cross-repo. Fix: add 'GW' to BANKING_RESTRICTED_RESIDENCE_ISO2 here and to BANKING_RESTRICTED_RESIDENCE in peanut-api-ts — the API-served /config/residence-restrictions list is what production actually reads, so the backend half is the one that takes effect without a release.
Checked clean
- Confirmed the detached worktree HEAD, PR author, base ref, base SHA, and head SHA match the supplied review target.
- Rechecked P1 and P3 through the ReceiverKycNeeded submit path and the sibling API's ROW-to-general verification mapping; both remain present.
- Rechecked P2 and P4 through the add-money bank verify page and page-presentation title logic; both remain present.
- Reviewed every changed production file and associated tests across bank entry points, QR pay, method-list messaging, checklist copy, restriction intent mapping, and translations.
- Exact-head CI checks were successful or intentionally skipped; no PR-caused failing gate was present.
- Security and adversarial passes found no new secret exposure, injection surface, privilege change, or unsafe workflow behavior beyond the reported correctness paths.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: abfc5f84d2f2 · Context: repo, sibling-api, ci · Took 12m (queued 9m)
Chip found two live defects in the residence guard. The bank-claim form submits through handleSuccess straight into the SDK, never rendering InitiateKycModal, so the choke point could not see it: a bank-restricted resident still got a general-level token for a verification their residence can never turn into a rail. That branch now refuses and hands over to the modal. Which meant hoisting the modal: it was rendered only by the confirm step, so setting its state from the form step would have shown nothing at all — a silent no-op on submit, worse than the wrong SDK. It is now defined once and rendered by both steps, and the new test fails without that. Page presentation computed titleIsGeneric from the caller's variant, so a resolved bank-unavailable ending kept the generic "Unlock payments" nav title above an unavailable description. It reads the resolved variant now. Not taken: the es-AR missing-key finding is wrong — es-AR is a deltas overlay on es-419 (loadMessages/deepMerge), so titleBankUnavailable resolves through it, and the title needs no voseo delta. The diff-only reviewer could not see the overlay; the catalog-parity test already covers this and passes.
Bridge's table blanks every rail for GW; the prose note on the same page omits it, and this list mirrored the prose (product/providers/fiat/ eligibility.md is explicit that the table is the authority). Harmless while the set was advisory display, but this PR makes it the authority for whether we sell a bank ID check — so a GW resident was still getting the outcome the PR exists to remove. peanut-api-ts BANKING_RESTRICTED_RESIDENCE needs the same entry; that is the copy production actually serves, and this one is only the bundled fallback.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The earlier direct-claim and page-title findings are fixed. Two major gaps remain: an enabled QR rail bypasses the new regional terminal state, and Guinea-Bissau is absent from the server-authoritative banking restriction set.
Findings
-
MAJOR · src/app/(mobile-ui)/qr-pay/page.tsx:287 · Check regional rejection before allowing an existing QR pay rail
A user can already own an ENABLED QR-pool rail from a prior Sumsub approval; a residence-change or reverification can later change the Sumsub identity row to REJECTED without revoking that user rail. In that statecanDo('pay', { provider: 'manteca' })returns before this new branch, so the terminal screen is skipped and the payment path remains available. CheckisRegionRestrictedbefore the enabled-pay return and add a case with both a jurisdictional rejection and an enabled pay rail. -
MAJOR · src/constants/residence.consts.ts:36 · Add Guinea-Bissau to the server-authoritative restriction set
GWis added only to the bundled fallback, whileuseResidenceRestrictionsgives/users/me.residenceRestrictionsprecedence and the public config replaces this set after load. The current dev/staging API still returnsbankingOnly: [BI, DZ, JP, TN], so an authenticated Guinea-Bissau resident resolvesbanking: false; every new guard and notice disappears and the bank KYC offer remains. Update the server's canonical residence set before or with this head and cover the server-authoritative GW case. -
MAJOR · src/i18n/app/messages/es-AR.json:1064 · [moonshotai/kimi-k3] es-AR catalog missing titleBankUnavailable (and addMoneyRoutesNoBank)
The diff adds kyc.initiate.descriptionBankUnavailable to es-AR.json but never adds the matching titleBankUnavailable, while InitiateKycModal.getTitle() calls t('initiate.titleBankUnavailable') for the bank-unavailable variant. next-intl throws on missing message keys by default, so an es-AR user with a bank-restricted residence hitting any of the eight InitiateKycModal render sites crashes the modal instead of seeing the honest ending — the exact users this PR is for. Same gap for home.checklist.addMoneyRoutesNoBank, which GettingStartedChecklist now renders for restricted residences. Add titleBankUnavailable and addMoneyRoutesNoBank to es-AR.json (en, es-419 and pt-BR all have both). -
MAJOR · src/constants/residence.consts.ts:36 · [claude-opus] GW banking restriction is inert without the paired peanut-api-ts change
BANKING_RESTRICTED_RESIDENCE_ISO2in this repo is only the bundled fallback —useResidenceRestrictionSets.tsdocuments it as "the offline/pre-fetch fallback, never the source of truth" and swaps toGET /config/residence-restrictionsonce it loads, anduseResidenceRestrictionsprefersuser.residenceRestrictionsfrom/users/me(user?.residenceRestrictions ?? deriveResidenceRestrictionsFrom(...)) over any local derivation. Both of those come from peanut-api-tssrc/kyc/residence-restrictions.ts:53, whereBANKING_RESTRICTED_RESIDENCEis stillnew Set(['DZ','BI','JP','TN'])— no GW; the route that serves the tier lists issrc/routes/config/residence-restrictions.ts:35. Failure scenario: a logged-in Guinea-Bissau resident getsresidenceRestrictions.banking === falsefrom the API, souseBankRegionIntentkeeps returning the destination intent,InitiateKycModalkeeps rendering the "Unlock now" offer, the checklist keeps naming the bank half, and the BankFlowManager refusal never fires — the exact sale this PR exists to stop. The pre-account signup window is no different, because the server config list (also GW-less) replaces the mirror as soon as it arrives. Nothing in peanut-api-ts breaks on merge; the UI half is simply a no-op for GW. This normally ships as a pair, and the API-side addition of'GW'toBANKING_RESTRICTED_RESIDENCEis likely an open PR that this pinned policy-branch checkout cannot see — if that PR exists, this is already handled and only needs a merge-order note (API first, or the UI change is dead on arrival); if it does not, GW needs to be added there before this claim is true in production. -
MINOR · src/constants/residence.consts.ts:25 · [claude-opus] The new GW row claims the card still works; the authoritative card denylist says it does not
The added GW entry lands in the banking-only tier, soderiveResidenceRestrictions('GW')returns{ banking: true, card: false }— asserted as expected in the new test case insrc/hooks/__tests__/useResidenceRestrictions.test.tsxand reinforced by the copy this PR adds (descriptionBankUnavailable: "you can still send and receive money with other people"), while the residence step keeps offering the card. product/support-answers/card-country-restrictions.md (last_validated 2026-08-17, residence-based, from Aleks 2026-07-04 with GH/NG removals applied) listsGWin the restricted set and says to treat that list as authoritative. Failure scenario: a Guinea-Bissau resident is told bank transfers are unavailable but is still walked into a card application the issuer refuses on residence, so the honest ending this PR is trying to give them is only half honest. The code is the wrong side here, but note the gap is systemic and pre-existing rather than introduced by this diff: that doc's list has 53 entries and bothCARD_RESTRICTED_RESIDENCE_ISO2here andCARD_RESTRICTED_RESIDENCE/RAIN_PROHIBITED_COUNTRIESin peanut-api-ts carry only the same 9-country subset, and the doc itself flags product/card.md as stale. Fix is either to addGWto the card tier in both repos alongside the banking one, or to leave it and reconcile the whole list separately — but the GW row is the one this PR is ruling on, so it is worth deciding here.
Checked clean
- P1/P3 fixed: the direct bank-claim form now stops restricted residences before invoking Sumsub and mounts the unavailable modal in the form step.
- P2/P4 fixed: page presentation derives heading visibility from the resolved unavailable variant.
- All six bank KYC entry points route through useBankRegionIntent; destination precedence and residence intersections were checked.
- Locale catalogs and es-AR fallback were checked; all new keys resolve.
- Exact-head CI checks for analysis, review, bot approval, and Vercel comments succeeded; Deploy Preview was still in progress.
- Security boundaries and money-moving claim changes were inspected; no additional defect was found.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 2 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 35c791176a5f · Context: repository, product, api-contract, lexicon, provider-docs, ci · Took 8m (queued 11m)
…tcut
Nothing revokes a pool rail granted by an earlier approval, so a residence
change that re-verifies into a region rejection leaves an ENABLED rail
behind. With `canDo('pay')` ranked first, that user kept the money path open
on an identity we can no longer verify, and never saw the terminal screen
every other surface gives them.
Deliberately unlike deriveGate's ready-wins hoist: that exists to stop a
STUCK SIBLING rail from blocking a working one (the 2026-06-01 guard), and a
terminally refused identity is not a sibling rail. Flagging it here because
it is the one ordering in this PR that closes a path rather than opening an
honest ending, so it is a compliance-leaning call worth a second opinion.
1 participant
The bug
The six bank KYC entry points derived the Sumsub region intent from the destination country, never from the user's residence:
AddWithdrawCountriesList.tsxadd-money/[country]/bank/page.tsxwithdraw/[country]/bank/page.tsxBankFlowManager.view.tsxSo a UK, Japanese or sanctioned resident tapping Withdraw → Spain → SEPA was routed to
bridge-requirements— a Bridge level that, for their residence, can only end on a terminal rejection. They uploaded a document and did liveness before anything told them.The residence-aware helper already existed and was already correct (
regionIntentForResidence, routingRESTRICTED_RESIDENCE_ISO2andBANKING_RESTRICTED_RESIDENCE_ISO2toROW) — it just had no call site on this path.Why the existing guards never fired
Both read post-hoc signals:
isRegionRestrictedisstatus === 'failed' && reason.code === 'identity_region_restricted'— it needs a Sumsub rejection to have already happened, so theInitiateKycModalchoke point could not help a user who had not yet been rejected.sumsubGeo(peanut-api-tskyc/capabilities/load.ts), which is null until KYC approves.Pre-KYC a restricted resident therefore has no rail to carry
uk_resident_blockedand no rejection to flag, reads as plainneeds-identity, and gets offered "Unlock now". Meanwhileuser.residenceRestrictions— the server's own answer, derived from the KYC-reported residence or the one declared at signup — was sitting right there unused.The change
1.
useBankRegionIntent— the destination stays in charge, because it is what decides which provider's rails the verification opens; a Brazilian resident really does need the Bridge level to withdraw to a Spanish IBAN. It forcesROWonly when every residence we know rules bank rails out.It reads
useResidenceRestrictions().bankingrather than the country code, which buys three things for free: the server's answer wins over the local declaration, the KYC-reported residence wins over the declared one, and the dual-resident intersection applies — someone who declared GB and BR keeps the destination intent, because a bank level is still winnable for them.2.
InitiateKycModal— the same decision made once, next to the outage and region-restricted short-circuits, so a future call site cannot miss it. All eight render sites are bank/Manteca money-movement gates, so no escape hatch is needed. Ordering: outage > region-restricted (a document-jurisdiction block is the more specific ending) >region-unavailable(the UK copy is more specific than the new country-neutral pair) > this.New copy in all four catalogs, country-neutral for the same reason
restricted-regions.tsgives — one string covers every restricted residence, and compliance changing the list needs no re-translation. CTA reusesuseRegionRestrictedCta: no retry, no contact-support (nobody can lift a residence block), just the part of the app that still works.3. Get-started checklist — decided yes, drop it. The row's subtitle read "Bank transfer or crypto · bank needs a one-time ID check" for everyone, naming the ID check as the price of a rail these residences cannot have. It now reads "Crypto from any wallet or exchange". This follows the ruling the signup residence step already makes for its congrats copy: where no rail exists, the ID-check clause is dropped entirely rather than promising a rail verification cannot deliver.
The two surfaces the guard above does not reach
qr-payrenders its own CTAs rather thanInitiateKycModal, so the choke point never covered it — and it derives every state from rail status, never reading the identity verdict. A user Sumsub refused on jurisdiction owns no Manteca rail at all, so the candidate list is empty, they fall through toREQUIRES_IDENTITY_VERIFICATION, and are offered the verification that just turned them down. Indefinitely.It now short-circuits on
isRegionRestrictedabove every rail-derived state, rendering the same ending the drawer andInitiateKycModalalready give.Keyed on the post-hoc Sumsub signal and deliberately not on residence. The QR pool is residence-agnostic by design (
rails.consts.ts: "any Sumsub-approved user can pay QR codes via the corporate pool account, regardless of residence"), and the resolver's UK block only touchesprovider === 'bridge'— so Japanese, Algerian, Burundian, Tunisian and UK residents all legitimately hold QR pay. Gating this onbankingwould have removed a capability those users really have. Only a jurisdictional document refusal makes the offer dishonest here, and that list stays in Sumsub whererestricted-regions.tsinsists it belongs.The add-money / withdraw country list keeps every entry — the residence sets are advisory offer-shaping, and hiding the list would leave a near-empty screen — but names the restriction once above it, instead of letting the user discover it three taps into a flow that cannot finish. Mirrors the note Unlock payments already carries.
Still not in scope — worth a separate task
BANKING_RESTRICTED_RESIDENCE(DZ/BI/JP/TN) has no backend enforcement at all. No capability block analogous to the UK one, andPOST /users/identitynever consults residence — it mintsverificationLevelForIntent(regionIntent)verbatim. The residence-awarecanonicalIntentForResidenceexists inkyc/residence-intent.tsbut is only wired intorestart-identity.ts. These residents completebridge-requirementsand land on a raw Bridge provider rejection → "contact support", a ticket nobody can resolve. This PR closes the client half; the server still accepts whatever intent an older build sends it.Verification
tsc --noEmitclean;eslintclean on all touched files (remaining warnings are pre-existingfetchUser/useCallbackdep warnings on untouched code, confirmed by stashing);prettier --checkclean;ds-lint --checkat baseline on every ratchet.useBankRegionIntent.test.tsx(8 cases — destination precedence, the three restricted tiers, card-only pass-through, dual-residence softening, server-over-local),InitiateKycModal.residence.test.tsx(6 — including that no path reaches the SDK, and both yield-to orderings), plus one case each for the qr-pay ending and the list notice.useResidenceRestrictionsstub now that the shared modal, the bank pages and the router view read it.