mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-26 06:01:49 +00:00
fix(security): reduce unnecessary shell=True in subprocess calls
- memory_setup.py: use shlex.split() for plugin dep checks instead of shell=True - transcription_tools.py: avoid shell=True for auto-detected whisper commands (user-provided templates via env var still use shell=True for compatibility) - cli.py: add comment clarifying intentional shell=True for user quick_commands - Add test verifying auto-detected template is shlex-safe Addresses CONTRIBUTING.md Priority #3 (Security hardening — shell injection).
This commit is contained in:
parent
a9b8254e5f
commit
d6c9711ba8
4 changed files with 53 additions and 2 deletions
2
cli.py
2
cli.py
|
|
@ -7600,6 +7600,8 @@ class HermesCLI:
|
||||||
exec_cmd = qcmd.get("command", "")
|
exec_cmd = qcmd.get("command", "")
|
||||||
if exec_cmd:
|
if exec_cmd:
|
||||||
try:
|
try:
|
||||||
|
# shell=True is intentional: quick_commands are user-defined
|
||||||
|
# shell snippets from config.yaml — not agent/LLM controlled.
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
exec_cmd, shell=True, capture_output=True,
|
exec_cmd, shell=True, capture_output=True,
|
||||||
text=True, timeout=30
|
text=True, timeout=30
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ from __future__ import annotations
|
||||||
import getpass
|
import getpass
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
|
import shlex
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from hermes_constants import get_hermes_home
|
from hermes_constants import get_hermes_home
|
||||||
|
|
@ -134,7 +135,7 @@ def _install_dependencies(provider_name: str) -> None:
|
||||||
if check_cmd:
|
if check_cmd:
|
||||||
try:
|
try:
|
||||||
subprocess.run(
|
subprocess.run(
|
||||||
check_cmd, shell=True, capture_output=True, timeout=5
|
shlex.split(check_cmd), check=True, capture_output=True, timeout=5
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
if install_cmd:
|
if install_cmd:
|
||||||
|
|
|
||||||
|
|
@ -1363,3 +1363,45 @@ class TestTranscribeAudioXAIDispatch:
|
||||||
transcribe_audio(sample_ogg, model="custom-stt")
|
transcribe_audio(sample_ogg, model="custom-stt")
|
||||||
|
|
||||||
assert mock_xai.call_args[0][1] == "custom-stt"
|
assert mock_xai.call_args[0][1] == "custom-stt"
|
||||||
|
|
||||||
|
|
||||||
|
# ============================================================================
|
||||||
|
# Shell safety — shlex.split on auto-detected templates
|
||||||
|
# ============================================================================
|
||||||
|
class TestShellSafety:
|
||||||
|
def test_auto_detected_template_is_shlex_safe(self, monkeypatch):
|
||||||
|
"""Auto-detected whisper command should be safely splittable."""
|
||||||
|
import shlex
|
||||||
|
monkeypatch.delenv("HERMES_LOCAL_STT_COMMAND", raising=False)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"tools.transcription_tools._find_whisper_binary",
|
||||||
|
lambda: "/usr/bin/whisper",
|
||||||
|
)
|
||||||
|
from tools.transcription_tools import _get_local_command_template
|
||||||
|
template = _get_local_command_template()
|
||||||
|
assert template is not None
|
||||||
|
cmd = template.format(
|
||||||
|
input_path=shlex.quote("/tmp/test.wav"),
|
||||||
|
output_dir=shlex.quote("/tmp/out"),
|
||||||
|
language=shlex.quote("en"),
|
||||||
|
model=shlex.quote("base"),
|
||||||
|
)
|
||||||
|
parts = shlex.split(cmd)
|
||||||
|
assert parts[0] == "/usr/bin/whisper"
|
||||||
|
assert "/tmp/test.wav" in parts
|
||||||
|
|
||||||
|
def test_env_var_template_uses_shell_path(self, monkeypatch):
|
||||||
|
"""When HERMES_LOCAL_STT_COMMAND is set, use_shell should be True."""
|
||||||
|
import os
|
||||||
|
from tools.transcription_tools import LOCAL_STT_COMMAND_ENV
|
||||||
|
monkeypatch.setenv(LOCAL_STT_COMMAND_ENV, "whisper {input_path} | tee log.txt")
|
||||||
|
use_shell = bool(os.getenv(LOCAL_STT_COMMAND_ENV, "").strip())
|
||||||
|
assert use_shell is True
|
||||||
|
|
||||||
|
def test_no_env_var_uses_list_mode(self, monkeypatch):
|
||||||
|
"""When no env var is set, use_shell should be False."""
|
||||||
|
import os
|
||||||
|
from tools.transcription_tools import LOCAL_STT_COMMAND_ENV
|
||||||
|
monkeypatch.delenv(LOCAL_STT_COMMAND_ENV, raising=False)
|
||||||
|
use_shell = bool(os.getenv(LOCAL_STT_COMMAND_ENV, "").strip())
|
||||||
|
assert use_shell is False
|
||||||
|
|
|
||||||
|
|
@ -505,7 +505,13 @@ def _transcribe_local_command(file_path: str, model_name: str) -> Dict[str, Any]
|
||||||
language=shlex.quote(language),
|
language=shlex.quote(language),
|
||||||
model=shlex.quote(normalized_model),
|
model=shlex.quote(normalized_model),
|
||||||
)
|
)
|
||||||
subprocess.run(command, shell=True, check=True, capture_output=True, text=True)
|
# User-provided templates (env var) may contain shell syntax; auto-detected commands are safe for list mode.
|
||||||
|
use_shell = bool(os.getenv(LOCAL_STT_COMMAND_ENV, "").strip())
|
||||||
|
if use_shell:
|
||||||
|
subprocess.run(command, shell=True, check=True, capture_output=True, text=True)
|
||||||
|
else:
|
||||||
|
subprocess.run(shlex.split(command), check=True, capture_output=True, text=True)
|
||||||
|
|
||||||
|
|
||||||
txt_files = sorted(Path(output_dir).glob("*.txt"))
|
txt_files = sorted(Path(output_dir).glob("*.txt"))
|
||||||
if not txt_files:
|
if not txt_files:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue