mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-01-10 00:28:31 +00:00
fix: add Windows-compatible subprocess execution for CLI commands
- 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 <joao@crewai.com>
This commit is contained in:
@@ -2,6 +2,8 @@ import subprocess
|
|||||||
|
|
||||||
import click
|
import click
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
def evaluate_crew(n_iterations: int, model: str) -> None:
|
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:
|
if n_iterations <= 0:
|
||||||
raise ValueError("The number of iterations must be a positive integer.")
|
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:
|
if result.stderr:
|
||||||
click.echo(result.stderr, err=True)
|
click.echo(result.stderr, err=True)
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import subprocess
|
import subprocess
|
||||||
from functools import lru_cache
|
from functools import lru_cache
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
class Repository:
|
class Repository:
|
||||||
def __init__(self, path="."):
|
def __init__(self, path="."):
|
||||||
@@ -17,7 +19,7 @@ class Repository:
|
|||||||
def is_git_installed(self) -> bool:
|
def is_git_installed(self) -> bool:
|
||||||
"""Check if Git is installed and available in the system."""
|
"""Check if Git is installed and available in the system."""
|
||||||
try:
|
try:
|
||||||
subprocess.run(
|
run_command(
|
||||||
["git", "--version"], capture_output=True, check=True, text=True
|
["git", "--version"], capture_output=True, check=True, text=True
|
||||||
)
|
)
|
||||||
return True
|
return True
|
||||||
@@ -26,7 +28,7 @@ class Repository:
|
|||||||
|
|
||||||
def fetch(self) -> None:
|
def fetch(self) -> None:
|
||||||
"""Fetch latest updates from the remote."""
|
"""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:
|
def status(self) -> str:
|
||||||
"""Get the git status in porcelain format."""
|
"""Get the git status in porcelain format."""
|
||||||
@@ -70,7 +72,7 @@ class Repository:
|
|||||||
def origin_url(self) -> str | None:
|
def origin_url(self) -> str | None:
|
||||||
"""Get the Git repository's remote URL."""
|
"""Get the Git repository's remote URL."""
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = run_command(
|
||||||
["git", "remote", "get-url", "origin"],
|
["git", "remote", "get-url", "origin"],
|
||||||
cwd=self.path,
|
cwd=self.path,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ import subprocess
|
|||||||
|
|
||||||
import click
|
import click
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
# Be mindful about changing this.
|
# Be mindful about changing this.
|
||||||
# on some environments we don't use this command but instead uv sync directly
|
# 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:
|
try:
|
||||||
command = ["uv", "sync"] + proxy_options
|
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:
|
except subprocess.CalledProcessError as e:
|
||||||
click.echo(f"An error occurred while running the crew: {e}", err=True)
|
click.echo(f"An error occurred while running the crew: {e}", err=True)
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ import subprocess
|
|||||||
|
|
||||||
import click
|
import click
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
def kickoff_flow() -> None:
|
def kickoff_flow() -> None:
|
||||||
"""
|
"""
|
||||||
@@ -10,7 +12,7 @@ def kickoff_flow() -> None:
|
|||||||
command = ["uv", "run", "kickoff"]
|
command = ["uv", "run", "kickoff"]
|
||||||
|
|
||||||
try:
|
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:
|
if result.stderr:
|
||||||
click.echo(result.stderr, err=True)
|
click.echo(result.stderr, err=True)
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ import subprocess
|
|||||||
|
|
||||||
import click
|
import click
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
def plot_flow() -> None:
|
def plot_flow() -> None:
|
||||||
"""
|
"""
|
||||||
@@ -10,7 +12,7 @@ def plot_flow() -> None:
|
|||||||
command = ["uv", "run", "plot"]
|
command = ["uv", "run", "plot"]
|
||||||
|
|
||||||
try:
|
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:
|
if result.stderr:
|
||||||
click.echo(result.stderr, err=True)
|
click.echo(result.stderr, err=True)
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ import subprocess
|
|||||||
|
|
||||||
import click
|
import click
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
def replay_task_command(task_id: str) -> None:
|
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]
|
command = ["uv", "run", "replay", task_id]
|
||||||
|
|
||||||
try:
|
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:
|
if result.stderr:
|
||||||
click.echo(result.stderr, err=True)
|
click.echo(result.stderr, err=True)
|
||||||
|
|
||||||
|
|||||||
@@ -1,12 +1,12 @@
|
|||||||
import subprocess
|
import subprocess
|
||||||
from enum import Enum
|
from enum import Enum
|
||||||
from typing import List, Optional
|
|
||||||
|
|
||||||
import click
|
import click
|
||||||
from packaging import version
|
from packaging import version
|
||||||
|
|
||||||
from crewai.cli.utils import read_toml
|
from crewai.cli.utils import read_toml
|
||||||
from crewai.cli.version import get_crewai_version
|
from crewai.cli.version import get_crewai_version
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
class CrewType(Enum):
|
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"]
|
command = ["uv", "run", "kickoff" if crew_type == CrewType.FLOW else "run_crew"]
|
||||||
|
|
||||||
try:
|
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:
|
except subprocess.CalledProcessError as e:
|
||||||
handle_error(e, crew_type)
|
handle_error(e, crew_type)
|
||||||
|
|||||||
60
src/crewai/cli/subprocess_utils.py
Normal file
60
src/crewai/cli/subprocess_utils.py
Normal file
@@ -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
|
||||||
|
)
|
||||||
@@ -1,6 +1,5 @@
|
|||||||
import base64
|
import base64
|
||||||
import os
|
import os
|
||||||
import subprocess
|
|
||||||
import tempfile
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
@@ -11,6 +10,7 @@ from rich.console import Console
|
|||||||
from crewai.cli import git
|
from crewai.cli import git
|
||||||
from crewai.cli.command import BaseCommand, PlusAPIMixin
|
from crewai.cli.command import BaseCommand, PlusAPIMixin
|
||||||
from crewai.cli.config import Settings
|
from crewai.cli.config import Settings
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
from crewai.cli.utils import (
|
from crewai.cli.utils import (
|
||||||
extract_available_exports,
|
extract_available_exports,
|
||||||
get_project_description,
|
get_project_description,
|
||||||
@@ -56,7 +56,7 @@ class ToolCommand(BaseCommand, PlusAPIMixin):
|
|||||||
os.chdir(project_root)
|
os.chdir(project_root)
|
||||||
try:
|
try:
|
||||||
self.login()
|
self.login()
|
||||||
subprocess.run(["git", "init"], check=True)
|
run_command(["git", "init"], check=True)
|
||||||
console.print(
|
console.print(
|
||||||
f"[green]Created custom tool [bold]{folder_name}[/bold]. Run [bold]cd {project_root}[/bold] to start working.[/green]"
|
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()
|
self._print_current_organization()
|
||||||
|
|
||||||
with tempfile.TemporaryDirectory() as temp_build_dir:
|
with tempfile.TemporaryDirectory() as temp_build_dir:
|
||||||
subprocess.run(
|
run_command(
|
||||||
["uv", "build", "--sdist", "--out-dir", temp_build_dir],
|
["uv", "build", "--sdist", "--out-dir", temp_build_dir],
|
||||||
check=True,
|
check=True,
|
||||||
capture_output=False,
|
capture_output=False,
|
||||||
@@ -196,7 +196,7 @@ class ToolCommand(BaseCommand, PlusAPIMixin):
|
|||||||
else:
|
else:
|
||||||
add_package_command.extend(["--index", index, tool_handle])
|
add_package_command.extend(["--index", index, tool_handle])
|
||||||
|
|
||||||
add_package_result = subprocess.run(
|
add_package_result = run_command(
|
||||||
add_package_command,
|
add_package_command,
|
||||||
capture_output=False,
|
capture_output=False,
|
||||||
env=self._build_env_with_credentials(repository_handle),
|
env=self._build_env_with_credentials(repository_handle),
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ import subprocess
|
|||||||
|
|
||||||
import click
|
import click
|
||||||
|
|
||||||
|
from crewai.cli.subprocess_utils import run_command
|
||||||
|
|
||||||
|
|
||||||
def train_crew(n_iterations: int, filename: str) -> None:
|
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"):
|
if not filename.endswith(".pkl"):
|
||||||
raise ValueError("The filename must not end with .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:
|
if result.stderr:
|
||||||
click.echo(result.stderr, err=True)
|
click.echo(result.stderr, err=True)
|
||||||
|
|||||||
141
tests/cli/test_subprocess_utils.py
Normal file
141
tests/cli/test_subprocess_utils.py
Normal file
@@ -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
|
||||||
Reference in New Issue
Block a user