fix(security): parser-based SVG/MDX scanning, replacing per-bypass regex patches

CONTENT-SECURITY.md

@@ -27,6 +27,39 @@ how to fix a rejected submission.
   User-Agent, and Referer to a host the submitter chose. All imported artwork is
   mirrored locally for exactly this reason.
 
+## How the scanners work
+
+The scanners read a parse tree; they do not pattern-match text. SVG is parsed
+with [`sax`](https://github.com/isaacs/sax-js) as strict XML, CSS inside SVG is
+tokenized with the [`css-tree`](https://github.com/csstree/csstree) tokenizer
+(so escapes, comments, and quote pairing are resolved the way a browser resolves
+them), and Markdown/MDX is parsed with the same micromark grammar Docusaurus
+compiles pages with. Shared URI rules live in `scripts/lib/uri-safety.mjs`.
+
+Two consequences matter when a submission is rejected:
+
+- **A document the parser rejects is itself a finding.** An unquoted attribute,
+  an unclosed tag, an undefined entity such as ` `, or an unterminated
+  DOCTYPE is reported as `is not well-formed XML`. A scanner that cannot read a
+  file cannot vouch for it, and a browser may read it differently. Fix the
+  markup rather than working around the message.
+- **Stripping rebuilds the document from the parse** instead of deleting
+  character ranges, so removing one element cannot join the text around it into
+  a new one.
+
+### Residual risks and compensating controls
+
+- The site's Content-Security-Policy is delivered as a `<meta>` tag, which
+  browsers do not apply to a file opened directly (for example a `.svg` URL).
+  The parser-based gates above are therefore the primary control, and the CSP is
+  defense in depth.
+- A parser can still disagree with a browser on exotic input. The gates fail
+  closed on anything the parser rejects, and every historical bypass is kept as
+  a regression fixture in `tests/svg-active-content.test.mjs` and
+  `tests/mdx-active-content.test.mjs`. A new bypass belongs in those fixtures
+  and, if it needs a new rule, in the parser walk, not in a new regular
+  expression.
+
 ## SVG rules (`active content: ...` findings)
 
 `validate:architecture-assets` rejects a diagram that contains any of the
@@ -38,9 +71,9 @@ characters, and whitespace obfuscation, so re-spelling a construct does not help
   `<listener>`, `<iframe>`, `<embed>`, `<object>`.
 - **Event-handler attributes**: any `on*` attribute (`onload`, `onclick`, ...).
 - **Script-executing URI schemes** in any attribute: `javascript:`, `vbscript:`,
-  `livescript:`, `mocha:`, and `data:` URIs carrying a markup media type
-  (`text/html`, `image/svg+xml`, `application/xhtml+xml`, `text/xml`,
-  `application/xml`).
+  `livescript:`, `mocha:`, and `data:` URIs carrying a markup or script media
+  type (`text/html`, `text/xml`, `application/xml`, `text/xsl`, any `+xml` type
+  such as `image/svg+xml`, and JavaScript types).
 - **Embedded-document attributes**: `srcdoc`.
 - **`<animate>`/`<set>` targeting `href`**: an animation can install a script
   URI at runtime on an element that looks inert in the source.
@@ -97,7 +130,9 @@ the check fails closed.
 Image references in page bodies are rewritten during import: relative paths are
 scoped to the architecture's own asset directory, and an image whose destination
 names any host — including protocol-relative `//host/...` — is demoted to a
-plain link unless it is a cncf/artwork URL the importer can mirror.
+plain link unless it is a cncf/artwork URL the importer can mirror. Images are
+found in the parse tree, so reference-style images (`![alt][ref]`) follow the
+same rules as inline ones.
 
 ## Project-card logo policy
 

package.json

@@ -89,6 +89,7 @@
     "@docusaurus/preset-classic": "3.10.2",
     "@mdx-js/react": "^3.1.1",
     "clsx": "^2.1.1",
+    "css-tree": "^2.3.1",
     "docusaurus-plugin-search-local": "^2.1.2",
     "mdast-util-from-markdown": "^2.0.3",
     "mdast-util-gfm": "^3.1.0",
@@ -97,7 +98,8 @@
     "micromark-extension-mdxjs": "^3.0.0",
     "prism-react-renderer": "^2.4.1",
     "react": "^19.3.0",
-    "react-dom": "^19.3.0"
+    "react-dom": "^19.3.0",
+    "sax": "^1.6.1"
   },
   "devDependencies": {
     "@axe-core/playwright": "^4.13.0",

scripts/import-architectures.mjs

@@ -16,7 +16,6 @@ import { tmpdir } from 'node:os';
 import { fileURLToPath } from 'node:url';
 import {
   findActiveContent,
-  hasDoctype,
   stripActiveContent,
   stripDoctype,
 } from './lib/svg-active-content.mjs';
@@ -242,17 +241,10 @@ function sanitizeArchitectureAssets(dir) {
       );
     }
 
-    // Remove DOCTYPE declarations that can break XML consumers. stripDoctype()
-    // leaves a malformed declaration (unclosed subset or quoted literal) in
-    // place rather than guessing where it ends, and an unstripped external DTD
-    // reference is inert to the findActiveContent() rescan below -- so verify
-    // the removal happened instead of assuming it.
+    // Remove DOCTYPE declarations that can break XML consumers. A document
+    // with an unclosed or otherwise malformed one was already rejected by
+    // stripActiveContent(), so nothing is left to guess at here.
     source = stripDoctype(source);
-    if (hasDoctype(source)) {
-      throw new Error(
-        `DOCTYPE could not be removed from ${relative(join(root, 'static'), file)}`,
-      );
-    }
 
     // Strip draw.io/Excalidraw editable metadata to reduce bloat.
     source = source.replace(/\scontent\s*=\s*["'][^"']*["']/gi, '');
@@ -271,10 +263,10 @@ function sanitizeArchitectureAssets(dir) {
       }
     }
 
