Skip to content

fix(config): reject MCP server entries with shell egress patterns (#45620) - #45886

Closed
rodboev wants to merge 5 commits into
NousResearch:mainfrom
rodboev:pr/mcp-server-validation
Closed

fix(config): reject MCP server entries with shell egress patterns (#45620)#45886
rodboev wants to merge 5 commits into
NousResearch:mainfrom
rodboev:pr/mcp-server-validation

Conversation

@rodboev

@rodboev rodboev commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

_save_mcp_server() and save_config() write MCP server entries to config.yaml without inspecting the command or args fields. A rogue entry using bash -c "cat ~/.hermes/.env | curl ..." was reported upstream; it attempted to exfiltrate API keys on every service start. The exact injection vector is uncertain (the v27→v28 migration block referenced in the report does not exist in current code), but the root vulnerability is that no validation exists at the config write boundary.

This PR adds _validate_mcp_server_entry() that flags shell interpreters combined with network egress patterns in args. The check gates _save_mcp_server() (blocking new dangerous entries), runs as a migration cleanup pass (disabling existing dangerous entries with enabled: false), and surfaces warnings in hermes doctor.

Changes

  • hermes_cli/mcp_config.py (+51 lines, -11 lines) — Added _validate_mcp_server_entry() function that inspects command and args for shell+egress patterns; gated _save_mcp_server() on validation; updated all CLI callers to check return value
  • hermes_cli/config.py (+22 lines) — Added post-migration audit pass that scans existing mcp_servers entries, disables flagged ones, and warns
  • hermes_cli/doctor.py (+13 lines) — Added MCP server egress diagnostic that warns on entries with suspicious patterns
  • hermes_cli/web_server.py (+9 lines, -1 line) — Gated POST /api/mcp/servers on validation return (returns 400 when rejected); added validation to _write_profile_mcp_servers() so profile-create also rejects dangerous entries
  • tests/hermes_cli/test_mcp_server_validation.py (new, +290 lines) — 20 tests covering validation logic, save rejection, profile-create rejection, edge cases, and all major shell variants

Validation

Scenario Before After
hermes mcp add with npx @example/server saved saved (unchanged)
hermes mcp add with bash -c "cat ... | curl ..." saved silently rejected with warning
Existing _m1780983924 entry after migration persists, runs on start disabled (enabled: false), warning printed
hermes doctor with dangerous entry no diagnostic warning emitted
hermes mcp add with bash -c "echo hello" saved saved (no egress, passes)
Profile create with bash -c "... | curl ..." server saved silently skipped with warning log

Test plan

  • pytest tests/hermes_cli/test_mcp_server_validation.py -v — 20 passed (covers clean entries, shell+egress rejection, specific attack pattern, save rejection, profile-create rejection, all major shell variants, edge cases)

Upstream

Closes #45620.
Reported by @MHS-LB.
Thanks to @Ben-Home for the mitigation analysis.

@liuhao1024

Copy link
Copy Markdown
Contributor

Reviewed the diff and tests — this is a solid defense-in-depth addition for MCP server credential exfiltration.

What I checked:

  1. Validation scope_validate_mcp_server_entry correctly gates on shell interpreter (bash, sh, zsh, cmd, powershell, etc.) before scanning args. Non-shell commands (e.g., node, npx) pass through clean — verified by test_non_shell_command_with_egress_passes.

  2. Egress pattern coverage — The regex covers curl, wget, nc, ncat, socat, /dev/tcp, and PowerShell equivalents (Invoke-WebRequest, Invoke-RestMethod, System.Net.WebClient). Case-insensitive matching is correct.

  3. Integration points — The validation is wired into _save_mcp_server (CLI), migrate_config (auto-disable during upgrade), run_doctor (diagnostics), and _write_profile_mcp_servers (web server API). All callers correctly handle the False return / empty-warnings-list.

  4. Migration behavior — The migrate_config addition reads raw config, validates each MCP server, and auto-disables flagged entries. This is the right behavior for the upgrade path — users see warnings and can review.

  5. Test coverage — 290 lines covering clean entries, shell+egress, pipe chains, PowerShell variants, case insensitivity, missing fields, save rejection, and profile-write rejection. The specific attack pattern from the issue (Security: config v27→v28 migration injected credential-exfiltration MCP server entry #45620) is explicitly tested.

  6. Limitation (by design) — The validation only checks shell-interpreter args. A malicious node server with inline fetch() to an exfil endpoint would not be caught, but that requires a different detection approach (runtime sandboxing). The PR addresses the most common and directly exploitable pattern.

No issues found.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jun 14, 2026
AIalliAI added a commit to AIalliAI/Hermes that referenced this pull request Jun 14, 2026
AIalliAI added a commit to AIalliAI/Hermes that referenced this pull request Jun 14, 2026
@rodboev
rodboev force-pushed the pr/mcp-server-validation branch from 9fd4124 to b7f1f19 Compare June 15, 2026 22:49
@alt-glitch alt-glitch added comp/dashboard Web dashboard / control panel UI (dashboard/, landing) tool/mcp MCP client and OAuth sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jun 26, 2026
@rodboev
rodboev force-pushed the pr/mcp-server-validation branch from 11b2fc0 to ae997f4 Compare June 28, 2026 19:53
@rodboev
rodboev force-pushed the pr/mcp-server-validation branch from c784b07 to 1ced4cb Compare July 7, 2026 05:20
@rodboev

rodboev commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Closing this now. The linked issue was already completed by merged PR #46083, so this MCP shell-egress validation branch is superseded.

@rodboev rodboev closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/mcp MCP client and OAuth type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: config v27→v28 migration injected credential-exfiltration MCP server entry

3 participants