From 9e04b65549f101de17067079c1b7a7f1b7fa20ae Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 16 Sep 2025 17:26:44 +0000 Subject: [PATCH] fix: add Windows-compatible subprocess execution for CLI commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create cross-platform subprocess utility with Windows shell=True support - Update all CLI commands to use new subprocess utility instead of direct subprocess.run() - Add comprehensive tests for Windows compatibility scenarios - Fixes #3522: Access denied errors on Windows CLI execution The core issue was that Windows with restrictive security policies blocks subprocess execution when shell=False (the default). Using shell=True on Windows allows commands to execute through the Windows shell, which typically has the necessary permissions. Files updated: - src/crewai/cli/subprocess_utils.py (new utility) - tests/cli/test_subprocess_utils.py (new tests) - All CLI files that use subprocess.run(): run_crew.py, kickoff_flow.py, install_crew.py, train_crew.py, evaluate_crew.py, replay_from_task.py, plot_flow.py, tools/main.py, git.py The solution maintains existing behavior on Unix-like systems while providing Windows compatibility through platform detection. Co-Authored-By: João --- src/crewai/cli/evaluate_crew.py | 4 +- src/crewai/cli/git.py | 8 +- src/crewai/cli/install_crew.py | 4 +- src/crewai/cli/kickoff_flow.py | 4 +- src/crewai/cli/plot_flow.py | 4 +- src/crewai/cli/replay_from_task.py | 4 +- src/crewai/cli/run_crew.py | 4 +- src/crewai/cli/subprocess_utils.py | 60 ++++++++++++ src/crewai/cli/tools/main.py | 8 +- src/crewai/cli/train_crew.py | 4 +- tests/cli/test_subprocess_utils.py | 141 +++++++++++++++++++++++++++++ 11 files changed, 230 insertions(+), 15 deletions(-) create mode 100644 src/crewai/cli/subprocess_utils.py create mode 100644 tests/cli/test_subprocess_utils.py diff --git a/src/crewai/cli/evaluate_crew.py b/src/crewai/cli/evaluate_crew.py index 374f9f27d..ed25d84a2 100644 --- a/src/crewai/cli/evaluate_crew.py +++ b/src/crewai/cli/evaluate_crew.py @@ -2,6 +2,8 @@ import subprocess import click +from crewai.cli.subprocess_utils import run_command + def evaluate_crew(n_iterations: int, model: str) -> None: """ @@ -17,7 +19,7 @@ def evaluate_crew(n_iterations: int, model: str) -> None: if n_iterations <= 0: raise ValueError("The number of iterations must be a positive integer.") - result = subprocess.run(command, capture_output=False, text=True, check=True) + result = run_command(command, capture_output=False, text=True, check=True) if result.stderr: click.echo(result.stderr, err=True) diff --git a/src/crewai/cli/git.py b/src/crewai/cli/git.py index 58836e733..61c872cd5 100644 --- a/src/crewai/cli/git.py +++ b/src/crewai/cli/git.py @@ -1,6 +1,8 @@ import subprocess from functools import lru_cache +from crewai.cli.subprocess_utils import run_command + class Repository: def __init__(self, path="."): @@ -17,7 +19,7 @@ class Repository: def is_git_installed(self) -> bool: """Check if Git is installed and available in the system.""" try: - subprocess.run( + run_command( ["git", "--version"], capture_output=True, check=True, text=True ) return True @@ -26,7 +28,7 @@ class Repository: def fetch(self) -> None: """Fetch latest updates from the remote.""" - subprocess.run(["git", "fetch"], cwd=self.path, check=True) + run_command(["git", "fetch"], cwd=self.path, check=True) def status(self) -> str: """Get the git status in porcelain format.""" @@ -70,7 +72,7 @@ class Repository: def origin_url(self) -> str | None: """Get the Git repository's remote URL.""" try: - result = subprocess.run( + result = run_command( ["git", "remote", "get-url", "origin"], cwd=self.path, capture_output=True, diff --git a/src/crewai/cli/install_crew.py b/src/crewai/cli/install_crew.py index bd0f35879..649da342d 100644 --- a/src/crewai/cli/install_crew.py +++ b/src/crewai/cli/install_crew.py @@ -2,6 +2,8 @@ import subprocess import click +from crewai.cli.subprocess_utils import run_command + # Be mindful about changing this. # on some environments we don't use this command but instead uv sync directly @@ -13,7 +15,7 @@ def install_crew(proxy_options: list[str]) -> None: """ try: command = ["uv", "sync"] + proxy_options - subprocess.run(command, check=True, capture_output=False, text=True) + run_command(command, check=True, capture_output=False, text=True) except subprocess.CalledProcessError as e: click.echo(f"An error occurred while running the crew: {e}", err=True) diff --git a/src/crewai/cli/kickoff_flow.py b/src/crewai/cli/kickoff_flow.py index 2123a6c15..4bb622bdd 100644 --- a/src/crewai/cli/kickoff_flow.py +++ b/src/crewai/cli/kickoff_flow.py @@ -2,6 +2,8 @@ import subprocess import click +from crewai.cli.subprocess_utils import run_command + def kickoff_flow() -> None: """ @@ -10,7 +12,7 @@ def kickoff_flow() -> None: command = ["uv", "run", "kickoff"] try: - result = subprocess.run(command, capture_output=False, text=True, check=True) + result = run_command(command, capture_output=False, text=True, check=True) if result.stderr: click.echo(result.stderr, err=True) diff --git a/src/crewai/cli/plot_flow.py b/src/crewai/cli/plot_flow.py index 848c55d69..01a841009 100644 --- a/src/crewai/cli/plot_flow.py +++ b/src/crewai/cli/plot_flow.py @@ -2,6 +2,8 @@ import subprocess import click +from crewai.cli.subprocess_utils import run_command + def plot_flow() -> None: """ @@ -10,7 +12,7 @@ def plot_flow() -> None: command = ["uv", "run", "plot"] try: - result = subprocess.run(command, capture_output=False, text=True, check=True) + result = run_command(command, capture_output=False, text=True, check=True) if result.stderr: click.echo(result.stderr, err=True) diff --git a/src/crewai/cli/replay_from_task.py b/src/crewai/cli/replay_from_task.py index 7e34c3394..20a29e6ae 100644 --- a/src/crewai/cli/replay_from_task.py +++ b/src/crewai/cli/replay_from_task.py @@ -2,6 +2,8 @@ import subprocess import click +from crewai.cli.subprocess_utils import run_command + def replay_task_command(task_id: str) -> None: """ @@ -13,7 +15,7 @@ def replay_task_command(task_id: str) -> None: command = ["uv", "run", "replay", task_id] try: - result = subprocess.run(command, capture_output=False, text=True, check=True) + result = run_command(command, capture_output=False, text=True, check=True) if result.stderr: click.echo(result.stderr, err=True) diff --git a/src/crewai/cli/run_crew.py b/src/crewai/cli/run_crew.py index 62241a4b5..f5c58f306 100644 --- a/src/crewai/cli/run_crew.py +++ b/src/crewai/cli/run_crew.py @@ -1,12 +1,12 @@ import subprocess from enum import Enum -from typing import List, Optional import click from packaging import version from crewai.cli.utils import read_toml from crewai.cli.version import get_crewai_version +from crewai.cli.subprocess_utils import run_command class CrewType(Enum): @@ -57,7 +57,7 @@ def execute_command(crew_type: CrewType) -> None: command = ["uv", "run", "kickoff" if crew_type == CrewType.FLOW else "run_crew"] try: - subprocess.run(command, capture_output=False, text=True, check=True) + run_command(command, capture_output=False, text=True, check=True) except subprocess.CalledProcessError as e: handle_error(e, crew_type) diff --git a/src/crewai/cli/subprocess_utils.py b/src/crewai/cli/subprocess_utils.py new file mode 100644 index 000000000..b56d0ab12 --- /dev/null +++ b/src/crewai/cli/subprocess_utils.py @@ -0,0 +1,60 @@ +import platform +import subprocess +from typing import Any + + +def run_command( + command: list[str], + capture_output: bool = False, + text: bool = True, + check: bool = True, + cwd: str | None = None, + env: dict[str, str] | None = None, + **kwargs: Any +) -> subprocess.CompletedProcess: + """ + Cross-platform subprocess execution with Windows compatibility. + + On Windows, uses shell=True to avoid permission issues with restrictive + security policies. On other platforms, uses the standard approach. + + Args: + command: List of command arguments + capture_output: Whether to capture stdout/stderr + text: Whether to use text mode + check: Whether to raise CalledProcessError on non-zero exit + cwd: Working directory + env: Environment variables + **kwargs: Additional subprocess.run arguments + + Returns: + CompletedProcess instance + + Raises: + subprocess.CalledProcessError: If check=True and command fails + """ + if platform.system() == "Windows": + if isinstance(command, list): + command_str = subprocess.list2cmdline(command) + else: + command_str = command + + return subprocess.run( + command_str, + shell=True, + capture_output=capture_output, + text=text, + check=check, + cwd=cwd, + env=env, + **kwargs + ) + return subprocess.run( + command, + capture_output=capture_output, + text=text, + check=check, + cwd=cwd, + env=env, + **kwargs + ) diff --git a/src/crewai/cli/tools/main.py b/src/crewai/cli/tools/main.py index 25cf89ee8..3a58d498b 100644 --- a/src/crewai/cli/tools/main.py +++ b/src/crewai/cli/tools/main.py @@ -1,6 +1,5 @@ import base64 import os -import subprocess import tempfile from pathlib import Path from typing import Any @@ -11,6 +10,7 @@ from rich.console import Console from crewai.cli import git from crewai.cli.command import BaseCommand, PlusAPIMixin from crewai.cli.config import Settings +from crewai.cli.subprocess_utils import run_command from crewai.cli.utils import ( extract_available_exports, get_project_description, @@ -56,7 +56,7 @@ class ToolCommand(BaseCommand, PlusAPIMixin): os.chdir(project_root) try: self.login() - subprocess.run(["git", "init"], check=True) + run_command(["git", "init"], check=True) console.print( f"[green]Created custom tool [bold]{folder_name}[/bold]. Run [bold]cd {project_root}[/bold] to start working.[/green]" ) @@ -94,7 +94,7 @@ class ToolCommand(BaseCommand, PlusAPIMixin): self._print_current_organization() with tempfile.TemporaryDirectory() as temp_build_dir: - subprocess.run( + run_command( ["uv", "build", "--sdist", "--out-dir", temp_build_dir], check=True, capture_output=False, @@ -196,7 +196,7 @@ class ToolCommand(BaseCommand, PlusAPIMixin): else: add_package_command.extend(["--index", index, tool_handle]) - add_package_result = subprocess.run( + add_package_result = run_command( add_package_command, capture_output=False, env=self._build_env_with_credentials(repository_handle), diff --git a/src/crewai/cli/train_crew.py b/src/crewai/cli/train_crew.py index 14a5e1a06..7ca2012b1 100644 --- a/src/crewai/cli/train_crew.py +++ b/src/crewai/cli/train_crew.py @@ -2,6 +2,8 @@ import subprocess import click +from crewai.cli.subprocess_utils import run_command + def train_crew(n_iterations: int, filename: str) -> None: """ @@ -19,7 +21,7 @@ def train_crew(n_iterations: int, filename: str) -> None: if not filename.endswith(".pkl"): raise ValueError("The filename must not end with .pkl") - result = subprocess.run(command, capture_output=False, text=True, check=True) + result = run_command(command, capture_output=False, text=True, check=True) if result.stderr: click.echo(result.stderr, err=True) diff --git a/tests/cli/test_subprocess_utils.py b/tests/cli/test_subprocess_utils.py new file mode 100644 index 000000000..2ce123297 --- /dev/null +++ b/tests/cli/test_subprocess_utils.py @@ -0,0 +1,141 @@ +import platform +import subprocess +from unittest import mock + +import pytest + +from crewai.cli.subprocess_utils import run_command + + +class TestRunCommand: + """Test the cross-platform subprocess utility.""" + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_windows_uses_shell_true(self, mock_subprocess_run, mock_platform): + """Test that Windows uses shell=True with proper command conversion.""" + mock_platform.return_value = "Windows" + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args="uv run test", returncode=0 + ) + + command = ["uv", "run", "test"] + run_command(command) + + mock_subprocess_run.assert_called_once() + call_args = mock_subprocess_run.call_args + + assert call_args[1]["shell"] is True + assert isinstance(call_args[0][0], str) + assert "uv run test" in call_args[0][0] + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_unix_uses_shell_false(self, mock_subprocess_run, mock_platform): + """Test that Unix-like systems use shell=False with list commands.""" + mock_platform.return_value = "Linux" + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=["uv", "run", "test"], returncode=0 + ) + + command = ["uv", "run", "test"] + run_command(command) + + mock_subprocess_run.assert_called_once() + call_args = mock_subprocess_run.call_args + + assert call_args[1].get("shell", False) is False + assert call_args[0][0] == command + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_windows_command_escaping(self, mock_subprocess_run, mock_platform): + """Test that Windows properly escapes command arguments.""" + mock_platform.return_value = "Windows" + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args="test", returncode=0 + ) + + command = ["echo", "hello world", "test&special"] + run_command(command) + + mock_subprocess_run.assert_called_once() + call_args = mock_subprocess_run.call_args + command_str = call_args[0][0] + + assert '"hello world"' in command_str or "'hello world'" in command_str + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_error_handling_preserved(self, mock_subprocess_run, mock_platform): + """Test that CalledProcessError is properly raised.""" + mock_platform.return_value = "Windows" + mock_subprocess_run.side_effect = subprocess.CalledProcessError(1, "test") + + with pytest.raises(subprocess.CalledProcessError): + run_command(["test"], check=True) + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_all_parameters_passed_through(self, mock_subprocess_run, mock_platform): + """Test that all subprocess parameters are properly passed through.""" + mock_platform.return_value = "Linux" + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=["test"], returncode=0 + ) + + run_command( + ["test"], + capture_output=True, + text=False, + check=False, + cwd="/tmp", + env={"TEST": "value"}, + timeout=30 + ) + + mock_subprocess_run.assert_called_once() + call_args = mock_subprocess_run.call_args + + assert call_args[1]["capture_output"] is True + assert call_args[1]["text"] is False + assert call_args[1]["check"] is False + assert call_args[1]["cwd"] == "/tmp" + assert call_args[1]["env"] == {"TEST": "value"} + assert call_args[1]["timeout"] == 30 + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_macos_uses_shell_false(self, mock_subprocess_run, mock_platform): + """Test that macOS uses shell=False with list commands.""" + mock_platform.return_value = "Darwin" + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=["uv", "run", "test"], returncode=0 + ) + + command = ["uv", "run", "test"] + run_command(command) + + mock_subprocess_run.assert_called_once() + call_args = mock_subprocess_run.call_args + + assert call_args[1].get("shell", False) is False + assert call_args[0][0] == command + + @mock.patch("platform.system") + @mock.patch("subprocess.run") + def test_windows_string_command_passthrough(self, mock_subprocess_run, mock_platform): + """Test that Windows passes through string commands unchanged.""" + mock_platform.return_value = "Windows" + mock_subprocess_run.return_value = subprocess.CompletedProcess( + args="test command", returncode=0 + ) + + command_str = "test command with spaces" + run_command(command_str) + + mock_subprocess_run.assert_called_once() + call_args = mock_subprocess_run.call_args + + assert call_args[0][0] == command_str + assert call_args[1]["shell"] is True