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

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

Abierto
#759 1 comentario 0 reacciones 0 asignados Ver en GitHub
registry security
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

Abrir la 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

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.