Subscribed to pull request

Generated by CodeMention

/review

🤖 AI code review

Decision: Approve with comments

Overall PR risk: Medium. The change modifies existing iOS Live Activity behavior: it moves the deep link URL from app-group storage into LiveActivityAttributes, adds .widgetURL to the Lock Screen banner, filters getInstances() by factory name and activity state, and raises that guard to iOS 16.2 availability. The blast radius is limited to apps that use expo-widgets Live Activities on iOS; Android, TypeScript signatures, and prebuild are untouched, and a revert is direct. The most plausible breakage is the tap URL: activities started by an older build now decode with url == nil and lose their deep link, the always-applied banner modifier can conflict with a widgetURL set inside the layout, and a push-to-start sender now controls the url attribute that reaches widgetURL without any scheme or host check. No critical defect was found, so these three warnings are comments rather than blockers.

🟡 Warning (3)

• Push-to-start payload can now set the Live Activity tap URL — packages/expo-widgets/ios/Widgets/WidgetLiveActivity.swift:41 (security) · id:1b6ea5ef1095
  Confidence: Medium — the data path is direct, but it needs an app that uses push-to-start.
  Impact if shipped: Medium — a push sender can choose the URL that opens when the user taps the Live Activity.
  Suggested remediation: Validate the URL before you pass it to widgetURL. Accept only the app's own URL schemes, or the app's universal link hosts, and drop everything else. A small helper in Utils.swift can do this check for both the banner and the Dynamic Island.

  Evidence and reasoning

  The PR moves the deep link URL from app-group storage into LiveActivityAttributes. Before the change, getLiveActivityUrl(forName:) read the URL from WidgetsStorage. Only a local start() call wrote that value, so the URL was always chosen on the device.

  LiveActivityAttributes is also the attributes type for push-to-start. This module supports push-to-start: WidgetsModule.swift:136 reads Activity<LiveActivityAttributes>.pushToStartToken and sends the token to JavaScript. In a push-to-start APNs payload, the sender supplies the attributes object, and ActivityKit decodes it into LiveActivityAttributes. The new url property is a stored property of that struct, so the payload now controls it.

  Both .widgetURL(...) calls pass that value straight to the system with no check on the scheme or the host. URL.init(string:) accepts any scheme.

  Attacker: any party that can send a push-to-start notification to the app. This includes a compromised app backend and a third-party push provider. Input they control: the url field inside attributes. Consequence: the user taps the Live Activity on the Lock Screen, and the system opens an attacker-chosen target. That target is either an external page that looks like part of the app, or an arbitrary in-app deep link route.

  The same string also reaches the Dynamic Island widgetURL at line 68.

• Live Activities started before the app update lose their deep link URL — packages/expo-widgets/ios/Widgets/WidgetLiveActivity.swift:68 (correctness) · id:2ea37b68b433
  Confidence: High — the diff deletes the only reader of the stored URL and adds no fallback for activities whose attributes predate the new field.
  Impact if shipped: Medium — a Live Activity that is running during an app update stops deep linking, and taps open the app's default route.
  Suggested remediation: Keep getLiveActivityUrl(forName:) for one release and use it as a fallback when context.attributes.url is nil, for example context.attributes.url.flatMap(URL.init(string:)) ?? getLiveActivityUrl(forName: context.state.name).

  Evidence and reasoning

  The PR moves the deep link URL from App Group storage into LiveActivityAttributes.url. Two places change together:

  1. LiveActivityFactory.start no longer writes __expo_widgets_live_activity_<name>_url (packages/expo-widgets/ios/LiveActivityFactory.swift).
  2. getLiveActivityUrl(forName:) is deleted from packages/expo-widgets/ios/Widgets/Utils.swift, and the widget extension now reads context.attributes.url.

  ActivityKit persists the ActivityAttributes value that existed when the activity started. A Live Activity that the previous app version started has no url key in its stored attributes. url is optional, so the synthesized decoder returns nil and the activity still appears in Activity<LiveActivityAttributes>.activities. But context.attributes.url is then nil, so .widgetURL(nil) applies.

  Before this PR the same activity still deep linked, because the widget extension read the URL from App Group storage by factory name. That reader is gone. A Live Activity can run for many hours, so an app update during an active delivery or ride removes the deep link for the rest of that activity.

  The Dynamic Island path at line 68 is the concrete regression: it worked through the storage lookup before, and now returns nil for every pre-update activity.

