From cc029b0bc9790bcd5cd237045ef51d4ce3056c84 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 23 Oct 2026 13:43:01 +0200 Subject: [PATCH] Write a URL as the ADF holds it, whatever its scheme; file item 142 to weigh carrying script URLs --- AGENTS.md | 2 +- CHANGELOG.md | 3 +-- README.md | 4 ++-- docs/decisions.md | 16 ++++++---------- spec/html.md | 15 +++++++-------- src/html/emit/adf-to-html.test.ts | 16 ++++++---------- src/html/emit/adf-to-html.ts | 7 ++----- todo.md | 18 +++++++++++++++++- 8 files changed, 42 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fa24a39..2872d3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,7 +67,7 @@ In `docs/decisions.md`: - The flavour spec is read as a source - The node tables answer to Atlassian's schema - Output past V8's longest string is refused on every engine -- A URL a browser would run is carried, never written +- HTML writes a URL as the ADF holds it, whatever its scheme - Nothing recurses unbounded - JSON text is read by `parseJsonText` - No value `JSON.parse` or `structuredClone` builds loops a call diff --git a/CHANGELOG.md b/CHANGELOG.md index 34d3906..8cc8ad9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,7 @@ - `adfToHtml` writes ADF as HTML: paragraphs, headings, lists, quotes, code blocks, rules, hard breaks, external images, and `em`, `strong`, `code` and links as their elements. Every other node - is an element of class `adf-carry` holding the node's JSON in `data-adf` and showing its text. A - link or image whose URL a browser would run, such as `javascript:`, rides that element too. + is an element of class `adf-carry` holding the node's JSON in `data-adf` and showing its text. - **Breaking:** `losslessMarkdownToAdf` gives a `date`, `emoji`, `inlineCard`, `mention`, `status` or `mediaInline` only the marks Atlassian's schema lets it hold: `**!adf:mention[@A]{id=…}**` now reads as a plain mention. A link or directive mark around only nodes that cannot take it is diff --git a/README.md b/README.md index 3c7c375..e3ecf1a 100644 --- a/README.md +++ b/README.md @@ -469,8 +469,8 @@ ships. The complete dialect: - Text and attribute values are escaped: `&`, `<`, `>`, `"` and a carriage return. - The carry element shows a mention, status or emoji by its text, a date as `2026-09-13` in UTC, a card by its URL, an image by its `alt`, else its URL, and an expand by its title. -- A link or image whose URL a browser would run or read as a file, such as `javascript:`, rides - the carry element, so the HTML follows no such URL. +- A link's or image's URL is written as the ADF holds it, `javascript:` included: sanitize the + HTML before showing a document from an author you do not trust. ## The errors diff --git a/docs/decisions.md b/docs/decisions.md index beb75d0..e5eaeff 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -801,18 +801,14 @@ carry's two-space JSON, which indents every value its depth over, stops at the b JavaScriptCore builds a joined block up to its own longest string, 2^31 − 1, before the check. Past that, JavaScriptCore throws `Out of memory`, which is `engine-limit` (§Which code a cause takes). -## A URL a browser would run is carried, never written +## HTML writes a URL as the ADF holds it, whatever its scheme -2026-10-23, written with `adfToHtml`, for the maintainer's review. Goals 1 and 6: an export tool's -HTML runs no script. Valid while a consumer may show the HTML in a browser without sanitizing it. +2026-10-23, the maintainer. Goal 1. Valid until `todo.md` item 142 weighs the alternatives. -`adfToHtml` writes no `href` or `src` a browser would run as script or read as a local file: the -link's text or the image rides the carry element, whose `data-adf` no browser follows, so nothing -is lost. The list is cmark's, the CommonMark reference implementation's, which leaves these URLs -out of its HTML by default: `javascript:`, `vbscript:`, `file:`, and `data:` other than a PNG, GIF, -JPEG or WebP image, any case. A browser drops tabs and line breaks anywhere in a URL, and leading -control characters and spaces, before it reads the scheme (the WHATWG URL standard), so the check -reads the URL so too: a tab inside `javascript:` still runs. +`adfToHtml` writes a link's `href` and an image's `src` exactly as the ADF holds them, escaped as +every attribute value is, `javascript:`, `vbscript:`, `file:` and `data:` included. Carrying such a +link or image in the carry element, as cmark's default leaves these URLs out of its HTML, was +proposed and not taken: item 142 weighs that against writing the URL. ## Nothing recurses unbounded diff --git a/spec/html.md b/spec/html.md index 8aaea9e..71f564e 100644 --- a/spec/html.md +++ b/spec/html.md @@ -41,7 +41,9 @@ marks are two `` elements. Text and attribute values escape `&`, `<`, `>` and `"` as `&`, `<`, `>` and `"`, and a carriage return as ` `, which an HTML parser would otherwise read as a line feed. -Attribute values are double-quoted. +Attribute values are double-quoted. A link's `href` and an image's `src` are the URL as the ADF +holds it, whatever its scheme (`docs/decisions.md` §HTML writes a URL as the ADF holds it, whatever +its scheme). ## The carry element @@ -58,18 +60,15 @@ the node's text. included. A document holding no `content` key is the block carry of `{"type":"doc","version":1}`. - The text shown is every text node's text below the node, and what a node holding no text shows in its place: a mention's text, else `@` and its id; an emoji's text, else its `shortName`; a - status's text; a date as `2026-09-13` in UTC; a card's URL, else its name, else the URL its data holds; media's `alt`, else an - external one's URL; an extension's text; an expand's title. A line feed parts two blocks and - stands for a hard break. Null characters are dropped. + status's text; a date as `2026-09-13` in UTC; a card's URL, else its name, else the URL its data + holds; media's `alt`, else an external one's URL; an extension's text; an expand's title. A line + feed parts two blocks and stands for a hard break. Null characters are dropped. - A node rides the carry where it holds an attribute, mark or key its element does not write, an empty `attrs`, `content` or `marks`, or a value its element cannot hold: - text holding no character, or a null character, which HTML cannot hold; text holding a mark twice, or a mark with attributes besides a link's `href` and `title`; - an unmarked text node straight after another, which would read back as one; - - a link or an image whose URL a browser would run or read as a file: a `javascript:`, - `vbscript:` or `file:` URL, or `data:` other than a PNG, GIF, JPEG or WebP image, read after - dropping tabs and line breaks and a leading control character or space (`docs/decisions.md` - §A URL a browser would run is carried, never written); + - a link or an image whose URL holds a null character; - a code block holding more than one text node, a marked one, or a language holding whitespace; - a list holding a node that is no `listItem`, or an item holding attributes or marks: the whole list rides the carry; diff --git a/src/html/emit/adf-to-html.test.ts b/src/html/emit/adf-to-html.test.ts index dff1e05..b821cad 100644 --- a/src/html/emit/adf-to-html.test.ts +++ b/src/html/emit/adf-to-html.test.ts @@ -123,16 +123,12 @@ test('carries inline what its element cannot hold, and a text node an unmarked o assert.equal(carriedSpans(paragraph(text('x'))), 1) }) -test('carries a link or image whose URL a browser would run as script, or read as a file or non-image data', () => { +test('writes a link\'s or image\'s URL as the ADF holds it, escaped, whatever its scheme', () => { const linked = (href: string): string => html(adfToHtml(document(paragraph(text('x', { attrs: { href }, type: 'link' }))))) - for (const href of ['javascript:alert(1)', 'JavaScript:alert(1)', ' \u0001java\tscript:alert(1)', 'vbscript:x', 'file:///etc/passwd', 'data:text/html,x', 'data:image/svg+xml,x']) { - assert.equal(linked(href).startsWith('

