Merge branch 'atomic-move' into Munki3dev

code/client/munkilib/installer/dmg.py

@@ -22,8 +22,10 @@ Routines for copying items from disk images
 """
 
 import os
+import shutil
 import stat
 import subprocess
+import tempfile
 import xattr
 
 from .. import display
@@ -110,7 +112,7 @@ def create_missing_dirs(destpath):
         return 0
 
 
-def remove_quarantine(some_path):
+def remove_quarantine_from_item(some_path):
     '''Removes com.apple.quarantine from some_path'''
     try:
         if "com.apple.quarantine" in xattr.xattr(some_path).list():
@@ -120,6 +122,17 @@ def remove_quarantine(some_path):
             "Error removing com.apple.quarantine from %s: %s", some_path, err)
 
 
+def remove_quarantine(some_path):
+    '''Removes com.apple.quarantine from some_path, recursively'''
+    remove_quarantine_from_item(some_path)
+    if os.path.isdir(some_path):
+        for (dirpath, dirnames, filenames) in os.walk(some_path, topdown=True):
+            for filename in filenames:
+                remove_quarantine_from_item(os.path.join(dirpath, filename))
+            for dirname in dirnames:
+                remove_quarantine_from_item(os.path.join(dirpath, dirname))
+
+
 def validate_source_and_destination(mountpoint, item):
     '''Validates source and destination for item to be copied from a mounted
     disk image. Returns a tuple of (errorcode, source_path, destination_path)'''
@@ -159,27 +172,6 @@ def validate_source_and_destination(mountpoint, item):
     full_destpath = os.path.join(
         destpath, os.path.basename(dest_itemname or source_itemname))
 
-    # remove item if it already exists at full_destpath
-    if os.path.exists(full_destpath):
-        retcode = subprocess.call(["/bin/rm", "-rf", full_destpath])
-        if retcode:
-            display.display_error(
-                "Error removing existing %s", full_destpath)
-            return (retcode, None, None)
-
-    if os.path.isdir(source_itempath):
-        # source item is a directory (application bundle counts as a dir!)
-        # to prevent a security vulnerability, recreate the destination item
-        # directory with 0700 mode to prevent other processes that may have
-        # write access to the parent directory from writing their own payloads
-        # to this directory
-        try:
-            os.makedirs(full_destpath, 0o0700)
-        except OSError, err:
-            display.display_error(
-                "Error creating %s: %s", full_destpath, err)
-            return (-1, None, None)
-
     return (0, source_itempath, full_destpath)
 
 
@@ -189,6 +181,7 @@ def copy_items_from_mountpoint(mountpoint, itemlist):
 
     If the 'destination_item' key is provided, items will be copied
     as its value.'''
+    temp_destination_dir = tempfile.mkdtemp(dir=osutils.tmpdir())
     for item in itemlist:
 
         (errorcode,
@@ -197,49 +190,45 @@ def copy_items_from_mountpoint(mountpoint, itemlist):
         if errorcode:
             return errorcode
 
-        # one last permissions check before we copy
-        if os.path.isdir(destination_path):
-            mode = os.stat(destination_path).st_mode & 0o7777
-            owner_uid = os.stat(destination_path).st_uid
-            # destination path that is a directory should have set the mode
-            # to 0700 and owner should be root. if mode and owner don't match,
-            # something insecure is happening
-            if mode != 0o0700 or owner_uid != 0:
-                display.display_error(
-                    "Error copying %s to %s: destination path is insecure.",
-                    source_path, destination_path)
-                return -1
-
         # validation passed, OK to copy
         display.display_status_minor(
             "Copying %s to %s",
             os.path.basename(source_path), destination_path)
 
+        temp_destination_path = os.path.join(
+            temp_destination_dir, os.path.basename(destination_path))
         # copy the file or directory, removing the quarantine xattr and
         # preserving HFS+ compression
         retcode = subprocess.call(["/usr/bin/ditto", "--noqtn",
-                                   source_path, destination_path])
+                                   source_path, temp_destination_path])
         if retcode:
             display.display_error(
-                "Error copying %s to %s", source_path, destination_path)
+                "Error copying %s to %s", source_path, temp_destination_path)
             return retcode
 
-        # if destination is a directory, set the mode to that of the source
-        # directory (since we set it to 0700 earlier for copying safety)
-        if os.path.isdir(destination_path):
-            source_mode = os.stat(source_path).st_mode & 0o7777
-            os.chmod(destination_path, source_mode)
-
         # remove com.apple.quarantine xattr since `man ditto` lies and doesn't
         # seem to actually always remove it
-        remove_quarantine(destination_path)
+        remove_quarantine(temp_destination_path)
 
         # set desired permissions for item
-        retcode = set_permissions(item, destination_path)
+        retcode = set_permissions(item, temp_destination_path)
         if retcode:
             return retcode
 
+        # mv temp_destination_path to final destination path
+        try:
+            if os.path.isdir(destination_path):
+                shutil.rmtree(destination_path)
+            os.rename(temp_destination_path, destination_path)
+        except (OSError, IOError), err:
+            display.display_error("Error moving item to destination: %s" % err)
+            return -1
+
     # all items copied successfully!
+    try:
+        os.rmdir(temp_destination_dir)
+    except (OSError, IOError):
+        pass
     return 0