Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #374 and #409 - Prevent XSS and add Markdown support #448

Merged
merged 10 commits into from
May 5, 2023
Merged

Fix #374 and #409 - Prevent XSS and add Markdown support #448

merged 10 commits into from
May 5, 2023

Conversation

aerovulpe
Copy link
Member

@aerovulpe aerovulpe commented Apr 19, 2023

Fixes the XSS vulnerability by handling three different formatting modes we support separately.

A new helper parse-message.js was added to utils that uses the popular battle-tested unified library with plugins for HTML and Markdown to sanitize and translate Markdown to HTML, adding full Markdown support.

The default formatting mode no longer uses v-html to dynamically add arbitrary HTML to the DOM, however, users can still customize messages with dynamic HTML, opting in using text-formatting='{ "html": true }'. The injected HTML is first sanitized to remove dangerous elements. text-formatting='{ "markdown": true }' was similarly added to allow users to use CommonMark-compliant Markdown.

The default formatting functions in FormatMessage.vue were refactored into parse-message.js to consolidate the parsing logic.

Note these changes also set the groundwork to address #409.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • All tests are passing

@antoine92190 antoine92190 changed the title fix #374 Fix #374 - Prevent XSS Apr 23, 2023
@antoine92190
Copy link
Collaborator

antoine92190 commented Apr 23, 2023

Note: I implemented prettier in the project, if you can please use it so we keep the code format consistent

@aerovulpe
Copy link
Member Author

Note: I implemented prettier in the project, if you can please use it so we keep the code format consistent

Hey @antoine92190, I've run prettier for the files changed in this PR. ✅

@aerovulpe
Copy link
Member Author

Also, happy to discuss the unified dependency and how it could play into Markdown support and #409.

The goal is to simplify format-string.js. Specifically compileToJSON. This method basically compiles Markdown into a JSON AST. We can use remark-parse to parse a richer mdast AST and process that in compileToHTML. This would minimize changes to the styling layer with the benefit of a standardized grammar for Markdown.

@aerovulpe aerovulpe changed the title Fix #374 - Prevent XSS Fix #374 and #409 - Prevent XSS and add Markdown support Apr 28, 2023
@antoine92190 antoine92190 merged commit 7a30bd5 into advanced-chat:master May 5, 2023
1 check passed
@aerovulpe aerovulpe deleted the fix/xss branch May 5, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants