Extract zip files with current default fs permission. (#3119)

However add the executable bits if the zipped file had it. This match
pip behavior and fix #3118.

---------

Co-authored-by: John Sirois <john.sirois@gmail.com>

Author: leameow

CHANGES.md

@@ -1,5 +1,12 @@
 # Release Notes
 
+## 2.91.3
+
+This release fixes extraction of wheels containing entries with bad permissions. Instead of
+preserving zip entry permissions, just the executable bit is preserved for file entries.
+
+* Extract zip files with current default fs permission. (#3119)
+
 ## 2.91.2
 
 This release fixes hermeticity of the Pex boot code against the Python stdlib itself. In some corner

pex/common.py

@@ -309,9 +309,9 @@ def from_path(cls, path):
     class Value(_ZipFileTypeValue):
         pass
 
-    DIRECTORY = Value("directory", 0o755)
-    EXECUTABLE = Value("executable", 0o755)
-    FILE = Value("file", 0o644)
+    DIRECTORY = Value("directory", stat.S_IFDIR | 0o755)
+    EXECUTABLE = Value("executable", stat.S_IFREG | 0o755)
+    FILE = Value("file", stat.S_IFREG | 0o644)
 
 
 ZipFileType.seal()
@@ -449,7 +449,9 @@ def _chmod(
         #   https://www.forensicswiki.org/wiki/ZIP#External_file_attributes
         if info.external_attr > 0xFFFF:
             attr = info.external_attr >> 16
-            os.chmod(path, attr)
+            # If the archive file was executable, we set the extracted file as executable.
+            if stat.S_ISREG(attr) and attr & 0o111:
+                chmod_plus_x(path)
 
     # Python 3 also takes PathLike[str] for the path arg, but we only ever pass str since we support
     # Python 2.7 and don't use pathlib as a result.

pex/version.py

@@ -1,4 +1,4 @@
 # Copyright 2015 Pex project contributors.
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-__version__ = "2.91.2"
+__version__ = "2.91.3"

tests/integration/test_issue_3118.py

@@ -0,0 +1,46 @@
+# Copyright 2026 Pex project contributors.
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import
+
+import subprocess
+import sys
+
+import pytest
+
+from pex.compatibility import commonpath
+from testing import run_pex_command
+from testing.pytest_utils.tmp import Tempdir
+
+
+@pytest.mark.skipif(
+    sys.version_info < (3, 9),
+    reason="The ag-ui-protocol 0.1.14 distribution under test requires Python >=3.9.",
+)
+def test_bad_perms_ignored(tmpdir):
+    # type: (Tempdir) -> None
+
+    pex_root = tmpdir.join("pex-root")
+    pex = tmpdir.join("pex")
+    run_pex_command(
+        args=[
+            "--pex-root",
+            pex_root,
+            "--runtime-pex-root",
+            pex_root,
+            "ag-ui-protocol==0.1.14",
+            "--intransitive",
+            "--ignore-errors",
+            "-o",
+            pex,
+        ]
+    ).assert_success()
+
+    assert pex_root == commonpath(
+        (
+            pex_root,
+            subprocess.check_output(args=[pex, "-c", "import ag_ui; print(ag_ui.__file__)"])
+            .decode("utf-8")
+            .strip(),
+        )
+    )

tests/test_common.py

@@ -4,21 +4,25 @@
 import contextlib
 import errno
 import os
+import stat
 
 import pytest
 
 from pex.common import (
     Chroot,
     ZipFileEx,
+    ZipFileType,
     deterministic_walk,
     open_zip,
+    safe_mkdir,
     safe_open,
     temporary_dir,
     touch,
 )
 from pex.executables import chmod_plus_x
 from pex.typing import TYPE_CHECKING
 from testing import NonDeterministicWalk
+from testing.pytest_utils.tmp import Tempdir
 
 try:
     from unittest import mock
@@ -29,51 +33,103 @@
     from typing import Iterator, Tuple
 
 
+def test_zip_file_type_mode(tmpdir):
+    # type: (Tempdir) -> None
+
+    directory = safe_mkdir(tmpdir.join("dir"))
+    zip_dir = ZipFileType.from_path(directory)
+    assert stat.S_ISDIR(zip_dir.deterministic_mode)
+    assert 0o755 == 0o755 & zip_dir.deterministic_mode
+
+    regular_file = touch(tmpdir.join("file"))
+    zip_reg_file = ZipFileType.from_path(regular_file)
+    assert stat.S_ISREG(zip_reg_file.deterministic_mode)
+    assert 0o644 == 0o644 & zip_reg_file.deterministic_mode
+
+    executable_file = touch(tmpdir.join("exe"))
+    chmod_plus_x(executable_file)
+    zip_exe_file = ZipFileType.from_path(executable_file)
+    assert stat.S_ISREG(zip_exe_file.deterministic_mode)
+    assert 0o755 == 0o755 & zip_exe_file.deterministic_mode
+
+
 def extract_perms(path):
     # type: (str) -> str
     return oct(os.stat(path).st_mode)
 
 
 @contextlib.contextmanager
 def zip_fixture():
-    # type: () -> Iterator[Tuple[str, str, str, str]]
+    # type: () -> Iterator[Tuple[str, str]]
     with temporary_dir() as target_dir:
-        one = os.path.join(target_dir, "one")
-        touch(one)
+        no_x = os.path.join(target_dir, "no-x")
+        touch(no_x)
 
-        two = os.path.join(target_dir, "two")
-        touch(two)
-        chmod_plus_x(two)
+        no_w = os.path.join(target_dir, "no-w")
+        touch(no_w)
+        os.chmod(no_w, 0o444)
+
+        with_x = os.path.join(target_dir, "with-x")
+        touch(with_x)
+        chmod_plus_x(with_x)
 
-        assert extract_perms(one) != extract_perms(two)
+        assert extract_perms(no_x) != extract_perms(no_w) != extract_perms(with_x)
 
         zip_file = os.path.join(target_dir, "test.zip")
         with contextlib.closing(ZipFileEx(zip_file, "w")) as zf:
-            zf.write(one, "one")
-            zf.write(two, "two")
+            zf.write(no_x, "no-x")
+            zf.write(no_w, "no-w")
+            zf.write(with_x, "with-x")
 
-        yield zip_file, os.path.join(target_dir, "extract"), one, two
+        yield zip_file, os.path.join(target_dir, "extract")
+
+
+def is_writeable(path):
+    # type: (str) -> bool
+    return bool(os.stat(path).st_mode & 0o222)
+
+
+def is_executable(path):
+    # type: (str) -> bool
+    return bool(os.stat(path).st_mode & 0o111)
 
 
 def test_perm_preserving_zipfile_extractall():
     # type: () -> None
-    with zip_fixture() as (zip_file, extract_dir, one, two):
+    with zip_fixture() as (zip_file, extract_dir):
         with contextlib.closing(ZipFileEx(zip_file)) as zf:
             zf.extractall(extract_dir)
 
-            assert extract_perms(one) == extract_perms(os.path.join(extract_dir, "one"))
-            assert extract_perms(two) == extract_perms(os.path.join(extract_dir, "two"))
+            no_x = os.path.join(extract_dir, "no-x")
+            no_w = os.path.join(extract_dir, "no-w")
+            with_x = os.path.join(extract_dir, "with-x")
+
+            assert not is_executable(no_x)
+            assert not is_executable(no_w)
+            assert is_executable(with_x)
+
+            files = [no_x, no_w, with_x]
+            assert all(is_writeable(f) for f in files)
 
 
 def test_perm_preserving_zipfile_extract():
     # type: () -> None
-    with zip_fixture() as (zip_file, extract_dir, one, two):
+    with zip_fixture() as (zip_file, extract_dir):
         with contextlib.closing(ZipFileEx(zip_file)) as zf:
-            zf.extract("one", path=extract_dir)
-            zf.extract("two", path=extract_dir)
+            zf.extract("no-x", path=extract_dir)
+            zf.extract("no-w", path=extract_dir)
+            zf.extract("with-x", path=extract_dir)
+
+            no_x = os.path.join(extract_dir, "no-x")
+            no_w = os.path.join(extract_dir, "no-w")
+            with_x = os.path.join(extract_dir, "with-x")
+
+            assert not is_executable(no_x)
+            assert not is_executable(no_w)
+            assert is_executable(with_x)
 
-            assert extract_perms(one) == extract_perms(os.path.join(extract_dir, "one"))
-            assert extract_perms(two) == extract_perms(os.path.join(extract_dir, "two"))
+            files = [no_x, no_w, with_x]
+            assert all(is_writeable(f) for f in files)
 
 
 def assert_chroot_perms(copyfn):
@@ -98,8 +154,8 @@ def assert_chroot_perms(copyfn):
                 with contextlib.closing(ZipFileEx(zip_path)) as zf:
                     zf.extractall(extract_dir)
 
-                    assert extract_perms(one) == extract_perms(os.path.join(extract_dir, "one"))
-                    assert extract_perms(two) == extract_perms(os.path.join(extract_dir, "two"))
+                    assert not is_executable(os.path.join(extract_dir, "one"))
+                    assert is_executable(os.path.join(extract_dir, "two"))
 
 
 def test_chroot_perms_copy():