Re: [PATCH 1/2] Documentation: explain how safe.directory works when running under sudo

[Junio C Hamano <gitster@pobox.com>]

Carlo Arenas <carenas@gmail.com> writes:

> On Wed, Apr 27, 2022 at 10:17 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:
>> > diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt
>> > index 6d764fe0ccf..67f8ef5d766 100644
>> > --- a/Documentation/config/safe.txt
>> > +++ b/Documentation/config/safe.txt
>> > @@ -26,3 +26,11 @@ directory was listed in the `safe.directory` list. If `safe.directory=*`
>> >  is set in system config and you want to re-enable this protection, then
>> >  initialize your list with an empty value before listing the repositories
>> >  that you deem safe.
>> > ++
>> > +When git tries to check for ownership of git repositories it will obviously
>>
>> Comma before "it will obviously".
>
> Obviously my whole paragraph could be improved further, do you want
> a reroll with this fix, or would instead fixup locally?

I think the patches (including the previous one) are still fluid and
expect them to be reworked; local fix-ups would be a bit premature
and leads to waste.  At least not yet.

>> This raises a design question.  In a repository is owned by root,
>> shouldn't "sudo git describe" work?  IOW, I am wondering if the
>> "instead" at the end of the description is what we want, or if we
>> want to check both the original user and "root".
>
> I think it makes sense to have both, and your implementation below
> seems like a good way to do it but it might not be as safe as it
> seems, since sometimes directories owned by root might be also world
> writable and therefore not necessarily safe.

I am not quite following you; that logic applies to directories
owned by euid (not necessarily root).  As we are loosening to make
"sudo" usable again, the use case to visit root-owned repository as
root via "sudo" is worth discussing, if not worth immediately
supporting, I would think.  I do not think it is absolutely needed
as there is an easy workaround (see below).

Assuming we will go without "same euid, whether it is root or not,
plus SUDO_UID when run as root", here a test addition, updated from
the one I gave you in the review of [2/2]

test_expect_success SUDO 'in root owned repository' '
	mkdir root/p
        sudo chown root root/p &&
	sudo git init root/p &&

	# owned by other person (root), do I see it as a repository?
	(
		cd root/p &&
		test_must_fail git status
	) &&

	# owned by root, can I access it under sudo?
	(
		cd root/p &&
		test_must_fail sudo git status
	) &&

	# workaround #1, giving GIT_DIR explicitly
	(
		cd root/p &&
		sudo "
			GIT_DIR=.git GIT_WORK_TREE=. git status
		"
	) &&

	# workaround #2, discarding SUDO_UID
	(
		cd root/p &&
		sudo "
			unset SUDO_UID;
			git status
		"
	)
'