GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix: validate object types when loading binary SHAs by Byron · Pull Request #2256 · gitpython-developers/GitPython

Conversation

@Byron

@Byron Byron commented Sep 28, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Tasks

This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.

  • refackiew

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:

  • 227 affected tests passed, with 2 skips, across the suite run and the staged-file test's rerun with init.defaultBranch=master. Commit signing was disabled only for test commands.
  • The extended object and index tests failed before the fix. The issue reproduction and valid binary-SHA operations were also checked with both GitCmdObjectDB and GitDB.
  • Ruff lint/format checks, mypy (46 source files), and basedpyright passed. The now-obsolete index assignment diagnostic was removed from the type-checking baseline.
  • Codex review of ca55dbf found no actionable regressions.

Git behavior reference: d38352cd43ab9745686d697872408bc3249a153f in the local Git checkout; the commit, tree, and tag readers check object types before parsing their contents.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Byron
Byron force-pushed the better-binsha-checks branch from ca55dbf to 0610404 Compare September 28, 2026 07:41
@Byron
Byron marked this pull request as ready for review September 28, 2026 07:41
Copilot AI lite review requested due to automatic review settings September 28, 2026 07:41
Copilot started reviewing on behalf of Byron September 28, 2026 07:42
View session
Copilot AI reviewed Sep 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved stream-safety, type-normalization, and added-submodule diff issues require fixes.

Review effort: Lite
Findings: 4 High severity · 1 Medium severity

Open (5)
  • High severity Drain unread payload after object type mismatch · New
  • High severity Drain OStream after failed object type assertion · New
  • High severity Drain rejected GitCmdObjectDB streams before re-raising · New
  • High severity Normalize object type representations before comparison · New
  • Medium severity Select submodule repository for added gitlinks · New
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 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.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread git/objects/base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread git/objects/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread git/objects/util.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread git/objects/util.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread git/diff.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Copilot AI review requested due to automatic review settings September 28, 2026 11:24
@Byron
Byron force-pushed the better-binsha-checks branch from 0610404 to 7ed5433 Compare September 28, 2026 11:24
Copilot started reviewing on behalf of Byron September 28, 2026 11:25
View session
Copilot AI reviewed Sep 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved object-type, raw tree traversal, and submodule repository-selection issues block approval.

Review effort: Lite
Findings: 1 High severity

Open (1)
  • High severity Normalize ODB object type before comparing or formatting · New
Resolved since last review (5)
  • High severity Normalize object type representations before comparison
  • High severity Drain rejected GitCmdObjectDB streams before re-raising
  • High severity Drain OStream after failed object type assertion
  • High severity Drain unread payload after object type mismatch
  • Medium severity Select submodule repository for added gitlinks

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread git/objects/base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Byron @codex
fix: validate size lookups and preserve binary SHAs (#2254)
6bf77c0 
<!-- 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 <[email protected]>
Copilot AI review requested due to automatic review settings September 28, 2026 11:48
@Byron
Byron force-pushed the better-binsha-checks branch from 7ed5433 to 6bf77c0 Compare September 28, 2026 11:48
Copilot started reviewing on behalf of Byron September 28, 2026 11:49
View session
Copilot AI reviewed Sep 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Three moderate unresolved issues remain in object validation, tree traversal, and added gitlink handling.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
  • Medium severity Validate binary SHA is a tree before recursive merge traversal · New
  • Medium severity Validate object type before direct commit, tree, and tag parsing · New
Resolved since last review (1)
  • High severity Normalize ODB object type before comparing or formatting

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread git/index/base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread git/objects/base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Byron
Byron merged commit 3c638d2 into main Sep 28, 2026
51 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@Byron
Byron deleted the better-binsha-checks branch October 5, 2026 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Exception suggestion: raise when making a Commit with non-commit binsha

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@Byron
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.