Skip to content

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

Description

@Kalindi-Dev

Source: BLOCKING #2 from @scottschreckengaust's review of #665#665 (review) (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:

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    registryAgent asset registry: capabilities, skills, plugins, MCP servers, blueprintssecurityCedar/HITL, IAM least-privilege, secrets, PII/DLP, guardrails, supply-chain/CVE

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions