Release: develop -> main - #4445
Merged
Merged
Conversation
…link (BUG-1289) (#4436) * fix(payment-link): scope externalPaymentId lookups to the authorized link (BUG-1289) Cross-merchant payment-link disclosure: `getPaymentByExternalId` matched only on `externalId` — a merchant-supplied reconciliation identifier that is not unique across tenants. The link lookup in `payment-link.service.ts` was correctly owner-scoped, but the payment enrichment was global, so any authenticated caller who owned one link could iterate the sequential externalPaymentId keyspace and read (and cancel/confirm) foreign merchants' payment records by pairing the foreign id with their own linkId. Same helper backed the wait / confirm / cancel paths, giving the leak a write primitive (own-asset PoC in the report; third-party path not exercised). Fix: require the resolved `linkId` at the type level for the lookup, so the query becomes `findOne({ where: { externalId, link: { id: linkId } } })`. The access-key POS path (`getPaymentLinkByAccessKey`) is reversed to resolve links via the repository first, then match access-keys in memory, so foreign records never enter the process. `getAllPaymentsByExternalLinkId` was only used there and is removed. Verified against the four attack scenarios from the report (read, iteration, confirm, cancel) — all now return 404 with an empty payments array. Legitimate paths (own link + own payment, POS access-key) unchanged. Lint, type-check, and the payment-link jest suite (7 files / 62 tests) pass. * refactor(payment-link): move externalPaymentId link lookup into a named repository method Follow-up to the previous commit. The access-key POS path inlined a `paymentLinkRepo.find(...)` in the service, bypassing the "repositories own data access" layering the rest of `payment-link.service.ts` follows. Every other access-key branch delegates to a named repo method (`getAllPaymentLinksByExternalLinkId` right above it); this brings the externalPaymentId branch in line. - Adds `PaymentLinkRepository.getAllPaymentLinksByExternalPaymentId`, mirroring the shape of the sibling `getAllPaymentLinksByExternalLinkId`. - Drops the inline query and the now-unused `Equal` import from the service. No behavior change. Same 62/62 payment-link jest tests, lint and type-check clean.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 29, 2026 10:58
…accounts too (#4444) * fix(custody): hide an empty legacy Safe next to read-only accounts too The hide rule only fired when the customer held write access on another entry. A viewer with read grants only — the operator-side case — kept an empty "Custody" entry in the selector next to the accounts they may see. Drop the access-level condition: any other visible entry is enough. The balance check is unchanged, so a legacy Safe that actually holds something still stays. The trade-off is stated on the branch: a read-only viewer is left without a writable entry, and the selector offers them no deposit path. * test(custody): assert the balance check actually runs for read-only viewers Two findings from the review. The new test passed without proving anything: with a read grant next to an empty Safe, the old write-access rule never consulted the balance either, so the case was green before the fix as well. Asserting the call is what ties it to the changed condition. The comment also claimed the balance check keeps the entry for anyone holding a balance. With no other entry visible that check never runs — the entry stays because the condition short-circuits, which is a different reason.
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
2 participants
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist