docs(todo): mark Discovery Pass 2 (PERF/SEC/COR) done
Replace the open Discovery-pass-2 list with a completion summary + commit refs (PERF-1..5, SEC-1..4, COR-1..6 shipped this session, gate-green, each reviewed by two agents). Record PERF-6 / SEC-5 as deferred-informational and note KE-1's storage.persist() preventive is already implemented. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+20
-25
@@ -67,35 +67,30 @@ Built and gate-green; verify per [LOTUS_TESTING.md](./LOTUS_TESTING.md), then gr
|
||||
|
||||
Agent-surveyed + TPVR-verified correctness / a11y / tech-debt fixes (DP1–DP18) are implemented, review-fixed, and gate-green (tsc + eslint + 737 tests + build). Verify per [LOTUS_TESTING.md](./LOTUS_TESTING.md) §R, then remove this note. Full detail in git history — commits `8eb961b6` `db864326` `6cf18c3b` `165714e1` `8c0e2b42` `e545706c` `b1ee3ada` `4fc3f7a3` `101e4116` `4fa4327a` `c2598d21`.
|
||||
|
||||
### 🔍 Discovery pass 2 (2026-07) — perf / security / correctness (agent-surveyed, single-pass — NOT yet TPVR'd)
|
||||
### ✅ Discovery pass 2 (2026-07) — perf / security / correctness — DONE
|
||||
|
||||
Three agents swept fresh lenses (performance, security/privacy, correctness in under-covered subsystems); each verified its own findings against the code + SDK. `Sev`/`Eff` = S/M/L. Confirm before fixing (no independent TPVR yet).
|
||||
Agent-surveyed findings, each **verified against the code before fixing**, then implemented and gate-green (tsc + eslint + prettier + 857 tests + build), with **two review agents on every staged diff before commit**. Verify per [LOTUS_TESTING.md](./LOTUS_TESTING.md), then remove this note.
|
||||
|
||||
**Performance / rendering:**
|
||||
- [x] **PERF-1 — presence: 3 client listeners PER avatar → one shared presence store** (3 listeners total). `8a154051`.
|
||||
- [x] **PERF-2 — `#`-mention autocomplete mutated + re-sorted the shared `allRoomsAtom` every keystroke** → copy + `useMemo` (also fixed a real shared-array mutation hitting ~27 consumers). `4708a179`.
|
||||
- [x] **PERF-3 — read-receipt rows: ~6 global `Members` listeners per row → one shared member-change store** (`useRoomMemberChange`). `1b8f5545`.
|
||||
- [x] **PERF-4 — message-search room filter re-sorted every render** → `useMemo`. `4708a179`.
|
||||
- [x] **PERF-5 — DM-preview `Decrypted` listener ran for every nav item** → gated on `direct`. `4708a179`.
|
||||
- [x] **SEC-1 / SEC-2 — scheduled-message plaintext + recent searches survived logout** → `clearPlaintextCaches()` on both logout paths, extended to the whole `recent_*` family + nav-paths. `726cefb5`.
|
||||
- [x] **SEC-3 — `window.open(_blank)` without `noopener`** → `noopener,noreferrer` at 5 sites (SSOStage excluded — needs the handle). `3e1106b2`.
|
||||
- [x] **SEC-4 — `/acl` self-lockout footgun** → shared `serverAcl.ts` validation, no-brick allow default, fail-closed self-ban guard. `3e1106b2`.
|
||||
- [x] **COR-1 — space-child UNLINK over-deleted** → targeted `UNLINK` reducer action. `fd3b8b42`.
|
||||
- [x] **COR-2 — `useCallJoined` stuck true on 2nd-call embed swap** → re-seed on `[embed]`. `1f80d1d1`.
|
||||
- [x] **COR-3 — incoming-call lifetime guard only corrected future clock-skew** → `Math.abs(...)` (±20s). `ab01d27a`.
|
||||
- [x] **COR-4 — shared notify-dedupe slot double-notified** → key by `roomId|threadId`. `1f80d1d1`.
|
||||
- [x] **COR-5 — upload cancel ignored during retry back-off** → `AbortSignal` threaded into the retry loop. `ab01d27a`.
|
||||
- [x] **COR-6 — `CallControl.forceState` dropped `screenshareAudioMuted`** → passes it. `ab01d27a`.
|
||||
|
||||
- [ ] **PERF-1 — Presence: 3 global client listeners registered PER avatar (Sev High · Eff M).** `hooks/useUserPresence.ts:43-53` (via `PresenceRingAvatar.tsx:19`, `MembersDrawer.tsx:142`) — every timeline/member avatar adds `mx.on` for `Presence`/`CurrentlyActive`/`LastPresenceTs` → 100–250 global listeners; each presence event fans out across all, and fast scroll churns add/remove per Message mount. Fix: one shared presence store (context + `useSyncExternalStore` or a jotai `atomFamily`) with 3 listeners total; components select by userId.
|
||||
- [ ] **PERF-2 — `#`-mention autocomplete re-sorts AND mutates the room list every keystroke (Sev Med · Eff S).** `components/editor/autocomplete/RoomMentionAutocomplete.tsx:83` — `useAtomValue(allRoomsAtom).sort(factoryRoomIdByActivity(mx))` is unmemoized (O(N log N) `mx.getRoom` per compare, N=all joined rooms, per keypress) **and `.sort()` mutates the shared `allRoomsAtom` array in place.** Fix: `useMemo(() => [...rooms].sort(...), [rooms, mx])`.
|
||||
- [ ] **PERF-3 — Read-receipt rows attach per-instance global `RoomStateEvent.Members` listeners (Sev Med · Eff M).** `components/read-receipt-avatars/ReadReceiptAvatars.tsx:74` + `hooks/useMemberAvatar.ts:44` — up to ~6 global `Members` listeners per receipt row; fires on any membership/name/avatar change in any room. Fix: one shared room-member-change subscription fanning out by roomId+userId (shares with PERF-1).
|
||||
- [ ] **PERF-4 — Message-search room filter re-sorts full room list every render (Sev Low–Med · Eff S).** `features/message-search/SearchFilters.tsx:155` — `Array.from(...).sort(factoryRoomIdByAtoZ(mx))` outside `useMemo` (re-sorts as the user types). Fix: `useMemo`.
|
||||
- [ ] **PERF-5 — DM-preview hook subscribes a global `Decrypted` listener for EVERY nav item incl. non-DMs (Sev Low–Med · Eff S).** `hooks/useRoomLatestRenderedEvent.ts:64-67` (called at `RoomNavItem.tsx:681`) — preview is only used for `direct` rooms but the hook + its `MatrixEventEvent.Decrypted` listener run for all; wasteful, and fans out across all during bulk decryption. Fix: gate the hook/subscription on `direct`.
|
||||
- [ ] **PERF-6 — Avatar-decoration profile-fetch on first render of large member/timeline views (Sev Low · Eff M, likely leave).** `hooks/useAvatarDecoration.ts` — one `/profile/{userId}` per new user (well-guarded: module cache + in-flight dedupe + backoff). Network/HS-load note only; batch/skip only if it proves costly.
|
||||
**Deferred / decided (not built):**
|
||||
|
||||
**Security / privacy:** _(no exploitable XSS found — `sanitize.ts` strips `javascript:`, forces mxc `<img>` + `rel=noopener` `<a>`; embeds use host-allowlist + origin-scoped `postMessage`; KaTeX `trust:false`. These are storage-hygiene / URL items.)_
|
||||
|
||||
- [ ] **SEC-1 — Scheduled-message plaintext persists in `localStorage`, survives logout (Sev Med · Eff S).** `state/scheduledMessages.ts:12-19` — full `IContent` (incl. E2EE cleartext `body`) under `cinny_scheduled_messages_v1`; normal logout (`removeFallbackSession`, N98) keeps it. Leak on shared/kiosk browser. Fix: clear on logout (add to `removeFallbackSession`) — or encrypt-at-rest with N97.
|
||||
- [ ] **SEC-2 — Recent search terms persist in `localStorage`, survive logout (Sev Low · Eff S).** `state/recentSearches.ts:4-12` — last 10 queries (often PII) under `cinny_recent_searches_v1`, survive normal logout. Fix: clear on logout.
|
||||
- [ ] **SEC-3 — `window.open(url, '_blank')` without `'noopener'` → reverse tab-nabbing (Sev Low · Eff S).** `components/user-profile/UserChips.tsx:117` (+ `SSOStage.tsx:23`, `settings/devices/Verification.tsx:276`, `OtherDevices.tsx:36,49`, `OidcManageAccount.tsx:23`) — `window.open` doesn't null `window.opener`. UserChips opens a user/room-derived server URL. Fix: pass `'noopener,noreferrer'` (or set `w.opener = null`).
|
||||
- [ ] **SEC-4 — `/acl` has no `*`/self-lockout guard or glob validation (Sev Low · Eff S–M).** `hooks/useCommands.ts:489-538` — `/acl -d *` writes `deny:['*']` (can brick the room); empty/whitespace globs pushed unvalidated into `m.room.server_acl`. PL-gated + matches upstream, but a destructive footgun. Fix: reject a `*`/empty deny matching the local server, validate glob syntax, confirm on `*`.
|
||||
- [ ] **SEC-5 — Embeds get `allow-popups-to-escape-sandbox` (Sev Low, informational · Eff M).** `components/url-preview/UrlPreviewCard.tsx:590-591` (`EMBED_SANDBOX`) — a hijacked provider script could spawn an un-sandboxed popup (phishing). Main-app hijack already prevented (no `allow-top-navigation`); popups arguably needed for "open in provider." Fix (if desired): drop the escape flag + verify per-provider.
|
||||
|
||||
**Correctness (under-covered subsystems):**
|
||||
|
||||
- [ ] **COR-1 — Space-child UNLINK nukes a room's OTHER parents + orphans descendants (Sev Med · Eff M).** `state/room/roomToParents.ts:102-103` (DELETE reducer `:53-66`) — removing one `m.space.child` fires the same `DELETE` used for "room left," wiping ALL of the child's parents and stripping it as a parent everywhere. Remove C from space A only → C also loses parent B, and C's own children lose C, until a full resync. Fix: a targeted "remove one parent from one child" action. (Reducer inherited from upstream.)
|
||||
- [ ] **COR-2 — `useCallJoined` stays `true` across a direct embed swap (answering a 2nd call) (Sev Med · Eff S).** `hooks/useCallEmbed.ts:140-158` — the reset effect only clears `joined` when `!embed`; answering call B from the banner swaps embed A→B directly, so `joined` stays true → B renders as live/joined before EC actually joins, skipping the loading/watchdog UI. Fix: `useEffect(() => setJoined(embed?.joined ?? false), [embed])`.
|
||||
- [ ] **COR-3 — Incoming-call lifetime guard only corrects FUTURE clock-skew, not past (Sev Low · Eff S).** `components/CallEmbedProvider.tsx:476-477` — distrusts `sender_ts` only when >20 s ahead of `origin_server_ts`; a caller clock that's behind leaves `sender_ts` in the past → `remaining ≤ 0` → ring auto-dismisses/never shows for a fresh invite. Fix: `Math.abs(...) > 20000` or just use `event.getTs()`.
|
||||
- [ ] **COR-4 — Shared notify-dedupe slot double-notifies a re-fired main message after an interleaving thread reply (Sev Low · Eff S).** `pages/client/ClientNonUIFeatures.tsx:521,530` — `lastNotifiedEventRef` is one `Map<roomId,eventId>` slot shared by the main + thread paths; a thread reply overwrites it, so a re-emitted main message (decrypt/edit re-fire, common in E2EE) mis-dedupes and notifies again. Fix: separate dedupe keys per path (or key by `roomId|eventId`).
|
||||
- [ ] **COR-5 — Upload cancel during retry back-off is silently ignored (upload resurrects) (Sev Low · Eff M).** `utils/matrix.ts:204-238` + `state/upload.ts:125-129` — during the 1–30 s back-off the atom holds the previous (rejected) promise, so `cancelUpload` is a no-op and the loop wakes to start the next attempt. Fix: thread an abort flag into the retry loop.
|
||||
- [ ] **COR-6 — `CallControl.forceState` resets `screenshareAudioMuted` to false (Sev Low · Eff S).** `plugins/call/CallControl.ts:155-162` — `forceState` passes only 5 args so the 6th defaults false; low impact today (runs at join, no screenshare yet) but silently discards the field. Fix: pass `this.screenshareAudioMuted`.
|
||||
- [DEFERRED] **PERF-6 — avatar-decoration `/profile` fetch** — well-guarded (module cache + in-flight dedupe + backoff); a network/HS-load note only. Batch/skip only if it proves costly.
|
||||
- [DEFERRED] **SEC-5 — embeds' `allow-popups-to-escape-sandbox`** — informational; main-app hijack already prevented (no `allow-top-navigation`), and popups are arguably needed for "open in provider." Revisit with per-provider verification if dropped.
|
||||
- **KE-1 preventive (`navigator.storage.persist()`)** is **already implemented** (`initClient` → `requestPersistentStorage()` + `src/index.tsx` boot). The rest of the KE cluster stays under **Encryption / E2EE** below (needs live capture).
|
||||
|
||||
### ✅ Unread/read-receipt flakiness (reported 2026-07) — FIXED (pending prod QA)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user