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

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

Aperta
#759 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
registry security
Lingua principale
TypeScript
Stelle
143
Fork
46
Merge medio
3g 9h
PR unite (30g)
20

Descrizione

**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

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia in agent/src/registry/loader.py, intorno a _protect_mcp_json_from_commit e apply_mcp_assets, poi esamina le chiamate esistenti agli helper git e la documentazione sul comportamento fail-closed. Riproduci il percorso di guardia con un errore git simulato tramite monkeypatch e aggiungi un test di regressione che verifichi che apply_mcp_assets sollevi un'eccezione invece di procedere con un committable secret; il lavoro è completo quando una protezione skip-worktree inefficace non può essere considerata un successo.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
git, python
Ambito
security
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
72/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.