Original flaw
Earlier advisory · GHSA-3F7W-8RR8-F37FTagReference.create accepted dangerous file options supplied through keyword arguments.
Sink: GitPython git/refs/tag.py TagReference.create option handling
Originally written by Sebastian Thiel
Loading…
How AI contributed
Incomplete remediationOnly the highlighted steps are this advisory. The first card is the earlier flaw the AI tried, and failed, to close.
Original flaw
Earlier advisory · GHSA-3F7W-8RR8-F37FSink: GitPython git/refs/tag.py TagReference.create option handling
Originally written by Sebastian Thiel
This advisoryGHSA-3WXW-XV34-2FRG
AI tried to fix this
The AI change was a real security patch, but it left the same advisory reachable.
Missed: Did not inspect the positional reference and path arguments.
Fixed again
This is the patch that actually stops the same attack path.
Code comparison
--- a/git/index/base.py+++ b/git/index/base.py@@ -130,6 +130,8 @@ class IndexFile(LazyMixin, git_diff.Diffable, Serializable): index directly before operating on it using the git command. """ + unsafe_git_checkout_index_options = ["--prefix"]+ __slots__ = ("repo", "version", "entries", "_extension_data", "_file_path") _VERSION = 2@@ -1212,6 +1214,7 @@ class IndexFile(LazyMixin, git_diff.Diffable, Serializable): paths: Union[None, Iterable[PathLike]] = None, force: bool = False, fprogress: Callable = lambda *args: None,+ allow_unsafe_options: bool = False, **kwargs: Any, ) -> Union[None, Iterator[PathLike], Sequence[PathLike]]: """Check out the given paths or all files from the version known to the index@@ -1238,6 +1241,9 @@ class IndexFile(LazyMixin, git_diff.Diffable, Serializable): no explicit paths are given. Otherwise progress information will be send prior and after a file has been checked out. + :param allow_unsafe_options:+ Allow unsafe options, such as ``--prefix``.+ :param kwargs: Additional arguments to be passed to :manpage:`git-checkout-index(1)`. @@ -1261,6 +1267,12 @@ class IndexFile(LazyMixin, git_diff.Diffable, Serializable): i.e. if you want :manpage:`git-checkout(1)`-like behaviour, use ``head.checkout`` instead of ``index.checkout``. """+ if not allow_unsafe_options:+ Git.check_unsafe_options(+ options=Git._option_candidates([], kwargs),+ unsafe_options=self.unsafe_git_checkout_index_options,+ )+ args = ["--index"] if force: args.append("--force")--- a/git/refs/tag.py+++ b/git/refs/tag.py@@ -16,6 +16,7 @@ from .reference import Reference from typing import Any, TYPE_CHECKING, Type, Union +from git.cmd import Git from git.types import AnyGitObject, PathLike if TYPE_CHECKING:@@ -42,6 +43,8 @@ class TagReference(Reference): __slots__ = + unsafe_git_tag_options = ["--file", "-F"]+ _common_default = "tags" _common_path_default = Reference._common_path_default + "/" + _common_default @@ -92,6 +95,7 @@ class TagReference(Reference): reference: Union[str, "SymbolicReference"] = "HEAD", logmsg: Union[str, None] = None, force: bool = False,+ allow_unsafe_options: bool = False, **kwargs: Any, ) -> "TagReference": """Create a new tag reference.@@ -121,12 +125,21 @@ class TagReference(Reference): :param force: If ``True``, force creation of a tag even though that tag already exists. + :param allow_unsafe_options:+ Allow unsafe options, such as ``--file``.+ :param kwargs: Additional keyword arguments to be passed to :manpage:`git-tag(1)`. :return: A new :class:`TagReference`. """+ if not allow_unsafe_options:+ Git.check_unsafe_options(+ options=Git._option_candidates([], kwargs),+ unsafe_options=cls.unsafe_git_tag_options,+ )+ if "ref" in kwargs and kwargs["ref"]: reference = kwargs["ref"] --- a/test/test_refs.py+++ b/test/test_refs.py@@ -23,6 +23,7 @@ from git import ( SymbolicReference, TagReference, )+from git.exc import UnsafeOptionError from git.objects.tag import TagObject import git.refs as refs from git.util import Actor@@ -60,6 +61,18 @@ class TestRefs(TestBase): # Check remoteness assert Reference(self.rorepo, "refs/remotes/origin").is_remote() + @with_rw_repo("HEAD")+ def test_tag_create_rejects_unsafe_file_options(self, rw_repo):+ with tempfile.NamedTemporaryFile("w", encoding="utf-8") as message:+ message.write("private tag message")+ message.flush()+ for index, option in enumerate(({"F": message.name}, {"file": message.name})):+ with self.assertRaises(UnsafeOptionError):+ TagReference.create(rw_repo, f"unsafe-{index}", **option)++ tag = TagReference.create(rw_repo, "allowed-file", F=message.name, allow_unsafe_options=True)+ self.assertEqual(tag.tag.message, "private tag message")+ def test_from_pathlike(self): # Should be able to create any reference directly. for ref_type in (Reference, Head, TagReference, RemoteReference):--- a/git/index/base.py+++ b/git/index/base.py@@ -1567,7 +1567,8 @@ class IndexFile(LazyMixin, git_diff.Diffable, Serializable): if not allow_unsafe_options: Git.check_unsafe_options( options=Git._option_candidates([other], kwargs),- unsafe_options=self.repo.unsafe_git_revision_options,+ unsafe_options=self.repo.unsafe_git_diff_options,+ clusterable_short_options="46abceflmnpqrstuvwzBCDMNRW", ) # Only run if we are the default repository index.Candidate 13198c760623fd069dddc396615cfd733f8e8df528a2f3a04ae7b4fd5b3a362f · Fix 7419f5aa8c4a5e4a7daab4484f167669951d9ad8076dbebfd821eb3949aea862
Releases