fix(producer): strip script bodies to a fixed point, not one pass

CodeQL (incomplete multi-character sanitization, code-scanning/803) on
the script-stripping regex added in c61a24b51 — a failing check, and
correct: a single replace can reform the pattern it just removed, since
`<scr<script>ipt>` leaves a whole `<script>` behind.

The security framing does not apply — the stripped string is counted and
discarded, never rendered, inserted, or served — but the incompleteness
is real for this use: a reformed tag survives into the match pass and
perturbs the element count the routing gate reads. Suppressing a gate
over a technicality when the fix is four lines is the wrong trade.

Now loops to a fixed point. Terminates by construction: each iteration
either strictly shortens the string or changes nothing and exits.
Regression covers the reform case and an unterminated `<script>` that
must not spin; fault injection confirms the reform test fails under the
old single pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-07-30 00:13:42 -07:00
co-authored by Claude Opus 5
parent c61a24b510
commit 042d5aaba6
2 changed files with 26 additions and 1 deletions
@@ -2020,6 +2020,20 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
expect(countElementTags('<div></div><style>a::after{content:"</div>"}</style>')).toBe(1); expect(countElementTags('<div></div><style>a::after{content:"</div>"}</style>')).toBe(1);
}); });
// CodeQL "incomplete multi-character sanitization": a single-pass replace
// can reform the very pattern it removed. Impact is nil here (the stripped
// string is counted, never rendered) but a reformed tag would perturb the
// count, so the strip runs to a fixed point.
it("strips script tags that reform after one pass", () => {
// Inner <script> removed by pass 1 leaves "<script>alert(1)</script>",
// which pass 2 removes. A single pass would leave a stray tag behind.
expect(countElementTags("<div></div><scr<script></script>ipt>alert(1)</script>")).toBe(1);
});
it("terminates on input with no closing tag rather than looping", () => {
expect(countElementTags("<div></div><script>unterminated")).toBe(1);
});
it("strips multiple and attributed script blocks, not just the first", () => { it("strips multiple and attributed script blocks, not just the first", () => {
expect( expect(
countElementTags( countElementTags(
@@ -1297,7 +1297,18 @@ export function countElementTags(html: string): number {
// no probe session, for which this scan is the only element signal (review // no probe session, for which this scan is the only element signal (review
// finding). Removing the bodies also drops their own closing tags, which // finding). Removing the bodies also drops their own closing tags, which
// costs 1-2 counts against a threshold in the thousands. // costs 1-2 counts against a threshold in the thousands.
const markup = html.replace(/<(script|style)\b[^>]*>[\s\S]*?<\/\1>/gi, ""); // Looped to a fixed point rather than a single pass: one pass can REFORM
// the pattern it just removed (`<scr<script>ipt>` leaves `<script>`), which
// CodeQL flags as incomplete multi-character sanitization. The impact here
// is nil — the stripped string is counted and discarded, never rendered —
// but the incompleteness is real, and a stray reformed tag would perturb
// the count this gate reads. Converges: every iteration strictly shortens
// the string or changes nothing and exits.
let markup = html;
for (let previous = ""; markup !== previous; ) {
previous = markup;
markup = markup.replace(/<(script|style)\b[^>]*>[\s\S]*?<\/\1>/gi, "");
}
const matches = markup.match( const matches = markup.match(
/<\/[a-zA-Z]|<(?:img|br|hr|input|source|track|area|base|col|embed|link|meta|param|wbr)\b|<[a-zA-Z][-a-zA-Z0-9]*\b[^>]*\/>/gi, /<\/[a-zA-Z]|<(?:img|br|hr|input|source|track|area|base|col|embed|link|meta|param|wbr)\b|<[a-zA-Z][-a-zA-Z0-9]*\b[^>]*\/>/gi,
); );