Skip to content

Keep NWC connection secrets out of localStorage#436

Open
ayushshrivastv wants to merge 6 commits into
shopstr-eng:mainfrom
ayushshrivastv:secure-nwc-credential-storage
Open

Keep NWC connection secrets out of localStorage#436
ayushshrivastv wants to merge 6 commits into
shopstr-eng:mainfrom
ayushshrivastv:secure-nwc-credential-storage

Conversation

@ayushshrivastv

@ayushshrivastv ayushshrivastv commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

The security issue here was that Shopstr was storing the full Nostr Wallet Connect connection string directly in localStorage, including the secret parameter inside the nostr+walletconnect://... URL. That means the wallet credential was being treated like normal browser data even though it is actually a sensitive secret.

Because localStorage is readable by any JavaScript running in the Shopstr origin, this created a real secret exposure risk. If there is any XSS, unsafe script execution, compromised dependency, or malicious injected script in the browser context, that stored NWC secret could be read and reused to perform wallet actions within the permissions of that connection. So the issue was not just “metadata in storage,” it was persistent client-side storage of a live wallet credential.

Safe reproduction: connect any NWC wallet in Settings, then open DevTools and read localStorage.getItem("nwcString"). The full nostr+walletconnect://... URL, including the secret parameter, is exposed in browser readable storage and later reused by checkout flows.

What Changed?

saveNWCString() no longer writes to localStorage; it keeps the value in runtime memory only.

  • saveNWCInfo() does the same for wallet metadata.
  • getLocalStorageData() now deletes legacy nwcString and nwcInfo entries from
    localStorage on read.
  • The two write sites in pages/onboarding/wallet.tsx and pages/settings/nostr-
    wallet-connect.tsx were updated to use the new in-memory path.

@vercel

vercel Bot commented Apr 19, 2026

Copy link
Copy Markdown

@ayushshrivastv is attempting to deploy a commit to the shopstr-eng Team on Vercel.

A member of the Team first needs to authorize it.

@calvadev

Copy link
Copy Markdown
Collaborator

Does this require re-inputting the NWC connectionstribg for every session? Because if so this is not good for usability. The string should persist, otherwise it's very clunky to support and use.

A better method for this would be to encryot the stored data using user passphrase or similar.

@ayushshrivastv

Copy link
Copy Markdown
Contributor Author

@calvadev I handled the immediate fix for the current issue, but I agree the better long term solution here is encrypted persistence rather than forcing a full reconnect every session.

Are you suggesting we should store the NWC connection only as an encrypted blob, with the decryption key coming from something user controlled like a passphrase or WebAuthn style unlock, so the app cannot silently decrypt it on its own? Then in later sessions, Shopstr could show that a wallet is already configured and ask the user to unlock it instead of reconnecting from scratch. After unlock, the decrypted NWC string would stay only in memory for the active session, while only non sensitive metadata remains persisted.

@calvadev

calvadev commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator

@calvadev I handled the immediate fix for the current issue, but I agree the better long term solution here is encrypted persistence rather than forcing a full reconnect every session.

Are you suggesting we should store the NWC connection only as an encrypted blob, with the decryption key coming from something user controlled like a passphrase or WebAuthn style unlock, so the app cannot silently decrypt it on its own? Then in later sessions, Shopstr could show that a wallet is already configured and ask the user to unlock it instead of reconnecting from scratch. After unlock, the decrypted NWC string would stay only in memory for the active session, while only non sensitive metadata remains persisted.

Yes, similar to how the nsec is handled. It's encrypted using the user inputted passphrase and only when necessary the user is prompted and the raw nsec is handled in-memory for the actions needed.

@ayushshrivastv

Copy link
Copy Markdown
Contributor Author

Yes, similar to how the nsec is handled. It's encrypted using the user inputted passphrase and only when necessary the user is prompted and the raw nsec is handled in-memory for the actions needed.

I’ll align this patch with how nsec is handled.

@ayushshrivastv

Copy link
Copy Markdown
Contributor Author

Yes, similar to how the nsec is handled. It's encrypted using the user inputted passphrase and only when necessary the user is prompted and the raw nsec is handled in-memory for the actions needed.

Also For nsec, Shopstr already had a signer flow where the encrypted private key is stored, the passphrase is asked only when needed, and the raw secret is handled in memory for signing actions. That logic lives in nostr-nsec-signer.ts. For NWC, before this PR, the raw connection string was just being saved directly in localStorage, so there was no similar protection.

What I changed is that NWC now follows the same security pattern, but it is still a different type of secret and flow. Instead of a signer decrypting an nsec to sign Nostr events, the app now decrypts the saved NWC connection only when it needs to create a wallet provider for payment actions.

One small difference still is that nsec protection is implemented inside the signer class itself, while NWC currently uses the new NWCContext wrapper to manage encrypted storage, unlock, lock, and migration. So the behavior is now similar

@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had recent activity for 3 weeks.
It will be closed if no further activity occurs within 7 days.
If this PR is still relevant, please leave a comment or push new commits to keep it open.
Thank you for your contribution!

@github-actions github-actions Bot added the stale label Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants