Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3d3bb97fd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| element.addEventListener("touchstart", addDeviceIdToLink); | ||
| element.addEventListener("touchend", removeDeviceIdFromLink); |
There was a problem hiding this comment.
Preserve deviceId through touch navigation
touchend removes the deviceId query param before link activation completes on mobile browsers (touch sequence is touchstart → touchend → click/default navigation), so outbound taps often navigate without the forwarded identifier. This means the device-id handoff works on hover/focus paths but silently fails for touch users; cleanup should happen after navigation (or on touchcancel) instead of on touchend.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6a79079. The touch cleanup now runs on touchcancel instead of touchend, so mobile taps keep the forwarded deviceId through navigation rather than stripping it before the browser follows the link.
| var selector = PREFECT_DESTINATIONS.map(function (url) { | ||
| return 'a[href^="' + url + '"]'; | ||
| }).join(","); |
There was a problem hiding this comment.
Match Prefect destinations by hostname, not prefix
Using href^="https://..." prefix matching allows lookalike URLs such as https://prefect.io.evil.com/... to be treated as trusted destinations, so this code can append deviceId to non-Prefect domains and leak a stable identifier if such a link is ever present. Parse each link URL and compare hostname against an allowlist instead of relying on string prefixes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6a79079. The forwarding logic now parses each link and compares url.hostname against an explicit allowlist before appending deviceId, which avoids leaking identifiers to lookalike prefix matches.
Add amplitude to fastmcp docs