mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tools): neutralize shell injection in _write_to_sandbox via path quoting (#7940)
_write_to_sandbox interpolated storage_dir and remote_path directly into a shell command passed to env.execute(). Paths containing shell metacharacters (spaces, semicolons, $(), backticks) could trigger arbitrary command execution inside the sandbox. Fix: wrap both paths with shlex.quote(). Clean paths (alphanumeric + slashes/hyphens/dots) are left unmodified by shlex.quote, so existing behavior is unchanged. Paths with unsafe characters get single-quoted. Tests added for spaces, $(command) substitution, and semicolon injection.
This commit is contained in:
parent
255f59de18
commit
f2893fe51a
2 changed files with 30 additions and 1 deletions
|
|
@ -124,6 +124,34 @@ class TestWriteToSandbox:
|
||||||
cmd = env.execute.call_args[0][0]
|
cmd = env.execute.call_args[0][0]
|
||||||
assert "mkdir -p /data/data/com.termux/files/usr/tmp/hermes-results" in cmd
|
assert "mkdir -p /data/data/com.termux/files/usr/tmp/hermes-results" in cmd
|
||||||
|
|
||||||
|
def test_path_with_spaces_is_quoted(self):
|
||||||
|
env = MagicMock()
|
||||||
|
env.execute.return_value = {"output": "", "returncode": 0}
|
||||||
|
remote_path = "/tmp/hermes results/abc file.txt"
|
||||||
|
_write_to_sandbox("content", remote_path, env)
|
||||||
|
cmd = env.execute.call_args[0][0]
|
||||||
|
assert "'/tmp/hermes results'" in cmd
|
||||||
|
assert "'/tmp/hermes results/abc file.txt'" in cmd
|
||||||
|
|
||||||
|
def test_shell_metacharacters_neutralized(self):
|
||||||
|
"""Paths with shell metacharacters must be quoted to prevent injection."""
|
||||||
|
env = MagicMock()
|
||||||
|
env.execute.return_value = {"output": "", "returncode": 0}
|
||||||
|
malicious_path = "/tmp/hermes-results/$(whoami).txt"
|
||||||
|
_write_to_sandbox("content", malicious_path, env)
|
||||||
|
cmd = env.execute.call_args[0][0]
|
||||||
|
# The $() must not appear unquoted — shlex.quote wraps it
|
||||||
|
assert "'/tmp/hermes-results/$(whoami).txt'" in cmd
|
||||||
|
|
||||||
|
def test_semicolon_injection_neutralized(self):
|
||||||
|
env = MagicMock()
|
||||||
|
env.execute.return_value = {"output": "", "returncode": 0}
|
||||||
|
malicious_path = "/tmp/x; rm -rf /; echo .txt"
|
||||||
|
_write_to_sandbox("content", malicious_path, env)
|
||||||
|
cmd = env.execute.call_args[0][0]
|
||||||
|
# The semicolons must be inside quotes, not acting as command separators
|
||||||
|
assert "'/tmp/x; rm -rf /; echo .txt'" in cmd
|
||||||
|
|
||||||
|
|
||||||
class TestResolveStorageDir:
|
class TestResolveStorageDir:
|
||||||
def test_defaults_to_storage_dir_without_env(self):
|
def test_defaults_to_storage_dir_without_env(self):
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ Defense against context-window overflow operates at three levels:
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
|
import shlex
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
from tools.budget_config import (
|
from tools.budget_config import (
|
||||||
|
|
@ -79,7 +80,7 @@ def _write_to_sandbox(content: str, remote_path: str, env) -> bool:
|
||||||
marker = _heredoc_marker(content)
|
marker = _heredoc_marker(content)
|
||||||
storage_dir = os.path.dirname(remote_path)
|
storage_dir = os.path.dirname(remote_path)
|
||||||
cmd = (
|
cmd = (
|
||||||
f"mkdir -p {storage_dir} && cat > {remote_path} << '{marker}'\n"
|
f"mkdir -p {shlex.quote(storage_dir)} && cat > {shlex.quote(remote_path)} << '{marker}'\n"
|
||||||
f"{content}\n"
|
f"{content}\n"
|
||||||
f"{marker}"
|
f"{marker}"
|
||||||
)
|
)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue