Skip to content

fix: Properly support multi-word executables set via SSH environment variable #3783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix: Properly support multi-word executables set via SSH environment variable #3783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d7a4283
fix: properly support multi-word SSH commands
muchzill4 Aug 1, 2025
a0e38e6
refactor: centralize SSH executable creation
muchzill4 Aug 1, 2025
5c3d467
fix: remove redundant LookPath call in detectOpenSSHInfo
muchzill4 Aug 1, 2025
18cb183
fix: appease linter, ensure no mutation
muchzill4 Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions cmd/limactl/copy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand Down
11 changes: 6 additions & 5 deletions cmd/limactl/shell.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,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,
Expand All @@ -212,7 +212,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")
Expand All @@ -224,7 +225,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"
}
Expand All @@ -235,7 +236,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
Expand Down
6 changes: 5 additions & 1 deletion cmd/limactl/show-ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions cmd/limactl/tunnel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion pkg/hostagent/hostagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,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,
Expand Down
55 changes: 32 additions & 23 deletions pkg/sshutil/sshutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change needed?

Copy link
Author

@muchzill4 muchzill4 Aug 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming you mean the introduction of NewSSHExe()? If so, then the idea behind it was to introduce a single, validated entry point for SSHExe instances.

The main motivation is ensuring pre-flight checks, such as exec.LookPath("ssh") has happened. This gives us (more) confidence that the instance you're working with is functional.

Without that, different parts of the code would create SSHExe using a literal, which would bypass custom environment variable or the pre-flight check.

As a bonus, this also cleaned up redundant validation. Previously detectOpenSSHInfo was redundantly calling exec.LookPath(), though the same check might've already happened in the previous version of this function (when it was called SSHArguments).


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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
Expand All @@ -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.
Expand Down
Loading