Skip to content
Open
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
33 changes: 32 additions & 1 deletion git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,11 @@ class Git(metaclass=_GitMeta):

re_unsafe_protocol = re.compile(r"(.+)::.+")

unsafe_git_ls_remote_options = [
# This option allows arbitrary command execution in git-ls-remote.
"--upload-pack",
]
Comment thread
Byron marked this conversation as resolved.

def __getstate__(self) -> Dict[str, Any]:
return slots_to_dict(self, exclude=self._excluded_)

Expand Down Expand Up @@ -973,6 +978,16 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
if unsafe_option is not None:
raise UnsafeOptionError(f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it.")

@classmethod
def _option_candidates(cls, args: Sequence[Any] = (), kwargs: Optional[Mapping[str, Any]] = None) -> List[str]:
"""Collect possible option spellings before command-line transformation."""
options = [
option for option in cls._unpack_args([arg for arg in args if arg is not None]) if option.startswith("-")
]
if kwargs:
options.extend(str(key) for key in kwargs)
return options
Comment on lines +982 to +989

AutoInterrupt: TypeAlias = _AutoInterrupt

CatFileContentStream: TypeAlias = _CatFileContentStream
Expand Down Expand Up @@ -1030,6 +1045,22 @@ def set_persistent_git_options(self, **kwargs: Any) -> None:

self._persistent_git_options = self.transform_kwargs(split_single_char_options=True, **kwargs)

def ls_remote(
self,
*args: Any,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], "Git.AutoInterrupt"]:
Comment thread
Byron marked this conversation as resolved.
"""List references in a remote repository.

:param allow_unsafe_options:
Allow unsafe options, like ``--upload-pack``.
"""
if not allow_unsafe_options:
candidate_options = self._option_candidates(args, kwargs)
Git.check_unsafe_options(options=candidate_options, unsafe_options=self.unsafe_git_ls_remote_options)
return self._call_process("ls_remote", *args, **kwargs)
Comment thread
Byron marked this conversation as resolved.
Comment thread
Byron marked this conversation as resolved.
Comment thread
Byron marked this conversation as resolved.

