Skip to content

fix(security): sanitize chat history HTML to prevent DOM XSS#366

Open
yashwant938 wants to merge 2 commits into
react-chatbotify:mainfrom
yashwant938:yash-chat-history-dom-xss
Open

fix(security): sanitize chat history HTML to prevent DOM XSS#366
yashwant938 wants to merge 2 commits into
react-chatbotify:mainfrom
yashwant938:yash-chat-history-dom-xss

Conversation

@yashwant938
Copy link
Copy Markdown

@yashwant938 yashwant938 commented Feb 7, 2026 •
edited
Loading

Description

Please include a brief summary of the change and include the relevant issue(s).

Closes #(issue)

What change does this PR introduce?

Please select the relevant option(s).

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (changes to docs/code comments)

What is the proposed approach?

Please give a short overview/explanation on the approach taken to resolve the issue(s).

Checklist:

  • The commit message follows our adopted guidelines
  • Testing has been done for the change(s) added (for bug fixes/features)
  • Relevant comments/docs have been added/updated (for bug fixes/features)

tjtanjin
tjtanjin reviewed Feb 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@tjtanjin tjtanjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @yashwant938, thanks for the PR! This hardening makes sense (storage is untrusted), but it may be a behavioral break for apps that relied on “rich” HTML surviving a persist→rehydrate round-trip (e.g. srcset, inline styles, uncommon URL schemes).

On that note, it might be worth considering having an explicit extension point for backward compatibility, e.g. chatHistorySanitizer?: (html: string) => string (defaulting to the new sanitizer). This keeps the secure default while giving advanced users a more controlled way to loosen/adjust allowlists without forking. Any thoughts on this?

@yashwant938
Copy link
Copy Markdown
Author

yashwant938 commented Feb 7, 2026 •
edited
Loading

Hey @tjtanjin Thanks for pointing that out. I agree it could break some use cases.

I added an option settings.chatHistorySanitizer?: (html: string) => string. By default it uses the safe sanitizer from this PR, but if someone needs to keep things like srcset or custom URL schemes, they can provide their own sanitizer. I also added a test for this.

Let me know if you’d prefer this option to live under settings.chatHistory instead of at the top level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tjtanjin tjtanjin tjtanjin left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yashwant938 @tjtanjin