fix: re-scan the SVG stripActiveContent returns

scripts/lib/svg-active-content.mjs

@@ -769,6 +769,11 @@ function rebuild(source, events, { dropDoctype = false, strip = false } = {}) {
  * handed back as sanitized.
  *
  * @param {string} source - SVG file contents.
+ * @param {Object} [options]
+ * @param {(source: string) => string[]} [options.scan] - Detector run over
+ *   the rebuilt output. Defaults to {@link findActiveContent}; the override
+ *   exists so the survival throw below can be exercised, because the default
+ *   detector and the stripper agree by construction today.
  * @returns {{ source: string, removed: string[] }}
  * @throws {Error} If the document cannot be edited safely or active content
  *   survives. A document that is not well-formed, declares entities, or is
@@ -777,7 +782,7 @@ function rebuild(source, events, { dropDoctype = false, strip = false } = {}) {
  *   whose payload it cannot see would be the opposite of what the
  *   verification is for.
  */
-export function stripActiveContent(source) {
+export function stripActiveContent(source, { scan = findActiveContent } = {}) {
   const text = String(source);
   const findings = findActiveContent(text);
   if (!findings.length) return { source: text, removed: [] };
@@ -791,8 +796,21 @@ export function stripActiveContent(source) {
   }
 
   // Findings and removals both come from classifyTag() over this same parse,
-  // and the output is serialized from the tree, so what is left is exactly the
-  // nodes that produced no finding.
+  // and the output is serialized from the tree, so what is left should be
+  // exactly the nodes that produced no finding. That is an argument about two
+  // code paths agreeing, not a property of the bytes being returned, and the
+  // bytes are what callers publish at the site origin: architecture-content.mjs
+  // and enduser-collector.mjs write this string into static/ with no active
+  // content check of their own. Re-scan the actual output so any future
+  // divergence between classifyTag()'s removal set, rebuild()'s serialization
+  // and findActiveContent()'s detection fails closed here instead of
+  // publishing a document that only looks sanitized.
   const result = rebuild(text, parsed.events, { strip: true });
+  const residual = scan(result.source);
+  if (residual.length) {
+    throw new Error(
+      `Active content survived stripping from SVG: ${residual.join('; ')}`,
+    );
+  }
   return result;
 }

tests/svg-active-content.test.mjs

@@ -781,6 +781,97 @@ test('throws rather than returning a source the detector still flags', () => {
   });
 });
 
+// The test above covers the structural refusal, which happens *before* the
+// document is rebuilt. This one covers the other half of the same promise: a
+// document that is well-formed enough to edit still has to come back inert.
+// architecture-content.mjs (mirrorArtworkUrls) and enduser-collector.mjs
+// (mirrorLandscapeLogo) write this return value straight into static/ and run
+// only findRemoteReferences() over it, so stripActiveContent() is the sole
+// active-content gate on the bytes those two paths publish at the site origin.
+// Asserting the post-condition over every construct the module strips keeps a
+// future change to classifyTag()'s removal set, rebuild()'s serialization or
+// findActiveContent()'s detection from silently drifting apart.
+const STRIPPABLE_CONSTRUCTS = [
+  ['<script> element', '<script>alert(1)</script>'],
+  ['<handler> element', '<handler type="text/javascript">alert(1)</handler>'],
+  ['<listener> element', '<listener handler="#h"/>'],
+  [
+    '<iframe> in foreignObject',
+    '<foreignObject><iframe src="https://evil.example/"/></foreignObject>',
+  ],
+  ['<embed> element', '<embed src="https://evil.example/x"/>'],
+  ['<object> element', '<object data="https://evil.example/x"/>'],
+  ['<base> element', '<base href="https://evil.example/"/>'],
+  [
+    '<meta> refresh',
+    '<meta http-equiv="refresh" content="0;url=https://evil.example/"/>',
+  ],
+  ['event handler attribute', '<rect width="1" height="1" onload="alert(1)"/>'],
+  [
+    'srcdoc document attribute',
+    '<foreignObject><iframe srcdoc="&lt;script&gt;alert(1)&lt;/script&gt;"/></foreignObject>',
+  ],
+  ['javascript: href', '<a href="javascript:alert(1)">x</a>'],
+  [
+    'namespace-prefixed script',
+    '<x:script xmlns:x="http://www.w3.org/2000/svg">alert(1)</x:script>',
+  ],
+  [
+    '<animate> installing an href',
+    '<a><animate attributeName="href" to="javascript:alert(1)"/></a>',
+  ],
+  [
+    '<set> installing an href',
+    '<a><set attributeName="href" to="javascript:alert(1)"/></a>',
+  ],
+];
+
+test('stripActiveContent throws instead of returning output its detector still flags', () => {
+  // The default detector and the stripper agree by construction, so the
+  // survival throw is only reachable through the scan seam. Drive it with a
+  // detector that reports a finding over the rebuilt bytes and assert the
+  // sanitized string never reaches the caller.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>';
+  const seen = [];
+  assert.throws(
+    () =>
+      stripActiveContent(svg, {
+        scan: (output) => {
+          seen.push(output);
+          return ['<script> element survived'];
+        },
+      }),
+    /Active content survived stripping from SVG: <script> element survived/,
+  );
+  assert.equal(seen.length, 1);
+  assert.doesNotMatch(seen[0], /<script/);
+});
+
+for (const [label, markup] of STRIPPABLE_CONSTRUCTS) {
+  test(`stripActiveContent returns an inert document for ${label}`, () => {
+    const svg = `<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">${markup}</svg>`;
+    assert.ok(
+      findActiveContent(svg).length > 0,
+      'fixture is meant to carry active content',
+    );
+    const { source, removed } = stripActiveContent(svg);
+    assert.ok(removed.length > 0, 'something should have been removed');
+    // The returned bytes, not the parse they came from: this is what the
+    // mirroring callers write to static/.
+    assert.deepEqual(findActiveContent(source), []);
+  });
+}
+
+test('stripActiveContent returns an inert document for every construct at once', () => {
+  const svg = `<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><?xml-stylesheet href="https://evil.example/x.xsl" type="text/xsl"?>${STRIPPABLE_CONSTRUCTS.map(
+    ([, markup]) => markup,
+  ).join('')}</svg>`;
+  assert.ok(findActiveContent(svg).length > 0);
+  const { source } = stripActiveContent(svg);
+  assert.deepEqual(findActiveContent(source), []);
+});
+
 // An XML parser expands author-defined general entities before the document
 // tree exists, so a payload parked in a declaration never appears in any
 // attribute value this module can normalize. The scanner cannot resolve them,