@property
def working_dir(self) -> Union[None, PathLike]:
""":return: Git directory we are working on"""
Expand Down Expand Up @@ -1536,7 +1567,7 @@ def transform_kwargs(self, split_single_char_options: bool = True, **kwargs: Any
return args

@classmethod
def _unpack_args(cls, arg_list: Sequence[str]) -> List[str]:
def _unpack_args(cls, arg_list: Sequence[Any]) -> List[str]:
outlist = []
if isinstance(arg_list, (list, tuple)):
for arg in arg_list:
Expand Down
13 changes: 13 additions & 0 deletions git/objects/commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ class Commit(base.Object, TraversableIterableObj, Diffable, Serializable):
# INVARIANTS
default_encoding = "UTF-8"

unsafe_git_rev_options = [
# This option can truncate or write arbitrary files before revision parsing.
"--output",
"-o",
]
"""Options to :manpage:`git-rev-list(1)` that can overwrite files."""
Comment on lines +89 to +94

type: Literal["commit"] = "commit"

__slots__ = (
Expand Down Expand Up @@ -302,6 +309,7 @@ def iter_items(
repo: "Repo",
rev: Union[str, "Commit", "SymbolicReference"],
paths: Union[PathLike, Sequence[PathLike]] = "",
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> Iterator["Commit"]:
R"""Find all commits matching the given criteria.
Expand Down Expand Up @@ -330,6 +338,11 @@ def iter_items(
raise ValueError("--pretty cannot be used as parsing expects single sha's only")
# END handle pretty

if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([rev], kwargs), unsafe_options=cls.unsafe_git_rev_options
)

# Use -- in all cases, to prevent possibility of ambiguous arguments.
# See https://github.com/gitpython-developers/GitPython/issues/264.

Expand Down
15 changes: 12 additions & 3 deletions git/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,10 @@ def fetch(
Git.check_unsafe_protocols(ref)

if not allow_unsafe_options:
Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_fetch_options)
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=self.unsafe_git_fetch_options,
)

proc = self.repo.git.fetch(
"--", self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=verbose, **kwargs
Expand Down Expand Up @@ -1125,7 +1128,10 @@ def pull(
Git.check_unsafe_protocols(ref)

if not allow_unsafe_options:
Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_pull_options)
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=self.unsafe_git_pull_options,
)

proc = self.repo.git.pull(
"--", self, refspec, with_stdout=False, as_process=True, universal_newlines=True, v=True, **kwargs
Expand Down Expand Up @@ -1198,7 +1204,10 @@ def push(
Git.check_unsafe_protocols(ref)

if not allow_unsafe_options:
Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_push_options)
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=self.unsafe_git_push_options,
)

proc = self.repo.git.push(
"--",
Expand Down
80 changes: 73 additions & 7 deletions git/repo/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,20 @@ class Repo:
https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---configltkeygtltvaluegt
"""

unsafe_git_archive_options = [
# Allows arbitrary command execution through the remote git-upload-archive command.
"--exec",
# Writes output to a caller-controlled filesystem path.
"--output",
"-o",
]

unsafe_git_revision_options = [
# This option allows output to be written to arbitrary files before revision parsing.
"--output",
"-o",
]

# Invariants
config_level: ConfigLevels_Tup = ("system", "user", "global", "repository")
"""Represents the configuration level of a configuration file."""
Expand Down Expand Up @@ -775,6 +789,7 @@ def iter_commits(
self,
rev: Union[str, Commit, "SymbolicReference", None] = None,
paths: Union[PathLike, Sequence[PathLike]] = "",
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> Iterator[Commit]:
Comment thread
Byron marked this conversation as resolved.
"""An iterator of :class:`~git.objects.commit.Commit` objects representing the
Expand All @@ -792,6 +807,9 @@ def iter_commits(
Arguments to be passed to :manpage:`git-rev-list(1)`.
Common ones are ``max_count`` and ``skip``.

:param allow_unsafe_options:
Allow unsafe options in the revision argument, like ``--output``.

:note:
To receive only commits between two named revisions, use the
``"revA...revB"`` revision specifier.
Expand All @@ -802,7 +820,18 @@ def iter_commits(
if rev is None:
rev = self.head.commit

return Commit.iter_items(self, rev, paths, **kwargs)
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([rev], kwargs), unsafe_options=self.unsafe_git_revision_options
)
Comment thread
Byron marked this conversation as resolved.

return Commit.iter_items(
self,
rev,
paths,
allow_unsafe_options=allow_unsafe_options,
**kwargs,
)

def merge_base(self, *rev: TBD, **kwargs: Any) -> List[Commit]:
R"""Find the closest common ancestor for the given revision
Expand Down Expand Up @@ -1079,7 +1108,9 @@ def active_branch(self) -> Head:
)
return active_branch

def blame_incremental(self, rev: str | HEAD | None, file: str, **kwargs: Any) -> Iterator["BlameEntry"]:
def blame_incremental(
self, rev: str | HEAD | None, file: str, allow_unsafe_options: bool = False, **kwargs: Any
) -> Iterator["BlameEntry"]:
Comment thread
Byron marked this conversation as resolved.
"""Iterator for blame information for the given file at the given revision.

Unlike :meth:`blame`, this does not return the actual file's contents, only a
Expand All @@ -1090,6 +1121,9 @@ def blame_incremental(self, rev: str | HEAD | None, file: str, **kwargs: Any) ->
uncommitted changes. Otherwise, anything successfully parsed by
:manpage:`git-rev-parse(1)` is a valid option.

:param allow_unsafe_options:
Allow unsafe options in revision argument, like ``--output``.

:return:
Lazy iterator of :class:`BlameEntry` tuples, where the commit indicates the
commit to blame for the line, and range indicates a span of line numbers in
Expand All @@ -1098,6 +1132,10 @@ def blame_incremental(self, rev: str | HEAD | None, file: str, **kwargs: Any) ->
If you combine all line number ranges outputted by this command, you should get
a continuous range spanning all line numbers in the file.
"""
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([rev], kwargs), unsafe_options=self.unsafe_git_revision_options
)

data: bytes = self.git.blame(rev, "--", file, p=True, incremental=True, stdout_as_string=False, **kwargs)
commits: Dict[bytes, Commit] = {}
Expand Down Expand Up @@ -1176,7 +1214,8 @@ def blame(
rev: Union[str, HEAD, None],
file: str,
incremental: bool = False,
rev_opts: Optional[List[str]] = None,
rev_opts: Optional[Sequence[str]] = None,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> List[List[Commit | List[str | bytes] | None]] | Iterator[BlameEntry] | None:
Comment thread
Byron marked this conversation as resolved.
"""The blame information for the given file at the given revision.
Expand All @@ -1186,6 +1225,9 @@ def blame(
uncommitted changes. Otherwise, anything successfully parsed by
:manpage:`git-rev-parse(1)` is a valid option.

:param allow_unsafe_options:
Allow unsafe options in revision argument, like ``--output``.

:return:
list: [git.Commit, list: [<line>]]

Expand All @@ -1195,9 +1237,14 @@ def blame(
appearance.
"""
if incremental:
return self.blame_incremental(rev, file, **kwargs)
rev_opts = rev_opts or []
data: bytes = self.git.blame(rev, *rev_opts, "--", file, p=True, stdout_as_string=False, **kwargs)
return self.blame_incremental(rev, file, allow_unsafe_options=allow_unsafe_options, **kwargs)
rev_opts_list = list(rev_opts or [])
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([rev, rev_opts_list], kwargs),
unsafe_options=self.unsafe_git_revision_options,
)
Comment thread
Byron marked this conversation as resolved.
data: bytes = self.git.blame(rev, *rev_opts_list, "--", file, p=True, stdout_as_string=False, **kwargs)
commits: Dict[str, Commit] = {}
blames: List[List[Commit | List[str | bytes] | None]] = []

Expand Down Expand Up @@ -1408,7 +1455,10 @@ def _clone(
if not allow_unsafe_protocols:
Git.check_unsafe_protocols(url)
if not allow_unsafe_options:
Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=cls.unsafe_git_clone_options)
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=cls.unsafe_git_clone_options,
)
if not allow_unsafe_options and multi:
Git.check_unsafe_options(options=multi, unsafe_options=cls.unsafe_git_clone_options)

Expand Down Expand Up @@ -1583,6 +1633,8 @@ def archive(
ostream: Union[TextIO, BinaryIO],
treeish: Optional[str] = None,
prefix: Optional[str] = None,
allow_unsafe_options: bool = False,
allow_unsafe_protocols: bool = False,
**kwargs: Any,
) -> Repo:
"""Archive the tree at the given revision.
Expand All @@ -1605,6 +1657,12 @@ def archive(
repository-relative path to a directory or file to place into the archive,
or a list or tuple of multiple paths.

:param allow_unsafe_options:
Allow unsafe options, like ``--exec`` or ``--output``.

:param allow_unsafe_protocols:
Allow unsafe protocols to be used in ``remote``, like ``ext``.

:raise git.exc.GitCommandError:
If something went wrong.

Expand All @@ -1615,6 +1673,14 @@ def archive(
treeish = self.head.commit
if prefix and "prefix" not in kwargs:
kwargs["prefix"] = prefix
remote = kwargs.get("remote")
if not allow_unsafe_protocols and remote:
Git.check_unsafe_protocols(str(remote))
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=self.unsafe_git_archive_options,
)
kwargs["output_stream"] = ostream
path = kwargs.pop("path", [])
path = cast(Union[PathLike, List[PathLike], Tuple[PathLike, ...]], path)
Expand Down
13 changes: 13 additions & 0 deletions test/test_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import copy
from datetime import datetime
from io import BytesIO
import tempfile
import os.path as osp
import re
import sys
Expand All @@ -15,6 +16,7 @@
from gitdb import IStream

from git import Actor, Commit, Repo
from git.exc import UnsafeOptionError
from git.objects.util import tzoffset, utc
from git.repo.fun import touch

Expand Down Expand Up @@ -288,6 +290,17 @@ def test_iter_items(self):
# pretty not allowed.
self.assertRaises(ValueError, Commit.iter_items, self.rorepo, "master", pretty="raw")

def test_iter_items_rejects_unsafe_revision(self):
with tempfile.TemporaryDirectory() as tdir:
marker = osp.join(tdir, "pwn")
self.assertRaises(UnsafeOptionError, Commit.iter_items, self.rorepo, f"--output={marker}")

def test_iter_items_rejects_unsafe_options(self):
with tempfile.TemporaryDirectory() as tdir:
marker = osp.join(tdir, "pwn")
with self.assertRaises(UnsafeOptionError):
list(Commit.iter_items(self.rorepo, "HEAD", output=marker))

def test_rev_list_bisect_all(self):
"""
'git rev-list --bisect-all' returns additional information
Expand Down
33 changes: 32 additions & 1 deletion test/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import random
import sys
import tempfile
from unittest import skipIf
from unittest import mock, skipIf

import pytest

Expand Down Expand Up @@ -1007,6 +1007,37 @@ def test_push_unsafe_options_allowed(self, rw_repo):
assert tmp_file.exists()
tmp_file.unlink()

@with_rw_repo("HEAD")
def test_ls_remote_unsafe_options(self, rw_repo):
with tempfile.TemporaryDirectory() as tdir:
tmp_dir = Path(tdir)
tmp_file = tmp_dir / "pwn"
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}]
for unsafe_option in unsafe_options:
with self.assertRaises(UnsafeOptionError):
rw_repo.git.ls_remote(".", **unsafe_option)
with self.assertRaises(UnsafeOptionError):
rw_repo.git.ls_remote([f"--upload-pack={tmp_file}"], ".")
with self.assertRaises(UnsafeOptionError):
rw_repo.git.ls_remote(f"--upload-pack={tmp_file}", ".")
with self.assertRaises(UnsafeOptionError):
rw_repo.git.ls_remote("--upload-pack", "touch", ".")

def test_ls_remote_allows_operand_named_like_unsafe_option(self):
with mock.patch.object(Git, "_call_process") as git:
Git().ls_remote("upload-pack")
git.assert_called_once()

@with_rw_repo("HEAD")
def test_ls_remote_unsafe_options_allowed(self, rw_repo):
with tempfile.TemporaryDirectory() as tdir:
tmp_dir = Path(tdir)
tmp_file = tmp_dir / "pwn"
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}]
for unsafe_option in unsafe_options:
with self.assertRaises(GitCommandError):
rw_repo.git.ls_remote(".", **unsafe_option, allow_unsafe_options=True)

@with_rw_and_rw_remote_repo("0.1.6")
def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo):
# Create branch with a name containing a NBSP
Expand Down
Loading
Loading