diff options
| -rw-r--r-- | docs/changelog/1127.feature.rst | 1 | ||||
| -rw-r--r-- | docs/config.rst | 18 | ||||
| -rw-r--r-- | docs/user_guide.rst | 2 | ||||
| -rw-r--r-- | setup.cfg | 4 | ||||
| -rw-r--r-- | src/tox/execute/local_sub_process/__init__.py | 15 | ||||
| -rw-r--r-- | src/tox/execute/request.py | 13 | ||||
| -rw-r--r-- | src/tox/tox_env/api.py | 14 | ||||
| -rw-r--r-- | src/tox/util/spinner.py | 2 | ||||
| -rw-r--r-- | tests/conftest.py | 17 | ||||
| -rw-r--r-- | tests/execute/local_subprocess/test_local_subprocess.py | 23 | ||||
| -rw-r--r-- | tests/execute/test_request.py | 14 | ||||
| -rw-r--r-- | tests/tox_env/test_tox_env_api.py | 14 | ||||
| -rw-r--r-- | whitelist.txt | 5 |
13 files changed, 124 insertions, 18 deletions
diff --git a/docs/changelog/1127.feature.rst b/docs/changelog/1127.feature.rst new file mode 100644 index 00000000..adba5ed8 --- /dev/null +++ b/docs/changelog/1127.feature.rst @@ -0,0 +1 @@ +Add support for :ref:`allowlist_externals`, commands not matching error - by :user:`gaborbernat`. diff --git a/docs/config.rst b/docs/config.rst index 296aec77..5ab71a6f 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -257,6 +257,15 @@ Base Always recreate virtual environment if this option is true, otherwise leave it up to tox. +.. conf:: + :keys: allowlist_externals, whitelist_externals + :default: <empty list> + + Each line specifies a command name (in glob-style pattern format) which can be used in the commands section even if + it's located outside of the tox environment. For example: if you use the unix make command for running tests you can list + ``allowlist_externals=make`` or ``allowlist_externals=/usr/bin/make``. If you want to allow all external commands + you can use ``allowlist_externals=*`` which will match all commands (not recommended). + Run ~~~ @@ -504,14 +513,7 @@ Python virtual environment In cases where a command line tool is also installed globally you have to make sure that you use the tool installed in the virtualenv by using ``python -m <command line tool>`` (if supported by the tool) or - ``{env_bin_dir}/<command line tool>``. If you forget to do that you will get a warning like this: - - .. code-block:: - - WARNING: test command found but not installed in testenv - cmd: /path/to/parent/interpreter/bin/<some command> - env: /foo/bar/.tox/python - Maybe you forgot to specify a dependency? See also the allowlist_externals envconfig settin + ``{env_bin_dir}/<command line tool>``. If you forget to do that you will get an error. .. conf:: :keys: always_copy, alwayscopy diff --git a/docs/user_guide.rst b/docs/user_guide.rst index d94af55c..795e7c39 100644 --- a/docs/user_guide.rst +++ b/docs/user_guide.rst @@ -90,7 +90,7 @@ tox roughly follows the following phases: tox will take care of environment isolation for you: it will strip away all operating system environment variables not specified via ``passenv``. Furthermore, it will also alter the ``PATH`` variable so that your commands resolve within the current active tox environment. In general, all executables in the path are available in ``commands``, but -tox will emit a warning if it was not explicitly allowed via ``whitelist_externals``. +tox will error if it was not explicitly allowed via :ref:`allowlist_externals`. Current features ---------------- @@ -72,6 +72,7 @@ testing = covdefaults>=1.2 devpi-client>=5.2 devpi-server>=6.1 + distlib>=0.3.2 filelock>=3 flaky>=3.7 freezegun>=1.1 @@ -144,6 +145,9 @@ ignore_missing_imports = True [mypy-coverage.*] ignore_missing_imports = True +[mypy-distlib.*] +ignore_missing_imports = True + [mypy-flaky.*] ignore_missing_imports = True diff --git a/src/tox/execute/local_sub_process/__init__.py b/src/tox/execute/local_sub_process/__init__.py index 5fb21d4b..7b8177b3 100644 --- a/src/tox/execute/local_sub_process/__init__.py +++ b/src/tox/execute/local_sub_process/__init__.py @@ -1,4 +1,5 @@ """Execute that runs on local file system via subprocess-es""" +import fnmatch import logging import os import shutil @@ -8,6 +9,8 @@ from subprocess import DEVNULL, PIPE, TimeoutExpired from types import TracebackType from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional, Sequence, Tuple, Type +from tox.tox_env.errors import Fail + from ..api import Execute, ExecuteInstance, ExecuteStatus from ..request import ExecuteRequest, StdinSource from ..stream import SyncWrite @@ -176,10 +179,20 @@ class LocalSubProcessExecuteInstance(ExecuteInstance): @property def cmd(self) -> Sequence[str]: if self._cmd is None: - executable = shutil.which(self.request.cmd[0], path=self.request.env["PATH"]) + base = self.request.cmd[0] + executable = shutil.which(base, path=self.request.env["PATH"]) if executable is None: cmd = self.request.cmd # if failed to find leave as it is else: + if self.request.allow is not None: + for allow in self.request.allow: + # 1. allow matches just the original name of the executable + # 2. allow matches the entire resolved path + if fnmatch.fnmatch(self.request.cmd[0], allow) or fnmatch.fnmatch(executable, allow): + break + else: + msg = f"{base} (resolves to {executable})" if base == executable else base + raise Fail(f"{msg} is not allowed, use allowlist_externals to allow it") # else use expanded format cmd = [executable, *self.request.cmd[1:]] self._cmd = cmd diff --git a/src/tox/execute/request.py b/src/tox/execute/request.py index 8cccbf9a..91d2e996 100644 --- a/src/tox/execute/request.py +++ b/src/tox/execute/request.py @@ -2,7 +2,7 @@ import sys from enum import Enum from pathlib import Path -from typing import Dict, List, Sequence, Union +from typing import Dict, List, Optional, Sequence, Union class StdinSource(Enum): @@ -20,7 +20,13 @@ class ExecuteRequest: """Defines a commands execution request""" def __init__( - self, cmd: Sequence[Union[str, Path]], cwd: Path, env: Dict[str, str], stdin: StdinSource, run_id: str + self, + cmd: Sequence[Union[str, Path]], + cwd: Path, + env: Dict[str, str], + stdin: StdinSource, + run_id: str, + allow: Optional[List[str]] = None, ) -> None: """ Create a new execution request. @@ -38,6 +44,9 @@ class ExecuteRequest: self.env = env #: the environment variables to use self.stdin = stdin #: the type of standard input interaction allowed self.run_id = run_id #: an id to identify this run + if allow is not None and "*" in allow: + allow = None # if we allow everything we can just disable the check + self.allow = allow @property def shell_cmd(self) -> str: diff --git a/src/tox/tox_env/api.py b/src/tox/tox_env/api.py index 46709b23..83163df5 100644 --- a/src/tox/tox_env/api.py +++ b/src/tox/tox_env/api.py @@ -128,6 +128,12 @@ class ToxEnv(ABC): default=False, desc="always recreate virtual environment if this option is true, otherwise leave it up to tox", ) + self.conf.add_config( + keys=["allowlist_externals", "whitelist_externals"], + of_type=List[str], + default=[], + desc="external command glob to allow calling", + ) @property def env_dir(self) -> Path: @@ -293,6 +299,12 @@ class ToxEnv(ABC): result = self._make_path() self._env_vars["PATH"] = result + @property + def _allow_externals(self) -> List[str]: + result: List[str] = [f"{i}{os.sep}*" for i in self._paths] + result.extend(i.strip() for i in self.conf["allowlist_externals"]) + return result + def _make_path(self) -> str: values = dict.fromkeys(str(i) for i in self._paths) values.update(dict.fromkeys(os.environ.get("PATH", "").split(os.pathsep))) @@ -338,7 +350,7 @@ class ToxEnv(ABC): cwd = self.core["tox_root"] if show is None: show = self.options.verbosity > 3 - request = ExecuteRequest(cmd, cwd, self._environment_variables, stdin, run_id) + request = ExecuteRequest(cmd, cwd, self._environment_variables, stdin, run_id, allow=self._allow_externals) if _CWD == request.cwd: repr_cwd = "" else: diff --git a/src/tox/util/spinner.py b/src/tox/util/spinner.py index 78edc8fa..ca2dbb06 100644 --- a/src/tox/util/spinner.py +++ b/src/tox/util/spinner.py @@ -93,7 +93,7 @@ class Spinner: self.render_frame() self._stop_spinner = threading.Event() self._spinner_thread = threading.Thread(target=self.render) - self._spinner_thread.setDaemon(True) + self._spinner_thread.daemon = True self._spinner_thread.start() return self diff --git a/tests/conftest.py b/tests/conftest.py index 4dce1fa0..bf1c77d5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,11 +1,14 @@ import os import sys from pathlib import Path -from typing import Callable, List, Optional, Sequence, Tuple +from typing import Callable, Iterator, List, Optional, Sequence, Tuple +from unittest.mock import patch +from uuid import uuid4 import pytest from _pytest.monkeypatch import MonkeyPatch # noqa # cannot import from tox.pytest yet from _pytest.tmpdir import TempPathFactory +from distlib.scripts import ScriptMaker from pytest_mock import MockerFixture from virtualenv import cli_run @@ -108,3 +111,15 @@ def _do_not_share_virtualenv_for_parallel_runs(tmp_path_factory: TempPathFactory seed_env_folder = str(tmp_path_factory.mktemp(f"seed-cache-{worker_id}")) # pragma: no cover args = [seed_env_folder, "--without-pip", "--activators", ""] # pragma: no cover cli_run(args, setup_logging=False) # pragma: no cover + + +@pytest.fixture(scope="session") +def fake_exe_on_path(tmp_path_factory: TempPathFactory) -> Iterator[Path]: + tmp_path = Path(tmp_path_factory.mktemp("a")) + cmd_name = uuid4().hex + maker = ScriptMaker(None, str(tmp_path)) + maker.set_mode = True + maker.variants = {""} + maker.make(f"{cmd_name} = b:c") + with patch.dict(os.environ, {"PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}"}): + yield tmp_path / cmd_name diff --git a/tests/execute/local_subprocess/test_local_subprocess.py b/tests/execute/local_subprocess/test_local_subprocess.py index 4fb50055..d6a9cbba 100644 --- a/tests/execute/local_subprocess/test_local_subprocess.py +++ b/tests/execute/local_subprocess/test_local_subprocess.py @@ -15,8 +15,9 @@ from psutil import AccessDenied from pytest_mock import MockerFixture from tox.execute.api import Outcome -from tox.execute.local_sub_process import SIG_INTERRUPT, LocalSubProcessExecutor +from tox.execute.local_sub_process import SIG_INTERRUPT, LocalSubProcessExecuteInstance, LocalSubProcessExecutor from tox.execute.request import ExecuteRequest, StdinSource +from tox.execute.stream import SyncWrite from tox.pytest import CaptureFixture, LogCaptureFixture, MonkeyPatch from tox.report import NamedBytesIO @@ -275,3 +276,23 @@ def test_local_subprocess_tty(monkeypatch: MonkeyPatch, mocker: MockerFixture, t "stdin": False, "terminal": [100, 100], } + + +@pytest.mark.parametrize("mode", ["stem", "full", "stem-pattern", "full-pattern", "all"]) +def test_allow_list_external_ok(fake_exe_on_path: Path, mode: str) -> None: + exe = f"{fake_exe_on_path}{'.EXE' if sys.platform == 'win32' else ''}" + allow = exe if "full" in mode else fake_exe_on_path.stem + allow = f"{allow[:-2]}*" if "pattern" in mode else allow + allow = "*" if mode == "all" else allow + + request = ExecuteRequest( + cmd=[fake_exe_on_path.stem], + cwd=Path.cwd(), + env={"PATH": os.environ["PATH"]}, + stdin=StdinSource.OFF, + run_id="run-id", + allow=[allow], + ) + inst = LocalSubProcessExecuteInstance(request, out=SyncWrite("out", None), err=SyncWrite("err", None)) + + assert inst.cmd == [exe] diff --git a/tests/execute/test_request.py b/tests/execute/test_request.py index 4b387af7..e87242d7 100644 --- a/tests/execute/test_request.py +++ b/tests/execute/test_request.py @@ -1,3 +1,5 @@ +import os +import sys from pathlib import Path from typing import Dict @@ -9,3 +11,15 @@ from tox.execute.request import ExecuteRequest, StdinSource def test_execute_request_raise_on_empty_cmd(os_env: Dict[str, str]) -> None: with pytest.raises(ValueError, match="cannot execute an empty command"): ExecuteRequest(cmd=[], cwd=Path().absolute(), env=os_env, stdin=StdinSource.OFF, run_id="") + + +def test_request_allow_star_is_none() -> None: + request = ExecuteRequest( + cmd=[sys.executable], + cwd=Path.cwd(), + env={"PATH": os.environ["PATH"]}, + stdin=StdinSource.OFF, + run_id="run-id", + allow=["*", "magic"], + ) + assert request.allow is None diff --git a/tests/tox_env/test_tox_env_api.py b/tests/tox_env/test_tox_env_api.py index 1d460c6b..42087fdf 100644 --- a/tests/tox_env/test_tox_env_api.py +++ b/tests/tox_env/test_tox_env_api.py @@ -1,3 +1,5 @@ +from pathlib import Path + from tox.pytest import ToxProjectCreator @@ -9,3 +11,15 @@ def test_requirements_txt(tox_project: ToxProjectCreator) -> None: result_second = prj.run("r") result_second.assert_success() assert "remove tox env folder" in result_second.out + + +def test_allow_list_external_fail(tox_project: ToxProjectCreator, fake_exe_on_path: Path) -> None: + prj = tox_project({"tox.ini": f"[testenv]\npackage=skip\ncommands={fake_exe_on_path.stem}"}) + execute_calls = prj.patch_execute(lambda r: 0 if "cmd" in r.run_id else None) + + result = prj.run("r") + + result.assert_failed(1) + out = fr".*py: failed with {fake_exe_on_path.stem} is not allowed, use allowlist_externals to allow it.*" + result.assert_out_err(out=out, err="", regex=True) + execute_calls.assert_called() diff --git a/whitelist.txt b/whitelist.txt index 24d50706..6140ee80 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -20,7 +20,7 @@ canonicalize capfd caplog capsys -Cfg +cfg changelog chardet chdir @@ -51,6 +51,7 @@ devenv DEVNULL devpi dirname +distlib divmod doc2path docname @@ -139,7 +140,7 @@ parsers pathlib pathname pathsep -pep517 +Pep517 platformdirs pluggy Popen |