mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
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 <takis312@hotmail.com>
This commit is contained in:
parent
273b986fd9
commit
b76acacbb9
4 changed files with 97 additions and 47 deletions
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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.<name>` 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/<name>/` |
|
||||
| **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).
|
||||
|
||||
|
|
|
|||
|
|
@ -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.<name>` 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.<name>` 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.<name>: 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: <name>` — built-ins are inline |
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue