Merge pull request #3783 from muchzill4/properly-support-multi-word-SSH-env

AkihiroSuda · web-flow · commit 0279010da83f · 2025-08-19T12:46:36.000+09:00
fix: Properly support multi-word executables set via `SSH` environment variable
diff --git a/cmd/limactl/copy.go b/cmd/limactl/copy.go
@@ -84,7 +84,11 @@ func copyAction(cmd *cobra.Command, args []string) error {
 		scpFlags = append(scpFlags, "-r")
 	}
 	// this assumes that ssh and scp come from the same place, but scp has no -V
-	legacySSH := sshutil.DetectOpenSSHVersion("ssh").LessThan(*semver.New("8.0.0"))
+	sshExe, err := sshutil.NewSSHExe()
+	if err != nil {
+		return err
+	}
+	legacySSH := sshutil.DetectOpenSSHVersion(sshExe).LessThan(*semver.New("8.0.0"))
 	for _, arg := range args {
 		if runtime.GOOS == "windows" {
 			if filepath.IsAbs(arg) {
@@ -135,14 +139,22 @@ func copyAction(cmd *cobra.Command, args []string) error {
 		// arguments such as ControlPath.  This is preferred as we can multiplex
 		// sessions without re-authenticating (MaxSessions permitting).
 		for _, inst := range instances {
-			sshOpts, err = sshutil.SSHOpts("ssh", inst.Dir, *inst.Config.User.Name, false, false, false, false)
+			sshExe, err := sshutil.NewSSHExe()
+			if err != nil {
+				return err
+			}
+			sshOpts, err = sshutil.SSHOpts(sshExe, inst.Dir, *inst.Config.User.Name, false, false, false, false)
 			if err != nil {
 				return err
 			}
 		}
 	} else {
 		// Copying among multiple hosts; we can't pass in host-specific options.
-		sshOpts, err = sshutil.CommonOpts("ssh", false)
+		sshExe, err := sshutil.NewSSHExe()
+		if err != nil {
+			return err
+		}
+		sshOpts, err = sshutil.CommonOpts(sshExe, false)
 		if err != nil {
 			return err
 		}
diff --git a/cmd/limactl/shell.go b/cmd/limactl/shell.go
@@ -214,13 +214,13 @@ func shellAction(cmd *cobra.Command, args []string) error {
 		)
 	}
 
-	arg0, arg0Args, err := sshutil.SSHArguments()
+	sshExe, err := sshutil.NewSSHExe()
 	if err != nil {
 		return err
 	}
 
 	sshOpts, err := sshutil.SSHOpts(
-		arg0,
+		sshExe,
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
@@ -230,7 +230,8 @@ func shellAction(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	sshArgs := sshutil.SSHArgsFromOpts(sshOpts)
+	sshArgs := append([]string{}, sshExe.Args...)
+	sshArgs = append(sshArgs, sshutil.SSHArgsFromOpts(sshOpts)...)
 	if isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()) {
 		// required for showing the shell prompt: https://stackoverflow.com/a/626574
 		sshArgs = append(sshArgs, "-t")
@@ -242,7 +243,7 @@ func shellAction(cmd *cobra.Command, args []string) error {
 	logLevel := "ERROR"
 	// For versions older than OpenSSH 8.9p, LogLevel=QUIET was needed to
 	// avoid the "Shared connection to 127.0.0.1 closed." message with -t.
-	olderSSH := sshutil.DetectOpenSSHVersion(arg0).LessThan(*semver.New("8.9.0"))
+	olderSSH := sshutil.DetectOpenSSHVersion(sshExe).LessThan(*semver.New("8.9.0"))
 	if olderSSH {
 		logLevel = "QUIET"
 	}
@@ -253,7 +254,7 @@ func shellAction(cmd *cobra.Command, args []string) error {
 		"--",
 		script,
 	}...)
-	sshCmd := exec.Command(arg0, append(arg0Args, sshArgs...)...)
+	sshCmd := exec.Command(sshExe.Exe, sshArgs...)
 	sshCmd.Stdin = os.Stdin
 	sshCmd.Stdout = os.Stdout
 	sshCmd.Stderr = os.Stderr
diff --git a/cmd/limactl/show-ssh.go b/cmd/limactl/show-ssh.go
@@ -92,8 +92,12 @@ func showSSHAction(cmd *cobra.Command, args []string) error {
 	}
 	logrus.Warnf("`limactl show-ssh` is deprecated. Instead, use `ssh -F %s %s`.",
 		filepath.Join(inst.Dir, filenames.SSHConfig), inst.Hostname)
+	sshExe, err := sshutil.NewSSHExe()
+	if err != nil {
+		return err
+	}
 	opts, err := sshutil.SSHOpts(
-		"ssh",
+		sshExe,
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
diff --git a/cmd/limactl/tunnel.go b/cmd/limactl/tunnel.go
@@ -82,13 +82,13 @@ func tunnelAction(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	arg0, arg0Args, err := sshutil.SSHArguments()
+	sshExe, err := sshutil.NewSSHExe()
 	if err != nil {
 		return err
 	}
 
 	sshOpts, err := sshutil.SSHOpts(
-		arg0,
+		sshExe,
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
@@ -98,7 +98,8 @@ func tunnelAction(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	sshArgs := sshutil.SSHArgsFromOpts(sshOpts)
+	sshArgs := append([]string{}, sshExe.Args...)
+	sshArgs = append(sshArgs, sshutil.SSHArgsFromOpts(sshOpts)...)
 	sshArgs = append(sshArgs, []string{
 		"-q", // quiet
 		"-f", // background
@@ -107,7 +108,7 @@ func tunnelAction(cmd *cobra.Command, args []string) error {
 		"-p", strconv.Itoa(inst.SSHLocalPort),
 		inst.SSHAddress,
 	}...)
-	sshCmd := exec.Command(arg0, append(arg0Args, sshArgs...)...)
+	sshCmd := exec.Command(sshExe.Exe, sshArgs...)
 	sshCmd.Stdout = stderr
 	sshCmd.Stderr = stderr
 	logrus.Debugf("executing ssh (may take a long)): %+v", sshCmd.Args)
diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go
@@ -155,8 +155,12 @@ func New(instName string, stdout io.Writer, signalCh chan os.Signal, opts ...Opt
 		return nil, err
 	}
 
+	sshExe, err := sshutil.NewSSHExe()
+	if err != nil {
+		return nil, err
+	}
 	sshOpts, err := sshutil.SSHOpts(
-		"ssh",
+		sshExe,
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go
@@ -35,29 +35,36 @@ import (
 // in place of the 'ssh' executable.
 const EnvShellSSH = "SSH"
 
-func SSHArguments() (arg0 string, arg0Args []string, err error) {
+type SSHExe struct {
+	Exe  string
+	Args []string
+}
+
+func NewSSHExe() (SSHExe, error) {
+	var sshExe SSHExe
+
 	if sshShell := os.Getenv(EnvShellSSH); sshShell != "" {
 		sshShellFields, err := shellwords.Parse(sshShell)
 		switch {
 		case err != nil:
 			logrus.WithError(err).Warnf("Failed to split %s variable into shell tokens. "+
 				"Falling back to 'ssh' command", EnvShellSSH)
 		case len(sshShellFields) > 0:
-			arg0 = sshShellFields[0]
+			sshExe.Exe = sshShellFields[0]
 			if len(sshShellFields) > 1 {
-				arg0Args = sshShellFields[1:]
+				sshExe.Args = sshShellFields[1:]
 			}
+			return sshExe, nil
 		}
 	}
 
-	if arg0 == "" {
-		arg0, err = exec.LookPath("ssh")
-		if err != nil {
-			return "", []string{""}, err
-		}
+	executable, err := exec.LookPath("ssh")
+	if err != nil {
+		return SSHExe{}, err
 	}
+	sshExe.Exe = executable
 
-	return arg0, arg0Args, nil
+	return sshExe, nil
 }
 
 type PubKey struct {
@@ -177,7 +184,7 @@ var sshInfo struct {
 //
 // The result always contains the IdentityFile option.
 // The result never contains the Port option.
-func CommonOpts(sshPath string, useDotSSH bool) ([]string, error) {
+func CommonOpts(sshExe SSHExe, useDotSSH bool) ([]string, error) {
 	configDir, err := dirnames.LimaConfigDir()
 	if err != nil {
 		return nil, err
@@ -243,7 +250,7 @@ func CommonOpts(sshPath string, useDotSSH bool) ([]string, error) {
 
 	sshInfo.Do(func() {
 		sshInfo.aesAccelerated = detectAESAcceleration()
-		sshInfo.openSSH = detectOpenSSHInfo(sshPath)
+		sshInfo.openSSH = detectOpenSSHInfo(sshExe)
 	})
 
 	if sshInfo.openSSH.GSSAPISupported {
@@ -287,12 +294,12 @@ func identityFileEntry(privateKeyPath string) (string, error) {
 }
 
 // SSHOpts adds the following options to CommonOptions: User, ControlMaster, ControlPath, ControlPersist.
-func SSHOpts(sshPath, instDir, username string, useDotSSH, forwardAgent, forwardX11, forwardX11Trusted bool) ([]string, error) {
+func SSHOpts(sshExe SSHExe, instDir, username string, useDotSSH, forwardAgent, forwardX11, forwardX11Trusted bool) ([]string, error) {
 	controlSock := filepath.Join(instDir, filenames.SSHSock)
 	if len(controlSock) >= osutil.UnixPathMax {
 		return nil, fmt.Errorf("socket path %q is too long: >= UNIX_PATH_MAX=%d", controlSock, osutil.UnixPathMax)
 	}
-	opts, err := CommonOpts(sshPath, useDotSSH)
+	opts, err := CommonOpts(sshExe, useDotSSH)
 	if err != nil {
 		return nil, err
 	}
@@ -361,27 +368,29 @@ var (
 	openSSHInfosRW sync.RWMutex
 )
 
-func detectOpenSSHInfo(ssh string) openSSHInfo {
+func detectOpenSSHInfo(sshExe SSHExe) openSSHInfo {
 	var (
 		info   openSSHInfo
 		exe    sshExecutable
 		stderr bytes.Buffer
 	)
-	path, err := exec.LookPath(ssh)
-	if err != nil {
-		logrus.Warnf("failed to find ssh executable: %v", err)
-	} else {
-		st, _ := os.Stat(path)
-		exe = sshExecutable{Path: path, Size: st.Size(), ModTime: st.ModTime()}
+	// Note: For SSH wrappers like "kitten ssh", os.Stat will check the wrapper
+	// executable (kitten) instead of the underlying ssh binary. This means
+	// cache invalidation won't work properly - ssh upgrades won't be detected
+	// since kitten's size/mtime won't change. This is probably acceptable.
+	if st, err := os.Stat(sshExe.Exe); err == nil {
+		exe = sshExecutable{Path: sshExe.Exe, Size: st.Size(), ModTime: st.ModTime()}
 		openSSHInfosRW.RLock()
 		info := openSSHInfos[exe]
 		openSSHInfosRW.RUnlock()
 		if info != nil {
 			return *info
 		}
 	}
+	sshArgs := append([]string{}, sshExe.Args...)
 	// -V should be last
-	cmd := exec.Command(path, "-o", "GSSAPIAuthentication=no", "-V")
+	sshArgs = append(sshArgs, "-o", "GSSAPIAuthentication=no", "-V")
+	cmd := exec.Command(sshExe.Exe, sshArgs...)
 	cmd.Stderr = &stderr
 	if err := cmd.Run(); err != nil {
 		logrus.Warnf("failed to run %v: stderr=%q", cmd.Args, stderr.String())
@@ -398,8 +407,8 @@ func detectOpenSSHInfo(ssh string) openSSHInfo {
 	return info
 }
 
-func DetectOpenSSHVersion(ssh string) semver.Version {
-	return detectOpenSSHInfo(ssh).Version
+func DetectOpenSSHVersion(sshExe SSHExe) semver.Version {
+	return detectOpenSSHInfo(sshExe).Version
 }
 
 // detectValidPublicKey returns whether content represent a public key.
