fix(core): bound inert region scans in timing compiler

[jrusso1020]

What

Fix CodeQL #748: repeated unclosed comment, script or style prefixes can make timing compilation spend quadratic time looking for a closing delimiter. Also recognize the browser’s --!> comment terminator, so hidden media stays untouched and visible media after that delimiter is compiled and extracted correctly.

Why

The old inert-region regex exceeded two seconds for each input containing 100,000 unclosed prefixes. Both timing compilation and resolved-media extraction accept these inputs. Its comment matcher also skipped the browser’s end-bang terminator: it could compile hidden media or mask visible media through a later -->.

How

Scan opening prefixes in source order and search forward for the first matching closer. If no closer exists, stop searching that region kind while continuing to recognize other kinds. Successful regions do not overlap; each failed suffix is searched at most once per kind. Comment closers use the bounded pattern --!?>; script/style closers retain their case and whitespace rules.

Preserve existing first-closer ordering, script/style boundaries, no-closer behavior and verbatim restoration. Recognizing --!> is the intentional behavior correction. Two files changed; no public API or workflow changes.

Test plan

• All 258 compiler tests pass, including 20 new cases for complete/malformed regions, mixed/nested delimiters, case/whitespace, unclosed prefixes and both public consumers.
• Browser-parser witnesses confirm hidden-before/visible-after behavior at standard and end-bang comment boundaries, with and without a later standard delimiter.
• 150,000 inputs without end-bang delimiters preserve original-main masked/restored output; another 150,000 including end-bang delimiters match a reference regex with only that boundary correction.
• Each 100,000-prefix input scans in about six milliseconds locally.
• Parser/core/lint builds, core/runtime typechecks, lint and formatting pass; commit hooks verify core, Studio and scripts types.
• Fresh CI, CodeQL and independent Magi re-review remain landing gates. Default-branch alert closure will be verified after merge.

[miguel-heygen]

Reviewed at exact head 22e2935684456470685d549f9d62bf1aee78c351. No blockers.

The scanner at packages/core/src/compiler/timingCompiler.ts:118-151 is linear by construction. Successful regions advance both the opening cursor and output cursor past the first matching closer, so their searched spans do not overlap. A failed closer search deletes that kind from the three-entry map; later prefixes of that kind are cheap skips while comments/scripts/styles of other kinds remain eligible. Thus each successful span and at most one failed suffix per kind are scanned, bounded by a constant multiple of input length.

Behavioral parity also holds. The opening expression preserves case-insensitive comment/script/style selection and the script/style word boundary; each closer preserves case and \s* rules. Because the earliest opener can consume any later same-kind closer, deleting a kind only after no closer remains cannot hide a match the old lazy regex would have made. parts/stash retain regions verbatim and in the same order, so compileTimingAttrs restores exact source while extractResolvedMedia shares the identical masking decision (timingCompiler.ts:226, :315).

Independent exact-head verification:

• 255/255 compiler tests passed after generating the runtime artifact required by the compiler suite
• changed-file formatting passed
• old/new masked output and stash were identical across 200,020 generated/adversarial mixed, nested, complete, and malformed inputs
• 100k unclosed prefixes completed in 6.81 ms (comment), 6.87 ms (script), and 6.37 ms (style) in an independent probe

CI had no failures; JavaScript CodeQL, Windows, and remaining build/regression jobs were still running and remain landing gates.

Verdict: APPROVE
Reasoning: The per-kind scanner gives a provable linear bound while preserving the old first-closer and verbatim-restoration semantics for both compiler consumers.

— Magi

[miguel-heygen]

Re-review at exact head 883b35a21e294b4dcdf486381ac4299723693ff5.

The revised indexOf("-->") path is linear and preserves main’s bytes, but #883 is not a false positive for this helper’s stated contract.

Blocker — recognize the browser’s comment end-bang state. packages/core/src/compiler/timingCompiler.ts:111-115 defines this helper as the browser-inert-region boundary that prevents media inside comments from being compiled. Browsers recognize --!> as a comment terminator; CodeQL’s js/bad-tag-filter recommendation calls out this exact HTML-parser case. Whether the source is restored rather than sanitized does not change which bytes are inside versus outside the comment.

I reproduced both wrong directions at this head against jsdom’s HTML parser:

• <!-- <video id="hidden" data-duration="1"> --!><video id="visible" data-duration="1">: DOM contains only visible, but compileTimingAttrs rewrites both tags and extractResolvedMedia returns [hidden, visible].
• <!-- --!><video id="visible" data-duration="1"> -->: DOM contains visible, but line 123’s --> search masks through the later delimiter; the visible tag is left uncompiled and extraction returns [].

The new cases at timingCompiler.test.ts:26 and :40 therefore pin old output, not compiler/browser parity. Please find the first --> or --!> with a single forward scan (so the linear bound remains), then assert the hidden-before/visible-after split through both compileTimingAttrs and extractResolvedMedia. A small indexOf("--", cursor) loop that checks the following characters avoids both the original quadratic whole-region regex and a second full suffix scan.

No-closer behavior remains byte-compatible and linear; treating unterminated comments/scripts/styles as open to EOF is a separate pre-existing malformed-input parity question, not this blocker.

Independent verification: 257/257 compiler tests and formatting pass, but the two tests above demonstrate why green parity-to-main is insufficient here. Current-head CI has no completed failure; JavaScript CodeQL and other jobs are still running (older cancelled runs appear as failures in gh pr checks).

Verdict: REQUEST CHANGES
Reasoning: The performance fix is bounded, but it intentionally preserves a browser-comment boundary error that makes the compiler mutate hidden media and skip visible media; #883 identifies a real correctness condition for this parser.

— Magi

[miguel-heygen]

Re-review at exact head bb74effceb5aeba523e4acafad824c241e9be1b6. The prior end-bang blocker is closed; no new blockers.

packages/core/src/compiler/timingCompiler.ts:121-145 now uses /--!?>/g, a bounded fixed-delimiter matcher that recognizes the first standard or end-bang comment close without reintroducing the whole-region backtracking from #748. It shares the existing per-kind cursor/disable machinery, so successful spans remain disjoint and a missing closer still costs one suffix scan before that kind becomes a cheap skip.

The replacement tests at timingCompiler.test.ts:48-73 use jsdom as the browser parser oracle, then assert the same hidden-before/visible-after split through both compileTimingAttrs and extractResolvedMedia, including a later standard delimiter and an end-bang with no later -->. That is the right correction to the two behaviors I reproduced. The scoped unterminated-region behavior remains independently pinned at :75-85.

I read the js/bad-tag-filter recommendation. A full parser is the default for general HTML filtering; here the implementation only locates three inert-region delimiter families, restores their bytes verbatim, and now carries a real HTML-parser oracle for the browser-sensitive comment boundary. The bounded closer is appropriate for that narrower contract.

Independent exact-head verification:

• 258/258 compiler tests passed
• changed-file formatting passed
• revised output/stash matched a reference implementation with the same end-bang correction across 200,008 generated/adversarial inputs
• 100k unclosed prefixes remained linear: 5.74 ms comment, 7.11 ms script, 6.75 ms style

Fresh CI has no completed failure; JavaScript CodeQL, Windows, and remaining jobs are still running and remain landing gates.

Verdict: APPROVE
Reasoning: The revised scanner preserves the proven linear bound and now matches the browser’s comment boundary through both public compiler consumers.

— Magi