mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-05-03 00:02:36 +00:00
fix: replace os.system with subprocess.run in unsafe mode pip install
* fix: replace os.system with subprocess.run in unsafe mode pip install Eliminates shell injection risk (A05) where a malicious library name like "pkg; rm -rf /" could execute arbitrary host commands. Using list-form subprocess.run with shell=False ensures the library name is always treated as a single argument with no shell metacharacter expansion. Adds two tests: one verifying list-form invocation, one verifying that shell metacharacters in a library name cannot trigger shell execution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use sys.executable -m pip to satisfy S607 linting rule S607 flags partial executable paths like ["pip", ...]. Using [sys.executable, "-m", "pip", ...] provides an absolute path and also ensures installation targets the correct Python environment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Greyson LaLonde <greyson.r.lalonde@gmail.com>
This commit is contained in:
@@ -8,6 +8,7 @@ potentially unsafe operations and importing restricted modules.
|
||||
import importlib.util
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from types import ModuleType
|
||||
from typing import Any, ClassVar, TypedDict
|
||||
|
||||
@@ -321,7 +322,7 @@ class CodeInterpreterTool(BaseTool):
|
||||
"""
|
||||
if self._check_docker_available():
|
||||
return self.run_code_in_docker(code, libraries_used)
|
||||
|
||||
|
||||
error_msg = (
|
||||
"Docker is required for safe code execution but is not available. "
|
||||
"The restricted sandbox fallback has been removed due to security vulnerabilities "
|
||||
@@ -411,7 +412,7 @@ class CodeInterpreterTool(BaseTool):
|
||||
Printer.print("WARNING: Running code in unsafe mode", color="bold_magenta")
|
||||
# Install libraries on the host machine
|
||||
for library in libraries_used:
|
||||
os.system(f"pip install {library}") # noqa: S605
|
||||
subprocess.run([sys.executable, "-m", "pip", "install", library], check=False) # noqa: S603
|
||||
|
||||
# Execute the code
|
||||
try:
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
from crewai_tools.tools.code_interpreter_tool.code_interpreter_tool import (
|
||||
@@ -152,6 +153,44 @@ x = 10
|
||||
assert result == "No result variable found."
|
||||
|
||||
|
||||
@patch("crewai_tools.tools.code_interpreter_tool.code_interpreter_tool.subprocess.run")
|
||||
def test_unsafe_mode_installs_libraries_without_shell(
|
||||
subprocess_run_mock, printer_mock, docker_unavailable_mock
|
||||
):
|
||||
"""Test that library installation uses subprocess.run with shell=False, not os.system."""
|
||||
tool = CodeInterpreterTool(unsafe_mode=True)
|
||||
code = "result = 1"
|
||||
libraries_used = ["numpy", "pandas"]
|
||||
|
||||
tool.run(code=code, libraries_used=libraries_used)
|
||||
|
||||
assert subprocess_run_mock.call_count == 2
|
||||
for call, library in zip(subprocess_run_mock.call_args_list, libraries_used):
|
||||
args, kwargs = call
|
||||
# Must be list form (no shell expansion possible)
|
||||
assert args[0] == [sys.executable, "-m", "pip", "install", library]
|
||||
# shell= must not be True (defaults to False)
|
||||
assert kwargs.get("shell", False) is False
|
||||
|
||||
|
||||
@patch("crewai_tools.tools.code_interpreter_tool.code_interpreter_tool.subprocess.run")
|
||||
def test_unsafe_mode_library_name_with_shell_metacharacters_does_not_invoke_shell(
|
||||
subprocess_run_mock, printer_mock, docker_unavailable_mock
|
||||
):
|
||||
"""Test that a malicious library name cannot inject shell commands."""
|
||||
tool = CodeInterpreterTool(unsafe_mode=True)
|
||||
code = "result = 1"
|
||||
malicious_library = "numpy; rm -rf /"
|
||||
|
||||
tool.run(code=code, libraries_used=[malicious_library])
|
||||
|
||||
subprocess_run_mock.assert_called_once()
|
||||
args, kwargs = subprocess_run_mock.call_args
|
||||
# The entire malicious string is passed as a single argument — no shell parsing
|
||||
assert args[0] == [sys.executable, "-m", "pip", "install", malicious_library]
|
||||
assert kwargs.get("shell", False) is False
|
||||
|
||||
|
||||
def test_unsafe_mode_running_unsafe_code(printer_mock, docker_unavailable_mock):
|
||||
"""Test behavior when no result variable is set."""
|
||||
tool = CodeInterpreterTool(unsafe_mode=True)
|
||||
|
||||
Reference in New Issue
Block a user