Skip to content

Commit bbd9a19

Browse files
RonnyPfannschmidtCursor AIclaude
committed
fix: rm_rf now handles directories without S_IXUSR permission (#7940)
The on_rm_rf_error handler previously only set S_IRUSR|S_IWUSR (read+write) when retrying failed removals, and silently skipped os.open/os.scandir failures. This meant directories lacking execute permission (S_IXUSR) could not be traversed or removed by shutil.rmtree. Now the handler: - Uses S_IRWXU (read+write+execute) for all permission fixes - Handles os.open and os.scandir PermissionError by fixing permissions on the path and its parent, then removing the entry directly - Includes a guard against infinite recursion when chmod has no effect Supersedes #7941 which took the approach of delegating to tempfile.TemporaryDirectory._rmtree. Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Claude Opus <claude@anthropic.com>
1 parent bc57e69 commit bbd9a19

3 files changed

Lines changed: 97 additions & 21 deletions

File tree

changelog/7940.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed ``rm_rf`` failing to remove directories when the ``S_IXUSR`` (owner execute) permission bit is missing -- supersedes :pr:`7941`.

src/_pytest/pathlib.py

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,25 @@ def get_lock_path(path: _AnyPurePath) -> _AnyPurePath:
7070
return path.joinpath(".lock")
7171

7272

73+
def _chmod_rwx(p: str) -> bool:
74+
"""Grant owner read, write, and execute permissions.
75+
76+
Returns True if permissions were actually changed, False if they were
77+
already sufficient or couldn't be changed.
78+
"""
79+
import stat
80+
81+
try:
82+
old_mode = os.stat(p).st_mode
83+
new_mode = old_mode | stat.S_IRWXU
84+
if old_mode == new_mode:
85+
return False
86+
os.chmod(p, new_mode)
87+
except OSError:
88+
return False
89+
return True
90+
91+
7392
def on_rm_rf_error(
7493
func: Callable[..., Any] | None,
7594
path: str,
@@ -98,32 +117,44 @@ def on_rm_rf_error(
98117
)
99118
return False
100119

120+
if func in (os.open, os.scandir):
121+
# Directory traversal failed (e.g. missing S_IXUSR). Fix permissions
122+
# on the path and its parent, then remove it ourselves since rmtree
123+
# skips entries after the error handler returns.
124+
# See: https://github.com/pytest-dev/pytest/issues/7940
125+
p = Path(path)
126+
if p.parent != p:
127+
_chmod_rwx(str(p.parent))
128+
if not _chmod_rwx(path):
129+
return False
130+
if os.path.isdir(path):
131+
rm_rf(Path(path))
132+
else:
133+
try:
134+
os.unlink(path)
135+
except OSError:
136+
return False
137+
return True
138+
101139
if func not in (os.rmdir, os.remove, os.unlink):
102-
if func not in (os.open,):
103-
warnings.warn(
104-
PytestWarning(
105-
f"(rm_rf) unknown function {func} when removing {path}:\n{type(exc)}: {exc}"
106-
)
140+
warnings.warn(
141+
PytestWarning(
142+
f"(rm_rf) unknown function {func} when removing {path}:\n{type(exc)}: {exc}"
107143
)
144+
)
108145
return False
109146

110147
# Chmod + retry.
111-
import stat
112-
113-
def chmod_rw(p: str) -> None:
114-
mode = os.stat(p).st_mode
115-
os.chmod(p, mode | stat.S_IRUSR | stat.S_IWUSR)
116-
117148
# For files, we need to recursively go upwards in the directories to
118-
# ensure they all are also writable.
149+
# ensure they all are also accessible and writable.
119150
p = Path(path)
120151
if p.is_file():
121152
for parent in p.parents:
122-
chmod_rw(str(parent))
153+
_chmod_rwx(str(parent))
123154
# Stop when we reach the original path passed to rm_rf.
124155
if parent == start_path:
125156
break
126-
chmod_rw(str(path))
157+
_chmod_rwx(str(path))
127158

128159
func(path)
129160
return True

testing/test_tmpdir.py

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,25 @@ def test_rm_rf_with_read_only_directory(self, tmp_path):
500500

501501
assert not adir.is_dir()
502502

503+
@pytest.mark.skipif(not hasattr(os, "getuid"), reason="unix permissions")
504+
def test_rm_rf_with_no_exec_permission_directories(self, tmp_path):
505+
"""Ensure rm_rf can remove directories without S_IXUSR (#7940).
506+
507+
This is the exact scenario from the original issue: nested directories
508+
and files with all permissions stripped.
509+
"""
510+
p = tmp_path / "foo" / "bar" / "baz"
511+
p.parent.mkdir(parents=True)
512+
p.touch(mode=0)
513+
for parent in p.parents:
514+
if parent == tmp_path:
515+
break
516+
parent.chmod(mode=0)
517+
518+
rm_rf(tmp_path / "foo")
519+
520+
assert not (tmp_path / "foo").exists()
521+
503522
def test_on_rm_rf_error(self, tmp_path: Path) -> None:
504523
adir = tmp_path / "dir"
505524
adir.mkdir()
@@ -527,17 +546,42 @@ def test_on_rm_rf_error(self, tmp_path: Path) -> None:
527546
on_rm_rf_error(None, str(fn), exc_info3, start_path=tmp_path)
528547
assert fn.is_file()
529548

530-
# ignored function
531-
with warnings.catch_warnings(record=True) as w:
532-
exc_info4 = PermissionError()
533-
on_rm_rf_error(os.open, str(fn), exc_info4, start_path=tmp_path)
534-
assert fn.is_file()
535-
assert not [x.message for x in w]
536-
549+
# os.unlink PermissionError is handled (chmod + retry)
537550
exc_info5 = PermissionError()
538551
on_rm_rf_error(os.unlink, str(fn), exc_info5, start_path=tmp_path)
539552
assert not fn.is_file()
540553

554+
def test_on_rm_rf_error_os_open_handles_file(self, tmp_path: Path) -> None:
555+
"""os.open PermissionError on a file is handled by fixing
556+
permissions and removing it (#7940)."""
557+
adir = tmp_path / "dir"
558+
adir.mkdir()
559+
fn = adir / "foo.txt"
560+
fn.touch()
561+
self.chmod_r(fn)
562+
563+
with warnings.catch_warnings(record=True) as w:
564+
exc_info = PermissionError()
565+
on_rm_rf_error(os.open, str(fn), exc_info, start_path=tmp_path)
566+
assert not fn.exists()
567+
assert not [x.message for x in w]
568+
569+
@pytest.mark.skipif(not hasattr(os, "getuid"), reason="unix permissions")
570+
def test_on_rm_rf_error_os_open_handles_directory(self, tmp_path: Path) -> None:
571+
"""os.open PermissionError on a directory is handled by fixing
572+
permissions and recursively removing it (#7940)."""
573+
adir = tmp_path / "dir"
574+
adir.mkdir()
575+
(adir / "child").mkdir()
576+
(adir / "child" / "file.txt").touch()
577+
os.chmod(str(adir), 0o600)
578+
579+
with warnings.catch_warnings(record=True) as w:
580+
exc_info = PermissionError()
581+
on_rm_rf_error(os.open, str(adir), exc_info, start_path=tmp_path)
582+
assert not adir.exists()
583+
assert not [x.message for x in w]
584+
541585

542586
def attempt_symlink_to(path, to_path):
543587
"""Try to make a symlink from "path" to "to_path", skipping in case this platform

0 commit comments

Comments
 (0)