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

registry: .mcp.json skip-worktree guard fails open (secret already on disk) (#665 B4 #2)

Open
#759 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 #2 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:157`)
**Parent:** #246 · **Sibling blockers:** #758 (symlink), and the skip-worktree data-loss issue

## Problem
`_protect_mcp_json_from_commit` logs and continues when `git update-index --skip-worktree` fails — but by then the unredacted MCP runtime is already written to `.mcp.json`, so the full `git add -u` → commit → push chain is live and will exfiltrate the secret to the PR. The guard trusts the command exit code rather than verifying the flag actually took.

Two **non-attacker** triggers reproduced against the real loader:
- **`index.lock` contention** — concurrent git (prek hooks, a parallel agent tool call, a crashed prior git) → `fatal: Unable to create '.git/index.lock': File exists`; guard WARNs, then `git add -u` stages the secret.
- **Unmerged index entry** for `.mcp.json` — a conflicted predecessor merge (`repo.py:656` does exactly this for `merge_branches`; the `--abort` window exists) → `fatal: Unable to mark file .mcp.json`; same result.

Both WARNs are followed by `TASK Registry: merged 1 MCP server(s)`, so an operator reading logs sees success.

## Fix
Verify the observed flag and fail closed once bytes are on disk:
```python
verify = _git("ls-files", "-v", "--", ".mcp.json").stdout
if not verify.startswith(("S", "h")):
raise RegistryAssetLoadError(
"could not make .mcp.json unstageable; refusing to run with a "
"secret-bearing config in a committable tree"
)
```
Checking the observed flag also covers the symlink case (#758) for free. If adopted, the "best-effort … not fatal" docstring and the fail-closed list in the loader need updating to match.

> Scott's recommended durable fix (in-process `mcp_servers`, no disk write) subsumes this.

## Acceptance
- A failed/ineffective skip-worktree (lock contention, unmerged index) causes `apply_mcp_assets` to raise rather than proceed with a committable secret
- Regression test: monkeypatch the git call to fail, assert the raise

Contributor guide

Open the contributing guide

Research direction

Start in agent/src/registry/loader.py around _protect_mcp_json_from_commit and apply_mcp_assets, then inspect the existing git helper calls and the fail-closed documentation. Reproduce the guard path with a monkeypatched git failure and add a regression test asserting that apply_mcp_assets raises instead of proceeding with a committable secret; done means ineffective skip-worktree protection cannot be treated as success.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, python
Domain
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.