Skip to content

fix: centralize chat markdown rendering - #181

Merged
mbakgun merged 1 commit into
heymrun:mainfrom
Rohithmatham12:fix/markdown-rendering-safety
Jun 12, 2026
Merged

fix: centralize chat markdown rendering#181
mbakgun merged 1 commit into
heymrun:mainfrom
Rohithmatham12:fix/markdown-rendering-safety

Conversation

@Rohithmatham12

@Rohithmatham12 Rohithmatham12 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reuse the shared sanitized markdown renderer in portal chat, dashboard chat, and main chat conversation surfaces
  • Remove duplicated markdown sanitizer configs that could drift across chat UIs
  • Drop the unnecessary style attribute from the shared user-markdown allowlist
  • Copy rendered plaintext, not raw markdown, from exported portal HTML assistant messages
  • Escape JSON embedded in exported portal chat HTML so message text cannot break out of the inline script

Testing

  • bun run lint:check
  • bun run typecheck
  • bun run build
  • git diff --check
  • uv run ruff check .

@mbakgun

mbakgun commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Thanks for cleaning this up @Rohithmatham12 👑 Centralizing the markdown rendering is a good idea and should reduce sanitizer drift across the chat surfaces.

Could you also fix one remaining security issue in the portal chat HTML export path?

plainTexts is embedded directly inside an inline <script> via JSON.stringify(plainTexts). If a message contains </script><script>...</script>, the browser HTML parser can close the script tag early and execute injected script in the exported HTML.

This looks pre-existing rather than introduced by this PR, but since this PR already touches the export plaintext behavior, it would be great to handle it here too. Please escape script-breaking sequences before embedding JSON into the inline script, for example by replacing <, >, &, U+2028, and U+2029, or move the data into a safer non-executable container such as a JSON script tag with properly escaped text content.

@Rohithmatham12
Rohithmatham12 force-pushed the fix/markdown-rendering-safety branch from 8f27fbf to 778a9ac Compare June 11, 2026 22:26

Copy link
Copy Markdown
Contributor Author

Thanks for catching that. I updated the export path to escape JSON before embedding it in the inline script, covering <, >, &, U+2028, and U+2029. I also reran lint, typecheck, build, and diff checks.

@mbakgun
mbakgun merged commit da0e03a into heymrun:main Jun 12, 2026
1 check passed
@mbakgun

mbakgun commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Thank you 💯

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants