fix: secure shop payments and locked links - #1158
Conversation
Co-authored-by: benk10 <ben.kaufman10@gmail.com>
Co-authored-by: benk10 <ben.kaufman10@gmail.com>
Co-authored-by: benk10 <ben.kaufman10@gmail.com>
Co-authored-by: benk10 <ben.kaufman10@gmail.com>
Greptile SummaryThis PR restricts the shop WebView to trusted Bitrefill HTTPS origins, makes QuickPay respect payment PIN settings, and defers payment inputs until authentication. It also adds focused tests and a security changelog entry.
Confidence Score: 4/5The PR should not merge until locked payment inputs can no longer silently overwrite earlier pending requests. The new authentication gate retains only one pending scan or deeplink, so consecutive Android payment intents received before unlock cause earlier requests to be discarded. Files Needing Attention: app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
|
| Filename | Overview |
|---|---|
| app/src/main/java/to/bitkit/ui/screens/shop/shopWebView/ShopOrigin.kt | Introduces HTTPS Bitrefill host validation and an origin-filtering JavaScript message bridge. |
| app/src/main/java/to/bitkit/ui/screens/shop/shopWebView/ShopWebViewClient.kt | Injects the restricted bridge and blocks untrusted main-frame navigation. |
| app/src/main/java/to/bitkit/ui/screens/shop/shopWebView/ShopWebViewInterface.kt | Moves bridge handling onto the main thread and validates the current page origin before forwarding payment intents. |
| app/src/main/java/to/bitkit/ui/screens/shop/shopWebView/ShopWebViewScreen.kt | Supplies the current WebView URL to the interface for origin validation. |
| app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt | Adds PIN-aware QuickPay suppression and locked-input deferral, but the single pending slot silently drops all but the latest input. |
| app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt | Covers replay of one locked input and PIN-aware QuickPay but does not exercise multiple inputs received before unlock. |
Sequence Diagram
sequenceDiagram
participant OS as Android intents
participant VM as AppViewModel
participant Lock as Authentication UI
participant Send as Send flow
OS->>VM: Payment input A
VM->>VM: "pendingLockedScan = A"
OS->>VM: Payment input B
VM->>VM: "pendingLockedScan = B"
Note over VM: Input A is overwritten
Lock->>VM: setIsAuthenticated(true)
VM->>VM: flushPendingLockedScan()
VM->>Send: Launch input B only
Reviews (1): Last reviewed commit: "fix: handle null shop origin scheme" | Re-trigger Greptile
Co-authored-by: benk10 <ben.kaufman10@gmail.com>
Co-authored-by: benk10 <ben.kaufman10@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
jvsena42
left a comment
There was a problem hiding this comment.
Code review + manual verification on a regtest dev build (Pixel_9 emulator).
Verified working on device: QuickPay is skipped when PIN-for-payments is on (Skipping QuickPay because PIN is required for payments → Confirm sheet → PIN prompt on swipe); QuickPay still runs with PIN-for-payments off; payment links fired while locked are queued and opened exactly once after auth; a second link replaces the first (Replacing deferred scan) and the earlier one is never replayed; off-Bitrefill main-frame navigation is blocked (Blocked shop navigation to untrusted origin 'https://policies.google.com/privacy') while in-shop navigation is unaffected.
Not covered on device: Bitrefill checkout (dev flavor talks to the real embed.bitrefill.com, so a checkout emits a mainnet invoice a regtest wallet can't act on), the Paykit handoff race, and the scanner-ordering case.
5 inline findings below.
|
@pwltr Thanks for clarifying. Agreed—QuickPay is the explicit opt-in exception to per-payment PIN confirmation. I removed the new PIN-for-payments gate in 32b59a5, kept the app-authentication deferral for locked links, and updated the regression test, changelog, title, and PR description to state the accepted behavior accurately. The focused test, full unit suite, compile, and detekt checks are green. |

Description
This PR:
https://embed.bitrefill.comorigin through an origin-scoped WebMessage listener.Addresses VulnHunter 01b finding 02 and the unauthenticated deeplink handling from finding 03. The explicit QuickPay bypass of per-payment PIN confirmation remains accepted product behavior.
Companion iOS PR: synonymdev/bitkit-ios#668
Preview
N/A — security boundary and payment-flow state handling only.
QA Notes
Manual Tests
regression:Shop → navigate between legitimate Bitrefill pages: navigation remains available.lightning:link → authenticate: no payment flow opens before authentication, then QuickPay opens once.Automated Checks
ShopOriginTest.kt: covers the exact payment origin, trusted port/user-info boundary, broader navigation allow-list, and bridge-page guard.ShopWebViewInterfaceTest.kt: covers feature detection, fail-closed fallback, exact listener origin, sibling-origin rejection, non-string messages, and payment parsing.ShopWebViewClientTest.kt: covers supported/unsupported bridge injection, sibling-page non-injection, and main-frame navigation boundaries.AppViewModelSendFlowTest.kt: covers authentication/QuickPay deferral, latest-wins replacement, context isolation, cancellation serialization, scanner ordering, sheet dismissal, and Paykit handoff races.compileDevDebugKotlin,testDevDebugUnitTest,detekt --rerun-tasks, andgit diff --check.