fix: Address Gemini Code Assist review — shell injection, shlex, check=True

1. Shell injection (HIGH): Use shlex.split() and pass args as separate
   list elements to subprocess.run instead of concatenating into bash -c
   command string. Args now arrive as positional params ($1, $2, ...).

2. Fragile arg parsing (MEDIUM): Replace .split() with shlex.split() for
   Python script sys.argv injection — correctly handles quoted arguments
   like --name "John Doe".

3. Status detection (MEDIUM): Add check=True to subprocess.run in shell
   wrapper so non-zero exit codes raise CalledProcessError, which the
   code executor captures as stderr.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: caohy1988

contributing/samples/skill_script_demo/test_skill_compat.py

@@ -241,6 +241,7 @@ async def test_execute_with_input_args(toolset_with_executor, mock_ctx):
   call_args = toolset_with_executor._code_executor.execute_code.call_args
   code_input = call_args[0][1]
   assert "sys.argv" in code_input.code
+  assert "shlex.split" in code_input.code
   assert "-t stdio -c python eval.xml" in code_input.code
 
 

src/google/adk/tools/skill_toolset.py

@@ -392,25 +392,28 @@ def _prepare_code(
       # Python script: execute directly, inject sys.argv if args
       if input_args:
         return (
-            "import sys\n"
-            f"sys.argv = [{script_name!r}] + {input_args!r}.split()\n"
+            "import sys, shlex\n"
+            f"sys.argv = [{script_name!r}]"
+            f" + shlex.split({input_args!r})\n"
             + script_src
         )
       return script_src
     elif ext in ("sh", "bash"):
-      # Shell script: wrap in subprocess.run
+      # Shell script: wrap in subprocess.run.
+      # Args are passed as separate list elements after the script
+      # name to avoid shell injection — bash -c receives the script
+      # source, and $0/$1/... get the positional parameters.
       return (
-          "import subprocess\n"
+          "import subprocess, shlex\n"
           "_result = subprocess.run(\n"
-          f"    ['bash', '-c', {script_src!r}"
-          + (f" + ' ' + {input_args!r}" if input_args else "")
-          + f"],\n"
-          f"    capture_output=True, text=True,\n"
-          f")\n"
-          f"print(_result.stdout, end='')\n"
-          f"if _result.stderr:\n"
-          f"    import sys\n"
-          f"    print(_result.stderr, end='', file=sys.stderr)\n"
+          f"    ['bash', '-c', {script_src!r},"
+          f" {script_name!r}]"
+          + (f" + shlex.split({input_args!r})" if input_args else "")
+          + ",\n"
+          "    capture_output=True, text=True,\n"
+          "    check=True,\n"
+          ")\n"
+          "print(_result.stdout, end='')\n"
       )
     return None
 

tests/unittests/tools/test_skill_toolset.py

@@ -473,11 +473,12 @@ async def test_execute_script_shell_success(mock_skill1):
   assert result["status"] == "success"
   assert result["stdout"] == "setup\n"
 
-  # Verify the code wraps in subprocess.run
+  # Verify the code wraps in subprocess.run with check=True
   call_args = executor.execute_code.call_args
   code_input = call_args[0][1]
   assert "subprocess.run" in code_input.code
   assert "bash" in code_input.code
+  assert "check=True" in code_input.code
 
 
 @pytest.mark.asyncio
@@ -499,6 +500,7 @@ async def test_execute_script_with_input_args_python(mock_skill1):
   call_args = executor.execute_code.call_args
   code_input = call_args[0][1]
   assert "sys.argv" in code_input.code
+  assert "shlex.split" in code_input.code
   assert "--verbose --count 3" in code_input.code
 
 
@@ -520,6 +522,7 @@ async def test_execute_script_with_input_args_shell(mock_skill1):
 
   call_args = executor.execute_code.call_args
   code_input = call_args[0][1]
+  assert "shlex.split" in code_input.code
   assert "--force" in code_input.code