Skip to content

Commit 221a573

Browse files
winsonlukorsenthil
authored andcommitted
[3.9] 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]> (cherry picked from commit 132131b) Co-authored-by: Winson Luk <[email protected]>
1 parent 024325d commit 221a573

File tree

3 files changed

+50
-0
lines changed

3 files changed

+50
-0
lines changed

Lib/shutil.py

Lines changed: 11 additions & 0 deletions
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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
from test.support import TESTFN, FakePath
3434

3535
TESTFN2 = TESTFN + "2"
36+
TESTFN_SRC = TESTFN + "_SRC"
37+
TESTFN_DST = TESTFN + "_DST"
3638
MACOS = sys.platform.startswith("darwin")
3739
AIX = sys.platform[:3] == 'aix'
3840
try:
@@ -2083,6 +2085,41 @@ def test_move_dir_caseinsensitive(self):
20832085
os.rmdir(dst_dir)
20842086

20852087

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

20882125
class Faux(object):
Lines changed: 2 additions & 0 deletions
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)