Merge pull request #2255 from gitpython-developers/fix-polish-url

fix(repo): preserve literal separate Git directory paths

Author: Byron

doc/source/changes.rst

@@ -9,6 +9,13 @@ Security fixes for
 
 * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-w8jc-g24h-crhw
 * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-m64x-33q8-m5h7
+* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-fx3j-rwgx-fr94
+
+If you can, also try and provide feedback on the upcoming v4 branch
+https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome.
+
+See the following for all changes.
+https://github.com/gitpython-developers/GitPython/releases/tag/3.2.1
 
 3.2.0
 =====
@@ -25,7 +32,7 @@ If you can, also try and provide feedback on the upcoming v4 branch
 https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome.
 
 See the following for all changes.
-https://github.com/gitpython-developers/GitPython/releases/tag/3.1.63
+https://github.com/gitpython-developers/GitPython/releases/tag/3.2.0
 
 3.1.62
 ======

git/repo/base.py

@@ -1537,7 +1537,7 @@ def _clone(
         clone_path = Git.polish_url(path) if Git.is_cygwin() and "bare" in kwargs else path
         sep_dir = kwargs.get("separate_git_dir")
         if sep_dir:
-            kwargs["separate_git_dir"] = Git.polish_url(sep_dir)
+            kwargs["separate_git_dir"] = Git.polish_url(os.fspath(sep_dir), expand_vars=False)
         multi = None
         if multi_options:
             multi = shlex.split(" ".join(multi_options))

git/repo/fun.py

@@ -151,7 +151,7 @@ def find_submodule_git_dir(d: PathLike) -> Optional[PathLike]:
         # Cygwin creates submodules prefixed with `/cygdrive/...`.
         # Cygwin git understands Cygwin paths much better than Windows ones.
         # Also the Cygwin tests are assuming Cygwin paths.
-        path = cygpath(path)
+        path = cygpath(path, expand_vars=False)
     if not osp.isabs(path):
         path = osp.normpath(osp.join(osp.dirname(d), path))
     return path if is_git_dir(path) else None

test/test_clone.py

@@ -21,6 +21,28 @@
 import pytest
 
 
+@pytest.mark.parametrize("clone_method", ["clone", "clone_from"])
+@pytest.mark.parametrize("path_type", [str, Path, PathLikeMock])
+@pytest.mark.parametrize("name", ["$GITPYTHON_TEST_SECRET", "${GITPYTHON_TEST_SECRET}", "%GITPYTHON_TEST_SECRET%"])
+def test_clone_preserves_literal_separate_git_dir(tmp_path, monkeypatch, caplog, clone_method, path_type, name):
+    monkeypatch.setenv("GITPYTHON_TEST_SECRET", "sensitive-value")
+    caplog.set_level("DEBUG", logger="git.cmd")
+    separate_git_dir = tmp_path / name
+    options = {"separate_git_dir": path_type(str(separate_git_dir)), "allow_unsafe_options": True}
+
+    with Repo.init(tmp_path / "source") as source:
+        if clone_method == "clone":
+            cloned = source.clone(tmp_path / "clone", **options)
+        else:
+            cloned = Repo.clone_from(source.git_dir, tmp_path / "clone", **options)
+        with cloned:
+            assert (separate_git_dir / "HEAD").is_file()
+            assert separate_git_dir.samefile(cloned.git_dir)
+
+    assert not (tmp_path / "sensitive-value").exists()
+    assert "sensitive-value" not in caplog.text
+
+
 class TestClone(TestBase):
     @with_rw_directory
     def test_checkout_in_non_empty_dir(self, rw_dir):

test/test_submodule.py

@@ -52,6 +52,36 @@ def _patch_git_config(name, value):
         yield
 
 
+@pytest.mark.parametrize(
+    "name", ["module", "$GITPYTHON_TEST_SECRET", "prefix-${GITPYTHON_TEST_SECRET}-suffix", "%GITPYTHON_TEST_SECRET%"]
+)
+def test_submodule_update_preserves_literal_name(tmp_path, monkeypatch, caplog, name):
+    monkeypatch.setenv("GITPYTHON_TEST_SECRET", "sensitive-value")
+    caplog.set_level("DEBUG", logger="git.cmd")
+    with git.Repo.init(tmp_path / "source") as source, git.Repo.init(tmp_path / "parent") as parent:
+        source.git.symbolic_ref("HEAD", "refs/heads/master")
+        source.index.commit("Initial commit")
+        with _patch_git_config("protocol.file.allow", "always"):
+            parent.git.submodule("add", "--name", name, source.working_tree_dir, "module")
+        parent.index.commit("Add submodule")
+
+        with git.Repo.clone_from(parent.working_tree_dir, tmp_path / "clone") as clone:
+            clone.submodule_update(init=True, recursive=True)
+
+            modules_dir = Path(clone.git_dir, "modules")
+            assert {path.name for path in modules_dir.iterdir()} == {name}
+            # Exercise relative gitfile conversion on every platform.
+            with mock.patch.object(Git, "is_cygwin", return_value=True):
+                resolved_git_dir = find_submodule_git_dir(Path(clone.working_tree_dir, "module", ".git"))
+            assert resolved_git_dir is not None
+            assert (modules_dir / name).samefile(resolved_git_dir)
+            with clone.submodules[0].module() as module:
+                assert (modules_dir / name).samefile(module.git_dir)
+                assert module.head.commit == source.head.commit
+
+    assert "sensitive-value" not in caplog.text
+
+
 @pytest.fixture
 def movable_submodule(tmp_path):
     """Create a committed local submodule whose logical name stays fixed when moved."""