From b68afff45af0f49e79a3e2d2162018986b37ad5d Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 11:20:03 +0200 Subject: [PATCH] Block separate git directories during clone 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 Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/objects/submodule/base.py | 10 ++++++++++ git/repo/base.py | 2 ++ test/test_clone.py | 4 ++++ test/test_submodule.py | 1 + 4 files changed, 17 insertions(+) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index da0e09af4..d52ee4459 100644 --- 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 index 583e96ca4..d0ec00ec9 100644 --- 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 index 5af67613a..a6c3db6f9 100644 --- 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 index 287986059..28e865ff2 100644 --- 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)