Fixed #29027 -- Fixed file_move_safe() crash when moving files with SELinux.

Thanks Florian Apolloner for the review.

2 files changed, 44 insertions, 16 deletions

diff --git a/django/core/files/move.py b/django/core/files/move.py
index 2d71e11885..95d69f9d94 100644
--- a/django/core/files/move.py
+++ b/django/core/files/move.py
@@ -5,9 +5,8 @@ Move a file in the safest way possible::
     >>> file_move_safe("/tmp/old_file", "/tmp/new_file")
 """
 
-import errno
 import os
-from shutil import copystat
+from shutil import copymode, copystat
 
 from django.core.files import locks
 
@@ -82,12 +81,15 @@ def file_move_safe(
 
     try:
         copystat(old_file_name, new_file_name)
-    except PermissionError as e:
+    except PermissionError:
         # Certain filesystems (e.g. CIFS) fail to copy the file's metadata if
         # the type of the destination filesystem isn't the same as the source
-        # filesystem; ignore that.
-        if e.errno != errno.EPERM:
-            raise
+        # filesystem. This also happens with some SELinux-enabled systems.
+        # Ignore that, but try to set basic permissions.
+        try:
+            copymode(old_file_name, new_file_name)
+        except PermissionError:
+            pass
 
     try:
         os.remove(old_file_name)
diff --git a/tests/files/tests.py b/tests/files/tests.py
index 9777d09e1c..b3478d2732 100644
--- a/tests/files/tests.py
+++ b/tests/files/tests.py
@@ -419,35 +419,61 @@ class FileMoveSafeTests(unittest.TestCase):
         os.close(handle_a)
         os.close(handle_b)
 
-    def test_file_move_copystat_cifs(self):
+    def test_file_move_permissionerror(self):
         """
-        file_move_safe() ignores a copystat() EPERM PermissionError. This
-        happens when the destination filesystem is CIFS, for example.
+        file_move_safe() ignores PermissionError thrown by copystat() and
+        copymode().
+        For example, PermissionError can be raised when the destination
+        filesystem is CIFS, or when relabel is disabled by SELinux across
+        filesystems.
         """
-        copystat_EACCES_error = PermissionError(errno.EACCES, "msg")
-        copystat_EPERM_error = PermissionError(errno.EPERM, "msg")
+        permission_error = PermissionError(errno.EPERM, "msg")
+        os_error = OSError("msg")
         handle_a, self.file_a = tempfile.mkstemp()
         handle_b, self.file_b = tempfile.mkstemp()
+        handle_c, self.file_c = tempfile.mkstemp()
         try:
             # This exception is required to reach the copystat() call in
             # file_safe_move().
             with mock.patch("django.core.files.move.os.rename", side_effect=OSError()):
-                # An error besides EPERM isn't ignored.
+                # An error besides PermissionError isn't ignored.
                 with mock.patch(
-                    "django.core.files.move.copystat", side_effect=copystat_EACCES_error
+                    "django.core.files.move.copystat", side_effect=os_error
                 ):
-                    with self.assertRaises(PermissionError):
+                    with self.assertRaises(OSError):
                         file_move_safe(self.file_a, self.file_b, allow_overwrite=True)
-                # EPERM is ignored.
+                # When copystat() throws PermissionError, copymode() error besides
+                # PermissionError isn't ignored.
                 with mock.patch(
-                    "django.core.files.move.copystat", side_effect=copystat_EPERM_error
+                    "django.core.files.move.copystat", side_effect=permission_error
+                ):
+                    with mock.patch(
+                        "django.core.files.move.copymode", side_effect=os_error
+                    ):
+                        with self.assertRaises(OSError):
+                            file_move_safe(
+                                self.file_a, self.file_b, allow_overwrite=True
+                            )
+                # PermissionError raised by copystat() is ignored.
+                with mock.patch(
+                    "django.core.files.move.copystat", side_effect=permission_error
                 ):
                     self.assertIsNone(
                         file_move_safe(self.file_a, self.file_b, allow_overwrite=True)
                     )
+                    # PermissionError raised by copymode() is ignored too.
+                    with mock.patch(
+                        "django.core.files.move.copymode", side_effect=permission_error
+                    ):
+                        self.assertIsNone(
+                            file_move_safe(
+                                self.file_c, self.file_b, allow_overwrite=True
+                            )
+                        )
         finally:
             os.close(handle_a)
             os.close(handle_b)
+            os.close(handle_c)
 
 
 class SpooledTempTests(unittest.TestCase):