Visitar URL original
Reject abbreviated forms of unsafe git options by Byron · Pull Request #2168 · gitpython-developers/GitPython · GitHub
Skip to content

Reject abbreviated forms of unsafe git options #2168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Byron merged 3 commits into from
Jul 12, 2026
+131 −7
Merged

Reject abbreviated forms of unsafe git options #2168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ab0516
Reject abbreviated forms of unsafe git options
abhiprd2000 Jul 9, 2026
181e8ed
Address review feedback about unsafe option abbreviations
codex Jul 12, 2026
5680608
fix: reject joined unsafe short options
codex Jul 12, 2026
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
59 changes: 54 additions & 5 deletions git/cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,17 +961,66 @@ def _canonicalize_option_name(cls, option: str) -> str:

@classmethod
def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None:
"""Check for unsafe options.

Some options that are passed to ``git <command>`` can be used to execute
arbitrary commands. These are blocked by default.
"""Raise :class:`~git.exc.UnsafeOptionError` for blocked option spellings.

In addition to exact matches, this rejects abbreviated long options accepted
by Git (for example, ``--upl`` for ``--upload-pack``) and unsafe short options
whose values are joined to the same token, including after clusterable flags
(for example, ``-uVALUE`` and ``-fuVALUE``).

A list containing only bare names is treated as normalized keyword arguments,
so multi-character names such as ``upload_p`` are checked as long-option
abbreviations. If any item starts with ``-``, the list is treated as tokenized
command-line input: bare items can be option values and are not checked as
abbreviations. Thus ``["--origin", "upload"]`` is allowed. Single-dash options
use short-option parsing rather than broad prefix matching, preserving safe
attached values such as ``-oupstream`` and ``-bcurrent``.

