mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(native-chat): show unknown HTML tags as text instead of dropping them (#30819)
* fix(native-chat): show unknown HTML tags as text instead of dropping them Chat UI renders messages through CommentMarkdown, which parses raw HTML and then sanitizes it. Sanitize unwraps any tag outside its allowlist, so prose that mentions a tag lost it: "use a <se-tabs>" rendered as "use a ". Add a remark plugin that turns raw HTML containing a non-allowlisted tag into literal text before sanitize runs. It reads the sanitize schema's own tag list, so the two cannot drift, and runs before remark-breaks so newlines inside it still break. Allowed tags (<sub>, <kbd>, <details>, ...) and inline code are unchanged. The behaviour is opt-in via a new `unknownHtmlAsText` prop and enabled only for native chat; GitHub, GitLab, Jira and Linear surfaces keep matching their host's rendering, which also strips unknown tags. Fixes #30604 * fix(native-chat): preserve markdown parsing around literal HTML * fix(native-chat): escape only unknown tags in mixed raw HTML blocks A raw HTML block that mixes allowed and unknown tags keeps rendering its allowed tags and comments; only the unknown tags are shown literally. --------- Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
@@ -136,6 +136,7 @@ export function NativeChatMarkdown({
|
||||
fadeWords={fadeWords}
|
||||
renderMermaid={!streaming}
|
||||
keepMermaidSourceWhilePending
|
||||
unknownHtmlAsText
|
||||
data-word-fade={wordFadeArmed ? '' : undefined}
|
||||
data-block-fade={wordFadeArmed && streaming ? '' : undefined}
|
||||
className={cn('native-chat-markdown', className)}
|
||||
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
} from './comment-markdown-native-chat-file-links'
|
||||
import { CommentMarkdownBlockLineContext } from './comment-markdown-code-renderers'
|
||||
import { rehypeWordFade } from './comment-markdown-word-fade'
|
||||
import { remarkUnknownHtmlAsText } from './comment-markdown-unknown-html-as-text'
|
||||
import { CommentMarkdownWords } from './CommentMarkdownWords'
|
||||
import {
|
||||
splitMarkdownTopLevelBlocks,
|
||||
@@ -207,6 +208,14 @@ const commentMarkdownSanitizeSchema = {
|
||||
}
|
||||
}
|
||||
|
||||
// Why: runs before remark-breaks so newlines inside the escaped HTML still break.
|
||||
const unknownHtmlAsTextRemarkPlugins = [
|
||||
remarkUnknownHtmlAsText(
|
||||
new Set((commentMarkdownSanitizeSchema.tagNames ?? []).map((name) => name.toLowerCase()))
|
||||
),
|
||||
...remarkPlugins
|
||||
]
|
||||
|
||||
// Why: GitHub comments often include safe raw HTML (`<sub>`, `<details>`,
|
||||
// `<br />`). Parse it, then sanitize immediately before React renders it.
|
||||
const rehypePlugins: MarkdownPlugins = [rehypeRaw, [rehypeSanitize, commentMarkdownSanitizeSchema]]
|
||||
@@ -251,6 +260,8 @@ type CommentMarkdownProps = React.ComponentPropsWithoutRef<'div'> & {
|
||||
/** Give each word its own `data-word` element, for a stylesheet to animate as words arrive. */
|
||||
fadeWords?: boolean
|
||||
extension?: CommentMarkdownExtension
|
||||
/** Show raw HTML that sanitize would strip as literal text instead of dropping its tags. */
|
||||
unknownHtmlAsText?: boolean
|
||||
}
|
||||
|
||||
/** One render of the pipeline; memoized so an unchanged block is not parsed again. */
|
||||
@@ -291,6 +302,7 @@ const CommentMarkdown = React.memo(
|
||||
growing = false,
|
||||
fadeWords = false,
|
||||
extension,
|
||||
unknownHtmlAsText = false,
|
||||
...rest
|
||||
},
|
||||
ref
|
||||
@@ -318,12 +330,13 @@ const CommentMarkdown = React.memo(
|
||||
[extension]
|
||||
)
|
||||
const activeRemarkPlugins = React.useMemo(() => {
|
||||
const basePlugins = unknownHtmlAsText ? unknownHtmlAsTextRemarkPlugins : remarkPlugins
|
||||
const plugins = fileLinkExists
|
||||
? [...remarkPlugins, remarkNativeChatFileLinks(fileLinkExists)]
|
||||
: remarkPlugins
|
||||
? [...basePlugins, remarkNativeChatFileLinks(fileLinkExists)]
|
||||
: basePlugins
|
||||
const withExtension = extension ? [...plugins, ...extension.remarkPlugins] : plugins
|
||||
return githubRepo ? [...withExtension, remarkGitHubReferences(githubRepo)] : withExtension
|
||||
}, [extension, fileLinkExists, githubRepo])
|
||||
}, [extension, fileLinkExists, githubRepo, unknownHtmlAsText])
|
||||
const blocks = useMarkdownBlocks(content, growing)
|
||||
const [hadWordFade, setHadWordFade] = React.useState(fadeWords)
|
||||
if (fadeWords && !hadWordFade) {
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import CommentMarkdown from './CommentMarkdown'
|
||||
import { NativeChatMarkdown } from '@/components/native-chat/NativeChatMarkdown'
|
||||
|
||||
function renderChat(content: string): string {
|
||||
return renderToStaticMarkup(<NativeChatMarkdown content={content} />)
|
||||
}
|
||||
|
||||
describe('CommentMarkdown unknown HTML as text', () => {
|
||||
it('keeps an unknown inline tag in native chat prose', () => {
|
||||
const markup = renderChat('use\na <se-tabs>')
|
||||
|
||||
expect(markup).toContain('a <se-tabs>')
|
||||
})
|
||||
|
||||
it('keeps unknown opening and closing tags around their content', () => {
|
||||
const markup = renderChat('wrap <se-tabs>x</se-tabs> here')
|
||||
|
||||
expect(markup).toContain('wrap <se-tabs>x</se-tabs> here')
|
||||
})
|
||||
|
||||
it('keeps a component-style tag that starts a block', () => {
|
||||
const markup = renderChat('<Button variant="ghost" />\n\nnext')
|
||||
|
||||
expect(markup).toContain('<Button variant="ghost" />')
|
||||
expect(markup).toContain('next')
|
||||
})
|
||||
|
||||
it('shows script tags as text instead of dropping them', () => {
|
||||
const markup = renderChat('see <script>alert(1)</script>')
|
||||
|
||||
expect(markup).toContain('<script>alert(1)</script>')
|
||||
expect(markup).not.toContain('<script>')
|
||||
})
|
||||
|
||||
it('still renders allowed HTML in native chat', () => {
|
||||
const markup = renderChat('H<sub>2</sub>O and <kbd>Ctrl</kbd>')
|
||||
|
||||
expect(markup).toContain('<sub>2</sub>')
|
||||
expect(markup).toContain('<kbd>Ctrl</kbd>')
|
||||
})
|
||||
|
||||
it('leaves inline code untouched', () => {
|
||||
const markup = renderChat('use `<se-tabs>`')
|
||||
|
||||
expect(markup).toContain('<se-tabs></code>')
|
||||
})
|
||||
|
||||
it('keeps sanitize dropping unknown tags on other surfaces', () => {
|
||||
const markup = renderToStaticMarkup(<CommentMarkdown content="a <se-tabs> b" />)
|
||||
|
||||
expect(markup).not.toContain('se-tabs')
|
||||
})
|
||||
|
||||
it('keeps allowed HTML when an attribute mentions an unknown tag', () => {
|
||||
const markup = renderChat('<kbd title="use <Button>">Ctrl</kbd>')
|
||||
|
||||
expect(markup).toContain('<kbd title="use <Button>">Ctrl</kbd>')
|
||||
})
|
||||
|
||||
it('does not reveal HTML comments that mention unknown tags', () => {
|
||||
const markup = renderChat('before <!-- <Button /> --> after')
|
||||
|
||||
expect(markup).not.toContain('Button')
|
||||
expect(markup).toContain('before after')
|
||||
})
|
||||
|
||||
it('preserves the existing bare URL punctuation boundary in chat', () => {
|
||||
const markup = renderChat('See https://example.com/a。Next')
|
||||
|
||||
expect(markup).toContain('href="https://example.com/a"')
|
||||
expect(markup).not.toContain('href="https://example.com/a%E3%80%82Next"')
|
||||
})
|
||||
|
||||
it('keeps multiline XML literal and preserves its line breaks', () => {
|
||||
const markup = renderChat('<project>\n<name>orca</name>\n</project>')
|
||||
|
||||
expect(markup).toContain('<project><br/>')
|
||||
expect(markup).toContain('<name>orca</name><br/>')
|
||||
expect(markup).toContain('</project>')
|
||||
expect(markup).not.toContain('<project>')
|
||||
})
|
||||
|
||||
it('keeps literal tags in a growing reply', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<NativeChatMarkdown content="use a <se-tabs>" growing streaming />
|
||||
)
|
||||
|
||||
expect(markup).toContain('a <se-tabs>')
|
||||
})
|
||||
|
||||
it('leaves fenced code untouched', () => {
|
||||
const markup = renderChat('```tsx\n<Button />\n```')
|
||||
|
||||
expect(markup).toContain('<Button />\n</code></pre>')
|
||||
})
|
||||
|
||||
it('leaves angle-bracket URL and email autolinks intact', () => {
|
||||
const markup = renderChat('<https://example.com> and <orca@example.com>')
|
||||
|
||||
expect(markup).toContain('href="https://example.com"')
|
||||
expect(markup).toContain('href="mailto:orca@example.com"')
|
||||
})
|
||||
|
||||
it('still sanitizes unsafe attributes and URLs on allowed HTML', () => {
|
||||
const markup = renderChat('<a href="javascript:alert(1)" onclick="alert(2)">link</a>')
|
||||
|
||||
expect(markup).toContain('>link</a>')
|
||||
expect(markup).not.toContain('javascript:')
|
||||
expect(markup).not.toContain('onclick')
|
||||
})
|
||||
|
||||
it('keeps disallowed elements inert even when they have executable attributes', () => {
|
||||
const markup = renderChat('<iframe srcdoc="<script>alert(1)</script>"></iframe>')
|
||||
|
||||
expect(markup).toContain('<iframe')
|
||||
expect(markup).not.toContain('<iframe')
|
||||
expect(markup).not.toContain('<script')
|
||||
})
|
||||
|
||||
it('escapes only the unknown tags in an HTML block that mixes allowed and unknown tags', () => {
|
||||
const markup = renderChat(
|
||||
'<details>\n<summary>More</summary>\n<se-tabs>x</se-tabs> <!-- <Button /> --> <kbd>K</kbd>\n</details>'
|
||||
)
|
||||
|
||||
expect(markup).toContain('<details>')
|
||||
expect(markup).toContain('<summary>More</summary>')
|
||||
expect(markup).toContain('<se-tabs>x</se-tabs>')
|
||||
expect(markup).toContain('<kbd>K</kbd>')
|
||||
expect(markup).not.toContain('Button')
|
||||
})
|
||||
|
||||
it('keeps an unknown tag inert inside a mixed HTML block', () => {
|
||||
const markup = renderChat('<div>\n<iframe srcdoc="<script>alert(1)</script>"></iframe>\n</div>')
|
||||
|
||||
expect(markup).toContain(
|
||||
'<iframe srcdoc="<script>alert(1)</script>">'
|
||||
)
|
||||
expect(markup).not.toContain('<iframe')
|
||||
expect(markup).not.toContain('<script')
|
||||
})
|
||||
|
||||
it('still renders an allowed disclosure', () => {
|
||||
const markup = renderChat('<details><summary>More</summary>\n\nbody\n\n</details>')
|
||||
|
||||
expect(markup).toContain('<details>')
|
||||
expect(markup).toContain('<summary>More</summary>')
|
||||
expect(markup).toContain('body')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,78 @@
|
||||
type MarkdownNode = {
|
||||
type: string
|
||||
value?: string
|
||||
children?: MarkdownNode[]
|
||||
}
|
||||
|
||||
const HTML_TAG_NAME_PATTERN =
|
||||
/<!--[\s\S]*?(?:-->|$)|<\/?([A-Za-z][A-Za-z0-9-]*)(?:[^"'<>]|"[^"]*"|'[^']*')*>/g
|
||||
|
||||
const PHRASING_PARENT_TYPES = new Set([
|
||||
'paragraph',
|
||||
'heading',
|
||||
'emphasis',
|
||||
'strong',
|
||||
'delete',
|
||||
'link',
|
||||
'linkReference',
|
||||
'tableCell'
|
||||
])
|
||||
|
||||
function isUnknownTag(name: string | undefined, allowedTagNames: ReadonlySet<string>): boolean {
|
||||
return name !== undefined && !allowedTagNames.has(name.toLowerCase())
|
||||
}
|
||||
|
||||
function escapeHtmlText(value: string): string {
|
||||
return value.replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>')
|
||||
}
|
||||
|
||||
function classifyHtml(
|
||||
html: string,
|
||||
allowedTagNames: ReadonlySet<string>
|
||||
): 'none' | 'unknown-only' | 'mixed' {
|
||||
let hasUnknown = false
|
||||
let hasOther = false
|
||||
for (const match of html.matchAll(HTML_TAG_NAME_PATTERN)) {
|
||||
if (isUnknownTag(match[1], allowedTagNames)) {
|
||||
hasUnknown = true
|
||||
} else {
|
||||
hasOther = true
|
||||
}
|
||||
}
|
||||
return hasUnknown ? (hasOther ? 'mixed' : 'unknown-only') : 'none'
|
||||
}
|
||||
|
||||
function replaceUnknownHtml(parent: MarkdownNode, allowedTagNames: ReadonlySet<string>): void {
|
||||
const children = parent.children
|
||||
if (!children) {
|
||||
return
|
||||
}
|
||||
const isPhrasingParent = PHRASING_PARENT_TYPES.has(parent.type)
|
||||
for (let index = 0; index < children.length; index++) {
|
||||
const child = children[index]
|
||||
if (child.type !== 'html' || !child.value) {
|
||||
replaceUnknownHtml(child, allowedTagNames)
|
||||
continue
|
||||
}
|
||||
const kind = classifyHtml(child.value, allowedTagNames)
|
||||
if (kind === 'unknown-only') {
|
||||
const text: MarkdownNode = { type: 'text', value: child.value }
|
||||
children[index] = isPhrasingParent ? text : { type: 'paragraph', children: [text] }
|
||||
} else if (kind === 'mixed') {
|
||||
// Why: escape only the unknown tags so allowed HTML and comments in the same block render as before.
|
||||
child.value = child.value.replace(HTML_TAG_NAME_PATTERN, (match, name?: string) =>
|
||||
isUnknownTag(name, allowedTagNames) ? escapeHtmlText(match) : match
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shows raw HTML tags that sanitize would strip (`<se-tabs>`, `<Button />`) as literal text,
|
||||
* so prose that mentions a tag keeps it instead of rendering an empty gap.
|
||||
*/
|
||||
export function remarkUnknownHtmlAsText(
|
||||
allowedTagNames: ReadonlySet<string>
|
||||
): () => (tree: MarkdownNode) => void {
|
||||
return () => (tree) => replaceUnknownHtml(tree, allowedTagNames)
|
||||
}
|
||||
Reference in New Issue
Block a user