fix: prevent environment expansion in clone URLs

Repo.clone_from() expanded environment-variable references in caller-supplied URLs before passing them to git clone. This could expose process environment values to an untrusted remote and made protocol validation apply to a different value than Git received.

Polish clone URLs without variable or home expansion, then apply unsafe-protocol validation to that exact normalized value. Preserve the literal URL when normalizing the stored origin after a successful clone, while retaining the existing Git.polish_url() default for callers that intentionally normalize local paths. Add regression coverage for POSIX and Windows variable syntax, stored origins, and post-normalization protocol validation.

Git baseline: git clone passes URL arguments through literally; t/t5601-clone.sh covers the accepted URL forms without shell-style environment expansion.

Security: GHSA-rwj8-pgh3-r573.

Validation:
- pytest test/test_clone.py -q (20 passed, 1 skipped)
- ruff check on changed files
- ruff format --check on changed files
- full pytest suite (681 passed, 73 skipped, 1 xfailed; 7 unrelated environment/baseline failures)

Author: codex

git/cmd.py

@@ -903,29 +903,34 @@ def is_cygwin(cls) -> bool:
 
     @overload
     @classmethod
-    def polish_url(cls, url: str, is_cygwin: Literal[False] = ...) -> str: ...
+    def polish_url(cls, url: str, is_cygwin: Literal[False] = ..., expand_vars: bool = ...) -> str: ...
 
     @overload
     @classmethod
-    def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> str: ...
+    def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None, expand_vars: bool = True) -> str: ...
 
     @classmethod
-    def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> PathLike:
+    def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None, expand_vars: bool = True) -> PathLike:
         """Remove any backslashes from URLs to be written in config files.
 
         Windows might create config files containing paths with backslashes, but git
         stops liking them as it will escape the backslashes. Hence we undo the escaping
         just to be sure.
+
+        :param expand_vars:
+            Expand environment variables and an initial ``~``. Disable this for values
+            obtained from an untrusted source, such as remote URLs.
         """
         if is_cygwin is None:
             is_cygwin = cls.is_cygwin()
 
         if is_cygwin:
             url = cygpath(url)
         else:
-            url = os.path.expandvars(url)
-            if url.startswith("~"):
-                url = os.path.expanduser(url)
+            if expand_vars:
+                url = os.path.expandvars(url)
+                if url.startswith("~"):
+                    url = os.path.expanduser(url)
             url = url.replace("\\\\", "\\").replace("\\", "/")
         return url
 

git/repo/base.py

@@ -1452,8 +1452,9 @@ def _clone(
         if multi_options:
             multi = shlex.split(" ".join(multi_options))
 
+        clone_url = Git.polish_url(url, expand_vars=False)
         if not allow_unsafe_protocols:
-            Git.check_unsafe_protocols(url)
+            Git.check_unsafe_protocols(clone_url)
         if not allow_unsafe_options:
             Git.check_unsafe_options(
                 options=Git._option_candidates([], kwargs),
@@ -1465,7 +1466,7 @@ def _clone(
         proc = git.clone(
             multi,
             "--",
-            Git.polish_url(url),
+            clone_url,
             clone_path,
             with_extended_output=True,
             as_process=True,
@@ -1505,7 +1506,7 @@ def _clone(
         # escape the backslashes. Hence we undo the escaping just to be sure.
         if repo.remotes:
             with repo.remotes[0].config_writer as writer:
-                writer.set_value("url", Git.polish_url(repo.remotes[0].url))
+                writer.set_value("url", Git.polish_url(repo.remotes[0].url, expand_vars=False))
         # END handle remote repo
         return repo
 

test/test_clone.py

@@ -7,8 +7,9 @@
 import sys
 import tempfile
 from unittest import skip
+from unittest import mock
 
-from git import GitCommandError, Repo
+from git import Git, GitCommandError, Repo
 from git.exc import UnsafeOptionError, UnsafeProtocolError
 
 from test.lib import TestBase, with_rw_directory, with_rw_repo, PathLikeMock
@@ -346,6 +347,38 @@ def test_clone_from_unsafe_protocol(self):
                     Repo.clone_from(url, tmp_dir / "repo")
                 assert not tmp_file.exists()
 
+    def test_clone_from_does_not_expand_environment_variables_in_url(self):
+        urls = [
+            "https://example.com/$GITPYTHON_TEST_SECRET/repo.git",
+            "https://example.com/${GITPYTHON_TEST_SECRET}/repo.git",
+            "https://example.com/%GITPYTHON_TEST_SECRET%/repo.git",
+        ]
+        with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
+            for url in urls:
+                with mock.patch.object(Git, "_call_process", side_effect=RuntimeError) as call_process:
+                    with self.assertRaises(RuntimeError):
+                        Repo.clone_from(url, "unused")
+
+                assert call_process.call_args.args[3] == url
+
+    @with_rw_directory
+    def test_clone_from_does_not_expand_environment_variables_in_stored_url(self, rw_dir):
+        url = pathlib.Path(rw_dir) / "$GITPYTHON_TEST_SECRET" / "source"
+        Git().init(url)
+
+        with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
+            cloned = Repo.clone_from(url, pathlib.Path(rw_dir) / "clone")
+
+        assert cloned.remotes.origin.url == str(url)
+
+    def test_clone_from_checks_polished_url_for_unsafe_protocol(self):
+        with mock.patch.object(Git, "polish_url", return_value="ext::command"):
+            with mock.patch.object(Git, "_call_process") as call_process:
+                with self.assertRaises(UnsafeProtocolError):
+                    Repo.clone_from("$GITPYTHON_TEST_URL", "unused")
+
+        call_process.assert_not_called()
+
     def test_clone_from_unsafe_protocol_allowed(self):
         with tempfile.TemporaryDirectory() as tdir:
             tmp_dir = pathlib.Path(tdir)