fix: reject joined unsafe short options

GitPython blocks options such as --upload-pack/-u and --config/-c
because they can execute arbitrary commands unless unsafe options are
explicitly allowed. Git also accepts a short option with its value
joined to the same token, including after clusterable flags. Forms
such as -uVALUE, -fuVALUE, -cVALUE, and -vcVALUE could therefore
bypass an exact-token check.

Parse single-dash option tokens sufficiently to recognize blocked
short options while preserving safe attached values such as
-oupstream. Also distinguish clone multi-option values from bare
kwargs so positional values are not treated as long-option
abbreviations.

This completes the option-validation hardening for
GHSA-2f96-g7mh-g2hx.

Author: codex

git/cmd.py

@@ -974,6 +974,12 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
         # 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
+        }
+        clusterable_short_options = frozenset("46flnqsv")
         options_are_kwargs = all(not option.startswith("-") for option in options)
         for option in options:
             candidate = cls._canonicalize_option_name(option)
@@ -982,6 +988,16 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
             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():

test/test_clone.py

@@ -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):
@@ -216,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
@@ -232,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):

test/test_git.py

@@ -180,6 +180,24 @@ def test_check_unsafe_options_does_not_treat_short_options_as_abbreviations(self
     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),

test/test_remote.py

@@ -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)
@@ -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)
@@ -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()