Re: [PATCH v4 1/7] refs: accept symref values in `ref_transaction[_add]_update`

[Jeff King <peff@peff.net>]

On Mon, Apr 29, 2024 at 09:02:56AM +0200, Patrick Steinhardt wrote:

> > IMHO these should _all_ be forbidden, because we only want to allow the
> > more limited pseudoref names everywhere (and never mischievous ones like
> > "config" or whatever). And once we are doing that, then show-ref has no
> > need to check the format. It can just call read_ref() and it either gets
> > an answer or doesn't.
> > 
> > I don't know if that is a #leftoverbit though. It perhaps more
> > complicated than that.
> 
> Yeah, this is something that I've repeatedly stumbled over myself. If I
> remember correctly, the plan was to clean up and consolidate all these
> different functions we have for checking ref names such that they become
> easier to use and hopefully lead to more consistent behaviour.

I think the code changes here aren't too bad (modulo some
head-scratching I had to do around per-worktree refs). I'll post a
series in a moment (split off from here since we're far off topic from
the original thread).

> In any case, I very much agree that git-update-ref(1) should refuse to
> write refs with names that are known-bad. There should probably be an
> escape hatch though that at least allows you to _delete_ those, or
> otherwise there is no way to remove such a ref in the reftable repo.
> Well, except for meddling with the binary format, but I doubt that
> anybody would really want to do that.

Yeah, ironically deleting is the one thing you cannot do with them even
right now. ;) That is because the supposedly-looser refname_is_safe() is
actually tighter.

It would be tough to loosen it safely, since you don't want to allow
deleting arbitrary files in .git. Or worse, escaping .git via clever
paths, symlinks, etc. So I think at most you'd want some kind of "trust
me, for this command don't bother with ref format checks" flag.

Or alternatively, refname_is_safe() should become a per-backend thing.
And then reftables can say "everything is safe", because the ref names
are never used as paths (I think, anyway; I didn't follow all of the
discussion around per-worktree refs, pseudorefs, etc).

-Peff