Fix unsanitized HTML in reply renderer#569
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a security gap in the reply preview renderer by sanitizing Matrix “formatted_body” HTML before it is parsed/rendered, and adds regression tests to prevent quote/XSS regressions.
Changes:
- Add
sanitizeReplyFormattedPreview()to strip reply quotes / block formatting and sanitize HTML prior to parsing. - Update the reply renderer to parse the sanitized preview HTML instead of raw stripped HTML.
- Add Vitest coverage for reply preview quote stripping and XSS sanitization behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/app/components/message/Reply.tsx |
Introduces and uses a helper to sanitize reply preview HTML before html-react-parser runs. |
src/app/components/message/ThreadIndicator.test.tsx |
Adds regression tests for reply preview sanitization alongside existing ThreadIndicator visibility tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
hey it seems like you're missing a changeset. Can you please add a changeset, as described here https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md#documenting-a-change Thank you :3 |
hazre
left a comment
There was a problem hiding this comment.
that's a clean fix. lgtm in terms of code but havent tested it.
> [!IMPORTANT] > Merging this PR will create a new release. ## Features * Add ability to click on usernames in member and state events to view user info ([#536](#536) by @thundertheidiot) * Add black theme ([#437](#437) by @Elec3137) * added a limited compatibility with `pk;member` commands ([#550](#550) by @dozro) * Add /location sharing command, and a /sharemylocation command. ([#509](#509) by @nushea) * added option to use shorthands to send a message with a Persona, for example `✨:test` ([#550](#550) by @dozro) * Add quick reply keybinds by using <kbd>ctrl</kbd>+<kbd>up</kbd> / <kbd>ctrl</kbd>+<kbd>down</kbd> you can now cycle through the message you are replying to with keybinds ([#524](#524) by @CodeF53) * Adds a `/html` command to send HTML messages ([#560](#560) by @Vespe-r) * Add room abbreviations with hover tooltips: moderators define term/definition pairs in room settings; matching terms are highlighted in messages. ([#514](#514) by @Just-Insane) * Add support for timestamps, playlists and youtube music links for the youtube embeds ([#534](#534) by @thundertheidiot) * Add settings sync across devices via Matrix account data, with JSON export/import ([#515](#515) by @Just-Insane) ## Fixes * Add detailed error messages to forwarding failures. ([#532](#532) by @7w1) * Cap unread badge numbers at `1k+`, and something extra :) ([#484](#484) by @hazre) * Fix scroll-to-bottom after room navigation, timeline pagination reliability, and URL preview deduplication. ([#529](#529) by @Just-Insane) * Fixes the most recent pmp message in encrypted rooms not consistently rendering the pmp and not grouping with previous pmps. ([#526](#526) by @7w1) * fixed sending sticker and attachments while having a persona selected ([#525](#525) by @dozro) * Fix push notifications missing sender/room avatar and showing stale display names when using event_id_only push format. ([#551](#551) by @Just-Insane) * Sanitize formatted reply previews before rendering to prevent unsafe HTML from being parsed in reply snippets. ([#569](#569) by @Just-Insane) * Fix broken link to Sliding Sync known issues — now points to #39 instead of the old repository. ([#519](#519) by @Just-Insane) * Fix service worker authenticated media requests returning 401 errors after SW restart or when session data is missing/stale. ([#516](#516) by @Just-Insane) * rephrased the command describtion for `/usepmp` and made `/usepmp reset` actually reset the room association of the pmp ([#550](#550) by @dozro) * Fix confusing ui with `Client Side Embeds in Encrypted Rooms` setting ([#535](#535) by @thundertheidiot) * fix forwarding metadata by removing the `null` value ([#540](#540) by @dozro) * fix forwarding issue for users on synapse homeservers, by removing the relation ([#558](#558) by @dozro) * fixed the syntax issues regarding `/addpmp` and `usepmp` (note that the syntax for `/usepmp` has changed) ([#550](#550) by @dozro) * fix the display of jumbo emojis on messages sent with a persona ([#530](#530) by @dozro) * Fix sidebar notification badge positioning so unread and unverified counts align consistently. ([#484](#484) by @hazre) * Use the browser's native compact number formatting for room and member counts. ([#484](#484) by @hazre) * fix(sentry): scrub percent-encoded Matrix IDs and opaque base64url tokens from Sentry URLs ([#531](#531) by @Just-Insane) ## Notes * new/changed bios will now also be saved in the format MSC4440 expects ([#559](#559) by @dozro) * moved the setting for filtering pronouns by language from experimental to the appearance setting ([#521](#521) by @dozro)
Description
Fix reply preview rendering so formatted reply content is sanitized before React HTML parsing.
Fixes #
Type of change
Checklist:
AI disclosure:
AI-assisted parts:
sanitizeReplyFormattedPreview()inReply.tsxso formatted reply previews are always sanitized before parsing.ThreadIndicator.test.tsxfor quote stripping and sanitizer behavior.