Some options passed to ``git <command>`` can execute arbitrary commands and
are therefore blocked by default unless the caller explicitly allows them.
"""
# Options can be of the form `foo`, `--foo`, `--foo bar`, or `--foo=bar`.
# Git accepts any unambiguous prefix of a long option, so an abbreviated
# spelling such as `--upl` for `--upload-pack` must be rejected too. An
# option is unsafe if its canonical name is a prefix of any blocked
# option's canonical name. Only long options and multi-character kwargs
# can be abbreviations; single-character short options remain exact-match
# only.
canonical_unsafe_options = {cls._canonicalize_option_name(option): option for option in unsafe_options}
unsafe_short_options = {
canonical: option
for canonical, option in canonical_unsafe_options.items()
if option.startswith("-") and not option.startswith("--") and len(canonical) == 1
}
# These value-less Git flags can be clustered before another short option
# (for example, ``-fuVALUE``). Stop at any other character because it may
# begin an attached value, as ``o`` does in the safe option ``-oupstream``.
clusterable_short_options = frozenset("46flnqsv")
options_are_kwargs = all(not option.startswith("-") for option in options)
for option in options:
unsafe_option = canonical_unsafe_options.get(cls._canonicalize_option_name(option))
candidate = cls._canonicalize_option_name(option)
if not candidate:
continue
unsafe_option = canonical_unsafe_options.get(candidate)
if unsafe_option is not None:
raise UnsafeOptionError(f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it.")
option_token = option.split("=", 1)[0].split(None, 1)[0]
if option_token.startswith("-") and not option_token.startswith("--"):
for option_char in option_token[1:]:
unsafe_option = unsafe_short_options.get(option_char)
if unsafe_option is not None:
raise UnsafeOptionError(
f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it."
)
if option_char not in clusterable_short_options:
break
if not (option.startswith("--") or (options_are_kwargs and len(candidate) > 1)):
continue
for canonical, unsafe_option in canonical_unsafe_options.items():
if canonical.startswith(candidate):
Comment thread
Byron marked this conversation as resolved.
raise UnsafeOptionError(
f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it."
)
Comment thread
Byron marked this conversation as resolved.

AutoInterrupt: TypeAlias = _AutoInterrupt

Expand Down
37 changes: 37 additions & 0 deletions test/test_clone.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,13 @@ def test_clone_unsafe_options(self, rw_repo):
unsafe_options = [
f"--upload-pack='touch {tmp_file}'",
f"-u 'touch {tmp_file}'",
f"-utouch {tmp_file}; false",
f"-futouch${{IFS}}{tmp_file}; false",
f"-qutouch${{IFS}}{tmp_file}; false",
"--config=protocol.ext.allow=always",
"-c protocol.ext.allow=always",
"-cprotocol.ext.allow=always",
"-vcprotocol.ext.allow=always",
]
for unsafe_option in unsafe_options:
with self.assertRaises(UnsafeOptionError):
Expand All @@ -138,6 +143,31 @@ def test_clone_unsafe_options(self, rw_repo):
rw_repo.clone(tmp_dir, **unsafe_option)
assert not tmp_file.exists()

@with_rw_repo("HEAD")
def test_clone_unsafe_options_abbreviated(self, rw_repo):
with tempfile.TemporaryDirectory() as tdir:
tmp_dir = pathlib.Path(tdir)
tmp_file = tmp_dir / "pwn"
unsafe_options = [
f"--upl='touch {tmp_file}'",
f"--upload-pac='touch {tmp_file}'",
"--conf=protocol.ext.allow=always",
]
for unsafe_option in unsafe_options:
with self.assertRaises(UnsafeOptionError):
rw_repo.clone(tmp_dir, multi_options=[unsafe_option])
assert not tmp_file.exists()
Comment thread
Byron marked this conversation as resolved.

unsafe_kwargs = [
{"upl": f"touch {tmp_file}"},
{"upload_pac": f"touch {tmp_file}"},
{"conf": "protocol.ext.allow=always"},
]
for unsafe_option in unsafe_kwargs:
with self.assertRaises(UnsafeOptionError):
rw_repo.clone(tmp_dir, **unsafe_option)
assert not tmp_file.exists()

@with_rw_repo("HEAD")
def test_clone_unsafe_options_are_checked_after_splitting_multi_options(self, rw_repo):
with tempfile.TemporaryDirectory() as tdir:
Expand Down Expand Up @@ -191,7 +221,9 @@ def test_clone_safe_options(self, rw_repo):
options = [
"--depth=1",
"--single-branch",
"--origin upload",
"-q",
"-oupstream",
]
for option in options:
destination = tmp_dir / option
Expand All @@ -207,8 +239,13 @@ def test_clone_from_unsafe_options(self, rw_repo):
unsafe_options = [
f"--upload-pack='touch {tmp_file}'",
f"-u 'touch {tmp_file}'",
f"-utouch {tmp_file}; false",
f"-futouch${{IFS}}{tmp_file}; false",
f"-qutouch${{IFS}}{tmp_file}; false",
"--config=protocol.ext.allow=always",
"-c protocol.ext.allow=always",
"-cprotocol.ext.allow=always",
"-vcprotocol.ext.allow=always",
]
for unsafe_option in unsafe_options:
with self.assertRaises(UnsafeOptionError):
Expand Down
28 changes: 28 additions & 0 deletions test/test_git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,34 @@ def test_check_unsafe_options_normalizes_kwargs(self):
with self.assertRaises(UnsafeOptionError):
Git.check_unsafe_options(options=options, unsafe_options=unsafe_options)

def test_check_unsafe_options_does_not_treat_short_options_as_abbreviations(self):
unsafe_options = ["--upload-pack"]

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

def test_check_unsafe_options_does_not_treat_option_values_as_abbreviations(self):
Git.check_unsafe_options(options=["--branch", "conf"], unsafe_options=["--config"])

def test_check_unsafe_options_rejects_joined_unsafe_short_options(self):
cases = [
(["-utouch /tmp/pwn"], ["-u"]),
(["-futouch /tmp/pwn"], ["-u"]),
(["-qutouch${IFS}/tmp/pwn"], ["-u"]),
(["-cprotocol.ext.allow=always"], ["-c"]),
(["-vcprotocol.ext.allow=always"], ["-c"]),
]

for options, unsafe_options in cases:
with self.assertRaises(UnsafeOptionError):
Git.check_unsafe_options(options=options, unsafe_options=unsafe_options)

def test_check_unsafe_options_allows_attached_safe_short_option_values(self):
unsafe_options = ["--upload-pack", "-u", "--config", "-c"]

Git.check_unsafe_options(options=["-oupstream", "-bcurrent"], unsafe_options=unsafe_options)

_shell_cases = (
# value_in_call, value_from_class, expected_popen_arg
(None, False, False),
Expand Down
14 changes: 12 additions & 2 deletions test/test_remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,11 @@ def test_fetch_unsafe_options(self, rw_repo):
remote = rw_repo.remote("origin")
tmp_dir = Path(tdir)
tmp_file = tmp_dir / "pwn"
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}]
unsafe_options = [
{"upload-pack": f"touch {tmp_file}"},
{"upload_pack": f"touch {tmp_file}"},
{"upload_p": f"touch {tmp_file}"},
]
for unsafe_option in unsafe_options:
with self.assertRaises(UnsafeOptionError):
remote.fetch(**unsafe_option)
Expand Down Expand Up @@ -900,7 +904,11 @@ def test_pull_unsafe_options(self, rw_repo):
remote = rw_repo.remote("origin")
tmp_dir = Path(tdir)
tmp_file = tmp_dir / "pwn"
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}]
unsafe_options = [
{"upload-pack": f"touch {tmp_file}"},
{"upload_pack": f"touch {tmp_file}"},
{"upload_p": f"touch {tmp_file}"},
]
for unsafe_option in unsafe_options:
with self.assertRaises(UnsafeOptionError):
remote.pull(**unsafe_option)
Expand Down Expand Up @@ -971,7 +979,9 @@ def test_push_unsafe_options(self, rw_repo):
unsafe_options = [
{"receive-pack": f"touch {tmp_file}"},
{"receive_pack": f"touch {tmp_file}"},
{"receive_p": f"touch {tmp_file}"},
{"exec": f"touch {tmp_file}"},
{"exe": f"touch {tmp_file}"},
]
for unsafe_option in unsafe_options:
assert not tmp_file.exists()
Expand Down
Loading