Skip to content
Draft
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
18 changes: 16 additions & 2 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,12 @@ class Git(metaclass=_GitMeta):
"--upload-pack",
]

unsafe_git_pathspec_from_file_options = [
# Reads pathspecs from a caller-controlled file. Some commands include an
# unmatched pathspec in their error output, which can disclose the file.
"--pathspec-from-file",
]

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

Expand Down Expand Up @@ -1044,13 +1050,21 @@ def _option_candidates(cls, args: Sequence[Any] = (), kwargs: Optional[Mapping[s
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:
if len(key) != 1:
options.append(f"--{dashify(key)}")
elif split_single_char_options:
options.append(f"-{key}")
options.extend(
str(value)
for value in values
if value is not True and value not in (False, None) and str(value).startswith("-")
)
else:
options.extend(
f"-{key}" if value is True else f"-{key}{value}"
for value in values
if value is True or (value is not False and value is not None)
)
return options

AutoInterrupt: TypeAlias = _AutoInterrupt
Expand Down
5 changes: 5 additions & 0 deletions git/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@
UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]")
"""Characters that cannot be safely written in config names or values."""

VALID_CONFIG_OPTION_NAME_RE = re.compile(r"^[A-Za-z0-9_.-]+$")
"""Pattern for option names that can be written without changing config syntax."""


class MetaParserBuilder(abc.ABCMeta): # noqa: B024
"""Utility class wrapping base-class methods into decorators that assure read-only
Expand Down Expand Up @@ -897,6 +900,8 @@ def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> s
def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None:
if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name):
raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label)
if label == "option" and isinstance(name, str) and not VALID_CONFIG_OPTION_NAME_RE.fullmatch(name):
raise ValueError("Git config option names may contain only letters, digits, '-', '_', or '.'")
if label == "section" and isinstance(name, str):
in_quotes = False
escaped = False
Expand Down
50 changes: 47 additions & 3 deletions git/index/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class IndexFile(LazyMixin, git_diff.Diffable, Serializable):
"""

unsafe_git_checkout_index_options = ["--prefix"]
unsafe_git_read_tree_options = ["--index-output"]

__slots__ = ("repo", "version", "entries", "_extension_data", "_file_path")

Expand Down Expand Up @@ -256,7 +257,12 @@ def write(

@post_clear_cache
@default_index
def merge_tree(self, rhs: Treeish, base: Union[None, Treeish] = None) -> "IndexFile":
def merge_tree(
self,
rhs: Treeish,
base: Union[None, Treeish] = None,
allow_unsafe_options: bool = False,
) -> "IndexFile":
"""Merge the given `rhs` treeish into the current index, possibly taking
a common base treeish into account.

Expand All @@ -270,6 +276,9 @@ def merge_tree(self, rhs: Treeish, base: Union[None, Treeish] = None) -> "IndexF
Optional treeish reference pointing to the common base of `rhs` and this
index which equals lhs.

:param allow_unsafe_options:
Allow options that may write to arbitrary paths.

:return:
self (containing the merge and possibly unmerged entries in case of
conflicts)
Expand All @@ -280,6 +289,12 @@ def merge_tree(self, rhs: Treeish, base: Union[None, Treeish] = None) -> "IndexF
yourself, you have to commit the changed index (or make a valid tree from
it) and retry with a three-way :meth:`index.from_tree <from_tree>` call.
"""
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([base, rhs]),
unsafe_options=self.unsafe_git_read_tree_options,
)

# -i : ignore working tree status
# --aggressive : handle more merge cases
# -m : do an actual merge
Expand Down Expand Up @@ -324,7 +339,13 @@ def new(cls, repo: "Repo", *tree_sha: Union[str, Tree]) -> "IndexFile":
return inst

@classmethod
def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile":
def from_tree(
cls,
repo: "Repo",
*treeish: Treeish,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> "IndexFile":
R"""Merge the given treeish revisions into a new index which is returned.
The original index will remain unaltered.

Expand All @@ -348,6 +369,9 @@ def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile
:param kwargs:
Additional arguments passed to :manpage:`git-read-tree(1)`.

:param allow_unsafe_options:
Allow options that may write to arbitrary paths.

:return:
New :class:`IndexFile` instance. It will point to a temporary index location
which does not exist anymore. If you intend to write such a merged Index,
Expand All @@ -365,6 +389,12 @@ def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile
if len(treeish) == 0 or len(treeish) > 3:
raise ValueError("Please specify between 1 and 3 treeish, got %i" % len(treeish))

if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates(treeish, kwargs),
unsafe_options=cls.unsafe_git_read_tree_options,
)

arg_list: List[Union[Treeish, str]] = []
# Ignore that the working tree and index possibly are out of date.
if len(treeish) > 1:
Expand Down Expand Up @@ -992,6 +1022,7 @@ def remove(
self,
items: Union[PathLike, Sequence[Union[PathLike, Blob, BaseIndexEntry, "Submodule"]]],
working_tree: bool = False,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> List[str]:
R"""Remove the given items from the index and optionally from the working tree
Expand Down Expand Up @@ -1022,6 +1053,10 @@ def remove(
physically removing the respective file. This may fail if there are
uncommitted changes in it.

:param allow_unsafe_options:
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
:manpage:`git-rm(1)`.

:param kwargs:
Additional keyword arguments to be passed to :manpage:`git-rm(1)`, such as
``r`` to allow recursive removal.
Expand All @@ -1033,6 +1068,11 @@ def remove(
This is interesting to know in case you have provided a directory or globs.
Paths are relative to the repository.
"""
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
)
args = []
if not working_tree:
args.append("--cached")
Expand Down Expand Up @@ -1414,6 +1454,7 @@ def reset(
working_tree: bool = False,
paths: Union[None, Iterable[PathLike]] = None,
head: bool = False,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> "IndexFile":
"""Reset the index to reflect the tree at the given commit. This will not adjust
Expand Down Expand Up @@ -1445,6 +1486,9 @@ def reset(
The paths need to exist at the commit, otherwise an exception will be
raised.

:param allow_unsafe_options:
Allow options that may write to arbitrary paths.

:param kwargs:
Additional keyword arguments passed to :manpage:`git-reset(1)`.

Expand All @@ -1461,7 +1505,7 @@ def reset(
"""
# What we actually want to do is to merge the tree into our existing index,
# which is what git-read-tree does.
new_inst = type(self).from_tree(self.repo, commit)
new_inst = type(self).from_tree(self.repo, commit, allow_unsafe_options=allow_unsafe_options)
if not paths:
self.entries = new_inst.entries
else:
Expand Down
27 changes: 26 additions & 1 deletion git/refs/head.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from typing import Any, Sequence, TYPE_CHECKING, Union

from git.cmd import Git
from git.types import Commit_ish, PathLike

if TYPE_CHECKING:
Expand Down Expand Up @@ -62,6 +63,7 @@ def reset(
index: bool = True,
working_tree: bool = False,
paths: Union[PathLike, Sequence[PathLike], None] = None,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> "HEAD":
"""Reset our HEAD to the given commit optionally synchronizing the index and
Expand All @@ -84,12 +86,21 @@ def reset(
Single path or list of paths relative to the git root directory
that are to be reset. This allows to partially reset individual files.

:param allow_unsafe_options:
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
:manpage:`git-reset(1)`.

:param kwargs:
Additional arguments passed to :manpage:`git-reset(1)`.

:return:
self
"""
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([commit], kwargs),
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
)
mode: Union[str, None]
mode = "--soft"
if index:
Expand Down Expand Up @@ -234,7 +245,12 @@ def rename(self, new_path: PathLike, force: bool = False) -> "Head":
self.path = "%s/%s" % (self._common_path_default, new_path)
return self

def checkout(self, force: bool = False, **kwargs: Any) -> Union["HEAD", "Head"]:
def checkout(
self,
force: bool = False,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> Union["HEAD", "Head"]:
"""Check out this head by setting the HEAD to this reference, by updating the
index to reflect the tree we point to and by updating the working tree to
reflect the latest index.
Expand All @@ -246,6 +262,10 @@ def checkout(self, force: bool = False, **kwargs: Any) -> Union["HEAD", "Head"]:
If ``False``, :exc:`~git.exc.GitCommandError` will be raised in that
situation.

:param allow_unsafe_options:
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
:manpage:`git-checkout(1)`.

:param kwargs:
Additional keyword arguments to be passed to git checkout, e.g.
``b="new_branch"`` to create a new branch at the given spot.
Expand All @@ -261,6 +281,11 @@ def checkout(self, force: bool = False, **kwargs: Any) -> Union["HEAD", "Head"]:
the HEAD detached which is allowed and possible, but remains a special state
that some tools might not be able to handle.
"""
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
)
kwargs["f"] = force
if kwargs["f"] is False:
kwargs.pop("f")
Expand Down
18 changes: 18 additions & 0 deletions git/repo/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ class Repo:
re_author_committer_start = re.compile(r"^(author|committer)")
re_tab_full_line = re.compile(r"^\t(.*)$")

unsafe_git_init_options = [
# Can install hooks that execute during later Git commands:
"--template",
# Redirects the repository metadata to a caller-controlled path:
"--separate-git-dir",
]
"""Options to :manpage:`git-init(1)` that permit unsafe code execution or I/O."""

unsafe_git_clone_options = [
# Executes arbitrary commands:
"--upload-pack",
Expand Down Expand Up @@ -1394,6 +1402,7 @@ def init(
mkdir: bool = True,
odbt: Type[GitCmdObjectDB] = GitCmdObjectDB,
expand_vars: bool = True,
allow_unsafe_options: bool = False,
**kwargs: Any,
) -> "Repo":
"""Initialize a git repository at the given path if specified.
Expand All @@ -1418,13 +1427,22 @@ def init(
information disclosure, allowing attackers to access the contents of
environment variables.

:param allow_unsafe_options:
Allow unsafe options to be used, such as ``--template`` and
``--separate-git-dir``.

:param kwargs:
Keyword arguments serving as additional options to the
:manpage:`git-init(1)` command.

:return:
:class:`Repo` (the newly created repo)
"""
if not allow_unsafe_options:
Git.check_unsafe_options(
options=Git._option_candidates([], kwargs),
unsafe_options=cls.unsafe_git_init_options,
)
if path:
path = expand_path(path, expand_vars)
if mkdir and path and not osp.exists(path):
Expand Down
37 changes: 37 additions & 0 deletions test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,43 @@ def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir):
self.assertEqual(git_config.get_value("user", "name"), "safe")
self.assertFalse(git_config.has_section("core"))

@with_rw_directory
def test_writer_rejects_invalid_option_names(self, rw_dir):
config_path = osp.join(rw_dir, "config")
bad_options = (
"name=value",
"name#comment",
"name;comment",
"name with space",
"name\twith-tab",
"name[section",
"name]section",
"name:colon",
'name"quote',
"name\\escape",
)

with GitConfigParser(config_path, read_only=False) as git_config:
git_config.add_section("user")
for bad_option in bad_options:
with pytest.raises(ValueError, match="option name"):
git_config.set("user", bad_option, "unsafe")
with pytest.raises(ValueError, match="option name"):
git_config.set_value("user", bad_option, "unsafe")
with pytest.raises(ValueError, match="option name"):
git_config.add_value("user", bad_option, "unsafe")

git_config.set_value("user", "safe-option1", "safe")
git_config.set_value("user", "safe_option2", "safe")
git_config.set_value("user", "3safe_option", "safe")
git_config.set_value("user", "safe.option3", "safe")

with GitConfigParser(config_path, read_only=True) as git_config:
self.assertEqual(git_config.get_value("user", "safe-option1"), "safe")
self.assertEqual(git_config.get_value("user", "safe_option2"), "safe")
self.assertEqual(git_config.get_value("user", "3safe_option"), "safe")
self.assertEqual(git_config.get_value("user", "safe.option3"), "safe")

@with_rw_directory
def test_writer_rejects_unquoted_section_terminators(self, rw_dir):
config_path = osp.join(rw_dir, "config")
Expand Down
22 changes: 21 additions & 1 deletion test/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,18 @@ def test_option_candidates_ignore_untransformed_kwargs(self):

self.assertEqual(options, ["--max-count"])

def test_option_candidates_include_falsey_non_boolean_values(self):
kwargs = {"pathspec_from_file": 0}
candidates = Git._option_candidates(kwargs=kwargs)

self.assertEqual(candidates, ["--pathspec-from-file"])
self.assertEqual(self.git.transform_kwargs(**kwargs), ["--pathspec-from-file=0"])
with self.assertRaises(UnsafeOptionError):
Git.check_unsafe_options(
options=candidates,
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
)

def test_option_candidates_include_split_single_char_option_values(self):
cases = [
({"n": "--upload-pack=helper"}, ["-n", "--upload-pack=helper"], ["--upload-pack"]),
Expand All @@ -230,7 +242,15 @@ def test_option_candidates_include_split_single_char_option_values(self):

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"])
self.assertEqual(Git._option_candidates(kwargs=unsplit_kwargs), ["-n--upload-pack=helper"])

def test_option_candidates_include_joined_single_char_option_values(self):
kwargs = {"n": "uhelper", "split_single_char_options": False}
candidates = Git._option_candidates(kwargs=kwargs)

self.assertEqual(candidates, ["-nuhelper"])
with self.assertRaises(UnsafeOptionError):
Git.check_unsafe_options(options=candidates, unsafe_options=["-u"])

_shell_cases = (
# value_in_call, value_from_class, expected_popen_arg
Expand Down
Loading
Loading