On Tue, Jul 30, 2024 at 09:03:35AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

quoted

+			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:

Oh, right, I totally misread it. You'd think after 25+ years of writing
C that I would be able to get the strcmp() return value right in my
head... :)

So yeah, it is doing the right thing.

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

Hmm. Yeah, that probably would have softened my confusion, but it's also
kind of hard to read. I think what you wrote originally is just fine,
and I just mis-read it.

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 "*".

I could see arguments in either direction, and I don't have a strong
opinion between them.

-Peff