From 40f50f26a88b00a188ee2b992cac0df2622db73d 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. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/utils/unpacking.py | 6 ++++ tests/unit/test_utils_unpacking.py | 43 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 8c2ac328b0..b15f1867a6 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -269,6 +269,12 @@ def _tar_member_paths(tar: tarfile.TarFile) -> set[str]: paths = set() 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 69e1bfe962..ad5b1189aa 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -18,6 +18,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, @@ -813,3 +814,45 @@ def test_polyglot_ambiguous_name_rejected(tmp_path: Path) -> None: _write_polyglot(archive) with pytest.raises(InstallationError): unpack_file(str(archive), str(tmp_path / "out")) + + +def _root_link_tar(explicit_root: 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() -> None: + """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() -> None: + """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()