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

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

オープン
#759 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
registry security
主要言語
TypeScript
スター
143
フォーク
46
平均マージ
3日 10時間
マージ済み PR(30日)
24

説明

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

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

agent/src/registry/loader.py の _protect_mcp_json_from_commit と apply_mcp_assets の周辺から始め、既存の git ヘルパー呼び出しと fail-closed のドキュメントを確認します。monkeypatch した git の失敗によってガード経路を再現し、committable secret を抱えたまま処理を続行せず、apply_mcp_assets が例外を発生させることを検証する回帰テストを追加します。無効な skip-worktree 保護が成功として扱われないことが完了条件です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
git, python
領域
security
issue の種類
バグ
難易度
3/5
見積もり時間
1〜2日
活発さ
静か
明瞭さ
明確に書かれている
初心者へのやさしさ
72/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。