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

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 10 Apr 2024 19:31:59 +0200

On Wed, Apr 10, 2024 at 09:06:45AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <[email-masked]> 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 ...)

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"
fatal: Needed a single revision
```

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