From b76acacbb9da3dae7f6c8813139720e6945dc8c0 Mon Sep 17 00:00:00 2001 From: Eugeniusz Gilewski Date: Wed, 22 Jul 2026 22:18:32 +0200 Subject: [PATCH] fix(tools): execute local STT templates without a shell HERMES_LOCAL_STT_COMMAND rendered quoted placeholders into a user-configured template and passed the result to shell=True. Shell metacharacters in the template therefore remained executable syntax even though the placeholder values themselves were quoted. Tokenize the rendered template and invoke it as an argv list while preserving the existing timeout, closed stdin, and Windows creation flags. Lock the invocation contract with metacharacter regression coverage and document explicit shell wrapping for trusted templates that need it. Salvages #32694 Co-authored-by: Ernest Hysa --- tests/tools/test_transcription_tools.py | 85 ++++++++++++++++--- tools/transcription_tools.py | 41 +++------ website/docs/developer-guide/plugins/index.md | 4 +- website/docs/user-guide/features/tts.md | 14 ++- 4 files changed, 97 insertions(+), 47 deletions(-) diff --git a/tests/tools/test_transcription_tools.py b/tests/tools/test_transcription_tools.py index 892725abff35..581b6ccbec50 100644 --- a/tests/tools/test_transcription_tools.py +++ b/tests/tools/test_transcription_tools.py @@ -617,12 +617,7 @@ class TestTranscribeLocalCommand: return _TempDir() def fake_run(cmd, *args, **kwargs): - if isinstance(cmd, list): - output_path = cmd[-1] - with open(output_path, "wb") as handle: - handle.write(b"RIFF....WAVEfmt ") - return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") - + assert isinstance(cmd, list) (out_dir / "test.txt").write_text("hello from local command\n", encoding="utf-8") return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") @@ -2142,13 +2137,77 @@ class TestShellSafety: 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_env_var_template_metacharacters_are_literal_argv( + self, monkeypatch, sample_wav, tmp_path + ): + from tools.transcription_tools import ( + LOCAL_STT_COMMAND_ENV, + _transcribe_local_command, + windows_hide_flags, + ) + + output_dir = tmp_path / "transcript-output" + output_dir.mkdir() + monkeypatch.setenv( + LOCAL_STT_COMMAND_ENV, + ( + "whisper {input_path} ; printf injected | tee log.txt " + "&& echo $(id) `whoami` --output_dir {output_dir}" + ), + ) + + def fake_tempdir(prefix=None): + class _TempDir: + def __enter__(self): + return str(output_dir) + + def __exit__(self, exc_type, exc, tb): + return False + + return _TempDir() + + invocation = {} + + def fake_run(command, **kwargs): + invocation["command"] = command + invocation["kwargs"] = kwargs + (output_dir / "transcript.txt").write_text("safe", encoding="utf-8") + return subprocess.CompletedProcess(command, 0, stdout="", stderr="") + + monkeypatch.setattr( + "tools.transcription_tools.tempfile.TemporaryDirectory", fake_tempdir + ) + monkeypatch.setattr("tools.transcription_tools.subprocess.run", fake_run) + + result = _transcribe_local_command(sample_wav, "base") + + assert result["transcript"] == "safe" + assert invocation["command"] == [ + "whisper", + sample_wav, + ";", + "printf", + "injected", + "|", + "tee", + "log.txt", + "&&", + "echo", + "$(id)", + "`whoami`", + "--output_dir", + str(output_dir), + ] + assert invocation["kwargs"] == { + "check": True, + "capture_output": True, + "text": True, + "encoding": "utf-8", + "errors": "replace", + "timeout": 300, + "stdin": subprocess.DEVNULL, + "creationflags": windows_hide_flags(), + } def test_no_env_var_uses_list_mode(self, monkeypatch): """When no env var is set, use_shell should be False.""" diff --git a/tools/transcription_tools.py b/tools/transcription_tools.py index 4d60ce6a56e2..4f228f470117 100644 --- a/tools/transcription_tools.py +++ b/tools/transcription_tools.py @@ -1549,41 +1549,24 @@ def _transcribe_local_command(file_path: str, model_name: str) -> Dict[str, Any] language=shlex.quote(language), model=shlex.quote(normalized_model), ) - # User-provided templates (env var) may contain shell syntax; auto-detected commands are safe for list mode. # Scrub Hermes secrets from the child env (sibling path to #56332 / # _run_command_stt — this local-whisper path previously inherited # the full process environment). from tools.environments.local import hermes_subprocess_env child_env = hermes_subprocess_env(inherit_credentials=False) - 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, - encoding="utf-8", - errors="replace", - timeout=300, - stdin=subprocess.DEVNULL, - env=child_env, - creationflags=windows_hide_flags(), - ) - else: - subprocess.run( - shlex.split(command), - check=True, - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - timeout=300, - stdin=subprocess.DEVNULL, - env=child_env, - creationflags=windows_hide_flags(), - ) + subprocess.run( + shlex.split(command), + check=True, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=300, + stdin=subprocess.DEVNULL, + env=child_env, + creationflags=windows_hide_flags(), + ) txt_files = sorted(Path(output_dir).glob("*.txt")) if not txt_files: diff --git a/website/docs/developer-guide/plugins/index.md b/website/docs/developer-guide/plugins/index.md index 7e748b6c1e1c..97bdf85690f0 100644 --- a/website/docs/developer-guide/plugins/index.md +++ b/website/docs/developer-guide/plugins/index.md @@ -28,7 +28,7 @@ Hermes has several distinct pluggable interfaces — some use Python `register_* | A **secret-manager backend** (vault / password manager / OS keystore) | [Secret Source Plugins](/developer-guide/secret-source-plugin) | | A **dashboard OIDC/auth provider** | [Web Dashboard — custom providers](/user-guide/features/web-dashboard#custom-providers) — `ctx.register_dashboard_auth_provider()` | | A **TTS backend** (any CLI — Piper, VoxCPM, Kokoro, voice cloning, …) | [TTS custom command providers](/user-guide/features/tts#custom-command-providers) — config-driven, no Python needed | -| An **STT backend** (custom whisper / ASR CLI) | [Voice Message Transcription](/user-guide/features/tts#voice-message-transcription-stt) — set `HERMES_LOCAL_STT_COMMAND` to a shell template | +| An **STT backend** (custom whisper / ASR CLI) | [Voice Message Transcription](/user-guide/features/tts#voice-message-transcription-stt) — set `HERMES_LOCAL_STT_COMMAND` to an argv-tokenized template | | **External tools via MCP** (filesystem, GitHub, Linear, any MCP server) | [MCP](/user-guide/features/mcp) — declare `mcp_servers.` in `config.yaml` | | **Gateway event hooks** (fire on startup, session events, commands) | [Event Hooks](/user-guide/features/hooks#gateway-event-hooks) — drop `HOOK.yaml` + `handler.py` into `~/.hermes/hooks//` | | **Shell hooks** (run a shell command on events) | [Shell Hooks](/user-guide/features/hooks#shell-hooks) — declare under `hooks:` in `config.yaml` | @@ -1199,7 +1199,7 @@ tts: voice_compatible: true ``` -For STT, point `HERMES_LOCAL_STT_COMMAND` at a shell template. Supported placeholders: `{input_path}`, `{output_path}`, `{format}`, `{voice}`, `{model}`, `{speed}` (TTS); `{input_path}`, `{output_dir}`, `{language}`, `{model}` (STT). Any path-interacting CLI is automatically a plugin. +For STT, point `HERMES_LOCAL_STT_COMMAND` at an argv-tokenized template. It runs without implicit shell interpretation; wrap it in `sh -c`, `cmd /c`, or PowerShell explicitly if the trusted local command requires shell syntax. Supported placeholders: `{input_path}`, `{output_path}`, `{format}`, `{voice}`, `{model}`, `{speed}` (TTS); `{input_path}`, `{output_dir}`, `{language}`, `{model}` (STT). Any path-interacting CLI is automatically a plugin. **Full guides:** [TTS custom command providers](/user-guide/features/tts#custom-command-providers) · [STT](/user-guide/features/tts#voice-message-transcription-stt). diff --git a/website/docs/user-guide/features/tts.md b/website/docs/user-guide/features/tts.md index 6713f77f14a0..7d3cfed0c49f 100644 --- a/website/docs/user-guide/features/tts.md +++ b/website/docs/user-guide/features/tts.md @@ -481,7 +481,7 @@ stt: **xAI Grok STT** — Requires `XAI_API_KEY`. Posts to `https://api.x.ai/v1/stt` as multipart/form-data. Good choice if you're already using xAI for chat or TTS and want one API key for everything. Auto-detection order puts it after Groq — explicitly set `stt.provider: xai` to force it. -**Custom local CLI fallback** — Set `HERMES_LOCAL_STT_COMMAND` if you want Hermes to call a local transcription command directly. The command template supports `{input_path}`, `{output_dir}`, `{language}`, and `{model}` placeholders. Your command must write a `.txt` transcript somewhere under `{output_dir}`. +**Custom local CLI fallback** — Set `HERMES_LOCAL_STT_COMMAND` if you want Hermes to call a local transcription command directly. The command template supports `{input_path}`, `{output_dir}`, `{language}`, and `{model}` placeholders. Hermes tokenizes the rendered template into an argument list and executes it without a shell, so operators such as `|`, `>`, `&&`, and `;` are passed as literal arguments. Your command must write a `.txt` transcript somewhere under `{output_dir}`. #### Example: Doubao / Volcengine ASR @@ -494,6 +494,14 @@ export VOLCENGINE_ACCESS_TOKEN="your-access-token" export HERMES_LOCAL_STT_COMMAND='doubao-speech transcribe {input_path} --out {output_dir}/transcript.txt' ``` +If a trusted local template intentionally needs pipes, redirects, or another shell feature, invoke the shell explicitly. Keep dynamic paths outside the shell program and pass them as positional arguments: + +```bash +export HERMES_LOCAL_STT_COMMAND='sh -c '\''whisper "$1" --output_format txt --output_dir "$2" | tee "$2/whisper.log"'\'' _ {input_path} {output_dir}' +``` + +On Windows, use an explicit `cmd /c` or PowerShell wrapper instead. An explicit wrapper makes shell interpretation an opt-in part of the configured argv rather than an implicit property of every local STT template. + ```yaml stt: provider: local_command @@ -538,7 +546,7 @@ stt: format: json ``` -This complements the legacy `HERMES_LOCAL_STT_COMMAND` escape hatch — that env var still works untouched via the built-in `local_command` path. Use `stt.providers.` when you want **multiple** shell-driven STT engines, a name you can pick via `stt.provider`, or anything that needs per-provider `language` / `model` / `timeout`. +This complements the legacy `HERMES_LOCAL_STT_COMMAND` escape hatch via the built-in `local_command` path. Unlike the shell-driven command-provider registry, the legacy template is tokenized into argv and runs without implicit shell interpretation. Use `stt.providers.` when you want **multiple** shell-driven STT engines, a name you can pick via `stt.provider`, or anything that needs per-provider `language` / `model` / `timeout`. #### STT placeholders @@ -595,7 +603,7 @@ For STT engines that aren't built-in AND can't be expressed as a shell command ( | Backend has… | Use | |--------------------------------------------------------------|------------------------------------------------------------------| | A single shell command that takes an audio file and emits text | `stt.providers.: type: command` (no Python needed) | -| Only the legacy single-command escape hatch is wanted | `HERMES_LOCAL_STT_COMMAND` env var (preserved for back-compat) | +| Only the legacy single-command escape hatch is wanted | `HERMES_LOCAL_STT_COMMAND` env var (argv-tokenized; no implicit shell) | | A Python SDK with no CLI | `register_transcription_provider()` plugin | | OAuth-refreshing auth, streaming chunks, voice-list metadata | `register_transcription_provider()` plugin | | A built-in already covers it (`local`, `groq`, `openai`, …) | Set `stt.provider: ` — built-ins are inline |