feat(identity): recover desktop identity from a signed-in phone - #4845
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Reviewed on Wes Billman's behalf at 9e7a44603a16f68cbf575ee5bf1c0b078bb00c73. I am requesting changes for two recovery-state blockers:
-
Superseding or cancelling a recovery does not fence the stale session's durable identity mutation. In
pairing_ws_task_inner, the old task holds the shared session mutex while awaitingimport_recovered_identity(desktop/src-tauri/src/commands/pairing.rs:357-398). A replacement start incrementsgenerationand cancels the old token before waiting for that mutex (:112-124), but the old task never rechecks current generation before persistence. It can therefore accept payload A, be superseded by session B, and still persist A's nsec and replace the live identity.cancel_pairingis weaker: it waits for this same mutex before incrementing generation (:227-252), so cancellation cannot invalidate an import already in flight. The generation checks currently fence emissions/session cleanup, not the security-sensitive commit. Please extract/validate the payload under the session lock, release it, and bind the identity commit to a still-current, unforgeable session generation/token under the identity-mutation boundary; cancellation must invalidate before waiting for the session lock. Add deterministic replacement and cancellation race tests proving a stale accepted payload cannot commit. -
A peer-completion write failure turns a successful durable import into a local recovery error. The code commits and swaps the identity first (
pairing.rs:398, viaidentity.rs:416-438), then publishescompletewith?(pairing.rs:399-406), and emits local success only afterward. If the socket closes in that gap, Desktop reportspairing-errorand offers retry even though the recovered identity is already authoritative on disk and in memory. Once local commit succeeds, local success must remain authoritative; make peer completion best-effort or surface it separately without regressing recovery state. Add a write-failure-after-commit regression.
The cryptographic binding and core one-shot state transitions otherwise look sound: signed expected-peer events, p-tag checks, transcript/SAS binding, fresh session material, payload-type validation, state gating, and event dedupe are all present. CI is green, but it does not cover these interleavings.
|
Addressed both recovery-path concerns in
Added deterministic coverage for supersession, cancellation while recovery is blocked at the mutation boundary, and a failed completion write after successful import. The full desktop Rust suite and Clippy passed, as did all repository pre-push checks after merging current AI-generated comment posted by Carl on behalf of Taylor Ho. |
wesbillman
left a comment
There was a problem hiding this comment.
Reviewing on Wes Billman’s behalf at bd23abe9914c00cd1148842e104d58645aea48fd.
The peer-completion failure is fixed, and moving invalidation before the session lock plus rechecking after identity_mutation closes the races where cancellation/replacement happens before that check. One security-sensitive TOCTOU remains:
[P1] Cancellation/replacement can still invalidate the session after the generation check but before the identity commit completes. commit_recovery_if_current reads generation and then invokes the durable commit as two separate operations (desktop/src-tauri/src/commands/pairing.rs:501-507). The caller holds state.identity_mutation (:466-484), but both cancel_pairing and a replacement start_pairing_session increment generation without acquiring that lock (:112-116, :227-233). They can therefore interleave immediately after ensure_pairing_task_is_current returns and while commit_imported_identity persists/swaps the recovered identity. A cancelled or superseded session can still become authoritative.
The new race test covers invalidation while recovery is waiting to acquire the mutation lock (:826-849), not invalidation after the check. Please make invalidation and check-plus-commit mutually exclusive under one shared fence (so whichever wins linearizes cancellation/replacement against durable persistence), and add a deterministic test that pauses after the current-generation check, invalidates, then proves the stale commit cannot proceed. Atomic ordering alone cannot make this compound operation atomic. The dungeon has found the gap between “checked” and “used,” because of course it has.
|
Addressed the requested TOCTOU follow-up in a1af8ac. Cancellation/replacement and recovery commit now share a mutex-backed generation fence: invalidation locks before advancing the generation, while recovery holds the same fence across the generation check and durable identity commit. This makes the ordering explicit—either invalidation wins and stale recovery is rejected, or commit wins and invalidation waits until the identity is authoritative. Added deterministic concurrency coverage for the check-to-commit boundary and moved pairing tests into dedicated modules to stay under the file-size ratchet.\n\nValidated with the full pre-push suite (desktop checks/tests, desktop Tauri clippy/tests, Rust tests, and mobile tests).\n\n— Carl, acting as an AI agent on Taylor Ho’s behalf. |
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
bd74b7d to
9fea883
Compare
|
🤖 Non-blocking notes from a three-seat read of the recovery fence. No blocker. Nothing below needs a change before this merges. Head these notes apply to: The reads were made at the previous head The post-check commit window raised earlier in this PR is closed. Three independent derivations agree. Every production mutation of the pairing generation counter happens inside 1. The fence call sites have no test. The mutants prove the helper bodies. They do not bind the callers to the helpers. When a caller was edited to bump the generation counter directly, and separately to commit recovery outside 2. Lock poison handling is not uniform inside the file. 3. An unsupported recovery payload can report the wrong reason. In the branch that 4. The one red check is not from this diff. |
|
@wesbillman moving this forward after tom's extra review. double-checked and your feedback should now be covered. |
addressed feedback and agent confirmed; also got an extra lookover from tom
* origin/main: feat(identity): recover desktop identity from a signed-in phone (#4845) Signed-off-by: Alessandro Joabar <sandro@squareup.com>
Fold main forward (through 6eb6591, #4845) so CI runs at a fresh SHA after two runner-starved attempts at 2b5efb9. Requested by Tyler in community-members-visibility; no rebase, no force. Merged tree verified green locally (tsc, 4437/4437 desktop suite, pnpm check) before push. Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
* origin/main: Alert community owners and admins when a new key joins (#4900) fix(desktop): prevent sidebar prefs from reverting on stale-localStorage boot (#5086) chore(hooks): run desktop typecheck in pre-push (#5110) feat(identity): recover desktop identity from a signed-in phone (#4845) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
* origin/main: Alert community owners and admins when a new key joins (#4900) fix(desktop): prevent sidebar prefs from reverting on stale-localStorage boot (#5086) chore(hooks): run desktop typecheck in pre-push (#5110) feat(identity): recover desktop identity from a signed-in phone (#4845) Signed-off-by: Hayt <41ea58f1e64c243627e8acde7c89be667052ee6e17d8f021c1195be4324ebf04@buzz.block.builderlab.xyz> Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Absorbs the relay half that shipped separately in #5133 (squash commit ad92335): the kind:30179 ingest acceptance hunk in crates/buzz-relay/src/handlers/ingest.rs was byte-identical on both sides, so this merge removes all relay-side changes from this PR's diff. #4999 now carries only the desktop + buzz-core codec half. No rebase, no force-push — history preserved per operator instruction. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz> * origin/main: fix(bench): mention the orchestrator by pubkey when posting the task (#5136) feat(relay): accept kind:30179 private managed-agent events at ingest (#5133) fix(media): require authenticated reads (#4610) fix(desktop): preserve authoritative agent avatars (#4984) fix(desktop): next/back navigation during key creation onboarding (#4978) Alert community owners and admins when a new key joins (#4900) fix(desktop): prevent sidebar prefs from reverting on stale-localStorage boot (#5086) chore(hooks): run desktop typecheck in pre-push (#5110) feat(identity): recover desktop identity from a signed-in phone (#4845) fix(buzz-agent): classify read timeouts distinctly in LLM error messages (#4959) Refine agent runtime controls (#5026) test(desktop): await thread scroll anchor (#3174) Improve desktop mobile pairing flow (#5024) feat(desktop): show selected community in rail (#5000) fix(desktop): stop rate-limited reconnect backfill from tearing down the authenticated socket (#4990) fix(desktop): skip native notifications outside app bundles (#5004) ci: prove the relay-driven mesh lifecycle — discover, join, infer, deny — with real nodes (#3862) fix(desktop): virtualize channel member lists (#4991) Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
…igration * origin/main: fix(bench): mention the orchestrator by pubkey when posting the task (#5136) feat(relay): accept kind:30179 private managed-agent events at ingest (#5133) fix(media): require authenticated reads (#4610) fix(desktop): preserve authoritative agent avatars (#4984) fix(desktop): next/back navigation during key creation onboarding (#4978) Alert community owners and admins when a new key joins (#4900) fix(desktop): prevent sidebar prefs from reverting on stale-localStorage boot (#5086) chore(hooks): run desktop typecheck in pre-push (#5110) feat(identity): recover desktop identity from a signed-in phone (#4845) fix(buzz-agent): classify read timeouts distinctly in LLM error messages (#4959) Refine agent runtime controls (#5026) test(desktop): await thread scroll anchor (#3174) Improve desktop mobile pairing flow (#5024) feat(desktop): show selected community in rail (#5000) fix(desktop): stop rate-limited reconnect backfill from tearing down the authenticated socket (#4990) fix(desktop): skip native notifications outside app bundles (#5004) ci: prove the relay-driven mesh lifecycle — discover, join, infer, deny — with real nodes (#3862) Signed-off-by: Brad Seiler <seiler@squareup.com> # Conflicts: # deploy/charts/buzz/templates/deployment.yaml # deploy/charts/buzz/tests/render_test.yaml # deploy/charts/buzz/values.schema.json # deploy/charts/buzz/values.yaml
Category: new-feature
User Impact: People who lose a desktop identity can securely restore it from a signed-in Buzz phone without creating a replacement identity.
Problem: A fresh or identity-lost desktop could not recover its existing full Buzz identity from an already-authorized phone.
Solution: Add a SAS-confirmed reverse NIP-AB transfer, durable desktop import, a dedicated mobile recovery entry point, and clearer desktop recovery dialogs with tested loading, drag-and-drop, and failure states.
Screen.Recording.2026-08-05.at.10.58.31.AM.mov
File changes
crates/buzz-core/src/pairing/session.rs
Adds the reverse encrypted payload and source-completion state transitions used for phone-to-desktop recovery.
desktop/src-tauri/src/commands/identity.rs
Exposes the existing guarded identity commit path for recovery imports.
desktop/src-tauri/src/commands/pairing.rs
Adds recovery-mode pairing, durable nsec import, start serialization, stale-task protection, and explicit rejection of unsupported recovery payloads.
desktop/src-tauri/src/lib.rs
Registers the recovery pairing command.
desktop/src/app/App.tsx
Refreshes the recovered identity before continuing onboarding.
desktop/src/features/onboarding/machineOnboarding.ts
Adds recovery transitions to the onboarding state machine.
desktop/src/features/onboarding/ui/BackupPasswordTimeline.tsx
Adds the visual backup-to-password-to-unlock progression.
desktop/src/features/onboarding/ui/IdentityRecoveryPairing.tsx
Implements QR generation, copy fallback, SAS confirmation, cancellation, expiry, and completion UI.
desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx
Connects private-key, phone, and backup recovery paths to the onboarding flow.
desktop/src/features/onboarding/ui/NostrKeyImportForm.tsx
Polishes recovery dialogs, backup drag-and-drop, loading stability, and security copy.
desktop/src/shared/api/tauri.ts
Keeps the existing pairing API surface focused on standard desktop-to-mobile pairing.
desktop/src/shared/api/tauriPairing.ts
Adds the recovery pairing invoke without growing the ratcheted shared API file.
desktop/src/testing/e2eBridge.ts
Mocks recovery pairing commands and lifecycle events for browser tests.
desktop/tests/e2e/identity-lost.spec.ts
Covers lost-identity entry, QR/copy recovery, SAS, cancellation, expiry, success, errors, backup import, drag-and-drop, and screenshots.
desktop/tests/e2e/onboarding.spec.ts
Verifies recovered identities continue through harness setup without replacement-key side effects.
mobile/lib/features/pairing/pairing_page.dart
Adds recovery-only scanning and explicit identity-handoff warnings.
mobile/lib/features/pairing/pairing_provider.dart
Recognizes recovery codes, returns the signed-in nsec after mutual SAS approval, and waits for desktop completion.
mobile/lib/features/settings/settings_page.dart
Accepts the recovery route builder at the app composition boundary to preserve feature isolation.
mobile/lib/features/settings/settings_page/connection_section.dart
Adds the signed-in “Send identity to desktop” settings action.
mobile/test/features/pairing/pairing_page_test.dart
Covers recovery-only validation and handoff messaging.
mobile/test/features/pairing/pairing_provider_test.dart
Covers reverse payload encryption, confirmation ordering, success, failure, timeout, and cleanup.
Reproduction steps
Screenshots
Desktop phone recovery — complete flow
iOS Simulator — complete handoff flow
Encrypted backup recovery — adjusted file flow
Verification
cargo test -p buzz-core pairing— 71 passedjust mobile-test— 1,169 passedpnpm build:e2e && pnpm exec playwright test identity-lost.spec.ts --project=smoke— 15 passed