Merge branch 'main' into vendoring-updates

Author: notatallshaw

.github/workflows/ci.yml

@@ -201,6 +201,12 @@ jobs:
           python-version: ${{ matrix.python }}
           allow-prereleases: true
 
+      - name: Install SVN via Chocolatey
+        shell: pwsh
+        run: |
+          choco install svn -y --no-progress
+          echo "C:\Program Files (x86)\Subversion\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
+
       - run: pip install nox
 
       # Main check

news/13550.bugfix.rst

@@ -0,0 +1,2 @@
+For Python versions that do not support PEP 706, pip will now raise an installation error for a
+source distribution when it includes a symlink that points outside the source distribution archive.

src/pip/_internal/build_env.py

@@ -169,10 +169,14 @@ def install(
             args.append("--prefer-binary")
         args.append("--")
         args.extend(requirements)
+
+        identify_requirement = (
+            f" for {for_req.name}" if for_req and for_req.name else ""
+        )
         with open_spinner(f"Installing {kind}") as spinner:
             call_subprocess(
                 args,
-                command_desc=f"pip subprocess to install {kind}",
+                command_desc=f"installing {kind}{identify_requirement}",
                 spinner=spinner,
             )
 

src/pip/_internal/exceptions.py

@@ -190,6 +190,23 @@ class InstallationError(PipError):
     """General exception during installation"""
 
 
+class FailedToPrepareCandidate(InstallationError):
+    """Raised when we fail to prepare a candidate (i.e. fetch and generate metadata).
+
+    This is intentionally not a diagnostic error, since the output will be presented
+    above this error, when this occurs. This should instead present information to the
+    user.
+    """
+
+    def __init__(
+        self, *, package_name: str, requirement_chain: str, failed_step: str
+    ) -> None:
+        super().__init__(f"Failed to build '{package_name}' when {failed_step.lower()}")
+        self.package_name = package_name
+        self.requirement_chain = requirement_chain
+        self.failed_step = failed_step
+
+
 class MissingPyProjectBuildRequires(DiagnosticPipError):
     """Raised when pyproject.toml has `build-system`, but no `build-system.requires`."""
 
@@ -384,7 +401,7 @@ def __init__(
         output_lines: list[str] | None,
     ) -> None:
         if output_lines is None:
-            output_prompt = Text("See above for output.")
+            output_prompt = Text("No available output.")
         else:
             output_prompt = (
                 Text.from_markup(f"[red][{len(output_lines)} lines of output][/]\n")
@@ -412,15 +429,15 @@ def __str__(self) -> str:
         return f"{self.command_description} exited with {self.exit_code}"
 
 
-class MetadataGenerationFailed(InstallationSubprocessError, InstallationError):
+class MetadataGenerationFailed(DiagnosticPipError, InstallationError):
     reference = "metadata-generation-failed"
 
     def __init__(
         self,
         *,
         package_details: str,
     ) -> None:
-        super(InstallationSubprocessError, self).__init__(
+        super().__init__(
             message="Encountered error while generating package metadata.",
             context=escape(package_details),
             hint_stmt="See above for details.",

src/pip/_internal/resolution/resolvelib/candidates.py

@@ -10,6 +10,7 @@
 from pip._vendor.packaging.version import Version
 
 from pip._internal.exceptions import (
+    FailedToPrepareCandidate,
     HashError,
     InstallationSubprocessError,
     InvalidInstalledPackage,
@@ -244,9 +245,19 @@ def _prepare(self) -> BaseDistribution:
             e.req = self._ireq
             raise
         except InstallationSubprocessError as exc:
-            # The output has been presented already, so don't duplicate it.
-            exc.context = "See above for output."
-            raise
+            if isinstance(self._ireq.comes_from, InstallRequirement):
+                request_chain = self._ireq.comes_from.from_path()
+            else:
+                request_chain = self._ireq.comes_from
+
+            if request_chain is None:
+                request_chain = "directly requested"
+
+            raise FailedToPrepareCandidate(
+                package_name=self._ireq.name or str(self._link),
+                requirement_chain=request_chain,
+                failed_step=exc.command_description,
+            )
 
         self._check_metadata_consistency(dist)
         return dist

src/pip/_internal/utils/unpacking.py

@@ -248,13 +248,30 @@ def pip_filter(member: tarfile.TarInfo, path: str) -> tarfile.TarInfo:
         tar.close()
 
 
+def is_symlink_target_in_tar(tar: tarfile.TarFile, tarinfo: tarfile.TarInfo) -> bool:
+    """Check if the file pointed to by the symbolic link is in the tar archive"""
+    linkname = os.path.join(os.path.dirname(tarinfo.name), tarinfo.linkname)
+
+    linkname = os.path.normpath(linkname)
+    linkname = linkname.replace("\\", "/")
+
+    try:
+        tar.getmember(linkname)
+        return True
+    except KeyError:
+        return False
+
+
 def _untar_without_filter(
     filename: str,
     location: str,
     tar: tarfile.TarFile,
     leading: bool,
 ) -> None:
     """Fallback for Python without tarfile.data_filter"""
+    # NOTE: This function can be removed once pip requires CPython ≥ 3.12.​
+    # PEP 706 added tarfile.data_filter, made tarfile extraction operations more secure.
+    # This feature is fully supported from CPython 3.12 onward.
     for member in tar.getmembers():
         fn = member.name
         if leading:
@@ -269,6 +286,14 @@ def _untar_without_filter(
         if member.isdir():
             ensure_dir(path)
         elif member.issym():
+            if not is_symlink_target_in_tar(tar, member):
+                message = (
+                    "The tar file ({}) has a file ({}) trying to install "
+                    "outside target directory ({})"
+                )
+                raise InstallationError(
+                    message.format(filename, member.name, member.linkname)
+                )
             try:
                 tar._extract_member(member, path)
             except Exception as exc:

tests/lib/venv.py

@@ -113,7 +113,6 @@ def _create(self, clear: bool = False) -> None:
                 _virtualenv.cli_run(
                     [
                         "--no-pip",
-                        "--no-wheel",
                         "--no-setuptools",
                         os.fspath(self.location),
                     ],

tests/unit/test_utils_unpacking.py

@@ -10,6 +10,7 @@
 from pathlib import Path
 
 import pytest
+from _pytest.monkeypatch import MonkeyPatch
 
 from pip._internal.exceptions import InstallationError
 from pip._internal.utils.unpacking import is_within_directory, untar_file, unzip_file
@@ -238,6 +239,148 @@ def test_unpack_tar_links(self, input_prefix: str, unpack_prefix: str) -> None:
         with open(os.path.join(unpack_dir, "symlink.txt"), "rb") as f:
             assert f.read() == content
 
+    def test_unpack_normal_tar_link1_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a normal tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            file_data = io.BytesIO(b"normal\n")
+            normal_file_tarinfo = tarfile.TarInfo(name="normal_file")
+            normal_file_tarinfo.size = len(file_data.getbuffer())
+            tar.addfile(normal_file_tarinfo, fileobj=file_data)
+
+            info = tarfile.TarInfo("normal_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = "normal_file"
+            tar.addfile(info)
+
+        untar_file(tar_filepath, extract_path)
+
+        assert os.path.islink(os.path.join(extract_path, "normal_symlink"))
+
+        link_path = os.readlink(os.path.join(extract_path, "normal_symlink"))
+        assert link_path == "normal_file"
+
+        with open(os.path.join(extract_path, "normal_symlink"), "rb") as f:
+            assert f.read() == b"normal\n"
+
+    def test_unpack_normal_tar_link2_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a normal tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            file_data = io.BytesIO(b"normal\n")
+            normal_file_tarinfo = tarfile.TarInfo(name="normal_file")
+            normal_file_tarinfo.size = len(file_data.getbuffer())
+            tar.addfile(normal_file_tarinfo, fileobj=file_data)
+
+            info = tarfile.TarInfo("sub/normal_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = ".." + os.path.sep + "normal_file"
+            tar.addfile(info)
+
+        untar_file(tar_filepath, extract_path)
+
+        assert os.path.islink(os.path.join(extract_path, "sub", "normal_symlink"))
+
+        link_path = os.readlink(os.path.join(extract_path, "sub", "normal_symlink"))
+        assert link_path == ".." + os.path.sep + "normal_file"
+
+        with open(os.path.join(extract_path, "sub", "normal_symlink"), "rb") as f:
+            assert f.read() == b"normal\n"
+
+    def test_unpack_evil_tar_link1_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a evil tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        import_filename = "import_file"
+        import_filepath = os.path.join(self.tempdir, import_filename)
+        open(import_filepath, "w").close()
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            info = tarfile.TarInfo("evil_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = import_filepath
+            tar.addfile(info)
+
+        with pytest.raises(InstallationError) as e:
+            untar_file(tar_filepath, extract_path)
+
+        msg = (
+            "The tar file ({}) has a file ({}) trying to install outside "
+            "target directory ({})"
+        )
+        assert msg.format(tar_filepath, "evil_symlink", import_filepath) in str(e.value)
+
+        assert not os.path.exists(os.path.join(extract_path, "evil_symlink"))
+
+    def test_unpack_evil_tar_link2_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a evil tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        import_filename = "import_file"
+        import_filepath = os.path.join(self.tempdir, import_filename)
+        open(import_filepath, "w").close()
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        link_path = ".." + os.sep + import_filename
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            info = tarfile.TarInfo("evil_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = link_path
+            tar.addfile(info)
+
+        with pytest.raises(InstallationError) as e:
+            untar_file(tar_filepath, extract_path)
+
+        msg = (
+            "The tar file ({}) has a file ({}) trying to install outside "
+            "target directory ({})"
+        )
+        assert msg.format(tar_filepath, "evil_symlink", link_path) in str(e.value)
+
+        assert not os.path.exists(os.path.join(extract_path, "evil_symlink"))
+
 
 def test_unpack_tar_unicode(tmpdir: Path) -> None:
     test_tar = tmpdir / "test.tar"