From 4b6acb3d58237312e57646ff0b6870715e8958b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Thu, 17 Oct 2024 00:36:13 +0200 Subject: [PATCH] Skip $PATH entries we cannot check rather than dying with PermissionError On Fedora CI, I used runuser to get rid of root privileges before testing virtualenv. The invocation I used did not clear the root's $PATH and as a result, one of the entries in $PATH was /root/.local/bin. But /root/ does not allow other users to browse (execute, read). That lead to an unhanded PermissionError. When this happens, the $PATH entry is now skipped as if it did not exist. As there might be other reasons we cannot check if the $PATH entry exists, I decided to suppress all OSError types. For clarity, here is the Traceback I was getting: # runuser testuser -c 'virtualenv --with-traceback --python pypy3.10 venv' Traceback (most recent call last): File "/usr/bin/virtualenv", line 8, in sys.exit(run_with_catch()) ~~~~~~~~~~~~~~^^ File "/usr/lib/python3.13/site-packages/virtualenv/__main__.py", line 56, in run_with_catch run(args, options, env) ~~~^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.13/site-packages/virtualenv/__main__.py", line 18, in run session = cli_run(args, options, env) File "/usr/lib/python3.13/site-packages/virtualenv/run/__init__.py", line 31, in cli_run of_session = session_via_cli(args, options, setup_logging, env) File "/usr/lib/python3.13/site-packages/virtualenv/run/__init__.py", line 49, in session_via_cli parser, elements = build_parser(args, options, setup_logging, env) ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.13/site-packages/virtualenv/run/__init__.py", line 77, in build_parser parser._interpreter = interpreter = discover.interpreter # noqa: SLF001 ^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.13/site-packages/virtualenv/discovery/discover.py", line 41, in interpreter self._interpreter = self.run() ~~~~~~~~^^ File "/usr/lib/python3.13/site-packages/virtualenv/discovery/builtin.py", line 58, in run result = get_interpreter(python_spec, self.try_first_with, self.app_data, self._env) File "/usr/lib/python3.13/site-packages/virtualenv/discovery/builtin.py", line 75, in get_interpreter for interpreter, impl_must_match in propose_interpreters(spec, try_first_with, app_data, env): ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.13/site-packages/virtualenv/discovery/builtin.py", line 147, in propose_interpreters for pos, path in enumerate(get_paths(env)): ~~~~~~~~~^^^^^^^^^^^^^^^^ File "/usr/lib/python3.13/site-packages/virtualenv/discovery/builtin.py", line 169, in get_paths if p.exists(): ~~~~~~~~^^ File "/usr/lib64/python3.13/pathlib/_abc.py", line 450, in exists self.stat(follow_symlinks=follow_symlinks) ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.13/pathlib/_local.py", line 515, in stat return os.stat(self, follow_symlinks=follow_symlinks) ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PermissionError: [Errno 13] Permission denied: '/root/.local/bin' --- docs/changelog/2782.bugfix.rst | 1 + src/virtualenv/discovery/builtin.py | 6 ++++-- tests/unit/discovery/test_discovery.py | 8 ++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 docs/changelog/2782.bugfix.rst diff --git a/docs/changelog/2782.bugfix.rst b/docs/changelog/2782.bugfix.rst new file mode 100644 index 000000000..6aa2b9cfb --- /dev/null +++ b/docs/changelog/2782.bugfix.rst @@ -0,0 +1 @@ +When a ``$PATH`` entry cannot be checked for existence, skip it instead of terminating - by :user:`hroncok`. \ No newline at end of file diff --git a/src/virtualenv/discovery/builtin.py b/src/virtualenv/discovery/builtin.py index ae0612b36..d2f1cf433 100644 --- a/src/virtualenv/discovery/builtin.py +++ b/src/virtualenv/discovery/builtin.py @@ -3,6 +3,7 @@ import logging import os import sys +from contextlib import suppress from pathlib import Path from typing import TYPE_CHECKING, Callable @@ -166,8 +167,9 @@ def get_paths(env: Mapping[str, str]) -> Generator[Path, None, None]: path = os.defpath if path: for p in map(Path, path.split(os.pathsep)): - if p.exists(): - yield p + with suppress(OSError): + if p.exists(): + yield p class LazyPathDump: diff --git a/tests/unit/discovery/test_discovery.py b/tests/unit/discovery/test_discovery.py index 0e11c2d90..680131e7d 100644 --- a/tests/unit/discovery/test_discovery.py +++ b/tests/unit/discovery/test_discovery.py @@ -59,6 +59,14 @@ def test_discovery_via_path_not_found(tmp_path, monkeypatch): assert interpreter is None +def test_discovery_via_path_in_nonbrowseable_directory(tmp_path, monkeypatch): + bad_perm = tmp_path / "bad_perm" + bad_perm.mkdir(mode=0o000) + monkeypatch.setenv("PATH", str(bad_perm / "bin")) + interpreter = get_interpreter(uuid4().hex, []) + assert interpreter is None + + def test_relative_path(session_app_data, monkeypatch): sys_executable = Path(PythonInfo.current_system(app_data=session_app_data).system_executable) cwd = sys_executable.parents[1]