docs(bugs): mark N100/N106/N109/N119 FIXED
CI / Build & Quality Checks (push) Failing after 30m49s
CI / Trigger Desktop Build (push) Has been skipped

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-28 12:35:35 -04:00
parent 51d468fbcc
commit 70ffd252bd
+4 -4
View File
@@ -531,7 +531,7 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
**N106 — Decrypted E2EE message plaintext leaked to OS notification center**
- **File:** `src/app/pages/client/ClientNonUIFeatures.tsx`, line 343
- **Status:** **OPEN** [Claude_Found]
- **Status:** **FIXED** (`51d468fb`) — OS notification body for encrypted rooms shows sender only (no decrypted plaintext); in-page toast still previews while focused.
- **Issue:** The `MessageNotifications` component passes `mEvent.getContent().body` directly as the notification body: `body: (mEvent.getContent().body as string | undefined) ?? ''`. By the time `RoomEvent.Timeline` fires, `matrix-js-sdk` has already decrypted the event in memory. The fully decrypted plaintext is then handed to `new window.Notification()`, which stores it in the OS notification center. This plaintext is visible on the device lock screen (if notification previews are enabled), in the OS notification history, and may be read by any app with `READ_NOTIFICATIONS` permission (e.g., accessibility services, backup apps) — even when the room uses end-to-end encryption. The 120-character slice (`slice(0, 120)`) does not mitigate this.
- **Root Cause:** No distinction is made between encrypted and unencrypted rooms when constructing notification bodies. There is no check such as `mEvent.isEncrypted()` or `room.hasEncryptionStateEvent()` that would substitute a generic body.
- **Fix:** Check whether the room is encrypted before populating the body. For encrypted rooms, use a generic string (e.g., `"New encrypted message"`) as the body instead of the decrypted content. If message previews in notifications are intentionally desired by the user, gate them behind an explicit opt-in setting that warns about OS-level plaintext exposure.
@@ -561,7 +561,7 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
**N109 — Authenticated media URLs passed to `Notification` icon/badge: OS cannot fetch them (produces 401)**
- **File:** `src/app/pages/client/ClientNonUIFeatures.tsx`, lines 333339 and 270273
- **Status:** **OPEN** [Claude_Found]
- **Status:** **FIXED** (`51d468fb`) — OS notification icon/badge use static LogoSVG instead of an authenticated avatar URL the OS can't fetch; toast keeps the room avatar.
- **Issue:** When the homeserver requires authenticated media (Matrix spec v1.11+, path `/_matrix/client/v1/media/download/...`), `mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop')` returns an authenticated URL. That URL is then passed directly as `icon` and `badge` to `new window.Notification()`. The OS/browser notification subsystem fetches `icon` and `badge` URLs directly — outside the page's JavaScript context — so the service worker's `fetch` handler never fires for them (the SW only intercepts fetches with a valid `event.clientId`, which these OS-initiated fetches lack). The homeserver returns HTTP 401, and the notification shows no icon or badge.
- **Root Cause:** The SW auth-header injection is designed for page-initiated `/_matrix/client/v1/media/` fetches. It does not (and cannot) intercept fetches made by the OS notification subsystem. Room avatar URLs are passed to `Notification` without first converting them to an auth-agnostic form.
- **Fix:** Before creating a `Notification`, fetch the avatar URL in-page (via the existing authenticated fetch path where the SW can inject headers), convert the response to a Blob URL (`URL.createObjectURL(blob)`), and pass the Blob URL as `icon`/`badge`. Alternatively, skip the avatar for notifications entirely and use the static app logo (already done for invite notifications via `LogoSVG`) to avoid the authenticated-media complexity.
@@ -639,7 +639,7 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
**N119 — `syncDecorations.mjs` Treats Network Errors the Same as 404 — CDN Outage Silently Wipes Entire Catalog**
- **File:** `scripts/syncDecorations.mjs`, lines 3946, 5665
- **Status:** **OPEN** [Claude_Found]
- **Status:** **FIXED** (`51d468fb`) — distinguishes confirmed 404 (remove) from network/5xx (abort) so a CDN outage can't wipe the catalog.
- **Issue:** `headCheck` catches all fetch exceptions (DNS failure, timeout, CORS error, TLS failure) and returns `{ ok: false, status: 0 }`. This is structurally identical to an HTTP 404 (`{ ok: false, status: 404 }`). The script classifies all non-ok results as "missing" and removes them from `avatarDecorations.ts`. If `drive.lotusguild.org` is temporarily unreachable when a developer runs `npm run sync:decorations`, every single decoration fails the HEAD check with `status: 0`, is marked missing, and is removed. The script writes an empty `avatarDecorations.ts`, logs "Done. Removed N entries from the catalog.", and exits 0 — permanently destroying the catalog in source control with no warning.
- **Root Cause:** The `catch` block does not distinguish transient network failures from confirmed HTTP 404 responses.
- **Fix:** Return a distinct value for network errors (e.g. `{ slug, ok: false, status: 0, networkError: true }`). Before writing the updated catalog, abort with `process.exit(1)` if any result has `networkError: true` — the CDN may be unreachable and removing all entries would be data loss. Only entries with a confirmed `status: 404` (file genuinely absent from the CDN) should be removed.
@@ -737,7 +737,7 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
**N100 — `sanitize.ts` allows unrestricted CSS class names on `<pre>` elements; `allowedClasses` not configured for `pre`**
- **File:** `src/app/utils/sanitize.ts`, lines 69 and 156163
- **Status:** **OPEN** [Claude_Found]
- **Status:** **FIXED** (`51d468fb`) — added `pre: ['language-*']` to sanitize-html allowedClasses.
- **Issue:** `permittedTagToAttributes` includes `pre: ['data-md', 'class']` (line 69), permitting the `class` attribute on `<pre>` elements in Matrix `formatted_body` messages. However, `allowedClasses` (lines 156163) restricts class names only for `code` elements (`language-*` patterns for Prism syntax highlighting). Per `sanitize-html` documentation: when `class` is listed in `allowedAttributes` for a tag but that tag has no entry in `allowedClasses`, **all class names are permitted** on that element. This allows a remote message sender to inject arbitrary class names onto `<pre>` blocks — e.g. `<pre class="some-cinny-class admin-notice">` — which could activate site-specific or folds-generated CSS rules keyed to those class names, override visual styling, or trigger `::before`/`::after` pseudo-element content defined in any loaded stylesheet. By contrast, the `code` element (which is typically the inner child of `<pre>`) is correctly restricted to `language-*` only, making the `pre` oversight inconsistent.
- **Root Cause:** When Prism syntax-highlighting class support was added for `<code>`, the `<pre>` element was given a `class` passthrough (to allow `<pre class="language-python">` wrappers) but no corresponding `allowedClasses` whitelist entry was added for it.
- **Fix:** Add `pre` to `allowedClasses` with the same `language-*` pattern already used for `code`: