[fix] translate bare ${VAR} env-var refs in self-defined MCP server headers (#944)#947
Merged
danielmeppiel merged 7 commits intomicrosoft:mainfrom Apr 30, 2026
Conversation
edenfunf
requested review from
danielmeppiel and
sergio-sisternes-epam
as code owners
…eaders
Self-defined MCP servers using bare ${VARNAME} or ${env:VARNAME} in headers had
those references written verbatim into .vscode/mcp.json and ~/.copilot/mcp-config.json,
so the literal placeholder string was sent as the header value at runtime.
VS Code mcp.json natively resolves ${env:VAR} and ${input:VAR} but not bare ${VAR},
so the VSCode adapter now translates ${VAR} -> ${env:VAR} before writing. Copilot
mcp-config.json has no native interpolation, so its existing _resolve_env_variable
method (previously matching only legacy <VAR> syntax) now also recognizes ${VAR}
and ${env:VAR}, with single-pass substitution preserving the original "resolve
exactly once" semantics. Gemini inherits the same fix via CopilotClientAdapter.
Fixes microsoft#944
Documents the new bare ${VAR} and ${env:VAR} placeholder syntaxes that
this PR enables for self-defined MCP server headers and env values. Adds:
- docs/src/content/docs/reference/manifest-schema.md: section 4.2.4
rewritten to cover all three placeholder syntaxes (${VAR},
${env:VAR}, ${input:<id>}) plus the legacy <VAR> form, with a
per-target resolution matrix (VS Code vs Copilot CLI vs Codex).
Field rows for `env` and `headers` updated to reference the new
syntaxes.
- packages/apm-guide/.apm/skills/apm-usage/dependencies.md: example
block annotated with the three placeholder syntaxes per the
doc-sync rule for primitive formats.
No source changes. Unit suite (6790 passed) and MCP integration tests
remain green.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pins the full install pipeline boundary: apm.yml self-defined HTTP MCP
server with both bare ${VAR} and explicit ${env:VAR} headers ->
apm install --target vscode -> .vscode/mcp.json on disk. Asserts both
syntaxes land as ${env:VAR} (the syntax VS Code resolves at server
start) and that no host env values leak into the file.
Complements unit coverage in tests/unit/test_vscode_adapter.py with one
concrete on-disk assertion so the fix can not regress when adapter
wiring changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
force-pushed
the
fix/mcp-bare-env-var-headers-944
branch
from
ddc30d4 to
eb06151
Compare
Collaborator
|
Wave 1 / FIX-NOW orchestrator update (2026-04-30T09:54Z) Following up on @xuyuanhao's rebase onto fresh main. The orchestrator has:
Validation evidence (local, on
CI workflow runs approved on this SHA. Approving for merge once CI confirms green. |
danielmeppiel
approved these changes
Apr 30, 2026
Collaborator
danielmeppiel
left a comment
danielmeppiel
left a comment
There was a problem hiding this comment.
Approving after orchestrator-side rebase + integration test addition. Logic preserved; full unit suite green on the rebased HEAD.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes env-var placeholder handling for self-defined MCP server headers (and VS Code stdio env) so generated client config files either emit VS Code-compatible ${env:VAR} placeholders or resolve secrets at install time for clients without runtime interpolation.
Changes:
- Add shared
${VAR}/${env:VAR}detection (_ENV_VAR_RE) and apply translation in VS Code adapter. - Extend Copilot adapter env-var resolution to also recognize
${VAR}/${env:VAR}(in addition to legacy<VAR>). - Add/extend unit tests, add an integration regression test, and update docs/guides to document the placeholder syntax.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/adapters/client/base.py |
Introduces shared _ENV_VAR_RE for ${VAR} / ${env:VAR} parsing. |
src/apm_cli/adapters/client/vscode.py |
Translates bare ${VAR} to ${env:VAR} for VS Code-generated configs (headers and stdio env). |
src/apm_cli/adapters/client/copilot.py |
Resolves ${VAR} / ${env:VAR} placeholders (plus legacy <VAR>) during install-time substitution. |
tests/unit/test_vscode_adapter.py |
Adds unit coverage for VS Code translation/idempotency and preservation of ${input:...}. |
tests/unit/test_copilot_adapter.py |
Adds unit coverage for Copilot resolution across all three syntaxes and non-recursive semantics. |
tests/integration/test_mcp_env_var_headers_e2e.py |
Adds an end-to-end regression test asserting .vscode/mcp.json output behavior. |
packages/apm-guide/.apm/skills/apm-usage/dependencies.md |
Documents env-var placeholder syntax in MCP headers/env values. |
docs/src/content/docs/reference/manifest-schema.md |
Updates manifest schema docs to describe ${VAR} / ${env:VAR} / ${input:...} behavior per target. |
Copilot's findings
Comments suppressed due to low confidence (3)
src/apm_cli/adapters/client/vscode.py:389
- The docstring says "A new dict is returned", but for falsy inputs (e.g. empty dict) the function returns the original object unchanged (
if not mapping: return mapping). Either update the docstring to reflect the actual behavior, or always return a new dict for dict inputs (including empty dict) for consistency.
self._infer_registry_name(p) or p.get("name", "unknown") for p in packages
]
raise ValueError(
f"No supported transport for VS Code runtime. "
f"Server '{server_info.get('name', 'unknown')}' provides stdio packages "
tests/integration/test_mcp_env_var_headers_e2e.py:3
- The module docstring hard-codes a PR number ("PR #947"). PR numbers are not stable documentation and can be confusing out of context; consider referencing only the user-facing issue (#944) or a durable spec link instead.
"""End-to-end regression guard for #944 / PR #947: bare ${VAR} env-var
references in self-defined MCP server headers must reach VS Code's mcp.json
as the runtime-resolvable ${env:VAR} placeholder (NOT a literal ${VAR}
tests/integration/test_mcp_env_var_headers_e2e.py:76
- This inline comment references "PR #947". Consider removing the PR number and instead referencing issue #944 or the documented VS Code placeholder contract, so the comment stays accurate long-term.
"headers": {
# Two syntaxes per PR #947's stated VS Code contract
"Authorization": "Bearer ${MY_BEARER_TOKEN}",
"X-Api-Key": "${env:MY_API_KEY}",
- Files reviewed: 8/8 changed files
- Comments generated: 3
Comment on lines
+407
to
417
| | Syntax | Source | VS Code | Copilot CLI / Codex | | ||
| |---|---|---|---| | ||
| | `${VAR}` | host environment | Translated to `${env:VAR}` (resolved at server-start by VS Code) | Resolved at install time from env (or interactive prompt) | | ||
| | `${env:VAR}` | host environment | Native — passed through verbatim | Resolved at install time from env (or interactive prompt) | | ||
| | `${input:<id>}` | user prompt | Native — VS Code prompts at runtime | Not supported — use `${VAR}` or `${env:VAR}` instead | | ||
| | `<VAR>` (legacy) | host environment | Not recognized | Resolved at install time (kept for back-compat) | | ||
|
|
||
| - **VS Code** has native `${env:VAR}` and `${input:VAR}` interpolation, so APM emits placeholders rather than baking secrets into `mcp.json`. Bare `${VAR}` is normalized to `${env:VAR}` for you. | ||
| - **Copilot CLI** and **Codex** have no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `<VAR>` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. | ||
| - **Registry-backed servers** — APM auto-generates input prompts from registry metadata for `${input:...}`. | ||
| - **Self-defined servers** — APM detects `${input:...}` patterns in `apm.yml` and generates matching input definitions automatically. |
There was a problem hiding this comment.
This section states that "Copilot CLI / Codex" resolve ${VAR} and ${env:VAR} at install time. In the current code, CodexClientAdapter only handles the legacy <VAR> placeholder (and does not handle remote headers at all), so ${VAR} / ${env:VAR} are not actually supported/resolved for Codex. Please either adjust the docs to match current Codex behavior, or extend the Codex adapter to resolve the new ${...} syntaxes as described here.
Suggested change
| | Syntax | Source | VS Code | Copilot CLI / Codex | | |
| |---|---|---|---| | |
| | `${VAR}` | host environment | Translated to `${env:VAR}` (resolved at server-start by VS Code) | Resolved at install time from env (or interactive prompt) | | |
| | `${env:VAR}` | host environment | Native — passed through verbatim | Resolved at install time from env (or interactive prompt) | | |
| | `${input:<id>}` | user prompt | Native — VS Code prompts at runtime | Not supported — use `${VAR}` or `${env:VAR}` instead | | |
| | `<VAR>` (legacy) | host environment | Not recognized | Resolved at install time (kept for back-compat) | | |
| - **VS Code** has native `${env:VAR}` and `${input:VAR}` interpolation, so APM emits placeholders rather than baking secrets into `mcp.json`. Bare `${VAR}` is normalized to `${env:VAR}` for you. | |
| - **Copilot CLI** and **Codex** have no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `<VAR>` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. | |
| - **Registry-backed servers** — APM auto-generates input prompts from registry metadata for `${input:...}`. | |
| - **Self-defined servers** — APM detects `${input:...}` patterns in `apm.yml` and generates matching input definitions automatically. | |
| | Syntax | Source | VS Code | Copilot CLI | | |
| |---|---|---|---| | |
| | `${VAR}` | host environment | Translated to `${env:VAR}` (resolved at server-start by VS Code) | Resolved at install time from env (or interactive prompt) | | |
| | `${env:VAR}` | host environment | Native - passed through verbatim | Resolved at install time from env (or interactive prompt) | | |
| | `${input:<id>}` | user prompt | Native - VS Code prompts at runtime | Not supported - use `${VAR}` or `${env:VAR}` instead | | |
| | `<VAR>` (legacy) | host environment | Not recognized | Resolved at install time (kept for back-compat) | | |
| Current Codex behavior is more limited: Codex currently supports the legacy `<VAR>` placeholder only, and does not currently resolve `${VAR}` or `${env:VAR}`. Codex also does not currently resolve remote `headers`. | |
| - **VS Code** has native `${env:VAR}` and `${input:VAR}` interpolation, so APM emits placeholders rather than baking secrets into `mcp.json`. Bare `${VAR}` is normalized to `${env:VAR}` for you. | |
| - **Copilot CLI** has no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `<VAR>` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. | |
| - **Codex** currently resolves only the legacy `<VAR>` placeholder where supported. `${VAR}` and `${env:VAR}` are not currently resolved for Codex, and remote `headers` are not currently interpolated. | |
| - **Registry-backed servers** - APM auto-generates input prompts from registry metadata for `${input:...}`. | |
| - **Self-defined servers** - APM detects `${input:...}` patterns in `apm.yml` and generates matching input definitions automatically. |
Comment on lines
+158
to
+163
| # Env-var placeholders in headers/env values: | ||
| # ${VAR} or ${env:VAR} -> resolved from host env (Copilot/Codex bake | ||
| # in at install; VS Code resolves at runtime) | ||
| # ${input:<id>} -> VS Code prompts user at runtime | ||
| # <VAR> -> legacy Copilot syntax (still supported) | ||
| Authorization: "Bearer ${MY_TOKEN}" |
There was a problem hiding this comment.
This guide claims ${VAR} / ${env:VAR} placeholders are "resolved from host env" for "Copilot/Codex". CodexClientAdapter does not currently resolve these ${...} syntaxes (it only supports legacy <VAR> in certain paths), so this statement is inaccurate. Please update the wording to reflect actual Codex behavior (or implement ${...} resolution in Codex to match the guide).
Suggested change
| # Env-var placeholders in headers/env values: | |
| # ${VAR} or ${env:VAR} -> resolved from host env (Copilot/Codex bake | |
| # in at install; VS Code resolves at runtime) | |
| # ${input:<id>} -> VS Code prompts user at runtime | |
| # <VAR> -> legacy Copilot syntax (still supported) | |
| Authorization: "Bearer ${MY_TOKEN}" | |
| # Placeholder support in headers/env values varies by client: | |
| # ${VAR} or ${env:VAR} -> VS Code-style placeholders | |
| # ${input:<id>} -> VS Code prompts user at runtime | |
| # <VAR> -> legacy Copilot/Codex env placeholder | |
| # Use <VAR> for Codex; do not assume ${...} is resolved there. | |
| Authorization: "Bearer <MY_TOKEN>" |
Comment on lines
+1
to
+12
| """End-to-end regression guard for #944 / PR #947: bare ${VAR} env-var | ||
| references in self-defined MCP server headers must reach VS Code's mcp.json | ||
| as the runtime-resolvable ${env:VAR} placeholder (NOT a literal ${VAR} | ||
| that VS Code would treat as opaque text). | ||
|
|
||
| This exercises the full pipeline: | ||
| apm.yml -> apm install --target vscode -> .vscode/mcp.json on disk | ||
|
|
||
| The unit tests in tests/unit/test_vscode_adapter.py cover all three syntaxes | ||
| in isolation; this test pins the integration boundary so the fix doesn't | ||
| regress when adapter wiring changes. | ||
| """ |
There was a problem hiding this comment.
This test is described as an end-to-end regression guard, but it is not currently invoked by CI's integration test runner (scripts/test-integration.sh enumerates specific files and does not include this one). To make this guard effective, add it to the integration script (or otherwise ensure it runs in CI).
This issue also appears in the following locations of the same file:
- line 1
- line 73
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
approved these changes
Apr 30, 2026
Collaborator
danielmeppiel
left a comment
danielmeppiel
left a comment
There was a problem hiding this comment.
Re-approving on b442256: ruff format fix on integration test (4 files reformatted, 9 insertions / 19 deletions). All 6 checks green. NOTE: branch protection requires a second non-pusher approver; flagging for board action.
- Clarify that Codex resolves only legacy <VAR>, not ${VAR}/${env:VAR}
(manifest-schema.md, dependencies.md)
- Wire test_mcp_env_var_headers_e2e.py into scripts/test-integration.sh
so CI exercises the regression guard
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
Addressed Copilot review on
Lint / format pass green locally. |
Folds the devx-ux-expert audit recommendation into PR microsoft#947 (no follow-up PR sprawl per board policy): 1. Reference doc (manifest-schema.md): explicit recommendation to use ${VAR} or ${env:VAR} in new manifests; <VAR> labelled legacy (Copilot CLI / Codex only) with note that VS Code would render it as literal text. 2. VS Code adapter: emit a warning at install time when self-defined server headers or env contain legacy <VAR> placeholders, naming the server, the field (headers/env), and the offending vars. Turns the prior silent failure mode into an observable one. 5 new unit tests cover the warning helper (legacy var detected, multiple unique vars deduped, modern syntax silent, empty/None mapping silent, non-string values silent). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
Folded devx-ux-expert audit verdict (SHIP-WITH-FIX) into Audit summary:
Coverage: 5 new unit tests in test_vscode_adapter.py covering detected/deduped/modern-syntax-silent/empty/non-string. All 70 vscode tests + lint/format green. |
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
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
Self-defined MCP servers (
registry: false) that use bare${VARNAME}or${env:VARNAME}inheaderswere written verbatim into target config files, causing the literal placeholder string to be sent as the header value at runtime instead of the resolved secret.Before:
After:
Why
Per issue #944, the user-facing apm.yml syntax
${VARNAME}is a natural choice for env-var placeholders, but APM previously recognized only${input:<id>}(input variables) and the legacy<VARNAME>(Copilot install-time resolution). Bare${VARNAME}and${env:VARNAME}fell through both filters and were copied verbatim, producing a silent failure where MCP servers received the literal placeholder string as their auth token.The fix is target-specific because the config formats differ:
${env:VAR},${input:VAR}) -- APM's job is to emit a correctly-formatted placeholder, not to bake the secret into the file.<VAR>flow.How
Three minimal, additive changes that follow existing adapter patterns:
base.py-- add a shared_ENV_VAR_REregex (matches${VAR}and${env:VAR}, capturesVAR). Designed with negative lookahead-style ordering so it never matches${input:...}(kept disjoint from_INPUT_VAR_RE) and never matches GitHub Actions${{ ... }}templates.vscode.py-- add a static_translate_env_vars_for_vscodehelper (mirrors the existing_extract_input_variablesstatic-helper style) and call it before writing both remoteheadersand self-defined stdioenv. Translation is purely textual and idempotent, so re-runningapm installis safe.copilot.py-- module-level_COPILOT_ENV_RE(alternation of legacy<VAR>and the new${...}patterns) drives a single-passre.subin_resolve_env_variable. The existing env_overrides -> os.environ -> interactive-prompt resolution flow now applies uniformly to all three syntaxes. Single-pass substitution preserves the original<VAR>semantics: a resolved value containing literal${...}text is NOT recursively re-expanded. Gemini inherits this for free viaCopilotClientAdapter.Codex is unchanged (it does not handle remote servers, so headers do not apply).
Why these patterns are safe
_ENV_VAR_REis mathematically disjoint from_INPUT_VAR_RE: the optionalenv:group cannot also satisfyinput:.${{ ... }}(GitHub Actions) is not matched: the second{fails the identifier class.${env:VAR}is captured but the substitution rewrites it back to${env:VAR}, so repeat installs are stable.<VAR>legacy syntax,${input:...}input vars, and the existing input-variable warning paths are all untouched.Test
Unit tests
tests/unit/test_vscode_adapter.pyandtests/unit/test_copilot_adapter.pyadd 13 new tests covering:${VAR}translation in headers and stdio env${env:VAR}and${input:...}round-trip preservation${{ ... }}left untouched<VAR>,${VAR},${env:VAR})${input:...}not resolved by Copilot path${OTHER}text is NOT recursively expanded (verified across all three syntaxes via subTest)Full unit suite: 5,943 passed (5 unrelated env-specific failures:
pythonvspython3PATH check, network-dependent github_downloader tests).Manual end-to-end verification
Built a sandbox
apm.ymlwith a self-defined HTTP MCP server using bare${VAR}headers, ranapm installagainst the local source tree, and inspected the generated.vscode/mcp.jsonand~/.copilot/mcp-config.json. Verified across 9 scenarios: original issue repro, mixed syntax (<VAR>+${VAR}+${env:VAR}+${input:...}+ GHA template + plain text), unresolvable env vars, idempotency (3 consecutive installs), self-defined stdio env, edge cases ($5.99,$HOME,${},${1},${MY-VAR}, JSON-quoted values), pre-existing mcp.json merge (top-level extras preserved), and multiple servers sharing env names.