Non-Custodial Partner Program — gate the partner payments API behind an explicit wallet flag - #4508
Open
joshuakrueger-dfx wants to merge 2 commits into
Open
Non-Custodial Partner Program — gate the partner payments API behind an explicit wallet flag#4508joshuakrueger-dfx wants to merge 2 commits into
joshuakrueger-dfx wants to merge 2 commits into
Conversation
GET /v1/kyc/client/payments returns TransactionDetailDto / PaymentWebhookData with customer IBANs, user addresses, on-chain hashes, amounts and rates. The per-user path /kyc/client/users/:id/payments is additional depth defense behind KYC_CLIENT_COMPANY and is gated by the same service flag. Access on the list endpoint was only RoleGuard(CLIENT_COMPANY). That role is granted to every wallet with an address on company login, so setting an address on a partner wallet silently opened the full transaction dump. Add wallet.paymentsApiEnabled (boolean, NOT NULL DEFAULT false) and check it fail-closed on both payment pull paths and on PAYMENT webhooks (KYC webhook types unchanged). Ops enable/disable via admin PUT /wallet/:id; create/update invalidate the wallet cache so an emergency disable is not delayed by the 5-minute cache. The flag is on the GS debug allowlist for support. Backfill: SET paymentsApiEnabled = true WHERE address IS NOT NULL. Why not isKycClient: #2939 (c20190a) introduced CLIENT_COMPANY and widened GET /kyc/client/payments to that role — effective for wallets with isKycClient = false. Backfilling isKycClient = true would exclude exactly those partners. Every wallet that can company-sign-in today (address set) keeps its access; new wallets start false so a newly set address does not open payments by itself. Behaviour change for existing partners (pull path): the list endpoints now apply the same payment webhook consent rule as the push path (webhookConfig.payment True / ConsentOnly / WalletOnly, with consented=false for the own-wallet channel and consented=true when the wallet is on the user's kycClientList). Users that neither channel admits drop out of the result. Partners that previously listed all wallet users regardless of webhookConfig will see fewer rows after deploy.
joshuakrueger-dfx
marked this pull request as ready for review
July 30, 2026 13:23
joshuakrueger-dfx
requested review from
TaprootFreak and
davidleomay
as code owners
July 30, 2026 13:23
The previous gate change filtered GET /kyc/client/payments (and the per-user path) to the same webhook consent set as the push path. That was not the intended behaviour: partners should still see the same row set as before the PR, but without bank accounts, wallet addresses or on-chain identifiers in the response. Pull path: - Remove the consent filter from getAllPayments / getAllUserPayments. paymentsApiEnabled remains the only new access gate on pull. - Introduce PartnerPaymentDto (OmitType of PaymentWebhookData) and strip identifying fields only on the pull responses. Webhook payloads stay on the full PaymentWebhookData / TransactionDetailDto. Breaking change for existing partners on the pull endpoints: - Same transactions as before the gate PR (subject to paymentsApiEnabled). - Response no longer includes: sourceAccount, targetAccount, inputTxId, inputTxUrl, outputTxId, outputTxUrl, depositAddress, chargebackTarget, chargebackTxId, chargebackTxUrl, networkStartTx. - Amounts, assets, fees, status, type, timestamps and dfxReference remain. Webhook PAYMENT push is unchanged (still full payload + its own consent and paymentsApiEnabled checks).
joshuakrueger-dfx
marked this pull request as draft
July 30, 2026 14:18
joshuakrueger-dfx
marked this pull request as ready for review
July 30, 2026 19:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
GET /kyc/client/paymentsandGET /kyc/client/users/:id/paymentsreturnTransactionDetailDto,which carries customer IBANs (
sourceAccount/targetAccount), user addresses, on-chain hashes,amounts and rates.
Access was gated only by
RoleGuard(CLIENT_COMPANY). That role is granted to every wallet that hasan address on company login, so setting an address on a partner wallet silently opened the full
transaction dump for its users — without anyone deciding it. This is the first part of the
Non-Custodial Partner Program: onboarding a partner must not implicitly hand out customer data.
Historical note for reviewers (deliberately not in the code): the guard was widened from
KYC_CLIENT_COMPANYtoCLIENT_COMPANYin c20190a (#2939) for a specific non-KYC-clientpartner. That is why the backfill below keys on
address, not onisKycClient— anisKycClientbackfill would by construction revoke exactly that partner.What
wallet.paymentsApiEnabled(boolean,NOT NULL DEFAULT false), checked fail-closed inboth payment paths before any transaction is loaded.
WHERE "address" IS NOT NULL: every wallet that can authenticate as a company todaykeeps exactly the access it has. Newly created wallets start closed, so a newly set address no
longer opens the payments door by itself.
WebhookType.PAYMENT, so the flag closes bothdoors to the same data. KYC webhook types are unaffected.
WalletServiceinvalidates the wallet cache after create/update, so an emergency revocation takeseffect immediately instead of after up to five minutes.
WalletDtoand the GS debug allowlist, so ops can both set and inspect it.Behaviour change for existing partners
The pull path previously returned every user's transactions regardless of consent. It now applies
the same rule as the webhook path, so partners receive fewer rows than before — only users whose
consent state permits delivery. This aligns the two channels, which previously disagreed about the
same data.
Verification
npm run lintempty,npm run format:checkclean.MIGRATION_TEST_PG, serial):up → down → upwithfixtures covering both dimensions (
address×isKycClient), asserting the full expected row set.down()writes the per-wallet flag state tologbefore dropping the column, inside themigration transaction, so a rollback cannot silently discard operational decisions. The Postgres
test flips one wallet's flag away from the backfill value and asserts the audit row reflects that
value, not the derived one.
address IS NOT NULL→WHERE truefails2/3; audit insert removed fails 2/3; audit storing the derived value fails 2/3; flag check removed
from
isValidForWebhookfails 2/13; consent filter always true fails 4/15; fail-closed check=== true→!== falsefails 2/28; entity default flipped fails 1/13.Out of scope
getByIdOrNamekeys its cache without the requested relations, so a wallet cached by one callercan be served to another without
usersloaded. Pre-existing, not touched here.getAllKycDataremains ungated — a different dataset and a separate decision.