aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

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

Open
#758 1 comment 0 reactions 0 assignees View on GitHub
registry security
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

**Source:** BLOCKING #1 from @scottschreckengaust's review of #665 — https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/665#pullrequestreview-4915535549 (`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:
```python
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

Contributor guide

Open the contributing guide

Research direction

Start at agent/src/registry/loader.py:190 and trace apply_mcp_assets, then review repo.py:299 for the untrusted checkout context. Add a regression test using a symlinked .mcp.json that points to another tracked file, and verify the operation raises without changing the target. Run the relevant registry tests and confirm git add -u && git diff --cached remains empty.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, python
Domain
backend, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.