diff --git a/src/fromager/build_environment.py b/src/fromager/build_environment.py index 1be2fe5b..ba4bd38d 100644 --- a/src/fromager/build_environment.py +++ b/src/fromager/build_environment.py @@ -186,6 +186,7 @@ def run( cwd=cwd, extra_environ=extra_environ, network_isolation=network_isolation, + env_filter=self._ctx.settings.external_commands, log_filename=log_filename, stdin=stdin, ) @@ -209,6 +210,7 @@ def _createenv(self) -> None: external_commands.run( cmd, network_isolation=self._ctx.network_isolation, + env_filter=self._ctx.settings.external_commands, ) logger.info("created build environment in %s", self.path) diff --git a/src/fromager/context.py b/src/fromager/context.py index 8ce9dd71..da29b7c5 100644 --- a/src/fromager/context.py +++ b/src/fromager/context.py @@ -154,7 +154,11 @@ def uv_clean_cache(self, *reqs: Requirement) -> None: req_list: list[str] = sorted(set(req.name for req in reqs)) logger.debug("invalidate uv cache for %s", req_list) cmd.extend(req_list) - external_commands.run(cmd, extra_environ=extra_environ) + external_commands.run( + cmd, + extra_environ=extra_environ, + env_filter=self.settings.external_commands, + ) def package_build_info( self, package: str | packagesettings.Package | Requirement diff --git a/src/fromager/dependencies.py b/src/fromager/dependencies.py index d80899e8..72dc2391 100644 --- a/src/fromager/dependencies.py +++ b/src/fromager/dependencies.py @@ -554,6 +554,7 @@ def _run_hook_with_extra_environ( cwd=cwd, extra_environ=extra_environ, network_isolation=ctx.network_isolation, + env_filter=ctx.settings.external_commands, log_filename=log_filename, ) diff --git a/src/fromager/external_commands.py b/src/fromager/external_commands.py index 6c4a2607..37f4011f 100644 --- a/src/fromager/external_commands.py +++ b/src/fromager/external_commands.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import logging import os import pathlib @@ -9,6 +11,9 @@ from . import log +if typing.TYPE_CHECKING: + from . import packagesettings + logger = logging.getLogger(__name__) HERE = pathlib.Path(__file__).absolute().parent @@ -55,6 +60,7 @@ def run( cwd: str | None = None, extra_environ: dict[str, typing.Any] | None = None, network_isolation: bool = False, + env_filter: packagesettings.ExternalCommands | None = None, log_filename: str | None = None, stdin: TextIOWrapper | None = None, ) -> str: @@ -64,10 +70,16 @@ def run( line with the current package name for easier searching. Raises ``NetworkIsolationError`` instead of ``CalledProcessError`` when the failure output indicates a network access problem. + + When *env_filter* is not ``None``, ``os.environ`` is filtered through + ``ExternalCommands.filter_env()`` before *extra_environ* is applied. + Variables injected via *extra_environ* are never filtered. """ if extra_environ is None: extra_environ = {} env = os.environ.copy() + if env_filter is not None: + env = dict(env_filter.filter_env(env)) env.update(extra_environ) if network_isolation: diff --git a/src/fromager/wheels.py b/src/fromager/wheels.py index dc9bd524..eb17668e 100644 --- a/src/fromager/wheels.py +++ b/src/fromager/wheels.py @@ -281,6 +281,7 @@ def add_extra_metadata_to_wheels( cmd, cwd=dir_name, network_isolation=ctx.network_isolation, + env_filter=ctx.settings.external_commands, ) wheel_file.unlink(missing_ok=True) diff --git a/tests/test_external_commands.py b/tests/test_external_commands.py index 12af7fab..b629f88a 100644 --- a/tests/test_external_commands.py +++ b/tests/test_external_commands.py @@ -9,7 +9,7 @@ from packaging.requirements import Requirement from packaging.version import Version -from fromager import external_commands, log +from fromager import external_commands, log, packagesettings def test_external_commands_environ() -> None: @@ -176,3 +176,65 @@ def test_format_exception_formats_chained_exceptions() -> None: assert "Higher level error" in formatted assert "because" in formatted assert "Root cause" in formatted + + +# --- env_filter wiring tests --- + + +@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0, stdout=b"")) +@mock.patch.dict(os.environ, {"HOME": "/h", "SECRET": "s"}, clear=True) +def test_run_env_filter_none_passes_full_environ(m_run: mock.Mock) -> None: + """Without env_filter, the full os.environ is passed to the subprocess.""" + external_commands.run(["true"]) + call_env = m_run.call_args.kwargs["env"] + assert call_env["HOME"] == "/h" + assert call_env["SECRET"] == "s" + + +@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0, stdout=b"")) +@mock.patch.dict(os.environ, {"HOME": "/h", "SECRET": "s"}, clear=True) +def test_run_env_filter_strips_deleted_vars(m_run: mock.Mock) -> None: + """With env_filter configured, deleted vars are stripped from the env.""" + env_filter = packagesettings.ExternalCommands(delete_env=["*"]) + external_commands.run(["true"], env_filter=env_filter) + call_env = m_run.call_args.kwargs["env"] + assert call_env["HOME"] == "/h" + assert "SECRET" not in call_env + + +@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0, stdout=b"")) +@mock.patch.dict(os.environ, {"HOME": "/h", "SECRET": "s"}, clear=True) +def test_run_env_filter_extra_environ_survives(m_run: mock.Mock) -> None: + """extra_environ values are applied after filtering and never stripped.""" + env_filter = packagesettings.ExternalCommands(delete_env=["*"]) + external_commands.run( + ["true"], + extra_environ={"MY_BUILD_VAR": "42"}, + env_filter=env_filter, + ) + call_env = m_run.call_args.kwargs["env"] + assert call_env["MY_BUILD_VAR"] == "42" + assert "SECRET" not in call_env + + +@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0, stdout=b"")) +@mock.patch.dict(os.environ, {"HOME": "/h", "CI_TOKEN": "t", "USER": "u"}, clear=True) +def test_run_env_filter_selective_delete(m_run: mock.Mock) -> None: + """Selective delete_env removes only matching vars.""" + env_filter = packagesettings.ExternalCommands(delete_env=["CI_TOKEN"]) + external_commands.run(["true"], env_filter=env_filter) + call_env = m_run.call_args.kwargs["env"] + assert call_env["HOME"] == "/h" + assert call_env["USER"] == "u" + assert "CI_TOKEN" not in call_env + + +@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0, stdout=b"")) +@mock.patch.dict(os.environ, {"HOME": "/h", "SECRET": "s"}, clear=True) +def test_run_env_filter_default_is_noop(m_run: mock.Mock) -> None: + """Default ExternalCommands (empty lists) passes all POSIX-valid vars.""" + env_filter = packagesettings.ExternalCommands() + external_commands.run(["true"], env_filter=env_filter) + call_env = m_run.call_args.kwargs["env"] + assert call_env["HOME"] == "/h" + assert call_env["SECRET"] == "s"