x

\n') + assert.equal(linked('data:text/html,x'), '

x

\n') + const image: AdfNode = { attrs: { layout: 'center' }, content: [{ attrs: { type: 'external', url: 'vbscript:x' }, type: 'media' }], type: 'mediaSingle' } + assert.equal(html(adfToHtml(document(image))), '

\n') }) test('writes a container holding no content key as its element holding nothing', () => { @@ -144,7 +140,7 @@ test('carries a link or image whose target the anchor cannot hold', () => { assert.equal(carried({ attrs: { title: 'No target' }, type: 'link' }), true) assert.equal(carried({ attrs: { href: 'https://example.com/\0' }, type: 'link' }), true) assert.equal(carried({ attrs: { href: 'https://example.com/', title: 'x\0' }, type: 'link' }), true) - const image: AdfNode = { attrs: { layout: 'center' }, content: [{ attrs: { type: 'external', url: 'https://example.com/a.png' }, marks: [{ attrs: { href: 'javascript:alert(1)' }, type: 'link' }], type: 'media' }], type: 'mediaSingle' } + const image: AdfNode = { attrs: { layout: 'center' }, content: [{ attrs: { type: 'external', url: 'https://example.com/a.png' }, marks: [{ attrs: { href: 'https://example.com/\0' }, type: 'link' }], type: 'media' }], type: 'mediaSingle' } assert.equal(html(adfToHtml(document(image))).startsWith('
{ diff --git a/todo.md b/todo.md index 758ca3f..edb4a21 100644 --- a/todo.md +++ b/todo.md @@ -6,12 +6,13 @@ `Bar = 0.1` -`Next ID = 142` +`Next ID = 143` ## Items | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| +| 142 | 0.3.0 | | security | **Settle in `docs/decisions.md`, weighing the pros and cons with the maintainer, whether `adfToHtml` writes a `javascript:`, `vbscript:`, `file:` or non-image `data:` URL as the ADF holds it.** | 2 | 2 | 2 | 4 | 3 | 6 by 3 | 4.5 | | 131 | 0.3.0 | | | **Ship `htmlToAdf`, reading back exactly what `adfToHtml` writes and refusing any other HTML as `unmappable-html`.** | 4 | 1 | 2 | 5 | 7 | 3 by 3 | 0.6 | | 133 | 0.3.0 | 131 | | **Write mentions, emoji, dates, statuses and inline cards as semantic HTML in `adfToHtml` and read them back in `htmlToAdf`, in place of the carry element.** | 2 | 2 | 2 | 4 | 4 | 6 by 3 | 3.0 | | 132 | 0.3.0 | 131 | | **Write tables as `` in `adfToHtml` and read them back in `htmlToAdf`, in place of the carry element.** | 2 | 2 | 2 | 4 | 5 | 6 by 4 | 2.4 | @@ -62,6 +63,21 @@ ## Details +### 142. Settle in `docs/decisions.md`, weighing the pros and cons with the maintainer, whether `adfToHtml` writes a `javascript:`, `vbscript:`, `file:` or non-image `data:` URL as the ADF holds it. + +The exposure: ADF from an author a consumer does not trust can hold a link's `href` or an external +image's `url` of `javascript:…`. `adfToHtml` writes it into `href` or `src` as held +(`docs/decisions.md` §HTML writes a URL as the ADF holds it, whatever its scheme). An export tool +showing that HTML unsanitized in a browser runs the script when a reader follows the link. + +Weigh writing the URL as held, leaving sanitizing to the consumer as for any HTML, against carrying +such a link or image in the carry element, which keeps the URL in `data-adf`. cmark, the CommonMark +reference implementation, leaves `javascript:`, `vbscript:`, `file:` and `data:` other than a PNG, +GIF, JPEG or WebP image out of its HTML by default; a browser reads a scheme after dropping tabs, +line breaks and leading control characters. Say whether the markdown writers' links follow the +same rule. Ask the maintainer with the pros and cons, and record the answer in place of that +decision. + ### 131. Ship `htmlToAdf`, reading back exactly what `adfToHtml` writes and refusing any other HTML as `unmappable-html`. A hand-written HTML tokenizer, linear in the input, its nesting bounded (`docs/decisions.md`