Skip to content

registry: symlinked .mcp.json exfiltrates secret to a different tracked path + arbitrary-file-write (#665 B4 #1) #758

Description

@Kalindi-Dev

Source: BLOCKING #1 from @scottschreckengaust's review of #665#665 (review) (agent/src/registry/loader.py:190)
Parent: #246 · Sibling blockers: the fail-open guard and the skip-worktree data-loss issues (linked below)

Problem

apply_mcp_assets computes mcp_path = os.path.join(repo_dir, ".mcp.json") and opens it "w". Python's open(..., "w") follows symlinks, so if the cloned repo ships .mcp.json -> config/mcp.json (both tracked — a normal shared-config layout), the resolved unredacted MCP runtime (bearer headers, url?token=, --api-key args) lands in config/mcp.json. The _protect_mcp_json_from_commit guard then flags only the symlink's index entry (.mcp.json), leaving config/mcp.json unguarded — so post_hooks.ensure_committed (git add -u) stages it and ensure_pushed pushes the secret into the PR's git history. Reproduced against the real loader at 0b06ff5.

The same symlink-follow is an arbitrary-file-write primitive: .mcp.json -> .github/workflows/ci.yml overwrites the workflow (and add -u stages it); .mcp.json -> .git/config corrupts repo config so every later git call fails. Repo layout is attacker-controlled input — the agent clones untrusted repos / PR branches (repo.py:299).

Fix

Refuse a non-regular target before writing:

if os.path.islink(mcp_path):
    raise RegistryAssetLoadError(f"refusing to write resolved MCP config through a symlink: {mcp_path}")

(os.open(..., O_NOFOLLOW) is the race-free variant.)

Note: Scott's recommended durable fix — route registry MCP servers through the SDK in-process mcp_servers option instead of writing .mcp.json at all — closes this together with the sibling blockers. If that path is taken, this issue is subsumed.

Acceptance

  • A symlinked .mcp.json (pointing at a second tracked file, or into .git/.github) causes apply_mcp_assets to raise, not write-through
  • Regression test: symlink .mcp.json at a second tracked file, assert raise + git add -u && git diff --cached empty

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    registryAgent asset registry: capabilities, skills, plugins, MCP servers, blueprintssecurityCedar/HITL, IAM least-privilege, secrets, PII/DLP, guardrails, supply-chain/CVE

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions