fix: prevent environment expansion in remote URLs

codex · Byron · commit 333c6ba74010 · 2026-07-22T15:42:44.000+02:00
Remote and submodule creation expanded environment-variable references in caller-supplied URLs before storing them in Git configuration. This could expose process environment values through a later network request or a tracked .gitmodules file. Preserve variables literally in the remaining untrusted URL callers while retaining Git.polish_url() expansion for intentional local-path normalization. Add regression tests covering remote configuration and submodule configuration. Git baseline: git remote add and git submodule add accept URL arguments literally; Git does not perform shell-style environment expansion on those arguments. Security: GHSA-94p4-4cq8-9g67. Validation: - focused remote and submodule regression tests - pre-commit hooks for changed files - git diff --check - full remote/submodule modules: 56 passed, 1 skipped, 14 fixture failures caused by unavailable nested test repositories and the historical master-branch fixture
diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
@@ -608,7 +608,7 @@ def add(
         # END verify url
 
         ## See #525 for ensuring git URLs in config-files are valid under Windows.
-        url = Git.polish_url(url)
+        url = Git.polish_url(url, expand_vars=False)
 
         # It's important to add the URL to the parent config, to let `git submodule` know.
         # Otherwise there is a '-' character in front of the submodule listing:
diff --git a/git/remote.py b/git/remote.py
@@ -808,7 +808,7 @@ def create(cls, repo: "Repo", name: str, url: str, allow_unsafe_protocols: bool
         """
         scmd = "add"
         kwargs["insert_kwargs_after"] = scmd
-        url = Git.polish_url(url)
+        url = Git.polish_url(url, expand_vars=False)
         if not allow_unsafe_protocols:
             Git.check_unsafe_protocols(url)
         repo.git.remote(scmd, "--", name, url, **kwargs)
diff --git a/test/test_remote.py b/test/test_remote.py
@@ -4,6 +4,7 @@
 # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
 
 import gc
+import os
 import os.path as osp
 from pathlib import Path
 import random
@@ -685,6 +686,14 @@ def test_multiple_urls(self, rw_repo):
         # Will raise fatal: Will not delete all non-push URLs.
         self.assertRaises(GitCommandError, remote.delete_url, test3)
 
+    @with_rw_repo("HEAD", bare=False)
+    def test_create_does_not_expand_environment_variables_in_url(self, rw_repo):
+        url = "https://example.com/${GITPYTHON_TEST_SECRET}/repo.git"
+        with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
+            remote = rw_repo.create_remote("untrusted", url)
+
+        assert remote.url == url
+
     def test_fetch_error(self):
         rem = self.rorepo.remote("origin")
         msg = (
diff --git a/test/test_submodule.py b/test/test_submodule.py
@@ -1355,6 +1355,20 @@ def test_add_no_clone_multi_options_argument(self, rwdir):
         with self.assertRaises(cp.NoOptionError):
             sm_config.get_value("core", "eol")
 
+    @with_rw_directory
+    def test_add_does_not_expand_environment_variables_in_url(self, rwdir):
+        parent = git.Repo.init(osp.join(rwdir, "parent"))
+        url = osp.join(rwdir, "${GITPYTHON_TEST_SECRET}")
+        source = git.Repo.init(url)
+        Path(source.working_tree_dir, "file").write_text("content")
+        source.index.add(["file"])
+        source.index.commit("initial")
+
+        with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
+            sm = Submodule.add(parent, "new", "new", url)
+
+        assert sm.url == Git.polish_url(url, expand_vars=False)
+
     @with_rw_repo("HEAD")
     def test_submodule_add_unsafe_url(self, rw_repo):
         with tempfile.TemporaryDirectory() as tdir:
