fix: escape shinychat's raw-HTML element names in markdown content

js/src/markdown/MarkdownContent.tsx

@@ -17,6 +17,7 @@ import {
 import { CopyableCodeBlock } from "./components/CopyableCodeBlock"
 import { BootstrapTable } from "./components/BootstrapTable"
 import { RawHTML } from "../chat/RawHTML"
+import { escapeReservedElements } from "./reservedElements"
 
 const RawHtmlIsland = (({ node }: { node?: Element }) => (
   <RawHTML html={node ? toHtml(node.children) : ""} displayContents />
@@ -71,13 +72,18 @@ export function MarkdownContent({
     streaming && !isText ? hideTrailingPartialAsideTag(content) : content
 
   // Stage 1 (expensive): parse markdown string → HAST. Cached by content+processor.
+  //
+  // Only the html branch may produce shinychat's raw-HTML elements; escaping
+  // them on the markdown branch keeps model output away from innerHTML. This
+  // has to happen here rather than server-side, because a streamed tag name can
+  // be split across chunks and is only whole once the client has reassembled it.
   const hast = useMemo(
     () =>
       isText
         ? null
         : isHtml
           ? parseHtml(parseSource, processor)
-          : parseMarkdown(parseSource, processor),
+          : parseMarkdown(escapeReservedElements(parseSource), processor),
     [parseSource, isText, isHtml, processor],
   )
 

js/src/markdown/reservedElements.ts

@@ -0,0 +1,42 @@
+/**
+ * Element names that take content out of React and into raw HTML.
+ *
+ * `shiny-chat-raw-html` (and its legacy alias `shinychat-raw-html`) is
+ * assigned to `innerHTML` by `RawHTML`; the two tool elements carry
+ * attributes (`icon`, `footer`, `tool-name`, `value` with `value-type="html"`)
+ * that reach `dangerouslySetInnerHTML`. Every other component in the tag maps
+ * renders through React and is inert.
+ *
+ * The server only ever emits these from `split_html_islands()` and the
+ * tool-card tagifier, which run when an app passes htmltools/Shiny UI rather
+ * than a string — and that content is always labelled `content_type: "html"`.
+ * So in markdown-parsed content these names are never legitimate, and content
+ * that names them there is model output trying to reach a raw-HTML sink.
+ */
+export const RESERVED_ELEMENTS = [
+  "shiny-chat-raw-html",
+  "shinychat-raw-html",
+  "shiny-tool-request",
+  "shiny-tool-result",
+] as const
+
+// Case-insensitive because parse5 lowercases tag names, so `<SHINYCHAT-RAW-HTML>`
+// would otherwise reach the sink. The lookahead requires a tag-name boundary so
+// that longer names starting with a reserved one (`<shiny-tool-resultant>`) are
+// left alone.
+const RESERVED_ELEMENT_RE = new RegExp(
+  `<(/?)(${RESERVED_ELEMENTS.join("|")})(?=[\\s/>]|$)`,
+  "gi",
+)
+
+/**
+ * Neutralize shinychat's raw-HTML element names so they render as visible text.
+ *
+ * Applied to markdown-parsed content only. Note that a reserved name inside a
+ * code fence is escaped too, so it displays as `&lt;shinychat-raw-html>` rather
+ * than `<shinychat-raw-html>`; fence-aware escaping would mean trusting fence
+ * detection to decide what is safe, which is the wrong thing to depend on.
+ */
+export function escapeReservedElements(content: string): string {
+  return content.replace(RESERVED_ELEMENT_RE, "&lt;$1$2")
+}

js/tests/chat/ToolBridge.test.tsx

@@ -49,7 +49,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-request data-shinychat-react request-id="req-1" tool-name="get_weather" tool-title="Get Weather" arguments=\'{"city":"NYC"}\'></shiny-tool-request>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -99,7 +99,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-request data-shinychat-react request-id="req-2" tool-name="get_weather" arguments="{}"></shiny-tool-request>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -116,7 +116,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-result data-shinychat-react request-id="req-2" tool-name="get_weather" status="success" value="Sunny, 72°F" value-type="text"></shiny-tool-result>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -160,7 +160,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-request data-shinychat-react request-id="req-inline-hide" tool-name="get_weather" arguments="{}"></shiny-tool-request>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -179,7 +179,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-result data-shinychat-react request-id="req-inline-hide" tool-name="get_weather" status="success" value="Sunny, 72°F" value-type="text"></shiny-tool-result>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -223,7 +223,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-request data-shinychat-react request-id="req-stream-hide" tool-name="get_weather" arguments="{}"></shiny-tool-request>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -248,6 +248,7 @@ describe("Tool component bridge rendering", () => {
         content:
           '<shiny-tool-result data-shinychat-react request-id="req-stream-hide" tool-name="get_weather" status="success" value="Done" value-type="text"></shiny-tool-result>',
         operation: "replace",
+        content_type: "html",
       })
     })
 
@@ -295,7 +296,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-request data-shinychat-react request-id="req-3" tool-name="search" arguments="{}"></shiny-tool-request>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -353,7 +354,7 @@ describe("Tool component bridge rendering", () => {
                 type: "content",
                 content:
                   '<shiny-tool-request data-shinychat-react request-id="req-preloaded" tool-name="search" arguments="{}"></shiny-tool-request>',
-                contentType: "markdown",
+                contentType: "html",
               },
             ],
           },
@@ -368,7 +369,7 @@ describe("Tool component bridge rendering", () => {
                 type: "content",
                 content:
                   '<shiny-tool-result data-shinychat-react request-id="req-preloaded" tool-name="search" status="success" value="Done" value-type="text"></shiny-tool-result>',
-                contentType: "markdown",
+                contentType: "html",
               },
             ],
           },
@@ -413,7 +414,7 @@ describe("Tool component bridge rendering", () => {
           segments: [
             {
               content: `<shiny-tool-result data-shinychat-react request-id="req-icon" tool-name="list_files" tool-title="List Files" status="success" value="file1.txt" value-type="text" icon="${folderIcon.replace(/"/g, "&quot;")}"></shiny-tool-result>`,
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -456,7 +457,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-result data-shinychat-react request-id="req-no-icon" tool-name="get_weather" status="success" value="Sunny" value-type="text"></shiny-tool-result>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },
@@ -500,7 +501,7 @@ describe("Tool component bridge rendering", () => {
             {
               content:
                 '<shiny-tool-result data-shinychat-react request-id="req-empty" tool-name="get_weather" status="success" value="" value-type="text" show-request full-screen expanded></shiny-tool-result>',
-              content_type: "markdown",
+              content_type: "html",
             },
           ],
         },

js/tests/chat/streamedReservedElements.test.tsx

@@ -0,0 +1,68 @@
+import { describe, it, expect, beforeEach } from "vitest"
+import { render, act } from "@testing-library/react"
+import { ChatApp } from "../../src/chat/ChatApp"
+import {
+  createMockTransport,
+  createMockShinyLifecycle,
+  installShinyWindowStub,
+} from "../helpers/mocks"
+
+beforeEach(() => {
+  installShinyWindowStub()
+})
+
+function renderChat(transport: ReturnType<typeof createMockTransport>) {
+  return render(
+    <ChatApp
+      transport={transport}
+      shinyLifecycle={createMockShinyLifecycle()}
+      elementId="test-chat"
+      inputId="test-input"
+      uploadAccept={["image/png"]}
+      maxUploadSize={30000000}
+    />,
+  )
+}
+
+// A streamed tag name arrives split across chunks. Escaping on the server
+// would inspect each chunk separately and miss it; the browser reassembles the
+// block before parsing, which is why the escape lives on the client.
+describe("a reserved element streamed across chunk boundaries", () => {
+  it("stays inert when the tag name is split mid-stream", () => {
+    const transport = createMockTransport()
+    const { container } = renderChat(transport)
+
+    act(() => {
+      transport.fire("test-chat", {
+        type: "chunk_start",
+        message: {
+          role: "assistant",
+          segments: [{ content: "", content_type: "markdown" }],
+        },
+      })
+    })
+
+    for (const piece of [
+      "<shinychat-raw",
+      "-html><img src=x onerror",
+      "=alert(1)></shinychat-raw-html>",
+    ]) {
+      act(() => {
+        transport.fire("test-chat", {
+          type: "chunk",
+          content: piece,
+          operation: "append",
+          content_type: "markdown",
+        })
+      })
+    }
+
+    act(() => {
+      transport.fire("test-chat", { type: "chunk_end" })
+    })
+
+    expect(container.querySelector("[onerror]")).toBeNull()
+    expect(container.innerHTML).not.toContain("onerror")
+    expect(container.textContent).toContain("shinychat-raw-html")
+  })
+})

js/tests/markdown/MarkdownContent.test.tsx

@@ -161,8 +161,12 @@ describe("MarkdownContent (pure)", () => {
     const content =
       '<shiny-tool-result request-id="req-1" tool-name="get_weather" status="success" value="Sunny" value-type="text"></shiny-tool-result>'
 
+    // contentType "html" is what the server sends for tool cards: they are
+    // built as htmltools tags, and tag content is always labelled "html".
+    // In markdown content these element names are escaped (see
+    // reservedElementsRendering.test.tsx).
     const { container } = render(
-      <MarkdownContent content={content} contentType="markdown" />,
+      <MarkdownContent content={content} contentType="html" />,
     )
 
     expect(container.querySelector("shiny-tool-result")).not.toBeNull()
@@ -178,7 +182,7 @@ describe("MarkdownContent (pure)", () => {
     const { container } = render(
       <MarkdownContent
         content={content}
-        contentType="markdown"
+        contentType="html"
         tagToComponentMap={chatTagToComponentMap}
       />,
     )

js/tests/markdown/reservedElements.test.ts

@@ -0,0 +1,74 @@
+import { describe, it, expect } from "vitest"
+import {
+  RESERVED_ELEMENTS,
+  escapeReservedElements,
+} from "../../src/markdown/reservedElements"
+
+describe("escapeReservedElements", () => {
+  it("escapes every reserved element name", () => {
+    for (const name of RESERVED_ELEMENTS) {
+      expect(escapeReservedElements(`<${name}>`)).toBe(`&lt;${name}>`)
+      expect(escapeReservedElements(`</${name}>`)).toBe(`&lt;/${name}>`)
+    }
+  })
+
+  it("escapes regardless of case, since parse5 lowercases tag names", () => {
+    expect(escapeReservedElements("<SHINYCHAT-RAW-HTML>")).toBe(
+      "&lt;SHINYCHAT-RAW-HTML>",
+    )
+    expect(escapeReservedElements("<Shiny-Tool-Result>")).toBe(
+      "&lt;Shiny-Tool-Result>",
+    )
+  })
+
+  it("escapes tags carrying attributes", () => {
+    expect(
+      escapeReservedElements(`<shiny-tool-result icon="<img onerror=x>">`),
+    ).toBe(`&lt;shiny-tool-result icon="<img onerror=x>">`)
+  })
+
+  it("escapes a tag with a newline before its attributes", () => {
+    expect(escapeReservedElements("<shiny-tool-request\nfoo>")).toBe(
+      "&lt;shiny-tool-request\nfoo>",
+    )
+  })
+
+  it("escapes a self-closing form", () => {
+    expect(escapeReservedElements("<shinychat-raw-html/>")).toBe(
+      "&lt;shinychat-raw-html/>",
+    )
+  })
+
+  it("escapes a bare name at end of input", () => {
+    expect(escapeReservedElements("<shinychat-raw-html")).toBe(
+      "&lt;shinychat-raw-html",
+    )
+  })
+
+  it("escapes every occurrence, not just the first", () => {
+    const input = "<shinychat-raw-html>a</shinychat-raw-html>"
+    expect(escapeReservedElements(input)).toBe(
+      "&lt;shinychat-raw-html>a&lt;/shinychat-raw-html>",
+    )
+  })
+
+  it("leaves names that merely start with a reserved name alone", () => {
+    expect(escapeReservedElements("<shinychat-raw-htmlx>")).toBe(
+      "<shinychat-raw-htmlx>",
+    )
+    expect(escapeReservedElements("<shiny-tool-resultant>")).toBe(
+      "<shiny-tool-resultant>",
+    )
+  })
+
+  it("leaves unrelated markup alone", () => {
+    const input =
+      "# Title\n\n<div class='x'>hi</div>\n\n<shiny-tool>no</shiny-tool>"
+    expect(escapeReservedElements(input)).toBe(input)
+  })
+
+  it("returns content unchanged when there is nothing to escape", () => {
+    const input = "just some **markdown**"
+    expect(escapeReservedElements(input)).toBe(input)
+  })
+})