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 <zspytz@gmail.com>
(cherry picked from commit 132131b404e06ee1a19b040a1f96cd1118abed0c)

Co-authored-by: Winson Luk <winson.luk@gmail.com>

diff --git a/Lib/shutil.py b/Lib/shutil.py
index 6986bce8aaee2d30a7a4a8b1b397f4ac15a8738e..fdadb838000c7e34f8406940b4fc6c56130db584 100644 (file)
--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -798,6 +798,12 @@ def move(src, dst, copy_function=copy2):
             if _destinsrc(src, dst):
                 raise Error("Cannot move a directory '%s' into itself"
                             " '%s'." % (src, dst))
+            if (_is_immutable(src)
+                    or (not os.access(src, os.W_OK) and os.listdir(src)
+                        and sys.platform == 'darwin')):
+                raise PermissionError("Cannot move the non-empty directory "
+                                      "'%s': Lacking write permission to '%s'."
+                                      % (src, src))
             copytree(src, real_dst, copy_function=copy_function,
                      symlinks=True)
             rmtree(src)
@@ -815,6 +821,11 @@ def _destinsrc(src, dst):
         dst += os.path.sep
     return dst.startswith(src)
 
+def _is_immutable(src):
+    st = _stat(src)
+    immutable_states = [stat.UF_IMMUTABLE, stat.SF_IMMUTABLE]
+    return hasattr(st, 'st_flags') and st.st_flags in immutable_states
+
 def _get_gid(name):
     """Returns a gid, given a group name."""
     if getgrnam is None or name is None:

diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index 77306170d9fc18754a7848ed1fc631a761fc1bd1..37940c8dcf71f21367a1630f4f92b3e4eb602d4a 100644 (file)
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -33,6 +33,8 @@ from test import support
 from test.support import TESTFN, FakePath
 
 TESTFN2 = TESTFN + "2"
+TESTFN_SRC = TESTFN + "_SRC"
+TESTFN_DST = TESTFN + "_DST"
 MACOS = sys.platform.startswith("darwin")
 AIX = sys.platform[:3] == 'aix'
 try:
@@ -2053,6 +2055,41 @@ class TestMove(unittest.TestCase):
         self.assertEqual(len(moved), 3)
 
 
+    @unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0
+                         and hasattr(os, 'lchflags')
+                         and hasattr(stat, 'SF_IMMUTABLE')
+                         and hasattr(stat, 'UF_OPAQUE'),
+                         'root privileges required')
+    def test_move_dir_permission_denied(self):
+        # bpo-42782: shutil.move should not create destination directories
+        # if the source directory cannot be removed.
+        try:
+            os.mkdir(TESTFN_SRC)
+            os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)
+
+            # Testing on an empty immutable directory
+            # TESTFN_DST should not exist if shutil.move failed
+            self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
+            self.assertFalse(TESTFN_DST in os.listdir())
+
+            # Create a file and keep the directory immutable
+            os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
+            os_helper.create_empty_file(os.path.join(TESTFN_SRC, 'child'))
+            os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)
+
+            # Testing on a non-empty immutable directory
+            # TESTFN_DST should not exist if shutil.move failed
+            self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
+            self.assertFalse(TESTFN_DST in os.listdir())
+        finally:
+            if os.path.exists(TESTFN_SRC):
+                os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
+                os_helper.rmtree(TESTFN_SRC)
+            if os.path.exists(TESTFN_DST):
+                os.lchflags(TESTFN_DST, stat.UF_OPAQUE)
+                os_helper.rmtree(TESTFN_DST)
+
+
 class TestCopyFile(unittest.TestCase):
 
     _delete = False

diff --git a/Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst b/Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst
new file mode 100644 (file)
index 0000000..065df9b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst
@@ -0,0 +1,2 @@
+Fail fast in :func:`shutil.move()` to avoid creating destination directories on
+failure.