Repository navigation
feat: render assistant answers through a sanitized markdown allowlist - #557
Merged
Merged
Conversation
|
Automatic reviews are disabled for this repository. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Problem
An assistant answer can carry text the model copied from project data, such as a project name or an expert's position, so whatever renders it must not turn that text into an image, a link to another address, or raw HTML.
Change
Adds
react-markdownpinned at 10.1.0 andAssistantMarkdown, which renders an answer from the elementsp,strong,em,ul,ol,li,codeandbronly.unwrapDisallowedkeeps the text of anything else, so a markdown link, an autolink or a heading shows as plain text and an image renders nothing;skipHtmldrops raw HTML. A[n]marker in the text of a paragraph, list item, bold or italic run stays plain text while the caller passes no source list (an answer still streaming has none yet); once a list is passed, the marker becomes an in-page anchor#assistant-src-<messageId>-<n>when the answer has sourcen, and its click callsonCite(n)instead of changing the location; a marker with no such source is struck through and is not a link, and its digits are shown as written. Markers inside inline code stay literal. The overrides are module-level components that read the citation data from a context, so a re-render or a streamed token does not remount the answer and a clicked citation keeps focus.isAllowedAssistantUrlaccepts only a string that starts withhttps://and parses as anhttps:URL; the source list in the next PR uses it. Nothing renders the component yet: the panel wires it in later.Verification
New tests in
AssistantMarkdown.test.tsxandurl-allowlist.test.tscover the allowed formatting, links, autolinks and headings coming out as text,javascript:links, hard line breaks, images (no element and noalttext), raw HTML, no leakednodeattribute, a[n]with no source list, a known and an unknown[n](including a number too large for a safe integer), a click that is default-prevented and leaveslocation.hashunchanged, citations inside nested emphasis and inside code, the anchor and paragraph DOM nodes surviving a re-render with a new source list and handler, and the paragraph node surviving appended content.npm run lint,npm run typecheck,npm run test:coverageand./scripts/ci/ci-local.sh fastare green.Migrations
None.
Screenshots
None, nothing renders the component yet.