On Fri, Jul 26, 2024 at 08:02:02AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

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 ;-)

Hmm. Would it depend on what's in the filesystem, though? That is, if I
had a file named "*", would it get confused?

That is unlikely in normal use, of course, but can we do something
tricky with it? I think probably no? Accessing "/path/to/*" implies that
the user meant to allow all of "/path/to", which would include that. So
the most an attacker could do is _disable_ safe.directory for something
that should have it on.

quoted

  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.

That's my general instinct, too. But I wanted to make sure we were
explicit about the choice.

quoted

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.

Yes, that was my first thought, too, but I noticed the bogus messages
while playing with it. I _think_ if it triggers it means that it points
to something that doesn't exist (or you don't have permission to read),
in which case it would by definition not have matched. So silently
ignoring might not be so bad.

-Peff