Skip to content

Commit e583f1e

Browse files
committed
fix: normalize Windows paths in MCP variable injection (#4739)
- Use path.posix.normalize to convert backslashes to forward slashes - Only normalize strings that contain path separators - Ensures cross-platform compatibility and prevents JSON parsing errors - Add comprehensive tests for Windows path normalization - Add tests to verify non-path variables remain unchanged - Fixes issue where ${workspaceFolder} with Windows paths caused invalid JSON
1 parent 2e2f83b commit e583f1e

File tree

2 files changed

+126
-4
lines changed

2 files changed

+126
-4
lines changed

src/utils/__tests__/config.spec.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { vitest, describe, it, expect, beforeEach, afterAll } from "vitest"
22
import { injectEnv, injectVariables } from "../config"
33

4-
54
describe("injectEnv", () => {
65
const originalEnv = process.env
76

@@ -147,5 +146,109 @@ describe("injectVariables", () => {
147146
expect(result).toEqual("Hello ")
148147
})
149148

149+
it("should normalize Windows paths with backslashes to use forward slashes in JSON objects", async () => {
150+
const config = {
151+
command: "mcp-server",
152+
args: ["${workspaceFolder}"],
153+
}
154+
const result = await injectVariables(config, { workspaceFolder: "C:\\Users\\project" })
155+
expect(result).toEqual({
156+
command: "mcp-server",
157+
args: ["C:/Users/project"],
158+
})
159+
})
160+
161+
it("should handle complex Windows paths in nested objects", async () => {
162+
const config = {
163+
servers: {
164+
git: {
165+
command: "node",
166+
args: ["${workspaceFolder}\\scripts\\mcp.js", "${workspaceFolder}\\data"],
167+
},
168+
},
169+
}
170+
const result = await injectVariables(config, { workspaceFolder: "C:\\Program Files\\My Project" })
171+
expect(result).toEqual({
172+
servers: {
173+
git: {
174+
command: "node",
175+
args: ["C:/Program Files/My Project\\scripts\\mcp.js", "C:/Program Files/My Project\\data"],
176+
},
177+
},
178+
})
179+
})
180+
181+
it("should handle Windows paths when entire path is a variable", async () => {
182+
const config = {
183+
servers: {
184+
git: {
185+
command: "node",
186+
args: ["${scriptPath}", "${dataPath}"],
187+
},
188+
},
189+
}
190+
const result = await injectVariables(config, {
191+
scriptPath: "C:\\Program Files\\My Project\\scripts\\mcp.js",
192+
dataPath: "C:\\Program Files\\My Project\\data",
193+
})
194+
expect(result).toEqual({
195+
servers: {
196+
git: {
197+
command: "node",
198+
args: ["C:/Program Files/My Project/scripts/mcp.js", "C:/Program Files/My Project/data"],
199+
},
200+
},
201+
})
202+
})
203+
204+
it("should normalize backslashes in plain string replacements", async () => {
205+
const result = await injectVariables("Path: ${path}", { path: "C:\\Users\\test" })
206+
expect(result).toEqual("Path: C:/Users/test")
207+
})
208+
209+
it("should handle paths with mixed slashes", async () => {
210+
const config = {
211+
path: "${testPath}",
212+
}
213+
const result = await injectVariables(config, { testPath: "C:\\Users/test/mixed\\path" })
214+
expect(result).toEqual({
215+
path: "C:/Users/test/mixed/path",
216+
})
217+
})
218+
219+
it("should not affect non-path strings", async () => {
220+
const config = {
221+
message: "This is a string with a backslash \\ and a value: ${myValue}",
222+
}
223+
const result = await injectVariables(config, { myValue: "test" })
224+
expect(result).toEqual({
225+
message: "This is a string with a backslash \\ and a value: test",
226+
})
227+
})
228+
229+
it("should handle various non-path variables correctly", async () => {
230+
const config = {
231+
apiKey: "${key}",
232+
url: "${endpoint}",
233+
port: "${port}",
234+
enabled: "${enabled}",
235+
description: "${desc}",
236+
}
237+
const result = await injectVariables(config, {
238+
key: "sk-1234567890abcdef",
239+
endpoint: "https://api.example.com",
240+
port: "8080",
241+
enabled: "true",
242+
desc: "This is a description with special chars: @#$%^&*()",
243+
})
244+
expect(result).toEqual({
245+
apiKey: "sk-1234567890abcdef",
246+
url: "https://api.example.com",
247+
port: "8080",
248+
enabled: "true",
249+
description: "This is a description with special chars: @#$%^&*()",
250+
})
251+
})
252+
150253
// Variable maps are already tested by `injectEnv` tests above.
151254
})

src/utils/config.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,19 @@ export type InjectableConfigType =
1010
| Array<undefined | null | boolean | number | InjectableConfigType>
1111
}
1212

13+
/**
14+
* Normalizes a path to use forward slashes consistently
15+
* This ensures paths work correctly in JSON across all platforms
16+
*/
17+
function normalizePath(value: string): string {
18+
// Check if the value looks like a path (contains backslashes or forward slashes)
19+
if (value.includes("\\") || value.includes("/")) {
20+
// Convert to forward slashes for cross-platform compatibility
21+
return value.replace(/\\/g, "/")
22+
}
23+
return value
24+
}
25+
1326
/**
1427
* Deeply injects environment variables into a configuration object/string/json
1528
*
@@ -45,14 +58,20 @@ export async function injectVariables<C extends InjectableConfigType>(
4558
for (const [key, value] of Object.entries(variables)) {
4659
if (value == null) continue
4760

48-
if (typeof value === "string") _config = _config.replace(new RegExp(`\\$\\{${key}\\}`, "g"), value)
49-
else
61+
if (typeof value === "string") {
62+
// Normalize paths to use forward slashes for cross-platform compatibility
63+
const normalizedValue = normalizePath(value)
64+
_config = _config.replace(new RegExp(`\\$\\{${key}\\}`, "g"), normalizedValue)
65+
} else {
5066
_config = _config.replace(new RegExp(`\\$\\{${key}:([\\w]+)\\}`, "g"), (match, name) => {
5167
if (value[name] == null)
5268
console.warn(`[injectVariables] variable "${name}" referenced but not found in "${key}"`)
5369

54-
return value[name] ?? propNotFoundValue ?? match
70+
const replacementValue = value[name] ?? propNotFoundValue ?? match
71+
// Normalize paths if the replacement is a string
72+
return typeof replacementValue === "string" ? normalizePath(replacementValue) : replacementValue
5573
})
74+
}
5675
}
5776

5877
return (isObject ? JSON.parse(_config) : _config) as C extends string ? string : C

0 commit comments

Comments
 (0)