Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Aug 25, 2026, 09:04 UTC
- Message-ID
- <d6940aa6-9336-481b-8ee5-5e3d9f3d3a50@gmail.com>
- In-Reply-To
- <20260729233215.398654-7-sandals@crustytoothpaste.net>
Hi brian
On 30/07/2026 00:32, brian m. carlson wrote:
Show 18 quoted lines
> Git has historically allowed either lowercase or uppercase hex for > object IDs, but it has always emitted only lowercase. This has caused > people to expect only lowercase and not handle uppercase. > > As an example, Git's own example hooks look for "[0-9a-f]" in several > places, but there are many other Git-adjacent pieces of software, > including Gitolite, which make the assumption that object IDs are always > lowercase. This is not to criticize the authors of these projects, but > rather to point out how common this assumption is. In fact, it's so > common that we have only one test in our codebase that fails when we > reject uppercase object IDs. > > More critically, it leads people to make security-based assumptions that > an object ID either does not contain uppercase characters or that an > object ID can be expressed uniquely in hex form, neither of which are > currently true. Git itself normally uses binary object IDs, which > avoids many of these problems, but most other projects deal primarily in > hex object IDs, so they are more affected.
Can you say a bit more about the security problems please - I'm trying to understand why ABCDEF is a security risk when abcdef^0 isn't.
Thanks
Phillip
Show 86 quoted lines
> In preparation for Git 3.0, only allow lowercase hex object IDs in
> breaking changes mode and document this as well. Update the single
> failing test and add a new one to verify we reject new uppercase object
> IDs. Note that in t5324, we change the hex character from "A" to "b"
> because in SHA-256 mode, "a" is the correct value, so our test_must_fail
> assertion will unexpectedly succeed in that case.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> Documentation/BreakingChanges.adoc | 5 +++++
> hex-ll.h | 4 ++++
> t/t1503-rev-parse-verify.sh | 5 +++++
> t/t5324-split-commit-graph.sh | 4 ++--
> 4 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
> index 73bb939359..dbc46d14e3 100644
> --- a/Documentation/BreakingChanges.adoc
> +++ b/Documentation/BreakingChanges.adoc
> @@ -171,6 +171,11 @@ JGit, libgit2 and Gitoxide need to support it.
> matches the default branch name used in new repositories by many of the
> big Git forges.
>
> +* Git will accept hex object IDs only in lowercase. The fact that Git has
> + historically allowed uppercase characters in hex object IDs has been the
> + source of a variety of bugs and security problems in software using Git. We
> + don't expect most users to notice any change.
> +
> * Git will require Rust as a mandatory part of the build process. While Git
> already started to adopt Rust in Git 2.49, all parts written in Rust are
> optional for the time being. This includes:
> diff --git a/hex-ll.h b/hex-ll.h
> index 9da76f17e8..2f9c8d7c25 100644
> --- a/hex-ll.h
> +++ b/hex-ll.h
> @@ -6,7 +6,11 @@ enum hexkind {
> HEX_KIND_LOWER = 1,
> };
>
> +#ifdef WITH_BREAKING_CHANGES
> +#define HEX_KIND_OID HEX_KIND_LOWER
> +#else
> #define HEX_KIND_OID HEX_KIND_MIXED
> +#endif
>
> extern const signed char hexval_table[256];
> extern const signed char hexval_lc_table[256];
> diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
> index 87638a4a2c..f07b45de5a 100755
> --- a/t/t1503-rev-parse-verify.sh
> +++ b/t/t1503-rev-parse-verify.sh
> @@ -60,6 +60,11 @@ test_expect_success 'works with one good rev' '
> test "$rev_head" = "$HASH4"
> '
>
> +test_expect_success WITH_BREAKING_CHANGES 'rejects uppercase revs' '
> + UC_HASH=$(echo "$HASH1" | tr a-f A-F) &&
> + test_must_fail git rev-parse --verify "$UC_HASH"
> +'
> +
> test_expect_success 'fails with any bad rev or many good revs' '
> test_must_fail git rev-parse --verify 2>error &&
> test_grep "single revision" error &&
> diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
> index bf7ba0e558..29db815c77 100755
> --- a/t/t5324-split-commit-graph.sh
> +++ b/t/t5324-split-commit-graph.sh
> @@ -349,7 +349,7 @@ test_expect_success 'verify after commit-graph-chain corruption (base)' '
> test_must_fail git commit-graph verify 2>test_err &&
> grep -v "^+" test_err >err &&
> test_grep "invalid commit-graph chain" err &&
> - corrupt_file "$graphdir/commit-graph-chain" 30 "A" &&
> + corrupt_file "$graphdir/commit-graph-chain" 30 "a" &&
> test_must_fail git commit-graph verify 2>test_err &&
> grep -v "^+" test_err >err &&
> test_grep "unable to find all commit-graph files" err
> @@ -364,7 +364,7 @@ test_expect_success 'verify after commit-graph-chain corruption (tip)' '
> test_must_fail git commit-graph verify 2>test_err &&
> grep -v "^+" test_err >err &&
> test_grep "invalid commit-graph chain" err &&
> - corrupt_file "$graphdir/commit-graph-chain" 70 "A" &&
> + corrupt_file "$graphdir/commit-graph-chain" 70 "b" &&
> test_must_fail git commit-graph verify 2>test_err &&
> grep -v "^+" test_err >err &&
> test_grep "unable to find all commit-graph files" err
>