Skip to content

Comments

fix(update): render raw HTML in release notes via rehype-raw#864

Open
kaizhou-lab wants to merge 1 commit intomainfrom
fix/update-modal-raw-html-display
Open

fix(update): render raw HTML in release notes via rehype-raw#864
kaizhou-lab wants to merge 1 commit intomainfrom
fix/update-modal-raw-html-display

Conversation

@kaizhou-lab
Copy link
Collaborator

Closes #655

Summary

  • Add an opt-in allowHtml prop to MarkdownView that enables the rehype-raw plugin, allowing raw HTML tags in markdown to be rendered as actual HTML elements
  • Apply allowHtml in UpdateModal where the content source is the project's own GitHub releases (trusted)

Root Cause

GitHub release notes often contain inline HTML (e.g. <img> tags for screenshots). react-markdown v10 does not render raw HTML by default — without the rehype-raw plugin, these tags are displayed as plain text, which is what users saw in the update dialog.

Test plan

  • Open "Check for Updates" when a newer release is available
  • Verify release notes render correctly, including embedded images and other HTML elements
  • Verify normal AI conversation markdown rendering is unaffected (no allowHtml = no change)

GitHub release notes often contain inline HTML tags (e.g. <img>).
react-markdown v10 does not render raw HTML by default, causing these
tags to appear as plain text in the update modal.

Add an opt-in `allowHtml` prop to MarkdownView that enables the
rehype-raw plugin for trusted content. Apply it in UpdateModal where
the source is the project's own GitHub releases.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code Review

CRITICAL Issues

1. Raw HTML rendering enables XSS in a security-sensitive surface (release notes)

File: src/renderer/components/Markdown.tsx:531-571

<ReactMarkdown
  remarkPlugins={[remarkGfm, remarkMath, remarkBreaks]}
  rehypePlugins={allowHtml ? [rehypeRaw, rehypeKatex] : [rehypeKatex]}
  components={{
    // ...
  }}
>
  {normalizedChildren}
</ReactMarkdown>

Problem: Enabling rehype-raw causes raw HTML in markdown to be parsed into real DOM nodes. Without an explicit sanitization step, this is a classic XSS vector. Even if the intended source is “trusted” (GitHub releases), the app is still rendering remote content inside an Electron renderer. A compromised GitHub account/repo, a malicious release author, or any upstream content injection would allow arbitrary HTML injection. At minimum this can lead to phishing UI inside the modal; depending on the app’s Electron hardening and what the renderer can access via IPC, it can become more serious.

react-markdown’s security model is: raw HTML is disabled by default; if you enable it, you should also sanitize (e.g., rehype-sanitize) with a strict allowlist.

Fix: Add sanitization when allowHtml is enabled, and restrict allowed tags/attributes to what you actually need (e.g., img, a, basic formatting). Example:

import rehypeRaw from 'rehype-raw';
import rehypeSanitize, { defaultSchema } from 'rehype-sanitize';

const releaseNotesSchema = {
  ...defaultSchema,
  tagNames: [
    ...(defaultSchema.tagNames ?? []),
    'img',
  ],
  attributes: {
    ...defaultSchema.attributes,
    img: ['src', 'alt', 'title', 'width', 'height'],
    a: ['href', 'title', 'target', 'rel'],
  },
};

<ReactMarkdown
  remarkPlugins={[remarkGfm, remarkMath, remarkBreaks]}
  rehypePlugins={
    allowHtml
      ? [rehypeRaw, [rehypeSanitize, releaseNotesSchema], rehypeKatex]
      : [rehypeKatex]
  }
/>

Also consider explicitly disallowing dangerous URL schemes in img/src and a/href (e.g., javascript:), even if sanitize should handle most of it.


HIGH Issues

2. allowHtml is enabled for remote content without any trust boundary enforcement

File: src/renderer/components/UpdateModal/index.tsx:214-224

<div className='text-13px text-t-secondary leading-relaxed'>
  <MarkdownView allowHtml>{updateInfo.body}</MarkdownView>
</div>

Problem: The PR description says the source is “the project's own GitHub releases (trusted)”, but the code does not enforce that trust boundary. updateInfo.body is still remote content. If update feeds ever change (mirror, self-hosted, prerelease channels, forks, etc.), this becomes an easy regression into rendering untrusted HTML.

Fix: Gate allowHtml behind an explicit “trusted source” check at the point of use (or in the update fetching layer), e.g. only enable when the release URL/owner/repo matches a hardcoded allowlist, or when the update metadata is signed/verified. If you can’t reliably verify trust, keep allowHtml off and instead support images via markdown (![]()) or a controlled subset via sanitization (preferred).


Summary

Level Count
CRITICAL 1
HIGH 1
MEDIUM 0

🤖 This review was generated by AI and may contain inaccuracies. Please focus on issues you agree with and feel free to disregard any that seem incorrect. Thank you for your contribution!

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.

[Bug]: 检查更新,显示html代码

1 participant