fix: prevent JS replace() special patterns from corrupting rendered prompts#30461
Conversation
…ing prompts
String.prototype.replace() treats certain character sequences in the
replacement string as special patterns ($&, $`, $', $$, $n). When
untrusted content from a repository (e.g. file contents pulled in via
{{#runtime-import}}) is used as a replacement string directly, those
special patterns corrupt the rendered prompt.
Fix by switching all affected .replace(pattern, value) calls to use
a replacer function .replace(pattern, () => value) which bypasses
special pattern interpretation.
Affected locations:
- interpolate_prompt.cjs: interpolateVariables() and experiment
placeholder substitution in conditions
- runtime_import.cjs: cached import and fresh import replacements
Adds regression tests for $$, $&, $`, $', and $1 patterns in values."
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bef4c406-66fd-4e91-8901-866a342c32f4
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR prevents JavaScript String.prototype.replace() replacement-string special patterns ($$, $&, $` , $', $n) from corrupting rendered prompts when interpolated values or runtime-imported file content contain those sequences.
Changes:
- Switch variable interpolation and runtime-import substitutions to use function replacers (
() => value/content) to bypass replacement-string pattern expansion. - Update experiment-condition substitution to use a replacer callback instead of
$1...replacement strings. - Add regression tests ensuring interpolated variable values containing the special
$patterns are preserved.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/interpolate_prompt.cjs | Uses replacer functions for variable interpolation and experiment-condition substitution to avoid $ replacement pattern expansion. |
| actions/setup/js/runtime_import.cjs | Uses replacer functions when inserting imported (and cached) content to prevent $ pattern expansion corrupting output. |
| actions/setup/js/interpolate_prompt.test.cjs | Adds regression tests covering $ special patterns in interpolated variable values. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
| // Reuse cached content | ||
| const cachedContent = importCache.get(filepathWithRange); | ||
| if (cachedContent !== undefined) { | ||
| processedContent = processedContent.replace(fullMatch, cachedContent); | ||
| processedContent = processedContent.replace(fullMatch, () => cachedContent); | ||
| core.info(`Reusing cached content for ${filepathWithRange}`); |
| const conditionPattern = new RegExp(`(\\{\\{#if[^}]*?)${exprForm.replace(".", "\\.")}`, "gi"); | ||
| if (conditionPattern.test(content)) { | ||
| conditionPattern.lastIndex = 0; | ||
| content = content.replace(conditionPattern, `$1${value || ""}`); | ||
| content = content.replace(conditionPattern, (_, prefix) => prefix + (value || "")); | ||
| core.info(` Substituted ${exprForm} in conditions → "${value || ""}"`); |
|
@copilot Review all comments |
…and experiment conditions - runtime_import.test.cjs: add 7 tests covering $$, $&, $`, $', $1 patterns in freshly-imported and cached content (both code paths in processRuntimeImports) - interpolate_prompt.test.cjs: add 2 tests for STEP 2.5 experiment placeholder condition substitution with $1 and $& values; tests set GH_AW_EXPERIMENTS_* env vars and apply the substitution logic directly through renderMarkdownTemplate" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f77dcfcd-e644-4140-a126-f79bf8ef0d13 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…uplication Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f77dcfcd-e644-4140-a126-f79bf8ef0d13 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added the requested tests in commit 530561b:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
String.prototype.replace()interprets certain sequences in the replacement string as special patterns ($$,$&,$`,$',$n). When untrusted file content (e.g. from{{#runtime-import}}) or GitHub event data is used directly as the replacement argument, these patterns silently corrupt the rendered prompt.Changes
interpolate_prompt.cjs— two call sites:interpolateVariables():result.replace(pattern, value)→result.replace(pattern, () => value)$1capture reference → replacer function(_, prefix) => prefix + valueruntime_import.cjs— two call sites inprocessRuntimeImports():() => contentreplacerinterpolate_prompt.test.cjs— regression tests for all five special patterns ($$,$&,$`,$',$1) in interpolated variable values, plus STEP 2.5 integration tests that setGH_AW_EXPERIMENTS_*env vars to values containing$1and$&and verify condition evaluation is not corrupted (using an extractedapplyExperimentSubstitutionhelper)runtime_import.test.cjs— regression tests covering all five special patterns in imported file content, verifying both the fresh-import and cache-hit code paths inprocessRuntimeImports()