[sec-check] stripActiveContent() never re-scans the SVG it returns, though its contract and two mirroring callers depend on it #1515

Security Finding

Severity: low (defence-in-depth)
Type: unsafe-pattern — documented sanitizer post-condition is not implemented
Component: scripts/lib/svg-active-content.mjs — stripActiveContent()

stripActiveContent() is the sanitizer that makes a third-party SVG safe to
publish from static/. Its own contract says it verifies its output:

The result is re-scanned before it is returned. Callers write it to the
site origin verbatim, so a source the detector still flags must never be
handed back as sanitized.

and its @throws tag promises an error "If the document cannot be edited
safely or active content survives".

The code does neither of the two things that sentence describes. The only
verification in the function is structuralFindings(), which runs on the
input before the rebuild. After rebuild(..., { strip: true }) the result
is returned unexamined:

  const result = rebuild(text, parsed.events, { strip: true });
  return result;

There is no findActiveContent(result.source) anywhere in the function, and
no caller supplies one for the two paths that matter.

Impact

The post-condition is not decorative — it is load-bearing for three callers,
two of which have no active-content check of their own:

• scripts/lib/architecture-content.mjs:268 (mirrorArtworkUrls) writes the
  returned string into static/ and gates the write with
  findRemoteReferences() only. This path is reached from
  scripts/import-architecture-issue.mjs, i.e. from a community-submitted
  reference-architecture issue.
• scripts/lib/enduser-collector.mjs:164 (mirrorLandscapeLogo) writes the
  returned string into static/ and likewise gates only on
  findRemoteReferences().
• scripts/import-architectures.mjs:253-262 does re-scan, but its comment
  says it is doing so because of its own later text edit, and states the
  premise explicitly: "stripActiveContent() verifies the string it returns".
  That premise is currently false.

So for the two mirroring paths, stripActiveContent() is the sole
active-content gate standing between third-party SVG bytes and a file
published at the site origin, where a directly-opened SVG executes script in
that origin.

This is not exploitable today: findings and removals are both derived from
classifyTag() over the same parse, and the output is serialized from that
tree, so detection and removal agree by construction at 1ae3403. The finding
is that the agreement is an argument about two code paths, not a property of
the bytes returned, and the file's own history (eight fix(security) commits,
#1019 #1025 #1044 #1068 #1090 #1168 #1176 #1245) is a record of that argument
being revised. Any future divergence between classifyTag()'s removal set,
rebuild()'s serialization and findActiveContent()'s detection publishes a
document that only looks sanitized, silently.

Recommendation

Implement the documented post-condition — re-scan the returned bytes and fail
closed:

  const result = rebuild(text, parsed.events, { strip: true });
  const residual = findActiveContent(result.source);
  if (residual.length) {
    throw new Error(
      `Active content survived stripping from SVG: ${residual.join('; ')}`,
    );
  }
  return result;

and cover the post-condition in tests/svg-active-content.test.mjs over every
construct the module strips, so detection/removal/serialization cannot drift
apart unnoticed. The existing test named "throws rather than returning a source
the detector still flags" only exercises the structural refusal, which
happens before the rebuild; the post-rebuild half of the same promise is
untested.

• stripActiveContent() re-scans its own output and throws when anything
  active survives
• tests/svg-active-content.test.mjs asserts the post-condition for every
  strippable construct

Filed by sec-check agent (ACMM L4/L5 — hold-gated mode)

🐝 Hive Agent: security | Instance: hosted-available-lke648397-260827-5n31 | SHA: 1ae3403

— hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88