From 1fd2baeeec598a25a435360cf3640be5c70407c3 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Wed, 21 Oct 2026 18:23:11 -0500 Subject: [PATCH 01/10] Backport CVE-2026-6357: run the self-version check before the command body GHSA-jp4c-xjxw-mgf9. pip ran its self-version check in the `finally` of Command._main, so after `pip install` had already written packages into site-packages. The check builds a PackageFinder and computes the supported tags, and the vendored packaging 20.8 tags._is_manylinux_compatible probes `import _manylinux` (the PEP 513/600 override hook) every time. A failed import is never cached in sys.modules, so the probe is retried after the install and imports a top-level `_manylinux.py` from a just-installed wheel. Reproduced on released 20.3.4.post2 under Python 2.7 with a wheel containing only `_manylinux.py`: pip executed it from the post-install check. `--disable-pip-version-check` avoided it. The June 20.3.4+security.1 assessment called this not applicable on the grounds that the check only touches already-imported modules. It missed this retried hook import. Split the check into a fetch half, run before the command body, and an emit half, which only logs afterwards: - self_outdated_check: new _pip_self_version_check_fetch(session, options) does the index lookup, state file and "installed by pip" decision and returns (installed, latest) or None. _pip_self_version_check_emit(prompt) only logs. pip_self_version_check(session, options) keeps its signature and behaviour as fetch followed by emit. - Command gains internal hooks _pip_version_check_fetch(options, args) and _pip_version_check_emit(options, fetched). _main() calls fetch before run() and emit in the existing `finally`. The default emit still calls handle_pip_version_check(options), so non-index commands behave as before. - IndexGroupCommand fetches before the body (with the same disable / --no-index short-circuit and error handling) and only emits afterwards. It does not fall back to handle_pip_version_check(), which stays public with an unchanged signature but is no longer called from _main for index commands. - InstallCommand skips the check when pip itself is a requirement, as upstream does. Upstream: pypa/pip#13923 (b369bfc96), released in 26.1. Reference ports: the AS 24.0 (bedea55d8) and 25.0.1 (57fcc05d0) lines. Py2.7 adaptation: no contextlib.ExitStack, so instead of upstream's pip_version_check() context manager the split is two internal hook methods around the existing try/finally. Public signatures (handle_pip_version_check, pip_self_version_check, was_installed_by_pip) are unchanged. Type comments, str.format. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/cli/base_command.py | 25 +++++- src/pip/_internal/cli/req_command.py | 43 ++++++++- src/pip/_internal/commands/install.py | 23 ++++- src/pip/_internal/self_outdated_check.py | 67 ++++++++++---- tests/unit/test_base_command.py | 26 ++++++ tests/unit/test_commands.py | 108 +++++++++++++++++++++++ tests/unit/test_self_check_outdated.py | 72 +++++++++++++++ 7 files changed, 346 insertions(+), 18 deletions(-) diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index 41e7dcf101..f30ae43d00 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -100,6 +100,25 @@ def handle_pip_version_check(self, options): # are present. assert not hasattr(options, 'no_index') + def _pip_version_check_fetch(self, options, args): + # type: (Values, List[str]) -> Any + """Gather what the pip version check needs, before the command runs. + + The result is passed to _pip_version_check_emit() once the command + body has finished. Doing the lookups up front means nothing is + imported after packages have been installed (CVE-2026-6357). + """ + return None + + def _pip_version_check_emit(self, options, fetched): + # type: (Values, Any) -> None + """Report the pip version check after the command has run. + + By default this calls handle_pip_version_check(), which is a no-op + for commands without the index_group options. + """ + self.handle_pip_version_check(options) + def run(self, options, args): # type: (Values, List[Any]) -> int raise NotImplementedError @@ -219,6 +238,10 @@ def _main(self, args): "This will become an error in pip 21.0." ) + # The version check does its lookups before the command body runs and + # only reports afterwards, so nothing is imported once packages have + # been installed (CVE-2026-6357). + version_check = self._pip_version_check_fetch(options, args) try: status = self.run(options, args) assert isinstance(status, int) @@ -257,4 +280,4 @@ def _main(self, args): return UNKNOWN_ERROR finally: - self.handle_pip_version_check(options) + self._pip_version_check_emit(options, version_check) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 008066ab1c..cd93c1cdce 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -27,7 +27,11 @@ install_req_from_req_string, ) from pip._internal.req.req_file import parse_requirements -from pip._internal.self_outdated_check import pip_self_version_check +from pip._internal.self_outdated_check import ( + _pip_self_version_check_emit, + _pip_self_version_check_fetch, + pip_self_version_check, +) from pip._internal.utils.temp_dir import tempdir_kinds from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -152,6 +156,43 @@ def handle_pip_version_check(self, options): with session: pip_self_version_check(session, options) + def _pip_version_check_fetch(self, options, args): + # type: (Values, List[str]) -> Any + """Query the index for the latest pip before the command body runs. + + handle_pip_version_check() does the same check in one step, but run + after `pip install` it would compute supported tags, which probes + ``import _manylinux``, and could import a module that a just-installed + wheel provided (CVE-2026-6357). So _main() fetches here and only + reports in _pip_version_check_emit(). + """ + # Make sure the index_group options are present. + assert hasattr(options, 'no_index') + + if options.disable_pip_version_check or options.no_index: + return None + + try: + session = self._build_session( + options, + retries=0, + timeout=min(5, options.timeout) + ) + with session: + return _pip_self_version_check_fetch(session, options) + except Exception: + logger.debug( + "There was an error checking the latest version of pip", + exc_info=True, + ) + return None + + def _pip_version_check_emit(self, options, fetched): + # type: (Values, Any) -> None + # Only report what was fetched before the command ran. Do not fall + # back to handle_pip_version_check(), which would query again now. + _pip_self_version_check_emit(fetched) + KEEPABLE_TEMPDIR_TYPES = [ tempdir_kinds.BUILD_ENV, diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 0f6c384e5c..5ae20e106f 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -9,6 +9,7 @@ from optparse import SUPPRESS_HELP from pip._vendor import pkg_resources +from pip._vendor.packaging.requirements import InvalidRequirement, Requirement from pip._vendor.packaging.utils import canonicalize_name from pip._internal.cache import WheelCache @@ -37,7 +38,7 @@ if MYPY_CHECK_RUNNING: from optparse import Values - from typing import Iterable, List, Optional + from typing import Any, Iterable, List, Optional from pip._internal.models.format_control import FormatControl from pip._internal.operations.check import ConflictDetails @@ -61,6 +62,15 @@ def check_binary_allowed(req): return check_binary_allowed +def _arg_refers_to_pip(arg): + # type: (str) -> bool + try: + req = Requirement(arg) + except InvalidRequirement: + return False + return canonicalize_name(req.name) == "pip" + + class InstallCommand(RequirementCommand): """ Install packages from: @@ -225,6 +235,17 @@ def add_options(self): self.parser.insert_option_group(0, index_opts) self.parser.insert_option_group(0, self.cmd_opts) + def _pip_version_check_fetch(self, options, args): + # type: (Values, List[str]) -> Any + # Skip the self-version check when pip itself is a requirement. The + # running pip may be replaced mid-command, and the upgrade prompt + # is redundant. + if any(_arg_refers_to_pip(arg) for arg in args): + return None + return super(InstallCommand, self)._pip_version_check_fetch( + options, args + ) + @with_cleanup def run(self, options, args): # type: (Values, List[str]) -> int diff --git a/src/pip/_internal/self_outdated_check.py b/src/pip/_internal/self_outdated_check.py index c2d166b184..cb10de6f1c 100644 --- a/src/pip/_internal/self_outdated_check.py +++ b/src/pip/_internal/self_outdated_check.py @@ -20,7 +20,7 @@ if MYPY_CHECK_RUNNING: import optparse - from typing import Any, Dict, Text, Union + from typing import Any, Dict, Optional, Text, Tuple, Union from pip._internal.network.session import PipSession @@ -118,10 +118,32 @@ def pip_self_version_check(session, options): Limit the frequency of checks to once per week. State is stored either in the active virtualenv or in the user's USER_CACHE_DIR keyed off the prefix of the pip script path. + + This fetches and reports in one go. Commands run the two halves + separately (_pip_self_version_check_fetch before the command body, + _pip_self_version_check_emit after it) so that nothing is looked up or + imported once packages have been installed (CVE-2026-6357). + """ + _pip_self_version_check_emit( + _pip_self_version_check_fetch(session, options) + ) + + +def _pip_self_version_check_fetch(session, options): + # type: (PipSession, optparse.Values) -> Optional[Tuple[str, str]] + """Do all the work of the pip self-version check, except reporting. + + Returns ``(installed_version, pypi_version)`` when an upgrade prompt + should be shown, otherwise None. This queries the index, computes the + supported tags and inspects the installed pip distribution, so it must + run before a command can install anything: the vendored packaging probes + ``import _manylinux`` while computing tags, and a failed import is not + cached, so after an install it would import whatever module of that name + a just-installed wheel provided (CVE-2026-6357). """ installed_version = get_installed_version("pip") if not installed_version: - return + return None pip_version = packaging_version.parse(installed_version) pypi_version = None @@ -161,7 +183,7 @@ def pip_self_version_check(session, options): ) best_candidate = finder.find_best_candidate("pip").best_candidate if best_candidate is None: - return + return None pypi_version = str(best_candidate.version) # save that we've performed a check @@ -177,21 +199,36 @@ def pip_self_version_check(session, options): # Determine if our pypi_version is older if not local_version_is_older: - return + return None - # We cannot tell how the current pip is available in the current - # command context, so be pragmatic here and suggest the command - # that's always available. This does not accommodate spaces in - # `sys.executable`. - pip_cmd = "{} -m pip".format(sys.executable) - logger.warning( - "You are using pip version %s; however, version %s is " - "available.\nYou should consider upgrading via the " - "'%s install --upgrade pip' command.", - pip_version, pypi_version, pip_cmd - ) + return str(pip_version), str(pypi_version) except Exception: logger.debug( "There was an error checking the latest version of pip", exc_info=True, ) + return None + + +def _pip_self_version_check_emit(upgrade_prompt): + # type: (Optional[Tuple[str, str]]) -> None + """Report the result of _pip_self_version_check_fetch, if any. + + Only logs: it must not query the index or import anything, because it + runs after the command body (CVE-2026-6357). + """ + if upgrade_prompt is None: + return + + pip_version, pypi_version = upgrade_prompt + # We cannot tell how the current pip is available in the current + # command context, so be pragmatic here and suggest the command + # that's always available. This does not accommodate spaces in + # `sys.executable`. + pip_cmd = "{} -m pip".format(sys.executable) + logger.warning( + "You are using pip version %s; however, version %s is " + "available.\nYou should consider upgrading via the " + "'%s install --upgrade pip' command.", + pip_version, pypi_version, pip_cmd + ) diff --git a/tests/unit/test_base_command.py b/tests/unit/test_base_command.py index 8ba3d9e252..65a6ef614c 100644 --- a/tests/unit/test_base_command.py +++ b/tests/unit/test_base_command.py @@ -96,6 +96,32 @@ def test_handle_pip_version_check_called(mock_handle_version_check): mock_handle_version_check.assert_called_once() +def test_version_check_fetch_before_run_emit_after(monkeypatch): + """ + CVE-2026-6357: _main() fetches for the version check before the command + body and reports after it. + """ + events = [] + + def fetch(self, options, args): + events.append('fetch') + return 'fetched' + + def emit(self, options, fetched): + events.append(('emit', fetched)) + + monkeypatch.setattr(Command, '_pip_version_check_fetch', fetch) + monkeypatch.setattr(Command, '_pip_version_check_emit', emit) + + def run_func(): + events.append('run') + return SUCCESS + + cmd = FakeCommand(run_func=run_func) + assert cmd.main([]) == SUCCESS + assert events == ['fetch', 'run', ('emit', 'fetched')] + + def test_log_command_success(fixed_time, tmpdir): """Test the --log option logs when command succeeds.""" cmd = FakeCommand() diff --git a/tests/unit/test_commands.py b/tests/unit/test_commands.py index 7fae427c69..c3f4d44fd2 100644 --- a/tests/unit/test_commands.py +++ b/tests/unit/test_commands.py @@ -6,7 +6,9 @@ RequirementCommand, SessionCommandMixin, ) +from pip._internal.cli.status_codes import SUCCESS from pip._internal.commands import commands_dict, create_command +from pip._internal.commands.install import _arg_refers_to_pip # These are the expected names of the commands whose classes inherit from # IndexGroupCommand. @@ -111,3 +113,109 @@ def is_requirement_command(command): return isinstance(command, RequirementCommand) check_commands(is_requirement_command, ['download', 'install', 'wheel']) + + +@pytest.mark.parametrize('command_name', EXPECTED_INDEX_GROUP_COMMANDS) +def test_index_group_version_check_runs_before_command_body( + monkeypatch, command_name, +): + """ + CVE-2026-6357: the self-version check must do its lookups before the + command body (which may install packages) and only report afterwards. + Run after `pip install`, the lookup computes supported tags, which + probes ``import _manylinux`` and could execute a just-installed module. + """ + # The isolate fixture disables the check through the environment. + monkeypatch.delenv('PIP_DISABLE_PIP_VERSION_CHECK', raising=False) + events = [] + fetched = object() + + def fake_fetch(session, options): + events.append('fetch') + return fetched + + def fake_emit(prompt): + events.append(('emit', prompt is fetched)) + + def fake_full_check(session, options): + events.append('full-check') + + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_fetch', + fake_fetch, + ) + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_emit', + fake_emit, + ) + monkeypatch.setattr( + 'pip._internal.cli.req_command.pip_self_version_check', + fake_full_check, + ) + + command = create_command(command_name) + + def fake_run(options, args): + events.append('run') + return SUCCESS + + monkeypatch.setattr(command, 'run', fake_run) + + assert command.main([]) == SUCCESS + assert events == ['fetch', 'run', ('emit', True)] + + +@pytest.mark.parametrize('command_name', EXPECTED_INDEX_GROUP_COMMANDS) +@pytest.mark.parametrize('extra_args', [ + ['--disable-pip-version-check'], + ['--no-index'], +]) +def test_index_group_version_check_disabled( + monkeypatch, command_name, extra_args, +): + events = [] + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_fetch', + lambda session, options: events.append('fetch'), + ) + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_emit', + lambda prompt: events.append(('emit', prompt)), + ) + command = create_command(command_name) + monkeypatch.setattr(command, 'run', lambda options, args: SUCCESS) + + assert command.main(extra_args) == SUCCESS + assert events == [('emit', None)] + + +@pytest.mark.parametrize('arg, expected', [ + ('pip', True), + ('PIP==20.3.4', True), + ('pip>=20', True), + ('pip[foo]', True), + ('pipx', False), + ('six', False), + ('./pip-20.3.4-py2.py3-none-any.whl', False), + ('-r', False), +]) +def test_arg_refers_to_pip(arg, expected): + assert _arg_refers_to_pip(arg) is expected + + +def test_install_skips_version_check_when_installing_pip(monkeypatch): + calls = [] + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_fetch', + lambda session, options: calls.append('fetch'), + ) + command = create_command('install') + options = command.parser.get_default_values() + options.disable_pip_version_check = False + options.no_index = False + + assert command._pip_version_check_fetch(options, ['pip']) is None + assert calls == [] + + command._pip_version_check_fetch(options, ['six']) + assert calls == ['fetch'] diff --git a/tests/unit/test_self_check_outdated.py b/tests/unit/test_self_check_outdated.py index c5e60d92fc..6a92f9e0a9 100644 --- a/tests/unit/test_self_check_outdated.py +++ b/tests/unit/test_self_check_outdated.py @@ -11,6 +11,8 @@ from pip._internal.models.candidate import InstallationCandidate from pip._internal.self_outdated_check import ( SelfCheckState, + _pip_self_version_check_emit, + _pip_self_version_check_fetch, logger, pip_self_version_check, ) @@ -139,6 +141,76 @@ def test_pip_self_version_check(monkeypatch, stored_time, installed_ver, assert len(logger.warning.calls) == 0 +def _patch_upgrade_available(monkeypatch): + monkeypatch.setattr(self_outdated_check, 'get_installed_version', + lambda name: '1.0') + monkeypatch.setattr(self_outdated_check, 'PackageFinder', + MockPackageFinder) + monkeypatch.setattr(self_outdated_check, 'get_distribution', + lambda name: MockDistribution('pip')) + monkeypatch.setattr(logger, 'warning', + pretend.call_recorder(lambda *a, **kw: None)) + fake_state = pretend.stub( + state={}, + save=pretend.call_recorder(lambda v, t: None), + ) + monkeypatch.setattr( + self_outdated_check, 'SelfCheckState', lambda **kw: fake_state + ) + + +def test_self_version_check_fetch_does_not_log(monkeypatch): + # CVE-2026-6357: the fetch half runs before the command body, the emit + # half only reports afterwards. + _patch_upgrade_available(monkeypatch) + + prompt = _pip_self_version_check_fetch(None, _options()) + + assert prompt == ('1.0', '6.9.0') + assert logger.warning.calls == [] + + +def test_self_version_check_emit_does_no_lookups(monkeypatch): + # The emit half runs after `pip install`; it must not query the index, + # compute tags or inspect installed distributions (CVE-2026-6357). + def fail(*args, **kwargs): + raise AssertionError("lookup after the command body") + + monkeypatch.setattr(self_outdated_check, 'get_installed_version', fail) + monkeypatch.setattr(self_outdated_check, 'get_distribution', fail) + monkeypatch.setattr(self_outdated_check, 'PackageFinder', + pretend.stub(create=fail)) + monkeypatch.setattr(self_outdated_check, 'SelfCheckState', fail) + monkeypatch.setattr(logger, 'warning', + pretend.call_recorder(lambda *a, **kw: None)) + + _pip_self_version_check_emit(('1.0', '6.9.0')) + + assert len(logger.warning.calls) == 1 + assert logger.warning.calls[0].args[1:3] == ('1.0', '6.9.0') + + +def test_self_version_check_emit_none_is_silent(monkeypatch): + monkeypatch.setattr(logger, 'warning', + pretend.call_recorder(lambda *a, **kw: None)) + _pip_self_version_check_emit(None) + assert logger.warning.calls == [] + + +def test_self_version_check_fetch_swallows_errors(monkeypatch): + _patch_upgrade_available(monkeypatch) + + def broken(**kwargs): + raise RuntimeError("boom") + + monkeypatch.setattr(self_outdated_check, 'SelfCheckState', broken) + monkeypatch.setattr(logger, 'debug', + pretend.call_recorder(lambda *a, **kw: None)) + + assert _pip_self_version_check_fetch(None, _options()) is None + assert len(logger.debug.calls) == 1 + + statefile_name_case_1 = ( "fcd2d5175dd33d5df759ee7b045264230205ef837bf9f582f7c3ada7" ) From 8448225eed0b2be60837a3976c899b6b708126ef Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Wed, 21 Oct 2026 18:23:22 -0500 Subject: [PATCH 02/10] Release 20.3.4.post3: backport CVE-2026-6357 Bump version 20.3.4.post2 -> 20.3.4.post3 and add a NEWS section for CVE-2026-6357 (GHSA-jp4c-xjxw-mgf9). The section states that the 20.3.4+security.1 not-applicable assessment was wrong and why. The old note gets a one-line pointer to the correction. Co-Authored-By: Claude Opus 5.5 --- NEWS.rst | 25 +++++++++++++++++++++++++ src/pip/__init__.py | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/NEWS.rst b/NEWS.rst index 1dcfbd480a..da3ff72cd1 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -9,6 +9,29 @@ .. towncrier release notes start +20.3.4.post3 (2026-10-21) +========================= + +Security Fixes +-------------- + +- **CVE-2026-6357**: Run the pip self-version check before the command body + and only report its result afterwards, so ``pip install`` can no longer be + made to import and execute a module from a just-installed wheel. The check + computes supported tags, and the vendored ``packaging`` probes + ``import _manylinux`` (the PEP 513/600 override hook) every time it does. + (`GHSA-jp4c-xjxw-mgf9 `_) + +.. note + + This corrects the 20.3.4+security.1 notes, which assessed CVE-2026-6357 + as **not applicable** to 20.3.4. That was wrong: a failed import is never + cached in ``sys.modules``, so the ``_manylinux`` probe is retried by the + post-install check and picks up a top-level ``_manylinux.py`` that a wheel + has just installed. 20.3.4+security.1 and 20.3.4.post2 are affected; + ``--disable-pip-version-check`` avoids it on those releases. + + 20.3.4.post2 (2026-10-21) ========================= @@ -75,6 +98,8 @@ Security Fixes vendored (``pip._vendor.*``) modules, so a newly-installed wheel has no well-known module name to shadow. + (Corrected in 20.3.4.post3: this release *is* affected, see above.) + 20.3.4 (2021-01-23) =================== diff --git a/src/pip/__init__.py b/src/pip/__init__.py index 2a094febd8..15135db80f 100644 --- a/src/pip/__init__.py +++ b/src/pip/__init__.py @@ -4,7 +4,7 @@ from typing import List, Optional -__version__ = "20.3.4.post2" +__version__ = "20.3.4.post3" def main(args=None): From 4a2cae260790afadd4fa22dd34a32900d92de734 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:04:31 -0500 Subject: [PATCH 03/10] Review: harden CVE-2025-8869 fix against symlink traversal (pypa/pip#14127) Review finding on ActiveState/pip#4/#6/#7, which applies equally to this line: the CVE-2025-8869 backport only checked that a symlink's target is an archive member, and "." and ".." are members. A crafted sdist could therefore write outside the extract directory through a symlink: - outside_link -> .., then outside_link/outside.txt (the error only came at a later ".." member, after the write); - ., redir -> ., redir/up -> .., redir/up/outside.txt (no error at all). Both escapes reproduce on every released 20.3.4.x. Python 2.7 has no tarfile.data_filter, so untar_file's member loop is the only guard. Port upstream pypa/pip#14127 (17eb176dc, released in 26.2): - is_within_directory() gains resolve_symlinks=False (positional default, so existing two-argument callers are unaffected); when true it compares realpaths. - Each member is rejected unless both the abspath and the realpath of its destination stay inside the extract directory. - A symlink is rejected when its target, resolved, leaves the extract directory, before is_symlink_target_in_tar() is consulted. The new symlink check reuses this line's existing "has a link ... trying to point outside target directory" message. Upstream's two tests are ported (no data_filter monkeypatch is needed on Python 2.7). Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/utils/unpacking.py | 38 +++++++++++++--- tests/unit/test_utils_unpacking.py | 65 ++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 2a926e648c..78df82d135 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -84,13 +84,21 @@ def has_leading_dir(paths): return True -def is_within_directory(directory, target): - # type: ((Union[str, Text]), (Union[str, Text])) -> bool +def is_within_directory(directory, target, resolve_symlinks=False): + # type: ((Union[str, Text]), (Union[str, Text]), bool) -> bool """ Return true if the absolute path of target is within the directory + (including when target is equal to the directory). + + When ``resolve_symlinks`` is true, resolve symlinks before comparing so + traversal through a symlink (e.g. "link/../file") is also caught. """ - abs_directory = os.path.abspath(directory) - abs_target = os.path.abspath(target) + if resolve_symlinks: + abs_directory = os.path.realpath(directory) + abs_target = os.path.realpath(target) + else: + abs_directory = os.path.abspath(directory) + abs_target = os.path.abspath(target) # NOTE: os.path.commonprefix compares character-by-character, so it treats # "/a/parent" and "/a/parentfoo" as sharing the prefix "/a/parent" @@ -220,7 +228,12 @@ def untar_file(filename, location): # https://github.com/python/mypy/issues/1174 fn = split_leading_dir(fn)[1] # type: ignore path = os.path.join(location, fn) - if not is_within_directory(location, path): + # The plain check rejects textual ".." escapes; resolving symlinks + # also catches a later member redirected outside by an earlier + # member's symlink (e.g. "link/../file"). + if not is_within_directory(location, path) or ( + not is_within_directory(location, path, resolve_symlinks=True) + ): message = ( 'The tar file ({}) has a file ({}) trying to install ' 'outside target directory ({})' @@ -231,6 +244,21 @@ def untar_file(filename, location): if member.isdir(): ensure_dir(path) elif member.issym(): + # Reject symlinks resolving outside the destination, so a + # later member cannot be written through them. + link_target = os.path.join( + os.path.dirname(path), member.linkname + ) + if not is_within_directory( + location, link_target, resolve_symlinks=True + ): + message = ( + 'The tar file ({}) has a link ({}) trying to point ' + 'outside target directory ({})' + ) + raise InstallationError( + message.format(filename, member.name, member.linkname) + ) if not is_symlink_target_in_tar(tar, member): message = ( 'The tar file ({}) has a link ({}) trying to point ' diff --git a/tests/unit/test_utils_unpacking.py b/tests/unit/test_utils_unpacking.py index 5162cecf94..0937c44fde 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -209,6 +209,71 @@ def test_unpack_tar_symlink_to_member_success(self): assert os.path.islink(link_path) assert os.readlink(link_path) == 'target.txt' + def test_unpack_tar_symlink_then_member(self): + """Reject a symlink to outside before a member is written through it. + + CVE-2025-8869 follow-up (pypa/pip#14127). Python 2.7 has no + ``tarfile.data_filter``, so this check is the only protection. + """ + tar_filepath = os.path.join(self.tempdir, 'symlink_then_member.tar') + extract_path = os.path.join(self.tempdir, 'extract_path') + outside_path = os.path.join(self.tempdir, 'outside.txt') + + with tarfile.open(tar_filepath, 'w') as tar: + info = tarfile.TarInfo('outside_link') + info.type = tarfile.SYMTYPE + info.linkname = '..' + tar.addfile(info) + + data = b'data\n' + info = tarfile.TarInfo('outside_link/outside.txt') + info.size = len(data) + tar.addfile(info, fileobj=io.BytesIO(data)) + + info = tarfile.TarInfo('..') + info.type = tarfile.DIRTYPE + tar.addfile(info) + + with pytest.raises(InstallationError): + untar_file(tar_filepath, extract_path) + + assert not os.path.exists(outside_path) + assert not os.path.exists(os.path.join(extract_path, 'outside_link')) + + def test_unpack_tar_nested_symlink_traversal(self): + """Reject a member that escapes through a chain of in-bounds symlinks. + + CVE-2025-8869 follow-up (pypa/pip#14127). + """ + tar_filepath = os.path.join(self.tempdir, 'nested_traversal.tar') + extract_path = os.path.join(self.tempdir, 'extract_path') + outside_path = os.path.join(self.tempdir, 'outside.txt') + + with tarfile.open(tar_filepath, 'w') as tar: + info = tarfile.TarInfo('.') + info.type = tarfile.DIRTYPE + tar.addfile(info) + + info = tarfile.TarInfo('redir') + info.type = tarfile.SYMTYPE + info.linkname = '.' + tar.addfile(info) + + info = tarfile.TarInfo('redir/up') + info.type = tarfile.SYMTYPE + info.linkname = '..' + tar.addfile(info) + + data = b'data\n' + info = tarfile.TarInfo('redir/up/outside.txt') + info.size = len(data) + tar.addfile(info, fileobj=io.BytesIO(data)) + + with pytest.raises(InstallationError): + untar_file(tar_filepath, extract_path) + + assert not os.path.exists(outside_path) + @pytest.mark.parametrize('args, expected', [ # Test the second containing the first. From ab0d758bacfed884d1fcb33461f7bb3f4355b17c Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:05:09 -0500 Subject: [PATCH 04/10] Review: root-safe path containment for is_within_directory and _script_within_dir Review finding (ActiveState/pip#3 nit carried over from #2, and #4-#7): both checks appended os.sep to the directory before a prefix test, so a directory that is a filesystem root ("/" or "C:\\") became "//" and rejected every target. With a scripts directory of "/", every entry point failed to install. - is_within_directory(): uses the directory itself as the prefix when it already ends with a separator. It still never uses os.path.commonpath (absent on Python 2.7, and it drops a POSIX "//" prefix), so the CVE-2026-1703 component boundary and "//" roots keep working. - _script_within_dir(): now delegates to is_within_directory() and additionally refuses a script that resolves to the directory itself. Tests cover "/" and "//" roots, the realpath mode added for CVE-2025-8869, and the Windows drive-root case (skipped off Windows). Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/operations/install/wheel.py | 4 +++- src/pip/_internal/utils/unpacking.py | 15 ++++++++---- tests/unit/test_utils_unpacking.py | 23 +++++++++++++++++++ tests/unit/test_wheel.py | 15 ++++++++++++ 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py index 59ca06f8d5..0d97ef51fe 100644 --- a/src/pip/_internal/operations/install/wheel.py +++ b/src/pip/_internal/operations/install/wheel.py @@ -478,7 +478,9 @@ def _script_within_dir(name, scripts_dir): """ root = os.path.normpath(scripts_dir) dest = os.path.normpath(os.path.join(scripts_dir, name)) - return dest.startswith(root + os.sep) + # The script must be a file *inside* the directory, never the directory + # itself. + return dest != root and is_within_directory(root, dest) def _raise_for_invalid_entrypoint(specification, scripts_dir): diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 78df82d135..38aa32d732 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -104,10 +104,17 @@ def is_within_directory(directory, target, resolve_symlinks=False): # "/a/parent" and "/a/parentfoo" as sharing the prefix "/a/parent" # and would report the latter as "within" the former (CVE-2026-1703). # os.path.commonpath compares on path components but is unavailable on - # Python 2.7, so enforce a component boundary explicitly instead. - return abs_target == abs_directory or abs_target.startswith( - abs_directory + os.sep - ) + # Python 2.7 (and drops a POSIX "//" prefix), so enforce a component + # boundary explicitly instead. + if abs_target == abs_directory: + return True + # A filesystem root ("/" or "C:\\") already ends with a separator; + # appending another would reject everything. + if abs_directory.endswith(os.sep): + prefix = abs_directory + else: + prefix = abs_directory + os.sep + return abs_target.startswith(prefix) def is_symlink_target_in_tar(tar, tarinfo): diff --git a/tests/unit/test_utils_unpacking.py b/tests/unit/test_utils_unpacking.py index 0937c44fde..141b58e511 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -289,12 +289,35 @@ def test_unpack_tar_nested_symlink_traversal(self): # CVE-2026-1703: a sibling whose name has the directory as a string # prefix must not be considered "within" it. (('parent/child', 'parent/childfoo'), False), + # A filesystem root contains everything below it. + (('/', '/usr/bin/pip'), True), + (('/', '/'), True), ]) def test_is_within_directory(args, expected): result = is_within_directory(*args) assert result == expected +@pytest.mark.skipif("sys.platform == 'win32'") +def test_is_within_directory_doubled_slash_root(): + # POSIX keeps a leading "//"; the containment check must too. + assert is_within_directory('//srv/env/bin', '//srv/env/bin/pip') + assert not is_within_directory('//srv/env/bin', '//srv/env/binfoo') + + +def test_is_within_directory_resolve_symlinks(tmpdir): + base = str(tmpdir) + inside = os.path.join(base, 'inside') + os.mkdir(inside) + if not hasattr(os, 'symlink'): + pytest.skip('os.symlink is unavailable') + os.symlink(base, os.path.join(inside, 'up')) + target = os.path.join(inside, 'up', 'outside.txt') + # Textually inside, but the symlink resolves to the parent. + assert is_within_directory(inside, target) + assert not is_within_directory(inside, target, resolve_symlinks=True) + + def _make_tar_zip_polyglot(path): """Write a valid .tar.gz with a valid ZIP appended (a polyglot file). diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index df18c77b3f..1f6538ad3c 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -769,3 +769,18 @@ def test_script_within_dir_allows_doubled_slash_root(): def test_script_within_dir_rejects_other_drive(): # A script on a different drive is rejected, and doesn't raise assert wheel._script_within_dir("D:\\outside", "C:\\env\\bin") is False + + +@pytest.mark.skipif(WINDOWS, reason="POSIX filesystem root") +def test_script_within_dir_allows_filesystem_root(): + # A scripts directory at the filesystem root must still accept scripts, + # but never resolve a script to the root directory itself. + assert wheel._script_within_dir("pip", "/") is True + assert wheel._script_within_dir(".", "/") is False + assert wheel._script_within_dir("..", "/") is False + + +@pytest.mark.skipif(not WINDOWS, reason="drive letters only matter on Windows") +def test_script_within_dir_allows_drive_root(): + assert wheel._script_within_dir("pip", "C:\\") is True + assert wheel._script_within_dir(".", "C:\\") is False From 50b1582e0cb6d5f70055fdb838e84da20c27217f Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:05:48 -0500 Subject: [PATCH 05/10] Review: accept normalized member names in is_symlink_target_in_tar Review finding (ActiveState/pip#4, same code on this line): the CVE-2025-8869 symlink check required the link target to match a member name exactly, so two valid archive shapes were refused that the pristine 20.3.4 unpacked: - "./"-prefixed archives (as produced by `tar -C dir .`), e.g. a ./pkg/link -> target.txt member; - a symlink to a directory that has no directory entry of its own. On Python 2.7 untar_file's member loop is the only extraction path, so this affected every such sdist. Upstream 26.2.2 behaves the same; this is a deliberate improvement. Containment is enforced by the realpath checks added in the previous commit, not by this lookup. - New _tar_member_paths(): posix-normalized member names plus every implied parent directory. - is_symlink_target_in_tar() compares the normalized link target against that set. It takes an optional member_paths so untar_file() computes the set once instead of once per symlink. Tests cover both shapes and a link to a non-member (still refused). Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/utils/unpacking.py | 42 ++++++++++++++++------- tests/unit/test_utils_unpacking.py | 51 ++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 38aa32d732..2ab7093cf2 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -5,6 +5,7 @@ import logging import os +import posixpath import shutil import stat import tarfile @@ -21,7 +22,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Iterable, List, Optional, Text, Union + from typing import Iterable, List, Optional, Set, Text, Union from zipfile import ZipInfo @@ -117,23 +118,35 @@ def is_within_directory(directory, target, resolve_symlinks=False): return abs_target.startswith(prefix) -def is_symlink_target_in_tar(tar, tarinfo): - # type: (tarfile.TarFile, tarfile.TarInfo) -> bool +def _tar_member_paths(tar): + # type: (tarfile.TarFile) -> Set[str] + """Normalized member paths plus every implied parent directory.""" + paths = set() # type: Set[str] + for member in tar.getmembers(): + name = posixpath.normpath(member.name.replace("\\", "/")) + while name not in ("", ".", "/"): + paths.add(name) + name = posixpath.dirname(name) + return paths + + +def is_symlink_target_in_tar(tar, tarinfo, member_paths=None): + # type: (tarfile.TarFile, tarfile.TarInfo, Optional[Set[str]]) -> bool """ Return true if the file the symlink member points to is itself a member of the same tar archive, i.e. the symlink does not escape the archive to reference an arbitrary path on the host filesystem (CVE-2025-8869). + + Member names are compared after normalization, and directories implied + by a member's path count as members, so "./"-prefixed archives and links + to a directory without its own entry are accepted. Containment itself is + enforced by the realpath checks in untar_file(). """ 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 + linkname = posixpath.normpath(linkname.replace("\\", "/")) + if member_paths is None: + member_paths = _tar_member_paths(tar) + return linkname in member_paths def set_extracted_file_to_default_mode_plus_executable(path): @@ -229,6 +242,9 @@ def untar_file(filename, location): leading = has_leading_dir([ member.name for member in tar.getmembers() ]) + # Computed once: is_symlink_target_in_tar() would otherwise rebuild + # it for every symlink member. + member_paths = _tar_member_paths(tar) for member in tar.getmembers(): fn = member.name if leading: @@ -266,7 +282,7 @@ def untar_file(filename, location): raise InstallationError( message.format(filename, member.name, member.linkname) ) - if not is_symlink_target_in_tar(tar, member): + if not is_symlink_target_in_tar(tar, member, member_paths): message = ( 'The tar file ({}) has a link ({}) trying to point ' 'outside target directory ({})' diff --git a/tests/unit/test_utils_unpacking.py b/tests/unit/test_utils_unpacking.py index 141b58e511..0c86ceddb4 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -209,6 +209,57 @@ def test_unpack_tar_symlink_to_member_success(self): assert os.path.islink(link_path) assert os.readlink(link_path) == 'target.txt' + def _make_tar(self, name, entries): + path = os.path.join(self.tempdir, name) + with tarfile.open(path, 'w') as tar: + for member_name, kind, arg in entries: + info = tarfile.TarInfo(member_name) + if kind == 'dir': + info.type = tarfile.DIRTYPE + tar.addfile(info) + elif kind == 'sym': + info.type = tarfile.SYMTYPE + info.linkname = arg + tar.addfile(info) + else: + info.size = len(arg) + tar.addfile(info, io.BytesIO(arg)) + return path + + def test_unpack_tar_dot_prefixed_symlink_success(self): + """A "./"-prefixed archive (``tar -C dir .``) with an in-archive + symlink is accepted: member names are compared normalized.""" + test_tar = self._make_tar('dot_prefixed.tar', [ + ('./', 'dir', None), + ('./pkg', 'dir', None), + ('./pkg/target.txt', 'file', b'data\n'), + ('./pkg/link', 'sym', 'target.txt'), + ]) + out = os.path.join(self.tempdir, 'out') + untar_file(test_tar, out) + assert os.readlink(os.path.join(out, 'pkg', 'link')) == 'target.txt' + + def test_unpack_tar_symlink_to_implied_dir_success(self): + """A symlink to a directory that has no entry of its own (only files + beneath it) is accepted: implied parent directories count.""" + test_tar = self._make_tar('implied_dir.tar', [ + ('pkg/sub/f.txt', 'file', b'data\n'), + ('pkg/link', 'sym', 'sub'), + ]) + out = os.path.join(self.tempdir, 'out') + untar_file(test_tar, out) + assert os.readlink(os.path.join(out, 'link')) == 'sub' + + def test_unpack_tar_symlink_to_non_member_failure(self): + """A symlink to a name that is not in the archive is still refused.""" + test_tar = self._make_tar('dangling.tar', [ + ('pkg/a.txt', 'file', b'data\n'), + ('pkg/link', 'sym', 'missing.txt'), + ]) + with pytest.raises(InstallationError) as e: + untar_file(test_tar, os.path.join(self.tempdir, 'out')) + assert 'trying to point outside target directory' in str(e.value) + def test_unpack_tar_symlink_then_member(self): """Reject a symlink to outside before a member is written through it. From 0889b6c7941f43af775c332538f4ec64894407b0 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:07:15 -0500 Subject: [PATCH 06/10] Review: pass the Mercurial revision as --rev= (pypa/pip#12373) Review finding (ActiveState/pip#4, same code on this line): the CVE-2023-5752 backport in 20.3.4+security.1 passes the revision as "-r=". hg parses that as the revision "=" and fails, so every `pip install hg+...@` has been broken since 20.3.4+security.1 (also in 20.3.4.post2): hg update -q -r=0 hg: parse error at 0: not a prefix: = Upstream hit the same regression with its first CVE-2023-5752 fix (pypa/pip#12306) and replaced it with the long form in pypa/pip#12373 (6dbd9c68f0, pip 23.3.1). Port that: get_base_rev_args() returns ["--rev="]. The revision is still a single token, so a malicious revision such as "--config=..." is still read by hg as a revision value (verified: `hg update --rev=--config=x` is a revision parse error, not an option), and the CVE protection holds. Verified end to end in python:2.7-slim with Mercurial 4.8.2: `pip install "hg+file:///repo@0#egg=hgdemo"` fails on the previous head with the parse error above and installs revision 0 with this change. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/vcs/mercurial.py | 10 ++++++---- tests/unit/test_vcs.py | 21 ++++++++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/pip/_internal/vcs/mercurial.py b/src/pip/_internal/vcs/mercurial.py index 1fc7f0d204..8b8e67e885 100644 --- a/src/pip/_internal/vcs/mercurial.py +++ b/src/pip/_internal/vcs/mercurial.py @@ -38,10 +38,12 @@ class Mercurial(VersionControl): @staticmethod def get_base_rev_args(rev): - # CVE-2023-5752: pass the revision as a single ``-r=`` token so a - # malicious revision (e.g. ``--config=...``) cannot be interpreted by - # hg as a separate command-line option. - return ["-r={}".format(rev)] + # CVE-2023-5752: pass the revision as a single ``--rev=`` token + # so a malicious revision (e.g. ``--config=...``) cannot be + # interpreted by hg as a separate command-line option. The short form + # ``-r=`` must not be used: hg reads it as the revision ``=`` + # and fails ("parse error ... not a prefix: =") (pypa/pip#12373). + return ["--rev={}".format(rev)] def export(self, location, url): # type: (str, HiddenText) -> None diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py index 1008f19b6a..26e4f4f90d 100644 --- a/tests/unit/test_vcs.py +++ b/tests/unit/test_vcs.py @@ -52,7 +52,7 @@ def test_rev_options_repr(): # First check VCS-specific RevOptions behavior. (Bazaar, [], ['-r', '123'], {}), (Git, ['HEAD'], ['123'], {}), - (Mercurial, [], ['-r=123'], {}), + (Mercurial, [], ['--rev=123'], {}), (Subversion, [], ['-r', '123'], {}), # Test extra_args. For this, test using a single VersionControl class. (Git, ['HEAD', 'opt1', 'opt2'], ['123', 'opt1', 'opt2'], @@ -228,14 +228,25 @@ def test_git_get_revision_sha_unicode_separator(run_command_mock): def test_mercurial_rev_not_interpreted_as_option(): """ CVE-2023-5752: a malicious hg revision (e.g. ``--config=...``) must be - passed as a single ``-r=`` token so hg cannot parse it as an option. + passed as a single ``--rev=`` token so hg cannot parse it as an + option. """ rev = '--config=alias.update=!touch owned' args = Mercurial.get_base_rev_args(rev) - assert args == ['-r={}'.format(rev)] - # A single token means hg treats the whole thing as the value of -r. + assert args == ['--rev={}'.format(rev)] + # A single token means hg treats the whole thing as the value of --rev. assert len(args) == 1 - assert args[0].startswith('-r=') + assert args[0].startswith('--rev=') + + +def test_mercurial_rev_uses_long_option(): + """ + hg parses ``-r=`` as the revision ``=`` and fails, so the + revision must be passed with the long ``--rev=`` form (pypa/pip#12373). + """ + args = Mercurial.get_base_rev_args('0') + assert args == ['--rev=0'] + assert not args[0].startswith('-r') # The non-SVN backends all use the same get_netloc_and_auth(), so only test From e6de4a2e23beb9211f6c0043dd3a941488cf3528 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:09:10 -0500 Subject: [PATCH 07/10] Review: run the version-check fetch inside pip's error handling Review finding (ActiveState/pip#3, should-fix): the CVE-2026-6357 port called _pip_version_check_fetch() just before _main()'s try:, so it bypassed pip's error handling: - `pip install "foo @ http://[bad/x"` escaped as a raw interpreter traceback with exit code 1, because _arg_refers_to_pip() only caught InvalidRequirement and urlparse raises ValueError("Invalid IPv6 URL"); post2 reported it through pip's handler ("ERROR: Exception:", exit 2); - a Ctrl-C during the (up to 5s) fetch escaped as a raw KeyboardInterrupt instead of "Operation cancelled by user". - The fetch is now the first statement inside the try (version_check is initialised to None before it), matching upstream, where the check is inside _inner_run(). The emit step stays in the finally. - _arg_refers_to_pip() also treats ValueError as "not pip", so a malformed requirement falls through to pip's normal parsing path. `pip install "foo @ http://[bad/x"` is back to post2's behaviour ("ERROR: Exception:", exit 2). New tests cover KeyboardInterrupt and ValueError raised by the fetch (on the previous head the KeyboardInterrupt escapes and interrupts the test run itself) and the malformed requirement. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/cli/base_command.py | 6 +++-- src/pip/_internal/commands/install.py | 4 +++- tests/unit/test_base_command.py | 32 +++++++++++++++++++++++++++ tests/unit/test_commands.py | 2 ++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index f30ae43d00..1e322b4c18 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -240,9 +240,11 @@ def _main(self, args): # The version check does its lookups before the command body runs and # only reports afterwards, so nothing is imported once packages have - # been installed (CVE-2026-6357). - version_check = self._pip_version_check_fetch(options, args) + # been installed (CVE-2026-6357). The fetch runs inside the try so + # its errors (and Ctrl-C) get pip's normal error handling. + version_check = None try: + version_check = self._pip_version_check_fetch(options, args) status = self.run(options, args) assert isinstance(status, int) return status diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 5ae20e106f..4f112c511a 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -66,7 +66,9 @@ def _arg_refers_to_pip(arg): # type: (str) -> bool try: req = Requirement(arg) - except InvalidRequirement: + except (InvalidRequirement, ValueError): + # A malformed requirement (e.g. a URL that urlparse rejects with + # ValueError) is not pip; leave the error to the normal parsing path. return False return canonicalize_name(req.name) == "pip" diff --git a/tests/unit/test_base_command.py b/tests/unit/test_base_command.py index 65a6ef614c..4ae13d5cf6 100644 --- a/tests/unit/test_base_command.py +++ b/tests/unit/test_base_command.py @@ -122,6 +122,38 @@ def run_func(): assert events == ['fetch', 'run', ('emit', 'fetched')] +@pytest.mark.parametrize('exc, message', [ + (KeyboardInterrupt, 'Operation cancelled by user'), + (ValueError, 'Exception:'), +]) +def test_version_check_fetch_errors_are_handled( + monkeypatch, capsys, exc, message +): + """ + The pre-command version-check fetch runs inside _main()'s error + handling, so Ctrl-C or an unexpected error during it is reported by pip + instead of escaping as a raw traceback. + """ + emitted = [] + + def fetch(self, options, args): + raise exc() + + def emit(self, options, fetched): + emitted.append(fetched) + + monkeypatch.setattr(Command, '_pip_version_check_fetch', fetch) + monkeypatch.setattr(Command, '_pip_version_check_emit', emit) + + cmd = FakeCommand(run_func=lambda: pytest.fail('run() must not be called')) + status = cmd.main([]) + assert status != SUCCESS + stderr = capsys.readouterr().err + assert message in stderr + # The emit step still runs (with nothing fetched). + assert emitted == [None] + + def test_log_command_success(fixed_time, tmpdir): """Test the --log option logs when command succeeds.""" cmd = FakeCommand() diff --git a/tests/unit/test_commands.py b/tests/unit/test_commands.py index c3f4d44fd2..61bde71fd7 100644 --- a/tests/unit/test_commands.py +++ b/tests/unit/test_commands.py @@ -198,6 +198,8 @@ def test_index_group_version_check_disabled( ('six', False), ('./pip-20.3.4-py2.py3-none-any.whl', False), ('-r', False), + # urlparse raises ValueError for this URL; it is simply not pip. + ('foo @ http://[bad/x', False), ]) def test_arg_refers_to_pip(arg, expected): assert _arg_refers_to_pip(arg) is expected From 8de4ab18c699f156a2bd1592e276cc853d255fcb Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:10:35 -0500 Subject: [PATCH 08/10] Review: report unsafe download file names as a pip error Review nit (ActiveState/pip#3, carried over from #2, and #4-#7): a download whose derived file name is not a single safe component (e.g. `Content-Disposition: filename=".."`) was refused, but as a bare ValueError, so pip printed "ERROR: Exception:" plus a traceback and exited 2. Add pip._internal.exceptions.UnsafeFilenameError(InstallationError, ValueError) and raise it from as_path_component() and join_within_directory() with the same messages. It remains a ValueError, so existing callers and tests are unaffected. As an InstallationError, pip's command loop reports it as a single line: ERROR: Unexpected file name derived from URL: '..' (exit 1). Verified end to end against a local server sending filename="..". The write was already blocked before; only the reporting changes. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/exceptions.py | 10 ++++++++++ src/pip/_internal/models/link.py | 5 +++-- tests/unit/test_link.py | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 8d7f7fa39b..47f6769e7a 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -36,6 +36,16 @@ class InstallationError(PipError): """General exception during installation""" +class UnsafeFilenameError(InstallationError, ValueError): + """A file name derived from a URL or header is not a single safe path + component. + + It is also a ValueError, so callers written against the plain + ValueError keep working, while pip's command loop reports it as an + ordinary ``ERROR:`` line instead of a traceback. + """ + + class UninstallationError(PipError): """General exception during uninstallation""" diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py index 129c4b2120..e646ece135 100644 --- a/src/pip/_internal/models/link.py +++ b/src/pip/_internal/models/link.py @@ -4,6 +4,7 @@ from pip._vendor.six.moves.urllib import parse as urllib_parse +from pip._internal.exceptions import UnsafeFilenameError from pip._internal.utils.filetypes import WHEEL_EXTENSION from pip._internal.utils.misc import ( redact_auth_from_url, @@ -44,7 +45,7 @@ def as_path_component(name): """ component = _to_path_component(name) if not component: - raise ValueError( + raise UnsafeFilenameError( "Unexpected file name derived from URL: {!r}".format(name) ) @@ -61,7 +62,7 @@ def join_within_directory(directory, component): has no runtime type to enforce that, so it is checked here instead. """ if not component or component != _to_path_component(component): - raise ValueError( + raise UnsafeFilenameError( "Not a single path component: {!r}".format(component) ) diff --git a/tests/unit/test_link.py b/tests/unit/test_link.py index 3788f5a8c2..c363180bbb 100644 --- a/tests/unit/test_link.py +++ b/tests/unit/test_link.py @@ -3,6 +3,7 @@ import pytest +from pip._internal.exceptions import InstallationError, UnsafeFilenameError from pip._internal.models.link import ( Link, as_path_component, @@ -240,3 +241,16 @@ def test_join_within_directory_rejects_non_component(component): # its argument itself instead of trusting the caller. with pytest.raises(ValueError): join_within_directory('downloads', component) + + +@pytest.mark.parametrize('call', [ + lambda: as_path_component('..'), + lambda: join_within_directory('downloads', '..'), +]) +def test_unsafe_filename_is_a_pip_installation_error(call): + # Reported by pip's command loop as an "ERROR:" line, not a traceback, + # while staying a ValueError for existing callers. + with pytest.raises(UnsafeFilenameError) as e: + call() + assert isinstance(e.value, InstallationError) + assert isinstance(e.value, ValueError) From f93643b55d26e18256eb39ab4dd0a356a2352890 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:11:17 -0500 Subject: [PATCH 09/10] =?UTF-8?q?Review:=20NEWS=20=E2=80=94=20render=20not?= =?UTF-8?q?es,=20record=20review=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review nit (ActiveState/pip#3, #7): ".. note" without "::" is a reStructuredText comment, so the notes did not render, including the post3 correction that says 20.3.4+security.1 and .post2 are affected by CVE-2026-6357. Make all three AS notes (post3, post2, +security.1) real ".. note::" directives; the upstream header comment on line 1 is left as is. Markup only, no wording changes to the older sections. Record the review fixes in the 20.3.4.post3 section: - Security: CVE-2025-8869 hardening (pypa/pip#14127) and the CVE-2023-5752 Mercurial --rev= regression fix (affects +security.1 and .post2). - Bug fixes: filesystem-root containment, version-check fetch inside error handling, normalized symlink member names, and unsafe download file names reported as a pip error. Checked with docutils: 3 notes render, no warnings. Co-Authored-By: Claude Opus 5.5 --- NEWS.rst | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index da3ff72cd1..8a11614d0c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,8 +21,41 @@ Security Fixes computes supported tags, and the vendored ``packaging`` probes ``import _manylinux`` (the PEP 513/600 override hook) every time it does. (`GHSA-jp4c-xjxw-mgf9 `_) +- **CVE-2025-8869** (hardening): Port upstream's follow-up fix (pypa/pip#14127) + to the tar extraction path, which is the only one on Python 2.7 (there is no + ``tarfile.data_filter``). A member is now rejected unless its resolved + (``realpath``) destination stays inside the extract directory, and a symlink + is rejected when its resolved target leaves it. Previously a symlink to + ``.`` or ``..`` (both archive members) let a later member be written outside + the extract directory. 20.3.4+security.1 and 20.3.4.post2 are affected. + (`GHSA-4xh5-x5gv-qwph `_) +- **CVE-2023-5752** (regression fix): Pass a Mercurial revision as + ``--rev=`` instead of ``-r=`` (pypa/pip#12373). Mercurial parses + ``-r=`` as the revision ``=`` and fails, so every + ``hg+...@`` install has been broken since 20.3.4+security.1 (also in + 20.3.4.post2). The revision is still a single token, so the CVE protection + is unchanged. + (`GHSA-mq26-g339-26xf `_) -.. note +Bug Fixes +--------- + +- ``is_within_directory`` and the CVE-2026-8643 entry point check accept a + directory that is a filesystem root (``/`` or ``C:\``); previously every + target, and every entry point, was rejected there. +- The pre-command version-check lookup now runs inside pip's error handling, + so Ctrl-C during it reports "Operation cancelled by user", and a malformed + requirement such as ``foo @ http://[bad/x`` gets pip's normal error report + instead of a raw traceback. +- Symlinks in an sdist are matched against normalized member names, including + implied parent directories, so ``./``-prefixed archives (``tar -C dir .``) + and links to a directory without its own entry unpack again. Containment is + still enforced by the ``realpath`` checks above. +- A download whose file name is not a single safe path component (e.g. + ``Content-Disposition: filename=".."``) is reported as a pip ``ERROR:`` + line instead of a traceback. It was already refused. + +.. note:: This corrects the 20.3.4+security.1 notes, which assessed CVE-2026-6357 as **not applicable** to 20.3.4. That was wrong: a failed import is never @@ -56,7 +89,7 @@ Security Fixes (`GHSA-43fp-rhv2-5gv8 `_, `GHSA-xqr8-7jwr-rhp7 `_) -.. note +.. note:: CVE-2024-39689 / GHSA-248v-346w-9cwc (certifi GLOBALTRUST roots) was assessed and found **not applicable**: those roots were added in certifi @@ -90,7 +123,7 @@ Security Fixes (instead of a character-prefix comparison) so an extracted file cannot land in a sibling directory whose name is a prefix of the install path. (`GHSA-6vgw-5pg2-w6jp `_) -.. note +.. note:: CVE-2026-6357 / GHSA-jp4c-xjxw-mgf9 was assessed and found **not applicable** to this release: pip 20.3.4 imports the self-version-check and all of its From b3080f9edb8d47b1b6dcd4a82315d26355faacb9 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Thu, 22 Oct 2026 10:19:41 -0500 Subject: [PATCH 10/10] Review: stop _tar_member_paths at a fixed point posixpath.normpath keeps a leading "//" and posixpath.dirname("//") is "//", so collecting the implied parent directories of a member named "//x/y" never terminated: a crafted archive could hang pip in the tar fallback path. Stop walking parents once dirname() stops changing the name, and add a regression test. The bug was in the shared review-round helper (item G) and was caught on the 26.0.1 line; this applies the same guard here. Co-Authored-By: Claude Opus 5.5 --- src/pip/_internal/utils/unpacking.py | 6 +++++- tests/unit/test_utils_unpacking.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 2ab7093cf2..740390c109 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -126,7 +126,11 @@ def _tar_member_paths(tar): name = posixpath.normpath(member.name.replace("\\", "/")) while name not in ("", ".", "/"): paths.add(name) - name = posixpath.dirname(name) + parent = posixpath.dirname(name) + if parent == name: + # posixpath.dirname("//") == "//": stop at any fixed point. + break + name = parent return paths diff --git a/tests/unit/test_utils_unpacking.py b/tests/unit/test_utils_unpacking.py index 0c86ceddb4..76d2e5bc73 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -12,6 +12,7 @@ from pip._internal.exceptions import InstallationError from pip._internal.utils.unpacking import ( + _tar_member_paths, is_within_directory, unpack_file, untar_file, @@ -422,3 +423,20 @@ def test_unpack_polyglot_ambiguous_rejected(tmpdir): _make_tar_zip_polyglot(archive) 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() + with tarfile.open(fileobj=buf, mode="w") as tar: + data = io.BytesIO(b"x") + info = tarfile.TarInfo("//abs/f.txt") + info.size = 1 + tar.addfile(info, fileobj=data) + buf.seek(0) + tar = tarfile.open(fileobj=buf) + try: + paths = _tar_member_paths(tar) + finally: + tar.close() + assert "//abs/f.txt" in paths + assert "//abs" in paths