fix(mcp): block exfil-shaped stdio server configs

[teknium1]

Summary

MCP stdio config now rejects the #45620 exfiltration shape before it can be saved or spawned.

Changes

• hermes_cli/mcp_security.py: shared narrow validator for shell interpreter + network egress/exfil-shaped args.
• MCP config write paths: reject suspicious entries from hermes mcp add, dashboard /api/mcp/servers, profile creation, and catalog installs.
• Runtime MCP startup: skip suspicious direct-edited config.yaml entries before spawning them.
• Config migration + doctor: disable/surface already-persisted suspicious MCP entries while preserving the stanza for auditability.
• Supply-chain workflow: require mcp-catalog-reviewed label when optional-mcps/** or hermes_cli/mcp_catalog.py changes.

Validation

Check	Result
pytest tests/hermes_cli/test_mcp_security.py tests/hermes_cli/test_mcp_config.py tests/hermes_cli/test_mcp_catalog.py tests/tools/test_mcp_tool.py -o "addopts=" --tb=short -q	284 passed
python -m py_compile hermes_cli/mcp_security.py hermes_cli/mcp_config.py hermes_cli/mcp_catalog.py hermes_cli/web_server.py tools/mcp_tool.py hermes_cli/config.py hermes_cli/doctor.py tests/hermes_cli/test_mcp_security.py	passed
workflow YAML parse	passed
git diff --check	passed

Fixes #45620 by hardening the persisted MCP command boundary; investigation found no evidence that the referenced config migration injected the reporter's entry.

Infographic

[github-actions]

🔎 Lint report: fix/mcp-exfil-guard vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 10899 on HEAD, 10895 on base (🆕 +4)

🆕 New issues (4):

Rule	Count
unresolved-import	2
invalid-assignment	2

First entries

tests/hermes_cli/test_mcp_security.py:7: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/hermes_cli/test_mcp_security.py:96: [unresolved-import] unresolved-import: Cannot resolve imported module `fastapi.testclient`
hermes_cli/config.py:4835: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to `def validate_mcp_server_entry(name: str, entry: dict[str, Any]) -> list[str]`
tools/mcp_tool.py:2701: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to `def validate_mcp_server_entry(name: str, entry: dict[str, Any]) -> list[str]`

✅ Fixed issues: none

Unchanged: 5726 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

[liuhao1024]

Verified: clean security hardening for MCP stdio exfiltration.

Applied security checklist:

1. ✅ Fail-closed gate — _save_mcp_server returns False and blocks persistence when validate_mcp_server_entry flags issues. All callers (mcp_config.py, mcp_catalog.py, web_server.py) check the return value.
2. ✅ Defense-in-depth at all entry points — validation is applied at config migration, doctor diagnostics, mcp add CLI, catalog install, REST API server creation, and profile MCP server writing. No bypass path.
3. ✅ Narrow heuristic, not a whitelist — the regex targets the specific exfiltration shape from Security: config v27→v28 migration injected credential-exfiltration MCP server entry #45620 (shell interpreter + network egress tooling in args). Legitimate MCPs using npx, uvx, or custom binaries pass cleanly. The _EXFIL_HINT_PATTERN (POST, --data-binary, .env) adds signal without over-blocking.
4. ✅ Migration auto-disables existing suspicious entries — migrate_config() re-scans existing mcp_servers and marks suspicious entries enabled: False with audit warnings, rather than silently deleting them.
5. ✅ Test coverage — 131-line test_mcp_security.py + catalog rejection test covers the dangerous entry, benign entry, save rejection, and runtime loader paths.

No issues found.

[liuhao1024]

Verified: clean security PR. Applied security checklist — no findings.

Checked:

• Fail-closed on ImportError: validate_mcp_server_entry import failure → entry NOT saved (correct)
• Migration safety: migrate_config() auto-disables existing suspicious entries via read_raw_config() + save_config(), preserving entries for audit
• Defense-in-depth: validation applied at 3 independent save points (_save_mcp_server, install_entry, _write_profile_mcp_servers) — no bypass path
• Pattern specificity: _SHELL_INTERPRETERS + _EGRESS_PATTERN + _EXFIL_HINT_PATTERN — narrow heuristic targeting the Security: config v27→v28 migration injected credential-exfiltration MCP server entry #45620 exfiltration shape; does not over-block legitimate MCP servers using Python/npx/uvx
• Test coverage: 96-line mcp_security.py + integration in config.py, doctor.py, web_server.py, mcp_catalog.py, mcp_config.py

Well-scoped security gate with clean fail-closed behavior throughout the save pipeline.

[liuhao1024]

Positive verification — MCP exfiltration hardening

Reviewed the full diff across 10 files. The defense-in-depth is well-structured:

1. mcp_security.py — Narrow, correctly-scoped heuristic: shell interpreter + network egress tool in args. _command_basename uses os.path.basename to catch absolute paths. _EGRESS_PATTERN covers curl/wget/nc/ncat/socat + /dev/tcp/ + PowerShell equivalents. Intentionally not a whitelist — legitimate local MCPs (npx, uvx, custom binaries) pass through.

2. 5-layer enforcement — write-time (_save_mcp_server), catalog install (install_entry), web API (add_mcp_server), runtime loader (_load_mcp_config), and migration (migrate_config). Even if one layer is bypassed, the others catch it.

3. Migration — Existing suspicious entries get enabled: False rather than deleted, preserving audit trail. The validate_mcp_server_entry import is wrapped in try/except so a broken mcp_security module doesn't block config migration entirely.

4. CI gate — mcp-catalog-review job requires explicit mcp-catalog-reviewed label when optional-mcps/** or mcp_catalog.py change. Prevents silent catalog poisoning via PRs.

5. Test coverage — 131-line test file covers: dangerous entry detection, clean npx/shell-pipe passthrough, save rejection, runtime skip, migration disabling, dashboard API rejection, and profile-write skip. Catalog test also verifies CatalogError on exfil-shaped manifests.

One observation: _EXFIL_HINT_PATTERN (.env, --data-binary, POST, < file) adds an "exfiltration-shaped" qualifier to the warning message but does not gate the block decision — the block is triggered by _EGRESS_PATTERN alone (shell interpreter + curl/wget/nc). The hint pattern is purely informational for the warning text. This is the correct design — the narrow heuristic blocks the mechanism (shell + egress), not the intent.

No issues found. Clean security PR.