Skip to content

Ensure selfcheck file inherits directory permissions #13528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/13528.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
selfcheck file in cache directory has same permissions as the rest of the cache.
17 changes: 6 additions & 11 deletions src/pip/_internal/network/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)

Expand Down
14 changes: 11 additions & 3 deletions src/pip/_internal/self_outdated_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
13 changes: 13 additions & 0 deletions src/pip/_internal/utils/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
4 changes: 4 additions & 0 deletions tests/unit/test_self_check_outdated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading