Skip to content

Commit 132131b

Browse files
bpo-42782: Fail fast for permission errors in shutil.move() (GH-24001)
* Fail fast in shutil.move() to avoid creating destination directories on failure. Co-authored-by: Zackery Spytz <[email protected]>
1 parent b36349a commit 132131b

File tree

3 files changed

+50
-0
lines changed

3 files changed

+50
-0
lines changed

Lib/shutil.py

+11
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,12 @@ def move(src, dst, copy_function=copy2):
813813
if _destinsrc(src, dst):
814814
raise Error("Cannot move a directory '%s' into itself"
815815
" '%s'." % (src, dst))
816+
if (_is_immutable(src)
817+
or (not os.access(src, os.W_OK) and os.listdir(src)
818+
and sys.platform == 'darwin')):
819+
raise PermissionError("Cannot move the non-empty directory "
820+
"'%s': Lacking write permission to '%s'."
821+
% (src, src))
816822
copytree(src, real_dst, copy_function=copy_function,
817823
symlinks=True)
818824
rmtree(src)
@@ -830,6 +836,11 @@ def _destinsrc(src, dst):
830836
dst += os.path.sep
831837
return dst.startswith(src)
832838

839+
def _is_immutable(src):
840+
st = _stat(src)
841+
immutable_states = [stat.UF_IMMUTABLE, stat.SF_IMMUTABLE]
842+
return hasattr(st, 'st_flags') and st.st_flags in immutable_states
843+
833844
def _get_gid(name):
834845
"""Returns a gid, given a group name."""
835846
if getgrnam is None or name is None:

Lib/test/test_shutil.py

+37
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
from test.support.os_helper import TESTFN, FakePath
3535

3636
TESTFN2 = TESTFN + "2"
37+
TESTFN_SRC = TESTFN + "_SRC"
38+
TESTFN_DST = TESTFN + "_DST"
3739
MACOS = sys.platform.startswith("darwin")
3840
AIX = sys.platform[:3] == 'aix'
3941
try:
@@ -2085,6 +2087,41 @@ def test_move_dir_caseinsensitive(self):
20852087
os.rmdir(dst_dir)
20862088

20872089

2090+
@unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0
2091+
and hasattr(os, 'lchflags')
2092+
and hasattr(stat, 'SF_IMMUTABLE')
2093+
and hasattr(stat, 'UF_OPAQUE'),
2094+
'root privileges required')
2095+
def test_move_dir_permission_denied(self):
2096+
# bpo-42782: shutil.move should not create destination directories
2097+
# if the source directory cannot be removed.
2098+
try:
2099+
os.mkdir(TESTFN_SRC)
2100+
os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)
2101+
2102+
# Testing on an empty immutable directory
2103+
# TESTFN_DST should not exist if shutil.move failed
2104+
self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
2105+
self.assertFalse(TESTFN_DST in os.listdir())
2106+
2107+
# Create a file and keep the directory immutable
2108+
os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
2109+
os_helper.create_empty_file(os.path.join(TESTFN_SRC, 'child'))
2110+
os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)
2111+
2112+
# Testing on a non-empty immutable directory
2113+
# TESTFN_DST should not exist if shutil.move failed
2114+
self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
2115+
self.assertFalse(TESTFN_DST in os.listdir())
2116+
finally:
2117+
if os.path.exists(TESTFN_SRC):
2118+
os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
2119+
os_helper.rmtree(TESTFN_SRC)
2120+
if os.path.exists(TESTFN_DST):
2121+
os.lchflags(TESTFN_DST, stat.UF_OPAQUE)
2122+
os_helper.rmtree(TESTFN_DST)
2123+
2124+
20882125
class TestCopyFile(unittest.TestCase):
20892126

20902127
class Faux(object):
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fail fast in :func:`shutil.move()` to avoid creating destination directories on
2+
failure.

0 commit comments

Comments
 (0)