chore: use env variable of subproces.run instead of patching os.envir…

…on directly

Signed-off-by: Trong Nhan Mai <[email protected]>

docs/source/pages/developers_guide/apidoc/macaron.rst

@@ -25,6 +25,14 @@ Subpackages
 Submodules
 ----------
 
+macaron.env module
+------------------
+
+.. automodule:: macaron.env
+   :members:
+   :undoc-members:
+   :show-inheritance:
+
 macaron.errors module
 ---------------------
 

src/macaron/env.py

@@ -1,46 +1,45 @@
 # Copyright (c) 2023 - 2023, Oracle and/or its affiliates. All rights reserved.
 # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/.
 
-"""Context managers related to environment variables."""
+"""Helper functions related to environment variables."""
 
-import contextlib
 import os
-from collections.abc import Generator, Mapping
+from collections.abc import Mapping, MutableMapping
 
 
-@contextlib.contextmanager
-def patched_env(
+def get_patched_env(
     patch: Mapping[str, str | None],
     _env: dict[str, str] | None = None,
-) -> Generator[None, None, None]:
-    """Create a context in which ``os.environ`` is temporarily updated according to ``patch``.
+) -> MutableMapping[str, str]:
+    """Return a mapping whose elements copied from ``os.environ`` and are updated according to ``patch``.
 
-    Out of the context, ``os.environ`` retains its original state.
+    This function does not modify ``os.environ``.
 
     Parameters
     ----------
     patch : Mapping[str, str | None]
         A mapping (immutable) in which:
-        - each key is an environment variable
+        - each key is an environment variable.
         - each value is the value to set to the corresponding environment variable.
         If value is ``None``, the environment variable is "unset".
     _env : dict[str, str] | None
         The environment being updated.
         This is ``None`` by default, in which case ``os.environ`` is being updated.
+
+    Returns
+    -------
+    MutableMapping[str, str]
+        The patched mutable mapping which will not reflect any changes on it to ``os.environ``.
     """
     env = os.environ if _env is None else _env
 
-    # Make a copy of the environment before updating.
-    before = dict(env)
+    # Make a copy of the environment.
+    copied_env = dict(env)
 
     for var, value in patch.items():
         if value is None:
-            env.pop(var, None)
+            copied_env.pop(var, None)
         else:
-            env[var] = value
-
-    yield
+            copied_env[var] = value
 
-    # Restore the environment to the original state.
-    env.clear()
-    env.update(before)
+    return copied_env

src/macaron/slsa_analyzer/git_url.py

@@ -19,7 +19,7 @@
 from pydriller.git import Git
 
 from macaron.config.defaults import defaults
-from macaron.env import patched_env
+from macaron.env import get_patched_env
 from macaron.errors import CloneError
 
 logger: logging.Logger = logging.getLogger(__name__)
@@ -287,16 +287,16 @@ def clone_remote_repo(clone_dir: str, url: str) -> Repo | None:
             # ``git clone`` from prompting for login credentials.
             "GIT_TERMINAL_PROMPT": "0",
         }
-        with patched_env(git_env_patch):
-            result = subprocess.run(  # nosec B603
-                args=["git", "clone", "--filter=tree:0", url],
-                capture_output=True,
-                cwd=parent_dir,
-                # If `check=True` and return status code is not zero, subprocess.CalledProcessError is
-                # raised, which we don't want. We want to check the return status code of the subprocess
-                # later on.
-                check=False,
-            )
+        result = subprocess.run(  # nosec B603
+            args=["git", "clone", "--filter=tree:0", url],
+            capture_output=True,
+            cwd=parent_dir,
+            # If `check=True` and return status code is not zero, subprocess.CalledProcessError is
+            # raised, which we don't want. We want to check the return status code of the subprocess
+            # later on.
+            check=False,
+            env=get_patched_env(git_env_patch),
+        )
     except (subprocess.CalledProcessError, OSError):
         # Here, we raise from ``None`` to be extra-safe that no token is leaked.
         # We should never store or print out the captured output from the subprocess

tests/test_env.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from macaron.env import patched_env
+from macaron.env import get_patched_env
 
 
 @pytest.mark.parametrize(
@@ -48,7 +48,4 @@ def test_patched_env(
     """Tests for the ``patched_env`` context manager."""
     env = dict(before)
 
-    with patched_env(patch, env):
-        assert env == after
-
-    assert env == before
+    assert get_patched_env(patch, env) == after