Skip to content

Commit f030bf8

Browse files
fix: resolve env-var references in MCP server environment config (closes #656) (#666)
* 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. * fix: export resolveEnvVars and import in tests instead of duplicating Addresses cubic-dev-ai review: tests were exercising a copy of the function, not the production code. Now imports from src/mcp directly. * refactor: use shared tmpdir fixture instead of manual temp-dir lifecycle Addresses coderabbitai review: switched discovery integration tests to use await using tmpdir() from fixture/fixture.ts for automatic cleanup, matching repository test standards. --------- Co-authored-by: anandgupta42 <93243293+anandgupta42@users.noreply.github.com>
1 parent 777a37b commit f030bf8

3 files changed

Lines changed: 247 additions & 1 deletion

File tree

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+
export 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: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
// altimate_change start — tests for MCP env-var interpolation (closes #656)
2+
import { describe, test, expect, beforeEach, afterEach } from "bun:test"
3+
import { mkdir, writeFile } from "fs/promises"
4+
import path from "path"
5+
import { resolveEnvVars } from "../../src/mcp"
6+
import { tmpdir } from "../fixture/fixture"
7+
8+
// -------------------------------------------------------------------------
9+
// resolveEnvVars — safety-net resolver at MCP launch site
10+
// -------------------------------------------------------------------------
11+
12+
describe("resolveEnvVars", () => {
13+
const ORIGINAL_ENV = { ...process.env }
14+
15+
beforeEach(() => {
16+
process.env["TEST_TOKEN"] = "secret-123"
17+
process.env["TEST_HOST"] = "gitlab.example.com"
18+
})
19+
20+
afterEach(() => {
21+
process.env = { ...ORIGINAL_ENV }
22+
})
23+
24+
test("resolves ${VAR} syntax", () => {
25+
const result = resolveEnvVars({
26+
API_TOKEN: "${TEST_TOKEN}",
27+
HOST: "${TEST_HOST}",
28+
})
29+
expect(result.API_TOKEN).toBe("secret-123")
30+
expect(result.HOST).toBe("gitlab.example.com")
31+
})
32+
33+
test("resolves {env:VAR} syntax", () => {
34+
const result = resolveEnvVars({
35+
API_TOKEN: "{env:TEST_TOKEN}",
36+
})
37+
expect(result.API_TOKEN).toBe("secret-123")
38+
})
39+
40+
test("resolves ${VAR:-default} with fallback when unset", () => {
41+
const result = resolveEnvVars({
42+
MODE: "${UNSET_VAR:-production}",
43+
})
44+
expect(result.MODE).toBe("production")
45+
})
46+
47+
test("resolves ${VAR:-default} to env value when set", () => {
48+
const result = resolveEnvVars({
49+
TOKEN: "${TEST_TOKEN:-fallback}",
50+
})
51+
expect(result.TOKEN).toBe("secret-123")
52+
})
53+
54+
test("preserves $${VAR} as literal ${VAR}", () => {
55+
const result = resolveEnvVars({
56+
TEMPLATE: "$${TEST_TOKEN}",
57+
})
58+
expect(result.TEMPLATE).toBe("${TEST_TOKEN}")
59+
})
60+
61+
test("resolves unset variable to empty string", () => {
62+
const result = resolveEnvVars({
63+
MISSING: "${COMPLETELY_UNSET_VAR_XYZ}",
64+
})
65+
expect(result.MISSING).toBe("")
66+
})
67+
68+
test("passes through plain values without modification", () => {
69+
const result = resolveEnvVars({
70+
PLAIN: "just-a-string",
71+
URL: "https://gitlab.com/api/v4",
72+
})
73+
expect(result.PLAIN).toBe("just-a-string")
74+
expect(result.URL).toBe("https://gitlab.com/api/v4")
75+
})
76+
77+
test("resolves multiple variables in a single value", () => {
78+
const result = resolveEnvVars({
79+
URL: "https://${TEST_HOST}/api?token=${TEST_TOKEN}",
80+
})
81+
expect(result.URL).toBe("https://gitlab.example.com/api?token=secret-123")
82+
})
83+
84+
test("handles mixed resolved and plain entries", () => {
85+
const result = resolveEnvVars({
86+
TOKEN: "${TEST_TOKEN}",
87+
STATIC_URL: "https://gitlab.com/api/v4",
88+
HOST: "{env:TEST_HOST}",
89+
})
90+
expect(result.TOKEN).toBe("secret-123")
91+
expect(result.STATIC_URL).toBe("https://gitlab.com/api/v4")
92+
expect(result.HOST).toBe("gitlab.example.com")
93+
})
94+
95+
test("does not interpolate bare $VAR without braces", () => {
96+
const result = resolveEnvVars({
97+
TOKEN: "$TEST_TOKEN",
98+
})
99+
expect(result.TOKEN).toBe("$TEST_TOKEN")
100+
})
101+
102+
test("handles empty environment object", () => {
103+
const result = resolveEnvVars({})
104+
expect(result).toEqual({})
105+
})
106+
})
107+
108+
// -------------------------------------------------------------------------
109+
// Discovery integration — env vars in external MCP configs
110+
// -------------------------------------------------------------------------
111+
112+
describe("discoverExternalMcp with env-var interpolation", () => {
113+
const ORIGINAL_ENV = { ...process.env }
114+
115+
beforeEach(() => {
116+
process.env["TEST_MCP_TOKEN"] = "glpat-secret-token"
117+
process.env["TEST_MCP_HOST"] = "https://gitlab.internal.com"
118+
})
119+
120+
afterEach(() => {
121+
process.env = { ...ORIGINAL_ENV }
122+
})
123+
124+
test("resolves ${VAR} in discovered .vscode/mcp.json environment", async () => {
125+
await using tmp = await tmpdir()
126+
const dir = tmp.path
127+
await mkdir(path.join(dir, ".vscode"), { recursive: true })
128+
await writeFile(
129+
path.join(dir, ".vscode/mcp.json"),
130+
JSON.stringify({
131+
servers: {
132+
gitlab: {
133+
command: "node",
134+
args: ["gitlab-server.js"],
135+
env: {
136+
GITLAB_TOKEN: "${TEST_MCP_TOKEN}",
137+
GITLAB_HOST: "${TEST_MCP_HOST}",
138+
STATIC_VALUE: "no-interpolation-needed",
139+
},
140+
},
141+
},
142+
}),
143+
)
144+
145+
const { discoverExternalMcp } = await import("../../src/mcp/discover")
146+
const { servers } = await discoverExternalMcp(dir)
147+
148+
expect(servers["gitlab"]).toBeDefined()
149+
expect(servers["gitlab"].type).toBe("local")
150+
const env = (servers["gitlab"] as any).environment
151+
expect(env.GITLAB_TOKEN).toBe("glpat-secret-token")
152+
expect(env.GITLAB_HOST).toBe("https://gitlab.internal.com")
153+
expect(env.STATIC_VALUE).toBe("no-interpolation-needed")
154+
})
155+
156+
test("resolves {env:VAR} in discovered .cursor/mcp.json environment", async () => {
157+
await using tmp = await tmpdir()
158+
const dir = tmp.path
159+
await mkdir(path.join(dir, ".cursor"), { recursive: true })
160+
await writeFile(
161+
path.join(dir, ".cursor/mcp.json"),
162+
JSON.stringify({
163+
mcpServers: {
164+
"my-tool": {
165+
command: "npx",
166+
args: ["-y", "my-mcp-tool"],
167+
env: {
168+
API_KEY: "{env:TEST_MCP_TOKEN}",
169+
},
170+
},
171+
},
172+
}),
173+
)
174+
175+
const { discoverExternalMcp } = await import("../../src/mcp/discover")
176+
const { servers } = await discoverExternalMcp(dir)
177+
178+
expect(servers["my-tool"]).toBeDefined()
179+
const env = (servers["my-tool"] as any).environment
180+
expect(env.API_KEY).toBe("glpat-secret-token")
181+
})
182+
183+
test("resolves ${VAR:-default} with fallback in discovered config", async () => {
184+
await using tmp = await tmpdir()
185+
const dir = tmp.path
186+
await mkdir(path.join(dir, ".vscode"), { recursive: true })
187+
await writeFile(
188+
path.join(dir, ".vscode/mcp.json"),
189+
JSON.stringify({
190+
servers: {
191+
svc: {
192+
command: "node",
193+
args: ["svc.js"],
194+
env: {
195+
MODE: "${UNSET_VAR_ABC:-production}",
196+
},
197+
},
198+
},
199+
}),
200+
)
201+
202+
const { discoverExternalMcp } = await import("../../src/mcp/discover")
203+
const { servers } = await discoverExternalMcp(dir)
204+
205+
const env = (servers["svc"] as any).environment
206+
expect(env.MODE).toBe("production")
207+
})
208+
})
209+
// altimate_change end

0 commit comments

Comments
 (0)