Skip to content

Commit 0f1f7c2

Browse files
committed
fix: address review feedback for XSS vulnerability fix
- Replace HTML string fallback with React elements to prevent potential XSS - Add error handling for toJsxRuntime conversion with proper fallback - Add explanatory comment about why hast-util-to-jsx-runtime was chosen - Improve test mock to handle transformers and be more comprehensive
1 parent 885c27f commit 0f1f7c2

File tree

2 files changed

+41
-12
lines changed

2 files changed

+41
-12
lines changed

webview-ui/src/components/common/CodeBlock.tsx

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,12 @@ const CodeBlock = memo(
255255
// Set mounted state at the beginning of this effect
256256
isMountedRef.current = true
257257

258-
const fallback = `<pre style="padding: 0; margin: 0;"><code class="hljs language-${currentLanguage || "txt"}">${source || ""}</code></pre>`
258+
// Create a safe fallback using React elements instead of HTML string
259+
const fallback = (
260+
<pre style={{ padding: 0, margin: 0 }}>
261+
<code className={`hljs language-${currentLanguage || "txt"}`}>{source || ""}</code>
262+
</pre>
263+
)
259264

260265
const highlight = async () => {
261266
// Show plain text if language needs to be loaded.
@@ -292,16 +297,25 @@ const CodeBlock = memo(
292297
})
293298
if (!isMountedRef.current) return
294299

295-
// Convert HAST to React elements
296-
const reactElement = toJsxRuntime(hast, {
297-
Fragment,
298-
jsx,
299-
jsxs,
300-
// Don't override components - let them render as-is to maintain exact output
301-
})
300+
// Convert HAST to React elements using hast-util-to-jsx-runtime
301+
// This approach eliminates XSS vulnerabilities by avoiding dangerouslySetInnerHTML
302+
// while maintaining the exact same visual output and syntax highlighting
303+
try {
304+
const reactElement = toJsxRuntime(hast, {
305+
Fragment,
306+
jsx,
307+
jsxs,
308+
// Don't override components - let them render as-is to maintain exact output
309+
})
302310

303-
if (isMountedRef.current) {
304-
setHighlightedCode(reactElement)
311+
if (isMountedRef.current) {
312+
setHighlightedCode(reactElement)
313+
}
314+
} catch (error) {
315+
console.error("[CodeBlock] Error converting HAST to JSX:", error)
316+
if (isMountedRef.current) {
317+
setHighlightedCode(fallback)
318+
}
305319
}
306320
}
307321

webview-ui/src/components/common/__tests__/CodeBlock.spec.tsx

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ vi.mock("../../../utils/highlighter", () => {
3939
}),
4040
codeToHast: vi.fn().mockImplementation((code, options) => {
4141
const theme = options.theme === "github-light" ? "light" : "dark"
42-
// Return a simple HAST node structure that includes the theme marker
43-
return {
42+
// Return a comprehensive HAST node structure that matches Shiki's output
43+
// Apply transformers if provided
44+
const preNode = {
4445
type: "element",
4546
tagName: "pre",
4647
properties: {},
@@ -58,6 +59,20 @@ vi.mock("../../../utils/highlighter", () => {
5859
},
5960
],
6061
}
62+
63+
// Apply transformers if they exist
64+
if (options.transformers) {
65+
for (const transformer of options.transformers) {
66+
if (transformer.pre) {
67+
transformer.pre(preNode)
68+
}
69+
if (transformer.code && preNode.children[0]) {
70+
transformer.code(preNode.children[0])
71+
}
72+
}
73+
}
74+
75+
return preNode
6176
}),
6277
}
6378

0 commit comments

Comments
 (0)