fix(replay): generate session_id client-side to avoid NAT collisions - #315
Merged
Conversation
The server's default replay session_id derivation hashes salt + projectId + ip + user-agent. Every user behind the same NAT (office, home WiFi, mobile carrier) collapses to the same session_id, so their replay timelines get merged into one shared session. Verified in our office — multiple colleagues' recordings showed up under the same replay id. Upstream is aware of this category of bug (issue #337) but has deprioritized fixing it on the privacy-analytics philosophy that no client storage should be required by default. The `__deviceId` override they suggest fixes attribution for track/identify events but does NOT fix replay — the replay handler ignores property overrides and recomputes deviceId from IP+UA directly. Two-part fix, scoped to replay only: 1. packages/sdks/web/src/index.ts - On replay start, generate a session_id client-side using crypto.randomUUID() and persist it in sessionStorage (per-tab, rotates after 30 min idle — matches the existing server-side rotation rule so analytics session counts don't drift). - Also persist a device_id in localStorage. Not sent on the wire today; reserved for future cross-tab stitching. - Use crypto.randomUUID() where available; fall back to a Math.random-based v4 UUID for old browsers and http:// contexts where the Web Crypto API is unavailable. - Refresh last-activity on every replay chunk send so an actively used tab keeps the same session until 30 min of true inactivity. - If sessionStorage is unavailable (sandboxed iframe, quota exhausted, private mode quirk), fall back to the existing /track/device-id server-derived path. No regression for those callers. 2. apps/api/src/controllers/track.controller.ts - Trust payload.session_id when the SDK provides one. Only derive from IP + UA as a fallback for SDKs that don't send it. - Fully backward-compatible: old SDKs that send the server-issued session_id still get the same id stored (server-side derivation and SDK-echoed values match). Scope: - track / identify / increment / decrement event flows are UNCHANGED. deviceId for those still derives from IP+UA hash (or the existing __deviceId property override). - Analytics dashboards, session counts, funnel analytics, retention all unchanged. - Only the REPLAY session_id derivation is altered, and only when the SDK sends one. Deployment order: - Server change is safe to deploy independently. Existing in-prod SDKs send the server-derived session_id back unchanged, and the new server uses it as-is (identical result to today). No regression possible without the SDK side. - SDK change activates the fix when consumers (e.g. frameo-pro) bump their @openpanel/web version and CDN propagates the new bundle to users.
ayushjhanwar-png
added a commit
that referenced
this pull request
Jun 15, 2026
…erly (#318) Without this, the server falls back to `hash(salt + projectId + ip + ua)` to derive deviceId/sessionId. Every user behind the same NAT (office WiFi, home WiFi, mobile carrier NAT) hashes to the same id — their sessions and replay timelines get merged together. Previous attempt (PR #315, reverted in #317) had the SDK generate its own session_id and the server trust it. That bypassed the `sessions` table entirely — chunks landed in `session_replay_chunks` with an id that had no matching row in `sessions`, so the dashboard's "session details" page returned 404. This PR avoids that by keeping session_id server-issued and only changing the deviceId source. Three small changes: 1. packages/sdks/web/src/index.ts - On init, read or create `_op_device_id` in localStorage and stash it on the base SDK as `this.deviceId`. - Call `setGlobalProperties({ __deviceId })` so every event carries the override. The server's track flow already honors this (line 146-155 — pre-existing code path). - Consumers can override with a stable identifier (e.g. Firebase UID) by calling `setGlobalProperties({ __deviceId: user.uid })` after auth resolves. That's strictly better than the auto-generated UUID because it stitches the same human across browsers / devices. - Falls back to legacy IP+UA path if storage is unavailable (sandboxed iframe, quota exhausted, private mode quirks). 2. packages/sdks/sdk/src/index.ts - `fetchDeviceId()` now passes `this.deviceId` as `?deviceId=` query param so the server-side device-id lookup uses our override instead of recomputing from IP+UA. Returns the matching session id so replay chunks reference the correct row in `sessions`. 3. apps/api/src/controllers/track.controller.ts - `fetchDeviceId` endpoint accepts the optional `?deviceId=` query param. When supplied, skips the IP+UA hash and looks up the session directly by the client-supplied id. When omitted (old SDKs), keeps the existing IP+UA derivation — fully backward compatible. - Track ingest flow itself is UNCHANGED; the `__deviceId` override mechanism it uses (line 146-155) has been there all along. What this fixes: - Office colleagues no longer share session/replay timelines (each browser has its own localStorage UUID, or its own Firebase UID). - Sessions table and session_replay_chunks now share the same session_id because both are server-issued (looked up via the same deviceId). Dashboard "session details" page will find the row this time. - Track-event attribution is also corrected (was NAT-merged before; now per-browser / per-user). What it doesn't change: - Track ingestion HTTP path, controller logic, buffer flow — all unchanged. - Old SDKs without `__deviceId` still hit the IP+UA fallback — no regression for clients that haven't upgraded. - Historical events stay keyed by their old IP+UA hash. New events post-upgrade get the corrected key (expected migration behavior). Bumps @openpanel/web 1.2.0-local -> 1.3.0-local for downstream publish.
2 tasks
ayushjhanwar-png
added a commit
that referenced
this pull request
Jun 15, 2026
Companion to #318. The replay handler still derived a session by recomputing deviceId from IP+UA, which overrode the SDK-provided session_id (and re-introduced NAT-collision behavior on the replay storage path even when the SDK was using the deviceId override correctly on the track path). With #318 shipped, the SDK's session_id is no longer self-generated: it comes from /track/device-id?deviceId=<our localStorage UUID or stable user id>, which is the same id the track flow uses to create sessions. So the SDK-supplied session_id is now a real row in the `sessions` table and trusting it is safe — which fixes the "Session not found" dashboard error from the earlier attempt (#315 / #317). Behavior: - payload.session_id present (modern SDKs): trust it. Chunks land under the same session row that the track flow created. Dashboard "session details" works. - payload.session_id missing (legacy SDKs): fall back to the IP+UA derivation that's been there all along. No regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The server's default replay `session_id` derivation hashes `salt + projectId + ip + user-agent`. Every user behind the same NAT (office, home WiFi, mobile carrier) collapses to the same `session_id` — their replay timelines get merged into one shared session.
Verified in our Dashverse office: multiple colleagues' recordings of Frameo showed up under the same replay id, viewable as one merged timeline.
Why upstream's suggested workaround doesn't fix this
Upstream is aware of the broader fingerprinting bug (issue #337). The maintainer suggested using the existing `__deviceId` property override:
BUT — the `__deviceId` override only affects `track` / `identify` event flows. The replay handler ignores property overrides and recomputes `deviceId` from IP+UA directly to look up the session. So `__deviceId` does not fix replay session merging.
The fix (two parts, scoped to replay only)
1. SDK side — `packages/sdks/web/src/index.ts`
2. Server side — `apps/api/src/controllers/track.controller.ts`
Trust the SDK-provided id when present; only derive from IP+UA as a fallback for SDKs that don't send one.
What this does NOT change
Only the replay session_id derivation is altered, and only when the SDK sends one.
Backward-compatibility / deployment safety
This is safe to ship to prod independently — frameo-pro and other consumers can update the SDK on their own timeline.
The only requirement: ship the server change first (this PR), then the SDK. Doing it in the opposite order would have the SDK send a UUID that the old server would override.
Trade-offs we accept
Test plan
Related