Skip to content

fix(desktop): stop losing imported identity on every launch#1568

Open
wpfleger96 wants to merge 1 commit into
mainfrom
wpfleger96/identity-import-keyring
Open

fix(desktop): stop losing imported identity on every launch#1568
wpfleger96 wants to merge 1 commit into
mainfrom
wpfleger96/identity-import-keyring

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

i dug into why Buzz keeps forcing re-onboarding every launch and why my imported identity silently vanishes each time. the root cause is three separate defects that chain together into the loop captured in journald:

22:26:50 buzz-desktop: imported identity pubkey 8e39cba681211b3782d0e4483e9343719b9b7be66515252da5491f26421896b1
22:27:06 buzz-desktop: persisted identity pubkey 423cc9ca3a8f1d37efbefb5cc6bf9a72206...
22:27:06 buzz-desktop: removed leftover identity.key (key is in keyring)
22:30:27 buzz-desktop: imported identity pubkey 8e39cba6...

defect 1 — import_identity never reached the keyring

commands/identity.rs:import_identity wrote the imported nsec only to identity.key and updated in-memory state. it never called store.store(). so every boot, the keyring saw its previously generated shadow identity and "won". the fix: import_identity now persists to the keyring first (store → read-back verify → migration marker → delete file), with identity.key as a fallback only when the keyring write fails.

defect 2 — boot cleanup deleted the imported key without comparing pubkeys

app_state.rs:resolve_identity_with_store, Present branch: when the keyring had a key and identity.key existed, it called cleanup_leftover_identity_file unconditionally — no pubkey comparison. so the imported key in identity.key got deleted every boot, and the shadow key resolved instead. the fix: compare pubkeys first. if they match, it's a stale leftover from a failed prior migration — clean it up, but write the migration marker first if it's missing (crash-safe ordering). if they're different, the file is the user's most recent explicit action (import): adopt it into the keyring (overwrite, read-back verify), write the migration marker before deleting the file, and resolve as the file's key. if the marker write fails on adoption, keep the file so a later keyring-unreachable boot has a fallback. this also auto-heals installs already stuck in the loop.

defect 3 — ReachableButEmpty + marker + no file silently regenerated a new key

ReachableButEmpty branch: when the keyring was reachable-but-empty, no file existed, and the migration marker was present, the code fell through to generate_and_persist(). that generated a fresh identity without the user ever asking for one. the fix: detect marker-present + no-file as an "identity lost" state. an ephemeral in-memory key is generated so the app boots, but the backend sets identity_lost: true on AppState and get_identity surfaces it to the frontend. the frontend opens onboarding directly at the nsec import step with a notice to re-import and relaunch.

to prevent the lost state from doing silent damage: when identity_lost is true, setup() skips all owner-keyed startup work — run_event_sync, launch-time agent restore, and the pending-event flush loop are all bypassed. these routines would otherwise sign or publish events under the ephemeral key. similarly, useProfileQuery is disabled while identity_lost is true so the frontend doesn't sign a NIP-98 auth header against the relay using the ephemeral key before the user re-imports. the recovery path is a relaunch: import_identity updates AppState.keys and clears the flag, but re-triggering async startup routines mid-session is fragile — a clean boot after re-import is the right path.

marker invariant: keyring-only implies marker exists

persist_identity_to_keyring had a gap: if the marker write fails after a verified keyring write and no identity.key exists (e.g. an import from lost state where the file was already deleted), the key was left keyring-only with no marker. a later keyring-unreachable boot would see no file + no marker and treat it as a fresh install, silently rotating identity. the fix: when the marker write fails, write identity.key as a fallback if it's absent, so the invariant holds regardless. the same crash-safe ordering (marker before file delete) now applies on every path that transitions from file-backed to keyring-backed: the mismatched-file adoption path, the same-pubkey leftover cleanup path, and persist_identity_to_keyring itself.

why PR #1508 didn't and couldn't fix this

#1508 added has_profile_event and a localStorage re-read in useFirstRunOnboardingGate to survive a webkit2gtk WAL race where the onboarding-complete flag wasn't visible yet on first read. that hardening is legitimate and i've kept it in place. but the re-check still reads localStorage for the wrong pubkey — the shadow key 423cc9ca from the keyring rather than the user's imported 8e39cba6. there's no localStorage entry for the shadow key, so it can't help here regardless.

what this PR changes

  • import_identity: keyring-first persistence; file fallback on availability failure; clears the identity_lost flag on success
  • resolve_identity_with_store Present branch: pubkey-compare before cleanup; mismatched file key adopted into keyring with marker written before file delete; same-pubkey cleanup writes marker first if missing; file kept if marker write fails on either path
  • resolve_identity_with_store ReachableButEmpty branch: marker + no file → lost state instead of silent generation
  • persist_identity_to_keyring: marker-write failure now writes identity.key as fallback when absent, preserving the keyring-only-implies-marker-exists invariant
  • setup(): skips run_event_sync, agent restore, and the pending-event flush loop when identity_lost is true (recovery-only mode)
  • AppState: identity_lost: AtomicBool field, initialized false
  • IdentityInfo / get_identity: new lost: bool field
  • frontend useFirstRunOnboardingGate: identityLost option forces gate open immediately
  • frontend useAppOnboardingState: useProfileQuery disabled while identity_lost is true to avoid signing relay auth under the ephemeral key
  • OnboardingFlow: identityLost prop defaults initial page to "key-import" with a notice to re-import and relaunch
  • 9 new behavior tests in app_state.rs using the existing FakeIdentityStore harness

proposed follow-up (not done here)

the deeper fix is to stop auto-generating a shadow identity before the user expresses any onboarding intent. that requires AppState.keys: Option<Keys> and threading None through all callers that currently assume a key always exists. it's a larger refactor and doesn't belong in this PR — i'd like to track it as a separate issue.

@wpfleger96 wpfleger96 force-pushed the wpfleger96/identity-import-keyring branch 5 times, most recently from e6d1fff to f37e55c Compare July 7, 2026 05:03
import_identity wrote only to identity.key and never to the keyring, so
the shadow key from first boot always won on every subsequent launch.
The Present-branch boot cleanup then deleted identity.key without
comparing pubkeys, discarding the imported key silently. On a clean
keyring with a migration marker but no file, a new identity was
generated without any user intent, completing the re-onboarding loop.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 force-pushed the wpfleger96/identity-import-keyring branch from f37e55c to f7fdb34 Compare July 7, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant