microsoft / microsoft/apm

[BUG] Cursor adapter bakes MCP secrets into .cursor/mcp.json, and GitHub-host token injection bypasses runtime substitution on every target

Open
#2,991 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area/docs-site area/mcp-config area/multi-target priority/high status/accepted theme/security triage/recommended type/bug
Dominant language
Python
Stars
3.8k
Forks
362
Avg merge
1d 17h
Merged PRs (30d)
132

Description

Describe the bug

Two related defects cause apm install to write plaintext credentials to disk. The first is the Cursor follow-up that #1152 deferred; the second is a path #1152 did not cover, which still affects the copilot target it fixed.

Defect 1 — the Cursor adapter still resolves placeholders at install time.

src/apm_cli/adapters/client/cursor.py pins the flag introduced by #1169:

# Cursor's mcp.json runtime-substitution support has not yet been
# individually audited (see #1152). Pin to the legacy install-time
# resolution behaviour so this adapter is unchanged by the Copilot
# security fix; revisit in a follow-up.
_supports_runtime_env_substitution: bool = False

The audit that comment waits on can now be closed out. Cursor documents ${env:NAME} interpolation and names the fields it resolves:

Config interpolation — Use variables in mcp.json values. Cursor resolves variables in these fields: command, args, env, url, and headers.

https://cursor.com/docs/mcp

That page also gives a remote-server example matching exactly what APM emits:

{
  "mcpServers": {
    "remote-server": {
      "url": "https://api.example.com/mcp",
      "headers": { "Authorization": "Bearer ${env:MY_SERVICE_TOKEN}" }
    }
  }
}

One difference from #1152 raises the impact: .cursor/mcp.json is project-local, so the resolved secrets land inside the consumer's git working tree rather than in a user-global file. Since committing deployed files is the documented default, a package whose setup docs say "commit .cursor/ so teammates get the MCP config" turns this into committed credentials in a shared repository.

Defect 2 — GitHub-host token injection ignores the flag, on every target.

_apply_auth_and_headers_impl in src/apm_cli/adapters/client/base.py resolves a literal token and replaces the whole headers dict before any placeholder translation can run:

is_github_server = self._is_github_server(server_name, remote.get("url", ""))
local_token_injected = False
if is_github_server:
    _tm = token_manager_class()
    github_token = _tm.get_token_for_purpose("copilot") or os.getenv(
        "GITHUB_PERSONAL_ACCESS_TOKEN"
    )
    if github_token:
        config["headers"] = {"Authorization": f"Bearer {github_token}"}
        local_token_injected = True

A manifest-declared Authorization header is then skipped outright (if header_name == "Authorization" and local_token_injected: continue), so an author's ${env:...} placeholder is discarded rather than translated.

This block never consults _supports_runtime_env_substitution. As a result the copilot target still bakes a literal GitHub token into mcp-config.json for any GitHub-host MCP server, despite #1152 being closed as fixed.

To Reproduce

Defect 1:

  1. Create apm.yml:

    name: repro
    version: 1.0.0
    target: [cursor]
    dependencies:
      mcp:
        - name: probe
          registry: false
          transport: http
          url: https://example.invalid/mcp
          headers:
            x-probe-key: "${env:PROBE_SECRET}"
    
  2. mkdir .cursor — the adapter is opt-in on the directory already existing.

  3. Run PROBE_SECRET=DUMMY_SECRET_VALUE_12345 apm install

  4. Inspect .cursor/mcp.json; the header contains DUMMY_SECRET_VALUE_12345.

Defect 2:

  1. Add a GitHub-host server to the same manifest:

        - name: github
          registry: false
          transport: http
          url: https://api.githubcopilot.com/mcp/
          headers:
            Authorization: "Bearer ${env:GITHUB_PAT}"
    
  2. Set GITHUB_APM_PAT (or GITHUB_COPILOT_PAT / GITHUB_TOKEN) in the environment.

  3. Run apm install. The Authorization header holds the resolved token, and the value comes from the injection path rather than the manifest — setting GITHUB_PAT to a distinct sentinel shows the manifest placeholder is ignored entirely.

  4. Repeat with target: [copilot] to observe the same result on the target #1152 fixed.

