* bug? subprocesses can use wrong Git if $PATH is unset
@ 2023-06-01 22:21 Priedhorsky, Reid
2023-06-02 0:35 ` brian m. carlson
0 siblings, 1 reply; 6+ messages in thread
From: Priedhorsky, Reid @ 2023-06-01 22:21 UTC (permalink / raw)
To: git@vger.kernel.org
Hello,
I may have found a bug in Git. It seems that if (1) multiple git(1) are installed on the system, (2) one is in the shell’s default path (i.e., used if $PATH is unset, not the default value of $PATH), and (3) the desired git(1) is at a different path, then subprocesses of the desired git(1) invoke the undesired git(1) instead.
$PATH unset is indeed a pathological situation; one of our own bugs in our software that calls git(1) inappropriately cleared it. However, in my view it’s surprising enough to be a usability bug. I would expect git(1) to call itself for subprocesses regardless of the environment.
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
1. If /usr/bin/git exists, move it out of the way.
2. Write a shell script at /usr/bin/git that prints an error message and then fails.
3. Install a different Git at prefix other than /usr and put it in your path before /usr/bin.
4. Run the following Python script. (It’s Python because I know how to manipulate the environment how I wanted, and I don’t in shell.)
~~~~
#!/usr/bin/env python3
import os
import subprocess
import sys
def chdir(p):
print("", file=sys.stderr)
print("$ cd %s" % p, file=sys.stderr)
os.chdir(p)
def c(msg, argv, **kwargs):
print("", file=sys.stderr)
print("# " + msg, file=sys.stderr)
print("$ " + " ".join(argv), file=sys.stderr)
try:
subprocess.run(argv, check=True, **kwargs)
except subprocess.CalledProcessError as x:
print("# command failed with exit code %d" % x.returncode)
else:
print("# success")
c("fake system Git contents", ["cat", "/usr/bin/git"])
c("path to desired Git", ["which", "git"])
c("Git version", ["git", "--version"])
chdir("/tmp")
c("remove existing test repo", ["rm", "-Rf", "test"])
c("create test repo", ["git", "init", "-q", "test"])
chdir("test")
c("test commit A", ["git", "commit", "-m", "a", "--allow-empty"])
c("branch", ["git", "checkout", "-b", "foo"])
c("test commit B", ["git", "commit", "-m", "b", "--allow-empty"])
# This uses the existing $PATH to find git(1), but then runs it with an empty
# environment.
c("checkout w/ empty env", ["git", "checkout", "master"], env=dict())
~~~~
> What happened instead? (Actual behavior)
> (Note re-ordering for clarity.)
Script output:
# fake system Git contents
$ cat /usr/bin/git
#!/bin/sh
echo 'you should not see this' 1>&2
exit 1
# success
# path to desired Git
$ which git
/usr/local/bin/git
# success
# Git version
$ git --version
git version 2.41.0
# success
$ cd /tmp
# remove existing test repo
$ rm -Rf test
# success
# create test repo
$ git init -q test
# success
$ cd test
# test commit A
$ git commit -m a --allow-empty
[master (root-commit) 03e2eb0] a
# success
# branch
$ git checkout -b foo
Switched to a new branch 'foo'
# success
# test commit B
$ git commit -m b --allow-empty
[foo c417cee] b
# success
# checkout w/ empty env
$ git checkout master
you should not see this
# command failed with exit code 1
> What did you expect to happen? (Expected behavior)
Every command in the script should be successful. In particular, I expected the final output to be:
# checkout w/ empty env
$ git checkout master
[... normal checkout chatter ...]
# success
> What's different between what you expected and what actually happened?
/usr/local/bin/git called /usr/bin/git for some subprocess. I expected it to call itself.
> Anything else you want to add
Usually for us, the version skew is close enough that everything seemed to work. I believe we installed Git 2.40.0 and subprocesses were presumably using the system’s 2.31 and everything appeared to work fine. Only when we tried our software on a different box where the system Git was 1.8 did it crash. Therefore, I worry about silent problems due to version skew in the internal command-line API.
> Please review the rest of the bug report below.
> You can delete any lines you don't wish to share.
>
> [System Info]
> git version:
> git version 2.41.0
> cpu: x86_64
> no commit associated with this build [I built the source tarball]
> sizeof-long: 8
> sizeof-size_t: 8
> shell-path: /bin/sh
> uname: Linux 5.10.0-23-amd64 #1 SMP Debian 5.10.179-1 (2023-05-12) x86_64
> compiler info: gnuc: 10.2
> libc info: glibc: 2.31
> $SHELL (typically, interactive shell): /bin/bash
>
> [Enabled Hooks]
Thanks,
Reid
—
he/his
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bug? subprocesses can use wrong Git if $PATH is unset
2023-06-01 22:21 bug? subprocesses can use wrong Git if $PATH is unset Priedhorsky, Reid
@ 2023-06-02 0:35 ` brian m. carlson
2023-06-02 16:14 ` Priedhorsky, Reid
0 siblings, 1 reply; 6+ messages in thread
From: brian m. carlson @ 2023-06-02 0:35 UTC (permalink / raw)
To: Priedhorsky, Reid; +Cc: git@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]
On 2023-06-01 at 22:21:05, Priedhorsky, Reid wrote:
> Hello,
>
> I may have found a bug in Git. It seems that if (1) multiple git(1) are installed on the system, (2) one is in the shell’s default path (i.e., used if $PATH is unset, not the default value of $PATH), and (3) the desired git(1) is at a different path, then subprocesses of the desired git(1) invoke the undesired git(1) instead.
>
> $PATH unset is indeed a pathological situation; one of our own bugs in our software that calls git(1) inappropriately cleared it. However, in my view it’s surprising enough to be a usability bug. I would expect git(1) to call itself for subprocesses regardless of the environment.
I don't believe this is a bug in Git, but rather a behaviour of your
operating system kernel. If you don't set PATH, then when Git does an
exec, the kernel or libc supplies a default PATH value. Traditionally
this includes /bin and /usr/bin, and on some systems, it used to contain
the current working directory, which has typically been removed for
security.
It isn't possibly to portably determine that path that was used to exec
the current binary, so Git doesn't try to do so, and it assumes that you
set PATH appropriately. In fact, on some systems, you can use fexecve
to execute file descriptors pointing to files that have been unlinked,
so in general, it's not possible to determine which binary to use
without the PATH.
I'm not aware of any other major programs which do this in a better or
more useful way, so I don't think there's anything to change here in
Git.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bug? subprocesses can use wrong Git if $PATH is unset
2023-06-02 0:35 ` brian m. carlson
@ 2023-06-02 16:14 ` Priedhorsky, Reid
2023-06-02 20:20 ` brian m. carlson
0 siblings, 1 reply; 6+ messages in thread
From: Priedhorsky, Reid @ 2023-06-02 16:14 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: brian m. carlson
Hello Brian,
> On Jun 1, 2023, at 6:35 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>
> It isn't possibly to portably determine that path that was used to exec
> the current binary, so Git doesn't try to do so, and it assumes that you
> set PATH appropriately. In fact, on some systems, you can use fexecve
> to execute file descriptors pointing to files that have been unlinked,
> so in general, it's not possible to determine which binary to use
> without the PATH.
Thank you for the explanation. This is fascinating. It also explains why my script works as expected if I substitute “/usr/local/bin/git” for plain “git”.
> I don't think there's anything to change here in Git.
That seems convincing.
I do wonder if the behavior would be worth documenting, e.g. at https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables, where Git’s relationship to $HOME is also documented. I would be happy to submit a pull request.
Thanks,
Reid
—
he/his
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bug? subprocesses can use wrong Git if $PATH is unset
2023-06-02 16:14 ` Priedhorsky, Reid
@ 2023-06-02 20:20 ` brian m. carlson
2023-06-03 1:38 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: brian m. carlson @ 2023-06-02 20:20 UTC (permalink / raw)
To: Priedhorsky, Reid; +Cc: git@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 544 bytes --]
On 2023-06-02 at 16:14:47, Priedhorsky, Reid wrote:
> Hello Brian,
>
> I do wonder if the behavior would be worth documenting, e.g. at
> https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables,
> where Git’s relationship to $HOME is also documented. I would be happy
> to submit a pull request.
I don't think it's necessary, since it's expected behaviour for me, but
I am not the only person on this list, and perhaps others would
appreciate a patch.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bug? subprocesses can use wrong Git if $PATH is unset
2023-06-02 20:20 ` brian m. carlson
@ 2023-06-03 1:38 ` Junio C Hamano
2023-06-07 17:43 ` Priedhorsky, Reid
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2023-06-03 1:38 UTC (permalink / raw)
To: brian m. carlson; +Cc: Priedhorsky, Reid, git@vger.kernel.org
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> On 2023-06-02 at 16:14:47, Priedhorsky, Reid wrote:
>> Hello Brian,
>>
>> I do wonder if the behavior would be worth documenting, e.g. at
>> https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables,
>> where Git’s relationship to $HOME is also documented. I would be happy
>> to submit a pull request.
>
> I don't think it's necessary, since it's expected behaviour for me, but
> I am not the only person on this list, and perhaps others would
> appreciate a patch.
I tend to agree that it is an expected behaviour. In addition,
unsetting PATH is not something people deliberately do every day
without understanding its implications, so I would rather not see us
add "if you do this esteric thing, this would happen" for them.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bug? subprocesses can use wrong Git if $PATH is unset
2023-06-03 1:38 ` Junio C Hamano
@ 2023-06-07 17:43 ` Priedhorsky, Reid
0 siblings, 0 replies; 6+ messages in thread
From: Priedhorsky, Reid @ 2023-06-07 17:43 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: brian m. carlson, Junio C Hamano
> On Jun 2, 2023, at 7:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> On 2023-06-02 at 16:14:47, Priedhorsky, Reid wrote:
>>> Hello Brian,
>>>
>>> I do wonder if the behavior would be worth documenting, e.g. at
>>> https://urldefense.com/v3/__https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables__;!!Bt8fGhp8LhKGRg!CTLDK9n1pGJTODDrapV9r_JrypUCk5A_v4w7bz6GgCTz8_sg91kpz2ID1NRptCjG2bUMiJ6ftC-PlM4$ ,
>>> where Git’s relationship to $HOME is also documented. I would be happy
>>> to submit a pull request.
>>
>> I don't think it's necessary, since it's expected behaviour for me, but
>> I am not the only person on this list, and perhaps others would
>> appreciate a patch.
>
> I tend to agree that it is an expected behaviour. In addition,
> unsetting PATH is not something people deliberately do every day
> without understanding its implications, so I would rather not see us
> add "if you do this esteric thing, this would happen" for them.
>
> Thanks.
Thank you; that sounds like a consensus.
To be clear though, we didn’t unset $PATH on purpose. We found unexpected behavior and tracked it down to a bug that cleared the environment, including $PATH.
Thanks,
Reid
—
he/his
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-07 17:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 22:21 bug? subprocesses can use wrong Git if $PATH is unset Priedhorsky, Reid
2023-06-02 0:35 ` brian m. carlson
2023-06-02 16:14 ` Priedhorsky, Reid
2023-06-02 20:20 ` brian m. carlson
2023-06-03 1:38 ` Junio C Hamano
2023-06-07 17:43 ` Priedhorsky, Reid
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).