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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 30, 2024, 16:03 UTC
Message-ID: <xmqq4j86g948.fsf@gitster.g>
In-Reply-To: <20240730073119.GA562212@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

>> +			if (!is_absolute_path(check) && strcmp(check, ".")) {
>> +				warning(_("safe.directory '%s' not absolute"),
>> +					check);
>> +				goto next;
>> +			}
>
> This is_absolute_path() check is redundant, isn't it? If we are checking
> for a literal ".", then we know the path must be non-absolute.

What I meant was "If it is not absolute, that is an error, but if the thing is a dot, that is allowed as an exception".

Is the lack of "!" confusing, I wonder? We could rewrite it to make it more explicit:

	if (is_absolute_path(check) || !strcmp(check, ".")) {
		; /* OK */
	} else {
		warning(_("not absolute %s"), check);
		goto next;
	}

My earlier draft for v3 had the check for dot a lot earlier in the function, i.e.

	-	} else if (!strcmp(value, "*")) {
	+	} else if (!strcmp(value, "*") || !strcmp(value, ".")) {
			data->is_safe = 1;

and this part said "If not absolute, that is an error" without anything about dot.

But then I changed my mind and made it unsafe to do this:

	cd .git/refs && git -c safe.directory=. foo

as safe.directory=. means "A repository at the current directory of the process is allowed" and the repository in this case is not at "." but at "..", meaning "." is a lot stricter than "*".