From e8d0fbf774d1f6baa3b481adfe48bd262e43b453 Mon Sep 17 00:00:00 2001 From: "GPT 5.6" Date: Wed, 22 Jul 2026 04:39:00 +0200 Subject: [PATCH 1/3] fix: validate split short-option values Single-character keyword arguments are transformed into an option token and a separate value token. The unsafe-option candidate builder only checked the keyword name, allowing an option-like value to bypass guards shared by clone, remote, revision, blame, and archive operations. Include dash-prefixed values only when short options are actually split, including sequence values, while preserving bare values and the non-splitting compatibility path. Git baseline a23bace9 defines clone -n and --upload-pack as distinct options, matching the argv boundary this validation now preserves. Refs GHSA-r9mr-m37c-5fr3. Co-authored-by: Sebastian Thiel --- git/cmd.py | 7 +++++++ test/test_git.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/git/cmd.py b/git/cmd.py index 1868862d3..5cba11f97 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1039,11 +1039,18 @@ def _option_candidates(cls, args: Sequence[Any] = (), kwargs: Optional[Mapping[s option for option in cls._unpack_args([arg for arg in args if arg is not None]) if option.startswith("-") ] if kwargs: + split_single_char_options = kwargs.get("split_single_char_options", True) for key, value in kwargs.items(): values = value if isinstance(value, (list, tuple)) else (value,) if any(value is True or (value is not False and value is not None) for value in values): key = str(key) options.append(f"-{key}" if len(key) == 1 else f"--{dashify(key)}") + if len(key) == 1 and split_single_char_options: + options.extend( + str(value) + for value in values + if value is not True and value not in (False, None) and str(value).startswith("-") + ) return options AutoInterrupt: TypeAlias = _AutoInterrupt diff --git a/test/test_git.py b/test/test_git.py index e95992ba8..a6698a021 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -214,6 +214,23 @@ def test_option_candidates_ignore_untransformed_kwargs(self): self.assertEqual(options, ["--max-count"]) + def test_option_candidates_include_split_single_char_option_values(self): + cases = [ + ({"n": "--upload-pack=helper"}, ["-n", "--upload-pack=helper"], ["--upload-pack"]), + ({"g": ("safe", "--out=target")}, ["-g", "--out=target"], ["--output"]), + ] + + for kwargs, candidates, unsafe_options in cases: + self.assertEqual(Git._option_candidates(kwargs=kwargs), candidates) + with self.assertRaises(UnsafeOptionError): + Git.check_unsafe_options(options=candidates, unsafe_options=unsafe_options) + + self.assertEqual(self.git.transform_kwargs(n="--upload-pack=helper"), ["-n", "--upload-pack=helper"]) + + unsplit_kwargs = {"n": "--upload-pack=helper", "split_single_char_options": False} + self.assertEqual(self.git.transform_kwargs(**unsplit_kwargs), ["-n--upload-pack=helper"]) + self.assertEqual(Git._option_candidates(kwargs=unsplit_kwargs), ["-n"]) + _shell_cases = ( # value_in_call, value_from_class, expected_popen_arg (None, False, False), From ffcb5359e87619f4fe4a70a4aff5f08c5580ba97 Mon Sep 17 00:00:00 2001 From: "GPT 5.6" Date: Wed, 22 Jul 2026 04:43:26 +0200 Subject: [PATCH 2/3] fix: reject unsafe clone templates Treat git clone --template as unsafe because caller-controlled templates can install hooks that execute during clone. Add regression coverage for both direct option and keyword forms. References GHSA-6p8h-3wgx-97gf. Validated against Git baseline a23bace9. Co-authored-by: Sebastian Thiel --- git/repo/base.py | 5 +++++ test/test_clone.py | 2 ++ 2 files changed, 7 insertions(+) diff --git a/git/repo/base.py b/git/repo/base.py index e478396b2..ae99ed5f9 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -149,6 +149,8 @@ class Repo: # Can override configuration variables that execute arbitrary commands: "--config", "-c", + # Can install hooks that execute during clone: + "--template", ] """Options to :manpage:`git-clone(1)` that allow arbitrary commands to be executed. @@ -159,6 +161,9 @@ class Repo: The ``--config``/``-c`` option allows users to override configuration variables like ``protocol.allow`` and ``core.gitProxy`` to execute arbitrary commands: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---configltkeygtltvaluegt + + The ``--template`` option can install hooks that execute during clone: + https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---templatetemplate-directory """ unsafe_git_archive_options = [ diff --git a/test/test_clone.py b/test/test_clone.py index 5fd59b3a3..e13d92f5b 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -128,6 +128,7 @@ def test_clone_unsafe_options(self, rw_repo): "-c protocol.ext.allow=always", "-cprotocol.ext.allow=always", "-vcprotocol.ext.allow=always", + f"--template={tmp_dir}", ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -142,6 +143,7 @@ def test_clone_unsafe_options(self, rw_repo): {"config": "protocol.ext.allow=always"}, {"conf": "protocol.ext.allow=always"}, {"c": "protocol.ext.allow=always"}, + {"template": tmp_dir}, ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): From 1d51b891d7f236044a6aa17498ec682b63dad6e6 Mon Sep 17 00:00:00 2001 From: "GPT 5.6" Date: Wed, 22 Jul 2026 04:46:13 +0200 Subject: [PATCH 3/3] fix: guard diff output options Reject unsafe diff options before revision parsing or Git invocation so callers cannot write command output to arbitrary filesystem paths. Cover commit and index diffs, including option-like revisions, and preserve an explicit allow_unsafe_options escape hatch. References GHSA-fjr4-x663-mwxc. Validated against Git baseline a23bace9. Co-authored-by: Sebastian Thiel --- git/diff.py | 14 ++++++++++++-- git/index/base.py | 18 ++++++++++++++++-- test/test_diff.py | 20 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/git/diff.py b/git/diff.py index 5af53e556..24f79681a 100644 --- a/git/diff.py +++ b/git/diff.py @@ -9,7 +9,7 @@ import re import warnings -from git.cmd import handle_process_output +from git.cmd import Git, handle_process_output from git.compat import defenc from git.objects.blob import Blob from git.objects.util import mode_str_to_int @@ -35,7 +35,6 @@ if TYPE_CHECKING: from subprocess import Popen - from git.cmd import Git from git.objects.base import IndexObject from git.objects.commit import Commit from git.objects.tree import Tree @@ -190,6 +189,7 @@ def diff( other: Union[DiffConstants, "Tree", "Commit", str, None] = INDEX, paths: Union[PathLike, List[PathLike], Tuple[PathLike, ...], None] = None, create_patch: bool = False, + allow_unsafe_options: bool = False, **kwargs: Any, ) -> "DiffIndex[Diff]": """Create diffs between two items being trees, trees and index or an index and @@ -219,6 +219,10 @@ def diff( applied makes the self to other. Patches are somewhat costly as blobs have to be read and diffed. + :param allow_unsafe_options: + If ``True``, allow options such as ``--output`` that can write to arbitrary + filesystem paths. + :param kwargs: Additional arguments passed to :manpage:`git-diff(1)`, such as ``R=True`` to swap both sides of the diff. @@ -231,6 +235,12 @@ def diff( an instance of :class:`~git.objects.tree.Tree` or :class:`~git.objects.commit.Commit`, or a git command error will occur. """ + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([other], kwargs), + unsafe_options=self.repo.unsafe_git_revision_options, + ) + args: List[Union[PathLike, Diffable]] = [] args.append("--abbrev=40") # We need full shas. args.append("--full-index") # Get full index paths, not only filenames. diff --git a/git/index/base.py b/git/index/base.py index f03b452dc..8d7dbfc1f 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -23,6 +23,7 @@ from gitdb.db import MemoryDB from git.compat import defenc, force_bytes +from git.cmd import Git import git.diff as git_diff from git.exc import CheckoutError, GitCommandError, GitError, InvalidGitRepositoryError from git.objects import Blob, Commit, Object, Submodule, Tree @@ -1492,6 +1493,7 @@ def diff( ] = git_diff.INDEX, paths: Union[PathLike, List[PathLike], Tuple[PathLike, ...], None] = None, create_patch: bool = False, + allow_unsafe_options: bool = False, **kwargs: Any, ) -> git_diff.DiffIndex[git_diff.Diff]: """Diff this index against the working copy or a :class:`~git.objects.tree.Tree` @@ -1504,6 +1506,12 @@ def diff( Will only work with indices that represent the default git index as they have not been initialized with a stream. """ + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([other], kwargs), + unsafe_options=self.repo.unsafe_git_revision_options, + ) + # Only run if we are the default repository index. if self._file_path != self._index_path(): raise AssertionError("Cannot call %r on indices that do not represent the default git index" % self.diff()) @@ -1560,7 +1568,13 @@ def diff( # Invert the existing R flag. cur_val = kwargs.get("R", False) kwargs["R"] = not cur_val - return other.diff(self.INDEX, paths, create_patch, **kwargs) + return other.diff( + self.INDEX, + paths, + create_patch, + allow_unsafe_options=allow_unsafe_options, + **kwargs, + ) # END diff against other item handling # If other is not None here, something is wrong. @@ -1568,4 +1582,4 @@ def diff( raise ValueError("other must be None, Diffable.INDEX, a Tree or Commit, was %r" % other) # Diff against working copy - can be handled by superclass natively. - return super().diff(other, paths, create_patch, **kwargs) + return super().diff(other, paths, create_patch, allow_unsafe_options=allow_unsafe_options, **kwargs) diff --git a/test/test_diff.py b/test/test_diff.py index 612fbd9e0..a395f4a44 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -14,6 +14,7 @@ from git import NULL_TREE, Diff, DiffIndex, Diffable, GitCommandError, Repo, Submodule from git.cmd import Git +from git.exc import UnsafeOptionError from test.lib import StringProcessAdapter, TestBase, fixture, with_rw_directory @@ -352,6 +353,25 @@ def test_diff_submodule(self): self.assertIsInstance(diff.a_blob.size, int) self.assertIsInstance(diff.b_blob.size, int) + def test_diff_rejects_unsafe_output_options(self): + commit = self.rorepo.head.commit + + calls = ( + lambda target: commit.diff(output=target), + lambda target: commit.diff(other=f"--output={target}"), + lambda target: self.rorepo.index.diff(NULL_TREE, output=target), + lambda target: self.rorepo.index.diff(f"--output={target}"), + ) + for index, call in enumerate(calls): + target = osp.join(self.repo_dir, f"diff-output-{index}") + with self.assertRaises(UnsafeOptionError): + call(target) + self.assertFalse(osp.exists(target)) + + allowed_target = osp.join(self.repo_dir, "allowed-diff-output") + commit.diff(output=allowed_target, allow_unsafe_options=True) + self.assertTrue(osp.isfile(allowed_target)) + def test_diff_interface(self): """Test a few variations of the main diff routine.""" assertion_map = {}