From 9dc54f79b0026da98ee06f6d72be6fece571151a Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 6 Mar 2021 17:01:29 +0200 Subject: [PATCH] tmpdir: fix temporary directories created with world-readable permissions (Written for a Unix system, but might be applicable to Windows as well). pytest creates a root temporary directory under /tmp, named `pytest-of-`, and creates tmp_path's and other under it. /tmp is shared between all users of the system. This root temporary directory was created with 0o777&~umask permissions, which usually becomes 0o755, meaning any user in the system could list and read the files, which is undesirable. Use 0o700 permissions instead. Also for subdirectories, because the root dir is adjustable. --- changelog/8414.bugfix.rst | 5 +++++ src/_pytest/pathlib.py | 8 ++++---- src/_pytest/pytester.py | 4 ++-- src/_pytest/tmpdir.py | 16 ++++++++++------ testing/test_tmpdir.py | 16 ++++++++++++++++ 5 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 changelog/8414.bugfix.rst diff --git a/changelog/8414.bugfix.rst b/changelog/8414.bugfix.rst new file mode 100644 index 00000000000..a8b791ae210 --- /dev/null +++ b/changelog/8414.bugfix.rst @@ -0,0 +1,5 @@ +pytest used to create directories under ``/tmp`` with world-readable +permissions. This means that any user in the system was able to read +information written by tests in temporary directories (such as those created by +the ``tmp_path``/``tmpdir`` fixture). Now the directories are created with +private permissions. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index a86a3e8b89f..7d9269a1855 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -207,7 +207,7 @@ def _force_symlink( pass -def make_numbered_dir(root: Path, prefix: str) -> Path: +def make_numbered_dir(root: Path, prefix: str, mode: int = 0o700) -> Path: """Create a directory with an increased number as suffix for the given prefix.""" for i in range(10): # try up to 10 times to create the folder @@ -215,7 +215,7 @@ def make_numbered_dir(root: Path, prefix: str) -> Path: new_number = max_existing + 1 new_path = root.joinpath(f"{prefix}{new_number}") try: - new_path.mkdir() + new_path.mkdir(mode=mode) except Exception: pass else: @@ -347,13 +347,13 @@ def cleanup_numbered_dir( def make_numbered_dir_with_cleanup( - root: Path, prefix: str, keep: int, lock_timeout: float + root: Path, prefix: str, keep: int, lock_timeout: float, mode: int, ) -> Path: """Create a numbered dir with a cleanup lock and remove old ones.""" e = None for i in range(10): try: - p = make_numbered_dir(root, prefix) + p = make_numbered_dir(root, prefix, mode) lock_path = create_cleanup_lock(p) register_cleanup_lock_removal(lock_path) except Exception as exc: diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 6833eb02149..31259d1bdcd 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -1426,7 +1426,7 @@ def runpytest_subprocess(self, *args, timeout: Optional[float] = None) -> RunRes :rtype: RunResult """ __tracebackhide__ = True - p = make_numbered_dir(root=self.path, prefix="runpytest-") + p = make_numbered_dir(root=self.path, prefix="runpytest-", mode=0o700) args = ("--basetemp=%s" % p,) + args plugins = [x for x in self.plugins if isinstance(x, str)] if plugins: @@ -1445,7 +1445,7 @@ def spawn_pytest( The pexpect child is returned. """ basetemp = self.path / "temp-pexpect" - basetemp.mkdir() + basetemp.mkdir(mode=0o700) invoke = " ".join(map(str, self._getpytestargs())) cmd = f"{invoke} --basetemp={basetemp} {string}" return self.spawn(cmd, expect_timeout=expect_timeout) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 73754672e02..53fc327964f 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -90,14 +90,14 @@ def mktemp(self, basename: str, numbered: bool = True) -> Path: basename = self._ensure_relative_to_basetemp(basename) if not numbered: p = self.getbasetemp().joinpath(basename) - p.mkdir() + p.mkdir(mode=0o700) else: - p = make_numbered_dir(root=self.getbasetemp(), prefix=basename) + p = make_numbered_dir(root=self.getbasetemp(), prefix=basename, mode=0o700) self._trace("mktemp", p) return p def getbasetemp(self) -> Path: - """Return base temporary directory.""" + """Return the base temporary directory, creating it if needed.""" if self._basetemp is not None: return self._basetemp @@ -105,7 +105,7 @@ def getbasetemp(self) -> Path: basetemp = self._given_basetemp if basetemp.exists(): rm_rf(basetemp) - basetemp.mkdir() + basetemp.mkdir(mode=0o700) basetemp = basetemp.resolve() else: from_env = os.environ.get("PYTEST_DEBUG_TEMPROOT") @@ -114,9 +114,13 @@ def getbasetemp(self) -> Path: # use a sub-directory in the temproot to speed-up # make_numbered_dir() call rootdir = temproot.joinpath(f"pytest-of-{user}") - rootdir.mkdir(exist_ok=True) + rootdir.mkdir(mode=0o700, exist_ok=True) basetemp = make_numbered_dir_with_cleanup( - prefix="pytest-", root=rootdir, keep=3, lock_timeout=LOCK_TIMEOUT + prefix="pytest-", + root=rootdir, + keep=3, + lock_timeout=LOCK_TIMEOUT, + mode=0o700, ) assert basetemp is not None, basetemp self._basetemp = basetemp diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index d123287aa38..fc2a62ea1c3 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -445,3 +445,19 @@ def test(tmp_path): # running a second time and ensure we don't crash result = pytester.runpytest("--basetemp=tmp") assert result.ret == 0 + + +@pytest.mark.skipif(not hasattr(os, "getuid"), reason="checks unix permissions") +def test_tmp_path_factory_create_directory_with_safe_permissions( + tmp_path: Path, monkeypatch, +) -> None: + """Verify that pytest creates directories under /tmp with private permissions.""" + # Use the test's tmp_path as the system temproot (/tmp). + monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(tmp_path)) + tmp_factory = TempPathFactory(None, lambda *args: None, _ispytest=True) + basetemp = tmp_factory.getbasetemp() + + # No world-readable permissions. + assert (basetemp.stat().st_mode & 0o077) == 0 + # Parent too (pytest-of-foo). + assert (basetemp.parent.stat().st_mode & 0o077) == 0