• Lock Screen banner always applies widgetURL, so it can override a layout widgetURL — packages/expo-widgets/ios/Widgets/WidgetLiveActivity.swift:41 (correctness) · id:2ea37b68b433
  Confidence: Medium — the conflict path is traced through the renderer, but SwiftUI does not define which of two widgetURL modifiers wins.
  Impact if shipped: Medium — a Live Activity started without a URL can lose the deep link that its own layout sets.
  Suggested remediation: Apply the modifier only when the activity has a URL. For example, branch in the ViewBuilder: if let url = context.attributes.url.flatMap(URL.init(string:)) { LiveActivityBannerView(context: context, nodes: nodes).widgetURL(url) } else { LiveActivityBannerView(context: context, nodes: nodes) }.

  Evidence and reasoning

  The diff adds .widgetURL(...) to the Lock Screen banner. The modifier runs on every render, also when context.attributes.url is nil, because flatMap gives nil.

  A layout author can already set a URL from JavaScript. packages/expo-widgets/bundle/ui-globals.ts re-exports @expo/ui/swift-ui/modifiers, which exports widgetURL. ViewModifierRegistry.swift:2167 registers it, and WidgetsDynamicView renders each node through UIBaseView, which applies the props modifiers. So a banner node can carry its own widgetURL.

  This repo's own documentation for that modifier states the rule, at packages/expo-ui/src/swift-ui/modifiers/widgets.ts:15: "Widgets support one widgetURL modifier in their view hierarchy. If multiple views have widgetURL modifiers, the behavior is undefined."

  Before this change the banner closure had no widgetURL, so the layout modifier was the only one. After this change there are two, and the new one sits above the layout. An app that calls start(props) with no URL and sets the link inside the layout can stop deep linking from the Lock Screen.

This review is advisory — it never blocks a merge and never auto-approves.

Replying to the AI review point by point:

1. Push-to-start payload can now set the Live Activity tap URL — acknowledged, leaving as is. widgetURL never opens the browser: tapping always opens the containing app, and the URL is delivered to the app's own link handler (this resolves the reviewer's stated uncertainty). So the payload gains no more power than any push notification carrying a deep link, and a sender with push-to-start access already controls name and props — the entire rendered content. A scheme/host allowlist in the library would impose policy it can't know (custom schemes, universal-link hosts vary per app), and a server-chosen URL is arguably a feature for push-to-start. Apps must validate incoming URLs in their link handler regardless of source.

2. Live Activities started before the app update lose their deep link — acknowledged (it's also called out in the PR description) and deliberately accepted. One caution about the suggested remediation: the bare fallback context.attributes.url.flatMap(URL.init(string:)) ?? getLiveActivityUrl(forName: context.state.name) would reintroduce #48401 — the legacy __expo_widgets_live_activity_<name>_url keys are never removed, so any new activity started without a URL would silently pick up the stale stored URL through the fallback, which is the exact bug this PR fixes. A correct shim needs the fallback plus unconditional key removal in start(), and a reminder to delete it a release later. Given the affected window — an activity already running at the moment of an app update, for the remainder of its ≤12 h lifetime — the transient regression seems preferable to a stateful migration path. Happy to add the corrected shim if maintainers prefer.

3. Lock Screen banner always applies widgetURL, so it can override a layout widgetURL — good catch, fixed in 028ab24. .widgetURL is now applied only when the activity actually has a URL, on both the Lock Screen banner and the Dynamic Island. The island had the always-applied pattern before this PR too (with the possibly-nil storage lookup), so this also removes a pre-existing conflict with layouts that set widgetURL on island sections.

@jakex7 FYI, point 3 of the review is fixed in 028ab24 (conditional widgetURL on banner + island)