Expected behavior

Placeholders emitted verbatim in Cursor's native syntax, so the secret never touches disk:

"headers": { "x-probe-key": "${env:PROBE_SECRET}" }

For defect 2, the injected GitHub token should likewise be emitted as a placeholder when the adapter supports runtime substitution, and a manifest-declared Authorization placeholder should be preserved rather than discarded.

Environment (please complete the following information):

  • OS: macOS (darwin 25.6.0), Apple silicon
  • Python Version: 3.14 (Homebrew venv)
  • APM Version: 0.30.0 (b3625f73d3), installed via Homebrew
  • VSCode Version (if relevant): N/A — reproduced against Cursor's project-local .cursor/mcp.json

Logs

N/A — no error is raised. The behavior is silent: apm install reports success and the secrets are written without warning.

Additional context

Proposed fix for defect 1 — mirror the existing IntelliJClientAdapter, which is the same CopilotClientAdapter subclass with the same native placeholder syntax:

    # Cursor natively resolves ${env:VAR} in command, args, env, url and
    # headers (https://cursor.com/docs/mcp -- "Config interpolation"), so
    # placeholders are emitted verbatim instead of resolved at install time.
    _supports_runtime_env_substitution: bool = True

    def _format_runtime_env_placeholder(self, name: str) -> str:
        """Return Cursor's native env-var placeholder syntax."""
        return "${env:" + name + "}"

The _format_runtime_env_placeholder override is not optional. Flipping the flag alone falls through to the base-class default, which emits bare ${VAR} — Copilot CLI's syntax, which Cursor does not resolve. That would swap baked secrets for headers silently transmitting the literal string ${VAR}, reintroducing #944 on the Cursor target. I confirmed this failure mode locally before discarding it.

For defect 2, _apply_auth_and_headers_impl should emit self._format_runtime_env_placeholder(...) rather than the resolved token when self._supports_runtime_env_substitution is true, and should not skip a manifest-declared Authorization placeholder.

Verification: I patched the installed 0.30.0 adapter locally. With both changes, a 10-server manifest emitted ${env:...} for all 7 secret-bearing headers and zero literal secrets — except the GitHub-host entry, which is defect 2. Worth noting for anyone already affected: re-running apm install after upgrading does not clean up an existing baked file. Servers are matched by name and left untouched, and --force --only mcp reports "already configured"; the file has to be deleted before the values are rewritten. A migration note or a detection warning may be warranted alongside the fix.

Two tests deliberately assert the current Cursor pin and will need updating:

  • tests/unit/test_copilot_adapter.py — asserts assertFalse(adapter._supports_runtime_env_substitution) for CursorClientAdapter
  • tests/integration/test_mcp_env_var_copilot_e2e.py — its docstring states the test "fails if that pin accidentally lifts"

In-tree precedent: intellij.py and kiro.py both set the flag True, and intellij.py overrides _format_runtime_env_placeholder for the env: prefix. vscode.py solves the same syntax problem via _translate_env_vars_for_vscode.

Scope: windsurf, gemini and opencode carry the identical pin and comment and plausibly warrant the same treatment, but each needs its own documentation audit and should be handled separately. claude and hermes must stay pinned — their config formats require literal values, as their own comments note.

Related: #1152 (Copilot install-time resolution, closed via #1169), #944 (bare ${VAR} in VS Code headers, closed via #947).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/apm_cli/adapters/client/cursor.py and the shared _apply_auth_and_headers_impl in src/apm_cli/adapters/client/base.py, then compare intellij.py and kiro.py. Update the Cursor-specific tests in tests/unit/test_copilot_adapter.py and tests/integration/test_mcp_env_var_copilot_e2e.py, and add coverage for GitHub-host authentication. Done means supported targets emit runtime placeholders without writing literal secrets and manifest Authorization placeholders are preserved.

Written by the indexing model from the issue text.

Assessment

Tech stack
github, python
Domain
cli, security, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.