feat: update app to beta version with tracking authorization and impr…#552
feat: update app to beta version with tracking authorization and impr…#552felix-schultz merged 1 commit intoalphafrom
Conversation
…oved release notes
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Code Review
This pull request implements App Tracking Transparency (ATT) for the iOS desktop app, adding a Tauri command to retrieve authorization status and integrating it with PostHog for privacy-compliant tracking. It also updates the app version to 0.1.1, includes necessary iOS entitlements for Wasmtime, and refreshes the release information in the swimlanes configuration. Key feedback includes ensuring the tracking check fails closed for better privacy compliance, addressing a race condition where PostHog might start capturing before the authorization check completes, and using idiomatic Rust types when interfacing with Objective-C.
| const status = await invoke<string>("get_tracking_authorization_status"); | ||
| return status === "authorized"; | ||
| } catch { | ||
| return true; |
There was a problem hiding this comment.
The isTrackingAuthorized function currently defaults to true if the tracking status check fails. For better privacy compliance (e.g., GDPR, App Store guidelines), it is recommended to "fail-closed" and return false if the authorization status cannot be determined or if the IPC call fails.
| return true; | |
| return false; |
| isTrackingAuthorized().then((authorized) => { | ||
| if (!authorized) { | ||
| posthog.opt_out_capturing(); | ||
| } else { | ||
| posthog.opt_in_capturing(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This implementation has two potential issues:
- Race Condition:
posthog.init(called earlier in the effect) starts capturing immediately by default. SinceisTrackingAuthorized()is asynchronous, some events might be sent beforeposthog.opt_out_capturing()is invoked. It is safer to initialize PostHog withopt_out_capturing_by_default: trueand then explicitly callopt_in_capturing()here if authorized. - Session Updates: This check only runs once when the provider mounts. If the user grants permission via the system prompt after the app has started, PostHog will remain opted-out for the rest of the session. Consider re-checking the status when the window gains focus or using a listener to react to authorization changes.
| ) -> Result<String, TauriFunctionError> { | ||
| #[cfg(target_os = "ios")] | ||
| { | ||
| let status: u32 = unsafe { |
There was a problem hiding this comment.
The trackingAuthorizationStatus method on ATTrackingManager returns an NSUInteger, which corresponds to usize in Rust (64-bit on iOS). While the current enum values (0-3) fit in a u32, using usize is more idiomatic and type-safe when interfacing with Objective-C via objc2.
| let status: u32 = unsafe { | |
| let status: usize = unsafe { |
| enable_heatmaps: true, | ||
| }); | ||
|
|
||
| isTrackingAuthorized().then((authorized) => { | ||
| if (!authorized) { | ||
| posthog.opt_out_capturing(); | ||
| } else { | ||
| posthog.opt_in_capturing(); | ||
| } |
There was a problem hiding this comment.
Bug: A race condition allows PostHog to capture events before the asynchronous tracking authorization check completes, violating user privacy settings on iOS.
Severity: CRITICAL
Suggested Fix
Ensure tracking authorization is confirmed before initializing PostHog. Move the posthog.init() call to execute only after the isTrackingAuthorized() promise successfully resolves with a status that permits tracking.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/desktop/app/provider.tsx#L36-L44
Potential issue: In the `PHProvider` component, PostHog is initialized synchronously
with `posthog.init()`, which can start capturing events immediately. However, the user's
tracking authorization is checked asynchronously via `isTrackingAuthorized()`. This
creates a race condition where events may be captured and sent before the authorization
check completes. For iOS users who have denied tracking, this violates App Tracking
Transparency (ATT) because their data is sent before `opt_out_capturing()` is called.
Furthermore, if the `invoke("get_tracking_authorization_status")` call fails,
authorization defaults to `true`, enabling tracking incorrectly.
Did we get this right? 👍 / 👎 to inform future reviews.
…oved release notes