| 1 | Patch-Source: https://github.com/pytest-dev/pytest/pull/14331
|
| 2 |
|
| 3 | diff --git a/changelog/7940.bugfix.rst b/changelog/7940.bugfix.rst
|
| 4 | new file mode 100644
|
| 5 | index 00000000000..99845153ace
|
| 6 | |
| 7 | |
| 8 | @@ -0,0 +1 @@
|
| 9 | +Fixed cleanup of temporary directories failing when subdirectories have their execute permission removed.
|
| 10 | diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py
|
| 11 | index 291bdf4ecbf..10326e1c9a6 100644
|
| 12 | |
| 13 | |
| 14 | @@ -24,6 +24,7 @@
|
| 15 | from pathlib import PurePath
|
| 16 | from posixpath import sep as posix_sep
|
| 17 | import shutil
|
| 18 | +import stat
|
| 19 | import sys
|
| 20 | import types
|
| 21 | from types import ModuleType
|
| 22 | @@ -69,6 +70,33 @@ def get_lock_path(path: _AnyPurePath) -> _AnyPurePath:
|
| 23 | return path.joinpath(".lock")
|
| 24 |
|
| 25 |
|
| 26 | +def _chmod_rwx(p: str) -> bool:
|
| 27 | + """Grant owner sufficient permissions for deletion.
|
| 28 | +
|
| 29 | + Directories get ``S_IRWXU`` (read+write+exec for traversal).
|
| 30 | + Regular files get ``S_IRUSR | S_IWUSR`` only, to avoid making
|
| 31 | + non-executable files executable as a side effect.
|
| 32 | +
|
| 33 | + Returns True if permissions were actually changed, False if they were
|
| 34 | + already sufficient or couldn't be changed.
|
| 35 | + """
|
| 36 | + try:
|
| 37 | + old_mode = os.stat(p).st_mode
|
| 38 | + except OSError:
|
| 39 | + # Path may have been removed concurrently, or be inaccessible.
|
| 40 | + return False
|
| 41 | + perm_mode = stat.S_IMODE(old_mode)
|
| 42 | + bits = stat.S_IRWXU if stat.S_ISDIR(old_mode) else stat.S_IRUSR | stat.S_IWUSR
|
| 43 | + new_mode = perm_mode | bits
|
| 44 | + if perm_mode == new_mode:
|
| 45 | + return False
|
| 46 | + try:
|
| 47 | + os.chmod(p, new_mode)
|
| 48 | + except OSError:
|
| 49 | + return False
|
| 50 | + return True
|
| 51 | +
|
| 52 | +
|
| 53 | def on_rm_rf_error(
|
| 54 | func: Callable[..., Any] | None,
|
| 55 | path: str,
|
| 56 | @@ -97,32 +125,48 @@ def on_rm_rf_error(
|
| 57 | )
|
| 58 | return False
|
| 59 |
|
| 60 | + p = Path(path)
|
| 61 | +
|
| 62 | + if func in (os.open, os.scandir):
|
| 63 | + # Directory traversal failed (e.g. missing S_IXUSR). Fix permissions
|
| 64 | + # on the path and its parent (bounded by start_path), then remove it
|
| 65 | + # ourselves since rmtree skips entries after the error handler returns.
|
| 66 | + # See: https://github.com/pytest-dev/pytest/issues/7940
|
| 67 | + parent_changed = False
|
| 68 | + parent = p.parent
|
| 69 | + # Never chmod outside the tree rooted at start_path.
|
| 70 | + if parent not in (p, start_path):
|
| 71 | + parent_changed = _chmod_rwx(str(parent))
|
| 72 | + path_changed = _chmod_rwx(str(p))
|
| 73 | + if not (parent_changed or path_changed):
|
| 74 | + return False
|
| 75 | + if p.is_dir():
|
| 76 | + rm_rf(p)
|
| 77 | + else:
|
| 78 | + try:
|
| 79 | + os.unlink(str(p))
|
| 80 | + except OSError:
|
| 81 | + return False
|
| 82 | + return True
|
| 83 | +
|
| 84 | if func not in (os.rmdir, os.remove, os.unlink):
|
| 85 | - if func not in (os.open,):
|
| 86 | - warnings.warn(
|
| 87 | - PytestWarning(
|
| 88 | - f"(rm_rf) unknown function {func} when removing {path}:\n{type(exc)}: {exc}"
|
| 89 | - )
|
| 90 | + warnings.warn(
|
| 91 | + PytestWarning(
|
| 92 | + f"(rm_rf) unknown function {func} when removing {path}:\n{type(exc)}: {exc}"
|
| 93 | )
|
| 94 | + )
|
| 95 | return False
|
| 96 |
|
| 97 | # Chmod + retry.
|
| 98 | - import stat
|
| 99 | -
|
| 100 | - def chmod_rw(p: str) -> None:
|
| 101 | - mode = os.stat(p).st_mode
|
| 102 | - os.chmod(p, mode | stat.S_IRUSR | stat.S_IWUSR)
|
| 103 | -
|
| 104 | # For files, we need to recursively go upwards in the directories to
|
| 105 | - # ensure they all are also writable.
|
| 106 | - p = Path(path)
|
| 107 | + # ensure they all are also accessible and writable.
|
| 108 | if p.is_file():
|
| 109 | - for parent in p.parents:
|
| 110 | - chmod_rw(str(parent))
|
| 111 | + for parent in p.parents: # pragma: no branch
|
| 112 | + _chmod_rwx(str(parent))
|
| 113 | # Stop when we reach the original path passed to rm_rf.
|
| 114 | if parent == start_path:
|
| 115 | break
|
| 116 | - chmod_rw(str(path))
|
| 117 | + _chmod_rwx(str(path))
|
| 118 |
|
| 119 | func(path)
|
| 120 | return True
|
| 121 | diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py
|
| 122 | index 610de9c5fd6..90a84da5bb5 100644
|
| 123 | |
| 124 | |
| 125 | @@ -14,6 +14,7 @@
|
| 126 | from _pytest import pathlib
|
| 127 | from _pytest.config import Config
|
| 128 | from _pytest.monkeypatch import MonkeyPatch
|
| 129 | +from _pytest.pathlib import _chmod_rwx
|
| 130 | from _pytest.pathlib import cleanup_numbered_dir
|
| 131 | from _pytest.pathlib import create_cleanup_lock
|
| 132 | from _pytest.pathlib import make_numbered_dir
|
| 133 | @@ -522,6 +523,10 @@ def test_removal_accepts_lock(self, tmp_path):
|
| 134 | assert dir.is_dir()
|
| 135 |
|
| 136 |
|
| 137 | +def _raise_oserror(*args: object, **kwargs: object) -> None:
|
| 138 | + raise OSError("simulated failure")
|
| 139 | +
|
| 140 | +
|
| 141 | class TestRmRf:
|
| 142 | def test_rm_rf(self, tmp_path):
|
| 143 | adir = tmp_path / "adir"
|
| 144 | @@ -566,6 +571,25 @@ def test_rm_rf_with_read_only_directory(self, tmp_path):
|
| 145 |
|
| 146 | assert not adir.is_dir()
|
| 147 |
|
| 148 | + @pytest.mark.skipif(not hasattr(os, "getuid"), reason="unix permissions")
|
| 149 | + def test_rm_rf_with_no_exec_permission_directories(self, tmp_path):
|
| 150 | + """Ensure rm_rf can remove directories without S_IXUSR (#7940).
|
| 151 | +
|
| 152 | + This is the exact scenario from the original issue: nested directories
|
| 153 | + and files with all permissions stripped.
|
| 154 | + """
|
| 155 | + p = tmp_path / "foo" / "bar" / "baz"
|
| 156 | + p.parent.mkdir(parents=True)
|
| 157 | + p.touch(mode=0)
|
| 158 | + for parent in p.parents: # pragma: no branch
|
| 159 | + if parent == tmp_path:
|
| 160 | + break
|
| 161 | + parent.chmod(mode=0)
|
| 162 | +
|
| 163 | + rm_rf(tmp_path / "foo")
|
| 164 | +
|
| 165 | + assert not (tmp_path / "foo").exists()
|
| 166 | +
|
| 167 | def test_on_rm_rf_error(self, tmp_path: Path) -> None:
|
| 168 | adir = tmp_path / "dir"
|
| 169 | adir.mkdir()
|
| 170 | @@ -593,17 +617,146 @@ def test_on_rm_rf_error(self, tmp_path: Path) -> None:
|
| 171 | on_rm_rf_error(None, str(fn), exc_info3, start_path=tmp_path)
|
| 172 | assert fn.is_file()
|
| 173 |
|
| 174 | - # ignored function
|
| 175 | - with warnings.catch_warnings(record=True) as w:
|
| 176 | - exc_info4 = PermissionError()
|
| 177 | - on_rm_rf_error(os.open, str(fn), exc_info4, start_path=tmp_path)
|
| 178 | - assert fn.is_file()
|
| 179 | - assert not [x.message for x in w]
|
| 180 | -
|
| 181 | + # os.unlink PermissionError is handled (chmod + retry)
|
| 182 | exc_info5 = PermissionError()
|
| 183 | on_rm_rf_error(os.unlink, str(fn), exc_info5, start_path=tmp_path)
|
| 184 | assert not fn.is_file()
|
| 185 |
|
| 186 | + def test_on_rm_rf_error_os_open_handles_file(self, tmp_path: Path) -> None:
|
| 187 | + """os.open PermissionError on a file is handled by fixing
|
| 188 | + permissions and removing it (#7940)."""
|
| 189 | + adir = tmp_path / "dir"
|
| 190 | + adir.mkdir()
|
| 191 | + fn = adir / "foo.txt"
|
| 192 | + fn.touch()
|
| 193 | + self.chmod_r(fn)
|
| 194 | +
|
| 195 | + with warnings.catch_warnings(record=True) as w:
|
| 196 | + exc_info = PermissionError()
|
| 197 | + on_rm_rf_error(os.open, str(fn), exc_info, start_path=tmp_path)
|
| 198 | + assert not fn.exists()
|
| 199 | + assert not [x.message for x in w]
|
| 200 | +
|
| 201 | + @pytest.mark.skipif(not hasattr(os, "getuid"), reason="unix permissions")
|
| 202 | + def test_on_rm_rf_error_os_open_handles_directory(self, tmp_path: Path) -> None:
|
| 203 | + """os.open PermissionError on a directory is handled by fixing
|
| 204 | + permissions and recursively removing it (#7940)."""
|
| 205 | + adir = tmp_path / "dir"
|
| 206 | + adir.mkdir()
|
| 207 | + (adir / "child").mkdir()
|
| 208 | + (adir / "child" / "file.txt").touch()
|
| 209 | + os.chmod(str(adir), stat.S_IRUSR | stat.S_IWUSR)
|
| 210 | +
|
| 211 | + with warnings.catch_warnings(record=True) as w:
|
| 212 | + exc_info = PermissionError()
|
| 213 | + on_rm_rf_error(os.open, str(adir), exc_info, start_path=tmp_path)
|
| 214 | + assert not adir.exists()
|
| 215 | + assert not [x.message for x in w]
|
| 216 | +
|
| 217 | + @pytest.mark.skipif(not hasattr(os, "getuid"), reason="unix permissions")
|
| 218 | + def test_on_rm_rf_error_os_open_parent_perms(self, tmp_path: Path) -> None:
|
| 219 | + """When the PermissionError is caused by the *parent* directory lacking
|
| 220 | + S_IXUSR, fixing the parent is sufficient even if the child already has
|
| 221 | + correct permissions."""
|
| 222 | + parent = tmp_path / "parent"
|
| 223 | + parent.mkdir()
|
| 224 | + child = parent / "child"
|
| 225 | + child.mkdir()
|
| 226 | + (child / "file.txt").touch()
|
| 227 | + # Child has full perms, but parent lacks execute -> os.open(child) fails.
|
| 228 | + os.chmod(str(parent), stat.S_IRUSR | stat.S_IWUSR)
|
| 229 | +
|
| 230 | + with warnings.catch_warnings(record=True) as w:
|
| 231 | + exc_info = PermissionError()
|
| 232 | + result = on_rm_rf_error(os.open, str(child), exc_info, start_path=tmp_path)
|
| 233 | + assert result is True
|
| 234 | + assert not child.exists()
|
| 235 | + assert not [x.message for x in w]
|
| 236 | +
|
| 237 | + def test_chmod_rwx_returns_false_on_nonexistent_path(self, tmp_path: Path) -> None:
|
| 238 | + """_chmod_rwx returns False when the path doesn't exist (OSError)."""
|
| 239 | + nonexistent = tmp_path / "does_not_exist"
|
| 240 | + assert _chmod_rwx(str(nonexistent)) is False
|
| 241 | +
|
| 242 | + def test_chmod_rwx_returns_false_when_chmod_fails(
|
| 243 | + self, tmp_path: Path, monkeypatch: MonkeyPatch
|
| 244 | + ) -> None:
|
| 245 | + """_chmod_rwx returns False when os.chmod raises OSError.
|
| 246 | +
|
| 247 | + A real FS layout that makes chmod fail while the path remains
|
| 248 | + (immutable bit, RO mount, foreign ownership) is not portable in CI,
|
| 249 | + so pin this branch with a monkeypatch.
|
| 250 | + """
|
| 251 | + fn = tmp_path / "file.txt"
|
| 252 | + fn.touch(mode=0) # needs bits so we reach os.chmod
|
| 253 | + monkeypatch.setattr(os, "chmod", _raise_oserror)
|
| 254 | + assert _chmod_rwx(str(fn)) is False
|
| 255 | +
|
| 256 | + def test_chmod_rwx_returns_false_when_already_sufficient(
|
| 257 | + self, tmp_path: Path
|
| 258 | + ) -> None:
|
| 259 | + """_chmod_rwx returns False when permissions are already sufficient."""
|
| 260 | + d = tmp_path / "dir"
|
| 261 | + d.mkdir(mode=stat.S_IRWXU)
|
| 262 | + assert _chmod_rwx(str(d)) is False
|
| 263 | +
|
| 264 | + f = tmp_path / "file"
|
| 265 | + f.touch(mode=stat.S_IRUSR | stat.S_IWUSR)
|
| 266 | + assert _chmod_rwx(str(f)) is False
|
| 267 | +
|
| 268 | + def test_on_rm_rf_error_os_open_returns_false_when_chmod_ineffective(
|
| 269 | + self, tmp_path: Path
|
| 270 | + ) -> None:
|
| 271 | + """os.open handler returns False when neither parent nor path chmod
|
| 272 | + changes anything (recursion guard)."""
|
| 273 | + adir = tmp_path / "dir"
|
| 274 | + adir.mkdir(mode=stat.S_IRWXU)
|
| 275 | + exc_info = PermissionError()
|
| 276 | + result = on_rm_rf_error(os.open, str(adir), exc_info, start_path=tmp_path)
|
| 277 | + assert result is False
|
| 278 | + assert adir.exists()
|
| 279 | +
|
| 280 | + def test_on_rm_rf_error_os_open_unlink_fails(
|
| 281 | + self, tmp_path: Path, monkeypatch: MonkeyPatch
|
| 282 | + ) -> None:
|
| 283 | + """os.open handler returns False when chmod succeeds but os.unlink
|
| 284 | + still raises OSError (e.g. sandbox or other mechanism)."""
|
| 285 | + fn = tmp_path / "stubborn.txt"
|
| 286 | + fn.touch(mode=0)
|
| 287 | +
|
| 288 | + monkeypatch.setattr(os, "unlink", _raise_oserror)
|
| 289 | +
|
| 290 | + exc_info = PermissionError()
|
| 291 | + result = on_rm_rf_error(os.open, str(fn), exc_info, start_path=tmp_path)
|
| 292 | + assert result is False
|
| 293 | + assert fn.exists()
|
| 294 | +
|
| 295 | + @pytest.mark.skipif(not hasattr(os, "getuid"), reason="unix permissions")
|
| 296 | + def test_on_rm_rf_error_chmod_retry_walks_parents(self, tmp_path: Path) -> None:
|
| 297 | + """The os.rmdir/os.unlink handler walks up through multiple parent
|
| 298 | + directories to fix permissions before retrying."""
|
| 299 | + deep = tmp_path / "a" / "b" / "c"
|
| 300 | + deep.mkdir(parents=True)
|
| 301 | + fn = deep / "file.txt"
|
| 302 | + fn.touch()
|
| 303 | + # Remove write from intermediate dirs (keep exec so traversal works,
|
| 304 | + # but os.unlink needs write on the parent).
|
| 305 | + for parent in fn.parents: # pragma: no branch
|
| 306 | + if parent == tmp_path:
|
| 307 | + break
|
| 308 | + parent.chmod(
|
| 309 | + stat.S_IRUSR
|
| 310 | + | stat.S_IXUSR
|
| 311 | + | stat.S_IRGRP
|
| 312 | + | stat.S_IXGRP
|
| 313 | + | stat.S_IROTH
|
| 314 | + | stat.S_IXOTH
|
| 315 | + )
|
| 316 | +
|
| 317 | + exc_info = PermissionError()
|
| 318 | + on_rm_rf_error(os.unlink, str(fn), exc_info, start_path=tmp_path)
|
| 319 | + assert not fn.exists()
|
| 320 | +
|
| 321 |
|
| 322 | def attempt_symlink_to(path, to_path):
|
| 323 | """Try to make a symlink from "path" to "to_path", skipping in case this platform
|