From 325e9e1b74f909f7fadd198503bb4107cdb9ef3e Mon Sep 17 00:00:00 2001 From: Chris Selzo Date: Tue, 5 May 2026 16:04:32 -0700 Subject: [PATCH] Refactor ssh option passing ai-assisted=yes [TNZ-94640] Signed-off-by: Aram Price Signed-off-by: Chris Selzo --- ssh/ssh_args.go | 12 +++-- ssh/ssh_args_test.go | 110 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/ssh/ssh_args.go b/ssh/ssh_args.go index 2288a93fa..dd298c066 100644 --- a/ssh/ssh_args.go +++ b/ssh/ssh_args.go @@ -56,7 +56,7 @@ func NewSSHArgs(connOpts ConnectionOpts, result boshdir.SSHResult, forceTTY bool } func (a SSHArgs) LoginForHost(host boshdir.Host) []string { - return []string{host.Host, "-l", host.Username} + return []string{"-l", host.Username, "--", host.Host} } func formProxyOpt(existenceChecker cmdExistenceChecker, proxyHostString string) string { @@ -71,6 +71,12 @@ func fmtAsProxyCommandOpt(command, proxyHost string) string { return fmt.Sprintf("ProxyCommand=%s", fmt.Sprintf(command, proxyHost)) } +// shellQuote wraps s in POSIX single-quotes, escaping any embedded single-quote +// with the standard '\” idiom. +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + func (a SSHArgs) OptsForHost(host boshdir.Host) []string { // Options are used for both ssh and scp cmdOpts := []string{} @@ -132,8 +138,8 @@ func (a SSHArgs) OptsForHost(host boshdir.Host) []string { // Always force TTY for gateway ssh "ProxyCommand=ssh -tt -W %s -l %s %s %s", proxyHostPortTmpl, - gwUsername, - gwHost, + shellQuote(gwUsername), + shellQuote(gwHost), strings.Join(gwCmdOpts, " "), ) diff --git a/ssh/ssh_args_test.go b/ssh/ssh_args_test.go index 28421e445..ceec5e1f4 100644 --- a/ssh/ssh_args_test.go +++ b/ssh/ssh_args_test.go @@ -44,13 +44,13 @@ var _ = Describe("SSHArgs", func() { return SSHArgs{}.LoginForHost(host) } - It("returns login details with IPv4", func() { - Expect(act()).To(Equal([]string{"127.0.0.1", "-l", "user"})) + It("returns login details with IPv4, placing '--' before host to stop option parsing", func() { + Expect(act()).To(Equal([]string{"-l", "user", "--", "127.0.0.1"})) }) - It("returns login details with IPv6 non-bracketed", func() { + It("returns login details with IPv6, placing '--' before host to stop option parsing", func() { host.Host = "::1" - Expect(act()).To(Equal([]string{"::1", "-l", "user"})) + Expect(act()).To(Equal([]string{"-l", "user", "--", "::1"})) }) }) @@ -118,7 +118,7 @@ var _ = Describe("SSHArgs", func() { "-o", "IdentitiesOnly=yes", "-o", "IdentityFile=/tmp/priv-key", "-o", "UserKnownHostsFile=/tmp/known-hosts", - "-o", "ProxyCommand=ssh -tt -W %h:%p -l gw-user gw-host -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + "-o", "ProxyCommand=ssh -tt -W %h:%p -l 'gw-user' 'gw-host' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", })) }) @@ -135,7 +135,7 @@ var _ = Describe("SSHArgs", func() { "-o", "IdentitiesOnly=yes", "-o", "IdentityFile=/tmp/priv-key", "-o", "UserKnownHostsFile=/tmp/known-hosts", - "-o", "ProxyCommand=ssh -tt -W %h:%p -l gw-user gw-host -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o IdentitiesOnly=yes -o IdentityFile=/tmp/gw-priv-key", + "-o", "ProxyCommand=ssh -tt -W %h:%p -l 'gw-user' 'gw-host' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o IdentitiesOnly=yes -o IdentityFile=/tmp/gw-priv-key", })) }) @@ -153,7 +153,7 @@ var _ = Describe("SSHArgs", func() { "-o", "IdentitiesOnly=yes", "-o", "IdentityFile=/tmp/priv-key", "-o", "UserKnownHostsFile=/tmp/known-hosts", - "-o", "ProxyCommand=ssh -tt -W %h:%p -l user-gw-user user-gw-host -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + "-o", "ProxyCommand=ssh -tt -W %h:%p -l 'user-gw-user' 'user-gw-host' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", })) }) @@ -249,7 +249,7 @@ var _ = Describe("SSHArgs", func() { "-o", "IdentitiesOnly=yes", "-o", "IdentityFile=/tmp/priv-key", "-o", "UserKnownHostsFile=/tmp/known-hosts", - "-o", "ProxyCommand=ssh -tt -W [%h]:%p -l gw-user gw-host -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + "-o", "ProxyCommand=ssh -tt -W [%h]:%p -l 'gw-user' 'gw-host' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", })) }) @@ -264,7 +264,7 @@ var _ = Describe("SSHArgs", func() { "-o", "IdentitiesOnly=yes", "-o", "IdentityFile=/tmp/priv-key", "-o", "UserKnownHostsFile=/tmp/known-hosts", - "-o", "ProxyCommand=ssh -tt -W %h:%p -l gw-user ::1 -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + "-o", "ProxyCommand=ssh -tt -W %h:%p -l 'gw-user' '::1' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", })) }) @@ -280,10 +280,37 @@ var _ = Describe("SSHArgs", func() { "-o", "IdentitiesOnly=yes", "-o", "IdentityFile=/tmp/priv-key", "-o", "UserKnownHostsFile=/tmp/known-hosts", - "-o", "ProxyCommand=ssh -tt -W [%h]:%p -l gw-user ::1 -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + "-o", "ProxyCommand=ssh -tt -W [%h]:%p -l 'gw-user' '::1' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", })) }) + It("single-quotes gateway username", func() { + result.GatewayUsername = "vcap; exit 1" + result.GatewayHost = "gw-host" + + Expect(act()).To(ContainElement( + "ProxyCommand=ssh -tt -W %h:%p -l 'vcap; exit 1' 'gw-host' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + )) + }) + + It("single-quotes gateway host", func() { + result.GatewayUsername = "gw-user" + result.GatewayHost = "x; exit 1 #" + + Expect(act()).To(ContainElement( + "ProxyCommand=ssh -tt -W %h:%p -l 'gw-user' 'x; exit 1 #' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + )) + }) + + It("escapes embedded single-quotes in gateway fields using the POSIX '\\'' idiom", func() { + result.GatewayUsername = "gw-user" + result.GatewayHost = "host'with'quotes" + + Expect(act()).To(ContainElement( + "ProxyCommand=ssh -tt -W %h:%p -l 'gw-user' 'host'\\''with'\\''quotes' -o ServerAliveInterval=30 -o ForwardAgent=no -o ClearAllForwardings=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + )) + }) + It("returns ssh options non-bracketed if host is IPv6 and SOCKS5Proxy is set", func() { host = boshdir.Host{Host: "::1", Username: "user"} connOpts.SOCKS5Proxy = "socks5://some-proxy" @@ -299,4 +326,67 @@ var _ = Describe("SSHArgs", func() { })) }) }) + + Describe("combined OptsForHost + LoginForHost assembly", func() { + buildArgs := func() []string { + args := SSHArgs{ + ConnOpts: connOpts, + Result: result, + ForceTTY: forceTTY, + PrivKeyFile: privKeyFile, + KnownHostsFile: knownHostsFile, + CmdExistenceChecker: connectProxyCmdRunner, + Socks5Proxy: proxy.NewSocks5Proxy(proxy.NewHostKey(), log.New(io.Discard, "", log.LstdFlags), 1*time.Minute), + } + return append(args.OptsForHost(host), args.LoginForHost(host)...) + } + + It("places '--' immediately before the host in the combined argument slice", func() { + combined := buildArgs() + + separatorIdx := -1 + for i, arg := range combined { + if arg == "--" { + separatorIdx = i + break + } + } + + Expect(separatorIdx).To(BeNumerically(">", 0), "\"--\" must be present in the combined argument slice") + Expect(combined[separatorIdx+1]).To(Equal(host.Host), "host must be the element immediately after \"--\"") + }) + + It("places all -o flags before '--'", func() { + combined := buildArgs() + + separatorIdx := -1 + for i, arg := range combined { + if arg == "--" { + separatorIdx = i + break + } + } + Expect(separatorIdx).To(BeNumerically(">", 0)) + + for i, arg := range combined[:separatorIdx] { + if arg == "-o" { + Expect(i).To(BeNumerically("<", separatorIdx), + "\"-o\" at index %d must appear before \"--\" at index %d", i, separatorIdx) + } + } + }) + + It("ensures '--' does not appear anywhere in OptsForHost output", func() { + args := SSHArgs{ + ConnOpts: connOpts, + Result: result, + ForceTTY: forceTTY, + PrivKeyFile: privKeyFile, + KnownHostsFile: knownHostsFile, + CmdExistenceChecker: connectProxyCmdRunner, + Socks5Proxy: proxy.NewSocks5Proxy(proxy.NewHostKey(), log.New(io.Discard, "", log.LstdFlags), 1*time.Minute), + } + Expect(args.OptsForHost(host)).NotTo(ContainElement("--")) + }) + }) })