From d7d261a0191fbd5ce7794e0c61b6991a7503d1ea Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Wed, 8 Jul 2026 01:11:42 -0400 Subject: [PATCH] =?UTF-8?q?docs(todo):=20add=20Discovery=20pass=202=20?= =?UTF-8?q?=E2=80=94=20PERF/SEC/COR=20findings=20(agent-surveyed)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three agents swept fresh lenses (performance, security/privacy, correctness in under-covered subsystems). 16 verified items filed: PERF-1..6, SEC-1..5 (no exploitable XSS found β€” sanitization surface is hardened), COR-1..6. Single-pass, not yet TPVR'd. Co-Authored-By: Claude Opus 4.8 --- LOTUS_TODO.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/LOTUS_TODO.md b/LOTUS_TODO.md index 118d3750b..b93b99361 100644 --- a/LOTUS_TODO.md +++ b/LOTUS_TODO.md @@ -67,6 +67,36 @@ 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) + +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). + +**Performance / rendering:** + +- [ ] **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. + +**Security / privacy:** _(no exploitable XSS found β€” `sanitize.ts` strips `javascript:`, forces mxc `` + `rel=noopener` ``; 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` 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`. + ### βœ… Unread/read-receipt flakiness (reported 2026-07) β€” FIXED (pending prod QA) Room unread dots were inconsistent: reading a message sometimes cleared the dot, sometimes left it stuck, sometimes it resurrected. Root cause (confirmed by tracing + diffing upstream cinny `dev`): **our own "N4" change.** `handleReceipt` recomputed via `getUnreadInfo`, which reads `room.getUnreadNotificationCount()` β€” server-computed and **stale on the synchronous synthetic receipt echo** (SDK only zeroes it immediately when the last event is your own message) β†’ it PUT the stale non-zero count back β†’ stuck/resurrecting. Compounded by `hasUnread = !!unread` lighting the dot on any present map entry, incl. phantom `{0,0}` PUTs from our `UnreadNotifications` listener. Plus a Mark-as-Unread (MSC2867) flag that never cleared on opening an already-read room (no receipt β†’ no auto-clear).