fix(core): insert content literally into LLM prompts to avoid $ substitution - #27552
fix(core): insert content literally into LLM prompts to avoid $ substitution#27552Pluviobyte wants to merge 2 commits into
Conversation
…itution
Several LLM prompt builders interpolate user/file content into a template via
`String.prototype.replace('{placeholder}', value)`. The replacement argument
honors special patterns (`$$`, `$&`, `` $` ``, `$'`), so any value containing a
`$` is silently corrupted before being sent to the model:
- `$&` -> the matched placeholder name itself
- `$$` -> a single `$`
- `` $` ``/`$'` -> the text before/after the match (can dump the template prefix
into the content or truncate it)
This affects the edit self-correction prompt (llm-edit-fixer.ts), the tool-output
summarizer (summarizer.ts), and the session summary (sessionSummaryService.ts) —
all of which routinely receive file contents, shell output, or conversation text
that can contain `$`.
Switch these call sites to the existing `safeLiteralReplace` helper
(utils/textUtils.ts), which escapes `$` so the value is inserted verbatim. Add a
regression test covering `$&`, `$$`, `` $` `` and `$'`; it fails before this fix
and passes after.
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 critical bug in how user-provided content is interpolated into LLM prompts. By switching from standard string replacement to a safe literal replacement utility, the changes ensure that special characters are treated as plain text rather than substitution patterns, preventing prompt corruption and potential template leakage. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces changes to safely handle literal replacements of $-sequences in prompt templates by utilizing a custom safeLiteralReplace helper across several services and utilities, along with adding corresponding unit tests. The reviewer feedback highlights a sequential interpolation vulnerability in llm-edit-fixer.ts where sequential replacements could lead to double-interpolation bugs if user inputs contain placeholder tokens, and suggests using a single-pass regex replacement with a replacer function. Additionally, the reviewer suggests using native String.prototype.replace with a replacer function in sessionSummaryService.ts to avoid the custom helper entirely, as JavaScript natively treats the returned string of a replacer function literally.
| // Use safeLiteralReplace so that `$`-sequences (e.g. `$&`, `$$`, `` $` ``) in | ||
| // file content or edit parameters are inserted literally instead of being | ||
| // interpreted as String.prototype.replace substitution patterns. | ||
| let userPrompt = safeLiteralReplace( | ||
| EDIT_USER_PROMPT, | ||
| '{instruction}', | ||
| instruction, | ||
| ); | ||
| userPrompt = safeLiteralReplace(userPrompt, '{old_string}', old_string); | ||
| userPrompt = safeLiteralReplace(userPrompt, '{new_string}', new_string); | ||
| userPrompt = safeLiteralReplace(userPrompt, '{error}', error); | ||
| userPrompt = safeLiteralReplace( | ||
| userPrompt, | ||
| '{current_content}', | ||
| current_content, | ||
| ); |
There was a problem hiding this comment.
The current implementation performs sequential replacements using safeLiteralReplace. This introduces a sequential interpolation vulnerability (double-interpolation bug). If any of the user-controlled inputs (such as instruction, old_string, new_string, or error) contain a placeholder string like {old_string} or {current_content}, that placeholder will be replaced in subsequent steps. This can lead to prompt corruption or prompt injection.
To fix this and also completely avoid the $ substitution issue without needing custom helpers, we can perform a single-pass replacement using a native regular expression with a replacer function. Since the replacement argument is a function, special patterns like $ are treated as literal characters natively.
| // Use safeLiteralReplace so that `$`-sequences (e.g. `$&`, `$$`, `` $` ``) in | |
| // file content or edit parameters are inserted literally instead of being | |
| // interpreted as String.prototype.replace substitution patterns. | |
| let userPrompt = safeLiteralReplace( | |
| EDIT_USER_PROMPT, | |
| '{instruction}', | |
| instruction, | |
| ); | |
| userPrompt = safeLiteralReplace(userPrompt, '{old_string}', old_string); | |
| userPrompt = safeLiteralReplace(userPrompt, '{new_string}', new_string); | |
| userPrompt = safeLiteralReplace(userPrompt, '{error}', error); | |
| userPrompt = safeLiteralReplace( | |
| userPrompt, | |
| '{current_content}', | |
| current_content, | |
| ); | |
| const replacements: Record<string, string> = { | |
| instruction, | |
| old_string, | |
| new_string, | |
| error, | |
| current_content, | |
| }; | |
| const userPrompt = EDIT_USER_PROMPT.replace( | |
| /\{(\w+)\}/g, | |
| (match, key) => replacements[key] ?? match, | |
| ); |
There was a problem hiding this comment.
Good catch — addressed in 7f06728. Replaced the sequential safeLiteralReplace calls with a single-pass EDIT_USER_PROMPT.replace(/\{(\w+)\}/g, (match, key) => replacements[key] ?? match). The replacer function inserts $-sequences literally, and the single pass ensures a placeholder token appearing inside an input value (e.g. old_string === '{current_content}') is no longer re-interpolated by a later step. Added a regression test (does not re-interpolate placeholder tokens that appear inside inputs).
| const prompt = safeLiteralReplace( | ||
| SUMMARY_PROMPT, | ||
| '{conversation}', | ||
| conversationText, | ||
| ); |
There was a problem hiding this comment.
Instead of importing and using the custom safeLiteralReplace helper, we can use the native String.prototype.replace with a replacer function. When the replacement argument is a function, JavaScript natively treats the returned string as a literal, completely ignoring any special $ substitution patterns. This is more idiomatic, standard, and avoids unnecessary helper imports.
const prompt = SUMMARY_PROMPT.replace('{conversation}', () => conversationText);There was a problem hiding this comment.
Done in 7f06728 — switched to the native replacer-function form SUMMARY_PROMPT.replace('{conversation}', () => conversationText) and dropped the safeLiteralReplace import (applied the same single-pass replacer approach to summarizer.ts as well for consistency).
Address review feedback on the prompt builders. Replace the sequential
safeLiteralReplace calls with a single-pass `String.prototype.replace` using a
replacer function:
- the function form already inserts `$`-sequences (`$&`, `$$`, `` $` ``, `$'`)
literally, so no custom helper is needed; and
- a single pass prevents a placeholder token that appears inside one input value
(e.g. an edit parameter that itself contains the text `{current_content}`) from
being re-interpolated by a later replacement (a sequential-interpolation bug).
Applies the same approach to llm-edit-fixer, summarizer and sessionSummaryService,
and adds a regression test covering a placeholder token supplied as input.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes template interpolation issues where special $-sequences (such as $& or $$) in user inputs or conversation texts were incorrectly treated as substitution patterns by String.prototype.replace. The fix introduces replacer functions and single-pass regex replacements with a lookup dictionary in sessionSummaryService.ts, llm-edit-fixer.ts, and summarizer.ts. This single-pass approach also prevents nested placeholder tokens from being re-interpolated. Corresponding unit tests have been added to verify the correct behavior. I have no feedback to provide as the changes are well-implemented and include appropriate test coverage.
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
Thanks for the intake-policy nudge. I added the requested source/version context on the linked issue (#27556) on 2026-05-30. The later Gemini Code Assist feedback was addressed in 7f06728, the old review threads are outdated, and the follow-up Gemini review reported no further feedback. If maintainers are open to accepting this fix, could #27556 be considered for |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
Summary
Several LLM prompt builders interpolate user/file content into a template with
String.prototype.replace('{placeholder}', value). The replacement argument honors special patterns, so any value containing$is silently corrupted before being sent to the model. This interpolates each prompt in a single pass with a replacer function, which both inserts content literally and avoids re-interpolating placeholder tokens that appear inside input values.Details
String.prototype.replace(searchString, replacement)interprets$$,$&,$`and$'in the replacement argument even when the search is a plain string. The affected values are file contents, shell/tool output, and conversation text — which routinely contain$. Concrete corruption (verified):$&{current_content}$$$$`$'So e.g.
echo $$ > pid.txtis sent to the model asecho $ > pid.txt, and a file containing$`leaks the prompt template into the<file_content>block.Fix: interpolate in a single pass with a replacer function, e.g.
The function form inserts
$-sequences literally (noGetSubstitution), and the single pass also fixes a sequential-interpolation issue: a placeholder token appearing inside one input value (e.g. an edit parameter that itself contains{current_content}) is no longer re-substituted by a later replacement.Affected call sites (all fixed):
packages/core/src/utils/llm-edit-fixer.ts— edit self-correction promptpackages/core/src/utils/summarizer.ts— tool-output summarizerpackages/core/src/services/sessionSummaryService.ts— session summary(
bugCommand.tsuses the same.replace('{x}', value)shape but wraps values inencodeURIComponent, so it is already safe and left unchanged.)Related Issues
Closes #27556
How to Validate
cd packages/core npm run typecheck npx vitest run src/utils/llm-edit-fixer.test.ts src/utils/summarizer.test.ts src/services/sessionSummaryService.test.tsNew tests in
llm-edit-fixer.test.tscover (a)$-content ($&,$$,$`,$') being inserted literally and (b) a placeholder token supplied as input not being re-interpolated. Both fail on the previous code and pass with this change. The pre-existing prompt test only used$-free fixtures, which is why the bug slipped through.Pre-Merge Checklist
$sequences and placeholder-in-input)vitest+tsc --noEmitfor@google/gemini-cli-core)