when unpacking archives, don't preserve permissions except execution for regular files

pip/util.py

@@ -467,9 +467,22 @@ def get_terminal_size():
     return int(cr[1]), int(cr[0])
 
 
+def current_umask():
+    """Get the current umask which involves having to set it temporarily."""
+    mask = os.umask(0)
+    os.umask(mask)
+    return mask
+
+
 def unzip_file(filename, location, flatten=True):
-    """Unzip the file (zip file located at filename) to the destination
-    location"""
+    """
+    Unzip the file (with path `filename`) to the destination `location`.  All
+    files are written based on system defaults and umask (i.e. permissions are
+    not preserved), except that regular file members with any execute
+    permissions (user, group, or world) have "chmod +x" applied after being
+    written. Note that for windows, any execute changes using os.chmod are
+    no-ops per the python docs.
+    """
     if not os.path.exists(location):
         os.makedirs(location)
     zipfp = open(filename, 'rb')
@@ -496,17 +509,25 @@ def unzip_file(filename, location, flatten=True):
                     fp.write(data)
                 finally:
                     fp.close()
-                    unix_attributes = info.external_attr >> 16
-                    if unix_attributes:
-                        os.chmod(fn, unix_attributes)
-
-
+                    mode = info.external_attr >> 16
+                    # if mode and regular file and any execute permissions for user/group/world?
+                    if mode and stat.S_ISREG(mode) and  mode & 0o111:
+                        # make dest file have execute for user/group/world (chmod +x)
+                        # no-op on windows per python docs
+                        os.chmod(fn, (0o777-current_umask() | 0o111))
     finally:
         zipfp.close()
 
 
 def untar_file(filename, location):
-    """Untar the file (tar file located at filename) to the destination location"""
+    """
+    Untar the file (with path `filename`) to the destination `location`.
+    All files are written based on system defaults and umask (i.e. permissions
+    are not preserved), except that regular file members with any execute
+    permissions (user, group, or world) have "chmod +x" applied after being
+    written.  Note that for windows, any execute changes using os.chmod are
+    no-ops per the python docs.
+    """
     if not os.path.exists(location):
         os.makedirs(location)
     if filename.lower().endswith('.gz') or filename.lower().endswith('.tgz'):
@@ -565,6 +586,11 @@ def untar_file(filename, location):
                 finally:
                     destfp.close()
                 fp.close()
+                # member have any execute permissions for user/group/world?
+                if member.mode & 0o111:
+                    # make dest file have execute for user/group/world
+                    # no-op on windows per python docs
+                    os.chmod(path, (0o777-current_umask() | 0o111))
     finally:
         tar.close()
 

tests/unit/test_util.py

@@ -3,13 +3,17 @@ util tests
 
 """
 import os
+import stat
 import sys
+import shutil
+import tempfile
 
 from mock import Mock, patch
 from nose.tools import eq_, assert_raises
 from pip.exceptions import BadCommand
-from pip.util import egg_link_path, Inf, get_installed_distributions, find_command
-from tests.lib import reset_env, mkdir, write_file
+from pip.util import (egg_link_path, Inf, get_installed_distributions, find_command,
+                      untar_file, unzip_file)
+from tests.lib import reset_env, mkdir, write_file, tests_data
 
 
 class Tests_EgglinkPath:
@@ -273,6 +277,71 @@ def test_find_command_trys_supplied_pathext(mock_isfile, getpath_mock):
     assert not getpath_mock.called, "Should not call get_pathext"
 
 
+class TestUnpackArchives(object):
+    """
+    test_tar.tgz/test_tar.zip have content as follows engineered to confirm 3 things:
+     1) confirm that reg files, dirs, and symlinks get unpacked
+     2) permissions are not preserved (and go by the 022 umask)
+     3) reg files with *any* execute perms, get chmod +x
 
+       file.txt         600 regular file
+       symlink.txt      777 symlink to file.txt
+       script_owner.sh  700 script where owner can execute
+       script_group.sh  610 script where group can execute
+       script_world.sh  601 script where world can execute
+       dir              744 directory
+       dir/dirfile      622 regular file
 
+    """
 
+    def setup(self):
+        self.tempdir = tempfile.mkdtemp()
+        self.old_mask = os.umask(0o022)
+        self.symlink_expected_mode = None
+
+    def teardown(self):
+        os.umask(self.old_mask)
+        shutil.rmtree(self.tempdir, ignore_errors=True)
+
+    def mode(self, path):
+        return stat.S_IMODE(os.stat(path).st_mode)
+
+    def confirm_files(self):
+        # expections based on 022 umask set above and the unpack logic that sets
+        # execute permissions, not preservation
+        for fname, expected_mode, test in [
+            ('file.txt', 0o644, os.path.isfile),
+            ('symlink.txt', 0o644, os.path.isfile),
+            ('script_owner.sh', 0o755, os.path.isfile),
+            ('script_group.sh', 0o755, os.path.isfile),
+            ('script_world.sh', 0o755, os.path.isfile),
+            ('dir', 0o755, os.path.isdir),
+            (os.path.join('dir', 'dirfile'), 0o644, os.path.isfile),
+            ]:
+            path = os.path.join(self.tempdir, fname)
+            if path.endswith('symlink.txt') and sys.platform == 'win32':
+                # no symlinks created on windows
+                continue
+            assert test(path), path
+            if sys.platform == 'win32':
+                # the permissions tests below don't apply in windows
+                # due to os.chmod being a noop
+                continue
+            mode = self.mode(path)
+            assert mode == expected_mode, "mode: %s, expected mode: %s" % (mode, expected_mode)
+
+    def test_unpack_tgz(self):
+        """
+        Test unpacking a *.tgz, and setting execute permissions
+        """
+        test_file =  os.path.join(tests_data, 'packages', 'test_tar.tgz')
+        untar_file(test_file, self.tempdir)
+        self.confirm_files()
+
+    def test_unpack_zip(self):
+        """
+        Test unpacking a *.zip, and setting execute permissions
+        """
+        test_file =  os.path.join(tests_data, 'packages', 'test_zip.zip')
+        unzip_file(test_file, self.tempdir)
+        self.confirm_files()