fix: validate object types when loading binary SHAs - #2256
Merged
Merged
Conversation
Byron
force-pushed
the
better-binsha-checks
branch
from
September 28, 2026 07:41
ca55dbf to
0610404
Compare
Byron
marked this pull request as ready for review
September 28, 2026 07:41
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved stream-safety, type-normalization, and added-submodule diff issues require fixes.
Review effort: Lite
Findings: 4
Open (5)
What changed in this PR
Adds lazy Git object-type validation, binary SHA support for index creation, and improved submodule diff handling.
Changes:
- Validates object types before parsing or streaming.
- Accepts binary and hexadecimal SHAs in
IndexFile.new(). - Uses
IndexObjectfor submodule diff entries and extends tests.
| File | Summary |
|---|---|
test/test_index.py |
Tests binary SHA and type handling. |
test/test_diff.py |
Tests submodule diff behavior. |
test/test_commit.py |
Updates commit stream tests. |
test/test_base.py |
Tests object validation. |
git/objects/util.py |
Implements shared type checking. |
git/objects/tree.py |
Validates tree loading. |
git/objects/tag.py |
Validates tag loading. |
git/objects/fun.py |
Validates tree traversal. |
git/objects/commit.py |
Validates commit loading. |
git/objects/base.py |
Adds lazy object validation. |
git/index/base.py |
Supports binary SHA inputs. |
git/diff.py |
Handles submodule commits as index objects. |
.basedpyright/baseline.json |
Removes an obsolete diagnostic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Byron
force-pushed
the
better-binsha-checks
branch
from
September 28, 2026 11:24
0610404 to
7ed5433
Compare
<!-- agent --> An object constructed with a SHA for another Git object type could silently expose that object's size. Check the type metadata already returned by `odb.info()` when loading an uncached `Object.size`, and raise `ValueError` naming the SHA, actual type, and expected type. Limit validation to this metadata lookup. Rejecting an unread stream can leave payload bytes in the persistent `git cat-file --batch` response while an exception retains its traceback, corrupting subsequent reads. Stream consumers keep their existing behavior, with no type assertions or added draining. Constructors remain lazy, and object-data parsing still relies on callers to supply a SHA of the appropriate type. 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 to cover binary SHA inputs, metadata mismatches, and raw streaming through wrappers of another object type. Remove the obsolete `IndexFile.new()` assignment diagnostic from the `basedpyright` baseline. Validation: 67 affected tests and Ruff lint/format checks passed. Subsequent reads were also verified with active parser exceptions using both `GitCmdObjectDB` and `GitDB`. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
Byron
force-pushed
the
better-binsha-checks
branch
from
September 28, 2026 11:48
7ed5433 to
6bf77c0
Compare
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.


Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by
Codex GPT-6.Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Fixes #2254.
Commit(repo, tag.binsha).treenow raises aValueErroridentifying the SHA, its actual type, and the expected type before parsing tag data as a commit. The shared check also covers trees, tags, blobs, object size/streams, and raw tree traversal. Validation happens on first data access, preserving lazy constructors and placeholder objects. Rejected streams are drained so subsequentgit cat-filereads remain usable even while an exception retains its traceback.The related audit found two more cases:
IndexFile.new()converted binary SHAs to their Python string representation, and submodule diffs wrapped commit SHAs inBlobobjects. Index creation now accepts binary SHAs and hexadecimal bytes alongside strings and trees; submodule diffs useIndexObjectentries while retaining access to the submodule's commit data.Existing object, index, and submodule diff tests were extended in place, including mixed binary/hex/tree arguments. No new test functions were added.
Validation:
init.defaultBranch=master. Commit signing was disabled only for test commands.GitCmdObjectDBandGitDB.ca55dbffound no actionable regressions.Git behavior reference:
d38352cd43ab9745686d697872408bc3249a153fin the local Git checkout; the commit, tree, and tag readers check object types before parsing their contents.