feat(agent-install): register generated MCPs safely

[sjungwon03]

Summary

• adds an opt-in project-only vs. agent-install flow with read-only client discovery and a reviewable, secret-free install plan
• adds digest-bound one-time install approval, config fingerprinting, symlink containment, atomic writes, mode-0600 backups, and target-local rollback
• provides verified native adapters for Codex TOML, Cursor/VS Code/Copilot/Gemini JSON, and OpenCode mcp.servers; Hermes and Claude Code remain portable-export-only until a tested native adapter exists
• adds portable mcpServers.json export and engine CLI coverage for preview → approval → apply

Verification

• python3 -m unittest discover -s skills/api-to-typemcp/tests -p 'test_*.py' -q — 127 passed
• python3 .agents/scripts/test_validate_docs.py — 8 passed
• python3 .agents/scripts/test_workspace.py — 3 passed
• python3 .agents/scripts/validate_docs.py — passed
• python3 .agents/scripts/test_skill_release.py — 15 passed
• python3 -m py_compile skills/api-to-typemcp/scripts/*.py
• git diff --check

Closes #71

[sjungwon03-ai]

Reviewed at exact head eac21f0. All required CI checks pass. Atomic install boundary is well-hardened: O_NOFOLLOW + dir-fd traversal, fingerprint-bound single-use receipts, fsync'd atomic replace, 0600 backups, reverse-order batch rollback, and zero secret values in any artifact. 33 new tests cover symlink traversal, race recheck, rollback, and receipt binding. Two minor suggestions below; nothing blocking.

[sjungwon03-ai]

Review Summary

Approved. This is a well-engineered, security-first implementation of opt-in agent MCP installation. The layered defense model (symlink containment, digest-bound single-use receipts, atomic O_NOFOLLOW writes, mode-0600 exclusive backups, and batch rollback) is thorough and correctly tested.

Strengths

• Defense in depth: Every mutation path re-validates fingerprint, inode identity, and content digest before and after backup creation, closing TOCTOU windows.
• Batch rollback: _apply_batch restores all previously completed targets when a later target fails — tested explicitly.
• Secret boundary: Plans carry env names only; _snapshot_spec rejects args resembling literal secrets; .env is never read.
• Fail-closed design: JSONC, missing configs, symlinks, and unsupported clients all fail closed with portable export as the safe fallback.
• OpenCode native format (latest commit): correctly emits "command": ["node", ...] without a separate args key.

Minor Observations (non-blocking)

See inline comments for two small output/dead-code items.

CI

All checks pass on head d7a4a2a: e2e ✓, bundled-engine ✓, docs-and-harness ✓.

[sjungwon03-ai]

Independent Review — Hermes Agent (code-reviewer)

Verdict: APPROVE — reviewed at exact head d7a4a2a.

Scope: 9 commits, 18 files, +2064/-2. New agent-install flow: read-only client discovery, secret-free plan preview, HMAC single-use receipt approval, atomic native config mutation (Codex TOML, Cursor/VSCode/Copilot/Gemini JSON, OpenCode mcp.servers), portable mcpServers.json export.

CI: verify workflow (docs-and-harness, bundled-engine, e2e) all SUCCESS at exact head SHA d7a4a2a. release-skill correctly SKIPPED on PR.

Security & safety review:

• dir_fd + O_NOFOLLOW traversal prevents symlink-following on every path component
• fstat identity (dev/ino) + sha256 digest re-checked before each os.replace — closes TOCTOU
• Atomic temp-file + os.replace with dir-fsync; exclusive 0600 backups refuse pre-placed attacker files
• Target-local rollback on verification failure; batch rollback restores prior targets in reverse
• Portable export rejects literal-secret-like args; plans expose env_names only, never values
• Receipts are HMAC-authenticated, TTL-bounded, single-use, bound to exact plan digest
• Codex codec is append-only after parse validation; JSONC fails closed until proven
• Hermes and Claude Code correctly remain portable-export-only (no untested native adapter)

Test coverage: 127 engine tests + 8 docs + 3 workspace + 15 skill-release all pass. Adversarial tests cover symlink ancestors, attacker pre-placed backups, injected TOCTOU races, literal-secret arg rejection, single-use receipt binding, and batch rollback.

Two minor non-blocking observations left as inline comments.

[sjungwon03-ai]

Minor observations (non-blocking)

1. install-plan --json flag is dead code — api_to_typemcp.py:309 declares --json on the install-plan subparser, but _cmd_install_plan always emits the full plan via _emit({"plan": ...}). The flag is never read.

2. Codex header check uses substring match — config_codecs.py:27 checks if header in source where header = [mcp_servers.petstore_mcp]. A user comment containing that literal text would trigger a false-positive refusal. This is fail-safe (refuses rather than corrupts), so no correctness risk, just a minor UX edge case.

Neither affects safety or correctness. Approved at d7a4a2a.

[sjungwon03]

Addressed in 8dfd21d: the unused install-plan --json option was removed and Codex table collision detection now uses parsed TOML structure, so a comment containing the header does not cause an unnecessary refusal.

[sjungwon03-ai]

Independent Review — Hermes Agent (code-reviewer)

Reviewed at exact head 8dfd21d. CI run 30375620348 passes all checks on this SHA. Prior 4 nits verified resolved. Fresh clone, diff against dev, full source + test inspection.

Safety: dir-fd O_NOFOLLOW traversal, fingerprint-bound single-use receipts, fsync'd atomic replace, 0600 backups, reverse-order batch rollback, post-write reread verification — all sound. Secret boundary enforced: env names only, no .env reads, descriptor fields screened for secret-like values.

Tests: 33 new tests cover symlink containment, race recheck, rollback, receipt binding, codec preservation, and CLI end-to-end. Coverage is thorough.

One non-blocking doc issue: SKILL.md install-plan example still passes --json, which was removed from the parser in nit remediation. See inline.

[sjungwon03-ai]

Independent Review — Hermes Agent (code-reviewer)

Verdict: APPROVE — reviewed at exact head 91ff018.

Scope: Full PR diff (18 files, +2069/−2) plus delta from previously reviewed head 8dfd21d.

Delta since last review: Single-line change in skills/api-to-typemcp/SKILL.md removing the obsolete --json flag from the install-plan example. Matches the resolved review thread exactly.

CI: All required checks pass on exact head 91ff018 (run 30380040063): bundled-engine ✓, docs-and-harness ✓, e2e ✓. release-skill correctly skipped for PR event.

Review threads: All prior inline comments resolved with referenced commits. No open threads.

Security posture: Unchanged from approved head 8dfd21d — symlink containment, atomic writes, mode-0600 backups, fingerprint-bound single-use receipts, and secret-free plans remain intact.

No blocking issues. LGTM.