Repository navigation
Conversation
A v0/v1 shallow fetch of a SHA-256 repository fails. `ShallowUpdate.Decode` accepted a shallow or unshallow line only at the length of one carrying a SHA-1 hash, while the server writes the repository's own hash, 64 hex digits for SHA-256 (`send_shallow` and `send_unshallow` use `oid_to_hex` [1]). Over go-git's in-process `file` transport the decode error does not even surface: the negotiation deadlocks, and cancelling the context does not end it. Protocol v2 was not affected, as its `shallow-info` decoder already accepts either hash size. The decoder now parses the hash after the prefix the way the v2 decoder does, with `parseFullHash`. This also rejects a line of SHA-1 length that is not hex, which was read as the zero hash before. `TestFetchShallowSHA256` fetches a SHA-256 repository at depth 1 over both protocol versions. It bounds the wait itself, so a regression fails after a minute instead of hanging the test binary. The SHA-256 skip in `TestSubmodulesFetchDepth` blamed this decoder, but submodule updates fetch over protocol v2 and pass with or without this change, so the skip is removed. [1]: https://github.com/git/git/blob/7e34d8174dcda3435d1404c9a30bef46afe6c13e/upload-pack.c#L789-L836 Assisted-by: Claude Opus 5.5 Signed-off-by: Hidde Beydals <[email protected]>
A fetch stored every shallow and unshallow hash the server sent without checking it against the repository's object format. A broken or hostile server could thereby leave a clone unusable: a SHA-1 hash in the shallow file of a SHA-256 repository makes every later read of it fail with "bad shallow line", and a SHA-256 hash in a SHA-1 repository is read back cut to its first 40 digits, naming a different commit. Git parses a shallow line with `get_oid_hex`, using the repository's hash algorithm [1], and dies on one it cannot parse before it reads the pack [2][3]. That rejects a SHA-1 hash in a SHA-256 repository, but in a SHA-1 repository Git takes the first 40 digits of a SHA-256 hash. go-git rejects both, as the cut hash is not the commit the server meant. Both protocol versions merged the update with their own identical copy of `updateShallow`. They now share `UpdateShallow` in `internal/transport`, and before reading the pack, check the update with the new `CheckShallowUpdate`, which fails the fetch when it holds an object ID of another format. A rejected fetch stores neither the pack nor the shallow boundary. The check also fails when the repository's config cannot be read, rather than assuming SHA-1. [1]: https://github.com/git/git/blob/7e34d8174dcda3435d1404c9a30bef46afe6c13e/hex.c#L8-L19 [2]: https://github.com/git/git/blob/7e34d8174dcda3435d1404c9a30bef46afe6c13e/fetch-pack.c#L461-L469 [3]: https://github.com/git/git/blob/7e34d8174dcda3435d1404c9a30bef46afe6c13e/fetch-pack.c#L1555-L1563 Assisted-by: Claude Opus 5.5 Signed-off-by: Hidde Beydals <[email protected]>
hiddeco
force-pushed
the
fix/shallow-update-sha256
branch
from
November 6, 2026 15:59
6162ab8 to
7cedf6b
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.
A v0/v1 shallow fetch of a SHA-256 repository fails.
ShallowUpdate.Decodeonly accepted ashalloworunshallowline at the length of one carrying a SHA-1 hash, while the server writes the repository's own hash 1. Over the in-processfiletransport the decode error does not even surface, as the negotiation deadlocks (see below). Protocol v2 was not affected, as itsshallow-infodecoder already accepts either hash size.This PR consists of two commits:
parseFullHash, a line of SHA-1 length that is not hex is now rejected, where it was read as the zero hash before.get_oid_hexusing the repository's hash algorithm 2, and dies on one it cannot parse before it reads the pack 3. That rejects a SHA-1 hash in a SHA-256 repository, but in a SHA-1 repository Git takes the first 40 digits of a SHA-256 hash; go-git rejects both. A rejected fetch stores neither the pack nor the shallow boundary, and an unreadable config fails the check instead of being taken for SHA-1. The v0/v1 and v2 paths each had their own identical copy ofupdateShallow; they now share one ininternal/transport.TestFetchShallowSHA256fetches a SHA-256 repository at depth 1 over both protocol versions. It bounds the wait itself, so a regression fails after a minute instead of hanging the test binary.Not in this PR, and worth a follow-up of their own:
filetransport, and cancelling the context does not end it. The client blocks writing its haves before it reads the error of the goroutine that decodes the shallow update (plumbing/transport/negotiate.go).maxSectionLines.unshallowline for a commit that is not shallow is silently ignored, where Git dies with "no shallow found".