From fe0ca048173275d839985a4c733bb448de2c73c6 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Thu, 12 Nov 2026 15:41:15 +0700 Subject: [PATCH 1/6] add plan --- docs/audit-stable-release.md | 825 +++++++++++++++++++++++++++++++++++ 1 file changed, 825 insertions(+) create mode 100644 docs/audit-stable-release.md diff --git a/docs/audit-stable-release.md b/docs/audit-stable-release.md new file mode 100644 index 00000000..826ca3db --- /dev/null +++ b/docs/audit-stable-release.md @@ -0,0 +1,825 @@ +# Coop Stability Audit — Pre-Release + +Date: 2026-11-12 · Revision audited: `8fa79aef` (master) · Scope: entire workspace (~35k lines, 18 crates + `desktop` + `web`) + +## 1. Scope and Method + +Every crate was audited file-by-file against the project conventions (`AGENTS.md`), Rust best +practices (ownership, error handling, async, API design, anti-patterns), and GPUI lifecycle rules +(entity updates, task/subscription ownership, `ElementId` stability, render purity). Severe claims +were verified against the vendored dependency sources (`gpui-pre 0.3.12`, `gpui-base 0.9.1`, +`gpui-component 0.9.1`, rust-nostr 0.45.6, `gpui-updater-pre 0.2.0`), and the highest-severity +findings were independently re-verified by hand. + +Tooling results: `cargo clippy --workspace --all-targets` — **clean, zero warnings**. +`cargo fmt --all --check` — **clean**. `cargo check -p workspace` and `-p person` — pass. + +## 2. Executive Summary + +**Verdict: not ready for stable release yet.** The codebase is well-structured — clean clippy, +disciplined GPUI entity/subscription hygiene in most crates, good hostile-input handling in the +crypto-adjacent code, and no lock-across-await anywhere. But there are **3 Critical** and +**~27 High** findings that must be addressed first, concentrated in three areas: + +1. **Money and key safety** (`nip60`, `state`, `browser-signer-proxy`): a one-event wallet-wipe + vector, a startup crash loop, a plaintext client key in web `localStorage`, and an + unauthenticated localhost signing API. +2. **Silent failure paths**: errors from wallet ops, message loads, relay connects, settings + persistence, and backup/keyring reads are swallowed; several failure states strand data or + wedge the UI (stuck `loading_more`, permanently-blacklisted profile fetches, dictation dying + after 30 s). +3. **Wrong results visible to users**: reaction double-counting, multi-line messages flattened to + one line, contact-list updates that erase relay hints and petnames, stale group state from + out-of-order relay delivery, duplicate tabs. + +### Must fix before stable (P0) + +| # | Severity | Location | Issue | +|---|----------|----------|-------| +| 1 | Critical | `nip60/src/lib.rs:420,552` | Spoofed kind:5 deletion from **any** author wipes wallet proofs (operator precedence bug + missing author check in `absorb_deletion`) | +| 2 | Critical | `state/src/lib.rs:117-122` | LMDB open `block_on` + `.expect` → unrecoverable startup crash loop on any DB error | +| 3 | Critical | `state/src/lib.rs:447-449` | Plaintext NIP-46 client secret key persisted to `localStorage` on web | +| 4 | High | `browser-signer-proxy/src/lib.rs:443-571` | Unauthenticated localhost signing API with `Access-Control-Allow-Origin: *`; plaintext theft, request suppression, response forgery | +| 5 | High | `nip60/src/ops.rs:455-488` | Reconcile marks relay proofs `Unspent` unconditionally → resurrects locally-spent proofs (double-spend risk) | +| 6 | High | `nip60/src/ops.rs:381-417` | Publish failure after store commit strands sent/received proofs; no retry path | +| 7 | High | `state/src/file.rs:418-443` | Decrypted-file cache keyed by filename only → user can be served another attachment's contents | +| 8 | High | `chat_ui/src/lib.rs:605-611` | Own reactions counted twice (backup gift-wrap re-delivery, no dedup) | +| 9 | High | `common/src/media_extractor.rs:52-58` | Whitespace collapse destroys line breaks in every message | +| 10 | High | `workspace/src/panels/contact_list.rs:187-191` | Contact-list Update silently strips NIP-02 relay hints and petnames from the published event | +| 11 | High | `nip29/src/lib.rs:541-555` + `group.rs:696-728` | No `created_at` guards: stale relay data resurrects left groups; group state applied last-write-wins | +| 12 | High | `settings/src/lib.rs:403-404,423-438` | Failed settings load silently resets and overwrites the user's file; saves discard errors and race | +| 13 | High | `speech/src/parakeet.rs:134` | Dictation worker exits after 30 s idle → longer dictations silently lost | +| 14 | High | `browser-signer-proxy/src/lib.rs:339-366` | Timed-out signing requests leak plaintext in memory forever | +| 15 | High | `dock/mod.rs:50-73` + `sidebar/rows.rs:54-57` | Tab dedup is dead code (duplicate tabs); "Ignore" screening action dispatches to a handler that can never receive it | +| 16 | High | `nip60_ui/src/wallet.rs:615-726` + `nip60/src/lib.rs:147-185` | Lightning invoice flow broken end-to-end (quotes never rendered or claimable) | +| 17 | High | `state/src/lib.rs:553-554` | Proxy credential write silently discarded (`save.await.ok()`) | +| 18 | High | `person/src/lib.rs:310-333` | One 2 s metadata timeout permanently blacklists those profiles until restart | +| 19 | High | `chat/src/lib.rs:398-403` | Failed-to-decrypt messages are trashed with no UI surface anywhere | +| 20 | High | `chat_ui/src/lib.rs:325-334`, `workspace/src/dialogs/profile.rs:227-235`, `workspace/src/panels/backup.rs:59-76` | Silent failure cluster: room loads, profile URL saves, keyring reads | + +The full finding list follows, crate by crate. + +## 3. Systemic Patterns (cross-cutting) + +**S1 — Task handles accumulate unboundedly (~20 containers, ~48 declaration sites).** +`tasks: Vec>` / `SmallVec<[Task<_>; N]>` fields are pushed on every operation and never +drained; finished `Task` handles held in a container are not reaped by GPUI. Worst offenders: +`nip60/src/lib.rs:79` (a handle per relay event, whole session), `nip4e/src/lib.rs:90` (runs on +every settings change), `chat_ui/src/lib.rs:110`, `nip29/src/lib.rs:90`, `nip29_ui/src/lib.rs:337`, +`person/src/directory.rs:110`, `state/src/lib.rs:95`, plus every workspace dialog. The codebase +already contains the correct patterns: single `Option>` replacement +(`screening.rs`, `greeter.rs`, `debounced_delay.rs`) and reset-on-teardown (`nip29::reset`). +Fix convention-wide: replace-per-spawn for one-shots, `detach_and_log_err` for true fire-and-forget. + +**S2 — Silent error discards (~156 `let _ =` / `.ok()` sites outside tests).** +Project rule forbids `let _ =` on fallible ops; `AGENTS.md` requires async errors to reach the UI. +Beyond the P0s above, the recurring case is `_ = this.update(cx, …)` — mostly benign +(entity dropped at shutdown) but should be `.log_err()` for visibility. Files with the most sites: +`dock/tab_panel.rs` (6), `nip60/src/lib.rs` (10), `nip4e/src/lib.rs` (8). + +**S3 — `chat_ui` ↔ `nip29_ui` message-panel duplication.** +`ListState` sync + splice bookkeeping (including divergent `ix + 1` vs `ix + 2` offsets), +`rendered_texts_by_id` cache (both share the stale-mention bug), speech reveal + `RevealText` +fallback timer + `end_events` loop, `Speaker` wiring, reaction grouping, `MessageRow` assembly are +near-verbatim copies — and they have already diverged: `nip29_ui` splices at the prepend position +while `chat_ui` always appends (bug 5.7.4). Extract a shared message-panel controller in `ui`. + +**S4 — Dialog error-banner + auto-clear timer duplicated four times** with drifted timings +(`restore.rs` 3 s, `import.rs` 3 s, `relays.rs` 2 s, `contact_list.rs` 2 s). One shared +`error: Entity>` helper removes the drift. + +**S5 — `DefaultHasher` used for network-persisted IDs.** +`common::EventExt::uniq_id` (`event.rs:12-18`) is hashed into published `r` tags by +`chat/src/lib.rs:1031-1041`; `DefaultHasher`'s algorithm is not stable across Rust versions, so a +toolchain bump silently changes every published room id. Also hand-rolled in three places +(`nip29/protocol.rs:282-286`, `nip29/utils.rs:27-32`). Replace with a stable hash (SHA-256 of +sorted hex pubkeys) in one shared helper. + +**S6 — Index-based element ids.** Rows keyed by list index (`sidebar/rows.rs:174+`, +`contact_list.rs:238`, `greeter.rs:226+`, `nip29_ui` reply previews `message.rs:359`) reassign to +different data after mutations; benign today because handlers capture data, but any future +stateful element (hover/focus) in a row will mis-attach. Prefer domain ids (pubkey hex, event id, +room id) as `group_view.rs` already does. + +## 4. Findings by Crate + +Severity: **[C]** Critical · **[H]** High · **[M]** Medium · **[L]** Low + +### 4.1 `state` — central Nostr registry, signers, persistence, files + +- **[C] `lib.rs:117-122` — Startup LMDB open: `block_on` + `.expect` = crash loop.** The + `block_on` itself is acceptable (runs once before the window opens; `NostrLmdb::open` schedules + nothing back onto the GPUI executor — add a comment saying so). The real bug: a corrupted or + unwritable data dir makes **every** launch panic with no dialog and no recovery except manually + deleting data. The web path already does it right (`web_database.rs:30-33` falls back to memory + with a logged error). Fix: mirror that on desktop — memory DB fallback + `StateEvent::Error` so + the UI can tell the user local history is unavailable. +- **[C] `lib.rs:447-449` (+ `credentials.rs:29-38`, `common/web_store.rs:21-25`) — Plaintext + bunker client key in web `localStorage`.** `import_bunker` embeds a fresh `Keys::generate()` into + the bunker URI and stores the credential unencrypted on wasm. XSS/malicious extension/shared + machine = signature capability through the bunker (bunkers commonly auto-approve registered + clients). Desktop stores it in the OS keyring, which is fine. Fix: on wasm, passphrase-encrypt + (NIP-49) like `import_nsec` does, or keep the session memory-only. +- **[H] `lib.rs:553-554` — Proxy credential write silently discarded.** + `cx.background_spawn(async move { save.await.ok() }).detach();` — keyring failure = user silently + dropped to login next launch. Log + emit `StateEvent::error`. +- **[H] `file.rs:418-443` — Decrypted-file cache collides by filename.** Cache key is + `sha256(name)` and the `metadata().is_err()` guard skips the write, so two attachments named + `invoice.pdf` from different chats resolve to the **first** file's path. Fix: include the + attachment sha256/URL in the key, or always overwrite via the atomic-rename path. +- **[M] `lib.rs:373-386` — Web `ncryptsec` stored with `KeySecurity::Medium` (2¹⁶ PBKDF2).** + Encrypted, but offline guessing at 2¹⁶ defeats human passphrases. Raise to `Strong` and/or move + to IndexedDB + session-only keys. Acceptable if documented as a tradeoff. +- **[M] `lib.rs:564-589` — Stale proxy monitor can log out a newer session.** The monitor's guard + is only `current_user.is_some()`; if the user disconnects the proxy and signs in with nsec/bunker + before the next 5 s tick, the monitor swaps the **active** signer for `Keys::generate()`. Fix: + capture the proxy pubkey at spawn and only act if it's still the active signer. +- **[M] `lib.rs:95` — `tasks` vec grows forever** (see S1; `connect_proxy` alone pushes four). +- **[L] `lib.rs:714` `to_bech32().expect("infallible")`; `signer.rs` ×7 + `.expect("RwLock poisoned")`.** Practically unreachable, but a poisoned signer lock is the worst + failure mode for a signer — prefer `unwrap_or_else(|e| e.into_inner())`. +- **[L] `lib.rs:93` — `passphrase_required` written, never read** (UI learns via + `StateEvent::PassphraseRequired`). Delete or expose. +- **[L] `file.rs:381-384` — `DownloadedFile::save` (wasm) has no callers.** Remove or wire up. +- **[L] `lib.rs:647-656` — Dead search-relay guard.** Vendored nostr-sdk: `Client::relay()` returns + `Result>`, so `is_ok()` is always true and the "Failed to add search relay" branch is + unreachable. Test `is_ok_and(|r| r.is_some())` or drop the check. +- **[L] `file.rs:221-258` — No size limit on pick/upload** (`MAX_FILE_SIZE` enforced only on + download); a multi-GB pick is read fully into memory. Check size before reading. +- **[L] `file.rs:427-441` — Plaintext decrypted files in `temp_dir()/coop-files`, default + permissions, never purged.** Consider 0600 + best-effort purge on exit. Also `file.rs:150` + builds `file://{path}` by string concat (breaks on `#`/`?`/`%`). +- **[L] `file.rs:291` — Blossom rejection classified by `message.contains("415")`** — match the + HTTP status instead. `file.rs:172` — 16-byte GCM nonce; 12 bytes is the norm. +- **[L] `lib.rs:280-289` — `restore_session` cfg branches compile to empty bodies on wrong + target**, silently emitting no event. `debug_assert!` or fall through to `NoSigner`. +- **[L] `file.rs:446-451` + `web_file.rs:164-174` — Web blob URLs never revoked** when replaced; + every decrypted file pins its bytes for the page lifetime. +- **[L] `lib.rs:683-687` — Search results can contain duplicate pubkeys.** Dedupe. + +Positives: device key stored only NIP-44-encrypted to self in the local DB; bunker credentials +verified before persisting; `UniversalSigner` clones the inner `Arc` out of the lock before +awaiting (no locks across `.await`); downloads size-capped and hash-verified; `Keys`' vendored +`Debug` hides secrets. + +### 4.2 `nip60` — NIP-60 Cashu wallet engine + +- **[C] `lib.rs:420` + `lib.rs:552-572` — Spoofed kind:5 deletion can wipe the wallet.** + `let wallet_event = ours && is_cashu || is_token_deletion(&event);` parses as + `(ours && is_cashu) || is_token_deletion(&event)` — deletion events from **any** author are + forwarded, and `absorb_deletion` never checks `event.pubkey == me`. Kind 7375 event ids are + public on relays; one attacker event removes our tokens, marks them stale, and the Reconcile + deletes the real proofs from the store (restore-only recovery). Fix: + `wallet_event = ours && (is_cashu || is_token_deletion(&event))` + author check inside + `absorb_deletion` for defense in depth. +- **[H] `ops.rs:455-488` (with `proof_info` `ops.rs:601-617`) — Reconcile resurrects spent + proofs.** Every relay-token proof is added to the store hardcoded `State::Unspent`, ignoring the + proof's existing store state. After any failed-publish op, reconcile re-adds proofs this device + itself marked spent/reserved (incl. offline sends the recipient also holds) → bogus balance, + real double-spend race. Fix: skip `y`s already in the store with non-Unspent state; verify + unknown proofs against the mint (NUT-08) before adding. +- **[H] `ops.rs:381-417` + `lib.rs:836-843` — Publish failure after store commit strands funds.** + If `send_event` fails, the confirmed `TokenUpdate` is dropped: for `SendToken` the token string + is lost (proofs spent locally, recipient never gets it); for `Receive` proofs sit unpublished and + a retry errors as duplicate. Reconcile only republishes when `mint_unissued_quotes() != 0` + (`ops.rs:429-432`), so it doesn't heal this. Fix: retain the confirmed update on error; force + republish whenever the store's unspent set differs from relay state. +- **[H] `ops.rs:420-453` — Quote-minting publish can `del` events whose proofs aren't converged + yet.** `previous_ids` is the schedule-time relay snapshot; if another device's token was absorbed + but not yet converged, the published event `del`s the id while omitting its proofs — those proofs + get pruned everywhere while remaining spendable at the mint. Fix: converge before publish, or + only `del` when the published proof set fully covers the id. +- **[H] `lib.rs:616,651` + `lib.rs:902-912` — Failed nutzap redemption stuck until restart.** + `redeeming.insert` happens before scheduling; the `Err` branch never removes the id and + `absorb_nutzap` skips anything in `redeeming`, and nothing requeues to `pending_nutzaps`. Fix: + remove + requeue with bounded retries on error. +- **[H] `lib.rs:79` — `tasks` grows by a handle per relay event** (see S1). +- **[H] `lib.rs:147-149,172-185` — `mint_quotes()`/`claim_mint_quote()` dead → created invoices + are never displayable or claimable.** `Outcome::QuoteCreated` pushes into `self.quotes` and + nothing ever reads it (workspace-wide grep). Quotes also accumulate forever. Fix with the invoice finding in 4.3 below. +- **[M] `lib.rs:188-215` — `melt()` and `send_token()` unreachable; `WalletEvent::TokenCreated` + emitted only from the dead path** while `workspace/src/lib.rs:226-236` promises "copy it from + the wallet panel". Wire into UI or remove; fix the notification text regardless. +- **[M] `lib.rs:285-370` — Concurrent `setup()` can fork the wallet key** (only guarded by + `loading`); absorption tie-breaks with `created_at >=` on second-granularity timestamps, and + stores are keyed by wallet pubkey — the losing key orphans its funds. Fix: `setup_in_progress` + guard; deterministic tie-break (event id). +- **[M] `lib.rs:430-432` — `processed_events.clear()` at 1000 entries** re-delivers a burst; + evict oldest instead. +- **[M] `lib.rs:615-661` — Nutzap `unit` tag never validated**; a `usd` nutzap passes local + validation. Require `unit == "sat"` in `Nutzap::from_event`. +- **[L] `lib.rs:301-303,699-701` — Silent no-ops**: `setup()` when mint already listed; + `ensure_wallets` on unparseable mint URLs forever. Surface feedback. +- **[L] `ops.rs:107` — ClaimQuote history records `quote.amount.unwrap_or_default()`** (0-sat + inflow when mint omits amount); use the summed proof amount. +- **[L] `Cargo.toml:23` — `tokio` dep unused** (only `gpui_tokio::Tokio` is referenced). Remove. + +Positives: store-before-publish enforced via cdk prepare/confirm sagas; all relay-derived content +author-checked before decryption; `keys.rs` secret handling clean (no Debug on `WalletData`, no +secrets in logs, HKDF salt pinned by test); single-flight job queue; `WalletEvent::Error` reaches +notifications even with the panel closed. + +### 4.3 `nip60_ui` — wallet panel + dialogs + +- **[H] `wallet.rs:615-726` — Invoice button creates an invoice the user can never see or pay.** + After `create_mint_quote` the bolt11 string lands in `WalletRegistry::quotes`, which nothing + renders. Receive-via-lightning is broken end-to-end, silently. Fix: render pending quotes + (request + copy + check/claim) in the dialog/panel, refreshed on `WalletEvent::Updated`. +- **[M] `wallet.rs:765` — `render()` clones the whole history `Vec` every frame** (history is + unbounded). Snapshot into `Rc` only on `WalletEvent::Updated`. Same for `filtered_contacts` + (`wallet.rs:542`) — cache on input change. +- **[L] `wallet.rs:206,284,356` — `SendDialog::tasks` accumulates** (see S1). +- **[L] `wallet.rs:262-264` — `load_contacts` gives up silently pre-login**; retry on + `StateEvent::SignerChanged` (sidebar already does this). +- **[L] `wallet.rs:728-733` vs `lib.rs:28-34` — amount parsing duplicated.** Share `parse_amount`. + +Positives: `cx.notify()` on all mutating paths; per-key hex ElementIds; stored subscriptions; +history list windowed via `uniform_list`. + +### 4.4 `browser-signer-proxy` — NIP-46 localhost bridge + +- **[H] `lib.rs:443-571` — Unauthenticated API + `ACAO: *`.** Every response sends + `Access-Control-Allow-Origin: *` (verified at :556, :588, :607, :631); no `Host`/`Origin` checks. + Consequences: (1) `GET /api/pending` returns full `sign_event`/`nip44_encrypt` params (message + plaintext) readable by any web page in Firefox-family browsers and by DNS rebinding anywhere; + (2) `:516-517` clears the queue on read → third party can steal requests so the real extension + page never sees them; (3) `POST /api/response` accepts any `{id, result}` → forged + `get_public_key` substitutes an attacker pubkey as "the user". Fix: drop CORS entirely (the + served page is same-origin), reject non-`127.0.0.1:7400` Host, gate `/api/*` on a per-session + random token embedded in the served page. +- **[H] `lib.rs:339-366` — Timed-out requests leak forever.** The `RequestData` and oneshot sender + stay in the maps after the 30 s timeout — plaintext retained for the session, re-served to + whoever polls next, map grows unboundedly. Remove entries on timeout (self-removing guard or + id-returning `request`). +- **[M] `lib.rs:374-380` — `_sign_event` verifies the signature but not the event.** Nothing checks + the returned event matches the requested `UnsignedEvent` (pubkey/content/tags/kind/created_at); + a compromised signer page can substitute content. Compare before accepting. +- **[M] `lib.rs:538` — Decrypted plaintext debug-logged.** `tracing::debug!("Received response … + {message:?}")` leaks `nip44_decrypt` output and event content the moment anyone runs with + `RUST_LOG=debug`. Log id + method only. +- **[M] `lib.rs:447,465-480,523` — Unbounded reads/allocations**: header line grows unbounded; + `vec![0u8; content_length]` trusts `Content-Length` (4 GiB reservation from one POST); no + connection timeout. Cap header block (16 KiB), cap body (1 MiB), add idle timeout. +- **[L] `lib.rs:650-678` — `race_timeout` reports channel-closed as timeout.** Distinguish. +- **[L] `lib.rs:574-638` — Three near-identical response writers**; one function with a header + options struct (also makes the CORS removal enforceable in one place). +- **[L] `lib.rs:546,563,593,612,636` — bare `let _ =` on writes**; centralize the + best-effort-write rationale in the helper. + +Positives: loopback-only bind, graceful shutdown tied to handle lifetime, per-request timeouts, +UUID ids, bounded channels. + +### 4.5 `nip4e` — per-device keys + +- **[M] `lib.rs:1036,774-860` — Approval button wedged + double-clickable.** `loading.set(true)` + without `cx.notify()` keeps the old button enabled (double-click sends the secret in two 4455 + events); on failure `loading` is never reset and the notification isn't removed. Notify after + setting, reset on error, rebuild the notification on both outcomes. +- **[M] `lib.rs:403-405` — Rotation skips archiving silently, then overwrites the key.** If the DB + read/decrypt fails transiently, `archive_key` is skipped but `set_keys` still overwrites → old + chats permanently undecryptable. Make archive failure abort rotation (or log loudly). +- **[M] `lib.rs:175-206` — Relay listener can die silently.** `tx.send_async(...).await?` aborts on + full bounded(100) channel or dead consumer; the task's `Err` is never inspected. All 4e event + processing stops until the next signer change. Log and handle backpressure. +- **[M] `lib.rs:90` — `tasks` grows on every settings change** (`get_announcement` runs per + settings mutation) — see S1. +- **[M] `lib.rs:177,184` — `processed_events` inserts **every** relay event** (insert precedes the + kind filter) and grows unbounded. Filter first; bound the set. +- **[L] `lib.rs:303-309` — Desktop key backup file written 0644** (subject to umask); use 0600. +- **[L] `lib.rs:178` — `current_user` snapshotted per listener**; events between signer change and + `get_public_key_async()` resolving are dropped, and a stale snapshot can briefly forward one + account's events to the next handler. Re-check per event. +- **[L] `storage.rs:29-32` — Malformed stored `created_at` silently resets the rotation + interval.** Log the fallback. +- **[L] Zeroization gaps** (`app_keys`, decrypted secrets in `decrypt_approval`/`approve`): Low — + the nostr stack itself doesn't zeroize; note as defense-in-depth. +- **[L] `lib.rs:141,246,426,458,845,854,894,1041` — `.ok()` discards** (see S2); `lib.rs:479` + single-letter `n`. +- **[L] `lib.rs:136,241,888,914` — `cx.windows().first()` targets an arbitrary window** (matters + only if multi-window ever lands). + +Positives: secret never published/logged; transfer encrypted to a fresh one-time keypair; +approvals re-validated against the live announcement; stale requests dropped by age; bounded +archive; account switch wipes pending state. + +### 4.6 `chat` — chat registry + rooms + +- **[H] `lib.rs:331-338` + `message.rs:203-220` — Failed-to-decrypt messages silently trashed; + nothing renders `trash`.** The `trash: Entity>` is written, cleared, and + reprocessed, but has **no reader or renderer workspace-wide**. A user whose incoming message + can't be decrypted gets zero feedback — core E2EE UX gap. Emit a `ChatEvent` or render an + indicator. +- **[M] `lib.rs:284,327,620-635,726,733,854,892` — `RwLock` `.read().unwrap()`/`.write().unwrap()` + poisoning crash loop.** `search.write()` runs on a background thread; one panic while holding it + turns every later foreground read into a panic. These maps are reconstructible heuristics — use + the tolerant `poisoned.into_inner()` pattern the codebase already has + (`person/src/directory.rs:226-233`). +- **[M] `lib.rs:779-780` — Off-by-one index stored in `room_map`** (`rooms.len()` after push). + Currently unreachable (batch rooms come from an id-keyed set) but any future reader panics at + `self.rooms[index]`. Store `len() - 1`, or drop the never-usefully-read index bookkeeping. +- **[M] `lib.rs:440-443,454-461,825-833` — Discarded subscribe result + `unwrap_or_default()`.** + `_ = client.subscribe(...)` (rule violation); a DB error becomes "not found" → false + "Messaging Relays not found" retry prompt, and a contacts DB error classifies peer rooms as + `Request` instead of `Ongoing` (wrong trust classification). Distinguish errors from empty. +- **[M] `lib.rs:110,199,232-358,362-414` — Stored task results never inspected** (see S1/S2); a + dead notification listener silently stops message delivery — at least `detach_and_log_err`. +- **[L] `room.rs:637-646` — `deliver` generic has one implementation**; inline it. +- **[L] `lib.rs:867` — `max_by_key().unwrap()`** relies on an invariant three lines up; + `let Some(...) else { continue }` documents it. +- **[L] `lib.rs:332-338` — Empty-tags rumor is both trashed and delivered** (appears as failure + entry *and* rendered message until reprocess). Pick one path. +- **[L] `room.rs:350` — `self.members[0]` safe only via a cross-crate invariant** + (`extract_public_keys` always appends the author). Add `debug_assert!` or a comment. +- **[L] `Cargo.toml:28-29` — `smol` unused.** Remove. +- **[L] `lib.rs:182-186` — `let _ = entity.update(...)`** in deferred init; `.log_err()`. + +Positives: signal batching bounds per-update work; wasm yield loop; gift-wrap dedup; seal +verification; `WeakEntity` room handles; `ChatEvent::Error`/`Loaded`/`Ping` genuinely surfaced. + +### 4.7 `chat_ui` — chat panel + +- **[H] `lib.rs:605-611` — Your own reactions count twice.** `insert_reaction` pushes + unconditionally (keyed only by target id); with `backup: true` (default) the reaction comes back + as your own backup gift wrap → inserted again → "👍 2". Text messages are dedup'd by + `binary_search`; reactions aren't. The dormant `RoomEvent::Reload` path double-counts too. + Track seen reaction event ids. +- **[H] `lib.rs:325-334` (+ `272-277`) — Room message-load and connect failures swallowed.** The + stored task's `Result` is never inspected: user sees an empty room with no error; relay + subscription failures invisible. Push a notification on error. +- **[M] `lib.rs:557-581` — Bulk load is O(n²) on the foreground thread.** Per-event + binary-search + index rebuild + splice inside one `update`; multi-second stall on big rooms. + Add a bulk variant (sort once, rebuild index once, splice once). +- **[M] `lib.rs:569-570` — Insert position and `ListState::splice` disagree.** Messages are + binary-inserted by `created_at` but the list always appends at `len..len`; mid-`Vec` arrivals + shift rows while the list state learns "appended one" → stale height caches, scroll jumps. + `nip29_ui/src/lib.rs:516` does it right: `splice(pos + 1..pos + 1, 1)`. +- **[M] `lib.rs:1183-1198` — `rendered_texts_by_id` caches mention names forever** (same defect as + `nip29_ui` 4.8). A message rendered before the profile loaded shows `@npub1…` all session. + Key the cache on directory generation or drop the resolved-text cache. +- **[M] `lib.rs:50-51,395-398` — `\p{Emoji}` matches plain digits: typing "2" while replying to + exactly one message sends a "2" reaction instead of a message.** Require + `[\p{Emoji_Presentation}\p{Extended_Pictographic}\u{200D}\u{FE0F}\u{20E3}]`. +- **[M] `lib.rs:110` — `tasks: Vec` grows without bound** (see S1). +- **[M] `lib.rs:547-549,618,623` — `reports_by_id: Arc>` nothing else shares.** Plain + `BTreeMap` field; removes the poisoning risk for zero benefit. +- **[L] `lib.rs:499` — `rumor.id.expect("rumor must have an id")`** — invariant holds today, but + degrade + notify instead of panic per house style (also `to_bech32().unwrap()` at 652, 1072). +- **[L] `lib.rs:362-370,797-802` — `.ok()` discards `room.update`** (subject change, retry action). +- **[L] `lib.rs:722-727` — `encrypted` snapshotted before the file-picker await**; typing while the + picker is open classifies the file by stale input state. Re-read after the await. +- **[L] `lib.rs:1170-1178` — `load_file` invoked from the render path** (guarded, but render should + be pure — move to a subscription). +- **[L] `lib.rs:147-221` — Summary comments** ("Define input state", …) violate the comment rule. + +### 4.8 `nip29` — NIP-29 registry + protocol + +- **[H] `lib.rs:541-555` (merge), `582-607` (absorb_list) — Stale group-list events resurrect left + groups and get republished.** The kind 10004 list is absorbed without comparing `created_at` and + `merge` never removes entries absent from older snapshots; a relay still holding the pre-leave + version re-adds the left group, `ensure()` re-creates subscription, and the next + `refresh_entry`/`publish_list` republishes the left group to all write relays (privacy angle). + Track the last accepted list event's `created_at`; ignore older. +- **[H] `group.rs:696-728` (absorb_state) — Group state applied last-write-wins, no ordering + guard.** Out-of-order relay delivery (newest-first is common) leaves the *oldest* + metadata/members/admins/roles/pins displayed indefinitely. Cache hydration is safe (sorted + ascending at `group.rs:507`); the live path is not. Keep `created_at` per state kind (the + `forks` map at `lib.rs:669-675` shows the pattern). +- **[M] `lib.rs:173-174,189` — Join published optimistically; refusal classified by relay error + prose** (`reason.starts_with("duplicate")`); a refused join stays in the public list. Drop the + entry on refusal; detect duplicates via a stable marker. +- **[M] `lib.rs:173,825-834` — Rejoin flips a private list entry to public** (`join_key` hardcodes + `private: false`, `upsert` replaces the whole entry). Preserve the existing flag. +- **[M] `group.rs:418-448` + `lib.rs:383-397,1146-1159` — Failed page load leaves `loading_more` + stuck true forever** (subscribe error path emits no signal; guard then permanently blocks + pagination for the session). Route a synthetic settle on error. +- **[M] `lib.rs:308-330` + `discovery.rs:253,267,274,281` — Network failures silently produce + "empty" results** (`and_connect().await.ok()`, discarded sync errors): browse shows zero + candidates, discovery silently stale. Surface `GroupsEvent::Error`. +- **[M] `discovery.rs:240-252,264-267` — Empty contact list → `Filter::authors(∅)` serializes as + absent → the sync reconciles **every user's** list, and the pool-wide sweep (kinds + `[GroupMetadata, GroupMembers]`, no ids/limit) pulls all NIP-29 state from all relays.** + Early-return when `authors.is_empty()`; constrain the sweep. +- **[M] `group.rs:457-511` + `lib.rs:906-941,1042-1071` — Every live event triggers a full + unlimited `load_envelopes` reload** (re-query + JSON-decode + sort of entire group history per + message) and every event is cached forever. Use the single-event absorb path live; cap/prune the + cache. +- **[M] `lib.rs:90` — `tasks: SmallVec<[Task; 2]>` accumulates per incoming event** (see S1). +- **[L] `protocol.rs:389-399,534,650,964` — One malformed optional tag fails the whole parse** + (one bad `supported_kinds` value blanks a group's metadata display). Skip + log instead. +- **[L] `protocol.rs:24` — Custom kind 9010 not in rust-nostr 0.45.6's NIP-29 range**; confirm + against target relay implementations before stable. +- **[L] `group.rs:602-611` — `previous` mismatch only logs a warning**; an empty `previous` tag + counts as fully verified (`protocol.rs:1046-1056`). Align comment with behavior or escalate. +- **[L] `protocol.rs:995-1016` — `TimelineWindow.seen` grows unbounded** (u32/event, never pruned). +- **[L] `lib.rs:582-607,661-677` — No client-side author re-check on list/fork subscriptions.** + Mitigated by `verify_subscriptions(true)` + `ban_relay_on_mismatch(true)`; note the residual + NIP-29 trust model rests on relay admission policy. +- **[L] `lib.rs:860-904` — Concurrent `publish_list` can complete out of order** (each snapshots at + call time; older snapshot can win). Serialize publishes. +- **[L] `lib.rs:1225-1230` — Negentropy/foreign-EOSE cross-talk** fires spurious `Synced` signals + from unrelated subscriptions. +- **[L] `utils.rs:24,31,35` — Subscription ids truncate the digest to 32 bits**; collision would + cross-wire routes. Negligible at this cardinality; use the full digest anyway. +- **[L] `lib.rs:1198-1200` — `processed` dedup wholesale-cleared at 10k**; evict oldest instead. +- **[L] `lib.rs:723-736` — `previews` grow unbounded per session** (entity + subscription each). +- **[L] `group.rs:197-202` — `Group::message(id)` O(n) scan** called per reply preview in render + paths → O(n²) timelines. Build an id→row map. +- **[L] `Cargo.toml:14` — `person` dep unused.** Remove. +- **[L] `lib.rs:1208-1229` — `.ok()` on signal sends** (benign; log or comment per rule). +- **[L] `lib.rs:358,949,975` — Inline global-handle chains** (rule: bind to a local). + +Positives: zero reachable panics; tag parsing consistently `get`/`skip`/`first`; +`saturating_sub` timestamp math; tasks/subscriptions stored; `WeakEntity` spawns; membership +derivation order-independent; strong publish-error propagation. + +### 4.9 `nip29_ui` — group panel + dialogs + +- **[H] `lib.rs:1053-1067` — `rendered_texts_by_id` bakes mention names in permanently.** + Computed once per `EventId`; when the profile arrives later the body keeps `@npub1ab…` (the + header recovers via re-render, the body doesn't). Key on directory generation or don't cache the + resolved text (only the markdown source needs caching). +- **[M] `lib.rs:337` — `tasks` accumulates** from every send/upload/react/delete/pin (see S1). +- **[M] `lib.rs:1068` — Content `TextView` keyed by row index.** Prepending shifts every index → + every visible `TextViewState` re-parses markdown in one frame ("Load earlier messages" jank). + Use the message id as element id. +- **[M] `message.rs:359` + `lib.rs:1068` — Sibling ElementId collision: reply previews use bare + reply index as sibling of the row-index TextView**; when `replies.count > row_index` both sides + share `…/[N]/[N]`. Latent today, breaks silently when element types coincide. Namespaced ids. +- **[L] `message.rs:192,227,238` — `let _ = panel.update_in(...)`** where sibling call sites log + (`log::warn!` at 110-160). Mirror the logged pattern. +- **[L] `lib.rs:630` — File-picker failure silently swallowed** (task result never inspected). +- **[L] `lib.rs:436-445` — Utterance-end loop clears the reveal unconditionally**; interrupted + utterances still fire `on_utterance_end` on many backends, cancelling B's reveal while B's audio + plays. Guard with the session id like `message.rs:144` does. +- **[L] `message.rs:97,130,168-180` — Per-frame per-row allocations** (`preview()` computed twice + per row per frame, custom-emoji `Vec` rebuilt + cloned into the submenu closure every render — + and gpui-pre redraws the window on every keystroke). Compute once per render. +- **[L] `lib.rs:677,956` — Inline global chains** (rule). +- **[L] `lib.rs:407-419` — Subscribe + observe both run `sync` + `track_participants`** (events + and notifies fire together) → double O(rows) scan per change. +- **[L] `message.rs:322` — Empty role list renders "is now "** (trailing space, no roles). Emit + "now a regular member" for the empty case. + +### 4.10 `common` — shared utilities + +- **[H] `media_extractor.rs:52-58` — Every message body has all whitespace collapsed, destroying + line breaks.** `cleanup_text`'s `\s+ → " "` runs on the entire remaining text unconditionally, + so multi-line messages render as one line in both chat and groups (via `chat/message.rs:99-103`). + Normalize only around removed URLs; never touch `\n` in untouched regions. +- **[M] `media_extractor.rs:15-18,56,62-65` — Regexes compiled per message** (the cache struct's + own entry point defeats it). `static LazyLock`. +- **[M] `parser.rs:240,254,260` — `NostrParserIter::next` recurses per token; depth is + attacker-controlled** (~9 bytes/token of crafted `nostr:` URIs). Desktop's 8 MB stack survives; + the wasm32 web build can crash the tab. Rewrite as a loop. +- **[M] `event.rs:12-18` — `uniq_id` uses `DefaultHasher` and is persisted to published events** + (see S5). +- **[M] `paths.rs:7,14,23,31` — `expect()` on `dirs::download_dir()` etc.** — legitimately `None` + on Linux without XDG user-dirs → panic on export (`workspace/src/lib.rs:405`). Return `Option` + and handle. +- **[L] `display.rs:18,20,39` — `as i64` on relay-controlled timestamps** (contained by + `LocalResult`); fallback magic strings `"9999"`/`"1m"`. +- **[L] `event.rs:12-53` — `Event`/`UnsignedEvent` impls duplicate the same logic with drift** + (one sorts, one doesn't). Factor into one helper — same helper fixes S5. +- **[L] `debounced_delay.rs:33` — `_ = channel.send(())`** explicit silent discard (harmless; + satisfy the rule with a comment + `if let Err`). + +Positives: `web_store.rs` error handling exemplary; parser char-boundary-safe throughout; +`DebouncedDelay` cancellation correct; time formatting degrades gracefully. + +### 4.11 `person` — profiles + web of trust + +- **[H] `lib.rs:310-333` + `204-245` — One failed metadata batch permanently blacklists those + pubkeys.** `in_flight` removal happens only on metadata arrival; `get_metadata` uses + `TIMEOUT = 2 s` (tight for bootstrap relays + EOSE) and on failure keys stay in `in_flight` + forever → those people show truncated-pubkey placeholders until restart. Return failed keys on + error and release them; raise the request timeout independently of the batching window. +- **[M] `lib.rs:175` — Malformed metadata wipes a good profile.** + `Metadata::from_json(...).unwrap_or_default()` replaces previously-loaded data with a default + `Person`; content is attacker-influenceable (anyone can publish metadata for any key). Skip on + parse failure. +- **[M] `directory.rs:110` (+ `lib.rs:255-262`) — Task handles accumulate per `track`** (see S1). +- **[M] `directory.rs:161-172` — `person()` performs side effects (lock write + network fetch) from + a render-time getter.** Deduped via `pending`, but order-dependent render behavior; consider an + explicit `ensure_tracked` from update paths. +- **[L] `wot.rs:100,343` — DB errors silently become empty results**; add `log::warn`. +- **[L] `wot.rs:150-153` — `let _ = this.update(...)`** (commented, periodic; noted for the rule). +- **[L] `person.rs:23-47` — `PartialEq/Eq/PartialOrd/Ord/Hash` for `Person` unused + workspace-wide**; the `Ord` impl clones `Metadata` twice per comparison. Delete. +- **[L] `person.rs:66-68,124-138` — `metadata()` clones per call; `name()` calls it twice** per + invocation → 2+ clones per avatar/name row per frame. Borrow or cache. +- **[L] `person.rs:166-177` — `shorten_pubkey` doc example doesn't match output; panics if + `len ≥ 63`** (callers use 4/8 today). Note: the `let Ok(...) =` pattern compiles only because + `ToBech32::Err = Infallible` — worth a comment since it looks like a compile error. + +Positives: WoT builder caps hostile publishers; poisoning-tolerant lock helpers; bounded dispatch +channel; `limit(1)` newest-event lookups verified correct against nostr-lmdb ordering. + +### 4.12 `speech` — dictation + TTS + +- **[H] `parakeet.rs:134` — Inference worker exits after 30 s idle, permanently disabling + dictation.** `recv_timeout(30s)` kills the thread; the `OnceLock` sender survives, so after + that every `send` fails — and `send_or_log` (`:200-204`) logs at **debug** and drops. A + `MAX_SECONDS = 120` dictation records fine, then `finish()`'s `Transcribe` goes into the void; + gpui-kit's 180 s `stop_timeout` then force-ends with an **empty** transcript. Net: dictations + longer than 30 s silently lost after up to a 3-minute hang. Fix: block on `recv()` (an `Unload` + command already exists for explicit retirement), and make `finish()` surface send failure via + `sink.error`. +- **[M] `parakeet.rs:63-68` — Audio past 120 s silently discarded** mid-sentence with no + indication. Surface once or stop capture at the cap. +- **[M] `parakeet.rs:119-128` — Worker-spawn failure leaves a permanently dead channel** (receiver + dropped, sender cached in the `OnceLock` forever). Don't cache on failure. +- **[M] `speaker.rs:36-53` — `Tts::default()` constructed lazily inside `speak()` invoked from + render paths**; first-use engine init can take tens-to-hundreds of ms on the UI thread + mid-render. Initialize eagerly at panel creation or on the background executor. +- **[L] `model.rs:82-97` — `status()` does filesystem I/O per call** while the settings dialog + polls every 500 ms on the foreground. Cache status in the state mutex. +- **[L] `model.rs:183` — `let _unused = fs::remove_file(...)`**; log per rule. +- **[L] `web.rs:23-25` — `is_available` hardcodes `true`**; mic button renders enabled in browsers + without `SpeechRecognition`. Reflect actual support. +- **[L] `web.rs:200,218` — `let _ignored` abort/stop** — commented, best-effort; fine. + +Positives: `catch_unwind` isolation around model load/transcribe; staged download with SHA-256 + +size cap + atomic rename; dedicated threads for inference/download; all other fallible paths +reach `sink.error`. + +### 4.13 `settings` — persistence + themes + +- **[H] `lib.rs:403-404` + `311-314` — Failed load silently resets settings and destroys the saved + file.** `unwrap_or(Settings::default())` swallows I/O and JSON errors; the observer then + immediately writes defaults over the user's file (losing `trusted_relays`, pins, wallet prefs) + with no log. Log, keep defaults in memory, but don't persist until a real user change. +- **[H] `lib.rs:423-438` — Saves discard errors and race each other.** `.await.ok()` hides + disk-full/permission failures; every change detaches a new concurrent write to the same path — + last-completion-wins can overwrite newer snapshots and interleaved truncate/write can corrupt + the file (font-size input ticks each trigger a save). Single writer slot + temp-file + rename + + `log_err`. +- **[L] `lib.rs:425` — Serialization failure silently skips saving**; log. +- **[L] `lib.rs:596-639,671-683` — `apply_persisted_theme` re-reads and re-parses the settings + file four times.** Parse once. +- **[L] `lib.rs:211,408` — `Url::parse(DEFAULT_FILE_SERVER).unwrap()`**; `OnceLock` or a reasoned + `expect`. + +Positives: accessor macros keep 20 settings consistent; serde default/alias migration fields with +tests covering old formats; wasm path handled. + +### 4.14 `workspace` — core + dock + sidebar + +- **[H] `dock/mod.rs:50-73` + `lib.rs:309-320` — Tab dedup is unreachable; repeated opens stack + duplicate tabs.** `find_panel` matches on `PanelId`, which is always `PanelId::from(entity_id())` + of a panel created in the same statement — so it can never match (verified against gpui-base + 0.9.1 `add_panel_inner`, which inserts without dedup). Clicking the same chat row twice opens + two live panels for one room; ShowInbox/Browse/Contacts/Backup multiply tabs. Key panels by + domain identity (singleton entities per panel name, or room/group key). The ~55 lines of + `find_panel`/`move_panel` are dead until then. +- **[H] `sidebar/rows.rs:54-57` — "Ignore" dispatches `ClosePanel` where it can never be handled.** + The only handler is on the tab-group frame; the alert dialog holds focus at that moment, and + dispatch resolves along the focus node's ancestor path — the dock tree isn't on it. Clicking + "Ignore" is a silent no-op for a destructive-intent action. Close the room via the dock handle + already in scope, or restore focus first. +- **[H] `sidebar/header.rs:38,236-268` — Updater buttons render stale status.** Nothing observes + the `AutoUpdater` entity (gpui-pre has no read-tracking); a staged update can sit invisible on + an idle window indefinitely. Add `cx.observe(&updater, …)`. +- **[M] `sidebar/mod.rs:192-206` + `search.rs:70-117` — Full message-index scan on the foreground + thread for every keystroke.** Only the *global* search is debounced; local search synchronously + scans every indexed message, sorts the entire match set, then truncates. Debounce + background + spawn + partial top-`limit` select. +- **[M] `sidebar/list.rs:41,178-248` + `group_view.rs:88-136` — Row model rebuilt on every sidebar + render** (clone + partition + pin-tag scans ×visible rows; sidebar re-renders on 500 ms ticks, + keystrokes, group updates). Cache the row model in state, refresh on registry events. +- **[M] `sidebar/search.rs:152-155` — Global search failure renders as "No profiles found".** + Surface the error state. +- **[M] `lib.rs:410-421` — Save-dialog failure silently cancels the backup** (`Err(_) => return`): + indistinguishable from user cancel; user believes the key was backed up. Split arms + notify. +- **[M] `sidebar/mod.rs:139-142` — Whole-sidebar re-render on every `GroupsEvent::Updated`** + (acknowledged TODO; compounds the row-rebuild cost). Gate on visibility. +- **[L] `dock/skin.rs:25,35-37,55` — `toggle_button_visible: Cell` is write-once** (set true + in ctor, never elsewhere) → dead state + dead branch at `tab_panel.rs:212`. Delete. +- **[L] `sidebar/rows.rs:174-374` — Index-based row ids** (see S6). +- **[L] `dock/tab_panel.rs` ×6, `skin.rs:262`, `sidebar/search.rs:164,178`, `lib.rs:641` — `let _ = +`/`.ok()` on fallible updates** (see S2); `lib.rs:641` is pre-destructive-reset — make it visible. +- **[L] `rows.rs:220-222,346-351`, `list.rs:53-55` — Inline global chains** (rule). +- **[L] `lib.rs:84`, `sidebar/mod.rs:99`, `sidebar/inbox.rs:68,118` — `tasks` accumulate** (see S1). +- **[L] `lib.rs:54` — Typo: `DeviceNotifcation`.** + +Positives: careful `WeakEntity` hygiene; virtualized lists via `uniform_list` + `Rc` snapshots; +`InvalidPanel` preserves unknown panels across load/save; `theme_name_at` maps the palette's +original `IndexPath` correctly; `cargo check -p workspace` passes. + +### 4.15 `workspace` — dialogs + panels + +- **[H] `panels/contact_list.rs:187-191` — "Update" republishes the contact list with relay hints + and petnames stripped.** `load()` keeps only `tags.public_keys()`; `update()` rebuilds every + entry as `Contact::new(*public_key)` (`relay_url: None, alias: None`) while the UI reports + success. Silent NIP-02 data loss on relays. Carry the original `Contact`s through. +- **[H] `dialogs/new_chat.rs:60-64` — Contact list doesn't re-filter while typing.** No + `InputEvent::Change` handling; the input renders its own state entity so typing doesn't notify + `NewChat`; results stay stale until an unrelated click/Enter. `cx.notify()` on Change. Same + pattern, cosmetic: `profile.rs:320-326`. +- **[H] `panels/backup.rs:59-76` — Keyring read failures silently swallowed**; backup panel just + shows empty inputs. Log + surface. +- **[H] `dialogs/profile.rs:227-235` — Invalid avatar/website URLs silently dropped on save** while + "Profile updated successfully" shows. Validate both up front; inline error. +- **[M] `dialogs/screening.rs:52-55` — `on_release_in` calls `window.close_all_dialogs(cx)`**, + nuking unrelated stacked dialogs (including the mutual-contacts dialog this view opens). Close + just the hosting dialog / cancel tasks only. +- **[M] `dialogs/screening.rs:126-168` — DB-fallback "mutual contacts" has different semantics than + the WoT list** (followers vs. your-contacts-who-follow). Intersect or relabel. +- **[M] `dialogs/screening.rs:211-232` — NIP-05 network errors render as "address does not + match"** (`unwrap_or(false)` asserts infrastructure failure as fact about the person). Tri-state. +- **[M] `dialogs/screening.rs:250-282` — Report submit gives no failure feedback.** +- **[M] `dialogs/import.rs:129-185` — Enter bypasses the loading guard** (no in-method guard; all + three `PressEnter` subscriptions call `login()` directly) → concurrent double import. Same class: + `contact_list.rs:176-228`, `relays.rs:377,521`. +- **[M] `dialogs/import.rs:31-40` — Unlock dialog is inescapable** (no cancel/logout path for a + forgotten passphrase). Add a "start over" escape hatch for stable. +- **[M] `panels/browse.rs:484-493,316-348` — Card list recomputed per render *and* per visible + row** (O(n) clone + registry reads ×(rows+1) per frame during scroll). Compute once per render + (`new_chat.rs`'s `Rc` pattern). +- **[M] `panels/greeter.rs:127-137` — Every refresh fetches the full message history of every room + to take `.last()`.** Query latest-per-room instead. +- **[M] `dialogs/new_chat.rs:138-160,388` — Filter allocates per contact per keystroke-scale + render** (`to_hex().to_lowercase()` per contact). Cache lowercase names / filter once per change. +- **[M] `dialogs/settings.rs:124-148` — Speech-model errors can be consumed before the status + change check** and dropped. Check `error` before the equality early-return. +- **[M] `new_chat.rs:282-315` vs `profile.rs:274-313` — Duplicated room-open logic with divergent + dedup.** Unify on find-or-create. +- **[L] `panels/contact_list.rs:238,259` + `greeter.rs:226-402` — HashSet/index-based element ids** + (see S6). +- **[L] `panels/contact_list.rs:288` — Copy bug: contacts panel says "Please add some relays."** +- **[L] `dialogs/new_group.rs:392` — `let _ = this.update(...)`** (see S2; `relays.rs:623-629` is + the correct logged pattern). +- **[L] `panels/backup.rs:66,70` — `.unwrap()` on bech32** (infallible today; still banned pattern). +- **[L] `dialogs/restore.rs:52-62`, `contact_list.rs:130`, `relays.rs:352-355,497-500` — Untrimmed + paste input fails parse** ("trailing newline = invalid key"); `relays` has a weak + `starts_with("ws")` check. +- **[L] `dialogs/relays.rs:124-140,360-366` — Relay metadata can never be reset; duplicate add is a + silent no-op** that can't update metadata. +- **[L] `dialogs/settings.rs:517-543` — Toggles compute `!rendered_value` instead of the switch's + `checked`**; two fast clicks collapse into one. Match `onboarding.rs:398-399`. +- **[L] Dialog `tasks` fields never pruned** (`import.rs:63`, `restore.rs:24`, `relays.rs:45,57`, + `contact_list.rs:52`, `profile.rs:55`, `new_chat.rs:50`, `new_group.rs:94`) — see S1; + `screening.rs`/`greeter.rs` show the right patterns. +- **[L] `new_chat.rs:93-95`, `profile.rs:278-280`, `screening.rs:90-92` — Signed-out paths return + silently**; no retry when login completes later. +- **[L] Dialogs can be stacked repeatedly** — no `has_active_dialog` checks in any `open()`. +- **[L] `new_group.rs:137-140` — `GroupsEvent::Created { warning, .. }` discards the non-fatal + warning.** + +Positives: `limit(1)` newest-event pattern verified correct on both backends; entities created +before `open_dialog` per gpui-kit contract; `new_group.rs` has a solid error lifecycle; heavy work +consistently on background; no unused deps in the crate. + +### 4.16 `ui` — shared components + +- **[M] `avatar.rs:128-186` — `PixelAvatar` regenerates the full pattern every frame** (FNV-1a + + xorshift 64-cell grid + ~50 child divs per avatar; ~30 visible avatars, redrawn on every + keystroke) — the biggest hot-path cost in the main view. Memoize the deterministic pattern + (seed → `[u8; 64]` cache); hoist `FALLBACK_SEED` to a `&'static str` path. +- **[L] `text.rs:11-38` — `InlineReplacement` is `pub` but unreachable externally**; same for + `RUN_WINDOW_SECS` (`message.rs:28`). `pub(crate)` or document as extension points. +- **[L] `message.rs:766` — `RevealText::new` is the only public constructor without a doc line.** +- **[L] `message.rs:775-796` — `RevealText` re-splits text into a `Vec` every + animation frame**; split once in `new`. +- **[L] `text.rs:89-95` — Non-web, non-nevent Nostr links are silently dead** (`npub`/`nprofile` + clicks do nothing). Route to a profile view or drop the link markup. +- **[L] `Cargo.toml:15,19` — `web-time` and `uuid` unused.** Remove. + +Positives: consistent `RenderOnce` + `Styled` builder pattern; `#[must_use]` builders; documented +public items; char-boundary-safe slicing; truncating casts provably bounded. + +### 4.17 `auto_update` + +- **[M] `lib.rs:92` — Checksum-only verification; no signing key configured.** SHA256SUMS comes + from the same host as the artifact — protects against corruption, not a compromised release + host/account. The engine already supports minisign/Ed25519; publish a sidecar signature and pin + the public key before stable. +- **Verified non-issue:** the `futures::executor::block_on` in `source.rs:152,170` runs on the GPUI + **background** executor (verified through `gpui-updater-pre 0.2.0`'s `Updater::check` → + `engine.check()` → `fetch_latest` call chain) — no UI freeze. Severity Low: it parks one + background thread up to ~60 s during a check. Optional tidy-up only. +- **[L] `source.rs:156-163,180-183` — Asset URLs not scheme-checked**; assert `https://`. +- **[L] `lib.rs:119-131,145-147` — A failed check shows for 5 s then reports "Up to date"** + (`error_revert` → `Idle`, and `Idle` counts as up-to-date). Distinct "check failed" state. +- **[L] `lib.rs:103,128` — `.ok()` on entity updates** (see S2). +- **[L] `lib.rs:134-139` — Auto-download/stage ~2 min after startup without a prompt**; fine on + unmetered, worth a setting. + +Positives: strict `latest > current` gate (no downgrades); drafts/prereleases skipped; 30 s +timeouts; defensive checksum parsing; good tests. + +### 4.18 `gpui_tokio` + +- **[L] `lib.rs:14` — `.expect("Failed to initialize Tokio")`**; per the no-panic rule, `init` + returning `Result` would be more in keeping (realistically unreachable). +- **[L] `lib.rs:38` — `pub struct Tokio {}`** namespace struct; `pub struct Tokio;` or free + functions. + +Positives: the `defer`-guard cancellation pattern is correct in both completion and drop cases; +`shutdown_background()` avoids blocking exit; `spawn_result` maps `JoinError` into `anyhow`. +No deadlock risk across the two runtimes (verified: nothing waits on GPUI from tokio or holds a +lock across the boundary). + +### 4.19 `desktop` / `web` / `assets` entries + +- **[M] `desktop/src/main.rs:20-26,153-170` — Secret key accepted as a CLI argument.** `--sec + nsec1…` puts the master key in the process list (`ps`, `/proc`, shell history); the parse-error + path echoes the raw (near-)key to stderr. Read from stdin/env; print error kind only, never the + input. +- **[L] `desktop/src/main.rs:131-150` — Seven startup unwraps on embedded assets.** Deterministic, + but one corrupt font aborts launch; skip bad fonts with a warning and fail only if zero loaded. + `open_window().expect` is fair (genuinely unrecoverable). The startup `block_on` for fonts + (main.rs:135) is intentional and acceptable — noted so it isn't confused with the anti-pattern. +- **[L] `web/src/lib.rs:122` — `expect("Failed to open window…")` aborts the wasm module** (blank + page + console error; `panic = "abort"`). A DOM-level error would be kinder. +- **[L] `web/src/lib.rs:43` — `.ok()` on console-log init** (justified; noted for completeness). +- **[L] `desktop/Cargo.toml:37` + `web/Cargo.toml:13` — `ui` dep unused in both entries.** Remove + unless it exists to force feature unification (then comment why). +- **[L] `assets/Cargo.toml:9` — `anyhow` unused** (code uses `gpui::Result`). Remove. + +Positives: `web` opens IndexedDB before `run()`, panic hook + both logging stacks via `OnceLock`; +`assets` ships the theme-schema test. + +## 5. Tooling Validation + +| Check | Result | +|---|---| +| `cargo clippy --workspace --all-targets` | clean — zero warnings | +| `cargo fmt --all --check` | clean | +| `cargo check -p workspace`, `-p person` | pass | +| `let _ =` / `.ok()` sites (non-test) | ~156 — mostly entity-update-expected-error cases; see S2 | +| `tasks:` container declarations | ~48 sites across 20 owners — see S1 | +| Startup `block_on` sites | 3 (`state` DB open, `desktop` fonts — both acceptable-at-startup; `auto_update` — background thread, non-issue) | + +## 6. Verified Non-Issues (checked, do not re-flag) + +- `auto_update/source.rs` `block_on`: background executor only (verified through the + `gpui-updater-pre 0.2.0` call chain) — no UI freeze. +- `nip29/src/protocol.rs:1072` `block_on`: test-only. +- No locks held across `.await` anywhere in the workspace (all audited crates). +- `person.rs:170` `let Ok(pubkey) = to_bech32()` — irrefutable (`Err = Infallible`). +- `stored_contact_list`'s `limit(1)` + `.next()` — correct: nostr-lmdb orders by `created_at` + descending. +- `nip60`/`nip60_ui` have zero non-test `unwrap`/`expect`/`let _ =`; indexing is guarded; no + truncating casts. +- `nip29_ui`'s `sync()` splice math traced against `ListState` semantics — correct (it's `chat_ui` +' that has the mismatch). +- gpui-pre's entity read-tracking re-renders panels on `Directory`/`SpeechState` notifications — + the missing-`cx.notify()` suspicion in speech handlers is fine; the stale-cache bugs above are + real, the observation isn't. +- `nip60::init` startup ordering is sound for normal flows; the CLI-key path fires `SignerChanged` + before wallet subscription exists (app-wide init-order caveat, affects all registries equally). +- `nip60` melt double-spend paths are latent (dead API), not live. + +## 7. What's Done Well + +- **Clippy-clean, fmt-clean workspace** with no reachable panics in `nip29`, `nip60`, `ui`, + `nip29_ui`, `person` production paths; hostile-input discipline in the WoT builder, speech + downloads (SHA-256 + size cap + atomic rename), and auto-update (fail-closed checksums, no + downgrades). +- **`verify_subscriptions(true)` + `ban_relay_on_mismatch(true)`** in `state` is the single most + valuable client-side trust boundary, and it's on. +- **Entity/subscription hygiene is strong overall**: subscriptions stored in `_subscriptions`, + `WeakEntity` used for room/group/dock back-references, no strong entity cycles found anywhere, + no dropped-subscription bugs. +- **Secrets discipline**: no secret is ever logged or published in `nip4e`/`nip60`/`state` desktop + paths (verified against every log call); `Keys`' vendored `Debug` hides secrets. +- **Errors generally do reach the UI** where it matters most (group writes via `util::report`, + wallet events, speech errors, import/restore flows) — the gaps are enumerated above. + +## 8. Recommended Remediation Order + +**P0 — before any release candidate (blocks release):** +1. nip60 deletion spoofing (4.2 #1) — one-line precedence fix + author check. +2. browser-signer-proxy auth/CORS/token + timeout cleanup (4.4 #1-2). +3. state startup DB fallback (4.1 #1) and web localStorage encryption (4.1 #2). +4. nip60 reconcile/republish money-safety trio (4.2 #2-4) + stuck `redeeming` (4.2 #5). +5. state file-cache collision (4.1 #4) — wrong-file disclosure. +6. chat_ui reaction double-count (4.7 #1) and common whitespace collapse (4.10 #1). +7. contact-list Update data loss (4.15 #1). +8. speech parakeet worker lifetime (4.12 #1). +9. settings load/save durability (4.13 #1-2). +10. nip29 `created_at` guards (4.8 #1-2). + +**P1 — fix during the release cycle:** +Dock panel identity + "Ignore" dispatch (4.14 #1-2); updater observation (4.14 #3); invoice flow +or removal (4.3 #1); person `in_flight` release (4.11 #1); chat trash UI (4.6 #1); chat_ui load +errors + splice + quadratic insert (4.7 #2-4); profile URL validation, backup keyring surfacing, +new_chat re-filter, screening dialog cluster (4.15); parser recursion for wasm (4.10 #3); +`DefaultHasher` → stable hash before more room ids ship (S5); nip4e approval button + rotation +archive + listener death (4.5 #1-3); proxy plaintext leak + event-substitution check (4.4 #3-4). + +**P2 — hygiene, in one mechanical sweep:** +S1 (`tasks` containers → replace-per-spawn / `detach_and_log_err`), S2 (`let _ =`/`.ok()` → +`.log_err()`), S6 (domain-keyed element ids), unused deps (`chat`: smol; `nip29`: person; +`nip60`: tokio; `ui`: web-time, uuid; `assets`: anyhow; entries: ui), dead code +(`nip60` melt/send/quotes decision, `state::passphrase_required`, `DownloadedFile::save`, +`dock::find_panel` fix-or-delete, `skin.rs` write-once flag, `Person` trait impls), avatar +memoization, sidebar row-model caching, rename `DeviceNotifcation`, summary-comment cleanup. + +--- + +*Method note: every crate's sources were read in full by a dedicated audit pass; GPUI/fork API +claims were verified against the vendored `gpui-pre 0.3.12`, `gpui-base 0.9.1`, +`gpui-component 0.9.1`, and rust-nostr sources; the Critical and top-High findings were +independently re-verified by hand against the working tree at `8fa79aef`. No files were modified +during the audit.* -- 2.54.0 From fd802a8870192ae13488795d5092fa5c55e40d7d Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Thu, 12 Nov 2026 17:59:28 +0700 Subject: [PATCH 2/6] harden proxy auth and event ordering --- crates/browser-signer-proxy/proxy.js | 6 +- crates/browser-signer-proxy/src/lib.rs | 174 ++++++++++-------- crates/chat_ui/src/lib.rs | 17 +- crates/common/src/media_extractor.rs | 186 +++++++++++++++----- crates/nip29/src/group.rs | 19 +- crates/nip29/src/lib.rs | 60 +++++-- crates/nip29/src/protocol.rs | 24 +++ crates/nip60/src/lib.rs | 69 +++++++- crates/nip60/src/ops.rs | 166 +++++++++++++---- crates/settings/src/lib.rs | 103 ++++++++--- crates/speech/src/parakeet.rs | 77 +++++--- crates/state/Cargo.toml | 1 + crates/state/src/file.rs | 26 ++- crates/state/src/lib.rs | 41 ++++- crates/workspace/src/panels/contact_list.rs | 65 +++---- docs/audit-stable-release.md | 82 +++++++++ 16 files changed, 819 insertions(+), 297 deletions(-) diff --git a/crates/browser-signer-proxy/proxy.js b/crates/browser-signer-proxy/proxy.js index f04465b2..d15e2724 100644 --- a/crates/browser-signer-proxy/proxy.js +++ b/crates/browser-signer-proxy/proxy.js @@ -5,7 +5,7 @@ async function pollForRequests() { isPolling = true; try { - const response = await fetch('/api/pending'); + const response = await fetch('/api/pending?token=' + encodeURIComponent(window.proxySessionToken)); const data = await response.json(); console.log('Polled for requests, got:', data); @@ -94,7 +94,7 @@ async function handleNip07Request(request) { console.log('Sending response:', responsePayload); - await fetch('/api/response', { + await fetch('/api/response?token=' + encodeURIComponent(window.proxySessionToken), { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -117,7 +117,7 @@ async function handleNip07Request(request) { console.log('Sending error response:', errorPayload); - await fetch('/api/response', { + await fetch('/api/response?token=' + encodeURIComponent(window.proxySessionToken), { method: 'POST', headers: { 'Content-Type': 'application/json', diff --git a/crates/browser-signer-proxy/src/lib.rs b/crates/browser-signer-proxy/src/lib.rs index 6617db1b..fc63f29b 100644 --- a/crates/browser-signer-proxy/src/lib.rs +++ b/crates/browser-signer-proxy/src/lib.rs @@ -265,6 +265,10 @@ impl BrowserSignerProxy { let state: Arc = self.inner.state.clone(); let custom_html: &'static str = self.inner.options.custom_html; let shutdown: Arc = self.inner.shutdown.clone(); + // Regenerated on every startup, never part of the opened URL + let session_token: String = Uuid::new_v4().simple().to_string(); + // Only the exact host:port the server binds may appear in the Host header + let expected_host: String = addr.to_string(); smol::spawn(async move { tracing::info!("Starting proxy server on {addr}"); @@ -291,13 +295,22 @@ impl BrowserSignerProxy { AcceptEvent::Connection(Ok((stream, _))) => { let state: Arc = state.clone(); let shutdown: Arc = shutdown.clone(); + let expected_host: String = expected_host.clone(); + let session_token: String = session_token.clone(); smol::spawn(async move { let shutdown_listener = shutdown.listen(); smol::future::or( async { - handle_connection(stream, state, custom_html).await; + handle_connection( + stream, + state, + custom_html, + expected_host, + session_token, + ) + .await; }, async { shutdown_listener.await; @@ -336,6 +349,18 @@ impl BrowserSignerProxy { outgoing_requests.push(request); } + /// Drop a request from both queues so timed-out request data is not retained. + #[inline] + async fn cancel_request(&self, id: Uuid) { + self.inner.state.pending_responses.lock().await.remove(&id); + self.inner + .state + .outgoing_requests + .lock() + .await + .retain(|request| request.id != id); + } + async fn request(&self, method: RequestMethod, params: Value) -> Result where T: DeserializeOwned, @@ -345,6 +370,7 @@ impl BrowserSignerProxy { // Construct the request let request: RequestData = RequestData::new(method, params); + let request_id = request.id; // Create a bounded channel of size 1 as a oneshot replacement let (tx, rx) = channel::bounded::>(1); @@ -361,7 +387,10 @@ impl BrowserSignerProxy { match response { Ok(Ok(res)) => Ok(serde_json::from_value(res)?), Ok(Err(error)) => Err(Error::generic(error)), - Err(TimeoutError) => Err(Error::timeout()), + Err(TimeoutError) => { + self.cancel_request(request_id).await; + Err(Error::timeout()) + } } } @@ -437,10 +466,14 @@ impl AsyncNip44 for BrowserSignerProxy { } } -// ── Minimal HTTP server ────────────────────────────────────────────────── - /// Handle a single HTTP connection. -async fn handle_connection(stream: TcpStream, state: Arc, custom_html: &'static str) { +async fn handle_connection( + stream: TcpStream, + state: Arc, + custom_html: &'static str, + expected_host: String, + session_token: String, +) { let mut reader = BufReader::new(stream); // Read the request line @@ -462,24 +495,49 @@ async fn handle_connection(stream: TcpStream, state: Arc, custom_htm // Read headers until empty line let mut headers = Vec::new(); let mut content_length: usize = 0; + let mut host: Option = None; + loop { let mut line = String::new(); if reader.read_line(&mut line).await.is_err() { return; } + let line = line.trim_end().to_string(); if line.is_empty() { break; } + if let Some(value) = line.strip_prefix("content-length:") { content_length = value.trim().parse().unwrap_or(0); } else if let Some(value) = line.strip_prefix("Content-Length:") { content_length = value.trim().parse().unwrap_or(0); + } else if let Some(value) = line.strip_prefix("host:") { + host = Some(value.trim().to_string()); + } else if let Some(value) = line.strip_prefix("Host:") { + host = Some(value.trim().to_string()); } + headers.push(line); } - match (method.as_str(), path.as_str()) { + // DNS rebinding defense: only the exact bound host:port is accepted + if host.as_deref() != Some(expected_host.as_str()) { + send_response(&mut reader, 403, "Forbidden", "", "").await; + return; + } + + let (route, query) = match path.split_once('?') { + Some((route, query)) => (route, Some(query)), + None => (path.as_str(), None), + }; + + if route.starts_with("/api/") && query_param(query, "token") != Some(session_token.as_str()) { + send_response(&mut reader, 403, "Forbidden", "", "").await; + return; + } + + match (method.as_str(), route) { // Serve the HTML proxy page ("GET", "/") => { let html = if custom_html.is_empty() { @@ -487,7 +545,8 @@ async fn handle_connection(stream: TcpStream, state: Arc, custom_htm } else { custom_html }; - send_response(&mut reader, 200, "OK", "text/html", html).await; + let html = inject_session_token(html, &session_token); + send_response(&mut reader, 200, "OK", "text/html", &html).await; } // Serve the JS proxy script ("GET", "/proxy.js") => { @@ -516,7 +575,7 @@ async fn handle_connection(stream: TcpStream, state: Arc, custom_htm // Clear the outgoing requests after sending them outgoing.clear(); - send_response_cors_json(&mut reader, 200, "OK", &json).await; + send_response(&mut reader, 200, "OK", "application/json", &json).await; } // Receive response from browser extension ("POST", "/api/response") => { @@ -541,27 +600,16 @@ async fn handle_connection(stream: TcpStream, state: Arc, custom_htm let mut pending = state.pending_responses.lock().await; match pending.remove(&id) { - Some(sender) => { - // Use try_send since we already hold the lock - let _ = sender.try_send(message.into_result()); - tracing::info!("Forwarded response for request {id}"); - } + Some(sender) => match sender.try_send(message.into_result()) { + Ok(()) => tracing::info!("Forwarded response for request {id}"), + Err(e) => { + tracing::warn!("Failed to forward response for request {id}: {e}") + } + }, None => tracing::warn!("No pending request found for {id}"), } - send_response_cors(&mut reader, 200, "OK", "text/plain", "OK").await; - } - // CORS preflight - ("OPTIONS", _) => { - let response = "HTTP/1.1 200 OK\r\n\ - Access-Control-Allow-Origin: *\r\n\ - Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n\ - Access-Control-Allow-Headers: Content-Type\r\n\ - Content-Length: 0\r\n\ - Connection: close\r\n\ - \r\n"; - let _ = reader.get_mut().write_all(response.as_bytes()).await; - let _ = reader.get_mut().flush().await; + send_response(&mut reader, 200, "OK", "text/plain", "OK").await; } // 404 - not found _ => { @@ -585,68 +633,44 @@ async fn send_response( } response.push_str(&format!("Content-Length: {}\r\n", body.len())); - response.push_str("Access-Control-Allow-Origin: *\r\n"); response.push_str("Connection: close\r\n"); response.push_str("\r\n"); response.push_str(body); - let _ = stream.write_all(response.as_bytes()).await; - let _ = stream.flush().await; + write_response(stream, &response).await; } -/// Write a response with CORS headers and JSON content type. -async fn send_response_cors_json( - stream: &mut (impl AsyncWriteExt + Unpin), - status: u16, - status_text: &str, - body: &str, -) { - let mut response = format!("HTTP/1.1 {status} {status_text}\r\n"); - response.push_str("Content-Type: application/json\r\n"); - response.push_str(&format!("Content-Length: {}\r\n", body.len())); - response.push_str("Access-Control-Allow-Origin: *\r\n"); - response.push_str("Connection: close\r\n"); - response.push_str("\r\n"); - response.push_str(body); - - let _ = stream.write_all(response.as_bytes()).await; - let _ = stream.flush().await; -} - -/// Write a response with CORS headers. -async fn send_response_cors( - stream: &mut (impl AsyncWriteExt + Unpin), - status: u16, - status_text: &str, - content_type: &str, - body: &str, -) { - let mut response = format!("HTTP/1.1 {status} {status_text}\r\n"); - - if !content_type.is_empty() { - response.push_str(&format!("Content-Type: {content_type}\r\n")); +async fn write_response(stream: &mut (impl AsyncWriteExt + Unpin), response: &str) { + if let Err(e) = stream.write_all(response.as_bytes()).await { + tracing::debug!("Failed to write response to peer: {e}"); + } + if let Err(e) = stream.flush().await { + tracing::debug!("Failed to flush response to peer: {e}"); } - - response.push_str(&format!("Content-Length: {}\r\n", body.len())); - response.push_str("Access-Control-Allow-Origin: *\r\n"); - response.push_str("Connection: close\r\n"); - response.push_str("\r\n"); - response.push_str(body); - - let _ = stream.write_all(response.as_bytes()).await; - let _ = stream.flush().await; } -// ── Timeout helper ─────────────────────────────────────────────────────── +/// Get a parameter value from an optional query string. +fn query_param<'a>(query: Option<&'a str>, name: &str) -> Option<&'a str> { + let query = query?; + query.split('&').find_map(|pair| { + let (key, value) = pair.split_once('=')?; + (key == name).then_some(value) + }) +} + +fn inject_session_token(html: &str, token: &str) -> String { + let script = format!(""); + match html.rfind("") { + Some(index) => format!("{}{script}{}", &html[..index], &html[index..]), + None => format!("{html}{script}"), + } +} /// An error indicating that an operation timed out. #[derive(Debug)] struct TimeoutError; /// Races a channel receive against a duration. -/// -/// Returns the channel value on success, or [`TimeoutError`] if the duration -/// elapses first or the channel is closed. async fn race_timeout( duration: Duration, recv: impl Future>, @@ -677,10 +701,6 @@ async fn race_timeout( } } -// ── Utility ────────────────────────────────────────────────────────────── - -/// Gets the current time in seconds since the Unix epoch (1970-01-01). If the -/// time is before the epoch, returns 0. #[inline] fn current_time() -> u64 { SystemTime::now() diff --git a/crates/chat_ui/src/lib.rs b/crates/chat_ui/src/lib.rs index 8f4c0d91..6701b8f3 100644 --- a/crates/chat_ui/src/lib.rs +++ b/crates/chat_ui/src/lib.rs @@ -598,16 +598,23 @@ impl ChatPanel { return; } + let content = SharedString::from(&event.content); let emojis = emojis_from_tags(&event.tags); + let image = Message::reaction_image(&event.content, &emojis) .map(|url| SharedUri::from(url.to_string())); for id in event.tags.event_ids() { - self.reactions.entry(id).or_default().push(( - SharedString::from(&event.content), - image.clone(), - event.pubkey, - )); + let entries = self.reactions.entry(id).or_default(); + // NIP-25: a newer reaction by the same author replaces their previous one + if let Some(existing) = entries + .iter_mut() + .find(|(_, _, author)| *author == event.pubkey) + { + *existing = (content.clone(), image.clone(), event.pubkey); + } else { + entries.push((content.clone(), image.clone(), event.pubkey)); + } } cx.notify(); diff --git a/crates/common/src/media_extractor.rs b/crates/common/src/media_extractor.rs index c688b3f8..34d13f0b 100644 --- a/crates/common/src/media_extractor.rs +++ b/crates/common/src/media_extractor.rs @@ -1,65 +1,161 @@ +use std::sync::LazyLock; + use gpui::SharedUri; use regex::Regex; -/// Extracts media URLs from a string and returns both -/// the extracted URLs and the string with media URLs removed -struct MediaExtractor { - image_regex: Regex, -} +/// Matches image URLs with common image extensions +static IMAGE_RE: LazyLock = LazyLock::new(|| { + Regex::new( + r#"(?i)\bhttps?://[^\s<>"']+\.(?:jpg|jpeg|png|gif|bmp|webp|svg|ico)(?:\?[^\s<>"']*)?\b"#, + ) + .unwrap() +}); + +/// Extracts media URLs from a string and returns the URLs and the cleaned text +struct MediaExtractor; impl MediaExtractor { - /// Creates a new MediaExtractor with compiled regex patterns - fn new() -> Self { - MediaExtractor { - // Match common image extensions - image_regex: Regex::new( - r#"(?i)\bhttps?://[^\s<>"']+\.(?:jpg|jpeg|png|gif|bmp|webp|svg|ico)(?:\?[^\s<>"']*)?\b"#, - ).unwrap(), - } - } - /// Extracts all media URLs from a string - fn extract_media_urls(&self, text: &str) -> Vec { - let mut urls = Vec::new(); - - // Extract image URLs - for capture in self.image_regex.find_iter(text) { - urls.push(capture.as_str().to_string().into()); - } - - urls + fn extract_media_urls(text: &str) -> Vec { + IMAGE_RE + .find_iter(text) + .map(|capture| capture.as_str().to_string().into()) + .collect() } /// Removes all media URLs from a string and returns the cleaned text - fn remove_media_urls(&self, text: &str) -> String { - let mut result = text.to_string(); + fn remove_media_urls(text: &str) -> String { + let mut cleaned = String::with_capacity(text.len()); + let mut last_end = 0; + // Whitespace bordered a removed URL, so one space stands in for it + let mut spaced = false; - // Remove image URLs - result = self.image_regex.replace_all(&result, "").to_string(); + for url in IMAGE_RE.find_iter(text) { + let raw_gap = &text[last_end..url.start()]; + let surrounded = raw_gap.ends_with(char::is_whitespace) + || text[url.end()..].starts_with(char::is_whitespace); - // Clean up extra whitespace that might result from removal - self.cleanup_text(&result) + let gap = if last_end == 0 { + raw_gap.trim_end() + } else { + raw_gap.trim() + }; + + if !gap.is_empty() { + if spaced && !cleaned.is_empty() { + cleaned.push(' '); + } + cleaned.push_str(gap); + } + + spaced |= surrounded; + last_end = url.end(); + } + + let tail = text[last_end..].trim_start(); + if spaced && !cleaned.is_empty() && !tail.is_empty() { + cleaned.push(' '); + } + cleaned.push_str(tail); + + cleaned.trim().to_string() } /// Extracts media URLs and removes them from the string, returning both - fn extract_and_remove(&self, text: &str) -> (Vec, String) { - let urls = self.extract_media_urls(text); - let cleaned_text = self.remove_media_urls(text); - (urls, cleaned_text) - } - - /// Helper function to clean up text after URL removal - fn cleanup_text(&self, text: &str) -> String { - let text = text.trim(); - - // Remove multiple consecutive spaces - let re = Regex::new(r"\s+").unwrap(); - re.replace_all(text, " ").trim().to_string() + fn extract_and_remove(text: &str) -> (Vec, String) { + ( + Self::extract_media_urls(text), + Self::remove_media_urls(text), + ) } } /// Convenience function for one-time extraction and removal pub fn extract_and_remove_media_urls(text: &str) -> (Vec, String) { - let extractor = MediaExtractor::new(); - extractor.extract_and_remove(text) + MediaExtractor::extract_and_remove(text) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn multi_line_text_without_urls_keeps_line_breaks() { + let text = "first line\n\nsecond line"; + let (urls, cleaned) = extract_and_remove_media_urls(text); + + assert!(urls.is_empty()); + assert_eq!(cleaned, text); + } + + #[test] + fn removes_url_and_tidies_around_it() { + let (urls, cleaned) = + extract_and_remove_media_urls("check https://example.com/pic.png here"); + + assert_eq!(urls, vec![SharedUri::from("https://example.com/pic.png")]); + assert_eq!(cleaned, "check here"); + } + + #[test] + fn removing_url_keeps_other_line_breaks() { + let (urls, cleaned) = extract_and_remove_media_urls( + "first\nsecond https://example.com/pic.png third\nfourth", + ); + + assert_eq!(urls.len(), 1); + assert_eq!(cleaned, "first\nsecond third\nfourth"); + } + + #[test] + fn url_on_its_own_line_joins_neighbors_with_one_space() { + let (urls, cleaned) = + extract_and_remove_media_urls("first\n\nhttps://example.com/pic.png\n\nlast"); + + assert_eq!(urls.len(), 1); + assert_eq!(cleaned, "first last"); + } + + #[test] + fn multiple_urls_collapse_to_single_spaces() { + let (urls, cleaned) = extract_and_remove_media_urls( + "a https://example.com/a.png b https://example.com/b.png c", + ); + + assert_eq!(urls.len(), 2); + assert_eq!(cleaned, "a b c"); + } + + #[test] + fn message_with_only_a_url_becomes_empty() { + let (urls, cleaned) = extract_and_remove_media_urls("https://example.com/pic.png"); + + assert_eq!(urls.len(), 1); + assert_eq!(cleaned, ""); + } + + #[test] + fn digits_glued_to_url_scheme_prevent_match() { + let text = "v1.2https://example.com/pic.png"; + let (urls, cleaned) = extract_and_remove_media_urls(text); + + assert!(urls.is_empty()); + assert_eq!(cleaned, text); + } + + #[test] + fn digits_around_removed_url_survive() { + let (urls, cleaned) = extract_and_remove_media_urls("123 https://example.com/pic.png 456"); + + assert_eq!(urls.len(), 1); + assert_eq!(cleaned, "123 456"); + } + + #[test] + fn emoji_around_removed_url_survives() { + let (urls, cleaned) = extract_and_remove_media_urls("👍 https://example.com/pic.png 👍"); + + assert_eq!(urls.len(), 1); + assert_eq!(cleaned, "👍 👍"); + } } diff --git a/crates/nip29/src/group.rs b/crates/nip29/src/group.rs index 23589d09..e5927852 100644 --- a/crates/nip29/src/group.rs +++ b/crates/nip29/src/group.rs @@ -13,7 +13,7 @@ use crate::LOCAL_KEYS; use crate::protocol::{ ACTIVITY_KINDS, Activity, GroupAdmins, GroupId, GroupKey, GroupMembers, GroupMetadata, GroupPins, GroupRoles, MEMBERSHIP_KINDS, Membership, PIN_LIST, Pin, RENDER_KINDS, STATE_KINDS, - TimelineWindow, tag_value, + TimelineWindow, is_newer_event, tag_value, }; const REPLAY_LIMIT: usize = 100; @@ -91,7 +91,8 @@ pub struct Group { membership: Membership, membership_events: Vec, elsewhere: Option, - state_ids: HashMap, + /// The last accepted (created_at, id) per state kind. + state_ids: HashMap, rows: Vec, index: HashSet, reactions: HashMap>, @@ -694,6 +695,15 @@ impl Group { } fn absorb_state(&mut self, event: &Event, me: PublicKey) -> bool { + // Relays deliver same-kind state events in any order. + if !is_newer_event( + self.state_ids.get(&event.kind).copied(), + event.created_at, + event.id, + ) { + return false; + } + match event.kind { Kind::GroupMetadata => match GroupMetadata::parse(event) { Ok(metadata) => self.metadata = Some(metadata), @@ -724,7 +734,10 @@ impl Group { _ => return false, } - self.state_ids.insert(event.kind, event.id) != Some(event.id) + self.state_ids + .insert(event.kind, (event.created_at, event.id)); + + true } fn adopt_roster_membership(&mut self, me: PublicKey) -> bool { diff --git a/crates/nip29/src/lib.rs b/crates/nip29/src/lib.rs index c1efc3e9..70cc5464 100644 --- a/crates/nip29/src/lib.rs +++ b/crates/nip29/src/lib.rs @@ -76,6 +76,8 @@ pub struct GroupsRegistry { groups: Vec>, previews: HashMap>, list: GroupList, + /// The (created_at, id) of the last accepted group list event. + list_stamp: Option<(Timestamp, EventId)>, browse: HashMap, routes: HashMap, observers: HashMap, @@ -132,6 +134,7 @@ impl GroupsRegistry { groups: Vec::new(), previews: HashMap::new(), list: GroupList::default(), + list_stamp: None, browse: HashMap::new(), routes: HashMap::new(), observers: HashMap::new(), @@ -455,6 +458,8 @@ impl GroupsRegistry { self.fork_authors.clear(); self.syncing.clear(); self.list = GroupList::default(); + // A new signer's list events must not be judged against the old stamp. + self.list_stamp = None; let nostr = NostrRegistry::global(cx); let client = nostr.read(cx).client(); @@ -488,24 +493,26 @@ impl GroupsRegistry { let signer = nostr.read(cx).signer(); // Show the cached groups before the relays answer. - let task: Task> = cx.background_spawn(async move { - let me = signer.get_public_key_async().await?; - let filter = Filter::new().author(me).kind(Kind::SimpleGroups).limit(1); + let task: Task), Error>> = + cx.background_spawn(async move { + let me = signer.get_public_key_async().await?; + let filter = Filter::new().author(me).kind(Kind::SimpleGroups).limit(1); - match client.database().query(filter).await?.into_iter().next() { - Some(event) => { - let list = GroupList::parse(&event)?; - Ok(Self::hydrate_list(list, &signer, &me).await) + match client.database().query(filter).await?.into_iter().next() { + Some(event) => { + let list = + Self::hydrate_list(GroupList::parse(&event)?, &signer, &me).await; + Ok((list, Some(event))) + } + None => Ok((GroupList::default(), None)), } - None => Ok(GroupList::default()), - } - }); + }); self.tasks .push(cx.spawn(async move |this, cx| match task.await { - Ok(list) => { + Ok((list, event)) => { this.update(cx, |this, cx| { - this.merge(list, cx); + this.merge(list, event.as_ref(), cx); }) .ok(); } @@ -538,7 +545,16 @@ impl GroupsRegistry { self.watch_forks(cx); } - fn merge(&mut self, list: GroupList, cx: &mut Context) { + fn merge(&mut self, list: GroupList, event: Option<&Event>, cx: &mut Context) { + if let Some(event) = event { + // Relays replay list events out of order; only strictly newer applies. + if !is_newer_event(self.list_stamp, event.created_at, event.id) { + return; + } + + self.list_stamp = Some((event.created_at, event.id)); + } + // Carry the event's private section so a later publish can preserve it. self.list .set_content(list.content().map(str::to_owned), list.content_readable()); @@ -589,20 +605,21 @@ impl GroupsRegistry { }; if list.content().is_none() { - self.merge(list, cx); + self.merge(list, Some(&event), cx); return; } let nostr = NostrRegistry::global(cx); let signer = nostr.read(cx).signer(); let Some(me) = nostr.read(cx).current_user() else { - self.merge(list, cx); + self.merge(list, Some(&event), cx); return; }; self.tasks.push(cx.spawn(async move |this, cx| { let list = Self::hydrate_list(list, &signer, &me).await; - this.update(cx, |this, cx| this.merge(list, cx)).ok(); + this.update(cx, |this, cx| this.merge(list, Some(&event), cx)) + .ok(); })); } @@ -864,7 +881,7 @@ impl GroupsRegistry { let me = nostr.read(cx).current_user(); let list = self.list.clone(); - self.tasks.push(cx.spawn(async move |_this, _cx| { + self.tasks.push(cx.spawn(async move |this, cx| { // Re-encrypt the private section when it carries local entries; a // section this session could not read is carried verbatim. let content = match (list.has_private(), list.content_readable()) { @@ -893,6 +910,15 @@ impl GroupsRegistry { } }; + // Our own publish is the newest list until a newer event arrives. + if let Err(error) = this.update(cx, |this, _cx| { + if is_newer_event(this.list_stamp, event.created_at, event.id) { + this.list_stamp = Some((event.created_at, event.id)); + } + }) { + log::warn!("recording the published list failed: {error}"); + } + match client.send_event(&event).to_nip65().await { Ok(output) if output.failed.is_empty() => {} Ok(output) => { diff --git a/crates/nip29/src/protocol.rs b/crates/nip29/src/protocol.rs index 0d680229..26502327 100644 --- a/crates/nip29/src/protocol.rs +++ b/crates/nip29/src/protocol.rs @@ -50,6 +50,15 @@ pub(crate) fn tag_value<'a>(tags: &'a Tags, name: &str) -> Option<&'a str> { .and_then(|tag| tag.content()) } +/// Whether `(created_at, id)` is strictly newer than the last accepted stamp. +pub(crate) fn is_newer_event( + accepted: Option<(Timestamp, EventId)>, + created_at: Timestamp, + id: EventId, +) -> bool { + accepted.is_none_or(|last| (created_at, id) > last) +} + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct GroupId(String); @@ -1152,4 +1161,19 @@ mod tests { let malformed = Tag::custom("previous", ["nothex"]); assert_eq!(window.references_seen(Some(&malformed)), Some(false)); } + + #[test] + fn is_newer_event_accepts_only_strictly_newer_events() { + let lower = EventId::from_byte_array([0x00; 32]); + let higher = EventId::from_byte_array([0xff; 32]); + let earlier = Timestamp::from(100); + let later = Timestamp::from(200); + + assert!(is_newer_event(None, earlier, lower)); + assert!(!is_newer_event(Some((earlier, lower)), earlier, lower)); + assert!(is_newer_event(Some((earlier, lower)), later, lower)); + assert!(!is_newer_event(Some((later, lower)), earlier, higher)); + assert!(is_newer_event(Some((earlier, lower)), earlier, higher)); + assert!(!is_newer_event(Some((earlier, higher)), earlier, lower)); + } } diff --git a/crates/nip60/src/lib.rs b/crates/nip60/src/lib.rs index 4e2473ce..4e032cf7 100644 --- a/crates/nip60/src/lib.rs +++ b/crates/nip60/src/lib.rs @@ -55,6 +55,9 @@ pub struct HistoryEntry { pub created_at: Timestamp, } +/// How many times a failed nutzap redemption is retried before giving up +const MAX_REDEEM_ATTEMPTS: u32 = 3; + pub struct WalletRegistry { wallet: Option, tokens: HashMap, @@ -69,6 +72,10 @@ pub struct WalletRegistry { pending_nutzaps: Vec, /// Nutzaps with a redemption job in flight redeeming: HashSet, + /// Redemption attempts per nutzap, reset on success + redeem_attempts: HashMap, + /// A job errored after possibly mutating the store + publish_failed: bool, store: Option>, wallets: HashMap, loading: bool, @@ -119,6 +126,8 @@ impl WalletRegistry { redeemed: HashSet::new(), pending_nutzaps: Vec::new(), redeeming: HashSet::new(), + redeem_attempts: HashMap::new(), + publish_failed: false, store: None, wallets: HashMap::new(), loading: false, @@ -378,6 +387,8 @@ impl WalletRegistry { self.redeemed.clear(); self.pending_nutzaps.clear(); self.redeeming.clear(); + self.redeem_attempts.clear(); + self.publish_failed = false; self.store = None; self.wallets.clear(); self.loading = false; @@ -397,6 +408,7 @@ impl WalletRegistry { let signer = nostr.read(cx).signer().clone(); let (tx, rx) = flume::bounded::(100); + let consumer_signer = signer.clone(); self.notification_listener = Some(cx.background_spawn(async move { const MAX_PROCESSED: usize = 1000; @@ -417,7 +429,7 @@ impl WalletRegistry { | Kind::CashuWalletSpendingHistory ); - let wallet_event = ours && is_cashu || is_token_deletion(&event); + let wallet_event = ours && (is_cashu || is_token_deletion(&event)); let nutzap = event.kind == Kind::Custom(9321) && me.is_some_and(|me| event.tags.public_keys().any(|key| key == me)); @@ -443,21 +455,22 @@ impl WalletRegistry { })); self.tasks.push(cx.spawn(async move |this, cx| { + let me = consumer_signer.get_public_key_async().await.ok(); while let Ok(event) = rx.recv_async().await { this.update(cx, |this, cx| { - this.absorb(event, cx); + this.absorb(event, me, cx); }) .ok(); } })); } - fn absorb(&mut self, event: Event, cx: &mut Context) { + fn absorb(&mut self, event: Event, me: Option, cx: &mut Context) { match event.kind { Kind::CashuWallet => self.absorb_wallet(event, cx), Kind::CashuWalletUnspentProof => self.absorb_token(event, cx), Kind::CashuWalletSpendingHistory => self.absorb_history(event, cx), - Kind::EventDeletion => self.absorb_deletion(event, cx), + Kind::EventDeletion => self.absorb_deletion(event, me, cx), Kind::Custom(9321) => self.absorb_nutzap(event, cx), _ => {} } @@ -549,7 +562,15 @@ impl WalletRegistry { })); } - fn absorb_deletion(&mut self, event: Event, cx: &mut Context) { + fn absorb_deletion(&mut self, event: Event, me: Option, cx: &mut Context) { + if me != Some(event.pubkey) { + log::warn!( + "ignoring kind 5 deletion {} from a foreign author", + event.id + ); + return; + } + let mut removed_any = false; for tag in event.tags.iter() { if tag.kind() != "e" { @@ -807,6 +828,9 @@ impl WalletRegistry { self.job_running = true; + let job_handle = job.clone(); + let publish_failed = self.publish_failed; + self.tasks.push(cx.spawn(async move |this, cx| { let result = Tokio::spawn_result(cx, async move { let ctx = ops::JobCtx { @@ -815,6 +839,7 @@ impl WalletRegistry { signer, client, tokens, + publish_failed, }; job.run(ctx).await @@ -823,7 +848,7 @@ impl WalletRegistry { this.update(cx, |this, cx| { this.job_running = false; - this.apply_outcome(result, cx); + this.apply_outcome(job_handle, result, cx); for job in std::mem::take(&mut this.job_queued) { this.schedule_job(job, cx); @@ -833,10 +858,38 @@ impl WalletRegistry { })); } - fn apply_outcome(&mut self, result: Result, cx: &mut Context) { + fn apply_outcome( + &mut self, + job: ops::Job, + result: Result, + cx: &mut Context, + ) { let outcome = match result { Ok(outcome) => outcome, Err(error) => { + if !matches!(job, ops::Job::CreateQuote { .. }) { + self.publish_failed = true; + } + + if !matches!(job, ops::Job::CreateQuote { .. } | ops::Job::Reconcile) { + self.schedule_job(ops::Job::Reconcile, cx); + } + + if let ops::Job::RedeemNutzap { nutzap_id, .. } = &job { + self.redeeming.remove(nutzap_id); + + let attempts = self.redeem_attempts.entry(*nutzap_id).or_default(); + *attempts += 1; + + if *attempts <= MAX_REDEEM_ATTEMPTS { + self.job_queued.push(job.clone()); + } else { + log::warn!( + "giving up redeeming nutzap {nutzap_id} after {MAX_REDEEM_ATTEMPTS} attempts" + ); + } + } + self.emit_error(format!("{error:#}"), cx); return; } @@ -845,6 +898,7 @@ impl WalletRegistry { match outcome { ops::Outcome::Idle => {} ops::Outcome::Reconciled { updates } => { + self.publish_failed = false; let changed = !updates.is_empty(); for update in updates { @@ -905,6 +959,7 @@ impl WalletRegistry { amount, } => { self.redeeming.remove(&nutzap_id); + self.redeem_attempts.remove(&nutzap_id); self.redeemed.insert(nutzap_id); self.apply_token_update(update); self.emit_updated(cx); diff --git a/crates/nip60/src/ops.rs b/crates/nip60/src/ops.rs index 722c37e4..e347c636 100644 --- a/crates/nip60/src/ops.rs +++ b/crates/nip60/src/ops.rs @@ -361,6 +361,8 @@ pub(crate) struct JobCtx { pub(crate) client: Client, /// Relay token events at schedule time. pub(crate) tokens: HashMap, + /// A previous job errored after possibly mutating the store + pub(crate) publish_failed: bool, } impl JobCtx @@ -426,35 +428,14 @@ where log::warn!("saga recovery failed for {mint_url}: {error:#}"); } - let minted = wallet.mint_unissued_quotes().await?; - if minted == Amount::ZERO { - continue; - } + let local_ys = proof_ys(wallet.get_unspent_proofs().await?.iter())?; + let states = vec![State::Pending, State::Reserved, State::PendingSpent]; - let update = self - .sync_token_events( - wallet, - nostr_mint(mint_url)?, - &previous_ids(&tokens, mint_url), - Some(keys::History { - direction: TransactionDirection::In, - amount: u64::from(minted), - redeemed: Vec::new(), - sender: None, - }), - ) - .await?; + let in_flight = wallet.get_proofs_by_states(states).await?; + let in_flight_ys = proof_ys(in_flight.iter())?; - for id in &update.destroyed { - tokens.remove(id); - } - tokens.insert(update.event_id, update.token.clone()); - updates.push(update); - } - - for (mint_url, wallet) in &self.wallets { let mut relay_ys = HashSet::new(); - let mut added = Vec::new(); + let mut unseen = Vec::new(); for token in tokens .values() @@ -468,23 +449,104 @@ where continue; } }; - if relay_ys.insert(info.y) { - added.push(info); + + if !relay_ys.insert(info.y) { + continue; } + + if local_ys.contains(&info.y) || in_flight_ys.contains(&info.y) { + continue; + } + + unseen.push(info); } } - let unspent = wallet.get_unspent_proofs().await?; - let removed = unspent - .iter() - .filter(|proof| match proof.y() { - Ok(y) => !relay_ys.contains(&y), - Err(_) => false, - }) - .map(|proof| proof.y()) - .collect::, _>>()?; + if unseen.is_empty() { + continue; + } - wallet.localstore.update_proofs(added, removed).await?; + let unseen_proofs: Proofs = unseen.iter().map(|info| info.proof.clone()).collect(); + + let spent = match wallet.check_proofs_spent(unseen_proofs).await { + Ok(states) => states + .into_iter() + .filter(|state| state.state == State::Spent) + .map(|state| state.y) + .collect::>(), + Err(error) => { + log::warn!("mint state check failed, accepting unseen proofs: {error:#}"); + HashSet::new() + } + }; + + let added: Vec = unseen + .into_iter() + .filter(|info| !spent.contains(&info.y)) + .collect(); + + if !added.is_empty() { + wallet.localstore.update_proofs(added, Vec::new()).await?; + } + } + + for (mint_url, wallet) in &self.wallets { + let minted = wallet.mint_unissued_quotes().await?; + if minted != Amount::ZERO { + let update = self + .sync_token_events( + wallet, + nostr_mint(mint_url)?, + &previous_ids(&tokens, mint_url), + Some(keys::History { + direction: TransactionDirection::In, + amount: u64::from(minted), + redeemed: Vec::new(), + sender: None, + }), + ) + .await?; + + for id in &update.destroyed { + tokens.remove(id); + } + tokens.insert(update.event_id, update.token.clone()); + updates.push(update); + } + + let relay_ys = relay_proof_ys(&tokens, mint_url); + let local_ys = proof_ys(wallet.get_unspent_proofs().await?.iter())?; + + if local_ys != relay_ys { + if self.publish_failed { + let update = self + .sync_token_events( + wallet, + nostr_mint(mint_url)?, + &previous_ids(&tokens, mint_url), + None, + ) + .await?; + + for id in &update.destroyed { + tokens.remove(id); + } + tokens.insert(update.event_id, update.token.clone()); + updates.push(update); + } else { + let removed = wallet + .get_unspent_proofs() + .await? + .into_iter() + .filter(|proof| match proof.y() { + Ok(y) => !relay_ys.contains(&y), + Err(_) => false, + }) + .map(|proof| proof.y()) + .collect::, _>>()?; + wallet.localstore.update_proofs(Vec::new(), removed).await?; + } + } } Ok(Outcome::Reconciled { updates }) @@ -499,6 +561,34 @@ fn previous_ids(tokens: &HashMap, mint: &MintUrl) -> Vec( + proofs: impl Iterator, +) -> Result, Error> { + proofs.map(|proof| proof.y().map_err(Error::from)).collect() +} + +/// Proof ys referenced by the relay token events of one mint +fn relay_proof_ys( + tokens: &HashMap, + mint: &MintUrl, +) -> HashSet { + let mut ys = HashSet::new(); + for token in tokens + .values() + .filter(|token| token.mint.as_str() == mint.to_string()) + { + for proof in &token.proofs { + match proof_info(proof, mint) { + Ok(info) => { + ys.insert(info.y); + } + Err(error) => log::warn!("skipping proof: {error:#}"), + } + } + } + ys +} + /// Resolve a recipient's 10019 wallet info and pick a mint we share with them. async fn recipient_wallet_info( ctx: &JobCtx, diff --git a/crates/settings/src/lib.rs b/crates/settings/src/lib.rs index a850bc7d..04ade7bb 100644 --- a/crates/settings/src/lib.rs +++ b/crates/settings/src/lib.rs @@ -223,6 +223,11 @@ impl Global for GlobalAppSettings {} pub struct AppSettings { /// Settings inner: Entity, + /// The pending save task, so writes to the settings file complete in order + #[cfg(not(target_arch = "wasm32"))] + save_task: Option>, + /// Set after a failed load so its own save cannot overwrite the file with defaults + suppress_next_save: bool, /// Event subscriptions _subscriptions: SmallVec<[Subscription; 2]>, } @@ -315,11 +320,16 @@ impl AppSettings { // Run at the end of current cycle cx.defer(move |cx| { - entity.update(cx, |this, cx| this.load(cx)).ok(); + if let Err(error) = entity.update(cx, |this, cx| this.load(cx)) { + log::error!("failed to load settings: {error:#}"); + } }); Self { inner, + #[cfg(not(target_arch = "wasm32"))] + save_task: None, + suppress_next_save: false, _subscriptions: subscriptions, } } @@ -381,27 +391,46 @@ impl AppSettings { /// Load settings fn load(&mut self, cx: &mut Context) { - let task: Task> = cx.background_spawn(async move { + let task: Task, Error>> = cx.background_spawn(async move { #[cfg(not(target_arch = "wasm32"))] { let path = config_dir().join(".settings"); - if let Ok(content) = smol::fs::read_to_string(&path).await { - return Ok(serde_json::from_str(&content)?); + match smol::fs::read_to_string(&path).await { + Ok(content) => { + let settings: Settings = + serde_json::from_str(&content).map_err(|error| { + anyhow!("failed to parse {}: {error}", path.display()) + })?; + return Ok(Some(settings)); + } + // No settings file yet on a fresh install, so keep the defaults + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(anyhow!("failed to read {}: {error}", path.display())); + } } } #[cfg(target_arch = "wasm32")] { if let Some(content) = common::web_store::get(SETTINGS_KEY)? { - return Ok(serde_json::from_str(&content)?); + let settings: Settings = serde_json::from_str(&content) + .map_err(|error| anyhow!("failed to parse saved settings: {error}"))?; + return Ok(Some(settings)); } } - Err(anyhow!("Not found")) + Ok(None) }); cx.spawn(async move |this, cx| { - let mut settings = task.await.unwrap_or(Settings::default()); + let (mut settings, load_failed) = match task.await { + Ok(settings) => (settings.unwrap_or_default(), false), + Err(error) => { + log::error!("failed to load settings, keeping defaults: {error:#}"); + (Settings::default(), true) + } + }; // Move settings still pointed at the old default file server over to the new one if settings.file_server.host_str() == Some(LEGACY_FILE_SERVER) { @@ -409,31 +438,63 @@ impl AppSettings { } // Update settings - this.update(cx, |this, cx| { + if let Err(error) = this.update(cx, |this, cx| { + // A failed load must not trigger the save that would overwrite the file + this.suppress_next_save = load_failed; this.set_settings(settings, cx); this.apply_theme(cx); cx.refresh_windows(); - }) - .ok(); + }) { + log::error!("failed to apply loaded settings: {error:#}"); + } }) .detach(); } /// Save settings pub fn save(&mut self, cx: &mut Context) { - let settings = self.inner.read(cx); - if let Ok(content) = serde_json::to_string(&settings) { - #[cfg(not(target_arch = "wasm32"))] - cx.background_spawn(async move { - let path = config_dir().join(".settings"); - smol::fs::write(&path, content).await.ok(); - }) - .detach(); + // Consume the failed-load suppression, so the next user change saves again + if self.suppress_next_save { + self.suppress_next_save = false; + return; + } - #[cfg(target_arch = "wasm32")] - if let Err(e) = common::web_store::set(SETTINGS_KEY, &content) { - log::warn!("failed to persist settings: {e}"); + let settings = self.inner.read(cx); + let content = match serde_json::to_string(settings) { + Ok(content) => content, + Err(error) => { + log::error!("failed to serialize settings: {error:#}"); + return; } + }; + + #[cfg(not(target_arch = "wasm32"))] + { + let previous_task = self.save_task.take(); + self.save_task = Some(cx.background_spawn(async move { + // Await the previous write, so saves to the file complete in order + if let Some(previous_task) = previous_task { + previous_task.await; + } + + let path = config_dir().join(".settings"); + let temporary_path = config_dir().join(".settings.tmp"); + + let result: std::io::Result<()> = async { + smol::fs::write(&temporary_path, &content).await?; + smol::fs::rename(&temporary_path, &path).await + } + .await; + + if let Err(error) = result { + log::error!("failed to save settings to {}: {error}", path.display()); + } + })); + } + + #[cfg(target_arch = "wasm32")] + if let Err(error) = common::web_store::set(SETTINGS_KEY, &content) { + log::error!("failed to persist settings: {error:#}"); } } diff --git a/crates/speech/src/parakeet.rs b/crates/speech/src/parakeet.rs index 6c19229b..fe7c38fa 100644 --- a/crates/speech/src/parakeet.rs +++ b/crates/speech/src/parakeet.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -use std::sync::OnceLock; +use std::sync::{Mutex, OnceLock}; use std::time::Duration; use anyhow::{Context as _, Result, anyhow}; @@ -30,7 +30,7 @@ impl SpeechRecognizer for ParakeetRecognizer { cx: &mut App, ) -> Result, SpeechError> { if super::model::installed() { - worker().send_or_log(Command::Warm(super::model::dir())); + send(Command::Warm(super::model::dir())).map_err(SpeechError::recognizer)?; } else if super::model::status().is_downloading() { return Err(SpeechError::recognizer(anyhow!( "The speech model is still downloading. Try again when it finishes." @@ -79,7 +79,11 @@ impl RecognitionSession for ParakeetSession { } let (sender, receiver) = flume::unbounded(); - worker().send_or_log(Command::Transcribe { samples, sender }); + + if let Err(error) = send(Command::Transcribe { samples, sender }) { + sink.error(SpeechError::recognizer(error), cx); + return; + } self._result = Some(cx.spawn(async move |cx| { let result = receiver.recv_async().await; @@ -109,23 +113,50 @@ enum Command { Unload, } -/// A handle to the single inference worker. -fn worker() -> &'static flume::Sender { - static WORKER: OnceLock> = OnceLock::new(); +fn worker_slot() -> &'static Mutex>> { + static WORKER: OnceLock>>> = OnceLock::new(); + WORKER.get_or_init(|| Mutex::new(None)) +} - WORKER.get_or_init(|| { - let (sender, receiver) = flume::unbounded::(); +/// A handle to the live worker, spawning a replacement after an idle exit. +fn worker() -> Result> { + let mut slot = worker_slot() + .lock() + .unwrap_or_else(|error| error.into_inner()); - let spawned = std::thread::Builder::new() - .name("speech-parakeet".into()) - .spawn(move || run_worker(receiver)); + if slot.as_ref().is_some_and(|sender| sender.is_disconnected()) { + *slot = None; + } - if let Err(error) = spawned { - log::error!("failed to spawn the speech worker: {error}"); + if let Some(sender) = slot.as_ref() { + return Ok(sender.clone()); + } + + let (sender, receiver) = flume::unbounded::(); + + match std::thread::Builder::new() + .name("speech-parakeet".into()) + .spawn(move || run_worker(receiver)) + { + Ok(_) => { + *slot = Some(sender.clone()); + Ok(sender) } + Err(error) => { + log::error!("failed to spawn the speech worker: {error}"); + Err(anyhow!("Could not start the speech worker: {error}")) + } + } +} - sender - }) +/// Deliver a command, retrying once on a fresh worker if the previous one exited. +fn send(command: Command) -> Result<()> { + let command = match worker()?.send(command) { + Ok(()) => return Ok(()), + Err(error) => error.into_inner(), + }; + // The worker can exit between the liveness check and the send, so retry once. + worker()?.send(command).context("the speech worker exited") } fn run_worker(receiver: flume::Receiver) { @@ -192,21 +223,11 @@ fn transcribe(cached: &mut Option<(PathBuf, ParakeetTDT)>, samples: Vec) -> .map_err(|error| anyhow!("Could not transcribe this recording: {error}")) } -trait SendOrLog { - fn send_or_log(&self, command: Command); -} - -impl SendOrLog for flume::Sender { - fn send_or_log(&self, command: Command) { - if self.send(command).is_err() { - log::debug!("speech worker is gone; command dropped"); - } - } -} - /// Retire cached weights, e.g. after the model was deleted. pub(crate) fn unload() { - worker().send_or_log(Command::Unload); + if let Err(error) = send(Command::Unload) { + log::warn!("failed to unload the speech model: {error}"); + } } fn flatten_panic(result: std::thread::Result) -> Result { diff --git a/crates/state/Cargo.toml b/crates/state/Cargo.toml index 116a721d..af27e9d0 100644 --- a/crates/state/Cargo.toml +++ b/crates/state/Cargo.toml @@ -51,6 +51,7 @@ sqlite-wasm-vfs = "0.2" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] browser-signer-proxy = { path = "../browser-signer-proxy" } nostr-lmdb.workspace = true +nostr-memory.workspace = true smol.workspace = true gpui_tokio.workspace = true rustls = "0.23" diff --git a/crates/state/src/file.rs b/crates/state/src/file.rs index 3f7f6e87..2aba636e 100644 --- a/crates/state/src/file.rs +++ b/crates/state/src/file.rs @@ -403,7 +403,7 @@ pub async fn download_and_decrypt_file( .clone() .unwrap_or_else(|| fallback_name(&file.url)); - let local = store_file(&name, &file.mime, &data).await?; + let local = store_file(&file.mime, &data).await?; Ok(DownloadedFile { local, @@ -415,10 +415,11 @@ pub async fn download_and_decrypt_file( } /// Persist decrypted bytes somewhere the platform can render or open them -async fn store_file(name: &str, mime: &str, data: &[u8]) -> Result { +async fn store_file(mime: &str, data: &[u8]) -> Result { #[cfg(not(target_arch = "wasm32"))] { - let digest = sha256_hex(name.as_bytes()); + // Key the cache entry on the content hash, so same-named attachments can't collide + let digest = sha256_hex(data); let extension = mime_guess::get_mime_extensions_str(mime) .and_then(|extensions| extensions.first()) .copied() @@ -428,24 +429,21 @@ async fn store_file(name: &str, mime: &str, data: &[u8]) -> Result (Arc::new(database) as Arc, None), + Err(e) => { + log::error!("Failed to open the local database, falling back to memory: {e}"); + + ( + Arc::new(MemoryDatabase::unbounded()) as Arc, + Some(format!( + "Local database unavailable, using an in-memory database instead: {e}" + )), + ) + } + }; #[cfg(target_arch = "wasm32")] - let database = web_database::take(); + let (database, database_error) = (web_database::take(), None::); // Construct the nostr client let client = ClientBuilder::default() @@ -141,6 +155,10 @@ impl NostrRegistry { // Connect to bootstrap relays once the registry has been returned to the app cx.defer(move |cx| { let _ = entity.update(cx, |this, cx| { + if let Some(error) = database_error { + cx.emit(StateEvent::error(error)); + } + this.connect_bootstrap_relays(cx); if let Some(secret) = cli_key { @@ -445,8 +463,14 @@ impl NostrRegistry { }; let keys = Keys::generate(); - let credential = with_master_key(&uri_string, &keys); - let write = credentials::write(cx, "bunker", credential.as_bytes()); + + // The URI embeds the client key, so never persist it where web stores it unencrypted + #[cfg(not(target_arch = "wasm32"))] + let write = + credentials::write(cx, "bunker", with_master_key(&uri_string, &keys).as_bytes()); + + #[cfg(target_arch = "wasm32")] + let write = Task::ready(Ok::<(), Error>(())); cx.spawn(async move |this, cx| { let signer = NostrConnect::builder(connect_uri, keys) @@ -709,6 +733,7 @@ fn extract_master_key(credential: &str) -> (&str, Keys) { } // Embed the master key in a bunker URI so reconnects reuse the same local key +#[cfg(not(target_arch = "wasm32"))] fn with_master_key(uri: &str, keys: &Keys) -> String { let separator = if uri.contains('?') { '&' } else { '?' }; let nsec = keys.secret_key().to_bech32().expect("infallible"); diff --git a/crates/workspace/src/panels/contact_list.rs b/crates/workspace/src/panels/contact_list.rs index 059eb876..1e6a3c57 100644 --- a/crates/workspace/src/panels/contact_list.rs +++ b/crates/workspace/src/panels/contact_list.rs @@ -29,26 +29,17 @@ pub fn init(window: &mut Window, cx: &mut App) -> Entity { pub struct ContactListPanel { name: SharedString, focus_handle: FocusHandle, - /// Npub input input: Entity, - /// Whether the panel is updating updating: bool, - /// Error message error: Option, - /// All contacts - contacts: HashSet, - + contacts: HashSet, /// Person directory for the contacts directory: Entity, - - /// Event subscriptions _subscriptions: SmallVec<[Subscription; 1]>, - - /// Background tasks tasks: Vec>>, } @@ -92,33 +83,50 @@ impl ContactListPanel { return; }; - let task: Task, Error>> = cx.background_spawn(async move { + let task: Task, Error>> = cx.background_spawn(async move { let filter = Filter::new() .author(public_key) .kind(Kind::ContactList) .limit(1); - let contact_list: HashSet = client + let contact_list: HashSet = client .database() .query(filter) .await? .into_iter() .next() - .map(|event| event.tags.public_keys().collect()) + .map(|event| { + event + .tags + .iter() + .filter_map(|tag| match Nip02Tag::try_from(tag) { + Ok(Nip02Tag::PublicKey { + public_key, + relay_hint, + alias, + }) => Some(Contact { + public_key, + relay_url: relay_hint, + alias, + }), + Err(_) => None, + }) + .collect() + }) .unwrap_or_default(); Ok(contact_list) }); self.tasks.push(cx.spawn_in(window, async move |this, cx| { - let public_keys = task.await?; + let contacts = task.await?; // Update state this.update(cx, |this, cx| { this.directory.update(cx, |directory, cx| { - directory.track(public_keys.iter().copied(), cx) + directory.track(contacts.iter().map(|contact| contact.public_key), cx) }); - this.contacts.extend(public_keys); + this.contacts.extend(contacts); cx.notify(); })?; @@ -130,7 +138,7 @@ impl ContactListPanel { let value = self.input.read(cx).value().to_string(); if let Ok(public_key) = PublicKey::parse(&value) { - if self.contacts.insert(public_key) { + if self.contacts.insert(Contact::new(public_key)) { self.directory .update(cx, |directory, cx| directory.track([public_key], cx)); self.input.update(cx, |this, cx| { @@ -144,7 +152,8 @@ impl ContactListPanel { } fn remove(&mut self, public_key: &PublicKey, cx: &mut Context) { - self.contacts.remove(public_key); + self.contacts + .retain(|contact| contact.public_key != *public_key); cx.notify(); } @@ -184,11 +193,7 @@ impl ContactListPanel { let signer = nostr.read(cx).signer(); // Get contacts - let contacts: Vec = self - .contacts - .iter() - .map(|public_key| Contact::new(*public_key)) - .collect(); + let contacts: Vec = self.contacts.iter().cloned().collect(); // Set updating state self.set_updating(true, cx); @@ -230,8 +235,9 @@ impl ContactListPanel { fn render_list_items(&mut self, cx: &mut Context) -> Vec { let mut items = Vec::new(); - for (ix, public_key) in self.contacts.iter().enumerate() { - let profile = self.directory.read(cx).person(public_key, cx); + for (ix, contact) in self.contacts.iter().enumerate() { + let public_key = contact.public_key; + let profile = self.directory.read(cx).person(&public_key, cx); items.push( h_flex() @@ -262,12 +268,9 @@ impl ContactListPanel { .ghost() .invisible() .group_hover("", |this| this.visible()) - .on_click({ - let public_key = public_key.to_owned(); - cx.listener(move |this, _ev, _window, cx| { - this.remove(&public_key, cx); - }) - }), + .on_click(cx.listener(move |this, _ev, _window, cx| { + this.remove(&public_key, cx); + })), ), ) } diff --git a/docs/audit-stable-release.md b/docs/audit-stable-release.md index 826ca3db..bd59ffec 100644 --- a/docs/audit-stable-release.md +++ b/docs/audit-stable-release.md @@ -2,6 +2,10 @@ Date: 2026-11-12 · Revision audited: `8fa79aef` (master) · Scope: entire workspace (~35k lines, 18 crates + `desktop` + `web`) +> **Remediation status (2026-11-12, later the same day):** all ten P0 items from §8 have been +> implemented and validated (workspace clippy clean, `cargo fmt --check` clean, 35 tests passing). +> See §9 for the per-fix log. + ## 1. Scope and Method Every crate was audited file-by-file against the project conventions (`AGENTS.md`), Rust best @@ -823,3 +827,81 @@ claims were verified against the vendored `gpui-pre 0.3.12`, `gpui-base 0.9.1`, `gpui-component 0.9.1`, and rust-nostr sources; the Critical and top-High findings were independently re-verified by hand against the working tree at `8fa79aef`. No files were modified during the audit.* + +## 9. P0 Remediation Log (2026-11-12) + +All ten P0 items fixed. Validation after the full batch: `cargo clippy --workspace --all-targets` +zero warnings, `cargo fmt --all --check` clean, `cargo test --workspace` 35 passed / 0 failed. +Diff: 15 files, +816/−273. + +1. **nip60 deletion spoofing** — `nip60/src/lib.rs`: precedence fixed to + `ours && (is_cashu || is_token_deletion(..))`; `absorb`/`absorb_deletion` now take `me` and + reject foreign-authored kind 5 events (defense in depth; the live notification gate was the + only protection before, and `absorb_deletion` had no author check). The DB load path was + already author-filtered and is unchanged. +2. **browser-signer-proxy hardening** — per-session UUID token generated at startup, embedded in + the served page (`inject_session_token`), required on `/api/*` (403 otherwise); exact `Host` + match (DNS-rebinding defense); all CORS headers and the OPTIONS preflight removed; + `cancel_request` removes timed-out requests from both maps so plaintext is dropped; + centralized best-effort `write_response`. `proxy.js` authenticates all three fetches. Token + never appears in the URL the app opens. +3. **state startup crash loop** — `NostrLmdb::open` failure now logs and falls back to + `MemoryDatabase::unbounded()` (nostr-memory added to the desktop deps), emitting + `StateEvent::error` via the existing deferred path; the `block_on` stays (safe: the LMDB open + schedules nothing back onto the GPUI executor — now commented). +4. **state web localStorage key** — `import_bunker` on wasm32 no longer persists the bunker + credential (session is memory-only; desktop keyring unchanged); `restore_session` degrades to + `NoSigner` when nothing is stored (verified). **Known behavior change: web users re-enter their + bunker URI after each reload.** +5. **state file-cache collision** — cache filename now derived from the decrypted content's + sha256 instead of the filename; the metadata guard is gone and every store goes through + temp-write + atomic rename. +6. **nip60 reconcile money-safety** — `ops.rs::reconcile` rewritten: relay tokens are converged + into the store BEFORE any publish (closes the `del`-before-converge race); relay proofs already + tracked locally (unspent or in-flight Pending/Reserved/PendingSpent) are never re-added, and + unseen proofs are checked against the mint (NUT-08 `check_proofs_spent`, best-effort) before + being accepted as Unspent; when a prior job errored after mutating the store + (`publish_failed` flag), reconcile republishes the store's unspent set instead of trusting + relay state — healing stranded sends/receives; without the flag the relay-trust removal path + (cross-device spend convergence) is preserved. A failing Reconcile does not reschedule itself + (no infinite loop). +7. **nip60 stuck nutzap redemptions** — `apply_outcome` now receives the failed job; on + `RedeemNutzap` error the id is removed from `redeeming` and requeued up to + `MAX_REDEEM_ATTEMPTS = 3` (cleared on success, reset on signer change), so transient mint + failures no longer wedge redemptions until restart. +8. **Reactions double-count** — `chat_ui::insert_reaction` replaces per (author, target) per + NIP-25, making the backup gift-wrap echo idempotent (verified: optimistic rumor and the + re-delivered seal carry the same event id) and history reloads non-duplicating. +9. **Whitespace collapse** — `common/media_extractor.rs` rewritten: whitespace is tidied only at + URL-removal boundaries; line breaks in untouched regions survive; regex hoisted to + `LazyLock`; 9 unit tests added (multi-line preservation, URL-only messages, digit/emoji edges). +10. **Contact-list data loss** — `contact_list.rs` now parses the stored event into full + `nostr::nips::nip02::Contact` values (relay hint + alias preserved through load → add/remove + → republish); new contacts still get `Contact::new`. +11. **Speech worker lifetime** — `parakeet.rs`: the `OnceLock` became a resettable + `Mutex>`; a dead worker (30 s idle exit) is detected via `is_disconnected`, the + slot cleared, and a replacement thread spawned lazily on the next command; spawn failure no + longer caches a dead sender; failures surface through `sink.error` / returned `Err` instead + of a debug-level swallow (one retry covers the liveness-check/send race). +12. **Settings durability** — a failed load logs and keeps defaults in memory without + persisting (`suppress_next_save`, consumed by the next real change); fresh-install NotFound is + no longer conflated with read/parse failures; saves are serialized through a single + `save_task` slot (chained, FIFO order) with temp-file + atomic rename; serialization and + write failures are logged. +13. **nip29 ordering guards** — shared `is_newer_event` (strict `(created_at, id)` total order): + `Group::absorb_state` keeps a per-kind stamp (written only after a successful apply, so a + malformed newer event can't poison the guard) and rejects stale replays while ascending cache + hydration still applies; the registry's `list_stamp` seeds from the cached list event at + startup, is checked in `merge` before applying, updated by `publish_list` (closing the + leave→resurrect window), and reset on signer change. 3 new tests across protocol/group. +14. **nip60_ui invoice flow** — *not* in this batch: it is P1 (the audit lists it as High but the + P0 list ranks it 16th via `nip60_ui/wallet.rs`); `mint_quotes()`/`claim_mint_quote()` remain + dead until the P1 UI work lands. No other P0 code paths depend on it. + +Behavior changes to be aware of when testing: +- Web: bunker credentials are no longer persisted (re-connect after each reload). +- Desktop: a corrupted/unreadable local database now starts with an empty in-memory history and + shows an error event instead of crash-looping; relays re-sync as before. +- nip60: reconcile now performs one NUT-08 mint round-trip when unseen relay proofs appear. +- Reactions render per NIP-25 replace semantics (one reaction per author per target). +- Messages keep their line breaks; only URL-adjacent whitespace is tidied. -- 2.54.0 From e9f29dc398c8e43f04a29c1fad1d9035658c88b5 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Thu, 12 Nov 2026 20:42:50 +0700 Subject: [PATCH 3/6] stabilize ids and harden release pipeline --- Cargo.lock | 2 + crates/auto_update/src/lib.rs | 21 +++ crates/browser-signer-proxy/src/lib.rs | 148 +++++++++++++++++--- crates/chat/src/lib.rs | 19 ++- crates/chat_ui/Cargo.toml | 1 + crates/chat_ui/src/lib.rs | 117 +++++++++++++--- crates/common/Cargo.toml | 1 + crates/common/src/event.rs | 122 +++++++++++++---- crates/common/src/parser.rs | 82 ++++++------ crates/nip29/src/protocol.rs | 7 +- crates/nip29/src/utils.rs | 11 +- crates/nip4e/src/lib.rs | 104 +++++++++++---- crates/nip60/src/lib.rs | 16 ++- crates/nip60_ui/src/wallet.rs | 113 +++++++++++++++- crates/person/src/lib.rs | 65 ++++++--- crates/workspace/src/dialogs/new_chat.rs | 3 + crates/workspace/src/dialogs/profile.rs | 94 ++++++++++++- crates/workspace/src/dialogs/screening.rs | 94 +++++++++---- crates/workspace/src/dock/mod.rs | 156 +++++++++++++++++----- crates/workspace/src/dock/skin.rs | 6 - crates/workspace/src/dock/tab_panel.rs | 2 +- crates/workspace/src/lib.rs | 44 +++++- crates/workspace/src/panels/backup.rs | 66 +++++++-- crates/workspace/src/panels/browse.rs | 5 +- crates/workspace/src/sidebar/header.rs | 32 +++-- crates/workspace/src/sidebar/mod.rs | 37 +++-- crates/workspace/src/sidebar/rows.rs | 21 ++- docs/audit-stable-release.md | 91 ++++++++++++- 28 files changed, 1199 insertions(+), 281 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ff5309cc..4bcfc046 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1463,6 +1463,7 @@ dependencies = [ "gpui-pre", "instant", "itertools 0.13.0", + "log", "nip60_ui", "nostr-sdk", "person", @@ -1681,6 +1682,7 @@ dependencies = [ "nostr", "nostr-sdk", "regex", + "sha2 0.10.9", "web-sys", ] diff --git a/crates/auto_update/src/lib.rs b/crates/auto_update/src/lib.rs index 3959bc6b..d3ffcc41 100644 --- a/crates/auto_update/src/lib.rs +++ b/crates/auto_update/src/lib.rs @@ -63,6 +63,18 @@ struct GlobalAutoUpdater(Entity); impl Global for GlobalAutoUpdater {} +/// The subset of updater state the UI renders, so views can hold it in a small +/// entity instead of observing the whole updater. +#[derive(Clone, PartialEq)] +pub struct UpdateDisplay { + /// Whether the running version is the newest release. + pub up_to_date: bool, + /// Whether a verified update is installed and waiting for a restart. + pub staged: bool, + /// A short, human-readable description of the current status. + pub status: SharedString, +} + pub struct AutoUpdater { updater: Entity, status: UpdateStatus, @@ -173,6 +185,15 @@ impl AutoUpdater { } } + /// Snapshot the state the UI renders. + pub fn display(&self) -> UpdateDisplay { + UpdateDisplay { + up_to_date: self.up_to_date(), + staged: self.staged(), + status: self.status(), + } + } + /// Check the release host for a newer version, then download and install it. pub fn check(&mut self, cx: &mut Context) { self.updater.update(cx, |updater, cx| { diff --git a/crates/browser-signer-proxy/src/lib.rs b/crates/browser-signer-proxy/src/lib.rs index fc63f29b..0a3afb7b 100644 --- a/crates/browser-signer-proxy/src/lib.rs +++ b/crates/browser-signer-proxy/src/lib.rs @@ -26,6 +26,13 @@ pub use self::error::Error; const DEFAULT_HTML: &str = include_str!("../index.html"); const JS: &str = include_str!("../proxy.js"); +/// Cap on the request line and headers so a peer cannot force unbounded reads +const MAX_HEADER_BYTES: usize = 16 * 1024; +/// Cap on the request body so a huge Content-Length cannot force a huge allocation +const MAX_BODY_BYTES: usize = 1024 * 1024; +/// Maximum time a connection may stay open before it is dropped +const CONNECTION_TIMEOUT: Duration = Duration::from_secs(30); + type PendingResponseMap = HashMap>>; #[derive(Debug, Deserialize)] @@ -303,12 +310,20 @@ impl BrowserSignerProxy { smol::future::or( async { - handle_connection( - stream, - state, - custom_html, - expected_host, - session_token, + smol::future::or( + async { + handle_connection( + stream, + state, + custom_html, + expected_host, + session_token, + ) + .await; + }, + async { + smol::Timer::after(CONNECTION_TIMEOUT).await; + }, ) .await; }, @@ -401,10 +416,19 @@ impl BrowserSignerProxy { #[inline] async fn _sign_event(&self, event: UnsignedEvent) -> Result { + let expected_id: EventId = event.compute_id(); let event: Event = self .request(RequestMethod::SignEvent, serde_json::to_value(event)?) .await?; event.verify()?; + + if event.id != expected_id { + return Err(Error::generic(format!( + "signed event doesn't match the requested event: expected ID {expected_id}, got {}", + event.id + ))); + } + Ok(event) } @@ -466,6 +490,54 @@ impl AsyncNip44 for BrowserSignerProxy { } } +/// Outcome of reading a line with [`read_line_capped`]. +enum HeaderLine { + /// A line, including the trailing newline when present + Line(Vec), + /// Clean EOF with no data + Eof, + /// The line exceeded the allowed length + TooLong, +} + +/// Read one `\n`-terminated line with a hard cap so a peer cannot force unbounded buffering. +async fn read_line_capped( + reader: &mut BufReader, + limit: usize, +) -> Result { + let mut line: Vec = Vec::new(); + loop { + let (data, line_complete) = { + let chunk = reader.fill_buf().await?; + if chunk.is_empty() { + break; + } + match chunk.iter().position(|byte| *byte == b'\n') { + Some(index) => (chunk[..=index].to_vec(), true), + None => (chunk.to_vec(), false), + } + }; + + if line.len() + data.len() > limit { + return Ok(HeaderLine::TooLong); + } + + let taken = data.len(); + line.extend_from_slice(&data); + reader.consume(taken); + + if line_complete { + return Ok(HeaderLine::Line(line)); + } + } + + Ok(if line.is_empty() { + HeaderLine::Eof + } else { + HeaderLine::Line(line) + }) +} + /// Handle a single HTTP connection. async fn handle_connection( stream: TcpStream, @@ -475,13 +547,26 @@ async fn handle_connection( session_token: String, ) { let mut reader = BufReader::new(stream); + let mut header_bytes: usize = 0; // Read the request line - let mut request_line = String::new(); - if reader.read_line(&mut request_line).await.is_err() { - return; - } - let request_line = request_line.trim_end().to_string(); + let request_line = match read_line_capped(&mut reader, MAX_HEADER_BYTES).await { + Ok(HeaderLine::Line(line)) => line, + Ok(HeaderLine::Eof) => Vec::new(), + Ok(HeaderLine::TooLong) => { + send_response(&mut reader, 400, "Bad Request", "", "").await; + return; + } + Err(_) => return, + }; + header_bytes += request_line.len(); + let request_line = match String::from_utf8(request_line) { + Ok(line) => line.trim_end().to_string(), + Err(_) => { + send_response(&mut reader, 400, "Bad Request", "", "").await; + return; + } + }; // Parse method, path, and HTTP version from request line let parts: Vec<&str> = request_line.split_whitespace().collect(); @@ -498,12 +583,29 @@ async fn handle_connection( let mut host: Option = None; loop { - let mut line = String::new(); - if reader.read_line(&mut line).await.is_err() { + let line = match read_line_capped(&mut reader, MAX_HEADER_BYTES).await { + Ok(HeaderLine::Line(line)) => line, + Ok(HeaderLine::Eof) => break, + Ok(HeaderLine::TooLong) => { + send_response(&mut reader, 400, "Bad Request", "", "").await; + return; + } + Err(_) => return, + }; + + header_bytes += line.len(); + if header_bytes > MAX_HEADER_BYTES { + send_response(&mut reader, 400, "Bad Request", "", "").await; return; } - let line = line.trim_end().to_string(); + let line = match String::from_utf8(line) { + Ok(line) => line.trim_end().to_string(), + Err(_) => { + send_response(&mut reader, 400, "Bad Request", "", "").await; + return; + } + }; if line.is_empty() { break; } @@ -521,6 +623,12 @@ async fn handle_connection( headers.push(line); } + // Reject oversized bodies before allocating the read buffer + if content_length > MAX_BODY_BYTES { + send_response(&mut reader, 413, "Payload Too Large", "", "").await; + return; + } + // DNS rebinding defense: only the exact bound host:port is accepted if host.as_deref() != Some(expected_host.as_str()) { send_response(&mut reader, 403, "Forbidden", "", "").await; @@ -594,7 +702,17 @@ async fn handle_connection( } }; - tracing::debug!("Received response from browser: {message:?}"); + // Never log the response value, it can contain decrypted plaintext + let outcome = if message.error.is_some() { + "error" + } else { + "ok" + }; + + tracing::debug!( + "Received response from browser: id={} outcome={outcome}", + message.id + ); let id: Uuid = message.id; let mut pending = state.pending_responses.lock().await; diff --git a/crates/chat/src/lib.rs b/crates/chat/src/lib.rs index 2c717d2d..18da14f8 100644 --- a/crates/chat/src/lib.rs +++ b/crates/chat/src/lib.rs @@ -396,10 +396,27 @@ impl ChatRegistry { } if !errors.is_empty() { + let mut newly_trashed = 0; trash.update(cx, |trash, cx| { - trash.extend(errors); + newly_trashed = errors + .iter() + .filter(|message| trash.insert((*message).clone())) + .count(); cx.notify(); })?; + + // Only newly trashed messages emit an error event so repeated + // reprocess attempts of the same message don't re-alert the user + if newly_trashed > 0 { + let reason = if newly_trashed == 1 { + "A message could not be decrypted".to_string() + } else { + format!("{newly_trashed} messages could not be decrypted") + }; + this.update(cx, |_, cx| { + cx.emit(ChatEvent::Error(reason)); + })?; + } } if eose && tracking.swap(false, Ordering::AcqRel) { diff --git a/crates/chat_ui/Cargo.toml b/crates/chat_ui/Cargo.toml index d41e9f78..accf867c 100644 --- a/crates/chat_ui/Cargo.toml +++ b/crates/chat_ui/Cargo.toml @@ -18,6 +18,7 @@ gpui.workspace = true instant.workspace = true nostr-sdk.workspace = true anyhow.workspace = true +log.workspace = true itertools.workspace = true smallvec.workspace = true serde.workspace = true diff --git a/crates/chat_ui/src/lib.rs b/crates/chat_ui/src/lib.rs index 6701b8f3..abb8b740 100644 --- a/crates/chat_ui/src/lib.rs +++ b/crates/chat_ui/src/lib.rs @@ -1,3 +1,4 @@ +use std::cmp::Ordering; use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::{Arc, LazyLock, RwLock}; use std::time::Duration; @@ -220,7 +221,7 @@ impl ChatPanel { // Define all functions that will run after the current cycle cx.defer_in(window, |this, window, cx| { - this.connect(cx); + this.connect(window, cx); this.subscribe_room_events(window, cx); this.get_messages(window, cx); }); @@ -269,11 +270,24 @@ impl ChatPanel { } /// Get messaging relays and announcement for each member - fn connect(&mut self, cx: &mut Context) { - if let Some(room) = self.room.upgrade() { - let task = room.read(cx).connect(cx); - self.tasks.push(task); - } + fn connect(&mut self, window: &mut Window, cx: &mut Context) { + let Some(room) = self.room.upgrade() else { + return; + }; + + let connect = room.read(cx).connect(cx); + + self.tasks.push(cx.spawn_in(window, async move |this, cx| { + if let Err(error) = connect.await { + log::error!("Connecting room relays failed: {error}"); + + this.update_in(cx, |this, window, cx| { + this.notify_failure("Connection failed", error, window, cx); + })?; + } + + Ok(()) + })); } /// Subscribe to room events @@ -317,23 +331,40 @@ impl ChatPanel { } /// Load all messages belonging to this room - fn get_messages(&mut self, _window: &mut Window, cx: &mut Context) { + fn get_messages(&mut self, window: &mut Window, cx: &mut Context) { let Ok(get_messages) = self.room.read_with(cx, |this, cx| this.get_messages(cx)) else { return; }; - self.tasks.push(cx.spawn(async move |this, cx| { - let events = get_messages.await?; - - // Update message list - this.update(cx, |this, cx| { - this.insert_messages(&events, cx); - })?; + self.tasks.push(cx.spawn_in(window, async move |this, cx| { + match get_messages.await { + Ok(events) => { + this.update(cx, |this, cx| { + this.insert_messages(&events, cx); + })?; + } + Err(error) => { + log::error!("Loading room messages failed: {error}"); + this.update_in(cx, |this, window, cx| { + this.notify_failure("Failed to load messages", error, window, cx); + })?; + } + } Ok(()) })); } + /// Show a failure of a background operation to the user until dismissed + fn notify_failure(&self, title: &'static str, error: Error, window: &mut Window, cx: &mut App) { + window.push_notification( + Notification::error(error.to_string()) + .title(title) + .autohide(false), + cx, + ); + } + /// Get user input content and merged all attachments if available fn get_input_value(&self, cx: &Context) -> String { // Get input's value @@ -566,8 +597,9 @@ impl ChatPanel { for (i, message) in self.messages.iter().enumerate().skip(pos) { self.message_index.insert(message.id, i); } - let len = self.list_state.item_count(); - self.list_state.splice(len..len, 1); + // The welcome row occupies row zero, so message rows sit one ahead + let row = pos + 1; + self.list_state.splice(row..row, 1); if scroll { self.list_state.scroll_to(ListOffset { @@ -582,13 +614,62 @@ impl ChatPanel { /// Convert and insert a vector of nostr events into the chat panel fn insert_messages(&mut self, events: &[UnsignedEvent], cx: &mut Context) { + let mut incoming: Vec = Vec::with_capacity(events.len()); + for event in events.iter() { if event.kind == Kind::Reaction { self.insert_reaction(event, cx); + } else { + incoming.push(event.into()); + } + } + + incoming.sort(); + + let old_len = self.messages.len(); + let mut stored = std::mem::take(&mut self.messages).into_iter().peekable(); + let mut merged: Vec = Vec::with_capacity(old_len + incoming.len()); + let mut insert_start = None; + let mut inserted = 0usize; + + for message in incoming { + while let Some(old) = stored.next_if(|old| old.cmp(&message) == Ordering::Less) { + merged.push(old); + } + + let duplicate = stored + .peek() + .is_some_and(|old| old.cmp(&message) == Ordering::Equal) + || merged + .last() + .is_some_and(|last| last.cmp(&message) == Ordering::Equal); + + if duplicate { continue; } - // Bulk inserting messages, so no need to scroll to the latest message - self.insert_message(event, false, cx); + + if inserted == 0 { + insert_start = Some(merged.len()); + } + inserted += 1; + merged.push(message); + } + + merged.extend(stored); + + self.messages = merged; + + if let Some(start) = insert_start { + self.message_index = self + .messages + .iter() + .enumerate() + .map(|(index, message)| (message.id, index)) + .collect(); + + let row = start + 1; + self.list_state.splice(row..row, inserted); + cx.notify(); } } diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index aefb5ab9..2d70ccfe 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -9,6 +9,7 @@ gpui.workspace = true nostr.workspace = true instant.workspace = true nostr-sdk.workspace = true +sha2.workspace = true itertools.workspace = true chrono.workspace = true diff --git a/crates/common/src/event.rs b/crates/common/src/event.rs index b1fd1e74..bedcf1da 100644 --- a/crates/common/src/event.rs +++ b/crates/common/src/event.rs @@ -1,20 +1,18 @@ -use std::hash::{DefaultHasher, Hash, Hasher}; - use itertools::Itertools; use nostr_sdk::prelude::*; +use sha2::{Digest, Sha256}; + +const HEX_DIGITS: [u8; 16] = *b"0123456789abcdef"; pub trait EventExt { + /// Id over the event's participants, stable across Rust versions. fn uniq_id(&self) -> u64; fn extract_public_keys(&self) -> Vec; } impl EventExt for Event { fn uniq_id(&self) -> u64 { - let mut hasher = DefaultHasher::new(); - let mut pubkeys: Vec = self.extract_public_keys(); - pubkeys.sort(); - pubkeys.hash(&mut hasher); - hasher.finish() + pubkey_set_uniq_id(&self.extract_public_keys()) } fn extract_public_keys(&self) -> Vec { @@ -27,31 +25,62 @@ impl EventExt for Event { impl EventExt for UnsignedEvent { fn uniq_id(&self) -> u64 { - let mut hasher = DefaultHasher::new(); - let mut pubkeys: Vec = vec![]; - - // Add all public keys from event - pubkeys.push(self.pubkey); - pubkeys.extend(self.tags.public_keys().collect::>()); - - // Generate unique hash - pubkeys - .into_iter() - .unique() - .sorted() - .collect::>() - .hash(&mut hasher); - - hasher.finish() + pubkey_set_uniq_id(&self.extract_public_keys()) } fn extract_public_keys(&self) -> Vec { let mut public_keys: Vec = self.tags.public_keys().collect(); public_keys.push(self.pubkey); + public_keys.into_iter().unique().sorted().collect() } } +/// Digest over NUL-separated parts, so part boundaries stay unambiguous. +fn parts_digest(parts: &[&str]) -> [u8; 32] { + let mut hasher = Sha256::new(); + + for part in parts { + hasher.update(part.as_bytes()); + hasher.update([0]); + } + + hasher.finalize().into() +} + +/// Stable id over parts: hex-encoded SHA-256 truncated to 128 bits. +pub fn stable_id(parts: &[&str]) -> String { + let digest = parts_digest(parts); + + let mut id = String::with_capacity(32); + for byte in &digest[..16] { + id.push(HEX_DIGITS[usize::from(byte >> 4)] as char); + id.push(HEX_DIGITS[usize::from(byte & 0x0f)] as char); + } + + id +} + +/// Stable 64-bit id over parts, for callers that key ids on `u64`. +pub fn stable_id_u64(parts: &[&str]) -> u64 { + let digest = parts_digest(parts); + + let mut lead = [0u8; 8]; + for (slot, byte) in lead.iter_mut().zip(digest.iter()) { + *slot = *byte; + } + + u64::from_be_bytes(lead) +} + +/// Hashes the sorted hex forms of the participant set. +fn pubkey_set_uniq_id(public_keys: &[PublicKey]) -> u64 { + let mut parts: Vec = public_keys.iter().map(|key| key.to_string()).collect(); + parts.sort(); + let parts: Vec<&str> = parts.iter().map(String::as_str).collect(); + stable_id_u64(&parts) +} + /// Custom emoji (NIP-30) declared by `emoji` tags, first per shortcode wins pub fn emojis_from_tags(tags: &Tags) -> Vec<(String, Url)> { let mut emojis: Vec<(String, Url)> = Vec::new(); @@ -100,4 +129,51 @@ mod tests { Url::parse("https://example.com/soapbox.png").unwrap() ); } + + fn event_with_participants(public_key: PublicKey, participants: Vec) -> Event { + let tags = Tags::from_list(participants.into_iter().map(Tag::public_key).collect()); + + Event::new( + EventId::from_byte_array([0; 32]), + public_key, + Timestamp::from(0u64), + Kind::TextNote, + tags, + "content".to_string(), + Signature::from_byte_array([0; 64]), + ) + } + + #[test] + fn uniq_id_ignores_participant_order() { + let me = Keys::new(SecretKey::generate()).public_key(); + let alice = Keys::new(SecretKey::generate()).public_key(); + let bob = Keys::new(SecretKey::generate()).public_key(); + + let first = event_with_participants(me, vec![alice, bob]); + let second = event_with_participants(me, vec![bob, alice]); + + assert_eq!(first.uniq_id(), second.uniq_id()); + } + + #[test] + fn uniq_id_tracks_participants() { + let me = Keys::new(SecretKey::generate()).public_key(); + let alice = Keys::new(SecretKey::generate()).public_key(); + let bob = Keys::new(SecretKey::generate()).public_key(); + + let with_alice = event_with_participants(me, vec![alice]); + let with_bob = event_with_participants(me, vec![bob]); + + assert_ne!(with_alice.uniq_id(), with_bob.uniq_id()); + } + + #[test] + fn stable_id_is_deterministic_128_bit_hex() { + let id = stable_id(&["alpha", "beta"]); + + assert_eq!(id.len(), 32); + assert!(id.chars().all(|character| character.is_ascii_hexdigit())); + assert_eq!(id, stable_id(&["alpha", "beta"])); + } } diff --git a/crates/common/src/parser.rs b/crates/common/src/parser.rs index 8ca3f374..8ba84c76 100644 --- a/crates/common/src/parser.rs +++ b/crates/common/src/parser.rs @@ -184,20 +184,11 @@ impl Iterator for FindMatches<'_> { } } -enum HandleMatch { - Token(Token), - Recursion, -} - pub struct NostrParserIter<'a> { /// The original text text: &'a str, /// Matches found matches: FindMatches<'a>, - /// A pending match - pending_match: Option, - /// Last match end index - last_match_end: usize, } impl<'a> NostrParserIter<'a> { @@ -205,62 +196,65 @@ impl<'a> NostrParserIter<'a> { Self { text, matches: FindMatches::new(text), - pending_match: None, - last_match_end: 0, } } - fn handle_match(&mut self, mat: Match) -> HandleMatch { - // Update last match end - self.last_match_end = mat.end; - + fn handle_match(&self, mat: Match) -> Option { // Extract the matched string let data: &str = &self.text[mat.start..mat.end]; // Parse NIP-21 URI match Nip21::parse(data) { - Ok(uri) => HandleMatch::Token(Token { + Ok(uri) => Some(Token { value: uri, range: mat.start..mat.end, }), // If the nostr URI parsing is invalid, skip it - Err(_) => HandleMatch::Recursion, + Err(_) => None, } } } -impl<'a> Iterator for NostrParserIter<'a> { +impl Iterator for NostrParserIter<'_> { type Item = Token; fn next(&mut self) -> Option { - // Handle a pending match - if let Some(pending_match) = self.pending_match.take() { - return match self.handle_match(pending_match) { - HandleMatch::Token(token) => Some(token), - HandleMatch::Recursion => self.next(), - }; - } + // Iterative, not recursive: message text controls the match count + loop { + let mat = self.matches.next()?; - match self.matches.next() { - Some(mat) => { - // Skip any text before this match - if mat.start > self.last_match_end { - // Update pending match - // This will be handled at next iteration, in `handle_match` method. - self.pending_match = Some(mat); - - // Skip the text before the match - self.last_match_end = mat.start; - return self.next(); - } - - // Handle match - match self.handle_match(mat) { - HandleMatch::Token(token) => Some(token), - HandleMatch::Recursion => self.next(), - } + if let Some(token) = self.handle_match(mat) { + return Some(token); } - None => None, } } } + +#[cfg(test)] +mod tests { + use super::*; + + // A long run of scanner matches that fail `Nip21::parse`: the count is + // attacker-controlled, so it must be consumed iteratively. + #[test] + fn skips_long_run_of_invalid_uris() { + let content = "nostr:1q".repeat(50_000); + + let tokens: Vec = NostrParser::new().parse(&content).collect(); + + assert!(tokens.is_empty()); + } + + #[test] + fn yields_valid_uri_after_invalid_run() { + let public_key = Keys::new(SecretKey::generate()).public_key(); + let npub = public_key.to_bech32().unwrap(); + let content = format!("{}nostr:{npub}", "nostr:1q".repeat(50_000)); + + let tokens: Vec = NostrParser::new().parse(&content).collect(); + + assert_eq!(tokens.len(), 1); + assert_eq!(tokens[0].range.start, 400_000); + assert_eq!(tokens[0].value, Nip21::Pubkey(public_key)); + } +} diff --git a/crates/nip29/src/protocol.rs b/crates/nip29/src/protocol.rs index 26502327..6c00a192 100644 --- a/crates/nip29/src/protocol.rs +++ b/crates/nip29/src/protocol.rs @@ -1,10 +1,9 @@ use std::collections::HashSet; -use std::collections::hash_map::DefaultHasher; use std::fmt; -use std::hash::{Hash, Hasher}; use std::str::FromStr; use anyhow::{Result, anyhow, bail}; +use common::stable_id_u64; use nostr_sdk::prelude::*; pub(crate) const TIMELINE_WINDOW: usize = 50; @@ -289,9 +288,7 @@ impl GroupKey { } pub fn uniq_id(&self) -> u64 { - let mut hasher = DefaultHasher::new(); - self.hash(&mut hasher); - hasher.finish() + stable_id_u64(&[self.relay.as_str(), self.id.as_str()]) } } diff --git a/crates/nip29/src/utils.rs b/crates/nip29/src/utils.rs index 153540ed..3b93d005 100644 --- a/crates/nip29/src/utils.rs +++ b/crates/nip29/src/utils.rs @@ -1,7 +1,6 @@ use std::collections::HashSet; -use std::collections::hash_map::DefaultHasher; -use std::hash::{Hash, Hasher}; +use common::stable_id_u64; use nostr_sdk::prelude::*; use crate::{GroupId, GroupKey}; @@ -25,10 +24,10 @@ pub(crate) fn subscription_id(key: &GroupKey) -> SubscriptionId { } pub(crate) fn browse_subscription(relay: &RelayUrl) -> SubscriptionId { - let mut hasher = DefaultHasher::new(); - relay.hash(&mut hasher); - - SubscriptionId::new(format!("{BROWSE_PREFIX}{:08x}", hasher.finish() as u32)) + SubscriptionId::new(format!( + "{BROWSE_PREFIX}{:08x}", + stable_id_u64(&[relay.as_str()]) as u32 + )) } pub(crate) fn page_subscription(key: &GroupKey) -> SubscriptionId { diff --git a/crates/nip4e/src/lib.rs b/crates/nip4e/src/lib.rs index f52f4cf2..c4295f34 100644 --- a/crates/nip4e/src/lib.rs +++ b/crates/nip4e/src/lib.rs @@ -83,6 +83,8 @@ pub struct DeviceRegistry { resync: Arc, /// Whether a key rotation is in flight, to coalesce overlapping requests rotating: bool, + /// Whether a key approval is in flight, to guard against a double click + approving: bool, /// Person directory for requesters shown in the approval dialog directory: Entity, /// Relay notification listener @@ -155,6 +157,7 @@ impl DeviceRegistry { announcement_existed: Arc::new(AtomicBool::new(false)), resync: Arc::new(AtomicBool::new(false)), rotating: false, + approving: false, directory: Directory::new(cx), notification_listener: None, tasks: vec![], @@ -186,22 +189,31 @@ impl DeviceRegistry { continue; } - match event.kind { - Kind::Custom(10044) if current_user == Some(event.pubkey) => { - announcement_existed.store(true, Ordering::Relaxed); - tx.send_async(event.into_owned()).await?; - } - Kind::Custom(4454) if current_user == Some(event.pubkey) => { - tx.send_async(event.into_owned()).await?; - } - Kind::Custom(4455) if current_user == Some(event.pubkey) => { - tx.send_async(event.into_owned()).await?; - } - _ => {} + let relevant = matches!( + event.kind, + Kind::Custom(10044) | Kind::Custom(4454) | Kind::Custom(4455) + ) && current_user == Some(event.pubkey); + + if !relevant { + continue; + } + + if event.kind == Kind::Custom(10044) { + announcement_existed.store(true, Ordering::Relaxed); + } + + let event_id = event.id; + // Dropping keeps the listener alive when the consumer is gone; + // missed 4e events are re-synced on the next signer change + if let Err(error) = tx.send_async(event.into_owned()).await { + log::error!("Dropping device key event {event_id}: {error}"); } } } + // The stream only ends without being replaced when the relay source is + // gone, after which no 4e event is processed anymore + log::error!("Device key notification listener stopped"); Ok(()) })); @@ -399,10 +411,16 @@ impl DeviceRegistry { self.tasks.push(cx.spawn(async move |this, cx| { let result: Result = async { - // Archive the current key before replacing it - if let Ok(old) = get_keys(&client, &signer).await { - archive_key(&client, &signer, &old.secret_key().to_secret_hex()).await?; - } + // Archive the current key before replacing it; a failed load or + // archive aborts the rotation so the old key is never overwritten + // without a record + let old = get_keys(&client, &signer) + .await + .context("Failed to load the current encryption key before rotation")?; + + archive_key(&client, &signer, &old.secret_key().to_secret_hex()) + .await + .context("Failed to archive the current encryption key")?; let created = this .update(cx, |this, cx| this.create_encryption(Keys::generate(), cx))? @@ -772,12 +790,21 @@ impl DeviceRegistry { /// Approve requests for device keys from other devices fn approve(&mut self, event: &Event, window: &mut Window, cx: &mut Context) { + // An approval already in flight must not send the device secret twice; + // the toast's disabled state only applies after its next re-render + if self.approving { + return; + } + self.approving = true; + cx.notify(); + + let approved_id = event.id; + let event = event.clone(); + let nostr = NostrRegistry::global(cx); let client = nostr.read(cx).client(); let signer = nostr.read(cx).signer(); - let event = event.clone(); - let task: Task> = cx.background_spawn(async move { // Get device keys let keys = get_keys(&client, &signer).await?; @@ -834,17 +861,41 @@ impl DeviceRegistry { self.tasks.push(cx.spawn_in(window, async move |this, cx| { match task.await { Ok(_) => { - this.update(cx, |this, cx| { - this.pending_request = None; + let superseded = this.update(cx, |this, cx| { + this.approving = false; cx.notify(); + // Keep a newer request's pending state and toast + if this + .pending_request + .as_ref() + .is_some_and(|pending| pending.event.id == approved_id) + { + this.pending_request = None; + false + } else { + true + } })?; - cx.update(|window, cx| { - window.remove_notification::(cx); - }) - .ok(); + if !superseded { + cx.update(|window, cx| { + window.remove_notification::(cx); + }) + .ok(); + } } Err(e) => { + this.update(cx, |this, cx| { + this.approving = false; + cx.notify(); + + // Pushing a fresh toast replaces the wedged one whose + // button stays disabled + if let Some(pending) = this.pending_request.take() { + this.show_approval(pending.event, pending.client_key, cx); + } + })?; + cx.update(|window, cx| { window.push_notification( Notification::error(e.to_string()).autohide(false), @@ -1032,9 +1083,10 @@ impl DeviceRegistry { .on_click({ let loading = Rc::clone(&loading); move |_ev, window, cx| { - // Set loading state to true loading.set(true); - // Process to approve the request + // The toast caches its render, so force a redraw + // to disable the button while the approval is in flight + window.refresh(); view.update(cx, |this, cx| { this.approve(&event, window, cx); }) diff --git a/crates/nip60/src/lib.rs b/crates/nip60/src/lib.rs index 4e032cf7..085a64d3 100644 --- a/crates/nip60/src/lib.rs +++ b/crates/nip60/src/lib.rs @@ -193,6 +193,16 @@ impl WalletRegistry { ); } + /// Drop a pending invoice at the user's request. + pub fn dismiss_mint_quote(&mut self, quote_id: String, cx: &mut Context) { + if !self.quotes.iter().any(|quote| quote.quote_id == quote_id) { + return; + } + + self.quotes.retain(|quote| quote.quote_id != quote_id); + self.emit_updated(cx); + } + /// Pay a lightning invoice from the wallet. pub fn melt(&mut self, invoice: String, cx: &mut Context) { let Some(mint_url) = self.primary_mint(cx) else { @@ -896,7 +906,11 @@ impl WalletRegistry { }; match outcome { - ops::Outcome::Idle => {} + ops::Outcome::Idle => { + if matches!(job, ops::Job::ClaimQuote { .. }) { + self.emit_error("Invoice is not paid yet".to_string(), cx); + } + } ops::Outcome::Reconciled { updates } => { self.publish_failed = false; let changed = !updates.is_empty(); diff --git a/crates/nip60_ui/src/wallet.rs b/crates/nip60_ui/src/wallet.rs index 38d1a6b0..e3ed3c6b 100644 --- a/crates/nip60_ui/src/wallet.rs +++ b/crates/nip60_ui/src/wallet.rs @@ -1,7 +1,7 @@ use std::ops::Range; use std::rc::Rc; -use ::nip60::{HistoryEntry, WalletEvent, WalletRegistry}; +use ::nip60::{HistoryEntry, MintQuoteEntry, WalletEvent, WalletRegistry}; use common::TimestampExt; use gpui::prelude::FluentBuilder; use gpui::{ @@ -13,6 +13,7 @@ use gpui::{ use gpui_kit::assets::IconName; use gpui_kit::base::StyledExt as _; use gpui_kit::component::button::{Button, ButtonVariants as _}; +use gpui_kit::component::clipboard::Clipboard; use gpui_kit::component::dock::{Panel, PanelEvent}; use gpui_kit::component::input::{Input, InputEvent, InputState, NumberInput}; use gpui_kit::component::notification::Notification; @@ -108,6 +109,111 @@ impl WalletPanel { ) } + fn render_quotes(&self, quotes: Vec, cx: &Context) -> Option { + if quotes.is_empty() { + return None; + } + + Some( + v_flex() + .gap_2() + .child( + div() + .text_xs() + .font_semibold() + .text_color(cx.theme().muted_foreground) + .child("Pending invoices"), + ) + .children( + quotes + .iter() + .map(|quote| self.render_quote_entry(quote, cx)), + ) + .into_any_element(), + ) + } + + fn render_quote_entry(&self, quote: &MintQuoteEntry, cx: &Context) -> impl IntoElement { + let wallet = WalletRegistry::global(cx); + let quote_id = quote.quote_id.clone(); + + v_flex() + .gap_1() + .w_full() + .child( + h_flex() + .justify_between() + .gap_2() + .child( + h_flex() + .gap_2() + .min_w_0() + .child( + div() + .flex_shrink_0() + .text_color(cx.theme().success) + .child(SharedString::from(format!("{} sats", quote.amount))), + ) + .child( + div() + .truncate() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(quote.mint.to_string())), + ), + ) + .child( + h_flex() + .flex_shrink_0() + .gap_1() + .child( + Button::new(format!("mint-quote-check-{quote_id}")) + .label("Check") + .tooltip("Check payment and claim") + .ghost() + .small() + .on_click({ + let wallet = wallet.clone(); + let quote_id = quote_id.clone(); + move |_, _, cx| { + wallet.update(cx, |registry, cx| { + registry.claim_mint_quote(quote_id.clone(), cx) + }); + } + }), + ) + .child( + Clipboard::new(format!("mint-quote-copy-{quote_id}")) + .value(quote.request.clone()) + .tooltip("Copy invoice") + .small(), + ) + .child( + Button::new(format!("mint-quote-dismiss-{quote_id}")) + .icon(IconName::Close) + .tooltip("Dismiss") + .ghost() + .small() + .on_click({ + let wallet = wallet.clone(); + move |_, _, cx| { + wallet.update(cx, |registry, cx| { + registry.dismiss_mint_quote(quote_id.clone(), cx) + }); + } + }), + ), + ), + ) + .child( + div() + .overflow_hidden() + .whitespace_nowrap() + .text_ellipsis_middle() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(quote.request.clone())), + ) + } + fn render_history( &mut self, history: Vec, @@ -762,7 +868,9 @@ impl Focusable for WalletPanel { impl Render for WalletPanel { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { - let history = WalletRegistry::global(cx).read(cx).history().to_vec(); + let wallet = WalletRegistry::global(cx); + let history = wallet.read(cx).history().to_vec(); + let quotes = wallet.read(cx).mint_quotes().to_vec(); v_flex() .size_full() @@ -771,6 +879,7 @@ impl Render for WalletPanel { .text_xs() .child(self.render_balance(cx)) .child(self.render_actions()) + .children(self.render_quotes(quotes, cx)) .child( v_flex() .gap_2() diff --git a/crates/person/src/lib.rs b/crates/person/src/lib.rs index 1bf0d447..9e35dd83 100644 --- a/crates/person/src/lib.rs +++ b/crates/person/src/lib.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; use anyhow::{Error, anyhow}; use common::emojis_from_tags; @@ -64,7 +64,7 @@ enum Dispatch { #[derive(Debug)] pub struct PersonRegistry { /// Public keys with a metadata request in flight - in_flight: RwLock>, + in_flight: Arc>>, /// Web of trust graph of the signed-in user wot: Entity, sender: flume::Sender, @@ -102,8 +102,12 @@ impl PersonRegistry { let client3 = client.clone(); let executor = cx.background_executor().clone(); + + let in_flight = Arc::new(RwLock::new(HashSet::new())); + let in_flight_requests = Arc::clone(&in_flight); + tasks.push(cx.background_spawn(async move { - Self::handle_requests(&client3, &metadata_rx, executor).await; + Self::handle_requests(&client3, &metadata_rx, executor, &in_flight_requests).await; })); tasks.push(cx.spawn(async move |this, cx| { @@ -140,7 +144,7 @@ impl PersonRegistry { }); Self { - in_flight: RwLock::new(HashSet::new()), + in_flight, wot, sender: metadata_tx, tasks, @@ -205,6 +209,7 @@ impl PersonRegistry { client: &Client, rx: &flume::Receiver, executor: BackgroundExecutor, + in_flight: &RwLock>, ) { let mut batch: HashSet = HashSet::new(); @@ -216,34 +221,44 @@ impl PersonRegistry { result = rx.recv_async().fuse() => match result { Ok(public_key) => public_key, Err(_) => { - if !batch.is_empty() - && let Err(e) = get_metadata(client, std::mem::take(&mut batch)).await - { - log::warn!("Failed to get metadata batch: {e}"); - } + Self::flush_batch(client, &mut batch, in_flight).await; return; } }, () = timer => { - if !batch.is_empty() - && let Err(e) = get_metadata(client, std::mem::take(&mut batch)).await - { - log::warn!("Failed to get metadata batch: {e}"); - } + Self::flush_batch(client, &mut batch, in_flight).await; continue; } }; batch.insert(request); // Process the batch if it's full - if batch.len() >= 20 - && let Err(e) = get_metadata(client, std::mem::take(&mut batch)).await - { - log::warn!("Failed to get metadata batch: {e}"); + if batch.len() >= 20 { + Self::flush_batch(client, &mut batch, in_flight).await; } } } + /// Send one metadata request for the batch, releasing its keys for retry on failure + async fn flush_batch( + client: &Client, + batch: &mut HashSet, + in_flight: &RwLock>, + ) { + let batch = std::mem::take(batch); + if batch.is_empty() { + return; + } + + if let Err(error) = get_metadata(client, batch.iter().cloned()).await { + Self::release_keys(in_flight, batch.iter().cloned()); + log::warn!( + "Failed to get metadata for {} public keys, released for retry: {error}", + batch.len() + ); + } + } + /// Subscribe to the signed-in user's emoji list fn get_emojis(&mut self, cx: &mut Context) { let nostr = NostrRegistry::global(cx); @@ -334,11 +349,21 @@ impl PersonRegistry { /// Allow future fetches for a key once its metadata has arrived fn release_in_flight(&self, public_key: &PublicKey) { - let mut in_flight = match self.in_flight.write() { + Self::release_keys(&self.in_flight, [*public_key]); + } + + /// Remove the given keys from the in-flight set + fn release_keys( + in_flight: &RwLock>, + keys: impl IntoIterator, + ) { + let mut in_flight = match in_flight.write() { Ok(in_flight) => in_flight, Err(poisoned) => poisoned.into_inner(), }; - in_flight.remove(public_key); + for public_key in keys { + in_flight.remove(&public_key); + } } } diff --git a/crates/workspace/src/dialogs/new_chat.rs b/crates/workspace/src/dialogs/new_chat.rs index 81d92ad0..a381dc57 100644 --- a/crates/workspace/src/dialogs/new_chat.rs +++ b/crates/workspace/src/dialogs/new_chat.rs @@ -60,6 +60,9 @@ impl NewChat { let subscription = cx.subscribe_in(&input, window, |this, _input, event, window, cx| { if let InputEvent::PressEnter { .. } = event { this.submit(window, cx) + } else if matches!(event, InputEvent::Change) { + // The contact list only re-filters when the dialog re-renders + cx.notify(); } }); diff --git a/crates/workspace/src/dialogs/profile.rs b/crates/workspace/src/dialogs/profile.rs index b3c83eb2..1f666266 100644 --- a/crates/workspace/src/dialogs/profile.rs +++ b/crates/workspace/src/dialogs/profile.rs @@ -5,16 +5,17 @@ use chat::{ChatRegistry, Room, RoomKind}; use gpui::prelude::FluentBuilder; use gpui::{ App, AppContext, Context, Div, Entity, IntoElement, ParentElement, Render, SharedString, - Styled, Task, Window, div, retain_all, + Styled, Subscription, Task, Window, div, retain_all, }; use gpui_kit::assets::IconName; use gpui_kit::base::StyledExt as _; use gpui_kit::component::button::{Button, ButtonVariants}; use gpui_kit::component::clipboard::Clipboard; use gpui_kit::component::dock::DockPlacement; -use gpui_kit::component::input::{Input, InputState, Textarea, TextareaState}; +use gpui_kit::component::input::{Input, InputEvent, InputState, Textarea, TextareaState}; use gpui_kit::component::notification::Notification; use gpui_kit::component::{ActiveTheme, Disableable, Sizable, WindowExt, h_flex, v_flex}; +use instant::Duration; use nostr_sdk::prelude::*; use person::{Directory, Person, PersonRegistry, shorten_pubkey}; use settings::AppSettings; @@ -51,13 +52,19 @@ struct ProfileDialog { uploading: bool, /// Updating state updating: bool, + /// Error message + error: Option, /// Tasks tasks: Vec>>, + /// Keeps the avatar preview live while the URL is typed + _subscription: Option, } impl ProfileDialog { fn new(public_key: PublicKey, window: &mut Window, cx: &mut Context) -> Self { + let directory = Directory::new(cx); let nostr = NostrRegistry::global(cx); + let is_current_user = nostr .read(cx) .current_user() @@ -75,7 +82,17 @@ impl ProfileDialog { .placeholder("A short introduce about you.") }); - let directory = Directory::new(cx); + let subscription = cx.subscribe_in( + &avatar_input, + window, + |_this, _avatar_input, event, _window, cx| { + if matches!(event, InputEvent::Change) { + // The avatar preview only re-renders when the dialog does + cx.notify(); + } + }, + ); + directory.update(cx, |directory, cx| { directory.track([public_key], cx); }); @@ -91,7 +108,9 @@ impl ProfileDialog { website_input, uploading: false, updating: false, + error: None, tasks: vec![], + _subscription: Some(subscription), } } @@ -190,6 +209,26 @@ impl ProfileDialog { cx.notify(); } + fn set_error(&mut self, error: E, window: &mut Window, cx: &mut Context) + where + E: Into, + { + self.error = Some(error.into()); + cx.notify(); + + self.tasks.push(cx.spawn_in(window, async move |this, cx| { + cx.background_executor().timer(Duration::from_secs(3)).await; + + // Clear the error message after a delay + this.update(cx, |this, cx| { + this.error = None; + cx.notify(); + })?; + + Ok(()) + })); + } + /// Set the metadata for the current user fn publish(&self, metadata: &Metadata, cx: &App) -> Task> { let nostr = NostrRegistry::global(cx); @@ -218,19 +257,51 @@ impl ProfileDialog { let bio = self.bio_input.read(cx).value(); let website = self.website_input.read(cx).value(); + // Validate both URLs before publishing anything so a partial profile is never sent + let avatar_url = if avatar.is_empty() { + None + } else { + match Url::from_str(&avatar) { + Ok(url) => Some(url), + Err(_) => { + self.set_error( + "Avatar URL is invalid. Include the full address, e.g. https://example.com/avatar.jpg", + window, + cx, + ); + return; + } + } + }; + + let website_url = if website.is_empty() { + None + } else { + match Url::from_str(&website) { + Ok(url) => Some(url), + Err(_) => { + self.set_error( + "Website URL is invalid. Include the full address, e.g. https://example.com", + window, + cx, + ); + return; + } + } + }; + // Construct the new metadata let mut new_metadata = old_metadata .display_name(name.as_ref()) .name(name.as_ref()) .about(bio.as_ref()); - // Verify the avatar URL before adding it - if let Ok(url) = Url::from_str(&avatar) { + // An empty input keeps the saved value, a valid URL replaces it + if let Some(url) = avatar_url { new_metadata = new_metadata.picture(url); } - // Verify the website URL before adding it - if let Ok(url) = Url::from_str(&website) { + if let Some(url) = website_url { new_metadata = new_metadata.website(url); } @@ -429,6 +500,15 @@ impl Render for ProfileDialog { this.child(field("Web of Trust", score, "Not loaded yet", cx)) }) }) + .when_some(self.error.as_ref(), |this, error| { + this.child( + div() + .italic() + .text_xs() + .text_color(cx.theme().danger) + .child(error.clone()), + ) + }) .child( h_flex() .gap_1() diff --git a/crates/workspace/src/dialogs/screening.rs b/crates/workspace/src/dialogs/screening.rs index 6defb5bb..edff994a 100644 --- a/crates/workspace/src/dialogs/screening.rs +++ b/crates/workspace/src/dialogs/screening.rs @@ -10,6 +10,7 @@ use gpui::{ use gpui_kit::assets::IconName; use gpui_kit::base::StyledExt as _; use gpui_kit::component::button::{Button, ButtonVariants}; +use gpui_kit::component::notification::Notification; use gpui_kit::component::spinner::Spinner; use gpui_kit::component::{ActiveTheme, Disableable, Icon, Sizable, WindowExt, h_flex, v_flex}; use instant::Duration; @@ -23,11 +24,23 @@ pub fn init(public_key: PublicKey, window: &mut Window, cx: &mut App) -> Entity< cx.new(|cx| Screening::new(public_key, window, cx)) } +/// Outcome of verifying the person's NIP-05 address +enum IdentifierStatus { + /// The check is still running + Pending, + /// The address matches the person's public key + Verified, + /// The address does not match the person's public key + Mismatched, + /// The check could not run, e.g. the network is unreachable + Failed, +} + pub struct Screening { /// Public Key of the person being screened. public_key: PublicKey, - /// Whether the person's address is verified. - verified: bool, + /// Outcome of the NIP-05 address verification + identifier_status: IdentifierStatus, /// Whether the person is followed by current user. followed: bool, /// Last time the person was active. @@ -49,9 +62,8 @@ impl Screening { let mut subscriptions = smallvec![]; - subscriptions.push(cx.on_release_in(window, move |this, window, cx| { + subscriptions.push(cx.on_release_in(window, move |this, _window, _cx| { this.tasks.clear(); - window.close_all_dialogs(cx); })); subscriptions.push(cx.subscribe(&wot, |_this, _wot, _event, cx| { @@ -71,7 +83,7 @@ impl Screening { Self { public_key, - verified: false, + identifier_status: IdentifierStatus::Pending, followed: false, last_active: None, mutual_contacts: vec![], @@ -221,10 +233,17 @@ impl Screening { cx.background_spawn(async move { address.verify(&http_client, &public_key).await }); self.tasks.push(cx.spawn(async move |this, cx| { - let result = task.await.unwrap_or(false); + let result = task.await; this.update(cx, |this, cx| { - this.verified = result; + this.identifier_status = match result { + Ok(true) => IdentifierStatus::Verified, + Ok(false) => IdentifierStatus::Mismatched, + Err(error) => { + log::error!("failed to verify NIP-05 address: {error:#}"); + IdentifierStatus::Failed + } + }; cx.notify(); }) .ok(); @@ -270,15 +289,27 @@ impl Screening { Ok(()) }); - self.tasks.push(cx.spawn_in(window, async move |_, cx| { - if task.await.is_ok() { - cx.update(|window, cx| { - window.close_dialog(cx); - window.push_notification("Report submitted successfully", cx); - }) - .ok(); - } - })); + self.tasks + .push(cx.spawn_in(window, async move |_, cx| match task.await { + Ok(_) => { + cx.update(|window, cx| { + window.close_dialog(cx); + window.push_notification("Report submitted successfully", cx); + }) + .ok(); + } + Err(error) => { + log::error!("failed to submit report: {error:#}"); + + cx.update(|window, cx| { + window.push_notification( + Notification::error(error.to_string()).autohide(false), + cx, + ); + }) + .ok(); + } + })); } fn mutual_contacts( @@ -341,6 +372,9 @@ impl Render for Screening { const NO_MUTUAL: &str = "You don't have any mutual contacts."; const NIP05_MATCH: &str = "The address matches the user's public key."; const NIP05_NOT_MATCH: &str = "The address does not match the user's public key."; + const NIP05_PENDING: &str = "Checking the address…"; + const NIP05_FAILED: &str = + "The address could not be verified, the network may be unavailable."; const NO_NIP05: &str = "This person has not set up their friendly address"; let persons = PersonRegistry::global(cx); @@ -349,6 +383,20 @@ impl Render for Screening { let last_active = self.last_active.map(|_| true); + let (identifier_badge, identifier_message) = match self.identifier_status { + IdentifierStatus::Pending => (None, NIP05_PENDING), + IdentifierStatus::Verified => (Some(true), NIP05_MATCH), + IdentifierStatus::Mismatched => (Some(false), NIP05_NOT_MATCH), + IdentifierStatus::Failed => (Some(false), NIP05_FAILED), + }; + + // Without an address the check never runs + let (identifier_badge, identifier_message) = if self.address(cx).is_some() { + (identifier_badge, identifier_message) + } else { + (Some(false), NO_NIP05) + }; + // Pull from the web of trust once loaded, else use the database fallback let mutual_contacts = { let wot = persons.read(cx).wot().read(cx); @@ -491,7 +539,7 @@ impl Render for Screening { h_flex() .items_start() .gap_2() - .child(status_badge(Some(self.verified), cx)) + .child(status_badge(identifier_badge, cx)) .child( v_flex() .text_xs() @@ -508,17 +556,7 @@ impl Render for Screening { div() .line_clamp(1) .text_color(cx.theme().muted_foreground) - .child({ - if self.address(cx).is_some() { - if self.verified { - SharedString::from(NIP05_MATCH) - } else { - SharedString::from(NIP05_NOT_MATCH) - } - } else { - SharedString::from(NO_NIP05) - } - }), + .child(SharedString::from(identifier_message)), ), ), ) diff --git a/crates/workspace/src/dock/mod.rs b/crates/workspace/src/dock/mod.rs index 7dd33d90..af7394cc 100644 --- a/crates/workspace/src/dock/mod.rs +++ b/crates/workspace/src/dock/mod.rs @@ -1,9 +1,11 @@ use std::sync::Arc; -use gpui::{App, Pixels, WeakEntity, Window, px}; -use gpui_kit::component::dock::{ - DockArea, DockPlacement, InsertTarget, NodeId, PaneRef, PanelHandle, PanelId, -}; +use chat_ui::ChatPanel; +use gpui::{App, Entity, Pixels, SharedString, WeakEntity, Window, px}; +use gpui_base::dock::PanelView; +use gpui_kit::component::dock::{DockArea, DockPlacement, PanelHandle, PanelId}; +use nip29::GroupKey; +use nip29_ui::GroupPanel; mod invalid_panel; mod skin; @@ -14,39 +16,83 @@ pub use skin::CoopDockSkin; /// The fixed height of the tab bar, which doubles as the window title bar. pub const TAB_BAR_HEIGHT: Pixels = px(44.); +/// Dock placements. +const DOCK_PLACEMENTS: [DockPlacement; 4] = [ + DockPlacement::Center, + DockPlacement::Left, + DockPlacement::Right, + DockPlacement::Bottom, +]; -/// The panel in any region of `area whose id is `key. -fn find_panel(area: &DockArea, key: PanelId, _cx: &App) -> Option<(PanelId, NodeId, usize)> { - let placements = [ - DockPlacement::Center, - DockPlacement::Left, - DockPlacement::Right, - DockPlacement::Bottom, - ]; +/// What makes a panel the same panel across opens. +#[derive(Clone, PartialEq, Eq)] +enum PanelKey { + /// A panel of which there should be one, keyed by its `panel_name`. + Named(&'static str), + /// A chat panel, keyed by the id of the room it displays. + Room(SharedString), +} - for placement in placements { +/// The key a panel handle carries, or `None` when it cannot be read off the panel. +fn panel_key(panel: &PanelHandle, cx: &App) -> Option { + let view = panel.panel(); + if let Some(chat) = view.as_any().downcast_ref::>() { + return Some(PanelKey::Room(chat.read(cx).room_id().clone())); + } + if view.as_any().downcast_ref::>().is_some() { + return None; + } + Some(PanelKey::Named(view.panel_name(cx))) +} + +/// The key of a panel docked in `area`, recovered through the handle base holds. +fn docked_panel_key(view: &Arc, cx: &App) -> Option { + let handle = PanelHandle::of(view)?; + panel_key(handle, cx) +} + +/// The docked panel whose key is `key`, and the dock it sits in. +fn find_panel(area: &DockArea, key: &PanelKey, cx: &App) -> Option<(PanelId, DockPlacement)> { + for placement in DOCK_PLACEMENTS { let Some(tree) = area.layout(placement) else { continue; }; for id in tree.panels() { - if id != key { - continue; - } - - let node = tree.find_panel_node(id)?; - let PaneRef::Tabs { panels, .. } = tree.find_node(node)?.kind() else { + let Some(view) = area.panel(id) else { continue; }; - let ix = panels.iter().position(|panel| *panel == key)?; - return Some((id, node, ix)); + if docked_panel_key(view, cx).is_some_and(|found| &found == key) { + return Some((id, placement)); + } } } None } -/// Add `panel to the area, or move it to the end of its existing tab group. +/// Bring the docked panel `id` on screen. +pub fn focus_panel( + area: &mut DockArea, + id: PanelId, + window: &mut Window, + cx: &mut gpui::Context, +) { + let Some(placement) = DOCK_PLACEMENTS.into_iter().find(|placement| { + area.layout(*placement) + .is_some_and(|tree| tree.panels().any(|panel| panel == id)) + }) else { + return; + }; + + // The center region has no open state, and `toggle_dock` would ignore it. + if placement != DockPlacement::Center && !area.is_dock_open(placement) { + area.toggle_dock(placement, window, cx); + } + area.select_panel(id, window, cx); +} + +/// Add `panel` to the area, or focus the panel already showing the same thing. pub fn add_panel( area: &mut DockArea, panel: PanelHandle, @@ -54,24 +100,44 @@ pub fn add_panel( window: &mut Window, cx: &mut gpui::Context, ) { - let key = panel.panel_id(cx); - if let Some((id, node, ix)) = find_panel(area, key, cx) { - area.move_panel( - id, - InsertTarget::Tabs { - node, - ix: Some(ix), - activate: true, - }, - window, - cx, - ); + let Some(key) = panel_key(&panel, cx) else { + area.add_panel_view(Arc::new(panel), placement, None, window, cx); + return; + }; + + if let Some((id, _)) = find_panel(area, &key, cx) { + focus_panel(area, id, window, cx); return; } area.add_panel_view(Arc::new(panel), placement, None, window, cx); } +/// Close the docked chat panel showing `room_id`, if there is one. +pub fn close_room_panel( + area: &mut DockArea, + room_id: &SharedString, + window: &mut Window, + cx: &mut gpui::Context, +) { + let key = PanelKey::Room(room_id.clone()); + + let Some((id, _)) = find_panel(area, &key, cx) else { + return; + }; + + let Some(view) = area.panel(id) else { + return; + }; + + let Some(chat) = view.as_any().downcast_ref::>() else { + return; + }; + + let chat = chat.clone(); + area.remove_panel(chat, window, cx); +} + /// Add a panel to a dock reached through a weak handle. pub fn add_panel_to( dock: &WeakEntity, @@ -86,3 +152,25 @@ pub fn add_panel_to( log::warn!("Failed to add panel to dock: {error}"); } } + +/// Add a group panel to a dock, or focus the panel already showing that group. +/// +/// Group panels hide their group, so the workspace records which panel it +/// docked per key; without that record, reopening a group stacks another tab. +pub fn add_group_panel_to( + dock: &WeakEntity, + group_key: GroupKey, + panel: PanelHandle, + placement: DockPlacement, + window: &mut Window, + cx: &mut App, +) { + let Some(root) = window.root::().flatten() else { + add_panel_to(dock, panel, placement, window, cx); + return; + }; + + root.update(cx, |this, cx| { + this.add_group_panel_to_dock(group_key, panel, placement, window, cx); + }); +} diff --git a/crates/workspace/src/dock/skin.rs b/crates/workspace/src/dock/skin.rs index bc7a0c7e..b4d4b2eb 100644 --- a/crates/workspace/src/dock/skin.rs +++ b/crates/workspace/src/dock/skin.rs @@ -22,7 +22,6 @@ use super::tab_panel::CoopTabGroupSkin; /// State the skin shares with its per-container renderers. pub(crate) struct SkinShared { area: WeakEntity, - toggle_button_visible: Cell, /// The dock whose resize handle is being dragged, if any. Only one can be. resizing_dock: Cell>, } @@ -32,10 +31,6 @@ impl SkinShared { &self.area } - pub(crate) fn is_toggle_button_visible(&self) -> bool { - self.toggle_button_visible.get() - } - pub(crate) fn resizing_dock(&self) -> &Cell> { &self.resizing_dock } @@ -52,7 +47,6 @@ impl CoopDockSkin { Rc::new(Self { shared: Rc::new(SkinShared { area: cx.weak_entity(), - toggle_button_visible: Cell::new(true), resizing_dock: Cell::new(None), }), }) diff --git a/crates/workspace/src/dock/tab_panel.rs b/crates/workspace/src/dock/tab_panel.rs index c92101b4..93de7640 100644 --- a/crates/workspace/src/dock/tab_panel.rs +++ b/crates/workspace/src/dock/tab_panel.rs @@ -209,7 +209,7 @@ impl CoopTabGroupSkin { group: &TabGroupContext, cx: &mut App, ) -> Option