🏗️ Enforce WKAppBoundDomains on iOS WebView#41
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enforces iOS WebKit “app-bound domains” behavior for the in-app WebView by adding a shared allowlist of domains, wiring it into the iOS Info.plist via an Expo config plugin, and intercepting WebView navigations to route off-allowlist top-frame links to the system browser.
Changes:
- Added a shared
APP_BOUND_DOMAINSallowlist plusisAppBoundHost()helper, reused by both runtime code and an Expo config plugin. - Added an Expo config plugin to write
WKAppBoundDomainsinto the generated iOS Info.plist. - Updated WebView navigation interception to open deep links and off-allowlist top-frame navigations externally; enabled
limitsNavigationsToAppBoundDomainson iOS.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| services/url-bridge.web.ts | Adds openExternalURL stub for web platform parity. |
| services/url-bridge.native.ts | Adds openExternalURL implementation (currently same mechanism as openDeepLink). |
| services/url-bridge.d.ts | Exposes openExternalURL in the typed facade. |
| services/app-bound-domains.js | Introduces shared WKAppBoundDomains allowlist and host-matching helper. |
| services/app-bound-domains.d.ts | Adds TypeScript declarations for the shared allowlist module. |
| plugins/withAppBoundDomains.js | Adds Expo config plugin to set WKAppBoundDomains in Info.plist. |
| app/index.tsx | Enforces app-bound navigation behavior and enables iOS-only limitsNavigationsToAppBoundDomains. |
| app.json | Registers the new Expo config plugin. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Intercept wallet deep links (wc:, metamask:, etc.) and route non-app-bound | ||
| // top-frame navigations to the system browser — WebKit's app-bound enforcement | ||
| // would otherwise silently block them. | ||
| const handleNavigationRequest = useCallback( | ||
| (request: ShouldStartLoadRequest) => { | ||
| if (isDeepLink(request.url)) { | ||
| openDeepLink(request.url).catch((e) => | ||
| console.warn('[deep link] failed to open:', request.url, e) | ||
| ); | ||
| return false; | ||
| } | ||
| return true; | ||
| // Leave iframes to WebKit; non-app-bound iframe loads (e.g. Stripe's | ||
| // metrics iframe) get silently blocked there, which is intended. | ||
| if (request.isTopFrame === false) return true; | ||
| try { | ||
| const parsed = new URL(request.url); | ||
| if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') return true; | ||
| if (isAppBoundHost(parsed.hostname)) return true; | ||
| openExternalURL(request.url).catch((e) => | ||
| console.warn('[external link] failed to open:', request.url, e) | ||
| ); | ||
| return false; |
There was a problem hiding this comment.
handleNavigationRequest currently routes all non-app-bound top-frame http(s) navigations to openExternalURL() on every platform. On Android (and likely web), request.isTopFrame is typically undefined, so this will still run and will block in-WebView navigation for any off-domain link (behavior change vs the previous return true). Consider gating this behavior to iOS only (e.g., after the deep-link branch: if (Platform.OS !== 'ios') return true;) so app-bound enforcement doesn’t affect Android/web.
There was a problem hiding this comment.
Keep for consitency
| return withInfoPlist(config, (config) => { | ||
| config.modResults.WKAppBoundDomains = APP_BOUND_DOMAINS; | ||
| return config; |
There was a problem hiding this comment.
In withAppBoundDomains, the callback passed to withInfoPlist reuses the parameter name config, shadowing the outer config. Renaming the inner parameter (e.g. modConfig) avoids confusion and also prevents potential no-shadow lint violations.
| return withInfoPlist(config, (config) => { | |
| config.modResults.WKAppBoundDomains = APP_BOUND_DOMAINS; | |
| return config; | |
| return withInfoPlist(config, (modConfig) => { | |
| modConfig.modResults.WKAppBoundDomains = APP_BOUND_DOMAINS; | |
| return modConfig; |
| const apex = host.toLowerCase(); | ||
| return APP_BOUND_DOMAINS.some( | ||
| (domain) => apex === domain || apex.endsWith(`.${domain}`) |
There was a problem hiding this comment.
isAppBoundHost assigns host.toLowerCase() to a variable named apex, but that value is still the full host (potentially a subdomain), not an apex/root domain. Renaming the variable to something like hostname/lowerHost would make the intent clearer.
| const apex = host.toLowerCase(); | |
| return APP_BOUND_DOMAINS.some( | |
| (domain) => apex === domain || apex.endsWith(`.${domain}`) | |
| const lowerHost = host.toLowerCase(); | |
| return APP_BOUND_DOMAINS.some( | |
| (domain) => lowerHost === domain || lowerHost.endsWith(`.${domain}`) |
williamchong
force-pushed
the
develop
branch
2 times, most recently
from
19a43d4 to
1c305ce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # app.json
# Conflicts: # app/index.tsx
Rename shadowed `config` parameter in withAppBoundDomains to `modConfig`, and rename `apex` to `lowerHost` in isAppBoundHost since the value may include subdomains.
|
Should use 🔒 instead of 🏗️? |
The goal is to allow PWA and lossen ITP in iOS, so not really security either |
No description provided.