Commit e8d0fbf
fix: validate split short-option values
<!-- agent -->
Single-character keyword arguments are transformed into an option token and
a separate value token. The unsafe-option candidate builder only checked the
keyword name, allowing an option-like value to bypass guards shared by clone,
remote, revision, blame, and archive operations.
Include dash-prefixed values only when short options are actually split,
including sequence values, while preserving bare values and the non-splitting
compatibility path.
Git baseline a23bace9 defines clone -n and --upload-pack as distinct options,
matching the argv boundary this validation now preserves.
Refs GHSA-r9mr-m37c-5fr3.
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>1 parent faf3c09 commit e8d0fbf
2 files changed
Lines changed: 24 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1039,11 +1039,18 @@ def _option_candidates(cls, args: Sequence[Any] = (), kwargs: Optional[Mapping[s | |||
| 1039 | 1039 | option for option in cls._unpack_args([arg for arg in args if arg is not None]) if option.startswith("-") | |
| 1040 | 1040 | ] | |
| 1041 | 1041 | if kwargs: | |
| 1042 | + split_single_char_options = kwargs.get("split_single_char_options", True) | ||
| 1042 | 1043 | for key, value in kwargs.items(): | |
| 1043 | 1044 | values = value if isinstance(value, (list, tuple)) else (value,) | |
| 1044 | 1045 | if any(value is True or (value is not False and value is not None) for value in values): | |
| 1045 | 1046 | key = str(key) | |
| 1046 | 1047 | options.append(f"-{key}" if len(key) == 1 else f"--{dashify(key)}") | |
| 1048 | + if len(key) == 1 and split_single_char_options: | ||
| 1049 | + options.extend( | ||
| 1050 | + str(value) | ||
| 1051 | + for value in values | ||
| 1052 | + if value is not True and value not in (False, None) and str(value).startswith("-") | ||
| 1053 | + ) | ||
| 1047 | 1054 | return options | |
| 1048 | 1055 | ||
| 1049 | 1056 | AutoInterrupt: TypeAlias = _AutoInterrupt | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -214,6 +214,23 @@ def test_option_candidates_ignore_untransformed_kwargs(self): | |||
| 214 | 214 | ||
| 215 | 215 | self.assertEqual(options, ["--max-count"]) | |
| 216 | 216 | ||
| 217 | + def test_option_candidates_include_split_single_char_option_values(self): | ||
| 218 | + cases = [ | ||
| 219 | + ({"n": "--upload-pack=helper"}, ["-n", "--upload-pack=helper"], ["--upload-pack"]), | ||
| 220 | + ({"g": ("safe", "--out=target")}, ["-g", "--out=target"], ["--output"]), | ||
| 221 | + ] | ||
| 222 | + | ||
| 223 | + for kwargs, candidates, unsafe_options in cases: | ||
| 224 | + self.assertEqual(Git._option_candidates(kwargs=kwargs), candidates) | ||
| 225 | + with self.assertRaises(UnsafeOptionError): | ||
| 226 | + Git.check_unsafe_options(options=candidates, unsafe_options=unsafe_options) | ||
| 227 | + | ||
| 228 | + self.assertEqual(self.git.transform_kwargs(n="--upload-pack=helper"), ["-n", "--upload-pack=helper"]) | ||
| 229 | + | ||
| 230 | + unsplit_kwargs = {"n": "--upload-pack=helper", "split_single_char_options": False} | ||
| 231 | + self.assertEqual(self.git.transform_kwargs(**unsplit_kwargs), ["-n--upload-pack=helper"]) | ||
| 232 | + self.assertEqual(Git._option_candidates(kwargs=unsplit_kwargs), ["-n"]) | ||
| 233 | + | ||
| 217 | 234 | _shell_cases = ( | |
| 218 | 235 | # value_in_call, value_from_class, expected_popen_arg | |
| 219 | 236 | (None, False, False), | |
0 commit comments