feat(identity): add native macOS import, creation and backup - #308
Conversation
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review
Published through Wes’s GitHub account (wesbillman); this is an automated, non-blocking COMMENT, not Wes’s manual approval or merge authorization.
No actionable source defects found in this revision.
Reviewed head f3eea5ac866c617fba031cd364a439169a47fde7 against base tip fbb3000688198a27a7618b93da1fa66e8c920ed2 (merge base 0a8deed1be7327eb30668393123b55291174c4cb), including the full 33-file diff, supported callers and tests as source. The independent native-custody lane also returned no actionable findings.
src-tauri/src/identity.rs:126–160: restore errors do not become absence; import/create adopts the candidate only after create-only persistence succeeds. Existing keys and competing writers are not overwritten.- Native command registration and main-webview capability wiring match the documented trust boundary. Trusted same-origin plugin access is explicitly not a sandbox or human-gesture guarantee.
src/features/identity/PrivateKey.tsxandsrc/app/Settings.tsx: private export is interaction-driven in the app UI, with stale results fenced on hide/cancel, blur, document hiding and Profile departure. Identity backup remains separate from community-profile loading.src/app/services.tsandsrc/features/communities/service.ts: native public identity hydration does not acquire a development-broker session. Join, remote profile editing and icon discovery respect the no-packaged-relay boundary; Personal profile and backup remain available.
Validation limits: source only. Pinned extracts were checked against Git blobs and the diff whitespace check passed; no tests, builds, app launches, Keychain access or CI inspection were performed. Author-reported checks were not rerun. Attended native consent/denial/import/create/quit/relaunch checks, signed installed-app/update behavior and explicit human acceptance remain outstanding as documented. Packaged live relay behavior is not implemented or certified by this review; this result does not clear the PR’s deferred native/release gates.
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 I reviewed this at 84d66bd6 and don't see anything blocking. The custody path holds up: Store::add goes through add_generic_password, which is SecKeychainAddGenericPassword (add-only, duplicate is refused), only not-found maps to Missing, and denied/corrupt reads stay errors, so save() can't generate or overwrite from a failed read. A failed write returns before self.state is committed. Import errors are fixed strings, the snapshots only ever carry the public viewer, and relayAvailable keeps the native identity from ever pairing with the dev broker signer. The trusted-plugin export caveat is already spelled out in docs/identity.md, so I'm treating that as the documented trust model and not as a finding.
On top of reading the source, we ran a headless pass at this head without touching any real Keychain. The 8 identity tests and the main-webview permission test pass. The mock-IPC identity fixture passed 14 checks each in Chromium and WebKit, covering import hex/npub against an independently calculated key, no echo of invalid input, reveal → hide, leaving Profile, a late export after navigation not repopulating the field, and read/write failures not falling back to Create. A throwaway probe also checked that 16 generated keys were valid, distinct, and restored exactly. As a mutation check, ignoring the self.store.add(...) error in save() turns failed_persistence_never_adopts_candidate_and_retry_uses_imported_key and cached_absence_cannot_overwrite_an_identity_created_by_another_process red, so those guards really are covered.
A few optional things:
Key::parseusesCheckedHrpstring::byte_iter(), which drops the trailing padding bits. So an nsec with a valid checksum but nonzero final padding is accepted, andnsec()hands back the canonical encoding. The key itself is the same, so this isn't a custody problem, but if you want strict NIP-19 input, rejecting whenkey.nsec()doesn't match the trimmed lowercase input (plus a test) would close it.- The native adapter is compiled out under test, so nothing would catch someone switching
add_generic_passwordtoset_generic_password. That's probably fine given the attended native checks you already list, but a short comment onplatform::OsStore::addsaying it has to stay add-only would help the next person who touches it. - "Create a new identity" is one click and can't be undone in the app, since there's no delete or replace. An existing user who clicks it by mistake can't import their real key afterward without going into Keychain Access. Might be worth a confirm step, or at least a line of copy saying that.
- The description's validation section still refers to
f3eea5acand calls this a draft, but the PR isn't marked as one. Might be worth refreshing so the deferred native gates are read against the current head.
Not verified here: native Keychain consent/denial, quit/relaunch persistence against the real item, and signed-app/update behavior. Those still need the attended native run you describe.
…ad-on-send * origin/main: (58 commits) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) fix(status): reopen a Today status as Today near 16:00 (#275) test: use current navigation for GIF send roundtrip (#309) Fix composer focus when selecting channels and DMs (#307) fix: retire mention searches after chips and refuted prose (#303) ... # Conflicts: # src/features/messages/MessageComposer.test.tsx # src/features/messages/MessageComposer.tsx
* origin/main: (45 commits) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentCard.tsx # src/bundled/agents/AgentsPage.tsx
Summary
Scope and safety
No account switching, key rotation/deletion, automatic migration, plaintext fallback, relay host, or session/cache/outbox redesign. Private strings necessarily enter the trusted main UI during explicit import/export; this is not a plugin sandbox or a zeroization guarantee for JavaScript. See identity design and limitations.
Validation
Based on main
0a8deed1, headf3eea5ac:No automated browser journey files were added or removed. The manual mock-IPC fixture exercises browser presentation/lifecycle only, not real native persistence.
Draft gates / deferred checks
No native app was launched and no real Keychain item was read or written during this implementation. Use a disposable test identity for native feedback. This is a draft for testing, not merge/release certification.