From 2e4aaa2cc02ac2b006763762f7af60a4a6d9a917 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Wed, 6 Aug 2025 13:32:50 +0200 Subject: [PATCH 1/3] Ensure selfcheck file inherits directory permissions --- news/13528.bugfix.rst | 1 + src/pip/_internal/network/cache.py | 17 ++++++----------- src/pip/_internal/self_outdated_check.py | 14 +++++++++++--- src/pip/_internal/utils/filesystem.py | 13 +++++++++++++ tests/unit/test_self_check_outdated.py | 4 ++++ 5 files changed, 35 insertions(+), 14 deletions(-) create mode 100644 news/13528.bugfix.rst diff --git a/news/13528.bugfix.rst b/news/13528.bugfix.rst new file mode 100644 index 00000000000..3d42b06dd3f --- /dev/null +++ b/news/13528.bugfix.rst @@ -0,0 +1 @@ +selfcheck file in cache directory has same permissions as the rest of the cache. diff --git a/src/pip/_internal/network/cache.py b/src/pip/_internal/network/cache.py index 0c5961c45b4..2a372f2e008 100644 --- a/src/pip/_internal/network/cache.py +++ b/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) diff --git a/src/pip/_internal/self_outdated_check.py b/src/pip/_internal/self_outdated_check.py index ca507f113a4..5999ddb3737 100644 --- a/src/pip/_internal/self_outdated_check.py +++ b/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 diff --git a/src/pip/_internal/utils/filesystem.py b/src/pip/_internal/utils/filesystem.py index d7c05243876..dd7dee35838 100644 --- a/src/pip/_internal/utils/filesystem.py +++ b/src/pip/_internal/utils/filesystem.py @@ -150,3 +150,16 @@ 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 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(target_file.fileno(), mode) + elif os.chmod in os.supports_follow_symlinks: + os.chmod(target_file.name, mode, follow_symlinks=False) diff --git a/tests/unit/test_self_check_outdated.py b/tests/unit/test_self_check_outdated.py index 55e6928a57a..76d14999fc5 100644 --- a/tests/unit/test_self_check_outdated.py +++ b/tests/unit/test_self_check_outdated.py @@ -190,6 +190,10 @@ def test_writes_expected_statefile(self, tmpdir: Path) -> None: "pypi_version": "1.0.0", } + statefile_permissions = os.stat(expected_path).st_mode & 0o666 + selfcheckdir_permissions = os.stat(cache_dir / "selfcheck").st_mode & 0o666 + assert statefile_permissions == selfcheckdir_permissions + @patch("pip._internal.self_outdated_check._self_version_check_logic") def test_suppressed_by_externally_managed(mocked_function: Mock, tmpdir: Path) -> None: From 239d109a16ab8eba48d08a7878c39d8ab7c9514b Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Thu, 14 Aug 2025 18:25:19 +0200 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Richard Si --- news/13528.bugfix.rst | 2 +- src/pip/_internal/utils/filesystem.py | 2 +- tests/unit/test_self_check_outdated.py | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/news/13528.bugfix.rst b/news/13528.bugfix.rst index 3d42b06dd3f..bd2ee8c6897 100644 --- a/news/13528.bugfix.rst +++ b/news/13528.bugfix.rst @@ -1 +1 @@ -selfcheck file in cache directory has same permissions as the rest of the cache. +Ensure the self-check files in the cache has same permissions as the rest of the cache. diff --git a/src/pip/_internal/utils/filesystem.py b/src/pip/_internal/utils/filesystem.py index dd7dee35838..8fb660ae34b 100644 --- a/src/pip/_internal/utils/filesystem.py +++ b/src/pip/_internal/utils/filesystem.py @@ -155,7 +155,7 @@ def format_directory_size(path: str) -> str: def copy_directory_permissions(directory: str, target_file: BinaryIO) -> None: mode = ( os.stat(directory).st_mode - & 0o666 # select read/write permissions of cache directory + & 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. diff --git a/tests/unit/test_self_check_outdated.py b/tests/unit/test_self_check_outdated.py index 76d14999fc5..0e0208d516c 100644 --- a/tests/unit/test_self_check_outdated.py +++ b/tests/unit/test_self_check_outdated.py @@ -189,10 +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 - assert statefile_permissions == selfcheckdir_permissions + 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") From 152672a34b5882e1816214b34cd6b79cb2c45c9e Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Thu, 14 Aug 2025 18:29:27 +0200 Subject: [PATCH 3/3] formatting of review suggestions Signed-off-by: Alessandro Molina --- src/pip/_internal/utils/filesystem.py | 3 +-- tests/unit/test_self_check_outdated.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/utils/filesystem.py b/src/pip/_internal/utils/filesystem.py index 8fb660ae34b..e34ffcf6dcd 100644 --- a/src/pip/_internal/utils/filesystem.py +++ b/src/pip/_internal/utils/filesystem.py @@ -154,8 +154,7 @@ def format_directory_size(path: str) -> str: def copy_directory_permissions(directory: str, target_file: BinaryIO) -> None: mode = ( - os.stat(directory).st_mode - & 0o666 # select read/write permissions of directory + 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. diff --git a/tests/unit/test_self_check_outdated.py b/tests/unit/test_self_check_outdated.py index 0e0208d516c..4b3c36f6826 100644 --- a/tests/unit/test_self_check_outdated.py +++ b/tests/unit/test_self_check_outdated.py @@ -189,7 +189,7 @@ 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. + # 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