Block separate git directories during clone

codex · Byron · commit dd59d2567b44 · 2026-08-10T11:43:38.000+02:00
Repo.clone() and Repo.clone_from() did not reject the clone option that redirects repository metadata to a caller-controlled path (GHSA-8mcc-hrx5-hvxc). Regression coverage exercises both keyword and multi-option input through both public clone APIs. Add the option to the existing clone denylist, matching Repo.init() and the documented allow_unsafe_options contract. Git itself registers The Python package and Alpine test workflows failed across the submodule suite because GitPython internally supplies --separate-git-dir when creating modern submodule layouts. The new public clone guard correctly rejected that option, but could not distinguish the library-generated path from caller input. Validate caller-provided keyword and multi-options before adding the library-controlled metadata path, then explicitly allow the resulting trusted clone invocation. This preserves rejection of unsafe clone_multi_options while restoring normal submodule creation. Update the one test that intentionally invokes Repo.clone_from() with its own separate git directory to opt in explicitly. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
@@ -9,6 +9,7 @@
 import ntpath
 import os
 import os.path as osp
+import shlex
 import stat
 import sys
 import uuid
@@ -363,6 +364,15 @@ def _clone_repo(
         module_abspath = cls._module_abspath(repo, path, name)
         module_checkout_path = module_abspath
         if cls._need_gitfile_submodules(repo.git):
+            if not allow_unsafe_options:
+                Git.check_unsafe_options(Git._option_candidates([], kwargs), repo.unsafe_git_clone_options)
+                multi_options = kwargs.get("multi_options")
+                if multi_options:
+                    Git.check_unsafe_options(
+                        shlex.split(" ".join(cast("Sequence[str]", multi_options))),
+                        repo.unsafe_git_clone_options,
+                    )
+            allow_unsafe_options = True
             kwargs["separate_git_dir"] = module_abspath
             module_abspath_dir = osp.dirname(module_abspath)
             if not osp.isdir(module_abspath_dir):
diff --git a/git/repo/base.py b/git/repo/base.py
@@ -159,6 +159,8 @@ class Repo:
         "-c",
         # Can install hooks that execute during clone:
         "--template",
+        # Redirects the repository metadata to a caller-controlled path:
+        "--separate-git-dir",
         # Fetches from an additional caller-controlled URI:
         "--bundle-uri",
     ]
diff --git a/test/test_clone.py b/test/test_clone.py
@@ -133,6 +133,7 @@ def test_clone_unsafe_options(self, rw_repo):
                 "-vcprotocol.ext.allow=always",
                 f"--template={tmp_dir}",
                 f"--bundle-uri=file://{tmp_dir}",
+                f"--separate-git-dir={tmp_dir / 'git-dir'}",
             ]
             for unsafe_option in unsafe_options:
                 with self.assertRaises(UnsafeOptionError):
@@ -149,6 +150,7 @@ def test_clone_unsafe_options(self, rw_repo):
                 {"c": "protocol.ext.allow=always"},
                 {"template": tmp_dir},
                 {"bundle_uri": f"file://{tmp_dir}"},
+                {"separate_git_dir": tmp_dir / "git-dir"},
             ]
             for unsafe_option in unsafe_options:
                 with self.assertRaises(UnsafeOptionError):
@@ -258,6 +260,7 @@ def test_clone_from_unsafe_options(self, rw_repo):
                 "-c protocol.ext.allow=always",
                 "-cprotocol.ext.allow=always",
                 "-vcprotocol.ext.allow=always",
+                f"--separate-git-dir={tmp_dir / 'git-dir'}",
             ]
             for unsafe_option in unsafe_options:
                 with self.assertRaises(UnsafeOptionError):
@@ -270,6 +273,7 @@ def test_clone_from_unsafe_options(self, rw_repo):
                 {"u": f"touch {tmp_file}"},
                 {"config": "protocol.ext.allow=always"},
                 {"c": "protocol.ext.allow=always"},
+                {"separate_git_dir": tmp_dir / "git-dir"},
             ]
             for unsafe_option in unsafe_options:
                 with self.assertRaises(UnsafeOptionError):
diff --git a/test/test_submodule.py b/test/test_submodule.py
@@ -954,6 +954,7 @@ def test_update_rejects_parent_component_in_name(self, rwdir):
             source.working_tree_dir,
             osp.join(clone.working_tree_dir, "module"),
             separate_git_dir=osp.join(rwdir, "escaped", "module"),
+            allow_unsafe_options=True,
         )
         with pytest.raises(ValueError, match="submodule name"):
             clone.submodules[0].update(init=True)

Original file line number	Diff line number	Diff line change
`@@ -159,6 +159,8 @@ class Repo:`
`159`	`159`	`"-c",`
`160`	`160`	`# Can install hooks that execute during clone:`
`161`	`161`	`"--template",`
	`162`	`+ # Redirects the repository metadata to a caller-controlled path:`
	`163`	`+ "--separate-git-dir",`
`162`	`164`	`# Fetches from an additional caller-controlled URI:`
`163`	`165`	`"--bundle-uri",`
`164`	`166`	`]`
Original file line number	Diff line number	Diff line change
`@@ -954,6 +954,7 @@ def test_update_rejects_parent_component_in_name(self, rwdir):`
`954`	`954`	`source.working_tree_dir,`
`955`	`955`	`osp.join(clone.working_tree_dir, "module"),`
`956`	`956`	`separate_git_dir=osp.join(rwdir, "escaped", "module"),`
	`957`	`+ allow_unsafe_options=True,`
`957`	`958`	`)`
`958`	`959`	`with pytest.raises(ValueError, match="submodule name"):`
`959`	`960`	`clone.submodules[0].update(init=True)`