fix(desktop): stop losing imported identity on every launch#1568
Open
wpfleger96 wants to merge 1 commit into
Open
fix(desktop): stop losing imported identity on every launch#1568wpfleger96 wants to merge 1 commit into
wpfleger96 wants to merge 1 commit into
Conversation
e6d1fff to
f37e55c
Compare
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>
f37e55c to
f7fdb34
Compare
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
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.
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:
defect 1 —
import_identitynever reached the keyringcommands/identity.rs:import_identitywrote the imported nsec only toidentity.keyand updated in-memory state. it never calledstore.store(). so every boot, the keyring saw its previously generated shadow identity and "won". the fix:import_identitynow persists to the keyring first (store → read-back verify → migration marker → delete file), withidentity.keyas 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,Presentbranch: when the keyring had a key andidentity.keyexisted, it calledcleanup_leftover_identity_fileunconditionally — no pubkey comparison. so the imported key inidentity.keygot 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 keyReachableButEmptybranch: when the keyring was reachable-but-empty, no file existed, and the migration marker was present, the code fell through togenerate_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 setsidentity_lost: trueonAppStateandget_identitysurfaces 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_lostis 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,useProfileQueryis disabled whileidentity_lostis 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_identityupdatesAppState.keysand 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_keyringhad a gap: if the marker write fails after a verified keyring write and noidentity.keyexists (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, writeidentity.keyas 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, andpersist_identity_to_keyringitself.why PR #1508 didn't and couldn't fix this
#1508 added
has_profile_eventand a localStorage re-read inuseFirstRunOnboardingGateto 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 key423cc9cafrom the keyring rather than the user's imported8e39cba6. 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 theidentity_lostflag on successresolve_identity_with_storePresentbranch: 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 pathresolve_identity_with_storeReachableButEmptybranch: marker + no file → lost state instead of silent generationpersist_identity_to_keyring: marker-write failure now writesidentity.keyas fallback when absent, preserving the keyring-only-implies-marker-exists invariantsetup(): skipsrun_event_sync, agent restore, and the pending-event flush loop whenidentity_lostis true (recovery-only mode)AppState:identity_lost: AtomicBoolfield, initializedfalseIdentityInfo/get_identity: newlost: boolfielduseFirstRunOnboardingGate:identityLostoption forces gate open immediatelyuseAppOnboardingState:useProfileQuerydisabled whileidentity_lostis true to avoid signing relay auth under the ephemeral keyOnboardingFlow:identityLostprop defaults initial page to"key-import"with a notice to re-import and relaunchapp_state.rsusing the existingFakeIdentityStoreharnessproposed 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 threadingNonethrough 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.