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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Wed, Feb 07, 2024 at 09:12:29AM -0800, Junio C Hamano wrote:
> "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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]