fix: block unsafe long-option prefixes (GHSA-2f96-g7mh-g2hx) [ruff-format]

Author: codex

git/cmd.py

@@ -960,18 +960,89 @@ def _canonicalize_option_name(cls, option: str) -> str:
         return dashify(option_tokens[0])
 
     @classmethod
-    def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None:
+    def _option_token(cls, option: str) -> str:
+        option_name = option.split("=", 1)[0]
+        option_tokens = option_name.split(None, 1)
+        if not option_tokens:
+            return ""
+        return option_tokens[0]
+
+    @classmethod
+    def _is_long_option(cls, option: str, bare_options_are_long: bool = True) -> bool:
+        """Return whether an option is a long option before canonicalization."""
+        option_token = cls._option_token(option)
+        if not option_token:
+            return False
+
+        if option_token.startswith("--"):
+            return True
+        if option_token.startswith("-"):
+            return False
+        return bare_options_are_long and len(option_token) > 1
+
+    @classmethod
+    def _matches_unsafe_short_option(
+        cls, option: str, canonical_unsafe_option: str, unsafe_option_is_long: bool
+    ) -> bool:
+        """Return whether a single-dash option matches an unsafe short option."""
+        if unsafe_option_is_long or len(canonical_unsafe_option) != 1:
+            return False
+
+        option_token = cls._option_token(option)
+        if not option_token:
+            return False
+
+        if not option_token.startswith("-") or option_token.startswith("--"):
+            return False
+
+        clusterable_flags = frozenset("46flnqsv")
+        for option_char in option_token[1:]:
+            if option_char == canonical_unsafe_option:
+                return True
+            if option_char not in clusterable_flags:
+                return False
+        return False
+
+    @classmethod
+    def check_unsafe_options(
+        cls, options: List[str], unsafe_options: List[str], bare_options_are_long: bool = True
+    ) -> 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.
         """
         # Options can be of the form `foo`, `--foo`, `--foo bar`, or `--foo=bar`.
-        canonical_unsafe_options = {cls._canonicalize_option_name(option): option for option in unsafe_options}
+        # We also treat long-option prefix forms as unsafe, matching Git's option
+        # parser behavior for long options.
+        canonical_unsafe_options = [
+            (cls._canonicalize_option_name(option), option, cls._is_long_option(option)) for option in unsafe_options
+        ]
         for option in options:
-            unsafe_option = canonical_unsafe_options.get(cls._canonicalize_option_name(option))
-            if unsafe_option is not None:
-                raise UnsafeOptionError(f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it.")
+            option_token = cls._option_token(option)
+            if not bare_options_are_long and not option_token.startswith("-"):
+                continue
+            canonical_option = cls._canonicalize_option_name(option)
+            if not canonical_option:
+                continue
+            option_is_long = cls._is_long_option(option, bare_options_are_long=bare_options_are_long)
+            for (
+                canonical_unsafe_option,
+                unsafe_option,
+                unsafe_option_is_long,
+            ) in canonical_unsafe_options:
+                if (
+                    canonical_option == canonical_unsafe_option
+                    or (
+                        option_is_long
+                        and unsafe_option_is_long
+                        and canonical_unsafe_option.startswith(canonical_option)
+                    )
+                    or cls._matches_unsafe_short_option(option, canonical_unsafe_option, unsafe_option_is_long)
+                ):
+                    raise UnsafeOptionError(
+                        f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it."
+                    )
 
     AutoInterrupt: TypeAlias = _AutoInterrupt
 

git/repo/base.py

@@ -1410,7 +1410,9 @@ def _clone(
         if not allow_unsafe_options:
             Git.check_unsafe_options(options=list(kwargs.keys()), 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)
+            Git.check_unsafe_options(
+                options=multi, unsafe_options=cls.unsafe_git_clone_options, bare_options_are_long=False
+            )
 
         proc = git.clone(
             multi,

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):
@@ -129,6 +134,7 @@ def test_clone_unsafe_options(self, rw_repo):
             unsafe_options = [
                 {"upload-pack": f"touch {tmp_file}"},
                 {"upload_pack": f"touch {tmp_file}"},
+                {"upload_p": f"touch {tmp_file}"},
                 {"u": f"touch {tmp_file}"},
                 {"config": "protocol.ext.allow=always"},
                 {"c": "protocol.ext.allow=always"},
@@ -191,7 +197,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
@@ -207,8 +215,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):
@@ -218,6 +231,7 @@ def test_clone_from_unsafe_options(self, rw_repo):
             unsafe_options = [
                 {"upload-pack": f"touch {tmp_file}"},
                 {"upload_pack": f"touch {tmp_file}"},
+                {"upload_p": f"touch {tmp_file}"},
                 {"u": f"touch {tmp_file}"},
                 {"config": "protocol.ext.allow=always"},
                 {"c": "protocol.ext.allow=always"},

test/test_git.py

@@ -158,10 +158,21 @@ def test_it_transforms_kwargs_into_git_command_arguments(self):
     def test_check_unsafe_options_normalizes_kwargs(self):
         cases = [
             (["upload_pack"], ["--upload-pack"]),
+            (["upload_p"], ["--upload-pack"]),
+            (["upload_pac"], ["--upload-pack"]),
             (["receive_pack"], ["--receive-pack"]),
+            (["receive_p"], ["--receive-pack"]),
             (["exec"], ["--exec"]),
+            (["exe"], ["--exec"]),
             (["u"], ["-u"]),
+            (["-utouch /tmp/pwn"], ["-u"]),
+            (["-futouch /tmp/pwn"], ["-u"]),
+            (["-qutouch${IFS}/tmp/pwn"], ["-u"]),
             (["c"], ["-c"]),
+            (["-cprotocol.ext.allow=always"], ["-c"]),
+            (["-vcprotocol.ext.allow=always"], ["-c"]),
+            (["conf"], ["--config"]),
+            (["confi"], ["--config"]),
             (["--upload-pack=/tmp/helper"], ["--upload-pack"]),
             (["--config core.filemode=false"], ["--config"]),
         ]
@@ -170,6 +181,35 @@ 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_keeps_short_options_exact(self):
+        cases = [
+            (["u"], ["--upload-pack"]),
+            (["-u helper"], ["--upload-pack"]),
+            (["c"], ["--config"]),
+            (["-c protocol.ext.allow=always"], ["--config"]),
+            (["e"], ["--exec"]),
+            (["r"], ["--receive-pack"]),
+        ]
+
+        for options, unsafe_options in cases:
+            Git.check_unsafe_options(options=options, unsafe_options=unsafe_options)
+
+    def test_check_unsafe_options_ignores_multi_option_values(self):
+        Git.check_unsafe_options(
+            options=["--origin", "upload"],
+            unsafe_options=["--upload-pack", "--config"],
+            bare_options_are_long=False,
+        )
+
+    def test_check_unsafe_options_allows_attached_safe_short_option_values(self):
+        cases = [
+            ["-oupstream"],
+            ["-bcurrent"],
+        ]
+
+        for options in cases:
+            Git.check_unsafe_options(options=options, unsafe_options=["--upload-pack", "-u", "--config", "-c"])
+
     _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()