Hide HTML comments in rendered Markdown, as GitHub does (#184)
* Hide HTML comments in rendered Markdown, as GitHub does
- CI / Typecheck and bundlesuccess
- CI / Unit tests (1/2)success
- CI / Unit tests (2/2)success
- Deploy / Deploy to Cloudflaresuccess
- End-to-end / End-to-end (agent reviews, tasks, Today)success
- End-to-end / End-to-end (git, PRs, Actions)success
- End-to-end / End-to-end (large pushes)success
- End-to-end / End-to-end (passkeys, registry, email)success
- End-to-end / End-to-end (real histories)success
- End-to-end / End-to-end (security)success
- Live test / Containers, registry, sandboxes, webhooks, schedules, LFS and pull requestssuccess
6 files changed+180−12
Files (6)
modifiedapps/diffinery/docs/collaboration/issues.md+1−1View file
| @@ -109,7 +109,7 @@ | ||
| 109 | 109 | |
| 110 | 110 | Descriptions, comments and review bodies are rendered as Markdown. Supported: headings, paragraphs, hard line breaks, fenced code blocks, horizontal rules, blockquotes, ordered, unordered and task lists (nested), tables, inline code, bold, italic, strikethrough, links, images and bare URLs. |
| 111 | 111 | |
| 112 | -- Raw HTML is escaped, never rendered. | |
| 112 | +- Raw HTML is escaped, never rendered. HTML comments (`<!-- ... -->`) are hidden, as on GitHub, except inside code. They stay in the body, so the API and the edit box still show them, but pages and notification emails don't. | |
| 113 | 113 | - Only `http(s)` URLs, `#fragment` links and relative paths are kept in links and images. Relative links resolve against the repository's default branch. |
| 114 | 114 | - Task list items (`- [ ]` and `- [x]`) render as checkboxes. They are display only; edit the text to tick them. |
| 115 | 115 | - A body larger than 512 KB is shown as plain preformatted text. |
modifiedapps/diffinery/docs/repositories/browsing-code.md+1−1View file
| @@ -126,7 +126,7 @@ | ||
| 126 | 126 | |
| 127 | 127 | Rules that differ from GitHub: |
| 128 | 128 | |
| 129 | -- Raw HTML is always escaped and shown as text, never rendered. `<details>`, `<img>`, `<br>` and similar tags appear literally. | |
| 129 | +- Raw HTML is always escaped and shown as text, never rendered. `<details>`, `<img>`, `<br>` and similar tags appear literally. HTML comments (`<!-- ... -->`) are hidden, as on GitHub, except inside code. | |
| 130 | 130 | - Links and images may use `http://`, `https://`, a `#fragment`, or a relative path. Other schemes (`javascript:`, `data:`, `mailto:` and so on) are dropped. |
| 131 | 131 | - Relative links resolve against the document's folder at the same ref: links open the file or folder page, images load from the raw URL. Links that climb above the repository root are dropped. |
| 132 | 132 | - Documents larger than 512 KB are shown as plain text. |
modifiedapps/diffinery/src/mail/templates.ts+3−3View file
| @@ -4,7 +4,7 @@ | ||
| 4 | 4 | * Mail, light and dark) and as plain text. The look follows the site: Kumo's |
| 5 | 5 | * neutral palette, blue for actions and the orange mark. |
| 6 | 6 | */ |
| 7 | -import { escapeHtml, renderMarkdown } from "../web/markdown.js"; | |
| 7 | +import { escapeHtml, renderMarkdown, stripHtmlComments } from "../web/markdown.js"; | |
| 8 | 8 | import type { DigestData, MailJob, Reason, RunData, ThreadData } from "./types.js"; |
| 9 | 9 | |
| 10 | 10 | export type Inline = string | { b: string } | { a: string; href: string } | { code: string }; |
| @@ -140,7 +140,7 @@ | ||
| 140 | 140 | |
| 141 | 141 | function blockText(b: Block): string { |
| 142 | 142 | if ("p" in b) return inlineText(b.p); |
| 143 | - if ("md" in b) return (b.md.trim() || "No description provided.").split("\n").map((l) => `> ${l}`.trimEnd()).join("\n"); | |
| 143 | + if ("md" in b) return (stripHtmlComments(b.md).trim() || "No description provided.").split("\n").map((l) => `> ${l}`.trimEnd()).join("\n"); | |
| 144 | 144 | if ("kv" in b) return b.kv.map(([k, v]) => `${k}: ${inlineText(v)}`).join("\n"); |
| 145 | 145 | if ("button" in b) return `${b.button.label}: ${b.button.url}${b.secondary ? `\n${b.secondary.label}: ${b.secondary.url}` : ""}`; |
| 146 | 146 | if ("callout" in b) return `! ${inlineText(b.callout)}`; |
| @@ -323,7 +323,7 @@ | ||
| 323 | 323 | break; |
| 324 | 324 | } |
| 325 | 325 | } |
| 326 | - return { subject, preheader: d.body?.trim() ? d.body.trim().slice(0, 140) : `${inlineText(title)}`, eyebrow, title, blocks, fromName: d.actor, context: repoContext(pub, d.repo), thread: `${d.repo}/${isPull ? "pull" : "issues"}/${d.number}`, url: d.url }; | |
| 326 | + return { subject, preheader: stripHtmlComments(d.body ?? "").trim().slice(0, 140) || `${inlineText(title)}`, eyebrow, title, blocks, fromName: d.actor, context: repoContext(pub, d.repo), thread: `${d.repo}/${isPull ? "pull" : "issues"}/${d.number}`, url: d.url }; | |
| 327 | 327 | } |
| 328 | 328 | |
| 329 | 329 | function runEmail(d: RunData, pub: string): Email { |
modifiedapps/diffinery/src/web/markdown.ts+105−6View file
| @@ -1,7 +1,8 @@ | ||
| 1 | 1 | /** |
| 2 | 2 | * A small, safe Markdown renderer for READMEs, issues, pull requests and job |
| 3 | - * summaries. All raw HTML in the source is escaped; only http(s), mailto-free | |
| 4 | - * relative and fragment URLs survive in links and images. | |
| 3 | + * summaries. All raw HTML in the source is escaped, except HTML comments, which | |
| 4 | + * are dropped as GitHub drops them; only http(s), mailto-free relative and | |
| 5 | + * fragment URLs survive in links and images. | |
| 5 | 6 | * |
| 6 | 7 | * Supported: ATX headings, paragraphs, hard breaks, fenced code, thematic |
| 7 | 8 | * breaks, blockquotes, ordered/unordered/task lists (nested), tables, inline |
| @@ -105,6 +106,53 @@ | ||
| 105 | 106 | |
| 106 | 107 | // ---------------------------------------------------------------- inline |
| 107 | 108 | |
| 109 | +/** | |
| 110 | + * Text with its HTML comments (`<!-- … -->`, also `<!-->` and `<!--->`) removed. Code spans and | |
| 111 | + * backslash-escaped `<` are kept as written, and an unclosed `<!--` stays as text. | |
| 112 | + */ | |
| 113 | +function stripComments(src: string): string { | |
| 114 | + if (!src.includes("<!--")) return src; | |
| 115 | + let out = ""; | |
| 116 | + let i = 0; | |
| 117 | + const n = src.length; | |
| 118 | + // Every comment closes at or before the last "-->", so an opener past it is text: no rescans. | |
| 119 | + const lastClose = src.lastIndexOf("-->"); | |
| 120 | + while (i < n) { | |
| 121 | + if (scanned > budget) return out + src.slice(i); | |
| 122 | + const ch = src[i]!; | |
| 123 | + if (ch === "\\") { | |
| 124 | + out += src.slice(i, i + 2); | |
| 125 | + i += 2; | |
| 126 | + continue; | |
| 127 | + } | |
| 128 | + if (ch === "`") { | |
| 129 | + let run = 1; | |
| 130 | + while (src[i + run] === "`") run++; | |
| 131 | + const fence = "`".repeat(run); | |
| 132 | + let close = src.indexOf(fence, i + run); | |
| 133 | + while (close >= 0 && src[close + run] === "`") { | |
| 134 | + let skip = close; | |
| 135 | + while (src[skip] === "`") skip++; | |
| 136 | + close = src.indexOf(fence, skip); | |
| 137 | + } | |
| 138 | + scanned += (close < 0 ? n : close) - i; | |
| 139 | + const end = close < 0 ? i + run : close + run; | |
| 140 | + out += src.slice(i, end); | |
| 141 | + i = end; | |
| 142 | + continue; | |
| 143 | + } | |
| 144 | + if (ch === "<" && src.startsWith("<!--", i) && lastClose >= i + 2) { | |
| 145 | + const close = src.indexOf("-->", i + 2); | |
| 146 | + scanned += close - i; | |
| 147 | + i = close + 3; | |
| 148 | + continue; | |
| 149 | + } | |
| 150 | + out += ch; | |
| 151 | + i++; | |
| 152 | + } | |
| 153 | + return out; | |
| 154 | +} | |
| 155 | + | |
| 108 | 156 | function findClosing(src: string, delim: string, from: number, wordy: boolean): number { |
| 109 | 157 | let at = from; |
| 110 | 158 | const limit = Math.min(src.length, from + MAX_SCAN * 2); |
| @@ -205,8 +253,9 @@ | ||
| 205 | 253 | |
| 206 | 254 | const BARE_URL = /^https?:\/\/[^\s<>"'`]*[^\s<>"'`.,:;!?)\]}*_~]/i; |
| 207 | 255 | |
| 208 | -function inline(src: string, opts: MarkdownOptions, depth = 0): string { | |
| 209 | - if (src.length > MAX_INLINE || depth > MAX_DEPTH) return escapeHtml(src); | |
| 256 | +function inline(text: string, opts: MarkdownOptions, depth = 0): string { | |
| 257 | + if (text.length > MAX_INLINE || depth > MAX_DEPTH) return escapeHtml(text); | |
| 258 | + const src = stripComments(text); | |
| 210 | 259 | let out = ""; |
| 211 | 260 | let i = 0; |
| 212 | 261 | const n = src.length; |
| @@ -363,6 +412,7 @@ | ||
| 363 | 412 | const HEADING = /^ {0,3}(#{1,6})(?:[ \t]([^]*))?$/; |
| 364 | 413 | const HR = /^ {0,3}([-*_])(?:[ \t]*\1){2,}[ \t]*$/; |
| 365 | 414 | const QUOTE = /^ {0,3}> ?([^]*)$/; |
| 415 | +const COMMENT = /^ {0,3}<!--/; | |
| 366 | 416 | const LIST_ITEM = /^( {0,12})([*+-]|\d{1,9}[.)])([ \t]+([^]*))?$/; |
| 367 | 417 | const SEP_CELL = /^:?-+:?$/; |
| 368 | 418 | |
| @@ -424,6 +474,7 @@ | ||
| 424 | 474 | HEADING.test(line) || |
| 425 | 475 | HR.test(line) || |
| 426 | 476 | QUOTE.test(line) || |
| 477 | + COMMENT.test(line) || | |
| 427 | 478 | /^ {0,3}([*+-]|1[.)])[ \t]+\S/.test(line) || |
| 428 | 479 | (line.includes("|") && next !== undefined && tableSep(next)) |
| 429 | 480 | ); |
| @@ -469,10 +520,24 @@ | ||
| 469 | 520 | parts.push(`<pre><code>${escapeHtml(body.join("\n"))}</code></pre>`); |
| 470 | 521 | continue; |
| 471 | 522 | } |
| 523 | + // HTML comments, hidden as GitHub hides them. A line that starts with "<!--" opens one that | |
| 524 | + // runs to the line holding "-->"; anything after the close on that line is rendered as usual. | |
| 525 | + // Left open, it hides the rest of the document (or of the list item or quote it is in). | |
| 526 | + const comment = COMMENT.exec(line); | |
| 527 | + if (comment) { | |
| 528 | + let j = i; | |
| 529 | + let close = line.indexOf("-->", comment[0].length - 2); | |
| 530 | + while (close < 0 && ++j < lines.length) close = lines[j]!.indexOf("-->"); | |
| 531 | + if (close < 0) break; | |
| 532 | + const rest = lines[j]!.slice(close + 3).trimStart(); | |
| 533 | + if (rest) lines[j] = rest; | |
| 534 | + i = rest ? j : j + 1; | |
| 535 | + continue; | |
| 536 | + } | |
| 472 | 537 | const h = HEADING.exec(line); |
| 473 | 538 | if (h) { |
| 474 | 539 | const level = h[1]!.length; |
| 475 | - const text = headingText(h[2] ?? ""); | |
| 540 | + const text = stripComments(headingText(h[2] ?? "")); | |
| 476 | 541 | const html = inline(text, opts, depth); |
| 477 | 542 | const id = escapeHtml(`${opts.idPrefix ?? "h-"}${slugify(text)}`); |
| 478 | 543 | parts.push(`<h${level} id="${id}"><a class="anchor" href="#${id}" aria-hidden="true">#</a>${html}</h${level}>`); |
| @@ -578,7 +643,7 @@ | ||
| 578 | 643 | // Setext headings (checked before "---" is taken as a thematic break). |
| 579 | 644 | if (/^ {0,3}=+\s*$/.test(lines[i]!) || /^ {0,3}-+\s*$/.test(lines[i]!)) { |
| 580 | 645 | const level = lines[i]!.includes("=") ? 1 : 2; |
| 581 | - const text = para.join(" "); | |
| 646 | + const text = stripComments(para.join(" ")); | |
| 582 | 647 | const id = escapeHtml(`${opts.idPrefix ?? "h-"}${slugify(text)}`); |
| 583 | 648 | para.length = 0; |
| 584 | 649 | parts.push(`<h${level} id="${id}"><a class="anchor" href="#${id}" aria-hidden="true">#</a>${inline(text, opts, depth)}</h${level}>`); |
| @@ -608,6 +673,40 @@ | ||
| 608 | 673 | return [...out]; |
| 609 | 674 | } |
| 610 | 675 | |
| 676 | +/** | |
| 677 | + * Markdown without its HTML comments, for plain-text views such as email. Comments inside | |
| 678 | + * fenced code and code spans stay. | |
| 679 | + */ | |
| 680 | +export function stripHtmlComments(source: string): string { | |
| 681 | + if (!source.includes("<!--") || source.length > MAX_SOURCE) return source; | |
| 682 | + scanned = 0; | |
| 683 | + budget = scanBudget(source.length); | |
| 684 | + const out: string[] = []; | |
| 685 | + let prose: string[] = []; | |
| 686 | + let fence: RegExp | null = null; | |
| 687 | + const flush = () => { | |
| 688 | + if (prose.length) out.push(stripComments(prose.join("\n"))); | |
| 689 | + prose = []; | |
| 690 | + }; | |
| 691 | + for (const line of source.split("\n")) { | |
| 692 | + if (fence) { | |
| 693 | + out.push(line); | |
| 694 | + if (fence.test(line)) fence = null; | |
| 695 | + continue; | |
| 696 | + } | |
| 697 | + const open = FENCE.exec(line); | |
| 698 | + if (open) { | |
| 699 | + flush(); | |
| 700 | + out.push(line); | |
| 701 | + fence = new RegExp(`^ {0,3}${open[1]![0] === "`" ? "`" : "~"}{${open[1]!.length},}\\s*$`); | |
| 702 | + continue; | |
| 703 | + } | |
| 704 | + prose.push(line); | |
| 705 | + } | |
| 706 | + flush(); | |
| 707 | + return out.join("\n").replace(/\n[ \t]*(?:\n[ \t]*){2,}/g, "\n\n").trim(); | |
| 708 | +} | |
| 709 | + | |
| 611 | 710 | /** Render Markdown to sanitized HTML. */ |
| 612 | 711 | export function renderMarkdown(source: string, opts: MarkdownOptions = {}): string { |
| 613 | 712 | if (source.length > MAX_SOURCE) return `<pre>${escapeHtml(source)}</pre>`; |
modifiedapps/diffinery/test/unit/email.test.ts+13−0View file
| @@ -151,3 +151,16 @@ | ||
| 151 | 151 | expect(await readUnsubToken({ ...env, MASTER_KEY: "another-key" } as Env, t)).toBeNull(); |
| 152 | 152 | }); |
| 153 | 153 | }); |
| 154 | + | |
| 155 | +describe("Markdown in email", () => { | |
| 156 | + it("hides HTML comments in the HTML and plain-text parts", async () => { | |
| 157 | + const thread = samples.find((s) => s.id === "comment")!.job; | |
| 158 | + if (thread.kind !== "thread") throw new Error("expected a thread sample"); | |
| 159 | + const job = { ...thread, data: { ...thread.data, body: '<!-- claude-projects-attribution: {"github_login":"diffy"} -->\n_Requested by Diffy_\n\nLooks good.' } } as MailJob; | |
| 160 | + const msg = await buildMessage(env, job, { email: "diffy@example.com", name: "Diffy", login: "diffy", accountId: "acc1" }); | |
| 161 | + for (const part of [msg.html, msg.text]) { | |
| 162 | + expect(part).not.toContain("claude-projects-attribution"); | |
| 163 | + expect(part).toContain("Looks good."); | |
| 164 | + } | |
| 165 | + }); | |
| 166 | +}); |
modifiedapps/diffinery/test/unit/markdown-safety.test.ts+57−1View file
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { describe, expect, it } from "vitest"; |
| 2 | -import { renderMarkdown, safeUrl } from "../../src/web/markdown.js"; | |
| 2 | +import { renderMarkdown, safeUrl, stripHtmlComments } from "../../src/web/markdown.js"; | |
| 3 | 3 | import { localPath, safeNext } from "../../src/web/auth.js"; |
| 4 | 4 | |
| 5 | 5 | const spaces = " ".repeat(50_000); |
| @@ -90,3 +90,59 @@ | ||
| 90 | 90 | expect(renderMarkdown("one \ntwo\nthree \nfour")).toBe("<p>one<br>\ntwo\nthree<br>\nfour</p>"); |
| 91 | 91 | }); |
| 92 | 92 | }); |
| 93 | + | |
| 94 | +describe("HTML comments", () => { | |
| 95 | + const attribution = '<!-- claude-projects-attribution: {"github_login":"someone"} -->\n_Requested by **Someone**_\n\nBody.'; | |
| 96 | + | |
| 97 | + it("hides a comment on its own lines, as GitHub does", () => { | |
| 98 | + expect(renderMarkdown(attribution)).toBe("<p><em>Requested by <strong>Someone</strong></em></p>\n<p>Body.</p>"); | |
| 99 | + expect(renderMarkdown("Before.\n\n<!--\nhidden\n\nstill hidden\n-->\n\nAfter.")).toBe("<p>Before.</p>\n<p>After.</p>"); | |
| 100 | + expect(renderMarkdown("<!---->\n<!-->\n<!--->\nShown.")).toBe("<p>Shown.</p>"); | |
| 101 | + }); | |
| 102 | + | |
| 103 | + it("ends a paragraph at a comment line and renders what follows the close", () => { | |
| 104 | + expect(renderMarkdown("One\n<!-- x -->\nTwo")).toBe("<p>One</p>\n<p>Two</p>"); | |
| 105 | + expect(renderMarkdown("<!-- a\nb --> **after**")).toBe("<p><strong>after</strong></p>"); | |
| 106 | + }); | |
| 107 | + | |
| 108 | + it("hides comments inside text, headings, lists, quotes and tables", () => { | |
| 109 | + expect(renderMarkdown("a <!-- b --> c")).toBe("<p>a c</p>"); | |
| 110 | + expect(renderMarkdown("a <!-- b\nc --> d")).toBe("<p>a d</p>"); | |
| 111 | + expect(renderMarkdown("# Title <!-- note -->")).toBe('<h1 id="h-title"><a class="anchor" href="#h-title" aria-hidden="true">#</a>Title </h1>'); | |
| 112 | + expect(renderMarkdown("- one <!-- x -->\n- two")).toBe("<ul><li>one </li><li>two</li></ul>"); | |
| 113 | + expect(renderMarkdown("> <!-- x -->\n> quoted")).toBe("<blockquote><p>quoted</p></blockquote>"); | |
| 114 | + expect(renderMarkdown("| a <!-- x --> |\n| - |\n| b |")).toContain("<th>a </th>"); | |
| 115 | + expect(renderMarkdown("*a <!-- * --> b*")).toBe("<p><em>a b</em></p>"); | |
| 116 | + }); | |
| 117 | + | |
| 118 | + it("keeps comments in code, escaped and unclosed ones as text", () => { | |
| 119 | + expect(renderMarkdown("```html\n<!-- kept -->\n```")).toContain("<!-- kept -->"); | |
| 120 | + expect(renderMarkdown(" <!-- kept -->")).toBe("<pre><code><!-- kept --></code></pre>"); | |
| 121 | + expect(renderMarkdown("`<!-- kept -->`")).toBe("<p><code><!-- kept --></code></p>"); | |
| 122 | + expect(renderMarkdown("\\<!-- kept -->")).toBe("<p><!-- kept --></p>"); | |
| 123 | + expect(renderMarkdown("a <!-- open")).toBe("<p>a <!-- open</p>"); | |
| 124 | + }); | |
| 125 | + | |
| 126 | + it("hides the rest of the document after a comment line that never closes, as GitHub does", () => { | |
| 127 | + expect(renderMarkdown("Shown.\n\n<!-- open\n\nhidden")).toBe("<p>Shown.</p>"); | |
| 128 | + }); | |
| 129 | + | |
| 130 | + it("never lets markup through around a comment", () => { | |
| 131 | + const html = renderMarkdown('<!-- x --><script>alert(1)</script>\n\na <!-- y --><img src=x onerror=alert(1)> b'); | |
| 132 | + expect(html).not.toMatch(/<(script|img)\b/); | |
| 133 | + expect(html).toContain("<script>"); | |
| 134 | + }); | |
| 135 | + | |
| 136 | + it("renders many comment openers in linear time", () => { | |
| 137 | + expect(timed("a <!-- ".repeat(40_000)).ms).toBeLessThan(500); | |
| 138 | + expect(timed("a <!-- b --> ".repeat(30_000)).ms).toBeLessThan(500); | |
| 139 | + expect(timed("<!-- x -->\n".repeat(40_000)).ms).toBeLessThan(500); | |
| 140 | + expect(timed("`` <!-- ` ".repeat(30_000) + "-->").ms).toBeLessThan(500); | |
| 141 | + }); | |
| 142 | + | |
| 143 | + it("strips comments from plain text, outside code", () => { | |
| 144 | + expect(stripHtmlComments(attribution)).toBe("_Requested by **Someone**_\n\nBody."); | |
| 145 | + expect(stripHtmlComments("a\n\n<!-- x -->\n\nb `<!-- y -->`\n```\n<!-- z -->\n```")).toBe("a\n\nb `<!-- y -->`\n```\n<!-- z -->\n```"); | |
| 146 | + expect(stripHtmlComments("no comments\n\n\n\nhere")).toBe("no comments\n\n\n\nhere"); | |
| 147 | + }); | |
| 148 | +}); |