aws-samples / aws-samples/sample-autonomous-cloud-coding-agents
registry: .mcp.json skip-worktree guard fails open (secret already on disk) (#665 B4 #2)
- Lenguaje dominante
- TypeScript
- Estrellas
- 143
- Forks
- 46
- Merge medio
- 3 d 10 h
- PR fusionados (30 d)
- 24
Descripción
**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
Guía de contribución
Línea de trabajo
Empieza en agent/src/registry/loader.py, alrededor de _protect_mcp_json_from_commit y apply_mcp_assets, y después inspecciona las llamadas existentes a los helpers de git y la documentación sobre el comportamiento fail-closed. Reproduce la ruta de protección con un fallo de git simulado mediante monkeypatch y añade una prueba de regresión que compruebe que apply_mcp_assets lanza una excepción en lugar de continuar con un committable secret; se considera terminado cuando una protección skip-worktree ineficaz no puede tratarse como un éxito.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- git, python
- Área
- security
- Tipo de issue
- Error
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Estado de actividad
- Tranquilo
- Claridad
- Bien especificado
- Aptitud para principiantes
- 72/100