diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b8b88586..450a1eb0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - CI: deleted `ci-integration-pr-stub.yml`. The four stubs were a holdover from the pre-merge-gate model where branch protection required each Tier 2 check name directly. After #867, branch protection requires only `gate`, so the stubs are dead weight. Reduced `EXPECTED_CHECKS` in `merge-gate.yml` to just `Build & Test (Linux)`. +### Fixed + +- `apm update` sanitises the subprocess environment before invoking the platform installer so the bundled PyInstaller `LD_LIBRARY_PATH` / `DYLD_*` no longer leak into system binaries (`curl`, `tar`, `sudo`) spawned by `install.sh`. Previously the installer's first `curl` call could abort with `libssl.so.3: version 'OPENSSL_3.2.0' not found` on distros whose system `libcurl` requires a newer OpenSSL ABI than the APM bundle ships (Debian trixie arm64 dev-containers, Fedora 43, and similar). Restoration uses PyInstaller's official `_ORIG` protocol, preserving the user's own `LD_LIBRARY_PATH` exports. Closes #894 + ## [0.9.2] - 2026-04-23 ### Added diff --git a/src/apm_cli/commands/update.py b/src/apm_cli/commands/update.py index 5d7d6ed3b..63b719a74 100644 --- a/src/apm_cli/commands/update.py +++ b/src/apm_cli/commands/update.py @@ -8,6 +8,7 @@ from ..core.command_logger import CommandLogger from ..update_policy import get_self_update_disabled_message, is_self_update_enabled +from ..utils.subprocess_env import external_process_env from ..version import get_version @@ -138,7 +139,16 @@ def update(check): logger.progress("Running installer...", symbol="gear") # Note: We don't capture output so the installer can prompt when needed. - result = subprocess.run(_get_installer_run_command(temp_script), check=False) + # Sanitise the environment so the installer (and the system binaries + # it spawns -- curl, tar, sudo) do not inherit the PyInstaller + # bootloader's LD_LIBRARY_PATH / DYLD_* overrides, which would + # otherwise redirect system linkers at this binary's bundled + # _internal directory. See issue #894. + result = subprocess.run( + _get_installer_run_command(temp_script), + check=False, + env=external_process_env(), + ) # Clean up temp file try: diff --git a/src/apm_cli/utils/subprocess_env.py b/src/apm_cli/utils/subprocess_env.py new file mode 100644 index 000000000..aa50e1881 --- /dev/null +++ b/src/apm_cli/utils/subprocess_env.py @@ -0,0 +1,83 @@ +"""Environment helpers for spawning external processes from the frozen CLI. + +When APM ships as a PyInstaller ``--onedir`` binary, the bootloader prepends +the bundle's ``_internal`` directory to ``LD_LIBRARY_PATH`` (Linux) and the +``DYLD_*`` variables (macOS) so that the main Python process can locate its +own shared libraries. Child processes inherit this environment by default, +which causes system binaries -- ``git``, ``curl``, the install script, ... -- +to resolve their dependencies against the bundled libraries. When a bundled +library predates the system caller's ABI requirements, the child aborts with +a symbol lookup error. This has produced two user-visible regressions: + +* #462: ``apm`` → ``git`` → ``git-remote-https`` on Fedora 43 + (``OPENSSL_3.2.0 not found``). +* #894: ``apm update`` → ``install.sh`` → system ``curl`` on Debian trixie + arm64 dev-containers (``OPENSSL_3.2.0 / OPENSSL_3.3.0 not found``). + +PyInstaller saves each rewritten variable's pre-launch value under +``_ORIG``. The canonical mitigation, documented in PyInstaller's +runtime notes, is to restore those values on the child environment before +spawning -- not to blindly ``pop`` the variables, because a user may have +legitimately exported ``LD_LIBRARY_PATH`` themselves (CUDA, Nix, custom +toolchains). This module centralises that restoration in one audited +helper so every subprocess call site gets identical, correct semantics. + +Typical use:: + + from apm_cli.utils.subprocess_env import external_process_env + + subprocess.run(cmd, env=external_process_env(), check=False) +""" +from __future__ import annotations + +import os +import sys +from typing import Mapping + +# Runtime-library search-path variables that PyInstaller's bootloader +# rewrites at launch. Each has a sibling ``_ORIG`` holding the +# pre-launch value that we must restore before handing env to a child +# process. The tuple is intentionally narrow: we do not touch ``PATH`` +# or other inherited variables, only the ones PyInstaller itself manages. +_PYINSTALLER_MANAGED_LIBRARY_VARS: tuple[str, ...] = ( + "LD_LIBRARY_PATH", # Linux and most Unixes + "DYLD_LIBRARY_PATH", # macOS dynamic library search path + "DYLD_FRAMEWORK_PATH", # macOS framework search path +) + + +def external_process_env(base: Mapping[str, str] | None = None) -> dict[str, str]: + """Return an environment dict safe for spawning external system binaries. + + Args: + base: Optional source mapping. Defaults to ``os.environ``. The + returned dict is always an independent copy -- mutating it + never touches the live process environment. + + Behaviour: + * When **not** running as a PyInstaller-frozen binary the base env + is returned as a fresh ``dict`` with no other modifications. + * When frozen, every library-path variable listed in + :data:`_PYINSTALLER_MANAGED_LIBRARY_VARS` is restored from its + ``_ORIG`` sibling (preserving the user's own exports); if + no ``_ORIG`` sibling exists the variable is removed entirely so + the child does not inherit the bundle's ``_internal`` path. The + ``_ORIG`` keys themselves are stripped so we do not leak + PyInstaller internals to the child. + + This is the single source of truth for child-process environment + sanitisation in the CLI; prefer it over per-call-site dict surgery. + """ + env: dict[str, str] = dict(base if base is not None else os.environ) + + if not getattr(sys, "frozen", False): + return env + + for key in _PYINSTALLER_MANAGED_LIBRARY_VARS: + orig_key = f"{key}_ORIG" + if orig_key in env: + env[key] = env[orig_key] + env.pop(orig_key) + else: + env.pop(key, None) + return env diff --git a/tests/unit/test_subprocess_env.py b/tests/unit/test_subprocess_env.py new file mode 100644 index 000000000..ca784d485 --- /dev/null +++ b/tests/unit/test_subprocess_env.py @@ -0,0 +1,163 @@ +"""Tests for :mod:`apm_cli.utils.subprocess_env`. + +These tests lock in the contract documented in the module's docstring: +the helper must be a no-op outside a frozen build, and must restore +every PyInstaller-managed library-path variable from its ``_ORIG`` +sibling (or drop it) inside a frozen build, without disturbing any +other inherited variable. +""" +from __future__ import annotations + +import os +import sys +import unittest +from unittest.mock import patch + +from apm_cli.utils.subprocess_env import external_process_env + + +class TestExternalProcessEnvNotFrozen(unittest.TestCase): + """Outside a frozen build the helper is a pure ``dict`` copy.""" + + def test_returns_independent_copy_of_os_environ(self): + with patch.object(sys, "frozen", False, create=True): + env = external_process_env() + self.assertIsNot(env, os.environ) + self.assertEqual(env, dict(os.environ)) + + def test_leaves_library_path_vars_alone_when_not_frozen(self): + base = { + "PATH": "/usr/bin", + "LD_LIBRARY_PATH": "/bundle/_internal", + "LD_LIBRARY_PATH_ORIG": "/usr/lib", + "DYLD_LIBRARY_PATH": "/bundle/_internal", + } + with patch.object(sys, "frozen", False, create=True): + env = external_process_env(base) + self.assertEqual(env, base) + self.assertIsNot(env, base) + + def test_frozen_attribute_absent_is_treated_as_not_frozen(self): + base = {"LD_LIBRARY_PATH": "/bundle/_internal"} + # Ensure sys.frozen is truly absent for this call. + had_frozen = hasattr(sys, "frozen") + prior = getattr(sys, "frozen", None) + if had_frozen: + del sys.frozen # type: ignore[attr-defined] + try: + env = external_process_env(base) + finally: + if had_frozen: + sys.frozen = prior # type: ignore[attr-defined] + self.assertEqual(env, base) + + +class TestExternalProcessEnvFrozen(unittest.TestCase): + """Inside a frozen build the helper restores the library-path vars.""" + + def test_restores_ld_library_path_from_orig(self): + base = { + "PATH": "/usr/bin", + "LD_LIBRARY_PATH": "/usr/local/lib/apm/_internal", + "LD_LIBRARY_PATH_ORIG": "/usr/lib/x86_64-linux-gnu", + } + with patch.object(sys, "frozen", True, create=True): + env = external_process_env(base) + self.assertEqual(env["LD_LIBRARY_PATH"], "/usr/lib/x86_64-linux-gnu") + self.assertNotIn("LD_LIBRARY_PATH_ORIG", env) + # PATH must not be touched. + self.assertEqual(env["PATH"], "/usr/bin") + + def test_drops_ld_library_path_when_no_orig(self): + """No ``_ORIG`` sibling means the user had no pre-launch value. + + Scenario that triggered issue #894: PyInstaller set + ``LD_LIBRARY_PATH`` but the user never exported one, so the only + correct restoration is to remove the variable entirely. + """ + base = { + "PATH": "/usr/bin", + "LD_LIBRARY_PATH": "/usr/local/lib/apm/_internal", + } + with patch.object(sys, "frozen", True, create=True): + env = external_process_env(base) + self.assertNotIn("LD_LIBRARY_PATH", env) + self.assertEqual(env["PATH"], "/usr/bin") + + def test_preserves_user_exported_empty_orig(self): + """An empty ``_ORIG`` reflects the user having no export; honour it.""" + base = { + "LD_LIBRARY_PATH": "/bundle", + "LD_LIBRARY_PATH_ORIG": "", + } + with patch.object(sys, "frozen", True, create=True): + env = external_process_env(base) + # The restored value is the original (empty string), which is + # semantically different from "unset" but matches what the user's + # shell had at launch -- we honour it rather than second-guessing. + self.assertEqual(env["LD_LIBRARY_PATH"], "") + self.assertNotIn("LD_LIBRARY_PATH_ORIG", env) + + def test_handles_all_dyld_variants(self): + base = { + "DYLD_LIBRARY_PATH": "/bundle", + "DYLD_LIBRARY_PATH_ORIG": "/usr/local/lib", + "DYLD_FRAMEWORK_PATH": "/bundle", + # No DYLD_FRAMEWORK_PATH_ORIG -> should be dropped. + } + with patch.object(sys, "frozen", True, create=True): + env = external_process_env(base) + self.assertEqual(env["DYLD_LIBRARY_PATH"], "/usr/local/lib") + self.assertNotIn("DYLD_LIBRARY_PATH_ORIG", env) + self.assertNotIn("DYLD_FRAMEWORK_PATH", env) + + def test_noop_when_no_library_path_vars_present(self): + base = {"PATH": "/usr/bin", "HOME": "/home/user"} + with patch.object(sys, "frozen", True, create=True): + env = external_process_env(base) + self.assertEqual(env, base) + self.assertIsNot(env, base) + + def test_base_mapping_overrides_os_environ(self): + """``base`` must take precedence over the live environment.""" + # Inject noise into os.environ that the helper must ignore when + # ``base`` is supplied. + with patch.dict( + os.environ, + {"LD_LIBRARY_PATH": "/noise", "LD_LIBRARY_PATH_ORIG": "/noise_orig"}, + clear=False, + ), patch.object(sys, "frozen", True, create=True): + base = { + "LD_LIBRARY_PATH": "/bundle", + "LD_LIBRARY_PATH_ORIG": "/real_orig", + } + env = external_process_env(base) + self.assertEqual(env["LD_LIBRARY_PATH"], "/real_orig") + self.assertNotIn("LD_LIBRARY_PATH_ORIG", env) + + def test_does_not_mutate_input_mapping(self): + base = { + "LD_LIBRARY_PATH": "/bundle", + "LD_LIBRARY_PATH_ORIG": "/usr/lib", + } + snapshot = dict(base) + with patch.object(sys, "frozen", True, create=True): + external_process_env(base) + self.assertEqual(base, snapshot) + + def test_does_not_mutate_os_environ(self): + with patch.dict( + os.environ, + { + "LD_LIBRARY_PATH": "/bundle", + "LD_LIBRARY_PATH_ORIG": "/usr/lib", + }, + clear=False, + ), patch.object(sys, "frozen", True, create=True): + snapshot = dict(os.environ) + external_process_env() + self.assertEqual(dict(os.environ), snapshot) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_update_command.py b/tests/unit/test_update_command.py index c8308cfef..8456fa75f 100644 --- a/tests/unit/test_update_command.py +++ b/tests/unit/test_update_command.py @@ -95,6 +95,11 @@ def test_update_uses_powershell_installer_on_windows( self.assertEqual(run_command[:3], ["powershell.exe", "-ExecutionPolicy", "Bypass"]) self.assertEqual(run_command[3], "-File") mock_chmod.assert_not_called() + # The installer is always spawned with an explicit sanitised env; + # see issue #894. On Windows the helper is effectively a no-op, but + # passing env= unconditionally keeps one code path across platforms. + self.assertIn("env", mock_run.call_args.kwargs) + self.assertIsNotNone(mock_run.call_args.kwargs["env"]) @patch("requests.get") @patch("subprocess.run") @@ -129,6 +134,11 @@ def test_update_uses_shell_installer_on_unix( self.assertEqual(run_command[0], "/bin/sh") self.assertEqual(run_command[1][-3:], ".sh") mock_chmod.assert_called_once() + # Regression guard for issue #894: the installer must be spawned with + # a sanitised env so system curl / tar do not inherit PyInstaller's + # LD_LIBRARY_PATH pointing at the bundle's _internal directory. + self.assertIn("env", mock_run.call_args.kwargs) + self.assertIsNotNone(mock_run.call_args.kwargs["env"]) class TestUpdatePlatformHelpers(unittest.TestCase):