fix: validate object types when loading binary SHAs (#2254)

Constructing a `Commit` with an annotated tag's `binsha` let the commit
parser interpret tag headers. Accessing its tree then tried to parse a
commit as tree data and raised an unrelated `IndexError`.

Validate the object database's type before parsing object data or exposing
its size and streams. Share the check across `Commit`, `Tree`, `TagObject`,
`Blob`, and the raw tree traversals used by index merging. Report the SHA,
actual type, and expected type in a `ValueError`. Constructors remain lazy
so placeholders and objects outside the current database still work.
Drain rejected streams in bounded chunks so a retained exception does not
desynchronize subsequent `git cat-file` requests.

Also preserve binary SHAs in `IndexFile.new()` instead of converting them
to their Python string representation. Accept binary and hexadecimal
bytes alongside hex strings and `Tree` objects. Represent submodule diff
entries with `IndexObject`, retaining their path, mode, and access to the
submodule's commit data without labeling those commits as blobs.

Extend existing object, index, and submodule diff tests rather than adding
dedicated tests. Update the existing commit stream mock to return an
`OStream`, and remove the obsolete `IndexFile.new()` assignment diagnostic
from the `basedpyright` baseline.

Git reference: commit `d38352cd43ab9745686d697872408bc3249a153f` in the local
Git reference checkout. Its `repo_parse_commit_internal()` in `commit.c`,
`repo_parse_tree_gently()` in `tree.c`, and `parse_tag()` in `tag.c` check
object types before parsing their contents.

Validation:

- The extended `test_base_object` and `test_index_new` failed before the fix.
- The affected object, blob, commit, tree, index, helper, reference, diff,
  and no-fetch submodule suites passed 227 tests, with 2 skipped, across
  the suite run and the staged-file test's rerun with
  `init.defaultBranch=master`. Test commands disabled `commit.gpgSign`;
  tests needing local sockets or the linked object store ran outside the
  sandbox.
- The issue reproduction, binary-SHA index creation, and generic/lazy
  objects were checked with both `GitCmdObjectDB` and `GitDB`.
- `ruff check .`, `ruff format --check .`, `mypy` (46 source files), and
  `basedpyright --warnings` passed.

Author: codex

.basedpyright/baseline.json

@@ -77,14 +77,6 @@
           "lineCount": 1
         }
       },
-      {
-        "code": "reportAssignmentType",
-        "range": {
-          "startColumn": 38,
-          "endColumn": 76,
-          "lineCount": 1
-        }
-      },
       {
         "code": "reportArgumentType",
         "range": {

git/diff.py

@@ -11,6 +11,7 @@
 
 from git.cmd import Git, handle_process_output
 from git.compat import defenc
+from git.objects.base import IndexObject
 from git.objects.blob import Blob
 from git.objects.util import mode_str_to_int
 from git.util import finalize_process, hex_to_bin
@@ -35,7 +36,6 @@
 if TYPE_CHECKING:
     from subprocess import Popen
 
-    from git.objects.base import IndexObject
     from git.objects.commit import Commit
     from git.objects.tree import Tree
     from git.repo.base import Repo
@@ -378,6 +378,10 @@ class Diff:
     Diffs keep information about the changed blob objects, the file mode, renames,
     deletions and new files.
 
+    For submodule changes, ``a_blob`` and ``b_blob`` are
+    :class:`~git.objects.base.IndexObject` instances whose SHAs refer to commits in
+    the submodule repository.
+
     There are a few cases where ``None`` has to be expected as member variable value:
 
     New File::
@@ -481,17 +485,22 @@ def __init__(
                         repo = submodule.module()
                     break
 
+        # Gitlinks reference commits; generic index objects preserve their path and mode.
         self.a_blob: Union["IndexObject", None]
         if a_blob_id is None or a_blob_id == self.NULL_HEX_SHA:
             self.a_blob = None
         else:
-            self.a_blob = Blob(repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=self.a_path)
+            self.a_blob = (IndexObject if self.a_mode == 0o160000 else Blob)(
+                repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=self.a_path
+            )
 
         self.b_blob: Union["IndexObject", None]
         if b_blob_id is None or b_blob_id == self.NULL_HEX_SHA:
             self.b_blob = None
         else:
-            self.b_blob = Blob(repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=self.b_path)
+            self.b_blob = (IndexObject if self.b_mode == 0o160000 else Blob)(
+                repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=self.b_path
+            )
 
         self.new_file: bool = new_file
         self.deleted_file: bool = deleted_file

git/index/base.py

@@ -310,7 +310,7 @@ def merge_tree(
         return self
 
     @classmethod
-    def new(cls, repo: "Repo", *tree_sha: Union[str, Tree]) -> "IndexFile":
+    def new(cls, repo: "Repo", *tree_sha: Union[str, bytes, Tree]) -> "IndexFile":
         """Merge the given treeish revisions into a new index which is returned.
 
         This method behaves like ``git-read-tree --aggressive`` when doing the merge.
@@ -326,7 +326,9 @@ def new(cls, repo: "Repo", *tree_sha: Union[str, Tree]) -> "IndexFile":
             If you intend to write such a merged Index, supply an alternate
             ``file_path`` to its :meth:`write` method.
         """
-        tree_sha_bytes: List[bytes] = [to_bin_sha(str(t)) for t in tree_sha]
+        tree_sha_bytes: List[bytes] = [
+            to_bin_sha(t if isinstance(t, bytes) else str(t).encode("ascii")) for t in tree_sha
+        ]
         base_entries = aggressive_tree_merge(repo.odb, tree_sha_bytes)
 
         inst = cls(repo)

git/objects/base.py

@@ -12,7 +12,7 @@
 from git.exc import WorkTreeRepositoryUnsupported
 from git.util import LazyMixin, bin_to_hex, join_path_native, stream_copy
 
-from .util import get_object_type_by_name
+from .util import _assert_object_type, get_object_type_by_name
 
 # typing ------------------------------------------------------------------
 
@@ -107,6 +107,10 @@ def __init__(self, repo: "Repo", binsha: bytes) -> None:
 
         :param binsha:
             20 byte SHA1
+
+        :note:
+            Object data is loaded lazily. If `binsha` refers to a different object
+            type, accessing that data raises :exc:`ValueError`.
         """
         super().__init__()
         self.repo = repo
@@ -155,6 +159,7 @@ def _set_cache_(self, attr: str) -> None:
         """Retrieve object information."""
         if attr == "size":
             oinfo = self.repo.odb.info(self.binsha)
+            _assert_object_type(oinfo, self.type)
             self.size = oinfo.size  # type: int
         else:
             super()._set_cache_(attr)
@@ -198,7 +203,9 @@ def data_stream(self) -> "OStream":
         :note:
             Returned streams must be read in order.
         """
-        return self.repo.odb.stream(self.binsha)
+        ostream = self.repo.odb.stream(self.binsha)
+        _assert_object_type(ostream, self.type)
+        return ostream
 
     def stream_data(self, ostream: SupportsWrite[bytes]) -> "Object":
         """Write our data directly to the given output stream.
@@ -209,7 +216,7 @@ def stream_data(self, ostream: SupportsWrite[bytes]) -> "Object":
         :return:
             self
         """
-        istream = self.repo.odb.stream(self.binsha)
+        istream = self.data_stream
         stream_copy(istream, ostream)
         return self
 

git/objects/commit.py

@@ -242,7 +242,7 @@ def replace(self, **kwargs: Any) -> "Commit":
     def _set_cache_(self, attr: str) -> None:
         if attr in Commit.__slots__:
             # Read the data in a chunk, its faster - then provide a file wrapper.
-            _binsha, _typename, self.size, stream = self.repo.odb.stream(self.binsha)
+            _binsha, _typename, self.size, stream = self.data_stream
             self._deserialize(BytesIO(stream.read()))
         else:
             super()._set_cache_(attr)

git/objects/fun.py

@@ -14,6 +14,8 @@
 
 from git.compat import safe_decode, defenc
 
+from .util import _assert_object_type
+
 # typing ----------------------------------------------
 
 from typing import (
@@ -200,8 +202,10 @@ def traverse_trees_recursive(
         if tree_sha is None:
             data: List[EntryTupOrNone] = []
         else:
+            stream = odb.stream(tree_sha)
+            _assert_object_type(stream, "tree")
             # Make new list for typing as list invariant.
-            data = list(tree_entries_from_data(odb.stream(tree_sha).read()))
+            data = list(tree_entries_from_data(stream.read()))
         # END handle muted trees
         trees_data.append(data)
     # END for each sha to get data for
@@ -268,7 +272,9 @@ def traverse_tree_recursive(odb: "ObjectDBR", tree_sha: bytes, path_prefix: str)
         Prefix to prepend to the front of all returned paths.
     """
     entries = []
-    data = tree_entries_from_data(odb.stream(tree_sha).read())
+    stream = odb.stream(tree_sha)
+    _assert_object_type(stream, "tree")
+    data = tree_entries_from_data(stream.read())
 
     # Unpacking/packing is faster than accessing individual items.
     for sha, mode, name in data:

git/objects/tag.py

@@ -105,7 +105,7 @@ def __init__(
     def _set_cache_(self, attr: str) -> None:
         """Cache all our attributes at once."""
         if attr in TagObject.__slots__:
-            ostream = self.repo.odb.stream(self.binsha)
+            ostream = self.data_stream
             lines: List[str] = ostream.read().decode(defenc, "replace").splitlines()
 
             _obj, hexsha = lines[0].split(" ")

git/objects/tree.py

@@ -208,7 +208,7 @@ def _get_intermediate_items(
     def _set_cache_(self, attr: str) -> None:
         if attr == "_cache":
             # Set the data when we need it.
-            ostream = self.repo.odb.stream(self.binsha)
+            ostream = self.data_stream
             self._cache: List[TreeCacheTup] = tree_entries_from_data(ostream.read())
         else:
             super()._set_cache_(attr)

git/objects/util.py

@@ -28,6 +28,8 @@
 import time
 import warnings
 
+from gitdb.base import OStream
+
 from git.util import Actor, IterableList, IterableObj
 
 # typing ------------------------------------------------------------
@@ -54,6 +56,8 @@
     from io import BytesIO, StringIO
     from subprocess import Popen
 
+    from gitdb.base import OInfo
+
     from git.types import Protocol, runtime_checkable
 
     from .blob import Blob
@@ -88,6 +92,19 @@ class TraverseNT(NamedTuple):
 # { Functions
 
 
+def _assert_object_type(oinfo: "OInfo", expected_type: Union[str, None]) -> None:
+    """Reject mismatched object types before interpreting their data."""
+    if expected_type is None or oinfo.type == expected_type.encode("ascii"):
+        return
+    if isinstance(oinfo, OStream):
+        # Drain the cat-file response even if the exception keeps the stream alive.
+        while oinfo.read(8192):
+            pass
+    raise ValueError(
+        "Object %s is a %s, not a %s" % (oinfo.hexsha.decode("ascii"), oinfo.type.decode("ascii"), expected_type)
+    )
+
+
 def mode_str_to_int(modestr: Union[bytes, str]) -> int:
     """Convert mode bits from an octal mode string to an integer mode for git.
 

test/test_base.py

@@ -4,6 +4,7 @@
 # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
 
 import gc
+from io import BytesIO
 import os
 import os.path as osp
 import sys
@@ -76,6 +77,21 @@ def test_base_object(self):
             # Remove the file this way, instead of with a context manager or "finally",
             # so it is only removed on success, and we can inspect the file on failure.
             os.remove(tmpfile.name)
+
+            for wrong_type, attr in ((Blob, "data_stream"), (Tree, "_cache"), (Commit, "tree"), (TagObject, "object")):
+                if wrong_type is obj_type:
+                    continue
+                invalid = wrong_type(self.rorepo, binsha)
+                for invalid_attr in ("size", attr):
+                    with self.assertRaisesRegex(ValueError, f"{hexsha}.*{typename}.*{wrong_type.type}"):
+                        getattr(invalid, invalid_attr)
+                try:
+                    invalid.stream_data(BytesIO())
+                except ValueError:
+                    # The next read must work even while the exception retains the stream.
+                    self.assertEqual(item.data_stream.read(), data)
+                else:
+                    self.fail("Expected ValueError for mismatched object type")
         # END for each object type to create
 
         # Each has a unique sha.