Route MoonPay iOS buys through a Private Relay interstitial check - #6151
Route MoonPay iOS buys through a Private Relay interstitial check#6151j0ntz wants to merge 1 commit into
Conversation
📸 Test evidencemoonpay buy quotes
safari widget fallback bound
safari interstitial unbound
Captured by the agent's in-app test run (build-and-test). |
1c042b5 to
4cf563e
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
18a9532 to
aab11af
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
51ea824 to
0c8bd6d
Compare
iOS buy opens the MoonPay widget in an SFSafariViewController whose traffic can egress through iCloud Private Relay, so a widget URL bound to the app-fetch IP mismatches what MoonPay observes for relay users and will fail their buys once IP-match enforcement turns on. The buy path now asks the info server for a relay-check interstitial URL and opens that in the Safari view: the server observes the Safari view's own egress and 302s to the widget URL signed with the IP binding when the addresses agree, or without it when they diverge. Any interstitial failure falls back to today's bound flow, so the change cannot regress buys even against servers without relay-check support. Sell and Android are untouched. MOONPAY_RELAY_CHECK_SIGN_PROXY (dev builds only) reroutes the one relay-check POST through an alternate egress so the unbound branch is reproducible on a simulator. The TDD at src/docs/moonpay-private-relay-interstitial.md documents both repos' changes.
0c8bd6d to
d16b014
Compare
paullinator
left a comment
paullinator
left a comment
There was a problem hiding this comment.
Additional Findings
- suggestion: PR Requirements leaves all device-test boxes unchecked with "No visual changes", while the Description/TDD correctly call out that the physical-device Private Relay pass is still outstanding and that MoonPay enforcement must stay off until it completes.
- Keep the no-visual-changes note, but add an explicit checklist item for the iCloud+ Private Relay on-device buy pass, and leave it unchecked until done.
| export const fetchMoonpayInterstitialUrl = async ( | ||
| url: string | ||
| ): Promise<string> => { | ||
| const signProxy = __DEV__ ? ENV.MOONPAY_RELAY_CHECK_SIGN_PROXY : undefined | ||
| const reply = await postSignUrl({ url, relayCheck: true }, signProxy) | ||
| const { interstitialUrl } = asMoonpayRelayCheckResponse(reply) | ||
| // An empty URL would open a blank Safari view with no error; throwing here | ||
| // routes the caller onto its bound-URL fallback instead. | ||
| if (interstitialUrl === '') { | ||
| throw new Error('Moonpay relay check returned an empty interstitial URL') | ||
| } | ||
| return interstitialUrl | ||
| } |
There was a problem hiding this comment.
Warning: No new unit tests cover fetchMoonpayInterstitialUrl, postSignUrl's MOONPAY_RELAY_CHECK_SIGN_PROXY / fetchWaterfall branch, the empty-interstitialUrl throw, or the iOS Platform.OS fallback in moonpayRampPlugin. The TDD (§8) only lists server mocha tests and Maestro/manual device passes for the GUI side, so the new client helpers ship without automated regression coverage despite being easily mockable (fetchInfo/fetchWaterfall).
Recommendation: Add Jest tests: mock fetchInfo/fetchWaterfall to assert relayCheck:true body + interstitialUrl parsing, empty-URL throw, non-OK status, and that a non-empty signProxy calls fetchWaterfall on the trimmed base. Optionally extract the iOS-vs-Android URL resolution so the Platform branch and .catch → signMoonpayUrl fallback can be asserted without mounting the full ramp plugin.
| const postSignUrl = async ( | ||
| body: object, | ||
| signProxy?: string | ||
| ): Promise<unknown> => { | ||
| const options = { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(body) | ||
| } | ||
| const response = | ||
| signProxy != null && signProxy !== '' | ||
| ? await fetchWaterfall( | ||
| [signProxy.replace(/\/+$/, '')], | ||
| SIGN_URL_PATH, | ||
| options, | ||
| SIGN_URL_TIMEOUT_MS | ||
| ) | ||
| : await fetchInfo(SIGN_URL_PATH, options, SIGN_URL_TIMEOUT_MS) | ||
| if (!response.ok) { | ||
| throw new Error(`Moonpay URL signing failed: ${response.status}`) | ||
| } | ||
| return await response.json() |
There was a problem hiding this comment.
Suggestion: postSignUrl is a good shared helper, but body is typed as object and HTTP failures always throw "Moonpay URL signing failed", including relayCheck requests. That weakens call-site type safety and makes interstitial fallback logs misleading.
Recommendation: Type the body as { url: string; relayCheck?: boolean } and use a mode-aware error message.
| let webViewUrl: string | ||
| if (Platform.OS === 'ios') { | ||
| webViewUrl = await fetchMoonpayInterstitialUrl( | ||
| urlObj.href | ||
| ).catch(async (error: unknown) => { | ||
| console.log( | ||
| 'Moonpay relay check unavailable, using bound URL: ' + | ||
| String(error) | ||
| ) | ||
| return await signMoonpayUrl(urlObj.href) | ||
| }) | ||
| } else { | ||
| webViewUrl = await signMoonpayUrl(urlObj.href) | ||
| } | ||
|
|
||
| deeplinkToken = await openExternalWebView({ | ||
| url: signedUrl, | ||
| url: webViewUrl, |
There was a problem hiding this comment.
Suggestion: webViewUrl is opened via openExternalWebView (SafariView / Custom Tabs), not a WebView; after the interstitial change the name no longer describes the value well.
Recommendation: Rename to openUrl or widgetUrl for clarity at the openExternalWebView call site.
| ).catch(async (error: unknown) => { | ||
| console.log( | ||
| 'Moonpay relay check unavailable, using bound URL: ' + | ||
| String(error) | ||
| ) | ||
| return await signMoonpayUrl(urlObj.href) | ||
| }) |
There was a problem hiding this comment.
Suggestion: The .catch handler is marked async only to return await signMoonpayUrl(...); the extra async/await is unnecessary.
Recommendation: Use .catch((error: unknown) => { console.log(...); return signMoonpayUrl(urlObj.href) }).
|
|
||
| One consequence of the fallback: when the relay-check POST fails and the plain retry succeeds against an upgraded server, a relaying v2 user is indistinguishable from the live app and lands on the update page. That session was already degraded (the primary path failed), and the page's advice is merely unnecessary rather than wrong. | ||
|
|
||
| The dev flag: `ENV.MOONPAY_RELAY_CHECK_SIGN_PROXY` (declared in `src/envConfig.ts`, empty by default) reroutes this one POST through an alternate egress, so the server observes divergent A1/A2 on a simulator and the unbound branch is reproducible. The flag is read behind `__DEV__`, so release builds cannot reach it ([decision 3](#103-divergence-testing-flag-lives-client-side)). |
There was a problem hiding this comment.
Suggestion: TDD says ENV.MOONPAY_RELAY_CHECK_SIGN_PROXY is "empty by default", but envConfig declares it as asOptional(asString) with no default, so an absent key is undefined (not "").
Recommendation: Rephrase to "absent/undefined by default (and ignored when empty)" to match asOptional(asString) and the signProxy guard.
| // to today's bound flow. Android (Custom Tabs, app network | ||
| // stack) keeps the fully bound flow. |
There was a problem hiding this comment.
Suggestion: Inline comment calls Android Custom Tabs the "app network stack". Sell's guiPluginWebView is the app stack; Custom Tabs use the device/browser stack (same egress as app fetch, but not the app WebView stack).
Recommendation: Align with the TDD: e.g. "Android (Custom Tabs, device network stack, same egress as app fetch) keeps the fully bound flow."
| /** | ||
| * Ask the info server to bind a Moonpay widget URL to the caller's public IP | ||
| * and sign it. Moonpay's on-ramp security upgrade refuses to load widget URLs | ||
| * that are not signed and IP-bound, so every buy/sell widget URL must be routed | ||
| * through here before it is opened. |
There was a problem hiding this comment.
Suggestion: signMoonpayUrl's JSDoc still says every buy/sell widget URL must be routed through that function before opening. After this PR, the iOS buy happy path uses fetchMoonpayInterstitialUrl instead.
Recommendation: Update the JSDoc to say direct IP-bound signing (sell, Android buy, and iOS buy fallback); point iOS buy's primary path at fetchMoonpayInterstitialUrl.
|
|
||
| ## Unreleased (develop) | ||
|
|
||
| - added: MoonPay iOS buys open through a Private Relay interstitial check, so IP-match enforcement cannot lock out iCloud Private Relay users; any interstitial failure falls back to the existing bound flow. |
There was a problem hiding this comment.
Suggestion: CHANGELOG uses "added:" for a change to the existing MoonPay IP-bound buy handoff. Nearby history already has "changed:" for signing/binding.
Recommendation: Prefer "changed:" (keep the same explanation about interstitial + fallback).
2 participants
Technical Design Document
moonpay-private-relay-interstitial.md
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
EdgeApp/edge-info-server#160 (soft dependency: without it the app falls back to today's bound flow, so this PR is safe to land first)
Requirements
If you have made any visual changes to the GUI. Make sure you have:
No visual changes: only the URL handed to the external Safari view differs.
Description
Asana task
iOS buys open the MoonPay widget through the server's Private Relay relay-check interstitial instead of a directly IP-bound URL, so the server can compare the app-fetch and Safari-view egress addresses and sign the widget URL with or without the binding. Any interstitial failure falls back to today's bound flow, so buys cannot regress; sell and Android are untouched.
MOONPAY_RELAY_CHECK_SIGN_PROXY(dev builds only) makes the divergence branch reproducible on a simulator.The TDD linked above carries the full design for both repos. The physical-device Private Relay pass is the outstanding manual item; MoonPay enforcement stays off until it completes.
Note
Medium Risk
Changes the critical fiat on-ramp URL handoff for iOS buys and depends on info-server relay-check behavior, but failures degrade to the prior bound flow.
Overview
iOS MoonPay buys no longer open a directly IP-bound widget URL in
SFSafariViewController. They first fetch a relay-check interstitial from the info server (fetchMoonpayInterstitialUrlwithrelayCheck: true); the Safari view loads that URL and the server redirects to a signed widget URL with or without IP binding after comparing app vs Safari egress. Sell and Android buy still use the existing boundsignMoonpayUrlpath.Resilience: If the interstitial request fails (old server, network, empty URL), the ramp plugin falls back to today's bound signing so buys are not blocked on partial rollout.
Dev tooling:
MOONPAY_RELAY_CHECK_SIGN_PROXYin env config (dev-only) can reroute the relay-check POST through alternate egress for simulator testing; signing helpers are consolidated behindpostSignUrl.A TDD (
moonpay-private-relay-interstitial.md) and CHANGELOG entry document the cross-repo flow and outcomes.Reviewed by Cursor Bugbot for commit d16b014. Bugbot is set up for automated code reviews on this repo. Configure here.