Block unsafe underscored git kwargs / Fix for GHSA-rpm5-65cw-6hj4

[WesR]

No description provided.

[Copilot]

Pull request overview

This PR addresses GHSA-rpm5-65cw-6hj4 by ensuring unsafe git options are blocked even when supplied via underscored kwarg names (e.g., upload_pack), and expands test coverage to prevent regressions.

Changes:

• Canonicalize option/kwarg names (strip -/--, drop values, convert _ → -) before performing unsafe-option checks.
• Extend remote/clone tests to include underscored unsafe kwargs.
• Add a focused unit test ensuring check_unsafe_options catches multiple normalized forms.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
git/cmd.py	Adds option-name canonicalization and uses it to match unsafe options robustly across kwarg/CLI forms.
test/test_remote.py	Expands fetch/pull/push unsafe option tests to include underscored kwargs.
test/test_git.py	Adds direct unit tests for Git.check_unsafe_options normalization behavior.
test/test_clone.py	Expands clone/clone_from unsafe option tests to include underscored kwargs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

_canonicalize_option_name will raise IndexError for inputs that become an empty string after stripping (e.g. "", "-", or "--") because .split(None, 1)[0] assumes at least one token. This can crash callers that pass option separators like -- via multi_options, or any user-supplied kwargs with an empty-string key. Consider guarding for empty/whitespace-only options (e.g., return "" early when the split yields no tokens) before calling dashify.

Suggested change

	option_name = option.lstrip("-").split("=", 1)[0].split(None, 1)[0]
	return dashify(option_name)
	option_name = option.lstrip("-").split("=", 1)[0]
	option_tokens = option_name.split(None, 1)
	if not option_tokens:
	return ""
	return dashify(option_tokens[0])

[Copilot]

test_push_unsafe_options currently passes a single kwargs dict containing both receive-pack and receive_pack. If only receive-pack is blocked but receive_pack is not, the test would still pass because the first unsafe kwarg triggers the exception. To actually verify the underscored form is blocked, split these into separate cases (like the fetch/pull tests) or parameterize so each unsafe key is tested in isolation.

Suggested change

	{
	"receive-pack": f"touch {tmp_file}",
	"receive_pack": f"touch {tmp_file}",
	"exec": f"touch {tmp_file}",
	}
	{"receive-pack": f"touch {tmp_file}"},
	{"receive_pack": f"touch {tmp_file}"},
	{"exec": f"touch {tmp_file}"},

[Byron]

Thanks a lot!

This looks good enough to me, but let's wait for Copilot.

[Byron]

Could this doc comment be more specific and show what it does to certain inputs? Because I do not get it when looking at the code. And it feels like it can fail on strange (but still valid) input.

[Byron]

Let's address the concerns stated in the auto review.