From 4bd7e481abbdfc802801e151113ecad5952cdfbd Mon Sep 17 00:00:00 2001
From: iuyup <23yntong@stu.edu.cn>
Date: Wed, 8 Apr 2026 20:44:34 +0800
Subject: [PATCH] fix(security): reduce unnecessary shell=True in subprocess
 calls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- 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).
---
 cli.py                                  |  2 ++
 hermes_cli/memory_setup.py              |  3 +-
 tests/tools/test_transcription_tools.py | 41 +++++++++++++++++++++++++
 tools/transcription_tools.py            |  8 ++++-
 4 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/cli.py b/cli.py
index f00e6b7feab06..e957d123b334e 100644
--- a/cli.py
+++ b/cli.py
@@ -4660,6 +4660,8 @@ def process_command(self, command: str) -> bool:
                     exec_cmd = qcmd.get("command", "")
                     if exec_cmd:
                         try:
+                            # shell=True is intentional: quick_commands are user-defined
+                            # shell snippets from config.yaml — not agent/LLM controlled.
                             result = subprocess.run(
                                 exec_cmd, shell=True, capture_output=True,
                                 text=True, timeout=30
diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py
index 2843f4f44445b..43c4c40434509 100644
--- a/hermes_cli/memory_setup.py
+++ b/hermes_cli/memory_setup.py
@@ -10,6 +10,7 @@
 import getpass
 import os
 import sys
+import shlex
 from pathlib import Path
 
 from hermes_constants import get_hermes_home
@@ -204,7 +205,7 @@ def _install_dependencies(provider_name: str) -> None:
         if check_cmd:
             try:
                 subprocess.run(
-                    check_cmd, shell=True, capture_output=True, timeout=5
+                    shlex.split(check_cmd), check=True, capture_output=True, timeout=5
                 )
             except Exception:
                 if install_cmd:
diff --git a/tests/tools/test_transcription_tools.py b/tests/tools/test_transcription_tools.py
index 0cd4c8e3c5efd..6a81fde58b8f8 100644
--- a/tests/tools/test_transcription_tools.py
+++ b/tests/tools/test_transcription_tools.py
@@ -858,3 +858,44 @@ def test_returns_none_when_model_key_missing(self, tmp_path, monkeypatch):
 
         from tools.transcription_tools import get_stt_model_from_config
         assert get_stt_model_from_config() is None
+
+# ============================================================================
+# 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
\ No newline at end of file
diff --git a/tools/transcription_tools.py b/tools/transcription_tools.py
index 4f07e5c472287..26347ef217a02 100644
--- a/tools/transcription_tools.py
+++ b/tools/transcription_tools.py
@@ -374,7 +374,13 @@ def _transcribe_local_command(file_path: str, model_name: str) -> Dict[str, Any]
                 language=shlex.quote(language),
                 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"))
             if not txt_files: