Render comment bodies as Markdown in the comment rail #1
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import { renderToStaticMarkup } from "react-dom/server"; | ||
| import CommentMarkdown from "./CommentMarkdown"; | ||
|
|
||
| // renderToStaticMarkup keeps this a pure string assertion (no DOM/jsdom needed), | ||
| // matching the repo's node-env vitest setup. | ||
| const render = (body: string) => renderToStaticMarkup(<CommentMarkdown body={body} />); | ||
|
|
||
| describe("CommentMarkdown", () => { | ||
| it("renders GFM structure", () => { | ||
| const html = render( | ||
| "**bold** and `code`\n\n1. one\n2. two\n\n```sql\nSELECT 1\n```\n\n> quote" | ||
| ); | ||
| expect(html).toContain("<strong>"); | ||
| expect(html).toContain("<code>"); | ||
| expect(html).toContain("<ol>"); | ||
| expect(html).toContain("<pre>"); | ||
| expect(html).toContain("<blockquote>"); | ||
| }); | ||
|
|
||
| it("renders a GFM table", () => { | ||
| const html = render("| a | b |\n| - | - |\n| 1 | 2 |"); | ||
| expect(html).toContain("<table>"); | ||
| }); | ||
|
|
||
| it("neutralizes raw HTML — no live script element", () => { | ||
| const html = render("<script>alert(document.cookie)</script>"); | ||
| expect(html).not.toContain("<script"); | ||
| }); | ||
|
|
||
| it("strips javascript: link hrefs", () => { | ||
| const html = render("[x](javascript:alert(1))"); | ||
| expect(html).not.toContain("javascript:"); | ||
| }); | ||
|
|
||
| it("hardens links with target, rel, and data-no-pin", () => { | ||
| const html = render("[k](https://example.com)"); | ||
| expect(html).toContain('href="https://example.com"'); | ||
| expect(html).toContain('target="_blank"'); | ||
| expect(html).toContain("noopener"); | ||
| expect(html).toContain("noreferrer"); | ||
| // a link click must not bubble to the card's click-to-pin handler | ||
| expect(html).toContain("data-no-pin"); | ||
| }); | ||
|
|
||
| it("strips images", () => { | ||
| const html = render(""); | ||
| expect(html).not.toContain("<img"); | ||
| }); | ||
|
|
||
| it("keeps in-page (#fragment) links inline, not target=_blank", () => { | ||
| const frag = render("[jump](#section)"); | ||
| expect(frag).toContain('href="#section"'); | ||
| expect(frag).not.toContain('target="_blank"'); | ||
| // external links still open a new tab | ||
| expect(render("[x](https://example.com)")).toContain('target="_blank"'); | ||
| }); | ||
|
|
||
| it("namespaces footnote ids per card with working forward + back links", () => { | ||
| const md = "needs a note[^1]\n\n[^1]: the note."; | ||
| // two cards in one tree must get distinct footnote ids (no cross-card collision) | ||
| const both = renderToStaticMarkup( | ||
| <> | ||
| <CommentMarkdown body={md} /> | ||
| <CommentMarkdown body={md} /> | ||
| </> | ||
| ); | ||
| const defIds = [...both.matchAll(/id="([^"]*-fn-1)"/g)].map((m) => m[1]); | ||
| expect(defIds).toHaveLength(2); | ||
| expect(defIds[0]).not.toBe(defIds[1]); | ||
|
|
||
| // within one card, every #fragment link (forward jump + ↩ back-ref) resolves to | ||
| // an id in that card, and ids are ascii-safe (React 19 useId delimiters stripped) | ||
| const one = render(md); | ||
| const ids = new Set([...one.matchAll(/id="([^"]+)"/g)].map((m) => m[1])); | ||
| const targets = [...one.matchAll(/href="#([^"]+)"/g)].map((m) => m[1]); | ||
| expect(targets.length).toBeGreaterThanOrEqual(2); | ||
| expect(targets.every((t) => ids.has(t))).toBe(true); | ||
| expect([...ids].every((id) => /^[A-Za-z0-9_-]+$/.test(id))).toBe(true); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| "use client"; | ||
|
|
||
| import { memo, useId, type ComponentPropsWithoutRef } from "react"; | ||
| import ReactMarkdown, { type ExtraProps } from "react-markdown"; | ||
| import remarkGfm from "remark-gfm"; | ||
| import remarkBreaks from "remark-breaks"; | ||
| import rehypeSanitize, { defaultSchema } from "rehype-sanitize"; | ||
|
|
||
| // Comment bodies are untrusted — any human grantee or agent can post them — and the | ||
| // rail renders in the justhtml.sh shell origin (cookies/session), not the sandboxed | ||
| // doc iframe, so this is a stored-XSS surface. react-markdown v10 is safe by default | ||
| // (raw HTML is inert, dangerous URL protocols are dropped); rehype-sanitize is | ||
| // defense-in-depth, so turning raw HTML on later can never escalate a comment into | ||
| // account takeover. Images are stripped: a remote <img> in a session origin is a | ||
| // tracking / CSRF-pixel vector with no value in a review thread. | ||
| // | ||
| // clobber is emptied because the only id/name attributes in the output are the | ||
| // footnote ids remark-rehype generates (raw HTML is off, so authors can't inject | ||
| // ids); those are namespaced per render below via clobberPrefix instead, which keeps | ||
| // each card's footnote refs pointing inside that card. | ||
| const schema = { | ||
| ...defaultSchema, | ||
| tagNames: defaultSchema.tagNames?.filter((tag) => tag !== "img"), | ||
| clobber: [], | ||
| attributes: { | ||
| ...defaultSchema.attributes, | ||
| a: [...(defaultSchema.attributes?.a ?? []), "target", "rel"], | ||
| }, | ||
| }; | ||
|
|
||
| const components = { | ||
| // data-no-pin: comment bodies render inside a click-to-pin card (see CommentsShell | ||
| // Card onClick); without it, clicking a link would also toggle the thread's pin. | ||
| // Only off-page links open a new tab — in-page (#fragment) links, e.g. footnote | ||
| // back-references, must navigate within the card rather than spawn a tab. The rest | ||
| // of the sanitized attributes (id, aria-*, data-footnote-*) are forwarded so | ||
| // footnote ref/backref anchors keep the ids their return links target. | ||
| a: ({ node, href, children, ...rest }: ComponentPropsWithoutRef<"a"> & ExtraProps) => { | ||
| const inPage = href?.startsWith("#") ?? false; | ||
| return ( | ||
| <a {...rest} href={href} data-no-pin {...(inPage ? {} : { target: "_blank", rel: "noopener noreferrer" })}> | ||
| {children} | ||
| </a> | ||
| ); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| }, | ||
| }; | ||
|
There was a problem hiding this comment. Task checkbox clicks toggle pinMedium Severity GFM task lists can render checkbox controls inside comment bodies, but only markdown links get Additional Locations (1)  Reviewed by Cursor Bugbot for commit 386ab64. Configure here.
Contributor
Author
There was a problem hiding this comment. Verified this isn't reachable in practice, so leaving it as-is. GFM task checkboxes render
The only genuinely new interactive element markdown introduces is links, which already carry |
||
|
|
||
| function CommentMarkdown({ body }: { body: string }) { | ||
| // Namespace footnote ids per comment so multiple cards in one shell DOM don't | ||
| // collide — a footnote ref must scroll to its own card's note, not another's. The | ||
| // id is stripped to ASCII (React 19's useId returns non-alphanumeric delimiters). | ||
| const prefix = useId().replace(/[^a-zA-Z0-9]/g, "") + "-"; | ||
| return ( | ||
| <div className="jh-md"> | ||
| <ReactMarkdown | ||
| remarkPlugins={[remarkGfm, remarkBreaks]} | ||
| remarkRehypeOptions={{ clobberPrefix: prefix }} | ||
| rehypePlugins={[[rehypeSanitize, schema]]} | ||
| components={components} | ||
| > | ||
| {body} | ||
| </ReactMarkdown> | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export default memo(CommentMarkdown); | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.