Reject abbreviated forms of unsafe git options#2168
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens GitPython’s unsafe-option guard (Git.check_unsafe_options) to also reject abbreviated long options (e.g. --upl for --upload-pack) and adds a clone regression test to cover abbreviated spellings that Git accepts.
Changes:
- Updated
Git.check_unsafe_optionsto treat any unambiguous long-option prefix of a blocked option as unsafe. - Added a clone regression test for abbreviated unsafe options (
--upl,--upload-pac,--conf).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
git/cmd.py |
Extends unsafe-option detection to include long-option prefix abbreviations. |
test/test_clone.py |
Adds regression coverage for abbreviated unsafe clone options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Byron
force-pushed
the
advisory-fix-1
branch
2 times, most recently
from
5ed4f88 to
25c684b
Compare
Byron
force-pushed
the
advisory-fix-1
branch
2 times, most recently
from
0f9720e to
8b53090
Compare
1 task
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. Reviewd-by: Sebastian Thiel <sebastian.thiel@icloud.com>
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. Reviewed-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Member
Author
|
Merging even without all CI green, as the runs that finish already show that it's probably working. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
check_unsafe_optionsonly matched exact canonical names, so an abbreviated option like--uplslipped past the guard and reached git as--upload-pack— arbitrary command execution even withallow_unsafe_options=False.Fix
Reject an option when its canonical name is a prefix of any blocked option's name. Empty candidates (e.g. a bare
--) are skipped so they don't false-match.Test
Added a clone-path regression test covering
--upl,--upload-pac, and--conf. Existing unsafe-option tests still pass.Refers: GHSA-f2m5-q89v-8m67.