Repository navigation
fix: validate object types when loading binary SHAs - #2256
Merged
Byron merged 1 commit intoSep 28, 2026
Merged
Byron merged 1 commit into
Byron merged 1 commit into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
Byron
force-pushed
the
better-binsha-checks
branch
from
ca55dbf to
0610404
Compare
September 28, 2026 07:41
Byron
marked this pull request as ready for review
September 28, 2026 07:41
Copy link
Copy Markdown
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 · 1
Drain unread payload after object type mismatch · New
Select submodule repository for added gitlinks · New
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 IndexObject for 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.
Sorry, something went wrong.
Comment thread
git/objects/base.py
Outdated
Comment thread
git/objects/fun.py
Outdated
Comment thread
git/objects/util.py
Outdated
Comment thread
git/objects/util.py
Outdated
Comment thread
git/diff.py
Byron
force-pushed
the
better-binsha-checks
branch
from
0610404 to
7ed5433
Compare
September 28, 2026 11:24
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved object-type, raw tree traversal, and submodule repository-selection issues block approval.
Review effort: Lite
Findings: 1
Sorry, something went wrong.
Comment thread
git/objects/base.py
Outdated
<!-- 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
7ed5433 to
6bf77c0
Compare
September 28, 2026 11:48
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three moderate unresolved issues remain in object validation, tree traversal, and added gitlink handling.
Review effort: Lite
Findings: 2
Sorry, something went wrong.
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).tree now raises a ValueError identifying 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 subsequent git cat-file reads 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 in Blob objects. Index creation now accepts binary SHAs and hexadecimal bytes alongside strings and trees; submodule diffs use IndexObject entries 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:
Git behavior reference: d38352cd43ab9745686d697872408bc3249a153f in the local Git checkout; the commit, tree, and tag readers check object types before parsing their contents.