threads / rfc / 44894

[BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

Subject: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

## tl;dr

4 messages between Jan 16, 2017 and Jan 18, 2017.

replies: 3people: 2as markdown or json

Max Kirillov· Jan 16, 2017, 22:40 UTC · lore

Hello.

Apparently various GIT_* environment variables (most interesting is GIT_DIR but AFAIR there were more) leak to shell session launched from Git Gui's "Git Bash" menu item. Which can cause unexpected results if user navigates in that shell to some other repository (in can also include starting another terminal window etc. to make user forget original context) and tries to do something there.

I think is would be appropriate to clean up the environment while starting any program from git gui. It may have downside that the environment could be set intentionally, and now is's removed, but (1) there seems to be no way to distinguish intentionally vs automatically set GIT_DIR, and (2) the spawned program may be intended to be used independly even if the environment was set intentionally for git gui.

But I would like to note that I find it wrong for the "git" dispatcher to set up any enrironment to the spawned command-specific executable. Because it mixes the concerns of that variables - are they user-settable variables to tweak the git's behavior, or internal ones to transport the discovered paths and other info to scripts which cannot do the standard discovery? Not clear. There has been bugs related to that already, for example [1].

[1] https://public-inbox.org/git/1409784120-2228-1-git-send-email-max@max630.net/

-- 
Max

Johannes Schindelin· Jan 17, 2017, 10:52 UTC · re: Max Kirillov · lore

Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

Hi Max,

On Tue, 17 Jan 2017, Max Kirillov wrote:

> Apparently various GIT_* environment variables (most
> interesting is GIT_DIR but AFAIR there were more) leak to
> shell session launched from Git Gui's "Git Bash" menu item.

Given that you call it from Git GUI, i.e. for a *specific* worktree, I do not think that this bug is *all* that critical. After all, you won't even notice unless you use the very same Git Bash to navigate to a *different* worktree.

Having said that, if you have the time and energy to come up with a patch, I will definitely try my best to help you get it integrated.

And having said *that*, I have to admit that I was unable to reproduce: GIT_DIR was set neither when starting Git GUI from the Start Menu (and then navigating to a previously-opened worktree because I am too lazy to navigate manually), nor when starting Git GUI from a Git CMD in a worktree via `git gui`. In both cases, the environment contained only the following variables whose name starts with "GIT_": $GIT_ASKPASS, $GIT_ASK_YESNO and $GIT_EXEC_PATH.

I tested with Git for Windows v2.11.0(3), the latest offical version.

Thanks, Johannes

Max Kirillov· Jan 17, 2017, 18:42 UTC · re: Johannes Schindelin · lore

Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

On Tue, Jan 17, 2017 at 11:52:29AM +0100, Johannes Schindelin wrote:

Show 7 quoted lines

> Hi Max,
> 
> On Tue, 17 Jan 2017, Max Kirillov wrote:
> 
>> Apparently various GIT_* environment variables (most
>> interesting is GIT_DIR but AFAIR there were more) leak to
>> shell session launched from Git Gui's "Git Bash" menu item.

...

> After all, you won't even notice unless you use the very
> same Git Bash to navigate to a *different* worktree.

That's right, I haven't been noticing it myself for quite a time. But once I decided to tweak my habits to having only one terminal window opened, and immediately noticed.

> And having said *that*, I have to admit that I was unable to reproduce

I have found exact way to reproduce it and actually the reason. Turns out to be obvious mistake in code. PR is in github: https://github.com/git-for-windows/git/pull/1032

-- 
Max

Johannes Schindelin· Jan 18, 2017, 13:04 UTC · re: Max Kirillov · lore

Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

Hi Max,

On Tue, 17 Jan 2017, Max Kirillov wrote:

Show 7 quoted lines

> On Tue, Jan 17, 2017 at 11:52:29AM +0100, Johannes Schindelin wrote:
> 
> > And having said *that*, I have to admit that I was unable to reproduce
> 
> I have found exact way to reproduce it and actually the reason. Turns
> out to be obvious mistake in code. PR is in github:
> https://github.com/git-for-windows/git/pull/1032

As I pointed out in my review, I think that GIT_DIR is actually only set for `gitk` in the non-submodule case. So I guess if I run Git GUI, spawn gitk, and then Git Bash, it will have GIT_DIR set.

*clicketyclick*

Bingo. That reproduces the problem here.

Thanks, Johannes