✨ server: add panda signature#991
Conversation
🦋 Changeset detectedLatest commit: adf8f7d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds Panda-backed signature flows: GET /card can return SIWE/WebAuthn challenges; PATCH /card accepts SIWE or WebAuthn assertions and verifies signatures; Panda utilities (nonce, verify, typed-data recovery) and runtime Panda signature checks in webhook spend/collection flows are introduced; tests updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CardAPI as Card API (server/api/card.ts)
participant Panda as Panda Service (server/utils/panda.ts)
participant DB as Database
Client->>CardAPI: GET /card?scope=siwe
CardAPI->>Panda: getNonce(userId)
Panda-->>CardAPI: { nonce }
CardAPI->>DB: fetch card
DB-->>CardAPI: card data
CardAPI-->>Client: { challenge, encryptedPan?, ... }
Note over Client: User signs SIWE message
Client->>CardAPI: PATCH /card { method:"siwe", message, signature }
CardAPI->>Panda: verify(userId, { authType:"siwe", message, signature })
Panda-->>CardAPI: success / error
CardAPI-->>Client: { verification: "OK" } or error
sequenceDiagram
participant Client
participant CardAPI as Card API (server/api/card.ts)
participant Panda as Panda Service (server/utils/panda.ts)
participant DB as Database
Client->>CardAPI: GET /card?scope=webauthn
CardAPI->>DB: fetch card
DB-->>CardAPI: card data
CardAPI-->>Client: { challenge: authorizationStatement, encryptedPan?, ... }
Note over Client: User performs WebAuthn assertion
Client->>CardAPI: PATCH /card { method:"webauthn", assertion }
CardAPI->>Panda: verify(userId, { authType:"webauthn", assertion })
Panda-->>CardAPI: success / error
CardAPI-->>Client: { verification: "OK" } or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Panda signatures, implementing SIWE and WebAuthn authentication methods for card operations. Key changes include a challenge-response mechanism in the card API, signature verification in transaction hooks, and new utility functions for interacting with Panda signature endpoints. Feedback focuses on handling signature parsing errors more explicitly and externalizing the hardcoded signer address to an environment variable for better maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f5fca28-0eb4-4199-80e0-1f5962737c44
📒 Files selected for processing (6)
.changeset/quiet-tigers-sign.mdserver/api/card.tsserver/hooks/panda.tsserver/test/api/card.test.tsserver/test/utils/panda.test.tsserver/utils/panda.ts
|
✅ All tests passed. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/card/CardDetails.tsx (1)
41-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear cached PAN/CVC when encrypted fields disappear.
If a previous fetch already populated
details, and a later refetch returns acardwithoutencryptedPan/encryptedCvc, this branch does nothing and the old PAN/CVC stay rendered. Reset the local state in the false path before returning.💡 Suggested fix
useEffect(() => { - if (card?.encryptedPan && card.encryptedCvc) { - Promise.all([ - decrypt(card.encryptedPan.data, card.encryptedPan.iv, card.secret), - decrypt(card.encryptedCvc.data, card.encryptedCvc.iv, card.secret), - ]) - .then(([pan, cvc]) => { - setDetails({ pan, cvc }); - }) - .catch(reportError); - } + if (!card?.encryptedPan || !card.encryptedCvc) { + setDetails({ pan: "", cvc: "" }); + return; + } + Promise.all([ + decrypt(card.encryptedPan.data, card.encryptedPan.iv, card.secret), + decrypt(card.encryptedCvc.data, card.encryptedCvc.iv, card.secret), + ]) + .then(([pan, cvc]) => { + setDetails({ pan, cvc }); + }) + .catch(reportError); }, [card]);
♻️ Duplicate comments (1)
server/api/card.ts (1)
695-698:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce the canonical SIWE
uritoo.
GET /?scope=siweissues a message withuri: https://${domain}, butPATCH /never checksmessage.uri. A client can change only the URI, sign it, and still reach{ verification: "OK" }.💡 Suggested fix
- if (message.statement !== statement || message.chainId !== chain.id || message.domain !== domain){ + if ( + message.statement !== statement || + message.chainId !== chain.id || + message.domain !== domain || + message.uri !== `https://${domain}` + ) { return c.json({ code: "bad signature" }, 400); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 79c4d18c-7f6b-40e4-8f31-150defd9a192
📒 Files selected for processing (8)
.changeset/quiet-tigers-sign.mdserver/api/card.tsserver/hooks/panda.tsserver/test/api/card.test.tsserver/test/utils/panda.test.tsserver/utils/panda.tssrc/components/card/CardDetails.tsxsrc/utils/server.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/card/CardDetails.tsx (1)
41-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
detailswhen the encrypted blobs are absent.The new guard skips decryption, but it also leaves the previous PAN/CVC in state. If a refetch returns a card without
encryptedPan/encryptedCvc, the sheet can keep rendering stale secrets.🩹 Proposed fix
useEffect(() => { if (card?.encryptedPan && card.encryptedCvc) { Promise.all([ decrypt(card.encryptedPan.data, card.encryptedPan.iv, card.secret), decrypt(card.encryptedCvc.data, card.encryptedCvc.iv, card.secret), ]) .then(([pan, cvc]) => { setDetails({ pan, cvc }); }) .catch(reportError); + return; } + setDetails({ pan: "", cvc: "" }); }, [card]);
♻️ Duplicate comments (1)
server/api/card.ts (1)
695-707:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlso reject SIWE messages with a mismatched
uri.This path now binds
statement,chainId, anddomain, but it still accepts a message whoseuridiffers from the challenge returned byGET /. A client can change that part of the signed consent text and still reachpanda.verify(...).🛡️ Proposed fix
- if (m.statement !== statement || m.chainId !== chain.id || m.domain !== domain) { + if ( + m.statement !== statement || + m.chainId !== chain.id || + m.domain !== domain || + m.uri !== `https://${domain}` + ) { return false; }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2df97e21-0ca1-4418-92c0-e417dd6a6c01
📒 Files selected for processing (8)
.changeset/quiet-tigers-sign.mdserver/api/card.tsserver/hooks/panda.tsserver/test/api/card.test.tsserver/test/utils/panda.test.tsserver/utils/panda.tssrc/components/card/CardDetails.tsxsrc/utils/server.ts
1 participant
closes #714
Summary by CodeRabbit