[fix] translate bare ${VAR} env-var refs in self-defined MCP server headers (#944)

docs/src/content/docs/reference/manifest-schema.md

@@ -359,12 +359,12 @@ A plain registry reference: `io.github.github/github-mcp-server`
 |---|---|---|---|---|
 | `name` | `string` | REQUIRED | Non-empty | Server identifier (registry name or custom name). |
 | `transport` | `enum<string>` | Conditional | `stdio` · `sse` · `http` · `streamable-http` | Transport protocol. REQUIRED when `registry: false`. Values are MCP transport names, not URL schemes: remote variants connect over HTTPS. |
-| `env` | `map<string, string>` | OPTIONAL | | Environment variable overrides. Values may contain `${input:<id>}` references (VS Code only — see §4.2.4). |
+| `env` | `map<string, string>` | OPTIONAL | | Environment variable overrides. Values may contain `${VAR}`, `${env:VAR}`, or `${input:<id>}` references — see §4.2.4. |
 | `args` | `dict` or `list` | OPTIONAL | | Dict for overlay variable overrides (registry), list for positional args (self-defined). |
 | `version` | `string` | OPTIONAL | | Pin to a specific server version. |
 | `registry` | `bool` or `string` | OPTIONAL | Default: `true` (public registry) | `false` = self-defined (private) server. String = custom registry URL. |
 | `package` | `enum<string>` | OPTIONAL | `npm` · `pypi` · `oci` | Package manager type hint. |
-| `headers` | `map<string, string>` | OPTIONAL | | Custom HTTP headers for remote endpoints. Values may contain `${input:<id>}` references (VS Code only — see §4.2.4). |
+| `headers` | `map<string, string>` | OPTIONAL | | Custom HTTP headers for remote endpoints. Values may contain `${VAR}`, `${env:VAR}`, or `${input:<id>}` references — see §4.2.4. |
 | `tools` | `list<string>` | OPTIONAL | Default: `["*"]` | Restrict which tools are exposed. |
 | `url` | `string` | Conditional | | Endpoint URL. REQUIRED when `registry: false` and `transport` is `http`, `sse`, or `streamable-http`. |
 | `command` | `string` | Conditional | Single binary path; no embedded whitespace unless `args` is also present | Binary path. REQUIRED when `registry: false` and `transport` is `stdio`. |
@@ -400,13 +400,26 @@ dependencies:
         API_KEY: ${{ secrets.KEY }}
 ```
 
-#### 4.2.4. `${input:...}` Variables
+#### 4.2.4. Variable References in `headers` and `env`
 
-Values in `headers` and `env` may contain VS Code input variable references using the syntax `${input:<variable-id>}`. At runtime, VS Code prompts the user for each referenced input before starting the server.
+Values in `headers` and `env` may contain three placeholder syntaxes. APM resolves them per-target so secrets stay out of generated config files where possible.
 
-- **Registry-backed servers** — APM auto-generates input prompts from registry metadata.
+| 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** 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 at install time; `${VAR}` / `${env:VAR}` are passed through verbatim in the Codex adapter today.
+- **Recommended:** Use `${VAR}` or `${env:VAR}` in all new manifests — they work on every target that supports remote MCP servers. `<VAR>` is legacy and only resolved by Copilot CLI and Codex; in VS Code it would silently render as literal text in the generated config.
+- **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.
 
+GitHub Actions templates (`${{ ... }}`) are intentionally left untouched.
+
 ```yaml
 dependencies:
   mcp:
@@ -415,16 +428,11 @@ dependencies:
       transport: http
       url: https://my-server.example.com/mcp/
       headers:
-        Authorization: "Bearer ${input:my-server-token}"
-        X-Project: "${input:my-server-project}"
+        Authorization: "Bearer ${MY_SECRET_TOKEN}"      # bare env-var
+        X-Tenant: "${env:TENANT_ID}"                    # env-prefixed
+        X-Project: "${input:my-server-project}"         # VS Code input prompt
 ```
 
-| Runtime | `${input:...}` support |
-|---------|----------------------|
-| VS Code | Yes — prompts user at runtime |
-| Copilot CLI | No — use environment variables |
-| Codex | No — use environment variables |
-
 ---
 
 ## 5. devDependencies

packages/apm-guide/.apm/skills/apm-usage/dependencies.md

@@ -155,6 +155,13 @@ dependencies:
       package: npm                              # npm|pypi|oci
       headers:
         X-Custom: "value"
+        # Env-var placeholders in headers/env values:
+        #   ${VAR} or ${env:VAR}  -> resolved from host env at install time
+        #                            by Copilot (VS Code resolves at runtime;
+        #                            Codex passes ${...} through unchanged)
+        #   ${input:<id>}         -> VS Code prompts user at runtime
+        #   <VAR>                 -> legacy Copilot syntax (still supported)
+        Authorization: "Bearer ${MY_TOKEN}"
       tools: ["repos", "issues"]
 
     # Self-defined server (not in registry)

scripts/test-integration.sh

@@ -366,6 +366,17 @@ run_e2e_tests() {
         log_error "MCP registry tests failed!"
         exit 1
     fi
+
+    # Run MCP env-var headers E2E tests (regression guard for ${VAR} -> ${env:VAR})
+    log_info "Running MCP env-var headers E2E tests..."
+    echo "Command: pytest tests/integration/test_mcp_env_var_headers_e2e.py -v -s --tb=short"
+
+    if pytest tests/integration/test_mcp_env_var_headers_e2e.py -v -s --tb=short; then
+        log_success "MCP env-var headers tests passed!"
+    else
+        log_error "MCP env-var headers tests failed!"
+        exit 1
+    fi
 
     # Run APM Dependencies integration tests (NEW - Task 8A)
     log_info "Running APM Dependencies integration tests with real repositories..."

src/apm_cli/adapters/client/base.py

@@ -7,6 +7,13 @@
 
 _INPUT_VAR_RE = re.compile(r"\$\{input:([^}]+)\}")
 
+# Matches ${VAR} and ${env:VAR}, capturing VAR. Intentionally does NOT match
+# ${input:VAR} (the optional ``env:`` group cannot also satisfy ``input:``),
+# nor GitHub Actions ``${{ ... }}`` templates (the second ``{`` fails the
+# identifier class). This keeps env-var handling fully disjoint from input
+# variable handling, so existing _INPUT_VAR_RE call sites are unaffected.
+_ENV_VAR_RE = re.compile(r"\$\{(?:env:)?([A-Za-z_][A-Za-z0-9_]*)\}")
+
 
 class MCPClientAdapter(ABC):
     """Base adapter for MCP clients."""

src/apm_cli/adapters/client/copilot.py

@@ -7,14 +7,25 @@
 
 import json
 import os
+import re
 from pathlib import Path
 
 from ...core.docker_args import DockerArgsProcessor
 from ...core.token_manager import GitHubTokenManager
 from ...registry.client import SimpleRegistryClient
 from ...registry.integration import RegistryIntegration
 from ...utils.github_host import is_github_hostname
-from .base import MCPClientAdapter
+from .base import _ENV_VAR_RE, MCPClientAdapter
+
+# Combined env-var placeholder regex covering all three syntaxes Copilot accepts:
+#   <VARNAME>          legacy APM (group 1, uppercase only)
+#   ${VARNAME}         POSIX shell (group 2)
+#   ${env:VARNAME}     VS Code-flavored (group 2)
+# A single-pass substitution preserves the original ``<VAR>`` semantics:
+# resolved values are NOT re-scanned, so a token whose literal text contains
+# ``${...}`` does not get recursively expanded. Module-level compile avoids
+# per-call cost. ``${input:...}`` is intentionally not matched here.
+_COPILOT_ENV_RE = re.compile(r"<([A-Z_][A-Z0-9_]*)>|" + _ENV_VAR_RE.pattern)
 
 
 class CopilotClientAdapter(MCPClientAdapter):
@@ -461,8 +472,6 @@ def _resolve_env_variable(self, name, value, env_overrides=None):
         Returns:
             str: Resolved environment variable value.
         """
-        import os
-        import re
         import sys
 
         from rich.prompt import Prompt
@@ -480,28 +489,26 @@ def _resolve_env_variable(self, name, value, env_overrides=None):
         if not is_interactive:
             skip_prompting = True
 
-        # Check if value contains environment variable reference
-        env_pattern = r"<([A-Z_][A-Z0-9_]*)>"
-        matches = re.findall(env_pattern, value)
-
-        if matches:
-            for env_name in matches:
-                # First check overrides, then environment
-                env_value = env_overrides.get(env_name) or os.getenv(env_name)
-                if not env_value and not skip_prompting:
-                    # Only prompt if not in managed mode
-                    prompt_text = f"Enter value for {env_name}"
-                    env_value = Prompt.ask(
-                        prompt_text,
-                        password=True  # noqa: SIM210
-                        if "token" in env_name.lower() or "key" in env_name.lower()
-                        else False,
-                    )
-
-                if env_value:
-                    value = value.replace(f"<{env_name}>", env_value)
+        # Three accepted placeholder syntaxes (see _COPILOT_ENV_RE at module
+        # top), all resolved against env_overrides -> os.environ -> optional
+        # interactive prompt. Single-pass substitution preserves the legacy
+        # ``<VAR>`` semantics: resolved values are not re-scanned for further
+        # placeholder expansion.
+        def _replace(match):
+            # Group 1 = legacy <VAR>; group 2 = ${VAR} / ${env:VAR}.
+            env_name = match.group(1) or match.group(2)
+            env_value = env_overrides.get(env_name) or os.getenv(env_name)
+            if not env_value and not skip_prompting:
+                prompt_text = f"Enter value for {env_name}"
+                env_value = Prompt.ask(
+                    prompt_text,
+                    password=True  # noqa: SIM210
+                    if "token" in env_name.lower() or "key" in env_name.lower()
+                    else False,
+                )
+            return env_value if env_value else match.group(0)
 
-        return value
+        return _COPILOT_ENV_RE.sub(_replace, value)
 
     def _inject_env_vars_into_docker_args(self, docker_args, env_vars):
         """Inject environment variables into Docker arguments following registry template.

src/apm_cli/adapters/client/vscode.py

@@ -7,11 +7,18 @@
 
 import json
 import os  # noqa: F401
+import re
 from pathlib import Path
 
 from ...registry.client import SimpleRegistryClient
 from ...registry.integration import RegistryIntegration
-from .base import _INPUT_VAR_RE, MCPClientAdapter
+from ...utils.console import _rich_warning
+from .base import _ENV_VAR_RE, _INPUT_VAR_RE, MCPClientAdapter
+
+# Legacy ``<VAR>`` placeholder (Copilot CLI / Codex only). VS Code does not
+# resolve angle-bracket placeholders, so emitting them produces literal
+# ``<VAR>`` text in headers / env values -- silently breaking auth at runtime.
+_LEGACY_ANGLE_VAR_RE = re.compile(r"<([A-Z_][A-Z0-9_]*)>")
 
 
 class VSCodeClientAdapter(MCPClientAdapter):
@@ -242,9 +249,16 @@ def _format_server_config(self, server_info):
                 "args": raw["args"],
             }
             if raw.get("env"):
-                server_config["env"] = raw["env"]
+                # Translate bare ${VAR} -> ${env:VAR} so VS Code's runtime env
+                # interpolation resolves them at server-start. ${input:...}
+                # references are preserved for input-variable extraction below.
+                self._warn_on_legacy_angle_vars(
+                    raw["env"], server_info.get("name", "unknown"), "env"
+                )
+                env_translated = self._translate_env_vars_for_vscode(raw["env"])
+                server_config["env"] = env_translated
                 input_vars.extend(
-                    self._extract_input_variables(raw["env"], server_info.get("name", ""))
+                    self._extract_input_variables(env_translated, server_info.get("name", ""))
                 )
             return server_config, input_vars
 
@@ -361,6 +375,13 @@ def _format_server_config(self, server_info):
                         headers = {
                             h["name"]: h["value"] for h in headers if "name" in h and "value" in h
                         }
+                    # Translate bare ${VAR} -> ${env:VAR} so VS Code resolves
+                    # them from the host environment at runtime, instead of
+                    # sending the literal placeholder as the header value.
+                    self._warn_on_legacy_angle_vars(
+                        headers, server_info.get("name", "unknown"), "headers"
+                    )
+                    headers = self._translate_env_vars_for_vscode(headers)
                     server_config = {
                         "type": transport,
                         "url": remote["url"].strip(),
@@ -389,6 +410,56 @@ def _format_server_config(self, server_info):
 
         return server_config, input_vars
 
+    @staticmethod
+    def _translate_env_vars_for_vscode(mapping):
+        """Normalize ``${VAR}`` and ``${env:VAR}`` references to ``${env:VAR}``.
+
+        VS Code's mcp.json natively resolves ``${env:VAR}`` from the host
+        environment at server-start time. Bare ``${VAR}`` is *not* part of the
+        mcp.json grammar, so VS Code would otherwise pass the literal text
+        through (silently breaking auth headers, env vars, etc.).
+
+        This translation is purely textual and idempotent:
+        - ``${VAR}``      -> ``${env:VAR}``
+        - ``${env:VAR}``  -> ``${env:VAR}`` (no change)
+        - ``${input:X}``  -> ``${input:X}`` (no change; handled separately)
+        - non-string values pass through
+
+        A new dict is returned so callers may continue to use the original
+        for input-variable extraction without ordering concerns.
+        """
+        if not mapping:
+            return mapping
+        return {
+            k: (_ENV_VAR_RE.sub(r"${env:\1}", v) if isinstance(v, str) else v)
+            for k, v in mapping.items()
+        }
+
+    @staticmethod
+    def _warn_on_legacy_angle_vars(mapping, server_name, field):
+        """Emit a warning when legacy ``<VAR>`` placeholders appear in *mapping*.
+
+        VS Code does not resolve ``<VAR>`` placeholders, so they would render
+        as literal ``<VAR>`` text in the generated mcp.json -- silently
+        breaking auth headers / env values at server-start. Surface this as
+        an explicit warning so authors can switch to the cross-harness
+        ``${VAR}`` / ``${env:VAR}`` syntax (see manifest-schema reference).
+        """
+        if not mapping:
+            return
+        offenders = []
+        for value in mapping.values():
+            if isinstance(value, str):
+                offenders.extend(_LEGACY_ANGLE_VAR_RE.findall(value))
+        if offenders:
+            unique = sorted(set(offenders))
+            _rich_warning(
+                f"Server '{server_name}' {field} use legacy <VAR> placeholder(s) "
+                f"({', '.join('<' + n + '>' for n in unique)}) which VS Code "
+                f"cannot resolve. Use ${{VAR}} or ${{env:VAR}} instead so the "
+                f"value resolves at runtime."
+            )
+
     def _extract_input_variables(self, mapping, server_name):
         """Scan dict values for ${input:...} references and return input variable definitions.