fix: ${VAR} env-var interpolation for configs (closes #635)#655
Merged
anandgupta42 merged 5 commits intomainfrom Apr 5, 2026
Merged
fix: ${VAR} env-var interpolation for configs (closes #635)#655anandgupta42 merged 5 commits intomainfrom
anandgupta42 merged 5 commits intomainfrom
Conversation
Config substitution previously only accepted {env:VAR}. Users arriving
from Claude Code, VS Code, dotenv, or docker-compose naturally write
${VAR} and hit silent failures — the literal string passes through to
MCP servers as the env value, shadowing the forwarded parent env.
This adds ${VAR} as an alias for {env:VAR}. Regex matches POSIX
identifier names only ([A-Za-z_][A-Za-z0-9_]*) to avoid collisions
with random ${...} content in URLs or paths. Bare $VAR (without
braces) is intentionally NOT interpolated — too collision-prone.
- paths.ts: add second regex replace after the existing {env:VAR} pass
- paths-parsetext.test.ts: 6 new tests covering shell syntax, mixed
use, invalid identifier names, bare $VAR rejection, and MCP env
regression scenario
- mcp-servers.md: document both syntaxes with a table
Closes #635
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as ConfigPaths.substitute()
participant Parser as Pattern Matcher
participant Env as process.env
participant Telemetry as telemetry module
participant Output as Result (JSONC parse)
Caller->>Parser: Scan input for patterns (${...}, $${...}, {env:...}, {file:...})
alt Found $${VAR}
Parser->>Caller: Unescape to literal `${VAR}` (no env lookup)
else Found ${VAR} or ${VAR:-default}
Parser->>Env: Lookup VAR
Env-->>Parser: value or undefined
Parser->>Parser: Apply default if `${...:-default}` and JSON-escape value
Parser->>Telemetry: dynamic import emit counts (dollar_refs / defaulted / unresolved / escaped)
Parser->>Caller: Inject JSON-escaped string value
else Found {env:VAR}
Parser->>Env: Lookup VAR
Env-->>Parser: value or undefined
Parser->>Telemetry: dynamic import emit counts (legacy_brace_refs / unresolved)
Parser->>Caller: Inject raw text (or "")
end
Caller->>Output: Continue `{file:...}` substitutions and JSONC parse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/config/paths.ts">
<violation number="1" location="packages/opencode/src/config/paths.ts:96">
P1: Double-substitution: the `${VAR}` regex runs on the output of the `{env:VAR}` pass, so an env value containing a literal `${X}` will have `X` resolved as a second variable. Combine both patterns into a single-pass replacement to avoid this injection vector.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Applies fixes from multi-model consensus review (Claude + GPT 5.4 + Gemini 3.1 Pro).
- C1 (CRITICAL — JSON injection): ${VAR} substitution is now JSON-safe via
JSON.stringify(value).slice(1, -1). Env values with quotes/commas/newlines
can no longer break out of the enclosing JSON string. Existing {env:VAR}
keeps raw-injection semantics for backward compat (documented as the
opt-in power-user syntax).
- M2 (MAJOR — escape hatch): $${VAR} now preserves a literal ${VAR} in
the output (docker-compose convention). Negative lookbehind in the match
regex prevents substitution when preceded by $.
- m3 (MINOR — documentation): docs expanded with a 3-row syntax comparison
table explaining string-safe vs raw-injection modes.
- m4 (MINOR — tests): added 3 edge-case tests covering JSON injection
attempt, multiline/backslash values, and the new $${VAR} escape hatch.
- n5 (NIT — stale tip): TUI tip at tips.tsx:150 now mentions both
${VAR} and {env:VAR} syntaxes.
Deferred (larger refactor, scope discipline):
- M1 (comment handling): substitutions still run inside // comments. Same
pre-existing behavior for {env:VAR}. Would require unified substitution
pass. Can be a follow-up PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/config/paths.ts`:
- Around line 92-104: Reorder the interpolation passes so the POSIX-style ${VAR}
alias runs before the {env:VAR} injection to preserve raw semantics: move the
block using text.replace(/(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g, ...) and the
unescape /\$\$(\{[A-Za-z_][A-Za-z0-9_]*\})/g, "$$$1" to execute prior to the
{env:...} replacement, keeping the same code and variable names (text, varName,
process.env). Also add a regression test that sets process.env.A = '${B}', calls
the code path that processes "{env:A}", and asserts the resulting string
contains the literal "${B}" (i.e., no second-pass expansion).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f4d1a170-7778-47eb-b366-217cf35d5ae0
📒 Files selected for processing (4)
docs/docs/configure/mcp-servers.mdpackages/opencode/src/cli/cmd/tui/component/tips.tsxpackages/opencode/src/config/paths.tspackages/opencode/test/config/paths-parsetext.test.ts
Comment on lines
+92
to
+104
| // altimate_change start — accept ${VAR} shell/dotenv syntax as alias for {env:VAR} | ||
| // Users arriving from Claude Code / VS Code / dotenv / docker-compose expect this | ||
| // convention. Only matches POSIX identifier names to avoid collisions with random | ||
| // ${...} content. Value is JSON-escaped so it can't break out of the enclosing | ||
| // string — use {env:VAR} for raw unquoted injection. Docker-compose convention: | ||
| // $${VAR} escapes to literal ${VAR}. See issue #635. | ||
| text = text.replace(/(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g, (_, varName) => { | ||
| const value = process.env[varName] || "" | ||
| return JSON.stringify(value).slice(1, -1) | ||
| }) | ||
| // Unescape: $${VAR} → ${VAR} (user-authored literal preservation, docker-compose style) | ||
| text = text.replace(/\$\$(\{[A-Za-z_][A-Za-z0-9_]*\})/g, "$$$1") | ||
| // altimate_change end |
There was a problem hiding this comment.
Preserve {env:VAR} raw semantics by reordering interpolation passes.
Right now, ${VAR} replacement runs after {env:VAR} replacement, so any ${...} that appears inside an env value injected by {env:...} is expanded in a second pass. That changes backward-compatible raw behavior unexpectedly.
Suggested fix
- text = text.replace(/\{env:([^}]+)\}/g, (_, varName) => {
- return process.env[varName] || ""
- })
- // altimate_change start — accept ${VAR} shell/dotenv syntax as alias for {env:VAR}
+ // altimate_change start — accept ${VAR} shell/dotenv syntax as alias for {env:VAR}
// Users arriving from Claude Code / VS Code / dotenv / docker-compose expect this
// convention. Only matches POSIX identifier names to avoid collisions with random
// ${...} content. Value is JSON-escaped so it can't break out of the enclosing
// string — use {env:VAR} for raw unquoted injection. Docker-compose convention:
// $${VAR} escapes to literal ${VAR}. See issue `#635`.
text = text.replace(/(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g, (_, varName) => {
const value = process.env[varName] || ""
return JSON.stringify(value).slice(1, -1)
})
// Unescape: $${VAR} → ${VAR} (user-authored literal preservation, docker-compose style)
text = text.replace(/\$\$(\{[A-Za-z_][A-Za-z0-9_]*\})/g, "$$$1")
+ // Keep legacy raw semantics for {env:VAR} without cascading ${...} expansion
+ text = text.replace(/\{env:([^}]+)\}/g, (_, varName) => {
+ return process.env[varName] || ""
+ })
// altimate_change endAlso add a regression test where {env:A} resolves to a value containing ${B} and remains literal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/config/paths.ts` around lines 92 - 104, Reorder the
interpolation passes so the POSIX-style ${VAR} alias runs before the {env:VAR}
injection to preserve raw semantics: move the block using
text.replace(/(?<!\$)\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g, ...) and the unescape
/\$\$(\{[A-Za-z_][A-Za-z0-9_]*\})/g, "$$$1" to execute prior to the {env:...}
replacement, keeping the same code and variable names (text, varName,
process.env). Also add a regression test that sets process.env.A = '${B}', calls
the code path that processes "{env:A}", and asserts the resulting string
contains the literal "${B}" (i.e., no second-pass expansion).
Extends the ${VAR} substitution to support POSIX/docker-compose-style
defaults. Matches user expectations from dotenv, docker-compose, and
shell: default value is used when the variable is unset OR empty
(matching :- semantics rather than bare -).
- ${VAR:-default} uses 'default' when VAR is unset or empty string
- ${VAR:-} (empty default) resolves to empty string
- Defaults with spaces/special chars supported (${VAR:-Hello World})
- Default values are JSON-escaped (same security properties as ${VAR})
- $${VAR:-default} escape hatch works for literal preservation
Per research, ${VAR:-default} is the de facto standard across
docker-compose, dotenv, POSIX shell, GitHub Actions, and Terraform —
users arrive from these tools expecting this syntax.
Added 7 tests covering unset/set/empty cases, empty default, spaces,
JSON-injection attempt, and escape hatch.
Collect signals on env-var interpolation usage to detect footguns and
guide future improvements. Tracks per config-load:
- dollar_refs: count of ${VAR} / ${VAR:-default} references
- dollar_unresolved: ${VAR} with no value and no default → empty string
(signal: users writing fragile configs without defaults)
- dollar_defaulted: ${VAR:-default} where the fallback was used
(signal: whether defaults syntax is actually being used)
- dollar_escaped: $${VAR} literal escapes used
- legacy_brace_refs: {env:VAR} references (raw-injection syntax)
- legacy_brace_unresolved: {env:VAR} with no value
Emitted via dynamic import to avoid circular dep with @/altimate/telemetry
(which imports @/config/config). Event fires only when interpolation
actually happens, so no-env-ref configs don't generate noise.
What this lets us answer after shipping:
- How many users hit 'unresolved' unintentionally? → consider failing loud
- Is ${VAR:-default} getting adopted? → iterate on docs if not
- Are users still writing the legacy {env:VAR}? → plan deprecation
- Is the $${VAR} escape rare? → simplify docs if so
Adds event type to ALL_EVENT_TYPES completeness list (43 → 44).
anandgupta42
changed the title
Both bots flagged a backward-compat regression: ${VAR} pass runs after
{env:VAR} pass, so an env value containing literal ${X} got expanded in
the second pass. Reordering only fixed one direction (reverse cascade
still possible when ${VAR}'s output contains {env:Y}).
Correct fix: single-pass substitution with one regex alternation that
evaluates all three patterns against the ORIGINAL text. Output of any
pattern cannot be re-matched by another.
Single regex handles:
$${VAR} or $${VAR:-default} → literal escape
(?<!$)${VAR}[:-default] → JSON-safe substitution
{env:VAR} → raw text injection
Regression tests added for both cascade directions:
- env.A="${B}" + {env:A} → literal ${B} stays
- env.A="{env:B}" + ${A} → literal {env:B} stays
All 34 tests pass. Typecheck + marker guard clean.
Contributor
Author
|
@cubic-dev-ai Already addressed in commit Regex: Added 2 regression tests covering both cascade directions:
Both pass. |
@anandgupta42 That's a solid fix for the double-substitution issue. The single-pass approach with the unified regex alternation correctly prevents cascading interpolations where an environment variable's value could inadvertently trigger a second replacement pass. I've verified the implementation and the two new regression tests in
The negative lookbehind I also like that you maintained the security properties we discussed:
The telemetry tracking inside the replacement callback is also well-handled, ensuring we still get signals on syntax adoption despite the unified pass. For more details on how we track these patterns, you can check the Learnings page. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #635 by adding shell/dotenv-style
${VAR}interpolation to config parsing. Users arriving from Claude Code, VS Code, dotenv, or docker-compose naturally write${VAR}and previously hit silent failures — the literal string passed through to MCP servers, shadowing the forwarded parent env.Syntax supported (after full PR):
${VAR}"API_KEY": "${MY_API_KEY}"${VAR:-default}"MODE": "${APP_MODE:-production}"{env:VAR}"count": {env:NUM}$${VAR}${VAR}"template": "$${VAR}"Bare
$VAR(without braces) is intentionally NOT interpolated — too collision-prone with prices, JSON pointers, etc.Commits:
${VAR}support with POSIX-identifier regex$${VAR}escape hatch, stale TUI tip${VAR:-default}fallback syntax (POSIX / docker-compose convention)config_env_interpolationtelemetry event to measure adoption and footgunsType of change
${VAR:-default}, telemetry)Issue for this PR
Closes #635
Research
Surveyed 10 other MCP clients + docker-compose + dotenv. Full findings in PR comments. Key takeaways:
${VAR}+${VAR:-default}is the de facto standard (POSIX shell, docker-compose, dotenv, GitHub Actions, Terraform, Claude Code, Cursor)$${VAR}— matched here.Security
${VAR}values are JSON-escaped viaJSON.stringify(value).slice(1, -1). Env values containing quotes, commas, braces, or newlines cannot break out of the enclosing JSON string. Tested with injection-attempt values. Existing{env:VAR}keeps raw-injection semantics for backward compat (documented).Telemetry
New event
config_env_interpolationemitted per config load (only when interpolation actually runs). Fields:dollar_refs,dollar_unresolved,dollar_defaulted,dollar_escaped(new syntax)legacy_brace_refs,legacy_brace_unresolved(old syntax)Lets us measure adoption of
${VAR:-default}defaults, detect users writing fragile configs without defaults, and plan future deprecation of{env:VAR}.How did you verify your code works?
test/config/paths-parsetext.test.ts:${VAR}happy path + unset → empty string${VAR}and{env:VAR}in same config${foo bar},${foo-bar},${foo.bar})$VARNOT interpolated$${VAR}escape hatch preserves literal${VAR:-default}unset/set/empty/empty-default/special-chars cases$${VAR:-default}escape hatch with defaultsALL_EVENT_TYPEScompleteness list updated (43 → 44)Checklist
altimate_changemarkers (diverges from upstream){env:VAR}syntax untouched (backward compat)mcp-servers.md+ TUI tip)Summary by CodeRabbit
New Features
${VAR},${VAR:-default},{env:VAR}, and$${VAR}escape sequences.Documentation
Updates