Re: [PATCH v2 3/3] t: add tests for safe.directory when running with sudo

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

<rsbecker@nexbridge.com> writes:

>>> +test_lazy_prereq SUDO '
>>> +	is_root sudo &&
>>> +	! sudo grep -E '^[^#].*secure_path' /etc/sudoers '
>
> /etc/sudoers is not standard although usual. This path should come
> from a knob somewhere. We can't run this test on our x86 system
> anyway - no access to root or sudo even though it is
> installed. Also, /etc/sudoers is typically secured 0600 so the
> grep will fail even if is_root passes - and I'm worried about the
> portability of is_root, which is mostly Linux.

True.

On a box I happen to be using, the grep gives me

    Defaults secure_path=/usr/sbin:/usr/bin:/sbin:/bin

and makes the prereq fail.  Which is sort-of expected, and I
understand why this check is here, but I suspect that any sane and
sensible installations would reinitialize the path to a fairly
restricted one with the mechanism, so we wouldn't be testing this on
majority of places, I am afraid.

What we really care is that we use the same PATH as the test
environment uses, including "we want to test the binary we just
built" (or "at the specified path", when GIT_TEST_INSTALLED is in
effect).  I wonder what the right way to carry $PATH and other
environment variables down to whatever "sudo" in the test runs.

    $ foo=foobla; export foo
    $ sudo sh -c set | grep foo; echo $?
    1

so resetting PATH from an environment we export, e.g.

    USE_THIS_PATH=$PATH sudo sh -c '
	PATH=$USE_THIS_PATH
	... invoke our git normally here  ...
	git status
    '

would not work X-<.  Worse yet, other environment variables such as
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME that we set in our tests may
probably be cleared before "sudo" runs any test command, so rejecting
an installaion whose "sudo" resets PATH with the above check is probably
insufficient to give our tests a reasonable envionment to run in.

>>Overall, I like the simplicity and clarity of "do not start this test as 'root'" in the
>>previous round much better.
>>
>>But other than that, the test coverage given by this patch looks quite sensible.

So, I unfortunately have to strike the last sentence from my earlier
response.  The tests do mean well, but I doubt they would work in
the way the good intentions planned them to work.