Skip to content

don't set shell=True on subprocess calls #7471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
radoering merged 1 commit into from
Feb 13, 2023
Merged

don't set shell=True on subprocess calls #7471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions src/poetry/utils/_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,11 @@ def to_str(string: str) -> str:
return decode(string)


def list_to_shell_command(cmd: list[str]) -> str:
return " ".join(
f'"{token}"' if " " in token and token[0] not in {"'", '"'} else token
for token in cmd
)


__all__ = [
"WINDOWS",
"cached_property",
"decode",
"encode",
"list_to_shell_command",
"metadata",
"to_str",
"tomllib",
Expand Down
55 changes: 14 additions & 41 deletions src/poetry/utils/env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
from poetry.utils._compat import WINDOWS
from poetry.utils._compat import decode
from poetry.utils._compat import encode
from poetry.utils._compat import list_to_shell_command
from poetry.utils._compat import metadata
from poetry.utils.helpers import get_real_windows_path
from poetry.utils.helpers import is_dir_writable
Expand Down Expand Up @@ -171,9 +170,9 @@ def _version_nodot(version):
"""

GET_PYTHON_VERSION_ONELINER = (
"\"import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))\""
"import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))"
)
GET_ENV_PATH_ONELINER = '"import sys; print(sys.prefix)"'
GET_ENV_PATH_ONELINER = "import sys; print(sys.prefix)"

GET_SYS_PATH = """\
import json
Expand Down Expand Up @@ -522,10 +521,7 @@ def _full_python_path(python: str) -> str:
try:
executable = decode(
subprocess.check_output(
list_to_shell_command(
[python, "-c", '"import sys; print(sys.executable)"']
),
shell=True,
[python, "-c", "import sys; print(sys.executable)"],
).strip()
)
except CalledProcessError as e:
Expand Down Expand Up @@ -572,10 +568,7 @@ def get_python_version(
if executable:
python_patch = decode(
subprocess.check_output(
list_to_shell_command(
[executable, "-c", GET_PYTHON_VERSION_ONELINER]
),
shell=True,
[executable, "-c", GET_PYTHON_VERSION_ONELINER],
).strip()
)

Expand Down Expand Up @@ -603,8 +596,7 @@ def activate(self, python: str) -> Env:
try:
python_version_string = decode(
subprocess.check_output(
list_to_shell_command([python, "-c", GET_PYTHON_VERSION_ONELINER]),
shell=True,
[python, "-c", GET_PYTHON_VERSION_ONELINER],
)
)
except CalledProcessError as e:
Expand Down Expand Up @@ -799,8 +791,7 @@ def remove(self, python: str) -> Env:
try:
env_dir = decode(
subprocess.check_output(
list_to_shell_command([python, "-c", GET_ENV_PATH_ONELINER]),
shell=True,
[python, "-c", GET_ENV_PATH_ONELINER],
)
).strip("\n")
env_name = Path(env_dir).name
Expand Down Expand Up @@ -859,8 +850,7 @@ def remove(self, python: str) -> Env:
try:
python_version_string = decode(
subprocess.check_output(
list_to_shell_command([python, "-c", GET_PYTHON_VERSION_ONELINER]),
shell=True,
[python, "-c", GET_PYTHON_VERSION_ONELINER],
)
)
except CalledProcessError as e:
Expand Down Expand Up @@ -935,10 +925,7 @@ def create_venv(
if executable:
python_patch = decode(
subprocess.check_output(
list_to_shell_command(
[executable, "-c", GET_PYTHON_VERSION_ONELINER]
),
shell=True,
[executable, "-c", GET_PYTHON_VERSION_ONELINER],
).strip()
)
python_minor = ".".join(python_patch.split(".")[:2])
Expand Down Expand Up @@ -985,11 +972,8 @@ def create_venv(
try:
python_patch = decode(
subprocess.check_output(
list_to_shell_command(
[python, "-c", GET_PYTHON_VERSION_ONELINER]
),
[python, "-c", GET_PYTHON_VERSION_ONELINER],
stderr=subprocess.STDOUT,
shell=True,
).strip()
)
except CalledProcessError:
Expand Down Expand Up @@ -1531,31 +1515,20 @@ def _run(self, cmd: list[str], **kwargs: Any) -> int | str:
env = kwargs.pop("env", dict(os.environ))

try:
if self._is_windows:
kwargs["shell"] = True

command: str | list[str]
if kwargs.get("shell", False):
command = list_to_shell_command(cmd)
else:
command = cmd

if input_:
output = subprocess.run(
command,
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
input=encode(input_),
check=True,
**kwargs,
).stdout
elif call:
return subprocess.call(
command, stderr=subprocess.STDOUT, env=env, **kwargs
)
return subprocess.call(cmd, stderr=subprocess.STDOUT, env=env, **kwargs)
else:
output = subprocess.check_output(
command, stderr=subprocess.STDOUT, env=env, **kwargs
cmd, stderr=subprocess.STDOUT, env=env, **kwargs
)
except CalledProcessError as e:
raise EnvCommandError(e, input=input_)
Expand All @@ -1570,7 +1543,7 @@ def execute(self, bin: str, *args: str, **kwargs: Any) -> int:
return os.execvpe(command[0], command, env=env)

kwargs["shell"] = True
exe = subprocess.Popen([command[0]] + command[1:], env=env, **kwargs)
exe = subprocess.Popen(command, env=env, **kwargs)
exe.communicate()
return exe.returncode

Expand Down Expand Up @@ -1909,7 +1882,7 @@ def execute(self, bin: str, *args: str, **kwargs: Any) -> int:
if not self._is_windows:
return os.execvpe(command[0], command, env=env)

exe = subprocess.Popen([command[0]] + command[1:], env=env, **kwargs)
exe = subprocess.Popen(command, env=env, **kwargs)
exe.communicate()

return exe.returncode
Expand Down
19 changes: 10 additions & 9 deletions tests/console/commands/env/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
if TYPE_CHECKING:
from collections.abc import Callable

from poetry.core.version.pep440.version import PEP440Version

VERSION_3_7_1 = Version.parse("3.7.1")


Expand All @@ -20,16 +18,19 @@ def build_venv(path: Path | str, **_: Any) -> None:


def check_output_wrapper(
version: PEP440Version = VERSION_3_7_1,
) -> Callable[[str, Any, Any], str]:
def check_output(cmd: str, *_: Any, **__: Any) -> str:
if "sys.version_info[:3]" in cmd:
version: Version = VERSION_3_7_1,
) -> Callable[[list[str], Any, Any], str]:
def check_output(cmd: list[str], *args: Any, **kwargs: Any) -> str:
# cmd is a list, like ["python", "-c", "do stuff"]
python_cmd = cmd[2]
if "sys.version_info[:3]" in python_cmd:
return version.text
elif "sys.version_info[:2]" in cmd:
elif "sys.version_info[:2]" in python_cmd:
return f"{version.major}.{version.minor}"
elif '-c "import sys; print(sys.executable)"' in cmd:
return f"/usr/bin/{cmd.split()[0]}"
elif "import sys; print(sys.executable)" in python_cmd:
return f"/usr/bin/{cmd[0]}"
else:
assert "import sys; print(sys.prefix)" in python_cmd
return str(Path("/prefix"))

return check_output
23 changes: 12 additions & 11 deletions tests/utils/test_env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def test_virtualenvs_with_spaces_in_their_path_work_as_expected(

venv = VirtualEnv(venv_path)

assert venv.run("python", "-V", shell=True).startswith("Python")
assert venv.run("python", "-V").startswith("Python")


@pytest.mark.skipif(sys.platform != "darwin", reason="requires darwin")
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_env_commands_with_spaces_in_their_arg_work_as_expected(
venv_path = Path(tmp_dir) / "Virtual Env"
manager.build_venv(str(venv_path))
venv = VirtualEnv(venv_path)
assert venv.run("python", venv.pip, "--version", shell=True).startswith(
assert venv.run("python", venv.pip, "--version").startswith(
f"pip {venv.pip_version} from "
)

Expand All @@ -135,9 +135,7 @@ def test_env_shell_commands_with_stdinput_in_their_arg_work_as_expected(
venv_path = Path(tmp_dir) / "Virtual Env"
manager.build_venv(str(venv_path))
venv = VirtualEnv(venv_path)
run_output_path = Path(
venv.run("python", "-", input_=GET_BASE_PREFIX, shell=True).strip()
)
run_output_path = Path(venv.run("python", "-", input_=GET_BASE_PREFIX).strip())
venv_base_prefix_path = Path(str(venv.get_base_prefix()))
assert run_output_path.resolve() == venv_base_prefix_path.resolve()

Expand Down Expand Up @@ -189,15 +187,18 @@ def build_venv(path: Path | str, **__: Any) -> None:

def check_output_wrapper(
version: Version = VERSION_3_7_1,
) -> Callable[[str, Any, Any], str]:
def check_output(cmd: str, *args: Any, **kwargs: Any) -> str:
if "sys.version_info[:3]" in cmd:
) -> Callable[[list[str], Any, Any], str]:
def check_output(cmd: list[str], *args: Any, **kwargs: Any) -> str:
# cmd is a list, like ["python", "-c", "do stuff"]
python_cmd = cmd[2]
if "sys.version_info[:3]" in python_cmd:
return version.text
elif "sys.version_info[:2]" in cmd:
elif "sys.version_info[:2]" in python_cmd:
return f"{version.major}.{version.minor}"
elif '-c "import sys; print(sys.executable)"' in cmd:
return f"/usr/bin/{cmd.split()[0]}"
elif "import sys; print(sys.executable)" in python_cmd:
return f"/usr/bin/{cmd[0]}"
else:
assert "import sys; print(sys.prefix)" in python_cmd
return str(Path("/prefix"))

return check_output
Expand Down