Phillip Wood <phillip.wood123@xxxxxxxxx> writes: (I'd be brief as it is my day-off today). >> +test_expect_success SUDO 'cannot access with sudo' ' >> + ( >> + # TODO: test_must_fail needs additional functionality >> + # 6a67c759489 blocks its use with sudo >> + cd root/p && >> + ! sudo git status >> + ) >> +' > > I think Junio suggested that this should work and showed it was simple > to make it work. It seems funny that if sudo is started as root it > does not work. It does feel odd. Any attacker who can prepare a root-owned trap do not have to trick "sudo git" to fall there, so I personally do not see much value in stopping this particular pattern, but since workarounds are easy (like double sudo), I do not mind if this does not work *IF* there is a good reason to stop this. >> +test_expect_success SUDO 'can access using a workaround' ' >> + # run sudo twice >> + ( >> + cd root/p && >> + run_with_sudo <<-END >> + sudo git status >> + END >> + ) && >> + # provide explicit GIT_DIR >> + ( >> + cd root/p && >> + run_with_sudo <<-END >> + GIT_DIR=.git && >> + GIT_WORK_TREE=. && >> + export GIT_DIR GIT_WORK_TREE && >> + git status > > I'm confused by this. Does this mean we don't do the ownership checks > if GIT_DIR and or GIT_WORK_TREE are set in the environment? That's by design. Remember, we are protecting people from "cd"ing into a (sub)directory, unknowingly, that happens to be controled by a Git repository somebody else prepared as a trap. So we try to ensure who owns such a repository when we do the discovery. Users who set these environment variables to tell Git that they are aware that they are working with that directory are different from the target audience. > Thanks for working on this Ditto. And thanks for reviewing and raising questions. I agree with all the things you said in the part of the message I did not touch in this reply.
