Skip to content

Commit

Permalink
don't set shell=True on subprocess calls
Browse files Browse the repository at this point in the history
  • Loading branch information
@dimbleby @radoering
dimbleby authored and radoering committed Feb 13, 2023
1 parent 8558f0c commit 6f0acbf
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 69 deletions.
8 changes: 0 additions & 8 deletions src/poetry/utils/_compat.py
View file
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
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
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
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

0 comments on commit 6f0acbf

Please sign in to comment.