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.*