diff --git a/scripts/lib/mdx-active-content.mjs b/scripts/lib/mdx-active-content.mjs index 4f27e499..9da11d48 100644 --- a/scripts/lib/mdx-active-content.mjs +++ b/scripts/lib/mdx-active-content.mjs @@ -19,6 +19,21 @@ * MDX evaluates a braced expression as JavaScript, so any expression node is a * finding unless it is one of the inert string-literal attributes the importer * emits itself. + * + * The scan answers the second of the two questions `uri-safety.mjs` states + * both gates must answer -- does the value make a visitor's browser contact + * another host -- only for destinations the browser fetches with no user + * action, which on this surface means images. An imported page is written + * verbatim from a repository this project does not control, so a remote `` + * in one is a third-party beacon that sees every visitor's IP, User-Agent and + * Referer on page load. `rewriteImages()` already demotes such a destination at + * import time, but that is a gate on what is *imported*; this is the gate on + * what is *published*, and it has to hold when a page reaches `docs/` without + * passing the importer. + * + * Ordinary links are deliberately left alone. A link is followed only when a + * reader clicks it, and `rewriteImages()` demotes a remote image *to* a remote + * link, so rejecting them would reject every imported page. */ import { fromMarkdown } from 'mdast-util-from-markdown'; @@ -26,7 +41,7 @@ import { mdxjs } from 'micromark-extension-mdxjs'; import { mdxFromMarkdown } from 'mdast-util-mdx'; import { gfm } from 'micromark-extension-gfm'; import { gfmFromMarkdown } from 'mdast-util-gfm'; -import { activeScheme } from './uri-safety.mjs'; +import { activeScheme, describeTarget, remoteTarget } from './uri-safety.mjs'; /** Inert inline elements that carry no script, network or layout capability. */ const ALLOWED_ELEMENTS = new Set([ @@ -65,6 +80,18 @@ const EVENT_HANDLER_ATTRIBUTE = /^on[a-z]+$/i; /** Link-like nodes whose `url` a browser navigates to or loads. */ const URL_NODES = new Set(['link', 'definition', 'image']); +/** + * The only attribute of the only allowed component that reaches an ``. + * + * `CNCFProjectCard` renders `logo` through `useBaseUrl()`, which returns a + * value carrying a protocol unchanged, so an absolute URL there is loaded + * off-origin exactly as a Markdown image destination is. `href` on the same + * card is remote by contract (`https://www.cncf.io/projects/...`) and is + * followed only on a click, so attributes are checked by name rather than as a + * class. + */ +const REMOTE_IMAGE_ATTRIBUTES = new Set(['logo']); + const parserOptions = { extensions: [mdxjs(), gfm()], mdastExtensions: [mdxFromMarkdown(), gfmFromMarkdown()], @@ -118,6 +145,24 @@ export function findActiveContent(markdown) { return findings; } + // Collected before the scan: a definition may be written after the + // reference that uses it, and an `imageReference` carries only an + // identifier. The definition node itself is never reported as remote -- + // `linkReference` resolves through the same map, and an imported page + // legitimately carries remote link definitions. + const definitions = new Map(); + eachNode(tree, (node) => { + if (node.type === 'definition') definitions.set(node.identifier, node.url); + }); + + // Every caller passes a string: an image node's `url`, a definition `url` + // resolved from the map, or an attribute value already narrowed to a string. + const reportRemoteImage = (line, url) => { + const target = remoteTarget(url); + if (target) + report(line, `remote image destination ${describeTarget(target)}`); + }; + eachNode(tree, (node) => { const line = node.position.start.line; @@ -143,8 +188,13 @@ export function findActiveContent(markdown) { value = literalValue(value.value); if (value === null) report(line, 'MDX expression'); } - if (typeof value === 'string' && activeScheme(value)) { - report(line, 'script-capable URL scheme'); + if (typeof value === 'string') { + if (activeScheme(value)) { + report(line, 'script-capable URL scheme'); + } + if (REMOTE_IMAGE_ATTRIBUTES.has(attribute.name)) { + reportRemoteImage(line, value); + } } } } else if ( @@ -156,8 +206,13 @@ export function findActiveContent(markdown) { if (node.value.trim() !== ALLOWED_IMPORT) { report(line, 'unexpected ESM statement'); } - } else if (URL_NODES.has(node.type) && activeScheme(node.url)) { - report(line, 'script-capable URL scheme'); + } else if (node.type === 'imageReference') { + const url = definitions.get(node.identifier); + // An unresolved reference renders as literal text and loads nothing. + if (url !== undefined) reportRemoteImage(line, url); + } else if (URL_NODES.has(node.type)) { + if (activeScheme(node.url)) report(line, 'script-capable URL scheme'); + if (node.type === 'image') reportRemoteImage(line, node.url); } }); diff --git a/tests/mdx-active-content.test.mjs b/tests/mdx-active-content.test.mjs index b30a2cc7..4f591634 100644 --- a/tests/mdx-active-content.test.mjs +++ b/tests/mdx-active-content.test.mjs @@ -419,3 +419,71 @@ test('reports a parse error on the line it occurs', () => { assert.equal(finding.line, 1); assert.equal(findActiveContent('')[0].line, 1); }); + +// The second half of the invariant stated in scripts/lib/uri-safety.mjs: an +// imported page is third-party content, and every in one is fetched by +// a visitor's browser with no user action, so a remote image destination is a +// beacon for their IP, User-Agent and Referer. +test('flags a remote image destination', () => { + assert.deepEqual(reasons('![p](https://evil.example/p.png)'), [ + 'remote image destination https://evil.example/p.png', + ]); + assert.deepEqual(reasons('![p](HTTP://Evil.Example/p.png)'), [ + 'remote image destination http://evil.example/p.png', + ]); + assert.deepEqual(reasons('![p](//evil.example/p.png)'), [ + 'remote image destination //evil.example/p.png', + ]); +}); + +test('resolves an image reference through its definition', () => { + assert.deepEqual(reasons('![p][x]\n\n[x]: https://evil.example/p.png'), [ + 'remote image destination https://evil.example/p.png', + ]); + // The definition may be written before the reference that uses it. + assert.deepEqual(reasons('[x]: https://evil.example/p.png\n\n![p][x]'), [ + 'remote image destination https://evil.example/p.png', + ]); + // A collapsed reference carries the label as its identifier. + assert.deepEqual(reasons('![evil][]\n\n[evil]: https://evil.example/p.png'), [ + 'remote image destination https://evil.example/p.png', + ]); +}); + +test('leaves an unresolved image reference alone', () => { + // Nothing resolves it, so it renders as literal text and loads nothing. + assert.deepEqual(reasons('![p][missing]'), []); +}); + +test('leaves links, link definitions and local images alone', () => { + // rewriteImages() demotes a remote image *to* a remote link, and + // docs/architectures/colopl.md carries remote link definitions, so a gate on + // either would reject every imported page. + assert.deepEqual(reasons('[x](https://good.example/)'), []); + assert.deepEqual(reasons('[x][d]\n\n[d]: https://good.example/'), []); + assert.deepEqual(reasons('![p](/img/architectures/acme/p.png)'), []); + assert.deepEqual(reasons('![p](images/p.png)'), []); + assert.deepEqual(reasons('![p](data:image/png;base64,AAAA)'), []); +}); + +test('flags a remote logo on the allowed component but not its remote href', () => { + const card = (attributes) => + reasons(``); + assert.deepEqual(card('logo={"https://evil.example/l.svg"}'), [ + 'remote image destination https://evil.example/l.svg', + ]); + assert.deepEqual(card('logo="https://evil.example/l.svg"'), [ + 'remote image destination https://evil.example/l.svg', + ]); + assert.deepEqual(card('logo={"/img/cncf-projects/k.svg"}'), []); + assert.deepEqual(card('href={"https://www.cncf.io/projects/k/"}'), []); +}); + +test('truncates a long remote image destination in the reason', () => { + const url = `https://evil.example/${'a'.repeat(200)}.png`; + const [finding] = findActiveContent(`![p](${url})`); + assert.equal( + finding.reason, + `remote image destination ${url.slice(0, 117)}...`, + ); +});