Re: [PATCH 1/2] show-ref --verify: accept pseudorefs

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 8, 2024, 08:24 UTC
Message-ID: <ZcSPpbL6AWAOkkCI@tanuki>
In-Reply-To: <xmqq5xz0b3lu.fsf@gitster.g>

On Wed, Feb 07, 2024 at 09:12:29AM -0800, Junio C Hamano wrote:

Show 8 quoted lines

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > ... when CHERRY_PICK_HEAD exists. By calling refname_is_safe() instead
> > of comparing the refname to "HEAD" we can accept all one-level refs that
> > contain only uppercase ascii letters and underscores.
> 
> Geez.  We have at least three implementations to determine if a ref
> is a valid name?

`check_refname_format()` and `refname_is_safe()` are often used in tandem. `check_refname_format()` performs the first set of checks to verify whether the pathname components are valid, whereas `refname_is_safe()` checks for refs which are unsafe e.g. because they try to escape out of "refs/".

I think that we really ought to merge `refname_is_safe()` into `check_refname_format()`. It would require us to introduce a new flag `REFNAME_ALLOW_BAD_NAME` to the latter function so that it would accept refs with a bad name that are otherwise safe. But I think we shouldn't ever allow unsafe names, so merging these two functions would overall reduce the potential for security-relevant issues.

Patrick