Re: [PATCH 7/8] refs: add 'update-symref' command to 'update-ref'

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 10, 2024, 17:31 UTC
Message-ID: <ZhbND7flvT7INnrz@ncase>
In-Reply-To: <xmqqzfu1w6ay.fsf@gitster.g>

On Wed, Apr 10, 2024 at 09:06:45AM -0700, Junio C Hamano wrote:

Show 26 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> > I might have missed it while scanning through this thread, but why
> >> > exactly is the zero OID not a good enough placeholder here to say that
> >> > the ref must not exist? A symref cannot point to a ref named after the
> >> > zero OID anyway.
> >
> >> > In my opinion, "update-symref" with an old-value must be able to accept
> >> > both object IDs and symrefs as old value. Like this it would be possible
> >> > to update a proper ref to a symref in a race-free way. So you can say:
> >> >
> >> >     git update-ref SYMREF refs/heads/main 19981daefd7c147444462739375462b49412ce33
> >
> >> > To update "SYRMEF" to "refs/heads/main", but only in case it currently
> >> > is a proper ref that points to 19981daefd7c147444462739375462b49412ce33.
> >> > Similarly...
> >> >
> >> >     git update-ref SYMREF refs/heads/main refs/heads/master
> >
> >> > would update "SYMREF" to "refs/heads/main", but only if it currently
> >> > points to the symref "refs/heads/master".
> 
> I think that would work well.  We need to explicitly forbid a file
> $GIT_DIR/[0-9a-f]{40} to be used as a pseudoref, which I think that
> is an improvement.  I do not know how the transition to move to a
> world with a stricter rule would look like, though.

I thought that Git already refuses such refnames anyway. Otherwise it would be impossible to distinguish a ref called [0-9a-f]{40} from the actual object hash in much of our tooling. I certainly know that GitLab does refuse such refnames, and thought that GitHub does, too.

But turns out that at least git-update-ref(1) is happy to write such refs:

``` $ git update-ref 1111111111111111111111111111111111111111 HEAD $ cat .git/1111111111111111111111111111111111111111 cf6ba211cd2fce88f5d22d9f036029d502565509 ```

What does it resolve to?

``` $ git rev-parse --verify 1111111111111111111111111111111111111111 warning: refname '1111111111111111111111111111111111111111' is ambiguous. Git normally never creates a ref that ends with 40 hex characters because it will be ignored when you just specify 40-hex. These refs may be created by mistake. For example,

  git switch -c $br $(git rev-parse ...)

where "$br" is somehow empty and a 40-hex ref is created. Please examine these refs and maybe delete them. Turn this message off by running "git config advice.objectNameWarning false" 1111111111111111111111111111111111111111 ```

It resolves to 1111111111111111111111111111111111111111 because it's a valid object ID already. And what if we try to peel it?

``` $ git rev-parse --verify 1111111111111111111111111111111111111111^{commit} warning: refname '1111111111111111111111111111111111111111' is ambiguous. Git normally never creates a ref that ends with 40 hex characters because it will be ignored when you just specify 40-hex. These refs may be created by mistake. For example,

  git switch -c $br $(git rev-parse ...)

Doesn't work.

So these refs essentially cannot be accessed anyway. Restricting git-update-ref(1) such that it cannot write them in the first place thus shouldn't be breaking much, I'd claim, and feels like a strict improvement overall.

Patrick