Skip to content

Commit

Permalink
Merge branch 'jk/connect-clear-env' into maint
Browse files Browse the repository at this point in the history
The ssh transport, just like any other transport over the network,
did not clear GIT_* environment variables, but it is possible to
use SendEnv and AcceptEnv to leak them to the remote invocation of
Git, which is not a good idea at all.  Explicitly clear them just
like we do for the local transport.

* jk/connect-clear-env:
  git_connect: clarify conn->use_shell flag
  git_connect: clear GIT_* environment for ssh
  • Loading branch information
Junio C Hamano committed Oct 16, 2015
2 parents 14d5a3e + a48b409 commit a3bbfe5
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 10 deletions.
25 changes: 15 additions & 10 deletions connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -724,10 +724,13 @@ 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->use_shell = 1;
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
const char *ssh;
int putty, tortoiseplink = 0;
int putty = 0, tortoiseplink = 0;
char *ssh_host = hostandport;
const char *port = NULL;
transport_check_allowed("ssh");
Expand All @@ -750,13 +753,17 @@ struct child_process *git_connect(int fd[2], const char *url,
}

ssh = getenv("GIT_SSH_COMMAND");
if (ssh) {
conn->use_shell = 1;
putty = 0;
} else {
if (!ssh) {
const char *base;
char *ssh_dup;

/*
* GIT_SSH is the no-shell version of
* GIT_SSH_COMMAND (and must remain so for
* historical compatibility).
*/
conn->use_shell = 0;

ssh = getenv("GIT_SSH");
if (!ssh)
ssh = "ssh";
Expand All @@ -766,8 +773,9 @@ struct child_process *git_connect(int fd[2], const char *url,

tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
!strcasecmp(base, "tortoiseplink.exe");
putty = !strcasecmp(base, "plink") ||
!strcasecmp(base, "plink.exe") || tortoiseplink;
putty = tortoiseplink ||
!strcasecmp(base, "plink") ||
!strcasecmp(base, "plink.exe");

free(ssh_dup);
}
Expand All @@ -782,9 +790,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;
transport_check_allowed("file");
}
argv_array_push(&conn->args, cmd.buf);
Expand Down
34 changes: 34 additions & 0 deletions t/t5507-remote-environment.sh
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a3bbfe5

Please sign in to comment.