Avoid persisting Cashu proofs in localStorage - #511
ayushshrivastv wants to merge 16 commits into
Conversation
…alStorage under tokens. Anyone with DevTools access can open Application storage or run localStorage.getItem("tokens") and copy spendable wallet proofs, which can lead to fund theft.
[_app.tsx](https://github.com/shopstr-eng/shopstr/blob/e1f7dc33b8d75201f55d223941a3a18e6599699e/pages/_app.tsx#L786-L788)
|
@calvadev can you review it once whenever feasible? |
|
This is a good start! Been reviewing to make sure there aren't any regressions in functionality. Will be dropping a comment on what to revise. |
|
The claim paths call publishProofEvent(...) before setCachedCashuProofs(...), with no try/catch. A mint quote can only be minted once, so if that publish throws on a relay error, the claim is skipped, the pending quote stays open, and the next boot re-attempts the mint -> the mint returns "already issued" -> the recovery state machine marks it failed_terminal. That's a real fund-loss window. It's made worse by the callers (product-invoice-card.tsx / cart-invoice-card.tsx) not wrapping the recovery call either, so the throw also skips markMintQuoteClaimed and the user-facing modal. Maybe credit the in-memory cache first, then make the publish best-effort. Durability is already guaranteed by the encrypted retry queue, and setCachedCashuProofs de-dupes by proof secret. There's also still a narrow window where if the relay publish and the encrypted-queue write both fail at the same instant, proofs live only in the cache until reload. We could close it later by having publishProofEvent return { published, queued } and only completing the claim once at least one durable path succeeded. |
The mint claim path now credits the Cashu proof cache first, then marks the pending quote claimed, then does the Nostr publish as best effort. So a relay error won’t keep the quote open anymore, and the next boot won’t hit the “already issued” path and mark it The encrypted retry queue still handles failed publishes, and the cache write uses the existing proof-secret de-dupe behavior. The remaining “publish + queue write both fail” window is still a follow up. A cleaner fix there would be to make |
|
@ayushshrivastv please solve the conflicts! |
…cashu-proof-localstorage # Conflicts: # components/cart-invoice-card.tsx # components/product-invoice-card.tsx # components/utility-components/claim-button.tsx # components/wallet/__tests__/receive-button.test.tsx # components/wallet/mint-button.tsx # components/wallet/pay-button.tsx # components/wallet/receive-button.tsx # pages/_app.tsx
Resolved! |
There was a problem hiding this comment.
Pull request overview
This PR hardens Shopstr’s Cashu wallet handling by removing spendable proof persistence from localStorage["tokens"] and replacing it with a module-level in-memory cache (plus migration + retry mechanisms) to reduce the risk of proof exfiltration via DevTools.
Changes:
- Replaces most
localStorage["tokens"]writes with an in-memory Cashu proof cache (setCachedCashuProofs/getCachedCashuProofs) and updates wallet flows to read from it. - Adds legacy-proof handling (read/validate/migrate/remove) and introduces an encrypted localStorage-backed queue for retrying failed proof publishes.
- Tightens error propagation in proof publishing + DB event caching and updates tests to reflect new behavior.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/nostr/nostr-helper-functions.ts | Introduces proof cache + legacy token helpers + pending publish retry queue; changes publishProofEvent error semantics. |
| utils/nostr/fetch-service.ts | Updates wallet fetch to read proofs from the in-memory cache. |
| utils/nostr/tests/nostr-helper-functions.test.ts | Adjusts expectations for getLocalStorageData().tokens and publishProofEvent error behavior. |
| utils/nostr/tests/local-storage-data.test.ts | Adds coverage for volatile proofs, legacy proof filtering/removal, and pending publish retry queue. |
| utils/nostr/tests/fetch-service.test.ts | Updates mocks to reflect getCachedCashuProofs usage. |
| utils/db/db-client.ts | Makes cacheEventToDatabase throw on failure (instead of only logging). |
| utils/db/tests/db-client.test.ts | Updates tests to expect thrown errors/rethrows. |
| utils/cashu/wallet-recovery.ts | Switches recovery bookkeeping to credit volatile cache and best-effort publish proof events. |
| utils/cashu/pending-mint-operations.ts | Updates recovery contract semantics (proof persistence vs best-effort publish). |
| utils/cashu/local-wallet-cache.ts | Adds helper to credit proofs to in-memory cache and write history entry. |
| utils/cashu/legacy-proof-migration.ts | Adds migration routine to publish legacy proofs and remove them from localStorage after persistence. |
| utils/cashu/tests/wallet-recovery.test.ts | Updates tests for volatile cache + history behavior during recovery. |
| utils/cashu/tests/pending-mint-operations.test.ts | Adjusts tests around recovery success/failure criteria. |
| utils/cashu/tests/legacy-proof-migration.test.ts | Adds tests for legacy proof migration outcomes. |
| pages/wallet/index.tsx | Wallet page now reads proofs via getCachedCashuProofs() instead of localStorage tokens. |
| pages/settings/community.tsx | Removes an unnecessary eslint disable directive. |
| pages/_app.tsx | Hydrates proof cache from wallet fetch, runs legacy migration, and logs migration results. |
| components/wallet/send-button.tsx | Stops writing proofs to localStorage; updates proof cache after sends. |
| components/wallet/receive-button.tsx | Stops writing proofs to localStorage; updates proof cache after receives. |
| components/wallet/pay-button.tsx | Stops writing proofs to localStorage; updates proof cache after melts/payments. |
| components/wallet/mint-button.tsx | Credits volatile cache + history, marks quote claimed, then best-effort publishes proof event. |
| components/wallet/tests/send-button.test.tsx | Updates tests to assert cache updates instead of localStorage token writes. |
| components/wallet/tests/receive-button.test.tsx | Updates tests to assert cache updates instead of localStorage token writes. |
| components/wallet/tests/pay-button.test.tsx | Updates tests to assert cache updates instead of localStorage token writes. |
| components/wallet/tests/mint-button.test.tsx | Updates tests to assert cache updates instead of localStorage token writes. |
| components/utility-components/mint-recovery-boot.tsx | Adds retry for queued proof publishes and uses recovery helper to credit volatile cache. |
| components/utility-components/migration-prompt-modal.tsx | Changes error rendering to an explicit role="alert" block. |
| components/utility-components/claim-button.tsx | Stops writing proofs to localStorage; updates proof cache after claim/receive/refund flows. |
| components/utility-components/tests/migration-prompt-modal.test.tsx | Updates alert expectations for new error rendering. |
| components/utility-components/tests/claim-button.test.tsx | Updates tests to assert cache updates instead of localStorage token writes. |
| components/storefront/storefront-wallet.tsx | Storefront wallet display now reads proofs via getCachedCashuProofs(). |
| components/product-invoice-card.tsx | Stops writing proofs to localStorage; updates cache after Cashu payments. |
| components/cart-invoice-card.tsx | Stops writing proofs to localStorage; updates cache after Cashu payments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@calvadev can u review it once? |
Shopstr saves decrypted Cashu wallet proofs directly into browser localStorage under tokens. Anyone with DevTools access can open Application storage or just run localStorage.getItem("tokens") and copy spendable wallet proofs... which can lead to fund theft.
shopstr/pages/_app.tsx
Lines 786 to 788 in e1f7dc3
the core change is: stop writing spendable Cashu proofs to localStorage["tokens"] and instead hold them in a module level in memory variable, accessed via a
setCachedCashuProofs()setter exported fromnostr-helper-functions.ts.Every
localStorage.setItem("tokens", JSON.stringify(...))across the codebase across 9 components is replaced withsetCachedCashuProofs(...), legacy localStorage tokens are migrated, and transaction critical wallet paths no longer depend ongetLocalStorageData().tokens. This avoids exposing spendable proofs through browser storage.