[PATCH] git_connect: clear GIT_* environment for ssh

[Jeff King <peff@peff.net>]

On Fri, Sep 04, 2015 at 05:44:54PM -0400, Jeff King wrote:

> > Just to make sure I got you correctly, you are saying that "we
> > propagate, but that is not correct. We should stop doing so", right?
> 
> Exactly. We do not propagate config over git:// or http:// (because we
> do not share our environment). Nor do we do so over same-machine
> connections (because we explicitly clean the environment). So ssh:// is
> the odd duck.

So here is the patch I would propose. I'm fairly certain it will solve
Giuseppe's problem, though I think there is something else odd going on
here that I don't understand. I'm worried that we're papering over
another regression. Giuseppe, if you can still find time to do that
bisect, it would be helpful.

-- >8 --
Subject: git_connect: clear GIT_* environment for ssh

When we "switch" to another local repository to run the server
side of a fetch or push, we must clear the variables in
local_repo_env so that our local $GIT_DIR, etc, do not
pollute the upload-pack or receive-pack that is executing in
the "remote" repository.

We have never done so for ssh connections. For the most
part, nobody has noticed because ssh will not pass unknown
environment variables by default. However, it is not out of
the question for a user to configure ssh to pass along GIT_*
variables using SendEnv/AcceptEnv.

We can demonstrate the problem by using "git -c" on a local
command and seeing its impact on a remote repository.  This
config ends up in $GIT_CONFIG_PARAMETERS. In the local case,
the config has no impact, but in the ssh transport, it does
(our test script has a fake ssh that passes through all
environment variables; this isn't normal, but does simulate
one possible setup).

Signed-off-by: Jeff King <peff@peff.net>
---
 connect.c                     |  4 ++--
 t/t5507-remote-environment.sh | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100755 t/t5507-remote-environment.sh

diff --git a/connect.c b/connect.c
index c0144d8..962f990 100644
--- a/connect.c
+++ b/connect.c
@@ -721,6 +721,8 @@ struct child_process *git_connect(int fd[2], const char *url,
 		strbuf_addch(&cmd, ' ');
 		sq_quote_buf(&cmd, path);
 
+		/* remove repo-local variables from the environment */
+		conn->env = local_repo_env;
 		conn->in = conn->out = -1;
 		if (protocol == PROTO_SSH) {
 			const char *ssh;
@@ -778,8 +780,6 @@ struct child_process *git_connect(int fd[2], const char *url,
 			}
 			argv_array_push(&conn->args, ssh_host);
 		} else {
-			/* remove repo-local variables from the environment */
-			conn->env = local_repo_env;
 			conn->use_shell = 1;
 		}
 		argv_array_push(&conn->args, cmd.buf);
diff --git a/t/t5507-remote-environment.sh b/t/t5507-remote-environment.sh
new file mode 100755
index 0000000..e614929
--- /dev/null
+++ b/t/t5507-remote-environment.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='check environment showed to remote side of transports'
+. ./test-lib.sh
+
+test_expect_success 'set up "remote" push situation' '
+	test_commit one &&
+	git config push.default current &&
+	git init remote
+'
+
+test_expect_success 'set up fake ssh' '
+	GIT_SSH_COMMAND="f() {
+		cd \"\$TRASH_DIRECTORY\" &&
+		eval \"\$2\"
+	}; f" &&
+	export GIT_SSH_COMMAND &&
+	export TRASH_DIRECTORY
+'
+
+# due to receive.denyCurrentBranch=true
+test_expect_success 'confirm default push fails' '
+	test_must_fail git push remote
+'
+
+test_expect_success 'config does not travel over same-machine push' '
+	test_must_fail git -c receive.denyCurrentBranch=false push remote
+'
+
+test_expect_success 'config does not travel over ssh push' '
+	test_must_fail git -c receive.denyCurrentBranch=false push host:remote
+'
+
+test_done
-- 
2.5.1.812.ge796bff