From 7990ab17a4ec9b039df61f3e08873f2c687379e5 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Mon, 27 Aug 2018 17:39:07 +0000 Subject: [PATCH] lfsapi: pass GIT_SSH_COMMAND to the shell Git documents GIT_SSH_COMMAND as being passed to the shell. However, we were not doing that, leading to unexpected behavior. Determine if we're getting the SSH command from GIT_SSH_COMMAND and if so, handle things specially. Set a flag indicating that we need to pass data to the shell, and return this flag as well as the standard data. Add a function that formats arguments appropriate for either the shell or non-shell case. Note that we cannot do the shell formatting in sshGetExeAndArgs, since we want to append arguments to the command line and the way we do that differs between the shell and non-shell variants. Continue to parse the first part of the shell command so we know if we're using plink or tortoiseplink. Update the tests to use both sshFormatArgs and sshGetExeAndArgs together, since it no longer makes sense to test them independently of each other. --- lfsapi/ssh.go | 30 +++++++++++++++------ lfsapi/ssh_test.go | 66 ++++++++++++++++++++++++---------------------- 2 files changed, 56 insertions(+), 40 deletions(-) diff --git a/lfsapi/ssh.go b/lfsapi/ssh.go index c623fcd8..35495d7d 100644 --- a/lfsapi/ssh.go +++ b/lfsapi/ssh.go @@ -115,17 +115,29 @@ func (c *sshAuthClient) Resolve(e Endpoint, method string) (sshAuthResponse, err return res, err } +func sshFormatArgs(cmd string, args []string, needShell bool) (string, []string) { + if !needShell { + return cmd, args + } + + joined := cmd + " " + strings.Join(tools.ShellQuote(args), " ") + return "sh", []string{"-c", joined} +} + func sshGetLFSExeAndArgs(osEnv config.Environment, e Endpoint, method string) (string, []string) { - exe, args := sshGetExeAndArgs(osEnv, e) + exe, args, needShell := sshGetExeAndArgs(osEnv, e) operation := endpointOperation(e, method) args = append(args, fmt.Sprintf("git-lfs-authenticate %s %s", e.SshPath, operation)) + exe, args = sshFormatArgs(exe, args, needShell) tracerx.Printf("run_command: %s %s", exe, strings.Join(args, " ")) return exe, args } // Return the executable name for ssh on this machine and the base args // Base args includes port settings, user/host, everything pre the command to execute -func sshGetExeAndArgs(osEnv config.Environment, e Endpoint) (exe string, baseargs []string) { +func sshGetExeAndArgs(osEnv config.Environment, e Endpoint) (exe string, baseargs []string, needShell bool) { + var cmd string + isPlink := false isTortoise := false @@ -133,14 +145,19 @@ func sshGetExeAndArgs(osEnv config.Environment, e Endpoint) (exe string, basearg sshCmd, _ := osEnv.Get("GIT_SSH_COMMAND") cmdArgs := tools.QuotedFields(sshCmd) if len(cmdArgs) > 0 { + needShell = true ssh = cmdArgs[0] - cmdArgs = cmdArgs[1:] + cmd = sshCmd } if ssh == "" { ssh = defaultSSHCmd } + if cmd == "" { + cmd = ssh + } + basessh := filepath.Base(ssh) if basessh != defaultSSHCmd { @@ -152,10 +169,7 @@ func sshGetExeAndArgs(osEnv config.Environment, e Endpoint) (exe string, basearg isTortoise = strings.EqualFold(basessh, "tortoiseplink") } - args := make([]string, 0, 5+len(cmdArgs)) - if len(cmdArgs) > 0 { - args = append(args, cmdArgs...) - } + args := make([]string, 0, 7) if isTortoise { // TortoisePlink requires the -batch argument to behave like ssh/plink @@ -185,7 +199,7 @@ func sshGetExeAndArgs(osEnv config.Environment, e Endpoint) (exe string, basearg args = append(args, sshOptPrefixRE.ReplaceAllString(e.SshUserAndHost, "")) } - return ssh, args + return cmd, args, needShell } const defaultSSHCmd = "ssh" diff --git a/lfsapi/ssh_test.go b/lfsapi/ssh_test.go index 38f68160..86576733 100644 --- a/lfsapi/ssh_test.go +++ b/lfsapi/ssh_test.go @@ -265,7 +265,7 @@ func TestSSHGetExeAndArgsSsh(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) assert.Equal(t, "ssh", exe) assert.Equal(t, []string{"--", "user@foo.com"}, args) } @@ -281,7 +281,7 @@ func TestSSHGetExeAndArgsSshCustomPort(t *testing.T) { endpoint.SshUserAndHost = "user@foo.com" endpoint.SshPort = "8888" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) assert.Equal(t, "ssh", exe) assert.Equal(t, []string{"-p", "8888", "--", "user@foo.com"}, args) } @@ -298,7 +298,7 @@ func TestSSHGetExeAndArgsPlink(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) assert.Equal(t, plink, exe) assert.Equal(t, []string{"user@foo.com"}, args) } @@ -316,7 +316,7 @@ func TestSSHGetExeAndArgsPlinkCustomPort(t *testing.T) { endpoint.SshUserAndHost = "user@foo.com" endpoint.SshPort = "8888" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) assert.Equal(t, plink, exe) assert.Equal(t, []string{"-P", "8888", "user@foo.com"}, args) } @@ -333,7 +333,7 @@ func TestSSHGetExeAndArgsTortoisePlink(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) assert.Equal(t, plink, exe) assert.Equal(t, []string{"-batch", "user@foo.com"}, args) } @@ -351,7 +351,7 @@ func TestSSHGetExeAndArgsTortoisePlinkCustomPort(t *testing.T) { endpoint.SshUserAndHost = "user@foo.com" endpoint.SshPort = "8888" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) assert.Equal(t, plink, exe) assert.Equal(t, []string{"-batch", "-P", "8888", "user@foo.com"}, args) } @@ -366,9 +366,9 @@ func TestSSHGetExeAndArgsSshCommandPrecedence(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, "sshcmd", exe) - assert.Equal(t, []string{"user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", "sshcmd user@foo.com"}, args) } func TestSSHGetExeAndArgsSshCommandArgs(t *testing.T) { @@ -380,9 +380,9 @@ func TestSSHGetExeAndArgsSshCommandArgs(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, "sshcmd", exe) - assert.Equal(t, []string{"--args", "1", "user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", "sshcmd --args 1 user@foo.com"}, args) } func TestSSHGetExeAndArgsSshCommandArgsWithMixedQuotes(t *testing.T) { @@ -394,9 +394,9 @@ func TestSSHGetExeAndArgsSshCommandArgsWithMixedQuotes(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, "sshcmd", exe) - assert.Equal(t, []string{"foo", `bar "baz"`, "user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", "sshcmd foo 'bar \"baz\"' user@foo.com"}, args) } func TestSSHGetExeAndArgsSshCommandCustomPort(t *testing.T) { @@ -409,9 +409,9 @@ func TestSSHGetExeAndArgsSshCommandCustomPort(t *testing.T) { endpoint.SshUserAndHost = "user@foo.com" endpoint.SshPort = "8888" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, "sshcmd", exe) - assert.Equal(t, []string{"-p", "8888", "user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", "sshcmd -p 8888 user@foo.com"}, args) } func TestSSHGetLFSExeAndArgsWithCustomSSH(t *testing.T) { @@ -485,9 +485,10 @@ func TestSSHGetExeAndArgsInvalidOptionsAsHost(t *testing.T) { assert.Equal(t, "-oProxyCommand=gnome-calculator", e.SshUserAndHost) assert.Equal(t, "", e.SshPath) - exe, args := sshGetExeAndArgs(cli.OSEnv(), e) + exe, args, needShell := sshGetExeAndArgs(cli.OSEnv(), e) assert.Equal(t, "ssh", exe) assert.Equal(t, []string{"--", "-oProxyCommand=gnome-calculator"}, args) + assert.Equal(t, false, needShell) } func TestSSHGetExeAndArgsInvalidOptionsAsPath(t *testing.T) { @@ -503,9 +504,10 @@ func TestSSHGetExeAndArgsInvalidOptionsAsPath(t *testing.T) { assert.Equal(t, "git@git-host.com", e.SshUserAndHost) assert.Equal(t, "-oProxyCommand=gnome-calculator", e.SshPath) - exe, args := sshGetExeAndArgs(cli.OSEnv(), e) + exe, args, needShell := sshGetExeAndArgs(cli.OSEnv(), e) assert.Equal(t, "ssh", exe) assert.Equal(t, []string{"--", "git@git-host.com"}, args) + assert.Equal(t, false, needShell) } func TestParseBareSSHUrl(t *testing.T) { @@ -541,9 +543,9 @@ func TestSSHGetExeAndArgsPlinkCommand(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, plink, exe) - assert.Equal(t, []string{"user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", plink + " user@foo.com"}, args) } func TestSSHGetExeAndArgsPlinkCommandCustomPort(t *testing.T) { @@ -558,9 +560,9 @@ func TestSSHGetExeAndArgsPlinkCommandCustomPort(t *testing.T) { endpoint.SshUserAndHost = "user@foo.com" endpoint.SshPort = "8888" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, plink, exe) - assert.Equal(t, []string{"-P", "8888", "user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", plink + " -P 8888 user@foo.com"}, args) } func TestSSHGetExeAndArgsTortoisePlinkCommand(t *testing.T) { @@ -574,9 +576,9 @@ func TestSSHGetExeAndArgsTortoisePlinkCommand(t *testing.T) { endpoint := cli.Endpoints.Endpoint("download", "") endpoint.SshUserAndHost = "user@foo.com" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, plink, exe) - assert.Equal(t, []string{"-batch", "user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", plink + " -batch user@foo.com"}, args) } func TestSSHGetExeAndArgsTortoisePlinkCommandCustomPort(t *testing.T) { @@ -591,7 +593,7 @@ func TestSSHGetExeAndArgsTortoisePlinkCommandCustomPort(t *testing.T) { endpoint.SshUserAndHost = "user@foo.com" endpoint.SshPort = "8888" - exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint) - assert.Equal(t, plink, exe) - assert.Equal(t, []string{"-batch", "-P", "8888", "user@foo.com"}, args) + exe, args := sshFormatArgs(sshGetExeAndArgs(cli.OSEnv(), endpoint)) + assert.Equal(t, "sh", exe) + assert.Equal(t, []string{"-c", plink + " -batch -P 8888 user@foo.com"}, args) }