Skip to content

Stop persisting serialized NIP-46 signer secrets in localStorage#437

Open
ayushshrivastv wants to merge 1 commit into
shopstr-eng:mainfrom
ayushshrivastv:secure-bunker-signer-storage
Open

Stop persisting serialized NIP-46 signer secrets in localStorage#437
ayushshrivastv wants to merge 1 commit into
shopstr-eng:mainfrom
ayushshrivastv:secure-bunker-signer-storage

Conversation

@ayushshrivastv

@ayushshrivastv ayushshrivastv commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Metadata Leakage: Shopstr persists the serialised NIP-46 bunker signer directly into localStorage via JSON.stringify(signer). For a bunker signer, that serialised object in cludes both the full bunker://... URL and the generated appPrivKey, which are the live secrets used to authenticate and continue the remote signing session.

That meant any script running in origin could read long lived bunker session secrets from storage. The fix keeps bunker signer data runtime only for the active session, removes any legacy persisted nip46 signer payloads when they are encountered, and leaves the existing persisted behavior unchanged for non bunker signer types. The main changes are in nostr-helper-functions.ts, nostr-context-provider.tsx, and the regression coverage in local-storage-data.test.ts.

// components/sign-in/SignInModal.tsx
  const startBunkerLogin = async () => {
    setIsBunkerConnecting(true);
    try {
      const signer = newSigner!("nip46", { bunker: bunkerToken });
      await signer.connect();
      saveSigner(signer); // bunker signer gets persisted after connection

  // utils/nostr/nostr-helper-functions.ts
  if (signer) {
    localStorage.setItem(LOCALSTORAGECONSTANTS.signer,
  JSON.stringify(signer)); // dangerous: persists serialized signer secrets
  }

  // utils/nostr/signers/nostr-nip46-signer.ts
  public toJSON(): { [key: string]: any } {
    return {
      type: "nip46",
      bunker: this.bunker.url,              // includes ?secret=...
      appPrivKey: bytesToHex(this.appPrivKey), // dangerous: live client private
  key
    };
  }

Before this PR, when a user connected with the nip46 bunker signer, Shopstr was serializing that signer and saving it into browser localStorage. That stored payload included the bunker connection URL and the app private key, which are the live secrets needed to continue the remote signing session. So if any script ever ran in the Shopstr origin, it could read those values from storage.

In this PR, I've changed that flow so nip46 signer data is kept only in runtime memory for the current session instead of being persisted in localStorage. I've also added cleanup so if an old persisted bunker signer is found, it gets removed, and we added tests to make sure that behaviour stays covered. Non-bunker signer types still follow the existing storage behaviour.

@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

Similar to the NWC connect, does this require re-inputting the bunker connection string 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 encrypt the stored data using user passphrase or similar.

Also a thing to consider, physical access to the bunker that is used for auth is requured to actualy do anything with the bunker string, so the string itself isn't enough to takeover an account.

@ayushshrivastv

Copy link
Copy Markdown
Contributor Author

Similar to the NWC connect, does this require re-inputting the bunker connection string 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 encrypt the stored data using user passphrase or similar.

Also a thing to consider, physical access to the bunker that is used for auth is requured to actualy do anything with the bunker string, so the string itself isn't enough to takeover an account.

I’ll first resolve the NWC connection, which will eventually provide better context for solving this issue. I’ll wait for that to be resolved, merged, and then fix this here as well.

@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