-    // stripActiveContent() verifies the string it returns, but the DOCTYPE and
-    // content-attribute removals above delete text, which can join two inert
-    // fragments into a live one ("<sc" + "ript>"). Verify the bytes that are
-    // actually written rather than the ones that were checked earlier.
+    // stripActiveContent() verifies the string it returns, but the
+    // content-attribute removal above is a text edit, which can leave a
+    // document that no longer parses. Verify the bytes that are actually
+    // written rather than the ones that were checked earlier.
     const residual = findActiveContent(source);
     if (residual.length) {
       throw new Error(

scripts/lib/architecture-content.mjs

@@ -12,6 +12,9 @@
 import { lstatSync, mkdirSync, writeFileSync } from 'node:fs';
 import { join } from 'node:path';
 import { parse as yamlParse } from 'yaml';
+import { fromMarkdown } from 'mdast-util-from-markdown';
+import { gfm } from 'micromark-extension-gfm';
+import { gfmFromMarkdown } from 'mdast-util-gfm';
 import {
   artworkMirrorPath,
   artworkPath,
@@ -86,51 +89,110 @@ export function renderProjectCards(body, id) {
 }
 
 /**
- * A Markdown link destination that carries its own authority: either an
- * absolute `scheme://host/...` or a protocol-relative `//host/...`, which
- * inherits the page's scheme and loads off-site exactly like an absolute one.
- *
- * Written as a character class rather than with the `i` flag so the rest of
- * each pattern it appears in stays case-sensitive.
+ * A Markdown destination that carries its own authority: either an absolute
+ * `scheme://host/...` or a protocol-relative `//host/...`, which inherits the
+ * page's scheme and loads off-site exactly like an absolute one.
  */
-const REMOTE_DESTINATION = '(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\\/\\/';
+const REMOTE_DESTINATION = /^(?:[a-z][a-z0-9+.-]*:)?\/\//i;
+
+/** Any URI scheme; a scheme-less destination is a path on this site. */
+const HAS_SCHEME = /^[a-z][a-z0-9+.-]*:/i;
+
+const markdownGrammar = {
+  extensions: [gfm()],
+  mdastExtensions: [gfmFromMarkdown()],
+};
+
+function escapeLabel(text) {
+  return text.replace(/[\\[\]]/g, '\\$&');
+}
+
+function destination(url) {
+  return /[\s()<>]/.test(url)
+    ? `<${url.replace(/[<>]/g, encodeURIComponent)}>`
+    : url;
+}
 
 /**
- * Strips remaining Hugo/Docsy shortcodes, rewrites image references to their
- * local `/img/architectures/<id>/...` or mirrored artwork path, and collapses
- * blank-line runs. `id` scopes relative image paths (`images/foo.png`) to the
- * architecture's own asset directory.
+ * Applies the image policy to every image in a parsed document.
+ *
+ * Images are found in the parse tree, not by pattern, so the reference style
+ * (`![alt][ref]` with `[ref]: url` elsewhere), nested brackets and titles are
+ * all seen as the parser sees them. Each image is replaced by exactly the
+ * source range it occupies.
  *
- * An image whose destination names a host is demoted to a plain link unless
- * projectAsset() can resolve it to a mirrored path. A published `<img>` is
- * fetched by every visitor's browser, so leaving one pointed at a third-party
- * host beacons their IP, User-Agent and Referer there -- the hot-linking the
- * mirror exists to prevent. The test for "names a host" is
- * REMOTE_DESTINATION, not `https?://`: a protocol-relative `//host/x.png`
- * inherits the page's scheme and loads off-site just the same, and an
- * uppercase `HTTPS://` is the same URL to a browser. Both slipped past the
- * old `https?://` test, and the local-path rewrite below then either left
- * them live or mangled them into a path that cannot resolve.
+ * - A destination that names a host (REMOTE_DESTINATION, which includes
+ *   protocol-relative and any-case schemes) is mirrored when projectAsset()
+ *   resolves it and otherwise demoted to a plain link, or to its alt text
+ *   inside an existing link. A published `<img>` is fetched by every visitor's
+ *   browser, so a third-party host would see their IP, User-Agent and Referer.
+ * - Any other scheme (`data:`, `javascript:`, ...) is reduced to its alt text.
+ * - A relative path is rebased onto `/img/architectures/<id>/`.
+ */
+function rewriteImages(body, id) {
+  const tree = fromMarkdown(body, markdownGrammar);
+  const definitions = new Map();
+  const images = [];
+
+  const walk = (node, inLink) => {
+    if (node.type === 'definition') definitions.set(node.identifier, node.url);
+    if (node.type === 'image' || node.type === 'imageReference') {
+      images.push({ node, inLink });
+    }
+    const nested =
+      inLink || node.type === 'link' || node.type === 'linkReference';
+    for (const child of node.children ?? []) walk(child, nested);
+  };
+  walk(tree, false);
+
+  const edits = [];
+  for (const { node, inLink } of images) {
+    const url =
+      node.type === 'image' ? node.url : definitions.get(node.identifier);
+    const { alt } = node;
+    let replacement;
+
+    if (REMOTE_DESTINATION.test(url)) {
+      const asset = projectAsset(url);
+      if (asset) replacement = `![${escapeLabel(alt)}](${asset})`;
+      else if (inLink) replacement = escapeLabel(alt);
+      else replacement = `[${escapeLabel(alt)}](${destination(url)})`;
+    } else if (HAS_SCHEME.test(url)) {
+      replacement = escapeLabel(alt);
+    } else if (url.startsWith('/')) {
+      if (node.type === 'image') continue;
+      replacement = `![${escapeLabel(alt)}](${destination(url)})`;
+    } else {
+      const rest = url.replace(/^\.\//, '').replace(/^images\//, '');
+      replacement = `![${escapeLabel(alt)}](${destination(`/img/architectures/${id}/${rest}`)})`;
+    }
+    edits.push({
+      start: node.position.start.offset,
+      end: node.position.end.offset,
+      replacement,
+    });
+  }
+
+  let output = body;
+  for (const edit of edits.sort((x, y) => y.start - x.start)) {
+    output =
+      output.slice(0, edit.start) + edit.replacement + output.slice(edit.end);
+  }
+  return output;
+}
+
+/**
+ * Strips remaining Hugo/Docsy shortcodes, applies the image policy described
+ * on {@link rewriteImages}, and collapses blank-line runs. `id` scopes
+ * relative image paths (`images/foo.png`) to the architecture's own asset
+ * directory.
  */
 export function cleanMarkdown(body, id) {
-  return body
-    .replace(/{{<[\s\S]*?>}}/g, '')
-    .replace(/{{<\/?[^>]+>}}/g, '')
-    .replace(
-      new RegExp(`!\\[([^\\]]*)\\]\\((${REMOTE_DESTINATION}[^\\)]+)\\)`, 'g'),
-      (_, alt, url) => {
-        const asset = projectAsset(url);
-        return asset ? `![${alt}](${asset})` : `[${alt}](${url})`;
-      },
-    )
+  return rewriteImages(
+    body.replace(/{{<[\s\S]*?>}}/g, '').replace(/{{<\/?[^>]+>}}/g, ''),
+    id,
+  )
     .replace(/\[\[([^\]]+)\]\((https?:\/\/[^\)]+)\)\]/g, '[$1]($2)')
-    .replace(
-      new RegExp(
-        `!\\[([^\\]]*)\\]\\((?!${REMOTE_DESTINATION})(?:\\.\\/)?(?:images\\/)?([^/][^\\)]*)\\)`,
-        'g',
-      ),
-      `![$1](/img/architectures/${id}/$2)`,
-    )
     .replace(/<>/g, '&lt;&gt;')
     .replace(/\n{3,}/g, '\n\n')
     .trim();

scripts/lib/enduser-collector.mjs

@@ -13,9 +13,7 @@ import {
 } from 'node:fs';
 import { basename, join, sep } from 'node:path';
 import {
-  findActiveContent,
   findRemoteReferences,
-  hasDoctype,
   stripActiveContent,
   stripDoctype,
 } from './svg-active-content.mjs';
@@ -177,27 +175,6 @@ export function mirrorLandscapeLogo({
         .map((line) => line.replace(/\s+$/, ''))
         .join('\n')
         .replace(/\s+$/, '');
-      // stripActiveContent() verifies the string it returns, but the DOCTYPE
-      // removal and trimming above delete text, which can join two inert
-      // fragments into a live one ("<sc" + "ript>"). Verify the bytes that are
-      // actually written rather than the ones that were checked earlier.
-      // hasDoctype() covers stripDoctype()'s documented failure mode: a
-      // malformed declaration is left in place instead of guessed at, and an
-      // unstripped external DTD reference is inert to the two scans below, so
-      // without this gate it would be published as cleaned.
-      if (hasDoctype(cleaned)) {
-        return warning(
-          record,
-          `landscape logo contains a DOCTYPE that could not be removed: ${filename}`,
-        );
-      }
-      const residual = findActiveContent(cleaned);
-      if (residual.length > 0) {
-        return warning(
-          record,
-          `landscape logo still contains active content after cleanup: ${filename} (${residual.join('; ')})`,
-        );
-      }
       if (findRemoteReferences(cleaned).length > 0) {
         return warning(
           record,