fix: resolve env-var placeholders in Codex/Gemini self-defined stdio MCP (closes #1266)

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Gemini CLI: `apm install -g --mcp NAME` now correctly writes to `~/.gemini/settings.json` (user scope) and `apm install` from outside the target project writes to `<project_root>/.gemini/settings.json` instead of `cwd`. Previously `--global` had no effect on Gemini and project-scope writes silently landed in the wrong directory. The matching opt-in gate and cleanup paths in `MCPIntegrator` are aligned in the same change. (#1299)
+- Codex, Gemini, and Cursor adapters now resolve `${VAR}`, `${env:VAR}`, and legacy `<VAR>` environment placeholders in stdio server arguments, matching the behavior already present in the Copilot CLI adapter. (#1266)
 - `apm install --target claude` now preserves self-defined stdio MCP `env` values from `apm.yml` and writes non-string values such as `PORT: 3000` and `DEBUG: false` as MCP-compatible strings. (#1222)
 - Non-skill integrators (agent, instruction, prompt, command, hook script-copy) silently adopt byte-identical pre-existing files so a degraded `deployed_files=[]` lockfile no longer permanently blocks installs gated by `required-packages-deployed`. (#1313)
 - `apm audit` drift check now returns skip-with-info (`passed=True`) when the install cache is cold, instead of failing the audit; bare `apm audit` surfaces the skip reason on stderr so CI pipelines that have not yet run `apm install` are not incorrectly red-marked. (#1289)

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

@@ -442,19 +442,18 @@ dependencies:
 
 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.
 
-| Syntax | Source | VS Code | Copilot CLI / Codex |
+| Syntax | Source | VS Code | Copilot CLI / Codex / Gemini / Cursor |
 |---|---|---|---|
 | `${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.
-- **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.
-- **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.
+- **Copilot CLI, Codex, Gemini, and Cursor** 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.
+- **Recommended:** Use `${VAR}` or `${env:VAR}` in all new manifests — they work on every target that supports remote MCP servers. `<VAR>` is legacy; 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.
 

src/apm_cli/adapters/client/codex.py

@@ -84,6 +84,7 @@ def update_config(self, config_updates):
 
         with open(config_path, "w", encoding="utf-8") as f:
             toml.dump(current_config, f)
+        os.chmod(config_path, 0o600)
         _log.debug("Codex config written to %s", config_path)
         return True
 
@@ -206,6 +207,9 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
         Returns:
             dict: Formatted server configuration for Codex CLI.
         """
+        if runtime_vars is None:
+            runtime_vars = {}
+
         # Default configuration structure with registry ID for conflict detection
         config = {
             "command": "unknown",
@@ -214,14 +218,30 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
             "id": server_info.get("id", ""),  # Add registry UUID for conflict detection
         }
 
-        # Self-defined stdio deps carry raw command/args  -- use directly
+        # Self-defined stdio deps carry raw command/args. Route ``env`` and
+        # ``args`` through the resolver pipeline so all three placeholder
+        # syntaxes (``<VAR>``, ``${VAR}``, ``${env:VAR}``) are resolved at
+        # install time before being written to ~/.codex/config.toml.
+        # See issue #1266.
         raw = server_info.get("_raw_stdio")
         if raw:
             config["command"] = raw["command"]
-            config["args"] = [self.normalize_project_arg(arg) for arg in raw["args"]]
+            resolved_env_for_args: dict = {}
             if raw.get("env"):
-                config["env"] = raw["env"]
+                resolved_env_for_args = self._resolve_environment_variables(
+                    raw["env"], env_overrides=env_overrides
+                )
+                config["env"] = resolved_env_for_args
                 self._warn_input_variables(raw["env"], server_info.get("name", ""), "Codex CLI")
+
+            def _process_stdio_arg(arg):
+                if isinstance(arg, str):
+                    arg = self._resolve_variable_placeholders(
+                        arg, resolved_env_for_args, runtime_vars
+                    )
+                return self.normalize_project_arg(arg)
+
+            config["args"] = [_process_stdio_arg(arg) for arg in raw.get("args") or []]
             return config
 
         # Note: Remote servers (SSE type) are handled in configure_mcp_server and rejected early
@@ -461,48 +481,6 @@ def _process_environment_variables(self, env_vars, env_overrides=None):
 
         return resolved
 
-    def _resolve_variable_placeholders(self, value, resolved_env, runtime_vars):
-        """Resolve both environment and runtime variable placeholders in values.
-
-        Args:
-            value (str): Value that may contain placeholders like <TOKEN_NAME> or {runtime_var}
-            resolved_env (dict): Dictionary of resolved environment variables.
-            runtime_vars (dict): Dictionary of resolved runtime variables.
-
-        Returns:
-            str: Processed value with actual variable values.
-        """
-        import re
-
-        if not value:
-            return value
-
-        processed = str(value)
-
-        # Replace <TOKEN_NAME> with actual values from resolved_env (for Docker env vars)
-        env_pattern = r"<([A-Z_][A-Z0-9_]*)>"
-
-        def replace_env_var(match):
-            env_name = match.group(1)
-            return resolved_env.get(env_name, match.group(0))  # Return original if not found
-
-        processed = re.sub(env_pattern, replace_env_var, processed)
-
-        # Replace {runtime_var} with actual values from runtime_vars
-        runtime_pattern = r"\{([a-zA-Z_][a-zA-Z0-9_]*)\}"
-
-        def replace_runtime_var(match):
-            var_name = match.group(1)
-            return runtime_vars.get(var_name, match.group(0))  # Return original if not found
-
-        processed = re.sub(runtime_pattern, replace_runtime_var, processed)
-
-        return processed
-
-    def _resolve_env_placeholders(self, value, resolved_env):
-        """Legacy method for backward compatibility. Use _resolve_variable_placeholders instead."""
-        return self._resolve_variable_placeholders(value, resolved_env, {})
-
     def _ensure_docker_env_flags(self, base_args, env_vars):
         """Ensure all environment variables are represented as -e flags in Docker args.
 

src/apm_cli/adapters/client/copilot.py

@@ -7,7 +7,6 @@
 
 import json
 import os
-import re
 from pathlib import Path
 from typing import ClassVar
 
@@ -19,85 +18,15 @@
 from ...registry.integration import RegistryIntegration
 from ...utils.console import _rich_warning
 from ...utils.github_host import is_github_hostname
-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)
-
-# Detects the legacy ``<VAR>`` placeholder syntax. Used both for translation
-# and for emitting an aggregated deprecation warning, mirroring the analogous
-# pattern in ``vscode.py``.
-_LEGACY_ANGLE_VAR_RE = re.compile(r"<([A-Z_][A-Z0-9_]*)>")
-
-
-def _translate_env_placeholder(value):
-    """Pure-textual translation of env-var placeholders to Copilot CLI's
-    native runtime substitution syntax (``${VAR}``).
-
-    This is the security-critical helper for issue #1152: it MUST NOT read
-    ``os.environ`` and MUST NOT resolve placeholders to their literal values.
-    Copilot CLI resolves ``${VAR}`` from the host environment at server-start
-    time, so APM emits placeholders verbatim rather than baking secrets into
-    ``~/.copilot/mcp-config.json``.
-
-    Translations:
-        ``${env:VAR}``     -> ``${VAR}``     (strip ``env:`` prefix)
-        ``${VAR}``         -> ``${VAR}``     (no-op)
-        ``<VAR>``          -> ``${VAR}``     (legacy syntax migration)
-        ``${VAR:-default}``-> passthrough    (regex doesn't match)
-        ``$VAR`` (bare)    -> passthrough    (regex doesn't match)
-        ``${input:foo}``   -> passthrough    (regex doesn't match)
-        non-string         -> passthrough
-
-    The translation is idempotent: applying it twice produces the same
-    result as applying it once.
-    """
-    if not isinstance(value, str):
-        return value
-
-    def _to_brace(match):
-        # group(1) = legacy <VAR>; group(2) = ${VAR} / ${env:VAR}
-        var_name = match.group(1) or match.group(2)
-        return "${" + var_name + "}"
-
-    return _COPILOT_ENV_RE.sub(_to_brace, value)
-
-
-def _extract_legacy_angle_vars(value):
-    """Return the set of legacy ``<VAR>`` names present in *value*.
-
-    Used to aggregate deprecation warnings across all servers in a single
-    install run, so authors see one helpful list instead of one warning per
-    occurrence.
-    """
-    if not isinstance(value, str):
-        return set()
-    return set(_LEGACY_ANGLE_VAR_RE.findall(value))
-
-
-def _has_env_placeholder(value):
-    """True if *value* is a string containing any recognised env-var
-    placeholder syntax (``${VAR}``, ``${env:VAR}``, or legacy ``<VAR>``).
-    Used to distinguish placeholder-sourced env values (which translate)
-    from hardcoded literal defaults (which stay literal).
-    """
-    if not isinstance(value, str):
-        return False
-    return bool(_COPILOT_ENV_RE.search(value))
-
-
-def _stringify_env_literal(value):
-    """Return MCP env literal values in the manifest ``map<string, string>`` shape."""
-    if isinstance(value, bool):
-        return str(value).lower()
-    return str(value)
+from .base import (
+    _ENV_PLACEHOLDER_RE,
+    _ENV_VAR_RE,
+    MCPClientAdapter,
+    _extract_legacy_angle_vars,
+    _has_env_placeholder,
+    _stringify_env_literal,
+    _translate_env_placeholder,
+)
 
 
 class CopilotClientAdapter(MCPClientAdapter):
@@ -169,14 +98,6 @@ def __init__(
         super().__init__(project_root=project_root, user_scope=user_scope)
         self.registry_client = SimpleRegistryClient(registry_url)
         self.registry_integration = RegistryIntegration(registry_url)
-        # Per-server tracking of placeholder-sourced env-var keys, populated
-        # during ``_format_server_config`` and consumed by the post-install
-        # summary line. Keys: env-var names; never holds resolved values.
-        self._last_env_placeholder_keys = set()
-        # Per-server collection of legacy ``<VAR>`` offenders, populated by
-        # the resolution helpers and consumed by ``configure_mcp_server`` to
-        # feed the aggregated deprecation warning.
-        self._last_legacy_angle_vars = set()
 
     def get_config_path(self):
         """Get the path to the Copilot CLI MCP configuration file.
@@ -949,7 +870,7 @@ def _replace(match):
                 )
             return env_value if env_value else match.group(0)
 
-        return _COPILOT_ENV_RE.sub(_replace, value)
+        return _ENV_PLACEHOLDER_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.
@@ -1120,74 +1041,6 @@ def _process_arguments(self, arguments, resolved_env=None, runtime_vars=None):
 
         return processed
 
-    def _resolve_variable_placeholders(self, value, resolved_env, runtime_vars):
-        """Resolve runtime template variables and translate or resolve env-var
-        placeholders in argument strings.
-
-        Behaviour depends on ``self._supports_runtime_env_substitution``:
-
-        - True (Copilot CLI default): env-var placeholders (``<VAR>``,
-          ``${VAR}``, ``${env:VAR}``) are translated to ``${VAR}`` for
-          runtime substitution by Copilot CLI. APM template variables
-          (``{runtime_var}``) are still resolved at install time because
-          they are an APM-internal concept Copilot cannot interpret.
-
-        - False (legacy / sibling-adapter behaviour): legacy ``<VAR>``
-          placeholders are resolved against ``resolved_env`` (the dict of
-          literal env-var values), and ``{runtime_var}`` against
-          ``runtime_vars``. Newer ``${VAR}`` / ``${env:VAR}`` syntaxes are
-          left as-is for backward compatibility.
-
-        Args:
-            value (str): Value that may contain placeholders.
-            resolved_env (dict): Dictionary of resolved env vars (legacy
-                mode) or placeholder strings (translate mode).
-            runtime_vars (dict): Dictionary of resolved runtime variables.
-
-        Returns:
-            str: Processed value with placeholders translated or resolved.
-        """
-        import re
-
-        if not value:
-            return value
-
-        processed = str(value)
-
-        if self._supports_runtime_env_substitution:
-            # Track legacy <VAR> offenders before translating them away.
-            self._last_legacy_angle_vars.update(_extract_legacy_angle_vars(processed))
-            # Translate all three env-var placeholder syntaxes to ${VAR}.
-            processed = _translate_env_placeholder(processed)
-        else:
-            # Replace <TOKEN_NAME> with actual values from resolved_env (for Docker env vars)
-            env_pattern = r"<([A-Z_][A-Z0-9_]*)>"
-
-            def replace_env_var(match):
-                env_name = match.group(1)
-                return resolved_env.get(env_name, match.group(0))  # Return original if not found
-
-            processed = re.sub(env_pattern, replace_env_var, processed)
-
-        # Replace {runtime_var} with actual values from runtime_vars (for NPM args).
-        # Negative lookbehind on `$` so we never re-substitute inside an already-translated
-        # ${VAR} env placeholder (the brace is part of a Copilot CLI runtime substitution,
-        # not an APM template variable).
-        if runtime_vars:
-            runtime_pattern = r"(?<!\$)\{([a-zA-Z_][a-zA-Z0-9_]*)\}"
-
-            def replace_runtime_var(match):
-                var_name = match.group(1)
-                return runtime_vars.get(var_name, match.group(0))
-
-            processed = re.sub(runtime_pattern, replace_runtime_var, processed)
-
-        return processed
-
-    def _resolve_env_placeholders(self, value, resolved_env):
-        """Legacy method for backward compatibility. Use _resolve_variable_placeholders instead."""
-        return self._resolve_variable_placeholders(value, resolved_env, {})
-
     @staticmethod
     def _select_remote_with_url(remotes):
         """Return the first remote entry that has a non-empty URL.

src/apm_cli/adapters/client/cursor.py

@@ -74,6 +74,7 @@ def update_config(self, config_updates):
 
         with open(config_path, "w", encoding="utf-8") as f:
             json.dump(current_config, f, indent=2)
+        os.chmod(config_path, 0o600)
 
     def get_current_config(self):
         """Read the current ``.cursor/mcp.json`` contents."""

src/apm_cli/adapters/client/gemini.py

@@ -28,6 +28,7 @@
 
 import json
 import logging
+import os
 from pathlib import Path
 
 from ...core.docker_args import DockerArgsProcessor
@@ -100,6 +101,7 @@ def update_config(self, config_updates):
         config_path.parent.mkdir(parents=True, exist_ok=True)
         with open(config_path, "w", encoding="utf-8") as f:
             json.dump(current_config, f, indent=2)
+        os.chmod(config_path, 0o600)
 
     def get_current_config(self):
         """Read the current ``settings.json`` contents for the active scope."""
@@ -136,13 +138,26 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
         config: dict = {}
 
         # --- raw stdio (self-defined deps) ---
+        # Route ``env`` and ``args`` through the resolver pipeline so all
+        # three placeholder syntaxes (``<VAR>``, ``${VAR}``, ``${env:VAR}``)
+        # are resolved at install time before being written to
+        # ``.gemini/settings.json``. See issue #1266.
         raw = server_info.get("_raw_stdio")
         if raw:
             config["command"] = raw["command"]
-            config["args"] = raw["args"]
+            resolved_env_for_args: dict = {}
             if raw.get("env"):
-                config["env"] = raw["env"]
+                resolved_env_for_args = self._resolve_environment_variables(
+                    raw["env"], env_overrides=env_overrides
+                )
+                config["env"] = resolved_env_for_args
                 self._warn_input_variables(raw["env"], server_info.get("name", ""), "Gemini CLI")
+            config["args"] = [
+                self._resolve_variable_placeholders(arg, resolved_env_for_args, runtime_vars)
+                if isinstance(arg, str)
+                else arg
+                for arg in raw.get("args") or []
+            ]
             return config
 
         # --- remote endpoints ---