fix: nsec import + backup failures and storage.sync data loss#1
Open
vveerrgg wants to merge 3 commits into
Open
fix: nsec import + backup failures and storage.sync data loss#1vveerrgg wants to merge 3 commits into
vveerrgg wants to merge 3 commits into
Conversation
Sync merge: - Match profiles by pubKey, not array index, so a reordered or partial sync can no longer overwrite one identity's key material with another's. - Treat bunker/remote-signer profiles (privKey:'' but a real identity) as identities in fresh-install detection, so they aren't wiped as blanks. - Stop syncing isEncrypted: the verifier is never synced, so adopting a remote isEncrypted=true permanently locked the second device out. - Add startup self-heal that clears a bogus isEncrypted flag when no password verifier and no encrypted blobs exist. - Extract pure computeMergeUpdates() so the merge rules are unit-testable. Key/backup flows: - savePrivateKey now replies via sendResponse + return true (MV3 does not deliver Promise returns), so imported keys can't silently fail. - Roll back the half-created profile when a key fails to persist. - Backup export encrypts with a supplied backup password instead of the session key, so users with no master password can export, even locked. Tests: add sync-merge and backup-password regression suites (+13).
vitest 4 / vite 8 rely on require(esm), which is only default on Node 20.19+. Add an engines floor and an .nvmrc pinning the dev/CI runtime to Node 22 LTS so the test suite runs without experimental flags.
Findings from a focused security/correctness audit of the touched subsystems: - Bunker exposure (critical): gate bunkerServer.start/stop behind isExtensionSender. They were callable from any web page via window.nostr.nip46.startBunker(), which returned a secret-bearing bunker:// string granting unlimited remote signing of the user's key. - Bunker connect: fail closed when the server has no secret, instead of authenticating any client. - savePrivateKey (critical): when encryption is enabled but the session is locked (no session key), throw instead of silently persisting the key as plaintext into a vault the user believes is encrypted. - Stale session-key cache (critical): getPlaintextPrivKey now verifies the index-cached key derives to the profile's pubkey before use. Deleting a profile shifts indices without updating the index-keyed cache, so a sign could otherwise use a different identity's key. - setPermission: use index == null (not !index) so per-site grants for profile 0 target profile 0, not whatever profile is active. - content.js permission sheet: always reply (catch errors) so a failed sheet can't hang the background ask() until its 10s timeout silently denies. - Sync version compare: numeric semver compare so 1.10.0 isn't treated as older than 1.9.0 by string comparison. Tests: add compareSemver coverage (+3).
This was referenced Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a cluster of bugs around key import, backup export, and
storage.syncmerging — several of which could silently lose key material or lock a device out permanently.Bugs fixed
storage.sync data loss
pubKey.privKey:''but a real identity) was mistaken for a blank fresh-install slot and wiped. Fresh-install detection now treats it as a real identity.isEncryptedwas synced while the password verifier intentionally is not, so a second device adoptingisEncrypted=truecould never unlock. Encryption state is now strictly device-local, plus a startup self-heal clears a bogus flag when there's no verifier and no encrypted blobs.Key + backup flows
sendMessagecallers, so imported keys could fail while the UI showed success. Now replies viasendResponse+return true, with rollback of the half-created profile on failure.Tests
computeMergeUpdates()so the merge rules are unit-testable.test/sync-merge.test.js(9) andtest/backup-password.test.js(4) regression suites.Dev environment
engines.node >=20.19and an.nvmrc(vitest 4 / vite 8 needrequire(esm), default only on Node 20.19+).Notes
distros/safari/rebuilt and included (Xcode Cloud needs it tracked).main, so it'll build on merge.🤖 Generated with Claude Code