Skip to content

Conversation

@blakebrunson
Copy link
Collaborator

Potential fix for https://github.com/github/rollup-and-away/security/code-scanning/1

General approach: Ensure that after stripHtml runs, no residual <script, <style, generic tags, or HTML comments can remain due to overlapping or nested matches. Following the recommended pattern, we can repeatedly apply the same replacement until the string stabilizes (no further changes), which guarantees that any new dangerous substrings created by prior replacements are also removed. This preserves existing behavior for typical inputs but closes the multi-character sanitization gap.

Concrete fix for src/util/string.ts:

  • Modify stripHtml so it does not call s.replace just once. Instead:
    • Store the original regex in a local constant for clarity.
    • Use a loop (do { … } while (…) or while (true) with a break) that repeatedly applies .replace(regex, "") to the current string.
    • Stop when a pass makes no changes (the string is identical to the previous iteration).
  • Keep the regex pattern exactly as-is, to avoid changing which constructs are stripped; only change how it is applied.
  • No new imports or helpers are needed; we can implement the loop directly in stripHtml.

This single change in stripHtml addresses all 4 alert variants, as the loop guarantees that any residual <script, <style, <-tags, or <!-- resulting from overlapping matches will be removed in subsequent iterations.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…er sanitization

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@blakebrunson blakebrunson marked this pull request as ready for review January 6, 2026 15:28
@blakebrunson blakebrunson requested a review from chrizbo as a code owner January 6, 2026 15:28
Copilot AI review requested due to automatic review settings January 6, 2026 15:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a code scanning alert for incomplete multi-character sanitization in the stripHtml function. The fix ensures that overlapping or nested HTML constructs that might be revealed after the first pass of sanitization are fully removed by repeatedly applying the regex replacement until the string stabilizes.

Key changes:

  • Modified stripHtml to use a do-while loop that repeatedly applies the HTML stripping regex until no further changes occur
  • Extracted the regex pattern into a constant for clarity
  • Added explanatory comments about the multi-character sanitization approach

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants