On Mon, Jul 22, 2024 at 07:18:59PM -0700, Junio C Hamano wrote:
> The pathname of a repository comes from getcwd() and it could be a
> path aliased via symbolic links, e.g., the real directory may be
> /home/u/repository but a symbolic link /home/u/repo may point at it,
> and the clone request may come as "git clone file:///home/u/repo/"
>
> A request to check if /home/u/repository is safe would be rejected
> if the safe.directory configuration allows /home/u/repo/ but not its
> alias /home/u/repository/. Normalize the paths configured for the
> safe.directory configuration variable before comparing them with the
> path being checked.
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?
2. This is shutting the door on using wildmatch or similar in the
future. Is that OK?
> @@ -1236,6 +1236,16 @@ static int safe_directory_cb(const char *key, const char *value,
>
> if (!git_config_pathname(&allowed, key, value)) {
> const char *check = allowed ? allowed : value;
> + char *to_free = real_pathdup(check, 0);
> +
> + if (!to_free) {
> + warning(_("safe.directory '%s' cannot be normalized"),
> + check);
> + goto next;
> + } else {
> + check = to_free;
> + }
I'm not sure about this warning. I don't know how people use this config
in the real world, but it seems plausible to me they might have:
[safe]
directory = /home/me/this/always/exists
directory = /home/me/this/might/not/exist
perhaps because they're using a generic config file across multiple
machines. And then they'd get:
warning: safe.directory '/home/me/this/might/not/exist' cannot be normalized
on every invocation (at least ones that need to consult safe.directory).
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)?
-Peff
