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/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/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): 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 = {} 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),