Skip to content

fix: fetch Crisp support token from the API instead of deriving it client-side - #2666

Merged
jjramirezn merged 2 commits into
mainfrom
hotfix/crisp-token-server-side
Aug 11, 2026
Merged

fix: fetch Crisp support token from the API instead of deriving it client-side#2666
jjramirezn merged 2 commits into
mainfrom
hotfix/crisp-token-server-side

Conversation

@jjramirezn

@jjramirezn jjramirezn commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Frontend half of the Crisp support-token IDOR hotfix. Pairs with peanut-api-ts #1325 — that API PR (and its migration) must deploy first.

The Crisp session token was derived in the browser as SHA-256("peanut-crisp-session-v1:" + userId). The salt ships in the bundle and userId is readable, so anyone could reproduce any user's token and open their support thread via /crisp-proxy?crisp_token_id=<token>.

This PR removes the client-side derivation. useCrispTokenId now fetches the token from the authenticated GET /user/crisp-token (the API issues a stored per-user random token — Crisp's documented safe pattern). The hook keeps the same contract — undefined until it resolves, in-memory cache per userId — so SupportDrawer's isAwaitingToken gate and reset logic are unchanged.

Hardening from code review is included:

  • Reset on account switch — the token is reset to the current user's value before fetching, so it never serves the previous user's token while the new one loads.
  • Identity check — the route echoes userId; the hook discards any token whose userId doesn't match the account it is fetching for (stale-bearer guard).
  • Bounded retry — a transient failure doesn't strand the support drawer for the session.

Task

TASK link pending — user security report. Will attach.

Risks / breaking changes

  • Hard dependency on peanut-api-ts [TASK-15768] feat: Clickable PFPs in txn receipt #1325. The endpoint (and its migration) must be live first, or GET /user/crisp-token 404s and support chat stays on its loading state.
  • No visible UI change — same drawer, same states; only the token's source changed.
  • Residual (follow-up, not here): on a persistent endpoint failure the drawer still shows the spinner with no fallback UI — a small SupportDrawer change, kept out of this security PR.

QA

  • Unit (src/hooks/__tests__/useCrispTokenId.test.ts): fetches from the authed endpoint, passes no userId parameter, discards a token minted for a different user, drops the previous user's token on account switch, and stays undefined (with retry) on failure.
  • SupportDrawer suite still green (it mocks the hook).

Screenshots

N/A (no visible change) — the drawer renders identically; only the token's source changed.

Summary by CodeRabbit

  • New Features

    • Crisp tokens are now securely retrieved from the authenticated service.
    • Added validation to ensure tokens match the correct account and token type.
    • Added automatic retry handling for temporary token request failures.
    • Tokens are cleared when switching accounts or when authentication is unavailable.
  • Tests

    • Added coverage for authentication states, account changes, token validation, and retry behavior.

…ient-side

The token was computed in the browser as SHA-256 of a salt that ships in the
client bundle plus the userId, both public. Anyone could reproduce any user's
token and open their support conversation to read it and post as them.

Fetch it from the new authenticated GET /user/crisp-token instead, which
derives it server-side with a secret the browser never sees and issues it only
to the authenticated caller. Same undefined-until-resolved contract and cache,
so SupportDrawer's token gate is unchanged.

Requires the peanut-api-ts endpoint to be deployed first.
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
peanut-wallet Ready Ready Preview Aug 11, 2026 12:44pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The useCrispTokenId hook now retrieves Crisp tokens from the authenticated /user/crisp-token API. It validates responses, retries failed requests, handles cancellation and account changes, caches valid tokens, and includes tests for these behaviors.

Changes

Crisp token retrieval

Layer / File(s) Summary
Authenticated token fetch and lifecycle handling
src/hooks/useCrispTokenId.ts
The hook replaces deterministic browser token generation with authenticated API retrieval. It validates the echoed user ID and token type, retries failed requests up to three times, caches valid tokens, resets state on account changes, and cancels pending work.
Token retrieval behavior coverage
src/hooks/__tests__/useCrispTokenId.test.ts
Tests cover unauthenticated behavior, authenticated retrieval without a user ID argument, mismatched server identity, account switching, and persistent endpoint failures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Auth
  participant useCrispTokenId
  participant CrispTokenAPI
  Auth->>useCrispTokenId: provide authenticated user
  useCrispTokenId->>CrispTokenAPI: request /user/crisp-token
  CrispTokenAPI-->>useCrispTokenId: return user ID and token type
  useCrispTokenId->>useCrispTokenId: validate and cache token
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fetching the Crisp support token from the API instead of deriving it client-side.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/crisp-token-server-side

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Code-analysis diff

Painscore total: 7137.94 → 7138.12 (+0.18)
Findings: +1 net (+1 new, -0 resolved)

🆕 New findings (1)

  • medium complexity — src/hooks/useCrispTokenId.ts — CC 19, MI 64.82, SLOC 55

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

🧪 UI test report — ✅ all green

Suites

  • unit: 2917 ran, 0 failed, 0 skipped, 50.4s

📊 Coverage (unit)

metric %
statements 66.1%
branches 51.2%
functions 56.2%
lines 66.9%
⏱ 10 slowest test cases
time test
3.9s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › never places two stickers in heavy overlap (broad seed sweep)
1.0s src/utils/__tests__/demo-api.test.ts › isDemoMode() is false when not running under Capacitor
0.4s src/app/actions/__tests__/api-headers.test.ts › should include Content-Type in validateInviteCode
0.3s src/utils/__tests__/sentry.utils.test.ts › defaults to the client budget under a browser global
0.3s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › every sticker stays within canvas at any count
0.3s src/utils/__tests__/auth-token.test.ts › authReady does not park — hydrates the plain token without an unlock
0.3s src/hooks/__tests__/useCrispTokenId.test.ts › retries then stays undefined when the endpoint keeps failing (no fallback token)
0.3s src/app/(mobile-ui)/withdraw/__tests__/withdraw-states.test.tsx › Bank withdrawal keeps the $1 minimum for sub-$1 amounts
0.3s src/utils/__tests__/sentry.utils.test.ts › still lets a per-call timeoutMs win over the default
0.3s src/utils/__tests__/auth-token.test.ts › ignores the guarded marker and falls back to the plain token
📍 Inline annotations are in the **Unit test report** check above. Coverage artifact: `coverage-unit`. Generated by `.github/workflows/tests.yml`.

@jjramirezn

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/hooks/useCrispTokenId.ts`:
- Around line 48-61: Update src/hooks/useCrispTokenId.ts lines 48-61 to
associate cached tokens with their owning userId, clear token state before
uncached requests and on invalid or failed responses, and return undefined
unless the stored token belongs to the current userId. Update
src/hooks/__tests__/useCrispTokenId.test.ts lines 29-49 by adding a rerender
scenario that switches users while the new token request is pending, failed, or
invalid, asserting undefined until the new user’s token resolves.
🪄 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: ec15b4aa-9825-4dd5-af90-44e64cb264e0

📥 Commits

Reviewing files that changed from the base of the PR and between d3f83ad and c1fac47.

📒 Files selected for processing (2)
  • src/hooks/__tests__/useCrispTokenId.test.ts
  • src/hooks/useCrispTokenId.ts

Comment thread src/hooks/useCrispTokenId.ts
…sient failures

Address code review of the client hook:
- Reset the token to the current user's cached value (or undefined) before
  fetching, so an account switch never serves the previous user's token while
  the new one loads (cross-user exposure window).
- Verify the userId the route echoes matches the account we fetch for, so a
  stale auth bearer can't bind the widget to another user.
- Retry the fetch a few times, so a transient blip doesn't strand the support
  drawer's loading gate for the whole session.
@jjramirezn

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@jjramirezn
jjramirezn marked this pull request as ready for review August 11, 2026 13:25

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/hooks/__tests__/useCrispTokenId.test.ts`:
- Around line 83-84: Update the retry test around the waitFor assertion to wait
for the configured three-request sequence to complete, rather than proceeding
once the second request starts. Use controlled timers or assert exactly three
API calls, then verify result.current remains undefined.
🪄 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: bee65b13-bfb6-413a-8fbb-470ba1e48bb1

📥 Commits

Reviewing files that changed from the base of the PR and between c1fac47 and bfdb460.

📒 Files selected for processing (2)
  • src/hooks/__tests__/useCrispTokenId.test.ts
  • src/hooks/useCrispTokenId.ts

Comment on lines +83 to +84
await waitFor(() => expect(apiFetchMock.mock.calls.length).toBeGreaterThan(1), { timeout: 3000 })
expect(result.current).toBeUndefined()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Wait for the configured retry sequence to finish.

Line 83 passes after the second request starts. The test does not verify that all three attempts complete before the hook remains undefined.

Use controlled timers or wait for exactly three calls. Then assert that no token is returned.

🤖 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/hooks/__tests__/useCrispTokenId.test.ts` around lines 83 - 84, Update the
retry test around the waitFor assertion to wait for the configured three-request
sequence to complete, rather than proceeding once the second request starts. Use
controlled timers or assert exactly three API calls, then verify result.current
remains undefined.

@jjramirezn
jjramirezn merged commit 3a50852 into main Aug 11, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants