Skip to content

Commit def75fd

Browse files
codexByron
authored andcommitted
Address review feedback about unsafe option abbreviations
The prefix check could reject otherwise allowed one-letter short options when an unrelated blocked long option began with the same character. Keep exact matches for every spelling, but restrict prefix matching to explicit long options and multi-character kwargs. This addresses the review comment: single-letter short options and kwargs must not be treated as long-option abbreviations. It also adds coverage for abbreviated unsafe kwargs so both clone validation paths remain protected.
1 parent 2ab0516 commit def75fd

3 files changed

Lines changed: 21 additions & 1 deletion

File tree

git/cmd.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,12 +970,19 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
970970
# Git accepts any unambiguous prefix of a long option, so an abbreviated
971971
# spelling such as `--upl` for `--upload-pack` must be rejected too. An
972972
# option is unsafe if its canonical name is a prefix of any blocked
973-
# option's canonical name.
973+
# option's canonical name. Only long options and multi-character kwargs
974+
# can be abbreviations; single-character short options remain exact-match
975+
# only.
974976
canonical_unsafe_options = {cls._canonicalize_option_name(option): option for option in unsafe_options}
975977
for option in options:
976978
candidate = cls._canonicalize_option_name(option)
977979
if not candidate:
978980
continue
981+
unsafe_option = canonical_unsafe_options.get(candidate)
982+
if unsafe_option is not None:
983+
raise UnsafeOptionError(f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it.")
984+
if not (option.startswith("--") or (not option.startswith("-") and len(candidate) > 1)):
985+
continue
979986
for canonical, unsafe_option in canonical_unsafe_options.items():
980987
if canonical.startswith(candidate):
981988
raise UnsafeOptionError(

test/test_clone.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ def test_clone_unsafe_options(self, rw_repo):
126126
rw_repo.clone(tmp_dir, multi_options=[unsafe_option])
127127
assert not tmp_file.exists()
128128

129+
unsafe_options = [
130+
{"upl": f"touch {tmp_file}"},
131+
{"upload_pac": f"touch {tmp_file}"},
132+
{"conf": "protocol.ext.allow=always"},
133+
]
134+
for unsafe_option in unsafe_options:
135+
with self.assertRaises(UnsafeOptionError):
136+
rw_repo.clone(tmp_dir, **unsafe_option)
137+
assert not tmp_file.exists()
138+
129139
unsafe_options = [
130140
{"upload-pack": f"touch {tmp_file}"},
131141
{"upload_pack": f"touch {tmp_file}"},

test/test_git.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,9 @@ def test_check_unsafe_options_normalizes_kwargs(self):
170170
with self.assertRaises(UnsafeOptionError):
171171
Git.check_unsafe_options(options=options, unsafe_options=unsafe_options)
172172

173+
def test_check_unsafe_options_does_not_treat_short_options_as_abbreviations(self):
174+
Git.check_unsafe_options(options=["-u", "u"], unsafe_options=["--upload-pack"])
175+
173176
_shell_cases = (
174177
# value_in_call, value_from_class, expected_popen_arg
175178
(None, False, False),

0 commit comments

Comments
 (0)