From f2893fe51a59e545ad05f459fb235296872c4561 Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Sat, 11 Apr 2026 14:26:11 -0700
Subject: [PATCH] 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.
---
 tests/tools/test_tool_result_storage.py | 28 +++++++++++++++++++++++++
 tools/tool_result_storage.py            |  3 ++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/tests/tools/test_tool_result_storage.py b/tests/tools/test_tool_result_storage.py
index f95b5dc08..0bbb95bbd 100644
--- a/tests/tools/test_tool_result_storage.py
+++ b/tests/tools/test_tool_result_storage.py
@@ -124,6 +124,34 @@ class TestWriteToSandbox:
         cmd = env.execute.call_args[0][0]
         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:
     def test_defaults_to_storage_dir_without_env(self):
diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py
index a8ec5440b..434226448 100644
--- a/tools/tool_result_storage.py
+++ b/tools/tool_result_storage.py
@@ -24,6 +24,7 @@ Defense against context-window overflow operates at three levels:
 
 import logging
 import os
+import shlex
 import uuid
 
 from tools.budget_config import (
@@ -79,7 +80,7 @@ def _write_to_sandbox(content: str, remote_path: str, env) -> bool:
     marker = _heredoc_marker(content)
     storage_dir = os.path.dirname(remote_path)
     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"{marker}"
     )