Skip to content

PluginUpdate: disable inline HTML in markdown renderer to prevent HTML injection#1447

Merged
Trouffman merged 12 commits into
masterfrom
copilot/fix-release-notes-html-escaping
Apr 23, 2026
Merged

PluginUpdate: disable inline HTML in markdown renderer to prevent HTML injection#1447
Trouffman merged 12 commits into
masterfrom
copilot/fix-release-notes-html-escaping

Conversation

Copilot AI commented Apr 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Use QTextDocument::MarkdownNoHTML flag via document()->setMarkdown() — prevents HTML injection at the parser level without any pre-escaping
  • Added #include <QTextDocument> so QTextDocument::MarkdownFeatures enum values are visible to the compiler
  • Explicitly construct QTextDocument::MarkdownFeatures(...) to fix no viable conversion from 'int' to 'MarkdownFeatures' compiler error
  • Add Chinese, Hindi (Devanagari), and Thai Unicode test lines to mock release notes
  • Fix garbled Unicode rendering: replace \xNN UTF-8 byte sequences with \uNNNN Unicode code point escapes (portable, compiler-independent), and add blank lines between language paragraphs so each renders as a distinct paragraph
  • Run validation (Code Review ✅, CodeQL ✅)

Copilot AI linked an issue Apr 21, 2026 that may be closed by this pull request
PluginUpdate: Ensure releaseNotes are HTML-escaped before displaying as markdown #1446
Closed
8 tasks
Copilot AI changed the title [WIP] Fix HTML escaping for release notes in markdown display PluginUpdate: HTML-escape releaseNotes before rendering as markdown Apr 21, 2026
Copilot AI requested a review from paulpv April 21, 2026 18:59
@paulpv
paulpv marked this pull request as ready for review April 21, 2026 19:01
Copilot AI review requested due to automatic review settings April 21, 2026 19:01
Copilot AI reviewed Apr 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR mitigates a potential HTML/XSS injection vector in the plugin update dialog by ensuring untrusted release notes are not interpreted as inline HTML by Qt’s markdown rendering path.

Changes:

  • HTML-escape pluginUpdateInfo.releaseNotes before rendering it via QTextBrowser::setMarkdown().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/forms/update.cpp Outdated
@paulpv paulpv added the !DO NOT MERGE! For PRs that should not be merged (incomplete or just PoC) label Apr 21, 2026
@paulpv
paulpv marked this pull request as draft April 21, 2026 22:59
Copilot AI changed the title PluginUpdate: HTML-escape releaseNotes before rendering as markdown PluginUpdate: disable inline HTML in markdown renderer to prevent HTML injection Apr 22, 2026
@paulpv paulpv removed the !DO NOT MERGE! For PRs that should not be merged (incomplete or just PoC) label Apr 23, 2026
@paulpv

paulpv commented Apr 23, 2026

Copy link
Copy Markdown
Member

@Trouffman @BitRate27 Ready for your review.

The mock hostile input renders much better now:
image

CC @RayneYoruka

@paulpv
Update: add debug test dialog to exercise releaseNotes HTML-escaping
dc25290 
Add a debug-only button in OutputSettings that opens a synthetic PluginUpdate
dialog populated with deliberately hostile releaseNotes (raw HTML, script tags,
javascript: links, etc.). The new showSampleUpdateDialog() helper constructs a
fake PluginUpdateInfo and shows the PluginUpdate UI so developers can visually
verify that markdown rendering properly escapes/handles unsafe input.

The button is only created when LOG_LEVEL >= LOG_DEBUG; the helper itself is
unrestricted (gating is done at the caller). Also add the function prototype to
update.h and include QPushButton where needed.
@paulpv
paulpv force-pushed the copilot/fix-release-notes-html-escaping branch from cb6030d to dc25290 Compare April 23, 2026 01:26
paulpv
paulpv requested changes Apr 23, 2026
View reviewed changes

@paulpv paulpv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot Also add some Chinese, Indian, and Thai characters to the mock release notes text

Copilot AI commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Also add some Chinese, Indian, and Thai characters to the mock release notes text

Added in 977c9aa. Three new lines in the "Character edge cases" section of the mock notes:

  • Chinese: 中文测试 欢迎使用 发布说明
  • Hindi (Devanagari): हिन्दी परीक्षण स्वागत
  • Thai: ไทย ทดสอบ ยินดีต้อนรับ

Copilot AI requested a review from paulpv April 23, 2026 01:32
paulpv
paulpv requested changes Apr 23, 2026
View reviewed changes

@paulpv paulpv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot The "Character edge cases" section looks pretty bad:

Image

Investigate why and get it rendering correctly.

Copilot AI commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

@copilot The "Character edge cases" section looks pretty bad:

Image...

Fixed in c4f1c3a. Two issues were causing the bad rendering:

  1. Garbled characters: \xNN hex escapes in QStringLiteral are treated as individual Latin-1 code points (U+00NN), not as UTF-8 byte sequences — so e.g. \xe4\xb8\xad became 中 instead of 中. Replaced all with \uNNNN Unicode code point escapes which QStringLiteral handles correctly across all compilers/platforms.

  2. Run-on paragraph: Markdown requires a blank line between paragraphs; a bare \n is just a line break within the same paragraph. Added "\n" separators between each language line so they each render as their own paragraph.

The result should now show:

  • Unicode/emoji sanity: é ñ 中文 🚀 🔥
  • Chinese (Simplified): 中文测试 欢迎使用 发布说明
  • Hindi (Devanagari): हिन्दी परीक्षण स्वागत
  • Thai: ไทย ทดสอบ ยินดีต้อนรับ

Copilot AI requested a review from paulpv April 23, 2026 01:38
@paulpv
PluginUpdate: add explicit Markdown hard breaks to sample release notes
39e7e6c 
Ensure the debug sample update dialog uses Markdown hard line breaks (trailing two spaces + newline)
so the mock releaseNotes render with the intended line breaks when testing HTML-escaping/formatting.
@paulpv

paulpv commented Apr 23, 2026

Copy link
Copy Markdown
Member

@Trouffman @BitRate27 Added Chinese, Indian, Thai text per Trouffman's request:

image

CC @RayneYoruka

@paulpv
paulpv marked this pull request as ready for review April 23, 2026 01:46
@Trouffman
Trouffman self-requested a review April 23, 2026 12:44
Trouffman
Trouffman approved these changes Apr 23, 2026
View reviewed changes

@Trouffman Trouffman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should be good, Only possible flag is that the code for the testing purpose might trigger some anti-malware scans.

@Trouffman
Trouffman merged commit 0f5fda2 into master Apr 23, 2026
9 checks passed
@Trouffman
Trouffman deleted the copilot/fix-release-notes-html-escaping branch April 23, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@Trouffman Trouffman Trouffman approved these changes
+1 more reviewer
@paulpv paulpv paulpv approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@paulpv paulpv

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PluginUpdate: Ensure releaseNotes are HTML-escaped before displaying as markdown

4 participants

@paulpv @Trouffman