Skip to content

Commit dd59d25

Browse files
codexByron
authored andcommitted
Block separate git directories during clone
<!-- agent --> 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>
1 parent 9729ed3 commit dd59d25

4 files changed

Lines changed: 17 additions & 0 deletions

File tree

git/objects/submodule/base.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import ntpath
1010
import os
1111
import os.path as osp
12+
import shlex
1213
import stat
1314
import sys
1415
import uuid
@@ -363,6 +364,15 @@ def _clone_repo(
363364
module_abspath = cls._module_abspath(repo, path, name)
364365
module_checkout_path = module_abspath
365366
if cls._need_gitfile_submodules(repo.git):
367+
if not allow_unsafe_options:
368+
Git.check_unsafe_options(Git._option_candidates([], kwargs), repo.unsafe_git_clone_options)
369+
multi_options = kwargs.get("multi_options")
370+
if multi_options:
371+
Git.check_unsafe_options(
372+
shlex.split(" ".join(cast("Sequence[str]", multi_options))),
373+
repo.unsafe_git_clone_options,
374+
)
375+
allow_unsafe_options = True
366376
kwargs["separate_git_dir"] = module_abspath
367377
module_abspath_dir = osp.dirname(module_abspath)
368378
if not osp.isdir(module_abspath_dir):

git/repo/base.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ class Repo:
159159
"-c",
160160
# Can install hooks that execute during clone:
161161
"--template",
162+
# Redirects the repository metadata to a caller-controlled path:
163+
"--separate-git-dir",
162164
# Fetches from an additional caller-controlled URI:
163165
"--bundle-uri",
164166
]

test/test_clone.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ def test_clone_unsafe_options(self, rw_repo):
133133
"-vcprotocol.ext.allow=always",
134134
f"--template={tmp_dir}",
135135
f"--bundle-uri=file://{tmp_dir}",
136+
f"--separate-git-dir={tmp_dir / 'git-dir'}",
136137
]
137138
for unsafe_option in unsafe_options:
138139
with self.assertRaises(UnsafeOptionError):
@@ -149,6 +150,7 @@ def test_clone_unsafe_options(self, rw_repo):
149150
{"c": "protocol.ext.allow=always"},
150151
{"template": tmp_dir},
151152
{"bundle_uri": f"file://{tmp_dir}"},
153+
{"separate_git_dir": tmp_dir / "git-dir"},
152154
]
153155
for unsafe_option in unsafe_options:
154156
with self.assertRaises(UnsafeOptionError):
@@ -258,6 +260,7 @@ def test_clone_from_unsafe_options(self, rw_repo):
258260
"-c protocol.ext.allow=always",
259261
"-cprotocol.ext.allow=always",
260262
"-vcprotocol.ext.allow=always",
263+
f"--separate-git-dir={tmp_dir / 'git-dir'}",
261264
]
262265
for unsafe_option in unsafe_options:
263266
with self.assertRaises(UnsafeOptionError):
@@ -270,6 +273,7 @@ def test_clone_from_unsafe_options(self, rw_repo):
270273
{"u": f"touch {tmp_file}"},
271274
{"config": "protocol.ext.allow=always"},
272275
{"c": "protocol.ext.allow=always"},
276+
{"separate_git_dir": tmp_dir / "git-dir"},
273277
]
274278
for unsafe_option in unsafe_options:
275279
with self.assertRaises(UnsafeOptionError):

test/test_submodule.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,7 @@ def test_update_rejects_parent_component_in_name(self, rwdir):
954954
source.working_tree_dir,
955955
osp.join(clone.working_tree_dir, "module"),
956956
separate_git_dir=osp.join(rwdir, "escaped", "module"),
957+
allow_unsafe_options=True,
957958
)
958959
with pytest.raises(ValueError, match="submodule name"):
959960
clone.submodules[0].update(init=True)

0 commit comments

Comments
 (0)