From 6ca47a7157a908525af80a18e5dfd7c68b31169f Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 13:31:37 -0500 Subject: [PATCH] Review: let a symlink resolve to an explicit archive root _tar_member_paths never recorded ".", so is_symlink_target_in_tar refused a symlink that resolves to the archive root even when the archive has an explicit "." member. A "tar -C dir ." archive containing "./sub/root -> .." extracted before this review round (tar.getmember(".") found the member) and on upstream 26.2.2, but failed with InstallationError after the member-name normalization change. Record "." only when a member normalizes to it, never implicitly, which matches the previous behaviour and upstream: without an explicit "." member the link is still refused. No security impact: this only removes a false rejection, and the realpath containment checks are unchanged (a link to the root resolves to the extract directory itself, which is inside it). Adds tests for the explicit-root (accepted) and implicit-root (refused) cases. Also restore the second blank line before test_tar_member_paths_terminates_on_doubled_slash (flake8 E302), which the previous review commit left out. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/utils/unpacking.py | 6 ++++ tests/unit/test_utils_unpacking.py | 45 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 740390c109..6039db965e 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -124,6 +124,12 @@ def _tar_member_paths(tar): paths = set() # type: Set[str] for member in tar.getmembers(): name = posixpath.normpath(member.name.replace("\\", "/")) + if name == ".": + # An explicit "." / "./" member is the archive root. Record it so a + # symlink resolving to the root (e.g. "./sub/root -> ..") is found, + # as tar.getmember(".") did. The root is never added implicitly. + paths.add(name) + continue while name not in ("", ".", "/"): paths.add(name) parent = posixpath.dirname(name) diff --git a/tests/unit/test_utils_unpacking.py b/tests/unit/test_utils_unpacking.py index 76d2e5bc73..b68d7de913 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -13,6 +13,7 @@ from pip._internal.exceptions import InstallationError from pip._internal.utils.unpacking import ( _tar_member_paths, + is_symlink_target_in_tar, is_within_directory, unpack_file, untar_file, @@ -424,6 +425,7 @@ def test_unpack_polyglot_ambiguous_rejected(tmpdir): with pytest.raises(InstallationError): unpack_file(archive, os.path.join(str(tmpdir), 'out')) + def test_tar_member_paths_terminates_on_doubled_slash(): """posixpath.dirname("//") == "//"; collecting parents must not loop.""" buf = io.BytesIO() @@ -440,3 +442,46 @@ def test_tar_member_paths_terminates_on_doubled_slash(): tar.close() assert "//abs/f.txt" in paths assert "//abs" in paths + + +def _root_link_tar(explicit_root): + # type: (bool) -> tarfile.TarFile + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w") as tar: + names = (["./"] if explicit_root else []) + ["./sub/"] + for name in names: + info = tarfile.TarInfo(name) + info.type = tarfile.DIRTYPE + tar.addfile(info) + link = tarfile.TarInfo("./sub/root") + link.type = tarfile.SYMTYPE + link.linkname = ".." + tar.addfile(link) + buf.seek(0) + return tarfile.open(fileobj=buf) + + +def test_symlink_to_explicit_archive_root_is_in_tar(): + """A "tar -C dir ." archive with "./sub/root -> .." keeps unpacking.""" + tar = _root_link_tar(explicit_root=True) + try: + paths = _tar_member_paths(tar) + assert "." in paths + assert is_symlink_target_in_tar( + tar, tar.getmember("./sub/root"), member_paths=paths + ) + finally: + tar.close() + + +def test_symlink_to_implicit_archive_root_is_not_in_tar(): + """Without an explicit "." member the root is not added (as upstream).""" + tar = _root_link_tar(explicit_root=False) + try: + paths = _tar_member_paths(tar) + assert "." not in paths + assert not is_symlink_target_in_tar( + tar, tar.getmember("./sub/root"), member_paths=paths + ) + finally: + tar.close()