fix(bash tool) implement proper command execution to not escape the sandbox (#1493)

It is currently possible for users to prompt an agent using skills to
escape srt and run bash commands without file or network restriction.
This change enforces proper bash command where it is passed as a single
argument without shell interpolation

---------

Signed-off-by: Josh Holt <[email protected]>
Co-authored-by: Copilot <[email protected]>

Author: jholt96

python/packages/kagent-skills/src/kagent/skills/shell.py

@@ -133,11 +133,12 @@ async def execute_command(
         env["PATH"] = f"{bash_venv_bin}:{env.get('PATH', '')}"
         env["VIRTUAL_ENV"] = bash_venv_path
 
-    sandboxed_command = f'srt "{command}"'
-
     try:
-        process = await asyncio.create_subprocess_shell(
-            sandboxed_command,
+        process = await asyncio.create_subprocess_exec(
+            "srt",
+            "sh",
+            "-c",
+            command,
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
             cwd=working_dir,

python/packages/kagent-skills/src/kagent/tests/unittests/test_skill_execution.py

@@ -1,8 +1,10 @@
+import asyncio
 import json
 import shutil
 import tempfile
 import textwrap
 from pathlib import Path
+from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
 
@@ -121,6 +123,42 @@ async def test_skill_core_logic(skill_test_env: Path):
     assert json.loads(json_content_str) == expected_data
 
 
+@pytest.mark.asyncio
+async def test_execute_command_no_shell_injection(tmp_path):
+    """
+    Verifies that shell metacharacters in the command are not interpreted by an
+    outer shell. The command must be passed as a single argument to srt, not
+    parsed by /bin/sh, so injection payloads cannot escape the sandbox.
+    """
+    captured = {}
+
+    async def mock_exec(*args, **kwargs):
+        captured["args"] = args
+        mock_process = MagicMock()
+        mock_process.communicate = AsyncMock(return_value=(b"ok", b""))
+        mock_process.returncode = 0
+        return mock_process
+
+    injection_payload = 'ls"; cat /etc/passwd; echo "pwned'
+
+    with (
+        patch("asyncio.create_subprocess_shell") as mock_shell,
+        patch("asyncio.create_subprocess_exec", side_effect=mock_exec),
+    ):
+        await execute_command(injection_payload, working_dir=tmp_path)
+
+    # Invariant 1: create_subprocess_shell must never be used.
+    assert not mock_shell.called
+
+    # Invariant 2: The entire payload must arrive as a single argument to srt, never split by a shell.
+    args = captured["args"]
+    # The first argument should still be the sandbox runner.
+    assert args[0] == "srt"
+    # The injection payload must appear exactly once as its own argument.
+    assert injection_payload in args
+    assert list(args).count(injection_payload) == 1
+
+
 def test_skill_discovery_and_loading(skill_test_env: Path):
     """
     Tests the core logic of discovering a skill and loading its instructions.