From 089d67ee6f1c909d6afec7c19604cb36d59650c2 Mon Sep 17 00:00:00 2001
From: Dhruba Datta <74358627+dhruba-datta@users.noreply.github.com>
Date: Sat, 16 May 2026 01:36:20 +0600
Subject: [PATCH] fix(filesystem): preserve literal $ in edit_file newText

Fixes modelcontextprotocol/servers#4157.

`applyFileEdits` (in src/filesystem/lib.ts) invoked
`String.prototype.replace(searchString, replacementString)` where the
second argument is interpreted as a replacement-string pattern, not a
literal string. This silently corrupted content containing literal
$ characters:

  | Pattern in newText | Replaced by                       |
  | ------------------ | --------------------------------- |
  | $$               | a single $                       |
  | $&                | the matched substring             |
  | $`               | the substring preceding the match |
  | $'                | the substring following the match |
  | $<name>           | (regex named-group syntax)        |

Particularly hit JS/TS code with $variables, currency, regex source,
and shell scripts.

Fix: pass the replacement via the callback form
`replace(needle, () => normalizedNew)`. The function's return value
is used verbatim, bypassing the pattern interpreter, while preserving
"replace first occurrence only" semantics. The fallback line-by-line
path was not affected.

Added regression test in lib.test.ts covering all five pattern cases.
---
 src/filesystem/__tests__/lib.test.ts | 32 +++++++++++++++++++++++++++-
 src/filesystem/lib.ts                | 11 ++++++++--
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts
index f7e585af22..b15dc9769c 100644
--- a/src/filesystem/__tests__/lib.test.ts
+++ b/src/filesystem/__tests__/lib.test.ts
@@ -498,11 +498,41 @@ describe('Lib Functions', () => {
         const edits = [
           { oldText: 'nonexistent line', newText: 'replacement' }
         ];
-        
+
         await expect(applyFileEdits('/test/file.txt', edits, false))
           .rejects.toThrow('Could not find exact match for edit');
       });
 
+      // Regression test for #4157: newText containing String.prototype.replace
+      // replacement-pattern characters ($$, $&, $`, $', $<name>) must be treated
+      // as a literal string, not a pattern. Otherwise content like "$$100 USD"
+      // silently corrupts to "$100 USD" on the exact-match path.
+      it('preserves literal $ in newText (issue #4157)', async () => {
+        mockFs.readFile.mockResolvedValue('price: PLACEHOLDER\n');
+
+        const cases: Array<[string, string]> = [
+          ['$$100 USD',       'price: $$100 USD\n'],
+          ['cost: $&',        'price: cost: $&\n'],
+          ['var: $`x$\'',     'price: var: $`x$\'\n'],
+          ['template: $<x>',  'price: template: $<x>\n'],
+        ];
+
+        for (const [newText, expected] of cases) {
+          mockFs.writeFile.mockClear();
+          mockFs.rename.mockResolvedValueOnce(undefined);
+
+          await applyFileEdits('/test/file.txt', [
+            { oldText: 'PLACEHOLDER', newText }
+          ], false);
+
+          expect(mockFs.writeFile).toHaveBeenCalledWith(
+            expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+            expected,
+            'utf-8'
+          );
+        }
+      });
+
       it('handles complex multi-line edits with indentation', async () => {
         mockFs.readFile.mockResolvedValue('function test() {\n  console.log("hello");\n  return true;\n}');
         
diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts
index 17e4654cd5..ec4531af8f 100644
--- a/src/filesystem/lib.ts
+++ b/src/filesystem/lib.ts
@@ -205,9 +205,16 @@ export async function applyFileEdits(
     const normalizedOld = normalizeLineEndings(edit.oldText);
     const normalizedNew = normalizeLineEndings(edit.newText);
 
-    // If exact match exists, use it
+    // If exact match exists, use it.
+    //
+    // Using the function form of replace() is important: with a string
+    // replacement, JavaScript interprets `$$`, `$&`, `` $` ``, `$'`, and
+    // `$<name>` as replacement-pattern syntax, which silently corrupts
+    // content that contains literal `$` characters (e.g. JS/TS code,
+    // currency amounts, shell variables). The arrow function returns
+    // the replacement as a literal string and bypasses the pattern.
     if (modifiedContent.includes(normalizedOld)) {
-      modifiedContent = modifiedContent.replace(normalizedOld, normalizedNew);
+      modifiedContent = modifiedContent.replace(normalizedOld, () => normalizedNew);
       continue;
     }