Re: "git symbolic-ref" doesn't do a very good job

[Jeff King <peff@peff.net>]

On Sun, Jul 31, 2022 at 12:10:01PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I'm tempted to say that the symref-reading code should
> > actually complain about following something outside of "refs/", but that
> > carries an even higher possibility of breaking somebody. But it seems
> > like we should be consistent between what we allow to be read, and what
> > we allow to be written.
> >
> > At any rate, with the code as you have it above, I think the "make sure
> > HEAD starts with refs/" code is now redundant.
> 
> Isn't the rule these days "HEAD must be either detached or point
> into refs/heads/"?  I thought "checkout" ensures that, and I am
> tempted to think that "symbolic-ref" that works on HEAD should be
> consistent with "checkout".  So "make sure HEAD is within refs/"
> would certainly be "not wrong per-se" but not sufficiently tight,
> I suspect.

No, sadly, that isn't the rule. See afe5d3d516 (symbolic ref: refuse
non-ref targets in HEAD, 2009-01-29) which tightened it to "refs/heads"
and then e9cc02f0e4 (symbolic-ref: allow refs/<whatever> in HEAD,
2009-02-13) which had to loosen it.

Likewise we seemed to touch the reading side at the same time, via
b229d18a80 (validate_headref: tighten ref-matching to just branches,
2009-01-29), and then 222b167386 (Revert "validate_headref: tighten
ref-matching to just branches", 2009-02-12).

In both cases it was refs/top-bases that triggered the revert. The
relevant thread is:

  https://lore.kernel.org/git/cc723f590902120009w432f5f61xd6550409835cdbb7@mail.gmail.com/

There's some discussion there about how topgit could do things
differently, but I don't see any plan for moving away. However, the
latest changelog for it I could find has:

  [from https://mackyle.github.io/topgit/changelog.html]
  TopGit 0.19.4 (2017-02-14) introduced support for a new top-bases
  location under heads. This new location will become the default as of
  the TopGit 0.20.0 release. The current location under refs will
  continue to be supported in the future. See tg help migrate-bases for
  more details.

So it looks like there is some will there to switch. But the default
hasn't flipped yet, and we'd be breaking any non-migrated installs. Not
to mention that we don't know if any _other_ tools care. The topgit
folks reported the original problem in 2009 within a few weeks, and it
never made it to a release.

So yeah. We could certainly work out a deprecation/migration plan, but
I'm not sure it's worth the effort. I do suspect there are other parts
of Git that assume HEAD points at refs/heads/, especially the
clone/fetch HEAD-selection code. But that code is getting the symref
target from an untrusted remote, and is already careful about discarding
nonsense and continuing.

Regarding "checkout" versus "symbolic-ref", I do think "checkout"
probably does limit us to refs/heads/. But it is OK for porcelain to be
opinionated and restrictive, but plumbing probably needs to support
existing possibly-silly use cases.

-Peff