Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2017, 01:08 UTC
Message-ID: <xmqq4m1911mf.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqy3ym1dsc.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 11 quoted lines

> I suspect that this will break when GIT_SSH_COMMAND, which is meant
> to be processed by the shell, hence the user can write anything,
> begins with a one-shot environment variable assignment, e.g.
>
> 	[core] sshcommand = VAR1=VAL1 VAR2=VAL2 //path/to/tortoiseplink
>
> One possible unintended side effect of this patch is when VAL1 ends
> with /plink (or /tortoiseplink) and the command is not either of
> these, in which case the "tortoiseplink" and "putty" variables will
> tweak the command line for an SSH implementation that does not want
> such a tweak to be made.  There may be other unintended fallouts.

Thinking about this further, as the sshcommand (or GIT_SSH_COMMAND) interface takes any shell-interpretable commands, the first "token" you see is not limited to a one-shot environment assignment. It could even be "cmd1 && cmd2 && ..." or even:

	if test -f ${TPLINK:=//path/to/tortoiseplink}
	then
		exec "$TPLINK" "$@"
	elif test -f ${PLINK:=//path/to/plink}
		exec "$PLINK" "$@"
	else
		echo >&2 no usable ssh on this host
	fi

Worse, the above may be in "myssh" script found on user's PATH and then core.sshcommand may be set to

	TPLINK=//my/path/to/tortoiseplink PLINK=//my/path/to/plink myssh

in the configuration the user uses on multiple hosts, some have tortoiseplink installed and some do not. The idea the user who set it up may be to use whatever available depending on the host.

The posted patch would be confused that we are using tortoiseplink but the "myssh" script would correctly notice that on a particular host it is unavailable and choose to use plink instead.

Show 6 quoted lines

> Sorry, no concrete suggestion to get this to work comes to my mind
> offhand. 
>
> Perhaps give an explicit way to force "tortoiseplink" and "putty"
> variables without looking at and guessing from the pathname, so that
> the solution does not have to split and peek the command line?

A user with such an elaborate set-up with multiple hosts with different configurations would likely to want to say "Just give me a regular SSH command line, and in my 'myssh' script I'll futz with -p/-P differences and such, as I'm writing a small script to cope with Tortoiseplink and Puttyplink anyway". With a separate, explicit configuration variable to tell Git what variant of SSH you have, even such a user can be served (she would just set ssh.variant to "vanilla" or whatever that is not "tortoiseplink" or "plink").