security(qa): sanitize assistant markdown rendering in QA panel by H-Chris233 · Pull Request #241 · Open-Less/openless

H-Chris233 · 2026-05-04T09:43:00Z

User description

Summary

address security(qa): 净化 QA assistant markdown，避免 WebView XSS 读取凭据 #221 by routing QA assistant markdown through a shared safe renderer
escape raw HTML and restrict link/image protocols to safe allowlisted schemes
apply the same rendering path to streaming and final assistant answers
harden fallback path so render failures never inject raw model output into dangerouslySetInnerHTML
add minimal regression checks for malicious markdown/html and safe markdown rendering

Validation

npm run build (openless-all/app)

Closes #221

PR Type

Bug fix, Tests

Description

Centralize markdown sanitization in a shared renderer
Escape raw HTML and restrict link protocols
Unify streaming and final answer rendering paths
Harden error fallback to prevent unsafe injection

Diagram Walkthrough

flowchart LR
  QA["QaPanel assistant & streaming bubbles"] -- use --> RM["renderQaMarkdown()"]
  RM -- sanitizes via --> CR["Custom marked.Renderer"]
  CR -- escapes HTML, restricts protocols --> OUT["Safe HTML"]
  QA -- fallback on error --> PT["renderQaPlainText()"]
  PT -- outputs --> PRE["<pre> inert text </pre>"]

File Walkthrough

Relevant files

Bug fix

qaMarkdown.ts `Add safe QA markdown renderer with XSS guards` openless-all/app/src/lib/qaMarkdown.ts Escape raw HTML with character replacements Normalize and allowlist link/image protocols Override marked renderer for html, link, image Export renderQaMarkdown and renderQaPlainText functions	+65/-0
QaPanel.tsx `Adopt safe markdown renderer in QaPanel` openless-all/app/src/pages/QaPanel.tsx Replace direct marked.parse with renderQaMarkdown Use renderQaPlainText for error fallback instead of raw content Remove redundant marked.setOptions call	+5/-7

Tests

qaMarkdown.test.ts `Add regression tests for markdown sanitization` openless-all/app/src/lib/qaMarkdown.test.ts Test that raw HTML and script tags are escaped Test that dangerous hrefs like javascript: are stripped Test that safe markdown (bold, lists, code, link) renders correctly Verify code snippets are not double-escaped	+34/-0

Issue Open-Less#221 requires closing the QA markdown XSS surface without broad CSP or architecture changes. This switches assistant and streaming markdown to one shared safe renderer that escapes raw HTML and rejects non-http(s)/mailto/tel links. Constraint: Keep markdown readability while preventing executable HTML in QA WebView Rejected: Add DOMPurify dependency | avoid new dependency for minimal fix Confidence: medium Scope-risk: narrow Directive: Keep streaming and final assistant rendering on the same sanitizer path Tested: npm run build (openless-all/app) Not-tested: Manual malicious payload interaction in packaged desktop app

Two parallel reviews flagged a high-risk fallback path and a link-encoding regression in the new QA sanitizer. This patch keeps the minimal Open-Less#221 approach while ensuring failure fallback stays inert and safe links preserve query strings correctly. Constraint: Preserve minimal change scope for issue Open-Less#221 Rejected: Expand into CSP/capability hardening in same patch | out of issue scope and higher blast radius Confidence: high Scope-risk: narrow Directive: Never return raw model output into dangerouslySetInnerHTML fallback paths Tested: npm run build (openless-all/app) Not-tested: Manual desktop exploit reproduction on Windows/macOS

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7edd0cde9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-04T09:46:24Z

+  const escaped = escapeHtml(markdown);
+  return marked.parse(escaped, {
+    async: false,
+    gfm: true,


Preserve hard line breaks in QA markdown renderer

This refactor removed the previous marked.setOptions({ breaks: true }) behavior and the new marked.parse call does not set breaks, so single newlines in assistant output are no longer rendered as line breaks. In QA responses (especially streamed text and lightly formatted lists), this collapses content into denser paragraphs and is a user-visible regression from prior behavior in QaPanel.tsx.

Useful? React with 👍 / 👎.

Code review flagged a UX regression after sanitizer refactor: assistant single newlines stopped rendering as hard breaks. Re-enable marked breaks mode in the shared QA renderer to preserve previous panel readability for streamed and lightly formatted answers. Constraint: Keep sanitizer fix while matching prior QA markdown display behavior Rejected: Add newline post-processing in QaPanel | duplicates renderer concern and increases drift risk Confidence: high Scope-risk: narrow Directive: QA markdown display flags must be centralized in qaMarkdown.ts to keep streaming/final parity Tested: npm run build (openless-all/app) Not-tested: Manual visual QA panel check on desktop runtime

chatgpt-codex-connector · 2026-05-04T09:48:15Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

github-actions · 2026-05-04T10:00:58Z

PR Reviewer Guide 🔍

(Review updated until commit `806fa26`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 221 - Fully compliant Compliant requirements: Sanitize QA assistant markdown before rendering it in the WebView. Prevent raw HTML, event attributes, and script-capable content from becoming executable DOM. Use the same safe rendering path for both streaming and final assistant answers. Preserve normal markdown features such as paragraphs, lists, code blocks, bold text, and safe links. Add a minimal regression test covering malicious markdown/HTML and safe markdown rendering.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

github-actions · 2026-05-04T10:05:23Z

Persistent review updated to latest commit d397f92

Review feedback found that pre-escaping the entire markdown payload caused double-encoding in inline/fenced code, making HTML/JSON/XML examples unreadable. Switch to raw markdown parsing with renderer-level HTML token escaping so code formatting stays readable while raw HTML remains inert. Constraint: Maintain issue Open-Less#221 security boundary without broad sanitizer rewrite Rejected: Keep full-string pre-escape | breaks code snippet readability Confidence: high Scope-risk: narrow Directive: Escape only raw HTML tokens, not the whole markdown source Tested: npm run build (openless-all/app) Not-tested: Manual QA panel rendering across all markdown edge cases

chatgpt-codex-connector · 2026-05-04T10:07:10Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

github-actions · 2026-05-04T10:21:38Z

Persistent review updated to latest commit 806fa26

H-Chris233 added 2 commits May 4, 2026 17:21

chatgpt-codex-connector Bot reviewed May 4, 2026

View reviewed changes

H-Chris233 self-assigned this May 4, 2026

github-actions Bot added the Review effort 2/5 label May 4, 2026

H-Chris233 merged commit 5d15f1d into Open-Less:main May 4, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

security(qa): sanitize assistant markdown rendering in QA panel#241

security(qa): sanitize assistant markdown rendering in QA panel#241
H-Chris233 merged 4 commits into
Open-Less:mainfrom
H-Chris233:main

H-Chris233 commented May 4, 2026 •

edited by github-actions Bot

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

H-Chris233 commented May 4, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Summary

Validation

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 4, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 806fa26)

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

H-Chris233 commented May 4, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented May 4, 2026 •

edited

Loading

(Review updated until commit `806fa26`)