Skip to content

fix(security): Security Hardening - Remediate Shell Injection Vulnerabilities - #4994

Closed
Xowiek wants to merge 1 commit into
NousResearch:mainfrom
Xowiek:fix/shell-injection-and-docker-cleanup
Closed

fix(security): Security Hardening - Remediate Shell Injection Vulnerabilities#4994
Xowiek wants to merge 1 commit into
NousResearch:mainfrom
Xowiek:fix/shell-injection-and-docker-cleanup

Conversation

@Xowiek

@Xowiek Xowiek commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Description

This pull request addresses two critical security vulnerabilities in the Hermes Agent related to subprocess.Popen and subprocess.run executions with shell=True. Using shell=True exposes the agent to arbitrary shell injection attacks, violating the Hermes Agent Contribution Guide's strict security policies.

Remediation Details

1. Docker Cleanup Shell Injection (tools/environments/docker.py)

  • Issue: The cleanup method for docker containers formerly scheduled background removal using subprocess.Popen(..., shell=True) with an unescaped self._container_id. Although currently generated securely via UUID, any future refactoring could allow untrusted inputs, resulting in severe command execution vulnerabilities.
  • Fix: Switched to a pure Python thread-based solution utilizing subprocess.run with list arguments (shell=False).

2. Local STT Shell Injection (tools/transcription_tools.py)

  • Issue: _transcribe_local_command relied on shlex.quote paired with subprocess.run(..., shell=True). This creates vulnerabilities on Windows since cmd.exe does not respect POSIX single-quotes. Additionally, any metacharacters bypassing shlex.quote in HERMES_LOCAL_STT_COMMAND could lead to full RCE.
  • Fix: Substituted shell=True string formatting with shlex.split. Format strings are safely populated securely within the subprocess argument list natively without invoking a subshell (shell=False), effectively neutralizing injection vectors while retaining argument templating functionality.

Validation

  • Tested STT parsing with shlex.split natively.
  • Validated background container cleanup with standard pure-Python threading.Thread.
  • Ensured complete avoidance of shell=True without breaking multi-platform dependencies (Posix vs Windows).

Code Base Compliance

  • Adheres to standard _HAS_FASTER_WHISPER fallback checks.
  • Compatible with all operating systems (Windows, macOS, Linux).
  • Prioritizes stability without unnecessary wrapper bloat as mandated by the Contribution Guide.

@teknium1

teknium1 commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

This is now fixed via PR #5629 (merged), which sanitizes workdir with tilde-aware shlex.quote + an allowlist validator. Thanks for flagging the vulnerability!

@teknium1 teknium1 closed this Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants