fix: pass Crisp proxy user data via postMessage handshake instead of URL query string - #2669
Conversation
…essage handshake The crisp-proxy iframe URL carried email, legal name, userId, wallet and provider links, plus the Crisp session token (a bearer credential after the server-side token change). A query string leaks into Vercel logs, browser history, Referer headers, and the $current_url of every analytics event fired from the iframe — confirmed stored in PostHog. Postmortem F5, TASK-21353. The proxy now asks its parent for the init payload (CRISP_PROXY_REQUEST_INIT) and boots Crisp only when the reply lands, so the parent can never post before the iframe listens — same reliability the URL transport was built for, with nothing identifying in the URL.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCrisp initialization now uses a same-origin ChangesCrisp initialization flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SupportDrawer
participant CrispProxyPage
participant CrispRuntime
SupportDrawer->>CrispProxyPage: Send CRISP_PROXY_INIT payload
CrispProxyPage->>CrispRuntime: Configure session and inject script
CrispRuntime-->>CrispProxyPage: Report load and readiness status
CrispProxyPage-->>SupportDrawer: Report ready or failed state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/Global/SupportDrawer/index.tsx (1)
47-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the ref write out of the render body.
Line 48 assigns
initPayloadRef.currentduring render. React 19 can discard or replay a render, so a payload built from state that never commits can leak into the reply. Compute the payload withuseMemoand sync the ref in an effect. The proxy re-requests init every 250 ms, so a one-tick delay in the ref update does not break the handshake.♻️ Proposed refactor
- const initPayloadRef = useRef<CrispInitPayload>({ locale: 'en' }) - initPayloadRef.current = { - locale: CRISP_LOCALE_BY_APP_LOCALE[locale] ?? 'en', - tokenId: crispTokenId, - email: userData.email, - nickname: userData.fullName || userData.username, - avatar: userData.avatar, - sessionData: hasSessionData - ? { - username: userData.username || '', - user_id: userData.userId || '', - full_name: userData.fullName || '', - wallet_address: userData.walletAddressLink || '', - bridge_user_id: userData.bridgeCustomerLink || '', - manteca_user_id: userData.mantecaUserId || '', - posthog_person: userData.posthogPersonLink || '', - } - : undefined, - prefilledMessage, - } + const initPayload = useMemo<CrispInitPayload>( + () => ({ + locale: CRISP_LOCALE_BY_APP_LOCALE[locale] ?? 'en', + tokenId: crispTokenId, + email: userData.email, + nickname: userData.fullName || userData.username, + avatar: userData.avatar, + sessionData: hasSessionData + ? { + username: userData.username || '', + user_id: userData.userId || '', + full_name: userData.fullName || '', + wallet_address: userData.walletAddressLink || '', + bridge_user_id: userData.bridgeCustomerLink || '', + manteca_user_id: userData.mantecaUserId || '', + posthog_person: userData.posthogPersonLink || '', + } + : undefined, + prefilledMessage, + }), + [locale, crispTokenId, userData, hasSessionData, prefilledMessage] + ) + const initPayloadRef = useRef<CrispInitPayload>(initPayload) + useEffect(() => { + initPayloadRef.current = initPayload + }, [initPayload])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Global/SupportDrawer/index.tsx` around lines 47 - 66, Move the `initPayloadRef.current` assignment out of the render body in `SupportDrawer` and compute the payload with `useMemo` from its existing locale, user, session, token, and message dependencies. Synchronize the ref to the memoized payload in a `useEffect`, preserving the current payload fields and allowing the existing proxy handshake to read the committed value.Source: Linters/SAST tools
src/app/crisp-proxy/page.tsx (1)
122-126: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider storing a digest instead of the raw token id.
crisp_last_token_idholds the Crisp session-continuity token, whichCrispInitPayloaddocuments as a bearer credential. The code needs equality comparison only, not the value itself. Storing a digest would remove the credential fromlocalStoragewhile keeping the identity-change detection at line 56.Note that the static analysis advice to use an
HttpOnlycookie does not apply here. The comparison runs entirely in the iframe with no server involved, and the parent already derives the token client-side. The residual risk is therefore low, so treat this as a hardening step rather than a blocker.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/crisp-proxy/page.tsx` around lines 122 - 126, Update the localStorage persistence and corresponding comparison for crisp_last_token_id to use a one-way digest of the Crisp token rather than the raw token value, while preserving identity-change detection. Reuse the same deterministic digest operation when reading and writing so existing-token comparisons remain consistent, and keep the existing storage error handling unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/crisp-proxy/page.tsx`:
- Around line 154-157: Update the watchdog failure branch near the CRISP_FAILED
notification to call clearInterval(requestTimer) before reporting failure,
ensuring the init-request loop stops when the parent does not respond. Keep the
existing successful-init and direct-visit interval behavior unchanged.
In `@src/components/Global/SupportDrawer/index.tsx`:
- Around line 279-283: Update the iframe key in the SupportDrawer render path to
include the current locale alongside iframeKey, crispTokenId, and
prefilledMessage, ensuring locale changes remount the proxy iframe and refresh
its Crisp payload.
---
Nitpick comments:
In `@src/app/crisp-proxy/page.tsx`:
- Around line 122-126: Update the localStorage persistence and corresponding
comparison for crisp_last_token_id to use a one-way digest of the Crisp token
rather than the raw token value, while preserving identity-change detection.
Reuse the same deterministic digest operation when reading and writing so
existing-token comparisons remain consistent, and keep the existing storage
error handling unchanged.
In `@src/components/Global/SupportDrawer/index.tsx`:
- Around line 47-66: Move the `initPayloadRef.current` assignment out of the
render body in `SupportDrawer` and compute the payload with `useMemo` from its
existing locale, user, session, token, and message dependencies. Synchronize the
ref to the memoized payload in a `useEffect`, preserving the current payload
fields and allowing the existing proxy handshake to read the committed value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7c6dce6-be0f-49c0-bf9b-db83848df5f7
📒 Files selected for processing (6)
src/app/crisp-proxy/page.tsxsrc/components/Global/SupportDrawer/__tests__/SupportDrawer.test.tsxsrc/components/Global/SupportDrawer/index.tsxsrc/constants/crisp.tssrc/hooks/useCrispProxyUrl.tssrc/types/global.d.ts
💤 Files with no reviewable changes (1)
- src/hooks/useCrispProxyUrl.ts
…, harden handshake /code-review findings: the handshake was one-shot, so identity fields resolving after boot (email/name during onboarding) or a new prefill never reached Crisp — the old URL transport remounted on any change. The parent now re-sends the payload when it changes and the proxy applies it live; token/locale changes still remount via the iframe key. Also: reuse setCrispUserData (util was the canonical setter; the payload now carries CrispUserData whole), shared handshake message constants, watchdog clears the 250ms request loop on failure, payload ref written in an effect instead of during render.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Global/SupportDrawer/index.tsx`:
- Around line 174-180: Restrict the CRISP_PROXY_REQUEST_INIT_MSG handling in the
SupportDrawer message listener to requests whose event.source exactly matches
iframeRef.current?.contentWindow. Only post CRISP_PROXY_INIT_MSG with
initPayloadRef.current after this source check, while preserving the existing
origin and message-type validation.
- Around line 281-286: Update the iframe remount flow around the key using
crispTokenId and crispLocale so token or locale changes reset both isCrispReady
and isCrispFailed before the new proxy loads. Preserve the existing status
handling for non-remount updates, ensuring the loader remains visible until the
replacement iframe reports its status.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 430d45a9-1547-4a6f-a2ff-d70d257bdea8
📒 Files selected for processing (4)
src/app/crisp-proxy/page.tsxsrc/components/Global/SupportDrawer/__tests__/SupportDrawer.test.tsxsrc/components/Global/SupportDrawer/index.tsxsrc/constants/crisp.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Global/SupportDrawer/tests/SupportDrawer.test.tsx
- src/app/crisp-proxy/page.tsx
…/locale remount CodeRabbit round 2: gate CRISP_PROXY_INIT replies on event.source matching our iframe's contentWindow so no other same-origin frame can pull the token and user data; clear isCrispReady/isCrispFailed when a token or locale change swaps the iframe, so the loader shows until the new proxy reports.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Code-analysis diffPainscore total: 7125 → 7119.83 (-5.17) 🆕 New findings (17)
✅ Resolved (19)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
Jota's review: a background user refresh re-fired the update effect and setCrispUserData unconditionally re-pushed message:text, overwriting whatever the user was typing in the composer. Prefill now applies at boot and, on updates, only when its value actually changed (a new support entry point) — metadata refreshes never touch the composer.
|
@jjramirezn addressed your review in 7816e11 — prefill is now applied at boot only; live updates re-push message:text only when the prefill value actually changed (new support entry point), so a background user refresh can no longer overwrite in-progress typing. Thread replied + resolved, CI green. Re-requested your review. |
main moved the Crisp proxy off the URL transport onto a postMessage handshake (PR #2669, useCrispProxyUrl deleted) and routes the iframe through setCrispUserData, so the verification fields this branch added to crisp-proxy/page.tsx and useCrispProxyUrl are now duplicated by the utils/crisp.ts mapping, which survives the merge unchanged.
Summary
The crisp-proxy iframe URL carried the user's email, legal name, userId, username, wallet/Arbiscan link, Bridge dashboard link, PostHog person link — and the Crisp session token, which after #2666/api#1325 is a per-user bearer credential. A query string is not a private channel: it rides into Vercel runtime logs, browser history, Referer headers, and the
$current_urlof every analytics event fired from the iframe document. Verified in PostHog: ~10k events in the last 7 days store the full PII query string in$current_url.This PR replaces the URL transport with a postMessage channel:
CRISP_PROXY_REQUEST_INIT, re-asked every 250ms until answered) and boots Crisp only when the reply (CRISP_PROXY_INIT) lands. The iframe initiates, so the parent can never post before the page listens — the timing problem the URL approach was built to avoid is solved structurally.setCrispUserData— the old transport handled this by remounting the whole embedded app on every URL change. Token/locale changes still remount via the iframe key (they need a session re-bind)./crisp-proxy.useCrispProxyUrlis deleted. Direct visits (no parent) boot an anonymous session, as before.CrispUserDatawhole; the proxy reusessetCrispUserDatafromutils/crisp.ts(the canonical setter) instead of a third hand-rolled field mapping. Handshake message types are shared constants inconstants/crisp.ts.crisp_needs_reset,crisp_last_token_id,CRISP_RESET_SESSION), the 8s readiness watchdog, and theCRISP_READY/CRISP_FAILEDcontract are unchanged. A never-answered handshake ends inCRISP_FAILED→ existing retry UI, and the watchdog stops the request loop it declared dead.Task
TASK-21353 — https://app.notion.com/p/peanutprotocol/F5-Remove-user-PII-from-Crisp-proxy-URLs-3b983811757981ce8f88e5accd6e094a (postmortem F5, probing-campaign-2026-08-10).
Risks / breaking changes
maindirectly. It only changes how the already-fetched token and user data reach the iframe.$current_urluntil this PR deploys — which makes the token-rotation follow-up below load-bearing, not optional.Design notes / accepted trade-offs
__crispProxyBooted); a retry remounts the iframe and gets a fresh window.Follow-ups (not in this PR)
users.crisp_token(null the column) for users who loaded the proxy between api#1325's deploy and this fix — those bearer tokens are in Vercel logs/PostHog$current_url. Cheap to do wholesale: Jota's legacy-session purge already resets in-widget history at rollout, so a full-column rotation adds no extra user-visible cost if done in the same window./crisp-proxyat all (duplicate pageviews, no analytics value — and it was the egress amplifier here).QA
SupportDrawersuite extended — handshake reply carries token +CrispUserDatato the asking window only; foreign-origin requests ignored; iframe src asserted PII-free. Full suite green (2916 passed), typecheck + build green, 0 new eslint warnings./crisp-proxy.Screenshots
N/A (no visible change) — the drawer renders identically; only the data transport changed.
Summary by CodeRabbit