fix: ${VAR} env-var interpolation for configs (closes #635)

docs/docs/configure/mcp-servers.md

@@ -20,6 +20,21 @@ Run an MCP server as a local subprocess:
 }
 ```
 
+### Environment variable interpolation
+
+Both syntaxes work anywhere in the config:
+
+| Syntax | Injection mode | Example |
+|--------|----------------|---------|
+| `${VAR}` | String-safe (JSON-escaped) | `"API_KEY": "${MY_API_KEY}"` — shell / dotenv style |
+| `${VAR:-default}` | String-safe with fallback | `"MODE": "${APP_MODE:-production}"` — used when `VAR` is unset or empty |
+| `{env:VAR}` | Raw text | `"count": {env:NUM}` — use for unquoted structural injection |
+| `$${VAR}` | Escape hatch | `"template": "$${VAR}"` — preserves literal `${VAR}` (docker-compose style) |
+
+If the variable is not set and no default is given, it resolves to an empty string. Bare `$VAR` (without braces) is **not** interpolated — use `${VAR}` or `{env:VAR}`.
+
+**Why two syntaxes?** `${VAR}` JSON-escapes the value so tokens containing quotes or braces can't break the config structure — the safe default for secrets. `{env:VAR}` does raw text injection for the rare case where you need to inject numbers or structure into unquoted JSON positions.
+
 | Field | Type | Description |
 |-------|------|-------------|
 | `type` | `"local"` | Local subprocess server |

packages/opencode/src/altimate/telemetry/index.ts

@@ -659,6 +659,25 @@ export namespace Telemetry {
         error_message?: string
       }
     // altimate_change end
+    // altimate_change start — config env-var interpolation telemetry
+    | {
+        type: "config_env_interpolation"
+        timestamp: number
+        session_id: string
+        /** ${VAR} / ${VAR:-default} references encountered */
+        dollar_refs: number
+        /** ${VAR} with no value and no default → resolved to empty string (footgun signal) */
+        dollar_unresolved: number
+        /** ${VAR:-default} where default was used */
+        dollar_defaulted: number
+        /** $${VAR} literal escape sequences found */
+        dollar_escaped: number
+        /** legacy {env:VAR} references (raw injection syntax) */
+        legacy_brace_refs: number
+        /** {env:VAR} with no value → empty string */
+        legacy_brace_unresolved: number
+      }
+    // altimate_change end
     // altimate_change start — plan-agent model tool-call refusal detection
     | {
         type: "plan_no_tool_generation"

packages/opencode/src/cli/cmd/tui/component/tips.tsx

@@ -147,7 +147,7 @@ const TIPS = [
   "Create JSON theme files in {highlight}.altimate-code/themes/{/highlight} directory",
   "Themes support dark/light variants for both modes",
   "Reference ANSI colors 0-255 in custom themes",
-  "Use {highlight}{env:VAR_NAME}{/highlight} syntax to reference environment variables in config",
+  "Use {highlight}${VAR_NAME}{/highlight} or {highlight}{env:VAR_NAME}{/highlight} to reference environment variables in config",
   "Use {highlight}{file:path}{/highlight} to include file contents in config values",
   "Use {highlight}instructions{/highlight} in config to load additional rules files",
   "Set agent {highlight}temperature{/highlight} from 0.0 (focused) to 1.0 (creative)",

packages/opencode/src/config/paths.ts

@@ -86,9 +86,69 @@ export namespace ConfigPaths {
 
   /** Apply {env:VAR} and {file:path} substitutions to config text. */
   async function substitute(text: string, input: ParseSource, missing: "error" | "empty" = "error") {
-    text = text.replace(/\{env:([^}]+)\}/g, (_, varName) => {
-      return process.env[varName] || ""
-    })
+    // altimate_change start — unified env-var interpolation
+    // Single-pass substitution against the ORIGINAL text prevents output of one
+    // pattern being re-matched by another (e.g. {env:A}="${B}" expanding B).
+    // Syntaxes (order tried, in one regex via alternation):
+    //   1. $${VAR} or $${VAR:-default} — literal escape (docker-compose style)
+    //   2. ${VAR} or ${VAR:-default}   — string-safe, JSON-escaped (shell/dotenv)
+    //   3. {env:VAR}                    — raw text injection (backward compat)
+    // Users arriving from Claude Code / VS Code / dotenv / docker-compose expect
+    // ${VAR}. Use {env:VAR} for raw unquoted injection. See issue #635.
+    let dollarRefs = 0
+    let dollarUnresolved = 0
+    let dollarDefaulted = 0
+    let dollarEscaped = 0
+    let legacyBraceRefs = 0
+    let legacyBraceUnresolved = 0
+    text = text.replace(
+      /\$\$(\{[A-Za-z_][A-Za-z0-9_]*(?::-[^}]*)?\})|(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}|\{env:([^}]+)\}/g,
+      (match, escaped, dollarVar, dollarDefault, braceVar) => {
+        if (escaped !== undefined) {
+          // $${VAR} → literal ${VAR}
+          dollarEscaped++
+          return "$" + escaped
+        }
+        if (dollarVar !== undefined) {
+          // ${VAR} / ${VAR:-default} → JSON-escaped string-safe substitution
+          dollarRefs++
+          const envValue = process.env[dollarVar]
+          const resolved = envValue !== undefined && envValue !== ""
+          if (!resolved && dollarDefault !== undefined) dollarDefaulted++
+          if (!resolved && dollarDefault === undefined) dollarUnresolved++
+          const value = resolved ? envValue : (dollarDefault ?? "")
+          return JSON.stringify(value).slice(1, -1)
+        }
+        if (braceVar !== undefined) {
+          // {env:VAR} → raw text injection
+          legacyBraceRefs++
+          const v = process.env[braceVar]
+          if (v === undefined || v === "") legacyBraceUnresolved++
+          return v || ""
+        }
+        return match
+      },
+    )
+    // Emit telemetry if any env interpolation happened. Dynamic import avoids a
+    // circular dep with @/altimate/telemetry (which imports @/config/config).
+    if (dollarRefs > 0 || legacyBraceRefs > 0 || dollarEscaped > 0) {
+      import("@/altimate/telemetry")
+        .then(({ Telemetry }) => {
+          Telemetry.track({
+            type: "config_env_interpolation",
+            timestamp: Date.now(),
+            session_id: Telemetry.getContext().sessionId,
+            dollar_refs: dollarRefs,
+            dollar_unresolved: dollarUnresolved,
+            dollar_defaulted: dollarDefaulted,
+            dollar_escaped: dollarEscaped,
+            legacy_brace_refs: legacyBraceRefs,
+            legacy_brace_unresolved: legacyBraceUnresolved,
+          })
+        })
+        .catch(() => {})
+    }
+    // altimate_change end
 
     const fileMatches = Array.from(text.matchAll(/\{file:[^}]+\}/g))
     if (!fileMatches.length) return text

packages/opencode/test/config/paths-parsetext.test.ts

@@ -97,6 +97,210 @@ describe("ConfigPaths.parseText: {env:VAR} substitution", () => {
   })
 })
 
+describe("ConfigPaths.parseText: ${VAR} substitution (shell/dotenv alias)", () => {
+  const envKey = "OPENCODE_TEST_SHELL_SYNTAX_KEY"
+
+  beforeEach(() => {
+    process.env[envKey] = "shell-style-value"
+  })
+
+  afterEach(() => {
+    delete process.env[envKey]
+  })
+
+  test("substitutes ${VAR} with environment variable value", async () => {
+    const text = `{"apiKey": "\${${envKey}}"}`
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result).toEqual({ apiKey: "shell-style-value" })
+  })
+
+  test("substitutes to empty string when env var is not set", async () => {
+    const text = '{"apiKey": "${OPENCODE_TEST_SHELL_NONEXISTENT_XYZ}"}'
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result).toEqual({ apiKey: "" })
+  })
+
+  test("${VAR} and {env:VAR} both work in same config", async () => {
+    process.env.OPENCODE_TEST_MIXED_A = "alpha"
+    process.env.OPENCODE_TEST_MIXED_B = "beta"
+    try {
+      const text = '{"a": "${OPENCODE_TEST_MIXED_A}", "b": "{env:OPENCODE_TEST_MIXED_B}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result).toEqual({ a: "alpha", b: "beta" })
+    } finally {
+      delete process.env.OPENCODE_TEST_MIXED_A
+      delete process.env.OPENCODE_TEST_MIXED_B
+    }
+  })
+
+  test("ignores ${...} with non-identifier names (spaces, special chars)", async () => {
+    // These should pass through unmodified — not valid POSIX identifiers
+    const text = '{"a": "${FOO BAR}", "b": "${foo-bar}", "c": "${foo.bar}"}'
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result).toEqual({ a: "${FOO BAR}", b: "${foo-bar}", c: "${foo.bar}" })
+  })
+
+  test("does not match bare $VAR (without braces)", async () => {
+    process.env.OPENCODE_TEST_BARE = "should-not-match"
+    try {
+      const text = '{"value": "$OPENCODE_TEST_BARE"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      // Bare $VAR stays literal — only ${VAR} is interpolated
+      expect(result).toEqual({ value: "$OPENCODE_TEST_BARE" })
+    } finally {
+      delete process.env.OPENCODE_TEST_BARE
+    }
+  })
+
+  test("JSON-safe: env value with quotes cannot inject JSON structure", async () => {
+    // Security regression test for C1 in consensus review of PR #655.
+    // {env:VAR} is raw injection (backward compat); ${VAR} is string-safe.
+    process.env.OPENCODE_TEST_INJECT = 'pwned", "isAdmin": true, "x": "y'
+    try {
+      const text = '{"token": "${OPENCODE_TEST_INJECT}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      // Value stays inside the "token" string — no injection into sibling keys
+      expect(result).toEqual({ token: 'pwned", "isAdmin": true, "x": "y' })
+      expect(result.isAdmin).toBeUndefined()
+    } finally {
+      delete process.env.OPENCODE_TEST_INJECT
+    }
+  })
+
+  test("JSON-safe: env value with backslash and newline escaped properly", async () => {
+    process.env.OPENCODE_TEST_MULTILINE = 'line1\nline2\tpath\\to\\file'
+    try {
+      const text = '{"value": "${OPENCODE_TEST_MULTILINE}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result).toEqual({ value: "line1\nline2\tpath\\to\\file" })
+    } finally {
+      delete process.env.OPENCODE_TEST_MULTILINE
+    }
+  })
+
+  test("default: ${VAR:-default} uses default when var unset", async () => {
+    // Variable is not set — default value should be used
+    const text = '{"mode": "${OPENCODE_TEST_UNSET_VAR:-production}"}'
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result).toEqual({ mode: "production" })
+  })
+
+  test("default: ${VAR:-default} uses env value when var set", async () => {
+    process.env.OPENCODE_TEST_DEFAULT_OVERRIDE = "staging"
+    try {
+      const text = '{"mode": "${OPENCODE_TEST_DEFAULT_OVERRIDE:-production}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result).toEqual({ mode: "staging" })
+    } finally {
+      delete process.env.OPENCODE_TEST_DEFAULT_OVERRIDE
+    }
+  })
+
+  test("default: ${VAR:-default} uses default when var is empty string", async () => {
+    // POSIX :- uses default for both unset AND empty (matches docker-compose)
+    process.env.OPENCODE_TEST_EMPTY_VAR = ""
+    try {
+      const text = '{"mode": "${OPENCODE_TEST_EMPTY_VAR:-fallback}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result).toEqual({ mode: "fallback" })
+    } finally {
+      delete process.env.OPENCODE_TEST_EMPTY_VAR
+    }
+  })
+
+  test("default: empty default ${VAR:-} resolves to empty string", async () => {
+    const text = '{"value": "${OPENCODE_TEST_EMPTY_DEFAULT:-}"}'
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result).toEqual({ value: "" })
+  })
+
+  test("default: default value with spaces and special chars", async () => {
+    const text = '{"msg": "${OPENCODE_TEST_MISSING:-Hello World 123}"}'
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result).toEqual({ msg: "Hello World 123" })
+  })
+
+  test("default: default value is JSON-escaped (security)", async () => {
+    const text = '{"token": "${OPENCODE_TEST_MISSING:-pwned\\", \\"isAdmin\\": true, \\"x\\": \\"y}"}'
+    const result = await ConfigPaths.parseText(text, "/fake/config.json")
+    expect(result.token).toContain("pwned")
+    expect(result.isAdmin).toBeUndefined()
+  })
+
+  test("escape hatch: $${VAR:-default} stays literal", async () => {
+    process.env.OPENCODE_TEST_ESCAPED_DEFAULT = "should-not-be-used"
+    try {
+      const text = '{"template": "$${OPENCODE_TEST_ESCAPED_DEFAULT:-my-default}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result).toEqual({ template: "${OPENCODE_TEST_ESCAPED_DEFAULT:-my-default}" })
+    } finally {
+      delete process.env.OPENCODE_TEST_ESCAPED_DEFAULT
+    }
+  })
+
+  test("escape hatch: $${VAR} stays literal (docker-compose convention)", async () => {
+    process.env.OPENCODE_TEST_SHOULD_NOT_SUB = "interpolated"
+    try {
+      const text = '{"template": "$${OPENCODE_TEST_SHOULD_NOT_SUB}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      // $${VAR} → literal ${VAR}, env value is NOT substituted
+      expect(result).toEqual({ template: "${OPENCODE_TEST_SHOULD_NOT_SUB}" })
+    } finally {
+      delete process.env.OPENCODE_TEST_SHOULD_NOT_SUB
+    }
+  })
+
+  test("single-pass: {env:A} value containing ${B} stays literal (no cascade)", async () => {
+    // Regression test for cubic/coderabbit P1: previously the {env:VAR} pass ran
+    // first, then the ${VAR} pass expanded any ${...} in its output. Single-pass
+    // substitution evaluates both patterns against the ORIGINAL text only.
+    process.env.OPENCODE_TEST_CASCADE_A = "${OPENCODE_TEST_CASCADE_B}"
+    process.env.OPENCODE_TEST_CASCADE_B = "should-not-expand"
+    try {
+      const text = '{"value": "{env:OPENCODE_TEST_CASCADE_A}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      // {env:VAR} is raw injection — its output is NOT re-interpolated
+      expect(result.value).toBe("${OPENCODE_TEST_CASCADE_B}")
+    } finally {
+      delete process.env.OPENCODE_TEST_CASCADE_A
+      delete process.env.OPENCODE_TEST_CASCADE_B
+    }
+  })
+
+  test("single-pass: ${A} value containing {env:B} stays literal (no cascade)", async () => {
+    // Reverse direction: ${VAR} output must not be matched by {env:VAR} pass.
+    process.env.OPENCODE_TEST_CASCADE_C = "{env:OPENCODE_TEST_CASCADE_D}"
+    process.env.OPENCODE_TEST_CASCADE_D = "should-not-expand"
+    try {
+      const text = '{"value": "${OPENCODE_TEST_CASCADE_C}"}'
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result.value).toBe("{env:OPENCODE_TEST_CASCADE_D}")
+    } finally {
+      delete process.env.OPENCODE_TEST_CASCADE_C
+      delete process.env.OPENCODE_TEST_CASCADE_D
+    }
+  })
+
+  test("works inside MCP environment config (issue #635 regression)", async () => {
+    process.env.OPENCODE_TEST_GITLAB_TOKEN = "glpat-xxxxx"
+    try {
+      const text = `{
+        "mcp": {
+          "gitlab": {
+            "type": "local",
+            "command": ["npx", "-y", "@modelcontextprotocol/server-gitlab"],
+            "environment": { "GITLAB_TOKEN": "\${OPENCODE_TEST_GITLAB_TOKEN}" }
+          }
+        }
+      }`
+      const result = await ConfigPaths.parseText(text, "/fake/config.json")
+      expect(result.mcp.gitlab.environment.GITLAB_TOKEN).toBe("glpat-xxxxx")
+    } finally {
+      delete process.env.OPENCODE_TEST_GITLAB_TOKEN
+    }
+  })
+})
+
 describe("ConfigPaths.parseText: {file:path} substitution", () => {
   test("substitutes {file:path} with file contents (trimmed)", async () => {
     await using tmp = await tmpdir()

packages/opencode/test/telemetry/telemetry.test.ts

@@ -246,11 +246,12 @@ const ALL_EVENT_TYPES: Telemetry.Event["type"][] = [
   "feature_suggestion",
   "core_failure",
   "sql_pre_validation",
+  "config_env_interpolation",
 ]
 
 describe("telemetry.event-types", () => {
   test("all event types are valid", () => {
-    expect(ALL_EVENT_TYPES.length).toBe(43)
+    expect(ALL_EVENT_TYPES.length).toBe(44)
   })
 })