Repository navigation
feat(mobile): verified https pairing links; custom scheme carries no secret - #27475
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
📥 Commits
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to Resolve the Android link-verification and code-bearing link concerns before merging. Also correct the scanner notice so it accurately describes refused HTTPS links. Pre-merge checks |
|
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
52e15619-d70b-4354-8009-525e6e0e79cb
📒 Files selected for processing (30)
.github/workflows/mobile-ios-release.ymlconfig/verified-links/README.mdconfig/verified-links/apple-app-site-associationconfig/verified-links/assetlinks.jsonconfig/verified-links/pair.htmlmobile/app.config.jsmobile/app/+native-intent.tsmobile/app/_layout.tsxmobile/app/pair-confirm.tsxmobile/app/pair-scan.tsxmobile/app/pair.tsxmobile/src/pair-scan-styles.tsmobile/src/transport/incoming-pairing-link.test.tsmobile/src/transport/incoming-pairing-link.tsmobile/src/transport/pair-confirm-state.test.tsmobile/src/transport/pair-confirm-state.tsmobile/src/transport/pairing.test.tsmobile/src/transport/pairing.tssrc/main/ipc/mobile-pairing-copy-link.test.tssrc/main/ipc/mobile.test.tssrc/main/ipc/mobile.tssrc/main/server/serve-readiness.test.tssrc/main/server/serve-readiness.tssrc/preload/api/mobile-api.tssrc/preload/api/mobile-bridge.tssrc/shared/pairing.test.tssrc/shared/pairing.tssrc/shared/verified-pairing-link-files.test.tssrc/shared/verified-pairing-link.test.tssrc/shared/verified-pairing-link.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| The direct-download APK built by `.github/workflows/mobile-android-release.yml` is currently signed | ||
| with the build template's public debug keystore. Do not list that fingerprint (`FA:C6:17:45:…`): it | ||
| is public, so any app signed with it could claim the links. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use a private signing key for direct-download APKs before relying on App Links.
The documented direct-download APK uses the public debug key, but assetlinks.json must not authorize that key. Android therefore cannot verify pairing links for that APK; a user who taps the new phone link reaches the browser fallback instead of the app. Sign this distribution with a private release key and publish its fingerprint before enabling the link flow for it. (developer.android.com)
🧰 Tools
🪛 LanguageTool
[uncategorized] ~43-~43: The official name of this software platform is spelled with a capital “H”.
Context: ...oads. The direct-download APK built by .github/workflows/mobile-android-release.yml i...
(GITHUB)
| That orca:// link can’t pair your phone, because any app can open orca:// links. Scan the QR | ||
| code on your computer instead. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Explain both types of refused link.
If a user opens https://evil.example/pair#abc123, the router shows this notice. The notice calls that HTTPS link an orca:// link. Use wording that covers unverified HTTPS links and custom-scheme links, so the refusal gives the correct reason.
| @@ -119,6 +120,12 @@ function renderHumanReadiness(readiness: ServeReadiness): string { | |||
| lines.push(`Mobile pairing QR:\n${readiness.pairing.qr}`) | |||
| } | |||
| lines.push(`Pairing URL: ${readiness.pairing.url}`) | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not print the custom-scheme URL as a mobile pairing link.
When readiness.pairing.scope is mobile, this line still prints orca://pair with the pairing code before the new HTTPS phone link. A user can tap or share the first link, which still exposes the code to an app that claimed the custom scheme. Print only the verified phone link for mobile scope; retain the existing URL where another scope requires it.
f188cd2 to
bc897e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
0ce561f7-64c2-4179-8534-41c5d4ed9dd3
📥 Commits
Reviewing files that changed from the base of the PR and between 2d3a7b98288b057f3f56285a90b28419a51c6c6a and 1b1f0eeedb4103d551735c11f67307572f5fa64c.
📒 Files selected for processing (3)
mobile/src/transport/incoming-pairing-link-router-agreement.test.tsmobile/src/transport/incoming-pairing-link.test.tsmobile/src/transport/incoming-pairing-link.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| [...target.searchParams.keys()].length > 0 || | ||
| target.hash.length > 1 || | ||
| (outer?.hash.length ?? 0) > 1 | ||
| return carriesPayload ? REFUSED_PATH : '/pair-scan' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use an accurate notice for refused HTTPS links.
When a user opens https://evil.example/pair#abc123, this branch sends them to the scanner with PAIR_SCAN_LINK_REFUSED_NOTICE. The scanner then says “That orca:// link can’t pair your phone,” although the refused link was HTTPS. Use a separate HTTPS notice or wording that applies to both link types. (raw.githubusercontent.com)
…secret The desktop's copy-link now emits https://www.onorca.dev/pair#<code>, with the code only in the fragment. Orca Mobile claims that link as a verified app link and refuses any pairing code delivered through orca://, which any app can claim; orca://pair alone opens the scanner with an explanation. In-app QR scan and paste still accept the custom scheme, so older desktops and phones keep pairing. The confirm screen shows the desktop address and host key fingerprint before pairing. config/verified-links/ holds the apple-app-site-association, assetlinks.json and fallback page to deploy on www.onorca.dev, with a test that keeps them, mobile/app.config.js and the link builder in agreement. Part of #17397. Co-Authored-By: Claude <noreply@anthropic.com>
…rified links The router unwraps expo-development-client ?url= links and normalizes dot segments after +native-intent runs, so an orca:// wrapper could still deliver a code to the confirm screen. Refuse any unverified link carrying code= at any encoding depth, and match pairing routes on decoded, dot-free segments. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ng codes An invalid UTF-8 escape made the whole escape run fail to decode, hiding an adjacent encoded &code= that the router still decodes leniently. Co-Authored-By: Claude <noreply@anthropic.com>
… codes Co-Authored-By: Claude <noreply@anthropic.com>
…ing codes Co-Authored-By: Claude <noreply@anthropic.com>
Stop re-implementing URL decoding. The guard now runs expo-router's own extractExpoPathFromURL and parseUrlUsingCustomBase, the functions its linking applies after +native-intent, and refuses any link from which the router would read a `code` param or reach a pairing route with a payload. A link the router cannot parse is refused. Release builds refuse expo-development-client wrappers outright. A property-style test walks wrapping depths, encoding layers, stripped control characters, case and unicode, and asserts that the guard never passes on a link from which the router would read a code. Co-Authored-By: Claude <noreply@anthropic.com>
A host link with a segment named pair was sent to the scanner. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
1b1f0ee to
82c3703
Compare
ELI5
Today the phone pairing secret travels in an
orca://pair?code=…link. Any app on a phone can register itself fororca://, so a link someone taps could hand the secret to the wrong app. Now the desktop gives out an ordinary web link,https://www.onorca.dev/pair#<code>. iOS and Android only open that link in the app that onorca.dev vouches for. The phone app also stops accepting a secret from anorca://link at all.What Changed
Before: "Copy pairing code" in Settings → Mobile (and the mobile page) copied
orca://pair?code=<secret>. Tapping anyorca://pair?code=…ororca://pair-confirm?code=…link on a phone opened Orca Mobile's "Pair with this desktop?" screen, whichever app had actually claimed the scheme.After:
https://www.onorca.dev/pair#<code>. The code is only in the part after#, which browsers never send to the server. The QR code is unchanged (see compatibility below).orca serve --mobile-pairingalso prints aPhone pairing link:line. ThePairing URL:line and the JSON readiness output are unchanged.app/+native-intent.tsrewrites every incoming link before the router sees it.orca://link (or unverified https link) that targets a pairing screen with a code goes to the scanner, with a note: "That orca:// link can't pair your phone, because any app can open orca:// links. Scan the QR code on your computer instead."orca://pairjust opens the scanner.app/pair.tsx, which pulled codes out oforca://links, are gone.app/pair.tsxis now a plain redirect to the scanner.orca://pair?code=, the new https link, and a bare code. These never pass through the OS link dispatcher, so another app can't intercept them.autoVerifyintent filter for exactlyhttps://www.onorca.dev/pair. iOS getsapplinks:www.onorca.dev, but only when the release setsORCA_IOS_ASSOCIATED_DOMAINS=1(see Notes).config/verified-links/holdsapple-app-site-association,assetlinks.json, and apair.htmlfallback page, plus a README that says exactly what to deploy and how. A test keeps these files,mobile/app.config.js, and the link builder (src/shared/verified-pairing-link.ts) in agreement.Mixed versions
orca://code works. Tapping the oldorca://pair?code=link is refused, and the scanner opens with the note telling the user to scan the QR.orca://pair?code=, so the old in-app scanner works. Tapping the https link opens the browser on/pair, where the fallback page explains what to do and can copy the bare code. Old apps accept a bare code in Paste. Pasting the https link itself into an old app fails with "Not a valid pairing code".Why
www.onorca.dev, not the apex.onorca.devanswers with a 308 redirect towww, and neither OS follows a redirect while verifying a domain.+native-intent, not in a screen. This runs before routing, so no screen can be reached from anorca://link with a code. That coversorca://pair-confirm?code=…too, which routed straight to the confirm screen before.orca://for now. Every shipped phone's in-app scanner only understands that form. An in-app scan never goes through the OS, so it can't be intercepted. The QR can flip together with the ticket work in the follow-up.Linked Issue
Part of #17397. That issue closes once single-use tickets with owner approval have also shipped.
Visual Proof
N/A for screenshots. Mobile UI changes are text-only: a note on the scanner screen, and an address plus host-key box on the confirm screen. Verified with unit tests. No device or simulator run was done (no simulator on this machine).
Testing
pnpm tc,pnpm run check:code-quality:changed, and everypnpm lintstep. The native-plugins step reportsimport/no-cyclewarnings inmobile/src/transport/client-context.tsxandmobile/src/terminal/document/*; those files are untouched here and the warnings are already onmain.src/shared/verified-pairing-link.test.ts,src/shared/verified-pairing-link-files.test.ts,src/shared/pairing.test.ts,src/main/ipc/mobile.test.ts,src/main/ipc/mobile-pairing-copy-link.test.ts,src/main/server/serve-readiness.test.ts, plus the renderer mobile/settings suites.pnpm typecheck,check:tests-typecheck, oxlint, and the fullvitest run. One failure,mobile-e2ee-v2-client-session"bounded binary string conversions", is environmental: local Node 26 has a native base64 encoder, sobtoais never called. It is unrelated to this change.Review
Agent skill upstream boundary
Notes
Owner deploy steps (outside this repo). These must happen before a desktop release that ships this; until then the copied https link opens a 404:
config/verified-links/apple-app-site-associationathttps://www.onorca.dev/.well-known/apple-app-site-associationandconfig/verified-links/assetlinks.jsonathttps://www.onorca.dev/.well-known/assetlinks.json. Both needContent-Type: application/json, a direct 200 with no redirect, and no auth or bot challenge.config/verified-links/pair.htmlathttps://www.onorca.dev/pair, with no third-party scripts.assetlinks.jsonthe SHA-256 of the key that actually signs the shipped APK (the Play app signing key, if the app goes to Play).com.stably.orca.mobile, then set the repository variableORCA_IOS_ASSOCIATED_DOMAINS=1.Finding: the direct-download APK from
mobile-android-release.ymlis signed with the Expo template's public debug keystore (FA:C6:17:…), because no release signing is configured. That fingerprint must not go inassetlinks.json, and the test refuses it. Android link verification needs a real private release key.Security: no secret ever travels in a query string or through the custom scheme into the app. Cross-platform: desktop changes are string-only. SSH/remote: unaffected; the runtime (desktop and CLI) pairing codes and their parsers still accept
orca://, and now also accept the https form.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)