Skip to content

Comments

Fix double-substitution bug in runtime import expression replacement#17332

Merged
pelikhan merged 2 commits intofrfrom
copilot/sub-pr-17330
Feb 21, 2026
Merged

Fix double-substitution bug in runtime import expression replacement#17332
pelikhan merged 2 commits intofrfrom
copilot/sub-pr-17330

Conversation

Copy link
Contributor

Copilot AI commented Feb 21, 2026

The iterative split/join loop in processExpressions could incorrectly re-substitute expression syntax found inside an already-evaluated value. For example, an issue title containing the literal text ${{ github.actor }} would have that text replaced with the actor's username — corrupting the output.

Changes

  • runtime_import.cjs: Replace the multi-pass split/join loop with a single regex that matches all original expressions simultaneously, so evaluated values are never scanned again:

    const escapeForRegex = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
    const pattern = new RegExp(Array.from(replacements.keys()).map(escapeForRegex).join("|"), "g");
    const result = content.replace(pattern, match => replacements.get(match) ?? match);
  • runtime_import.test.cjs: The existing test for this scenario had the correct assertion but was missing the mock-context override to set the issue title to a value containing expression syntax ("Use ${{ github.actor }} here"), so it wasn't actually exercising the bug.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…egex

Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on expression replacement fix PR Fix double-substitution bug in runtime import expression replacement Feb 21, 2026
@pelikhan pelikhan marked this pull request as ready for review February 21, 2026 01:21
Copilot AI review requested due to automatic review settings February 21, 2026 01:21
@pelikhan pelikhan merged commit 9779875 into fr Feb 21, 2026
@pelikhan pelikhan deleted the copilot/sub-pr-17330 branch February 21, 2026 01:21
Copy link
Contributor

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 fixes a double-substitution bug in the expression replacement logic where literal expression syntax within evaluated values could be incorrectly replaced again. For example, an issue title containing ${{ github.actor }} would have that text replaced with the actual actor username, corrupting the output.

Changes:

  • Replaced the iterative split/join replacement loop with a single-pass regex-based replacement to prevent re-scanning of evaluated values
  • Added missing test setup to properly exercise the double-substitution scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
actions/setup/js/runtime_import.cjs Replaced multi-pass split/join loop with single regex pattern matching all expressions simultaneously to prevent double-substitution
actions/setup/js/runtime_import.test.cjs Added missing mock context setup to set issue title containing expression syntax, enabling the test to actually validate the fix

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

dsyme added a commit that referenced this pull request Feb 21, 2026
* fix replacements

* Update actions/setup/js/runtime_import.test.cjs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix double-substitution bug in runtime import expression replacement (#17332)

* Initial plan

* Fix expression replacement double-substitution bug with single-pass regex

Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>

* Add fuzz tests for processExpressions expression insertion combinations (#17365)

* Initial plan

* Initial plan: add fuzz tests for processExpressions expression insertion combinations

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Add fuzz tests for processExpressions expression insertion combinations

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* fix mocks

* fix mocks

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
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.

3 participants