Skip to content

Commit 8e1671f

Browse files
committed
fix: resolve env-var references in MCP server environment config (closes #656)
${VAR}, ${VAR:-default}, and {env:VAR} patterns in MCP server environment blocks were passed as literal strings to child processes, causing auth failures for tools like gitlab-mcp-server. Two gaps fixed: - mcp/index.ts: add resolveEnvVars() safety net at launch site that resolves env-var patterns in mcp.environment before spawning - discover.ts: use ConfigPaths.parseText() in readJsonSafe() so external MCP configs (Claude Code, Cursor, Copilot, Gemini) get interpolation before JSON parsing 14 new tests covering both ${VAR} and {env:VAR} syntax, defaults, escapes, and discovery integration.
1 parent dafd16a commit 8e1671f

File tree

3 files changed

+267
-1
lines changed

3 files changed

+267
-1
lines changed

packages/opencode/src/mcp/discover.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,18 @@ async function readJsonSafe(filePath: string): Promise<any | undefined> {
117117
} catch {
118118
return undefined
119119
}
120+
// altimate_change start — apply env-var interpolation to external MCP configs
121+
// External configs (Claude Code, Cursor, Copilot, Gemini) may contain ${VAR}
122+
// or {env:VAR} references that need resolving before JSON parsing.
123+
// Uses ConfigPaths.parseText which runs substitute() then parses JSONC.
124+
try {
125+
const { ConfigPaths } = await import("../config/paths")
126+
return await ConfigPaths.parseText(text, filePath, "empty")
127+
} catch {
128+
// Substitution or parse failure — fall back to direct parse without interpolation
129+
log.debug("env-var interpolation failed for external MCP config, falling back to direct parse", { file: filePath })
130+
}
131+
// altimate_change end
120132
const errors: any[] = []
121133
const result = parseJsonc(text, errors, { allowTrailingComma: true })
122134
if (errors.length > 0) {

packages/opencode/src/mcp/index.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,29 @@ import { TuiEvent } from "@/cli/cmd/tui/event"
2525
import open from "open"
2626
import { Telemetry } from "@/telemetry"
2727

28+
// altimate_change start — resolve env-var references in MCP environment values
29+
// Handles ${VAR}, ${VAR:-default}, and {env:VAR} patterns that may have survived
30+
// config parsing (e.g. discovered external configs, config updates via updateGlobal).
31+
const ENV_VAR_PATTERN =
32+
/\$\$(\{[A-Za-z_][A-Za-z0-9_]*(?::-[^}]*)?\})|(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}|\{env:([^}]+)\}/g
33+
34+
function resolveEnvVars(environment: Record<string, string>): Record<string, string> {
35+
const resolved: Record<string, string> = {}
36+
for (const [key, value] of Object.entries(environment)) {
37+
resolved[key] = value.replace(ENV_VAR_PATTERN, (match, escaped, dollarVar, dollarDefault, braceVar) => {
38+
if (escaped !== undefined) return "$" + escaped
39+
if (dollarVar !== undefined) {
40+
const envValue = process.env[dollarVar]
41+
return envValue !== undefined && envValue !== "" ? envValue : (dollarDefault ?? "")
42+
}
43+
if (braceVar !== undefined) return process.env[braceVar] || ""
44+
return match
45+
})
46+
}
47+
return resolved
48+
}
49+
// altimate_change end
50+
2851
export namespace MCP {
2952
const log = Log.create({ service: "mcp" })
3053
const DEFAULT_TIMEOUT = 30_000
@@ -509,7 +532,9 @@ export namespace MCP {
509532
env: {
510533
...process.env,
511534
...(cmd === "altimate" || cmd === "altimate-code" ? { BUN_BE_BUN: "1" } : {}),
512-
...mcp.environment,
535+
// altimate_change start — resolve ${VAR} / {env:VAR} patterns that survived config parsing
536+
...(mcp.environment ? resolveEnvVars(mcp.environment) : {}),
537+
// altimate_change end
513538
},
514539
})
515540
transport.stderr?.on("data", (chunk: Buffer) => {
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
// altimate_change start — tests for MCP env-var interpolation (closes #656)
2+
import { describe, test, expect, beforeEach, afterEach } from "bun:test"
3+
import { mkdtemp, rm, mkdir, writeFile } from "fs/promises"
4+
import { tmpdir } from "os"
5+
import path from "path"
6+
7+
// -------------------------------------------------------------------------
8+
// resolveEnvVars — safety-net resolver at MCP launch site
9+
// -------------------------------------------------------------------------
10+
11+
// Import the module to access resolveEnvVars indirectly via the env spread.
12+
// Since resolveEnvVars is a module-level function (not inside the MCP namespace),
13+
// we test it directly by importing the file and extracting the function.
14+
// For now, inline the same logic here for unit testing.
15+
16+
const ENV_VAR_PATTERN =
17+
/\$\$(\{[A-Za-z_][A-Za-z0-9_]*(?::-[^}]*)?\})|(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}|\{env:([^}]+)\}/g
18+
19+
function resolveEnvVars(environment: Record<string, string>): Record<string, string> {
20+
const resolved: Record<string, string> = {}
21+
for (const [key, value] of Object.entries(environment)) {
22+
resolved[key] = value.replace(ENV_VAR_PATTERN, (match, escaped, dollarVar, dollarDefault, braceVar) => {
23+
if (escaped !== undefined) return "$" + escaped
24+
if (dollarVar !== undefined) {
25+
const envValue = process.env[dollarVar]
26+
return envValue !== undefined && envValue !== "" ? envValue : (dollarDefault ?? "")
27+
}
28+
if (braceVar !== undefined) return process.env[braceVar] || ""
29+
return match
30+
})
31+
}
32+
return resolved
33+
}
34+
35+
describe("resolveEnvVars", () => {
36+
const ORIGINAL_ENV = { ...process.env }
37+
38+
beforeEach(() => {
39+
process.env["TEST_TOKEN"] = "secret-123"
40+
process.env["TEST_HOST"] = "gitlab.example.com"
41+
})
42+
43+
afterEach(() => {
44+
process.env = { ...ORIGINAL_ENV }
45+
})
46+
47+
test("resolves ${VAR} syntax", () => {
48+
const result = resolveEnvVars({
49+
API_TOKEN: "${TEST_TOKEN}",
50+
HOST: "${TEST_HOST}",
51+
})
52+
expect(result.API_TOKEN).toBe("secret-123")
53+
expect(result.HOST).toBe("gitlab.example.com")
54+
})
55+
56+
test("resolves {env:VAR} syntax", () => {
57+
const result = resolveEnvVars({
58+
API_TOKEN: "{env:TEST_TOKEN}",
59+
})
60+
expect(result.API_TOKEN).toBe("secret-123")
61+
})
62+
63+
test("resolves ${VAR:-default} with fallback when unset", () => {
64+
const result = resolveEnvVars({
65+
MODE: "${UNSET_VAR:-production}",
66+
})
67+
expect(result.MODE).toBe("production")
68+
})
69+
70+
test("resolves ${VAR:-default} to env value when set", () => {
71+
const result = resolveEnvVars({
72+
TOKEN: "${TEST_TOKEN:-fallback}",
73+
})
74+
expect(result.TOKEN).toBe("secret-123")
75+
})
76+
77+
test("preserves $${VAR} as literal ${VAR}", () => {
78+
const result = resolveEnvVars({
79+
TEMPLATE: "$${TEST_TOKEN}",
80+
})
81+
expect(result.TEMPLATE).toBe("${TEST_TOKEN}")
82+
})
83+
84+
test("resolves unset variable to empty string", () => {
85+
const result = resolveEnvVars({
86+
MISSING: "${COMPLETELY_UNSET_VAR_XYZ}",
87+
})
88+
expect(result.MISSING).toBe("")
89+
})
90+
91+
test("passes through plain values without modification", () => {
92+
const result = resolveEnvVars({
93+
PLAIN: "just-a-string",
94+
URL: "https://gitlab.com/api/v4",
95+
})
96+
expect(result.PLAIN).toBe("just-a-string")
97+
expect(result.URL).toBe("https://gitlab.com/api/v4")
98+
})
99+
100+
test("resolves multiple variables in a single value", () => {
101+
const result = resolveEnvVars({
102+
URL: "https://${TEST_HOST}/api?token=${TEST_TOKEN}",
103+
})
104+
expect(result.URL).toBe("https://gitlab.example.com/api?token=secret-123")
105+
})
106+
107+
test("handles mixed resolved and plain entries", () => {
108+
const result = resolveEnvVars({
109+
TOKEN: "${TEST_TOKEN}",
110+
STATIC_URL: "https://gitlab.com/api/v4",
111+
HOST: "{env:TEST_HOST}",
112+
})
113+
expect(result.TOKEN).toBe("secret-123")
114+
expect(result.STATIC_URL).toBe("https://gitlab.com/api/v4")
115+
expect(result.HOST).toBe("gitlab.example.com")
116+
})
117+
118+
test("does not interpolate bare $VAR without braces", () => {
119+
const result = resolveEnvVars({
120+
TOKEN: "$TEST_TOKEN",
121+
})
122+
expect(result.TOKEN).toBe("$TEST_TOKEN")
123+
})
124+
125+
test("handles empty environment object", () => {
126+
const result = resolveEnvVars({})
127+
expect(result).toEqual({})
128+
})
129+
})
130+
131+
// -------------------------------------------------------------------------
132+
// Discovery integration — env vars in external MCP configs
133+
// -------------------------------------------------------------------------
134+
135+
describe("discoverExternalMcp with env-var interpolation", () => {
136+
let tempDir: string
137+
const ORIGINAL_ENV = { ...process.env }
138+
139+
beforeEach(async () => {
140+
tempDir = await mkdtemp(path.join(tmpdir(), "mcp-envvar-"))
141+
process.env["TEST_MCP_TOKEN"] = "glpat-secret-token"
142+
process.env["TEST_MCP_HOST"] = "https://gitlab.internal.com"
143+
})
144+
145+
afterEach(async () => {
146+
process.env = { ...ORIGINAL_ENV }
147+
await rm(tempDir, { recursive: true, force: true })
148+
})
149+
150+
test("resolves ${VAR} in discovered .vscode/mcp.json environment", async () => {
151+
await mkdir(path.join(tempDir, ".vscode"), { recursive: true })
152+
await writeFile(
153+
path.join(tempDir, ".vscode/mcp.json"),
154+
JSON.stringify({
155+
servers: {
156+
gitlab: {
157+
command: "node",
158+
args: ["gitlab-server.js"],
159+
env: {
160+
GITLAB_TOKEN: "${TEST_MCP_TOKEN}",
161+
GITLAB_HOST: "${TEST_MCP_HOST}",
162+
STATIC_VALUE: "no-interpolation-needed",
163+
},
164+
},
165+
},
166+
}),
167+
)
168+
169+
const { discoverExternalMcp } = await import("../../src/mcp/discover")
170+
const { servers } = await discoverExternalMcp(tempDir)
171+
172+
expect(servers["gitlab"]).toBeDefined()
173+
expect(servers["gitlab"].type).toBe("local")
174+
const env = (servers["gitlab"] as any).environment
175+
expect(env.GITLAB_TOKEN).toBe("glpat-secret-token")
176+
expect(env.GITLAB_HOST).toBe("https://gitlab.internal.com")
177+
expect(env.STATIC_VALUE).toBe("no-interpolation-needed")
178+
})
179+
180+
test("resolves {env:VAR} in discovered .cursor/mcp.json environment", async () => {
181+
await mkdir(path.join(tempDir, ".cursor"), { recursive: true })
182+
await writeFile(
183+
path.join(tempDir, ".cursor/mcp.json"),
184+
JSON.stringify({
185+
mcpServers: {
186+
"my-tool": {
187+
command: "npx",
188+
args: ["-y", "my-mcp-tool"],
189+
env: {
190+
API_KEY: "{env:TEST_MCP_TOKEN}",
191+
},
192+
},
193+
},
194+
}),
195+
)
196+
197+
const { discoverExternalMcp } = await import("../../src/mcp/discover")
198+
const { servers } = await discoverExternalMcp(tempDir)
199+
200+
expect(servers["my-tool"]).toBeDefined()
201+
const env = (servers["my-tool"] as any).environment
202+
expect(env.API_KEY).toBe("glpat-secret-token")
203+
})
204+
205+
test("resolves ${VAR:-default} with fallback in discovered config", async () => {
206+
await mkdir(path.join(tempDir, ".vscode"), { recursive: true })
207+
await writeFile(
208+
path.join(tempDir, ".vscode/mcp.json"),
209+
JSON.stringify({
210+
servers: {
211+
svc: {
212+
command: "node",
213+
args: ["svc.js"],
214+
env: {
215+
MODE: "${UNSET_VAR_ABC:-production}",
216+
},
217+
},
218+
},
219+
}),
220+
)
221+
222+
const { discoverExternalMcp } = await import("../../src/mcp/discover")
223+
const { servers } = await discoverExternalMcp(tempDir)
224+
225+
const env = (servers["svc"] as any).environment
226+
expect(env.MODE).toBe("production")
227+
})
228+
})
229+
// altimate_change end

0 commit comments

Comments
 (0)