Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions git/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
18 changes: 16 additions & 2 deletions git/index/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand All @@ -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())
Expand Down Expand Up @@ -1560,12 +1568,18 @@ 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.
if other is not None:
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)
5 changes: 5 additions & 0 deletions git/repo/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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 = [
Expand Down
2 changes: 2 additions & 0 deletions test/test_clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down
20 changes: 20 additions & 0 deletions test/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 = {}
Expand Down
17 changes: 17 additions & 0 deletions test/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Loading