fix(prompts): use function replacer in applySubstitutions to prevent $-pattern corruption#28013
fix(prompts): use function replacer in applySubstitutions to prevent $-pattern corruption#28013bisma-nawaz wants to merge 1 commit into
Conversation
…$ pattern corruption
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the prompt substitution utility where dynamic content containing special characters was being incorrectly processed by JavaScript's regex replacement engine. By switching to functional replacers, the system now ensures that injected content is treated as a literal string, preserving the integrity of system prompts regardless of their internal character patterns. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
📊 PR Size: size/S
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
There was a problem hiding this comment.
Code Review
This pull request updates the applySubstitutions function in packages/core/src/prompts/utils.ts to use replacer functions instead of direct string replacements when calling String.prototype.replace. This prevents special replacement patterns (such as $&, $', and $$) in the substitution values from being interpreted incorrectly. Corresponding unit tests have been added to packages/core/src/prompts/utils.test.ts to verify literal insertion of these special characters. There are no review comments, so I have no feedback to provide.
Summary
applySubstitutionsinpackages/core/src/prompts/utils.tspasses a plain string as the second argument toString.prototype.replace. JavaScript interprets$-prefixed patterns in string replacements ($$,$&,$`,$',$n), so any skill, sub-agent, or tool description that legitimately contains shell notation like$'…',$$, or$VARsilently corrupts the assembled system prompt — duplicating content, collapsing$$to$, or re-inserting the placeholder text.Details
The fix replaces all four string-form replacements with arrow-function replacers, which bypass
$-pattern substitution entirely. This is the same pattern already used inHookRunner.expandCommand(packages/core/src/hooks/hookRunner.ts):The same change is applied to the
${SubAgents},${AvailableTools}, and per-tool${toolName_ToolName}replacements.Related Issues
Fixes #27993
How to Validate
Run the targeted test file:
The three new tests (
$',$$,$&cases) would have failed before this change. All 33 tests pass after it.Reproduce the original corruption manually (Node ≥18):
Pre-Merge Checklist
utils.test.ts