Re: [PATCH v2 2/3] safe.directory: normalize the configured path

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 26, 2024, 15:02 UTC
Message-ID: <xmqqikwsyz6d.fsf@gitster.g>
In-Reply-To: <20240726050253.GC642208@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

> This may be a dumb question, but... will this always work, if the config
> option is not necessarily an exact path, and might have globbing
> characters in it?
>
> We don't currently treat it like a wildmatch glob, but:
>
>   1. We do allow "/*" on the end. Should we strip that off so it doesn't
>      confuse the realpath lookup?

This one I wondered, too, but the test seems to show that it is OK ;-)

>   2. This is shutting the door on using wildmatch or similar in the
>      future. Is that OK?

I am inclined to keep this part and any other logic that are meant to address "security" things simple and stupid. So in that sense, I am not so worried that it would be hard to retrofit wildmatch to this codepath.

> Should we be quiet there, and maybe fall back to using the
> non-normalized path (though I guess if we could not normalize it, that
> probably means it could never match anyway)?

The only reason I did the warning was because it makes me feel a bit uneasy to have a configuration item that either gets ignored or triggers a fallback behaviour and not to tell users about it. Other than that, I have no strong preference.

Unfortunately this is not a very good fit for the advise mechanism, as we do not even have a repository yet.