Ensure selfcheck file inherits directory permissions (#13528)

When writing files to the cache, pip copies the permissions from the cache
directory. This is an expected behaviour to ensure that any user that has
access to the cache has also access to its content.

Copying the permissions explicitly is necessary due to the use of
adjacent_tmp_file() which by virtue of tempfile.mkstemp() (indirectly via
NamedTemporaryFile) causes all files to be created with 600
permissions.

This is not happening for the selfcheck cache, rendering it unaccessible
by users different from the one that first created the cache.

Signed-off-by: Alessandro Molina <[email protected]>
Co-authored-by: Richard Si <[email protected]>

Author: amol-

news/13528.bugfix.rst

@@ -0,0 +1 @@
+Ensure the self-check files in the cache has same permissions as the rest of the cache.

src/pip/_internal/network/cache.py

@@ -13,7 +13,11 @@
 from pip._vendor.cachecontrol.caches import SeparateBodyFileCache
 from pip._vendor.requests.models import Response
 
-from pip._internal.utils.filesystem import adjacent_tmp_file, replace
+from pip._internal.utils.filesystem import (
+    adjacent_tmp_file,
+    copy_directory_permissions,
+    replace,
+)
 from pip._internal.utils.misc import ensure_dir
 
 
@@ -82,16 +86,7 @@ def _write_to_file(self, path: str, writer_func: Callable[[BinaryIO], Any]) -> N
                 writer_func(f)
                 # Inherit the read/write permissions of the cache directory
                 # to enable multi-user cache use-cases.
-                mode = (
-                    os.stat(self.directory).st_mode
-                    & 0o666  # select read/write permissions of cache directory
-                    | 0o600  # set owner read/write permissions
-                )
-                # Change permissions only if there is no risk of following a symlink.
-                if os.chmod in os.supports_fd:
-                    os.chmod(f.fileno(), mode)
-                elif os.chmod in os.supports_follow_symlinks:
-                    os.chmod(f.name, mode, follow_symlinks=False)
+                copy_directory_permissions(self.directory, f)
 
             replace(f.name, path)
 

src/pip/_internal/self_outdated_check.py

@@ -27,7 +27,12 @@
     get_best_invocation_for_this_pip,
     get_best_invocation_for_this_python,
 )
-from pip._internal.utils.filesystem import adjacent_tmp_file, check_path_owner, replace
+from pip._internal.utils.filesystem import (
+    adjacent_tmp_file,
+    check_path_owner,
+    copy_directory_permissions,
+    replace,
+)
 from pip._internal.utils.misc import (
     ExternallyManagedEnvironment,
     check_externally_managed,
@@ -100,13 +105,15 @@ def set(self, pypi_version: str, current_time: datetime.datetime) -> None:
         if not self._statefile_path:
             return
 
+        statefile_directory = os.path.dirname(self._statefile_path)
+
         # Check to make sure that we own the directory
-        if not check_path_owner(os.path.dirname(self._statefile_path)):
+        if not check_path_owner(statefile_directory):
             return
 
         # Now that we've ensured the directory is owned by this user, we'll go
         # ahead and make sure that all our directories are created.
-        ensure_dir(os.path.dirname(self._statefile_path))
+        ensure_dir(statefile_directory)
 
         state = {
             # Include the key so it's easy to tell which pip wrote the
@@ -120,6 +127,7 @@ def set(self, pypi_version: str, current_time: datetime.datetime) -> None:
 
         with adjacent_tmp_file(self._statefile_path) as f:
             f.write(text.encode())
+            copy_directory_permissions(statefile_directory, f)
 
         try:
             # Since we have a prefix-specific state file, we can just

src/pip/_internal/utils/filesystem.py

@@ -150,3 +150,15 @@ def directory_size(path: str) -> int | float:
 
 def format_directory_size(path: str) -> str:
     return format_size(directory_size(path))
+
+
+def copy_directory_permissions(directory: str, target_file: BinaryIO) -> None:
+    mode = (
+        os.stat(directory).st_mode & 0o666  # select read/write permissions of directory
+        | 0o600  # set owner read/write permissions
+    )
+    # Change permissions only if there is no risk of following a symlink.
+    if os.chmod in os.supports_fd:
+        os.chmod(target_file.fileno(), mode)
+    elif os.chmod in os.supports_follow_symlinks:
+        os.chmod(target_file.name, mode, follow_symlinks=False)

tests/unit/test_self_check_outdated.py

@@ -189,6 +189,11 @@ def test_writes_expected_statefile(self, tmpdir: Path) -> None:
             "last_check": "2000-01-01T00:00:00+00:00",
             "pypi_version": "1.0.0",
         }
+        # Check that the self-check cache entries inherit the root cache permissions.
+        statefile_permissions = os.stat(expected_path).st_mode & 0o666
+        selfcheckdir_permissions = os.stat(cache_dir / "selfcheck").st_mode & 0o666
+        cache_permissions = os.stat(cache_dir).st_mode & 0o666
+        assert statefile_permissions == selfcheckdir_permissions == cache_permissions
 
 
 @patch("pip._internal.self_outdated_check._self_version_check_logic")