From 54fb5309508a65ac09071db8ebd7a02ac292f40b Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Sat, 25 Jul 2026 23:53:28 -0700 Subject: [PATCH 1/6] copytool: support native Windows OpenSSH On Windows, `limactl copy` converted host paths to the Cygwin/MSYS form no matter which tool would consume them, and it passed ControlMaster options unconditionally. Native OpenSSH cannot use that path form and has no multiplexing, so scp failed with "getsockname failed". The two backends need different path forms. scp takes whatever its own toolchain uses, while rsync on Windows is always Cygwin-based and reads a drive letter as a hostspec. So every path now takes the form of the binary that reads it, operands and ssh options alike, because scp hands its options to the ssh beside itself while rsync hands them to the ssh it names. Both backends now skip multiplexing on Windows, so they stop building a control socket path they never use. Signed-off-by: Jan Dubois --- cmd/limactl/shell.go | 1 - hack/test-templates.sh | 6 +- pkg/copytool/copytool.go | 39 +++++-- pkg/copytool/copytool_test.go | 168 ++++++++++++++++++++++++++++ pkg/copytool/rsync.go | 38 ++++++- pkg/copytool/scp.go | 48 +++++--- pkg/hostagent/hostagent.go | 3 +- pkg/sshutil/sshutil.go | 100 +++++++++++------ pkg/sshutil/sshutil_windows_test.go | 6 +- 9 files changed, 341 insertions(+), 68 deletions(-) diff --git a/cmd/limactl/shell.go b/cmd/limactl/shell.go index 48864650479..ef76378fb4e 100644 --- a/cmd/limactl/shell.go +++ b/cmd/limactl/shell.go @@ -386,7 +386,6 @@ func shellAction(cmd *cobra.Command, args []string) error { // - Prevents error messages such as: // > mux_client_request_session: read from master failed: Connection reset by peer // > ControlSocket ....sock already exists, disabling multiplexing - // Only remove these options when writing the SSH config file and executing `limactl shell`, since multiplexing seems to work with port forwarding. sshOpts = sshutil.SSHOptsRemovingControlPath(sshOpts) } sshArgs := append([]string{}, sshExe.Args...) diff --git a/hack/test-templates.sh b/hack/test-templates.sh index b72410245ea..b2d61572cc2 100755 --- a/hack/test-templates.sh +++ b/hack/test-templates.sh @@ -302,7 +302,11 @@ if command -v rsync >/dev/null && limactl shell "$NAME" command -v rsync >/dev/n testdir="$tmpdir/test-rsync-dir" mkdir -p "$testdir" echo "test content" >"$testdir/testfile.txt" - limactl cp --backend=rsync -r -v "$testdir" "$NAME":/tmp/ + testdir_host=$testdir + if [ "${OS_HOST}" = "Msys" ]; then + testdir_host="$(cygpath -w "$testdir")" + fi + limactl cp --backend=rsync -r -v "$testdir_host" "$NAME":/tmp/ if ! limactl shell "$NAME" test -f /tmp/test-rsync-dir/testfile.txt; then ERROR "rsync recursive copy failed" exit 1 diff --git a/pkg/copytool/copytool.go b/pkg/copytool/copytool.go index 3f36b8c4ad8..ec33adc0f13 100644 --- a/pkg/copytool/copytool.go +++ b/pkg/copytool/copytool.go @@ -15,8 +15,8 @@ import ( "github.com/sirupsen/logrus" - "github.com/lima-vm/lima/v2/pkg/fsutil" "github.com/lima-vm/lima/v2/pkg/limatype" + "github.com/lima-vm/lima/v2/pkg/sshutil" "github.com/lima-vm/lima/v2/pkg/store" ) @@ -88,7 +88,9 @@ func New(ctx context.Context, backend string, paths []string, opts *Options) (Co tool, err = newSCPTool(opts) if err != nil { - return nil, fmt.Errorf("neither rsync nor scp found on host: %w", err) + // rsync may well have been found and rejected above, so name the + // outcome rather than guessing which tool is missing. + return nil, fmt.Errorf("no usable copy tool on host: %w", err) } return tool, nil default: @@ -116,21 +118,40 @@ func hasRemoteSourceAndDestination(ctx context.Context, paths []string) bool { return hasRemoteSource && hasRemoteDestination } +// sshOptsForInstance returns the ssh options for copying to or from inst. The +// rsync availability probe and the rsync command must both use it, or the probe +// rejects a working install. On Windows it drops connection multiplexing, since +// native OpenSSH has none and Cygwin ssh's is unreliable for scp and rsync. +// +// toolPath names the binary whose path form the options take. scp hands them to +// the ssh beside itself, while rsync hands them to the ssh it names with -e. +func sshOptsForInstance(ctx context.Context, sshExe sshutil.SSHExe, toolPath string, inst *limatype.Instance) ([]string, error) { + if runtime.GOOS == "windows" { + return sshutil.SSHOptsWithoutMultiplexing(ctx, sshExe, toolPath, *inst.Config.User.Name, false) + } + return sshutil.SSHOpts(ctx, sshExe, inst.Dir, *inst.Config.User.Name, false, false, false, false) +} + func parseCopyPaths(ctx context.Context, paths []string) ([]*Path, error) { var copyPaths []*Path for _, path := range paths { cp := &Path{} if runtime.GOOS == "windows" { + // Classify absolute paths before the ":" split, so a drive letter + // is not read as an instance name. Drive-relative "C:foo" is not + // absolute, so single-letter instance names still work. + // + // The path stays in native form here. scp and rsync can come from + // different toolchains, so each backend formats it for the binary + // that consumes it. if filepath.IsAbs(path) { - var err error - path, err = fsutil.WindowsSubsystemPath(ctx, path) - if err != nil { - return nil, err - } - } else { - path = filepath.ToSlash(path) + cp.Path = path + cp.IsRemote = false + copyPaths = append(copyPaths, cp) + continue } + path = filepath.ToSlash(path) } parts := strings.SplitN(path, ":", 2) diff --git a/pkg/copytool/copytool_test.go b/pkg/copytool/copytool_test.go index 939ded0f07f..1a29333ffbd 100644 --- a/pkg/copytool/copytool_test.go +++ b/pkg/copytool/copytool_test.go @@ -4,10 +4,19 @@ package copytool import ( + "os" + "path/filepath" + "runtime" "slices" + "strings" "testing" "gotest.tools/v3/assert" + + "github.com/lima-vm/lima/v2/pkg/limatype" + "github.com/lima-vm/lima/v2/pkg/limatype/dirnames" + "github.com/lima-vm/lima/v2/pkg/limatype/filenames" + "github.com/lima-vm/lima/v2/pkg/sshutil" ) // TestCommandDoesNotMutateOptions verifies that passing opts to Command() does not @@ -44,3 +53,162 @@ func TestRsyncCommandEndsOptionParsing(t *testing.T) { assert.Assert(t, sep != -1, "rsync args must contain the %#q separator: %v", "--", cmd.Args) assert.Assert(t, slices.Index(cmd.Args, dashPath) > sep, "path %#q must come after %#q: %v", dashPath, "--", cmd.Args) } + +// TestParseCopyPathsWindowsDriveLetter locks in the split between Windows +// absolute paths, which are local, and drive-relative paths like "C:foo.txt", +// which must stay instance "C" so single-letter instance names keep working. +func TestParseCopyPathsWindowsDriveLetter(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows-only path handling") + } + t.Setenv("LIMA_HOME", t.TempDir()) + ctx := t.Context() + + // Absolute drive-letter paths are local, and keep the native form the + // backends convert for their own tool. + for _, p := range []string{`C:\foo`, `C:/foo`} { + cps, err := parseCopyPaths(ctx, []string{p}) + assert.NilError(t, err) + assert.Equal(t, len(cps), 1) + assert.Equal(t, cps[0].IsRemote, false, "%q must be a local path", p) + assert.Equal(t, cps[0].Path, p, "%q must reach the backend unconverted", p) + } + + // "C:foo" is drive-relative (not absolute). It must reach the + // instance-lookup branch as instance "C" path "foo" and fail at + // store.Inspect with an instance-not-found error. + _, err := parseCopyPaths(ctx, []string{"C:foo"}) + assert.ErrorContains(t, err, "instance `C`") + assert.ErrorContains(t, err, "does not exist") + + // Explicit instance:path behaves the same. + _, err = parseCopyPaths(ctx, []string{"nonexistent-instance-for-test:/tmp/x"}) + assert.ErrorContains(t, err, "instance `nonexistent-instance-for-test`") + assert.ErrorContains(t, err, "does not exist") +} + +// looksRemoteToRsync reports whether rsync would read arg as host:path, which it +// does for any colon before the first slash. +func looksRemoteToRsync(arg string) bool { + colon := strings.Index(arg, ":") + slash := strings.Index(arg, "/") + return colon >= 0 && (slash < 0 || colon < slash) +} + +// TestRsyncCommandLocalPathForm locks in that local operands reach rsync as +// paths. Windows has no native rsync, so a drive letter would be read as a +// hostspec and the transfer would fail. +func TestRsyncCommandLocalPathForm(t *testing.T) { + // Nothing beside this path, so the conversion takes its in-process + // fallback and the expected operand holds with or without a Cygwin install. + tool := &rsyncTool{toolPath: filepath.Join(t.TempDir(), "rsync"), Options: &Options{}} + + src, dst := "/tmp/src", "/tmp/dst" + wantSrc, wantDst := src, dst + if runtime.GOOS == "windows" { + src, dst = `C:\src`, `C:\dst` + wantSrc, wantDst = "/c/src", "/c/dst" + } + + cmd, err := tool.Command(t.Context(), []string{src, dst}, nil) + assert.NilError(t, err) + + operands := cmd.Args[len(cmd.Args)-2:] + assert.Equal(t, operands[0], wantSrc) + assert.Equal(t, operands[1], wantDst) + for _, arg := range operands { + assert.Assert(t, !looksRemoteToRsync(arg), "rsync reads %q as host:path", arg) + } +} + +// TestSCPCommandLocalPathForm locks in that local operands reach scp in the +// form its own toolchain reads. Native Windows OpenSSH takes a drive letter, +// which a Cygwin scp would read as a hostspec instead. +func TestSCPCommandLocalPathForm(t *testing.T) { + limaHome := t.TempDir() + t.Setenv("LIMA_HOME", limaHome) + // CommonOpts requires the internal identity to exist. + configDir := filepath.Join(limaHome, "_config") + assert.NilError(t, os.MkdirAll(configDir, 0o700)) + assert.NilError(t, os.WriteFile(filepath.Join(configDir, filenames.UserPrivateKey), nil, 0o600)) + + // Nothing beside this path, so the conversion takes its native fallback and + // the expected operand holds with or without a Cygwin install. + tool := &scpTool{ + toolPath: filepath.Join(t.TempDir(), "scp"), + sshExe: sshutil.SSHExe{Exe: "ssh"}, + Options: &Options{}, + } + + src, dst := "/tmp/src", "/tmp/dst" + wantSrc, wantDst := src, dst + if runtime.GOOS == "windows" { + src, dst = `C:\src`, `C:\dst` + wantSrc, wantDst = "C:/src", "C:/dst" + } + + // Two local operands, so no instance lookup and no host-specific options. + cmd, err := tool.Command(t.Context(), []string{src, dst}, nil) + assert.NilError(t, err) + + operands := cmd.Args[len(cmd.Args)-2:] + assert.Equal(t, operands[0], wantSrc) + assert.Equal(t, operands[1], wantDst) + + // scp hands IdentityFile to the ssh beside itself, so it must carry scp's + // path form, which is not always the form of the ssh Lima selected. + var identityFile string + for _, arg := range cmd.Args { + if strings.HasPrefix(arg, "IdentityFile=") { + identityFile = arg + } + } + assert.Assert(t, identityFile != "", "no IdentityFile option: %v", cmd.Args) + if runtime.GOOS == "windows" { + // Pin the whole value: the Cygwin form carries no backslash either, so + // a laxer check would pass even if the form followed sshExe again. + // LimaConfigDir resolves symlinks, which on Windows runners also expands + // the 8.3 temp path, so build the expectation from the same helper. + resolvedConfigDir, err := dirnames.LimaConfigDir() + assert.NilError(t, err) + want := "IdentityFile='" + filepath.ToSlash(filepath.Join(resolvedConfigDir, filenames.UserPrivateKey)) + "'" + assert.Equal(t, identityFile, want) + } +} + +// TestSSHOptsForInstanceMultiplexing locks in the multiplexing policy shared by +// the rsync availability probe and the rsync and scp commands. +func TestSSHOptsForInstanceMultiplexing(t *testing.T) { + limaHome := t.TempDir() + t.Setenv("LIMA_HOME", limaHome) + // CommonOpts requires the internal identity to exist. + configDir := filepath.Join(limaHome, "_config") + assert.NilError(t, os.MkdirAll(configDir, 0o700)) + assert.NilError(t, os.WriteFile(filepath.Join(configDir, filenames.UserPrivateKey), nil, 0o600)) + + name := "test-instance" + inst := &limatype.Instance{ + // Only the control socket path derives from Dir, and nothing opens it. + // Keep it short: SSHOpts rejects a socket path over UNIX_PATH_MAX, which + // the temp directory on macOS exceeds on its own. + Dir: filepath.Join("/tmp", name), + Config: &limatype.LimaYAML{User: limatype.User{Name: &name}}, + } + + opts, err := sshOptsForInstance(t.Context(), sshutil.SSHExe{Exe: "ssh"}, "ssh", inst) + assert.NilError(t, err) + assert.Assert(t, slices.Contains(opts, "User="+name), "opts: %v", opts) + + var mux []string + for _, o := range opts { + if strings.HasPrefix(o, "ControlMaster") || strings.HasPrefix(o, "ControlPath") || strings.HasPrefix(o, "ControlPersist") { + mux = append(mux, o) + } + } + if runtime.GOOS == "windows" { + // Native OpenSSH cannot multiplex, and Cygwin ssh's is unreliable. + assert.Equal(t, len(mux), 0, "Windows must not multiplex: %v", mux) + } else { + assert.Equal(t, len(mux), 3, "other platforms multiplex: %v", opts) + } +} diff --git a/pkg/copytool/rsync.go b/pkg/copytool/rsync.go index f3c2069f505..0aa02873f3c 100644 --- a/pkg/copytool/rsync.go +++ b/pkg/copytool/rsync.go @@ -7,11 +7,14 @@ import ( "context" "fmt" "os/exec" + "path/filepath" + "runtime" "strings" "al.essio.dev/pkg/shellescape" "github.com/sirupsen/logrus" + "github.com/lima-vm/lima/v2/pkg/fsutil" "github.com/lima-vm/lima/v2/pkg/limatype" "github.com/lima-vm/lima/v2/pkg/sshutil" ) @@ -63,7 +66,7 @@ func checkRsyncOnGuest(ctx context.Context, inst *limatype.Instance) bool { logrus.Debugf("failed to create SSH executable: %v", err) return false } - sshOpts, err := sshutil.SSHOpts(ctx, sshExe, inst.Dir, *inst.Config.User.Name, false, false, false, false) + sshOpts, err := sshOptsForInstance(ctx, sshExe, sshExe.Exe, inst) if err != nil { logrus.Debugf("failed to get SSH options for rsync check: %v", err) return false @@ -82,12 +85,43 @@ func checkRsyncOnGuest(ctx context.Context, inst *limatype.Instance) bool { return err == nil } +// pathForRsync formats a host path for the rsync binary t will run. Windows +// has no native rsync build, so any rsync is Cygwin- or MSYS-based and reads a +// colon before the first slash as host:path; the sibling cygpath applies that +// install's own fstab. Without that sibling the conversion falls back to the +// MSYS form, which a stock Cygwin rsync resolves under its own install root +// rather than the drive. +func (t *rsyncTool) pathForRsync(ctx context.Context, orig string) (string, error) { + if runtime.GOOS != "windows" || !filepath.IsAbs(orig) { + return orig, nil + } + // Resolve here rather than in newRsyncTool, which would also change the + // binary exec runs and the argv[0] it sees. + toolPath := t.toolPath + if resolved, err := filepath.EvalSymlinks(toolPath); err == nil { + toolPath = resolved + } + cygpathExe := filepath.Join(filepath.Dir(toolPath), "cygpath.exe") + return fsutil.WindowsSubsystemPathWithCygpath(ctx, cygpathExe, orig) +} + func (t *rsyncTool) Command(ctx context.Context, paths []string, opts *Options) (*exec.Cmd, error) { copyPaths, err := parseCopyPaths(ctx, paths) if err != nil { return nil, err } + // Format before the trailing-slash handling below, which decides whether + // rsync copies a directory or its contents. + for _, cp := range copyPaths { + if cp.IsRemote { + continue + } + if cp.Path, err = t.pathForRsync(ctx, cp.Path); err != nil { + return nil, err + } + } + effectiveOpts := t.Options if opts != nil { effectiveOpts = opts @@ -123,7 +157,7 @@ func (t *rsyncTool) Command(ctx context.Context, paths []string, opts *Options) if err != nil { return nil, err } - sshOpts, err := sshutil.SSHOpts(ctx, sshExe, cp.Instance.Dir, *cp.Instance.Config.User.Name, false, false, false, false) + sshOpts, err := sshOptsForInstance(ctx, sshExe, sshExe.Exe, cp.Instance) if err != nil { return nil, err } diff --git a/pkg/copytool/scp.go b/pkg/copytool/scp.go index db5a63248c7..39a81570705 100644 --- a/pkg/copytool/scp.go +++ b/pkg/copytool/scp.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "os/exec" + "path/filepath" "github.com/coreos/go-semver/semver" @@ -17,15 +18,22 @@ import ( type scpTool struct { toolPath string + sshExe sshutil.SSHExe Options *Options } func newSCPTool(opts *Options) (*scpTool, error) { - path, err := exec.LookPath("scp") + // scp must come from the same toolchain as the ssh whose path form it + // receives, and NewSSHExe can select an ssh that PATH would not. + sshExe, err := sshutil.NewSSHExe() + if err != nil { + return nil, fmt.Errorf("ssh not found on host: %w", err) + } + path, err := exec.LookPath(sshutil.CompanionForSSH(sshExe, "scp")) if err != nil { return nil, fmt.Errorf("scp not found on host: %w", err) } - return &scpTool{toolPath: path, Options: opts}, nil + return &scpTool{toolPath: path, sshExe: sshExe, Options: opts}, nil } func (t *scpTool) Name() string { @@ -66,12 +74,22 @@ func (t *scpTool) Command(ctx context.Context, paths []string, opts *Options) (* scpFlags = append(scpFlags, effectiveOpts.AdditionalArgs...) } - // this assumes that ssh and scp come from the same place, but scp has no -V - sshExeForVersion, err := sshutil.NewSSHExe() - if err != nil { - return nil, err + // scp has no -V, so the version comes from t.sshExe, which is scp's own + // toolchain unless that install shipped no scp. + legacySSH := sshutil.DetectOpenSSHVersion(ctx, t.sshExe).LessThan(*semver.New("8.0.0")) + + // Only absolute paths carry a drive letter to convert; a relative path is + // already in a form both toolchains accept. Key the form to scp itself, + // which parses these operands: it usually comes from the same toolchain as + // t.sshExe, but falls back to PATH when that install ships no scp. + for _, cp := range copyPaths { + if cp.IsRemote || !filepath.IsAbs(cp.Path) { + continue + } + if cp.Path, err = sshutil.PathForTool(ctx, t.toolPath, cp.Path); err != nil { + return nil, err + } } - legacySSH := sshutil.DetectOpenSSHVersion(ctx, sshExeForVersion).LessThan(*semver.New("8.0.0")) for _, cp := range copyPaths { if cp.IsRemote { @@ -98,24 +116,18 @@ func (t *scpTool) Command(ctx context.Context, paths []string, opts *Options) (* if len(instances) == 1 { // Only one (instance) host is involved; we can use the instance-specific // arguments such as ControlPath. This is preferred as we can multiplex - // sessions without re-authenticating (MaxSessions permitting). + // sessions without re-authenticating (MaxSessions permitting), except on + // Windows. for _, inst := range instances { - sshExe, err := sshutil.NewSSHExe() - if err != nil { - return nil, err - } - sshOpts, err = sshutil.SSHOpts(ctx, sshExe, inst.Dir, *inst.Config.User.Name, false, false, false, false) + sshOpts, err = sshOptsForInstance(ctx, t.sshExe, t.toolPath, inst) if err != nil { return nil, err } } } else { // Copying among multiple hosts; we can't pass in host-specific options. - sshExe, err := sshutil.NewSSHExe() - if err != nil { - return nil, err - } - sshOpts, err = sshutil.CommonOpts(ctx, sshExe, false) + // CommonOpts carries no multiplexing options to begin with. + sshOpts, err = sshutil.CommonOpts(ctx, t.sshExe, t.toolPath, false) if err != nil { return nil, err } diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go index 80ff71d9115..ed4502aa2b1 100644 --- a/pkg/hostagent/hostagent.go +++ b/pkg/hostagent/hostagent.go @@ -302,7 +302,7 @@ func writeSSHConfigFile(sshPath, instName, instDir, instSSHAddress string, sshLo // - Prevents error messages such as: // > mux_client_request_session: read from master failed: Connection reset by peer // > ControlSocket ....sock already exists, disabling multiplexing - // Only remove these options when writing the SSH config file and executing `limactl shell`, since multiplexing seems to work with port forwarding. + // The sshConfig the hostagent builds from the same options keeps them. sshOpts = sshutil.SSHOptsRemovingControlPath(sshOpts) } if err := sshutil.Format(b, sshPath, instName, sshutil.FormatConfig, @@ -571,7 +571,6 @@ func (a *HostAgent) startHostAgentRoutines(ctx context.Context) error { } a.cleanUp(func() error { // Skip ExitMaster when the control socket does not exist. - // On Windows, the ControlMaster is used only for SSH port forwarding. if !sshutil.IsControlMasterExisting(a.instDir) { return nil } diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index 9830cb635b9..c121a8901a4 100644 --- a/pkg/sshutil/sshutil.go +++ b/pkg/sshutil/sshutil.go @@ -133,24 +133,24 @@ func missingSiblings(dir string, siblings ...string) []string { } var ( - // cygwinDetectCache maps a resolved ssh path to its sibling cygpath.exe, - // or "" when that ssh is not Cygwin-based. Each path is probed once. + // cygwinDetectCache maps a resolved tool path to its sibling cygpath.exe, + // or "" when that tool is not Cygwin-based. Each path is probed once. cygwinDetectCache = map[string]string{} cygwinDetectCacheRW sync.RWMutex ) -// resolvedSSHPath returns the absolute, symlink-resolved path of sshExe's -// binary — the directory Lima reads its companion tools from. It returns +// resolvedToolPath returns the absolute, symlink-resolved path of toolExe's +// binary, whose directory Lima reads its companion tools from. It returns // ("", false) when a bare name does not resolve on PATH. -func resolvedSSHPath(sshExe SSHExe) (string, bool) { - if sshExe.Exe == "" { +func resolvedToolPath(toolExe SSHExe) (string, bool) { + if toolExe.Exe == "" { return "", false } - path := sshExe.Exe + path := toolExe.Exe if !filepath.IsAbs(path) { resolved, err := exec.LookPath(path) if err != nil { - logrus.WithError(err).Debugf("cannot resolve ssh at %#q via PATH", path) + logrus.WithError(err).Debugf("cannot resolve tool at %#q via PATH", path) return "", false } path = resolved @@ -161,15 +161,15 @@ func resolvedSSHPath(sshExe SSHExe) (string, bool) { return path, true } -// companionForSSH returns tool from the same toolchain as sshExe. NewSSHExe can +// CompanionForSSH returns tool from the same toolchain as sshExe. NewSSHExe can // select an ssh outside PATH, so resolving a companion by bare name can pick a // different toolchain than the one PathForSSH formats paths for. It returns tool // unchanged on non-Windows, or when no sibling exists, leaving it to PATH. -func companionForSSH(sshExe SSHExe, tool string) string { +func CompanionForSSH(sshExe SSHExe, tool string) string { if runtime.GOOS != "windows" { return tool } - path, ok := resolvedSSHPath(sshExe) + path, ok := resolvedToolPath(sshExe) if !ok { return tool } @@ -181,16 +181,17 @@ func companionForSSH(sshExe SSHExe, tool string) string { return companion } -// cygpathForSSH returns the cygpath.exe beside sshExe (resolved through -// symlinks) and whether sshExe is Cygwin-based. Callers pass the returned -// path to exec.Command so conversions run through that toolchain's own -// cygpath, even when $SSH points outside PATH. It returns -// ("", false) on non-Windows or empty input; results are cached per resolved path. -func cygpathForSSH(sshExe SSHExe) (string, bool) { +// cygpathForSSH returns the cygpath.exe beside toolExe (resolved through +// symlinks) and whether toolExe is Cygwin-based. PathForTool passes non-ssh +// binaries here too. Callers pass the returned path to exec.Command so +// conversions run through that toolchain's own cygpath, even when $SSH points +// outside PATH. It returns ("", false) on non-Windows or empty input; results +// are cached per resolved path. +func cygpathForSSH(toolExe SSHExe) (string, bool) { if runtime.GOOS != "windows" { return "", false } - path, ok := resolvedSSHPath(sshExe) + path, ok := resolvedToolPath(toolExe) if !ok { return "", false } @@ -202,10 +203,10 @@ func cygpathForSSH(sshExe SSHExe) (string, bool) { } cygpathExe := filepath.Join(filepath.Dir(path), "cygpath.exe") if _, err := os.Stat(cygpathExe); err != nil { - logrus.Debugf("ssh at %#q detected as native Windows OpenSSH (no cygpath.exe alongside)", path) + logrus.Debugf("tool at %#q detected as native Windows OpenSSH (no cygpath.exe alongside)", path) cygpathExe = "" } else { - logrus.Debugf("ssh at %#q detected as Cygwin-based (found %#q alongside)", path, cygpathExe) + logrus.Debugf("tool at %#q detected as Cygwin-based (found %#q alongside)", path, cygpathExe) } cygwinDetectCacheRW.Lock() cygwinDetectCache[path] = cygpathExe @@ -219,11 +220,23 @@ func cygpathForSSH(sshExe SSHExe) (string, bool) { // Windows, MSYS2) gets a /c/Users/... form from the sibling cygpath, which // respects the toolchain's fstab; native Windows OpenSSH gets forward // slashes (C:/Users/...), which native ssh, ssh-keygen, and scp accept. +// It is a convenience wrapper for callers holding an SSHExe; PathForTool takes +// the binary's path directly. func PathForSSH(ctx context.Context, sshExe SSHExe, orig string) (string, error) { + return PathForTool(ctx, sshExe.Exe, orig) +} + +// PathForTool is PathForSSH keyed to an arbitrary binary, for a tool that Lima +// resolves separately from ssh. Pass the tool's own path so the form follows +// the toolchain that reads it, whether the tool parses the path itself or hands +// it to the ssh beside it. Callers whose tool is Cygwin-based on every +// Windows host should convert through cygpath directly instead: the native form +// this returns as a fallback would be wrong for them. +func PathForTool(ctx context.Context, toolPath, orig string) (string, error) { if runtime.GOOS != "windows" { return orig, nil } - if cygpathExe, ok := cygpathForSSH(sshExe); ok { + if cygpathExe, ok := cygpathForSSH(SSHExe{Exe: toolPath}); ok { return fsutil.WindowsSubsystemPathWithCygpath(ctx, cygpathExe, orig) } return filepath.ToSlash(orig), nil @@ -252,7 +265,7 @@ func SftpServerForSSH(ctx context.Context, sshExe SSHExe) string { } return "" } - path, ok := resolvedSSHPath(sshExe) + path, ok := resolvedToolPath(sshExe) if !ok { return "" } @@ -309,8 +322,10 @@ func DefaultPubKeys(ctx context.Context, loadDotSSH bool) ([]PubKey, error) { if sshErr != nil { return sshErr } - keygenExe = companionForSSH(sshExe, "ssh-keygen") - privPath, err = PathForSSH(ctx, sshExe, privPath) + keygenExe = CompanionForSSH(sshExe, "ssh-keygen") + // ssh-keygen parses this path, and it falls back to PATH when + // the selected ssh ships none, so key the form to keygenExe. + privPath, err = PathForTool(ctx, keygenExe, privPath) if err != nil { return err } @@ -386,7 +401,11 @@ var sshInfo struct { // // The result always contains the IdentityFile option. // The result never contains the Port option. -func CommonOpts(ctx context.Context, sshExe SSHExe, useDotSSH bool) ([]string, error) { +// +// Path options take toolPath's form, for the binary that ends up reading them. +// scp hands its options to the ssh beside itself, which need not be sshExe. +// Version detection still runs against sshExe, since only ssh reports a version. +func CommonOpts(ctx context.Context, sshExe SSHExe, toolPath string, useDotSSH bool) ([]string, error) { configDir, err := dirnames.LimaConfigDir() if err != nil { return nil, err @@ -397,7 +416,7 @@ func CommonOpts(ctx context.Context, sshExe SSHExe, useDotSSH bool) ([]string, e return nil, err } var opts []string - idf, err := identityFileEntry(ctx, sshExe, privateKeyPath) + idf, err := identityFileEntry(ctx, toolPath, privateKeyPath) if err != nil { return nil, err } @@ -432,7 +451,7 @@ func CommonOpts(ctx context.Context, sshExe SSHExe, useDotSSH bool) ([]string, e // Fail on permission-related and other path errors return nil, err } - idf, err = identityFileEntry(ctx, sshExe, privateKeyPath) + idf, err = identityFileEntry(ctx, toolPath, privateKeyPath) if err != nil { return nil, err } @@ -484,9 +503,9 @@ func CommonOpts(ctx context.Context, sshExe SSHExe, useDotSSH bool) ([]string, e return opts, nil } -func identityFileEntry(ctx context.Context, sshExe SSHExe, privateKeyPath string) (string, error) { +func identityFileEntry(ctx context.Context, toolPath, privateKeyPath string) (string, error) { if runtime.GOOS == "windows" { - privateKeyPath, err := PathForSSH(ctx, sshExe, privateKeyPath) + privateKeyPath, err := PathForTool(ctx, toolPath, privateKeyPath) if err != nil { return "", err } @@ -568,13 +587,15 @@ func RemoveStaleControlMaster(ctx context.Context, instDir string) (bool, error) return existed, nil } -// SSHOpts adds the following options to CommonOptions: User, ControlMaster, ControlPath, ControlPersist. +// SSHOpts adds the following options to CommonOpts: User, ControlMaster, +// ControlPath, ControlPersist, and whichever of ForwardAgent, ForwardX11, and +// ForwardX11Trusted the caller asks for. func SSHOpts(ctx context.Context, 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(ctx, sshExe, useDotSSH) + opts, err := SSHOptsWithoutMultiplexing(ctx, sshExe, sshExe.Exe, username, useDotSSH) if err != nil { return nil, err } @@ -587,7 +608,6 @@ func SSHOpts(ctx context.Context, sshExe SSHExe, instDir, username string, useDo controlPath = fmt.Sprintf(`ControlPath='%s'`, controlSock) } opts = append(opts, - fmt.Sprintf("User=%s", username), // guest and host have the same username, but we should specify the username explicitly (#85) "ControlMaster=auto", controlPath, "ControlPersist=yes", @@ -604,6 +624,22 @@ func SSHOpts(ctx context.Context, sshExe SSHExe, instDir, username string, useDo return opts, nil } +// SSHOptsWithoutMultiplexing returns CommonOpts plus an explicit User, adding +// neither multiplexing nor forwarding options. Use it for invocations that +// cannot share a control socket: native Windows OpenSSH has no multiplexing, +// and Cygwin ssh's is unreliable. It builds no control path, so the caller also +// escapes the socket length limit SSHOpts enforces. +// Path options take toolPath's form; pass sshExe.Exe unless another binary +// receives the options, as described on CommonOpts. +func SSHOptsWithoutMultiplexing(ctx context.Context, sshExe SSHExe, toolPath, username string, useDotSSH bool) ([]string, error) { + opts, err := CommonOpts(ctx, sshExe, toolPath, useDotSSH) + if err != nil { + return nil, err + } + // guest and host have the same username, but we should specify the username explicitly (#85) + return append(opts, fmt.Sprintf("User=%s", username)), nil +} + // SSHArgsFromOpts returns ssh args from opts. // The result always contains {"-F", "/dev/null} in addition to {"-o", "KEY=VALUE", ...}. func SSHArgsFromOpts(opts []string) []string { diff --git a/pkg/sshutil/sshutil_windows_test.go b/pkg/sshutil/sshutil_windows_test.go index 1aacd4c61e7..1732b7fd151 100644 --- a/pkg/sshutil/sshutil_windows_test.go +++ b/pkg/sshutil/sshutil_windows_test.go @@ -137,7 +137,7 @@ func TestCompanionForSSH(t *testing.T) { assert.NilError(t, os.WriteFile(sshExe, nil, 0o644)) assert.NilError(t, os.WriteFile(keygenExe, nil, 0o644)) - assert.Equal(t, companionForSSH(SSHExe{Exe: sshExe}, "ssh-keygen"), keygenExe) + assert.Equal(t, CompanionForSSH(SSHExe{Exe: sshExe}, "ssh-keygen"), keygenExe) }) t.Run("no sibling falls back to the bare name", func(t *testing.T) { @@ -145,11 +145,11 @@ func TestCompanionForSSH(t *testing.T) { sshExe := filepath.Join(dir, "ssh.exe") assert.NilError(t, os.WriteFile(sshExe, nil, 0o644)) - assert.Equal(t, companionForSSH(SSHExe{Exe: sshExe}, "ssh-keygen"), "ssh-keygen") + assert.Equal(t, CompanionForSSH(SSHExe{Exe: sshExe}, "ssh-keygen"), "ssh-keygen") }) t.Run("empty input falls back to the bare name", func(t *testing.T) { - assert.Equal(t, companionForSSH(SSHExe{}, "ssh-keygen"), "ssh-keygen") + assert.Equal(t, CompanionForSSH(SSHExe{}, "ssh-keygen"), "ssh-keygen") }) } From a8a139f58c03324a0fef592d332ef03028451f61 Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Sat, 25 Jul 2026 23:52:04 -0700 Subject: [PATCH 2/6] copytool: report a bad path instead of a missing tool A path naming an instance that does not exist reached the user as "neither rsync nor scp found on host", because the auto backend discarded the parse error, skipped rsync, and reported whatever the scp fallback said. Only a host without scp saw that message; everywhere else the auto backend built the copy tool without complaint and the real error surfaced later, from the copy command itself. The explicit rsync backend reduced the same error to "rsync not available on guest(s)". The fallback message no longer guesses which tool is missing, since rsync may well have been found and rejected for another reason. Signed-off-by: Jan Dubois --- pkg/copytool/copytool.go | 33 +++++++++++++++++++-------------- pkg/copytool/copytool_test.go | 10 ++++++++++ pkg/copytool/rsync.go | 1 + 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/pkg/copytool/copytool.go b/pkg/copytool/copytool.go index ec33adc0f13..bbdeaa3ba4b 100644 --- a/pkg/copytool/copytool.go +++ b/pkg/copytool/copytool.go @@ -62,23 +62,28 @@ func New(ctx context.Context, backend string, paths []string, opts *Options) (Co if err != nil { return nil, err } - + // Report a bad path as itself; IsAvailableOnGuest would reduce it to + // "rsync not available on guest(s)". + if _, err := parseCopyPaths(ctx, paths); err != nil { + return nil, err + } if !rsync.IsAvailableOnGuest(ctx, paths) { return nil, errors.New("rsync not available on guest(s)") } return rsync, nil case BackendAuto: - var ( - tool CopyTool - err error - ) - // For rsync, the source and destination cannot both be remote - if !hasRemoteSourceAndDestination(ctx, paths) { - tool, err = newRsyncTool(opts) + bothRemote, err := hasRemoteSourceAndDestination(ctx, paths) + if err != nil { + // A bad path is fatal for every backend, so report it here. Falling + // through to scp would replace it with "scp not found on host". + return nil, err + } + if !bothRemote { + rsync, err := newRsyncTool(opts) if err == nil { - if tool.IsAvailableOnGuest(ctx, paths) { - return tool, nil + if rsync.IsAvailableOnGuest(ctx, paths) { + return rsync, nil } logrus.Debugf("rsync not available on guest(s), falling back to scp") } else { @@ -86,7 +91,7 @@ func New(ctx context.Context, backend string, paths []string, opts *Options) (Co } } - tool, err = newSCPTool(opts) + tool, err := newSCPTool(opts) if err != nil { // rsync may well have been found and rejected above, so name the // outcome rather than guessing which tool is missing. @@ -98,10 +103,10 @@ func New(ctx context.Context, backend string, paths []string, opts *Options) (Co } } -func hasRemoteSourceAndDestination(ctx context.Context, paths []string) bool { +func hasRemoteSourceAndDestination(ctx context.Context, paths []string) (bool, error) { copyPaths, err := parseCopyPaths(ctx, paths) if err != nil { - return true + return false, err } var hasRemoteSource, hasRemoteDestination bool @@ -115,7 +120,7 @@ func hasRemoteSourceAndDestination(ctx context.Context, paths []string) bool { } } - return hasRemoteSource && hasRemoteDestination + return hasRemoteSource && hasRemoteDestination, nil } // sshOptsForInstance returns the ssh options for copying to or from inst. The diff --git a/pkg/copytool/copytool_test.go b/pkg/copytool/copytool_test.go index 1a29333ffbd..fc0862978de 100644 --- a/pkg/copytool/copytool_test.go +++ b/pkg/copytool/copytool_test.go @@ -212,3 +212,13 @@ func TestSSHOptsForInstanceMultiplexing(t *testing.T) { assert.Equal(t, len(mux), 3, "other platforms multiplex: %v", opts) } } + +// TestNewAutoSurfacesPathError verifies that the default backend reports a bad +// path as itself, rather than as a missing copy tool. +func TestNewAutoSurfacesPathError(t *testing.T) { + t.Setenv("LIMA_HOME", t.TempDir()) + paths := []string{"nonexistent-instance-for-test:/tmp/x", "/tmp/y"} + + _, err := New(t.Context(), string(BackendAuto), paths, &Options{}) + assert.ErrorContains(t, err, "instance `nonexistent-instance-for-test`") +} diff --git a/pkg/copytool/rsync.go b/pkg/copytool/rsync.go index 0aa02873f3c..b37db34360a 100644 --- a/pkg/copytool/rsync.go +++ b/pkg/copytool/rsync.go @@ -39,6 +39,7 @@ func (t *rsyncTool) Name() string { func (t *rsyncTool) IsAvailableOnGuest(ctx context.Context, paths []string) bool { copyPaths, err := parseCopyPaths(ctx, paths) if err != nil { + // New() has already reported this to the user. logrus.Debugf("failed to parse copy paths for rsync availability check: %v", err) return false } From 8c0cbf8893c77106724b6477a407ff2310eedb8d Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Mon, 6 Jul 2026 14:47:39 -0700 Subject: [PATCH 3/6] hostagent: support reverse-sshfs on plain Windows The sftp-server that serves a mount and the host path handed to it now come from one install, the one holding the ssh.exe Lima selected at startup. Otherwise sshocker picks an sftp-server off PATH that may come from a different toolchain than the one the path was converted for, and cannot resolve what it is given. The builtin driver instead gets the native form, because sshocker serves that one in-process, where a Cygwin path resolves to the wrong directory. Signed-off-by: Jan Dubois --- pkg/hostagent/hostagent.go | 2 + pkg/hostagent/mount.go | 45 +++++++++--- pkg/hostagent/mount_windows_test.go | 97 +++++++++++++++++++++++++ pkg/sshutil/sshutil.go | 42 ++++++----- pkg/sshutil/sshutil_windows_test.go | 83 +++++++++++++++++++++ website/content/en/docs/config/mount.md | 6 ++ 6 files changed, 248 insertions(+), 27 deletions(-) create mode 100644 pkg/hostagent/mount_windows_test.go diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go index ed4502aa2b1..e874e553fc2 100644 --- a/pkg/hostagent/hostagent.go +++ b/pkg/hostagent/hostagent.go @@ -59,6 +59,7 @@ type HostAgent struct { instDir string instName string instSSHAddress string + sshExe sshutil.SSHExe sshConfig *ssh.SSHConfig portForwarder *portForwarder // legacy SSH port forwarder grpcPortForwarder *portfwd.Forwarder @@ -256,6 +257,7 @@ func New(ctx context.Context, instName string, stdout io.Writer, signalCh chan o instDir: inst.Dir, instName: instName, instSSHAddress: inst.SSHAddress, + sshExe: sshExe, sshConfig: sshConfig, driver: limaDriver, signalCh: signalCh, diff --git a/pkg/hostagent/mount.go b/pkg/hostagent/mount.go index ba2f522495c..aa87bac72f6 100644 --- a/pkg/hostagent/mount.go +++ b/pkg/hostagent/mount.go @@ -8,12 +8,12 @@ import ( "errors" "fmt" "os" + "path/filepath" "runtime" "github.com/lima-vm/sshocker/pkg/reversesshfs" "github.com/sirupsen/logrus" - "github.com/lima-vm/lima/v2/pkg/fsutil" "github.com/lima-vm/lima/v2/pkg/limatype" "github.com/lima-vm/lima/v2/pkg/sshutil" ) @@ -22,6 +22,29 @@ type mount struct { close func() error } +// mountPathAndSftpServer returns location in the path form sshExe's toolchain +// expects, and the sftp-server from that same toolchain, so the server can +// resolve the path it is given. The builtin driver gets a native path and no +// server, because sshocker serves it in-process, where a Cygwin form would +// resolve to the wrong directory. The server is also empty when the toolchain +// ships none. +func mountPathAndSftpServer(ctx context.Context, sshExe sshutil.SSHExe, driver, location string) (resolvedLocation, sftpServerBinary string, err error) { + if driver == limatype.SFTPDriverBuiltin { + return filepath.ToSlash(location), "", nil + } + resolvedLocation, err = sshutil.PathForSSH(ctx, sshExe, location) + if err != nil { + return "", "", err + } + sftpServerBinary = sshutil.SftpServerForSSH(ctx, sshExe) + if sftpServerBinary == "" { + logrus.Infof("reverse-sshfs: no sftp-server in the toolchain of %#q; falling back to a generic search, which may pick one that cannot resolve %#q", sshExe.Exe, resolvedLocation) + } else { + logrus.Debugf("reverse-sshfs: host path %#q resolved to %#q for sftp-server %#q", location, resolvedLocation, sftpServerBinary) + } + return resolvedLocation, sftpServerBinary, nil +} + func (a *HostAgent) setupMounts(ctx context.Context) ([]*mount, error) { var ( res []*mount @@ -53,9 +76,10 @@ func (a *HostAgent) setupMount(ctx context.Context, m limatype.Mount) (*mount, e logrus.Infof("Mounting %#q on %#q", m.Location, *m.MountPoint) resolvedLocation := m.Location + var sftpServerBinary string if runtime.GOOS == "windows" { var err error - resolvedLocation, err = fsutil.WindowsSubsystemPath(ctx, m.Location) + resolvedLocation, sftpServerBinary, err = mountPathAndSftpServer(ctx, a.sshExe, *m.SSHFS.SFTPDriver, m.Location) if err != nil { return nil, err } @@ -66,14 +90,15 @@ func (a *HostAgent) setupMount(ctx context.Context, m limatype.Mount) (*mount, e // modifying HostAgent's sshConfig in case of Windows sshConfig := *a.sshConfig rsf := &reversesshfs.ReverseSSHFS{ - Driver: *m.SSHFS.SFTPDriver, - SSHConfig: &sshConfig, - LocalPath: resolvedLocation, - Host: sshAddress, - Port: sshPort, - RemotePath: *m.MountPoint, - Readonly: !(*m.Writable), - SSHFSAdditionalArgs: []string{"-o", sshfsOptions}, + Driver: *m.SSHFS.SFTPDriver, + OpensshSftpServerBinary: sftpServerBinary, + SSHConfig: &sshConfig, + LocalPath: resolvedLocation, + Host: sshAddress, + Port: sshPort, + RemotePath: *m.MountPoint, + Readonly: !(*m.Writable), + SSHFSAdditionalArgs: []string{"-o", sshfsOptions}, } if runtime.GOOS == "windows" { // cygwin/msys2 doesn't support full feature set over mux socket, this has at least 2 side effects: diff --git a/pkg/hostagent/mount_windows_test.go b/pkg/hostagent/mount_windows_test.go new file mode 100644 index 00000000000..9cba190da91 --- /dev/null +++ b/pkg/hostagent/mount_windows_test.go @@ -0,0 +1,97 @@ +// SPDX-FileCopyrightText: Copyright The Lima Authors +// SPDX-License-Identifier: Apache-2.0 + +package hostagent + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "gotest.tools/v3/assert" + + "github.com/lima-vm/lima/v2/pkg/limatype" + "github.com/lima-vm/lima/v2/pkg/sshutil" +) + +// TestMountPathAndSftpServer: the mount path and the sftp-server serving it +// come from one toolchain, and the builtin driver gets no server. The Cygwin +// conversion runs cygpath.exe and is covered in pkg/sshutil, whose test binary +// doubles as a fake; here a Cygwin toolchain only shows that builtin skips it. +func TestMountPathAndSftpServer(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows-only path handling") + } + ctx := t.Context() + + const location = `C:\Users\lima\shared` + const wantPath = "C:/Users/lima/shared" + + // nativeToolchain writes an ssh.exe, plus a sibling sftp-server.exe when + // withSftpServer, and returns both paths. The production helpers expand + // the 8.3 short path (C:\Users\RUNNER~1\...) that t.TempDir() returns on + // GitHub Windows runners, so this fixture expands it too. + nativeToolchain := func(t *testing.T, withSftpServer bool) (sshExe, sftpExe string) { + t.Helper() + dir := t.TempDir() + resolved, err := filepath.EvalSymlinks(dir) + assert.NilError(t, err) + sshExe = filepath.Join(resolved, "ssh.exe") + assert.NilError(t, os.WriteFile(sshExe, nil, 0o644)) + if withSftpServer { + sftpExe = filepath.Join(resolved, "sftp-server.exe") + assert.NilError(t, os.WriteFile(sftpExe, nil, 0o644)) + } + return sshExe, sftpExe + } + + t.Run("sibling sftp-server pairs with the native path form", func(t *testing.T) { + sshExe, sftpExe := nativeToolchain(t, true) + + gotPath, gotServer, err := mountPathAndSftpServer(ctx, sshutil.SSHExe{Exe: sshExe}, "", location) + assert.NilError(t, err) + assert.Equal(t, gotPath, wantPath) + assert.Equal(t, gotServer, sftpExe) + }) + + t.Run("no sibling sftp-server leaves detection to sshocker", func(t *testing.T) { + sshExe, _ := nativeToolchain(t, false) + + gotPath, gotServer, err := mountPathAndSftpServer(ctx, sshutil.SSHExe{Exe: sshExe}, "", location) + assert.NilError(t, err) + assert.Equal(t, gotPath, wantPath) + assert.Equal(t, gotServer, "", "no sibling -> sshocker detects one itself") + }) + + t.Run("builtin driver gets no sftp-server", func(t *testing.T) { + sshExe, _ := nativeToolchain(t, true) + + gotPath, gotServer, err := mountPathAndSftpServer(ctx, sshutil.SSHExe{Exe: sshExe}, limatype.SFTPDriverBuiltin, location) + assert.NilError(t, err) + assert.Equal(t, gotPath, wantPath) + assert.Equal(t, gotServer, "", "builtin serves the mount in-process") + }) + + t.Run("builtin driver gets a native path from a Cygwin toolchain", func(t *testing.T) { + sshExe, _ := nativeToolchain(t, false) + // cygpath.exe beside ssh.exe marks the toolchain as Cygwin. This one + // is not executable, so a builtin path that consulted it would fail + // the call rather than quietly return a Cygwin form. + assert.NilError(t, os.WriteFile(filepath.Join(filepath.Dir(sshExe), "cygpath.exe"), nil, 0o644)) + + gotPath, gotServer, err := mountPathAndSftpServer(ctx, sshutil.SSHExe{Exe: sshExe}, limatype.SFTPDriverBuiltin, location) + assert.NilError(t, err) + assert.Equal(t, gotPath, wantPath, "the in-process server needs a native path") + assert.Equal(t, gotServer, "") + }) + + t.Run("explicit openssh-sftp-server driver pairs with the sibling", func(t *testing.T) { + sshExe, sftpExe := nativeToolchain(t, true) + + gotPath, gotServer, err := mountPathAndSftpServer(ctx, sshutil.SSHExe{Exe: sshExe}, limatype.SFTPDriverOpenSSHSFTPServer, location) + assert.NilError(t, err) + assert.Equal(t, gotPath, wantPath) + assert.Equal(t, gotServer, sftpExe) + }) +} diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index c121a8901a4..fb4e7a9877b 100644 --- a/pkg/sshutil/sshutil.go +++ b/pkg/sshutil/sshutil.go @@ -215,11 +215,15 @@ func cygpathForSSH(toolExe SSHExe) (string, bool) { return cygpathExe, cygpathExe != "" } -// PathForSSH converts orig to the path form Lima's ssh-family invocations -// expect; unchanged on non-Windows. On Windows, Cygwin-based ssh (Git for -// Windows, MSYS2) gets a /c/Users/... form from the sibling cygpath, which -// respects the toolchain's fstab; native Windows OpenSSH gets forward -// slashes (C:/Users/...), which native ssh, ssh-keygen, and scp accept. +// PathForSSH converts orig to the path form the selected ssh toolchain +// expects; unchanged on non-Windows. On Windows, Cygwin-based ssh (Git for +// Windows, MSYS2) gets a Unix form from the sibling cygpath, which respects +// the toolchain's fstab (/c/Users/... under MSYS2, /cygdrive/c/... under +// stock Cygwin); native Windows OpenSSH gets forward slashes (C:/Users/...), +// which native ssh, ssh-keygen, and scp accept. +// Tools outside that family read the result too: a locally spawned sftp-server +// chdirs into it, and the guest's sshfs prefixes it to every request. sshocker +// rewrites a /c/... form to C:\... before either sees it. // It is a convenience wrapper for callers holding an SSHExe; PathForTool takes // the binary's path directly. func PathForSSH(ctx context.Context, sshExe SSHExe, orig string) (string, error) { @@ -243,11 +247,11 @@ func PathForTool(ctx context.Context, toolPath, orig string) (string, error) { } // SftpServerForSSH returns the path of sftp-server binary from sshExe's toolchain, -// so a locally-spawned sftp-server (reverse-sshfs) uses the same path form -// PathForSSH produces. For Cygwin-based ssh the sibling cygpath resolves -// /usr/lib/ssh/sftp-server; for native OpenSSH it is sftp-server.exe beside -// ssh.exe. Returns "" on non-Windows, empty input, or no match, leaving the -// caller to fall back to its library's auto-detection. +// so a locally-spawned sftp-server (reverse-sshfs) and the host paths Lima +// converts for it come from one install. For Cygwin-based ssh the sibling +// cygpath resolves /usr/lib/ssh/sftp-server; for native OpenSSH it is +// sftp-server.exe beside ssh.exe. Returns "" on non-Windows, empty input, or no +// match, leaving the caller to fall back to its library's auto-detection. func SftpServerForSSH(ctx context.Context, sshExe SSHExe) string { if runtime.GOOS != "windows" || sshExe.Exe == "" { return "" @@ -257,20 +261,24 @@ func SftpServerForSSH(ctx context.Context, sshExe SSHExe) string { if err != nil { return "" } + // Validate with LookPath, which appends the ".exe" that cygpath omits. + // Under the default driver sshocker resolves this path with LookPath as + // well and fails the mount when it does not resolve, so returning + // nothing beats returning a path it will reject. candidate := strings.TrimSpace(string(out)) - for _, p := range []string{candidate, candidate + ".exe"} { - if _, err := os.Stat(p); err == nil { - return p - } + sftpServer, err := exec.LookPath(candidate) + if err != nil { + logrus.WithError(err).Debugf("cannot use the sftp-server at %#q that cygpath resolved", candidate) + return "" } - return "" + return sftpServer } path, ok := resolvedToolPath(sshExe) if !ok { return "" } - sftpServer := filepath.Join(filepath.Dir(path), "sftp-server.exe") - if _, err := os.Stat(sftpServer); err != nil { + sftpServer, err := exec.LookPath(filepath.Join(filepath.Dir(path), "sftp-server.exe")) + if err != nil { return "" } return sftpServer diff --git a/pkg/sshutil/sshutil_windows_test.go b/pkg/sshutil/sshutil_windows_test.go index 1732b7fd151..50e8d7627aa 100644 --- a/pkg/sshutil/sshutil_windows_test.go +++ b/pkg/sshutil/sshutil_windows_test.go @@ -7,11 +7,43 @@ import ( "os" "path/filepath" "runtime" + "slices" + "strings" "testing" "gotest.tools/v3/assert" ) +const ( + fakeCygpathOutEnv = "LIMA_TEST_FAKE_CYGPATH_OUT" + fakeCygpathFailure = "exit-non-zero" +) + +// TestMain doubles as a fake cygpath.exe. With fakeCygpathOutEnv set it prints +// that value and exits 0, or exits 1 when the value is fakeCygpathFailure, so a +// test can copy this binary beside an ssh.exe and drive the Cygwin branch. An +// empty value would not work as the failure signal, because Windows cannot +// distinguish an empty environment variable from an unset one. +// +// It answers only the sftp-server probe and rejects any other command line. A +// change to the arguments in SftpServerForSSH then breaks this test instead of +// going unnoticed. A test driving a different cygpath call needs its own arm. +func TestMain(m *testing.M) { + if out, ok := os.LookupEnv(fakeCygpathOutEnv); ok { + if want := []string{"-w", "/usr/lib/ssh/sftp-server"}; !slices.Equal(os.Args[1:], want) { + os.Stderr.WriteString("fake cygpath: got " + strings.Join(os.Args[1:], " ") + + ", want " + strings.Join(want, " ") + "\n") + os.Exit(2) + } + if out == fakeCygpathFailure { + os.Exit(1) + } + os.Stdout.WriteString(out + "\n") + os.Exit(0) + } + os.Exit(m.Run()) +} + // TestPickCompleteSSHOnWindows: an ssh.exe missing scp.exe or // ssh-keygen.exe (MinGit's shape) is skipped for the next complete // install on PATH. @@ -123,6 +155,41 @@ func TestSftpServerForSSH(t *testing.T) { }) } +// TestSftpServerForSSHCygwin: the Cygwin branch reports what the sibling +// cygpath resolves, but only once that names an executable. cygpath omits the +// .exe suffix, so the candidate reaches LookPath without one. +func TestSftpServerForSSHCygwin(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows-only path handling") + } + ctx := t.Context() + + t.Run("candidate without .exe resolves to the binary beside it", func(t *testing.T) { + sshExe := cygwinToolchain(t) + dir := filepath.Dir(sshExe) + sftpExe := filepath.Join(dir, "sftp-server.exe") + assert.NilError(t, os.WriteFile(sftpExe, nil, 0o644)) + t.Setenv(fakeCygpathOutEnv, filepath.Join(dir, "sftp-server")) + + assert.Equal(t, SftpServerForSSH(ctx, SSHExe{Exe: sshExe}), sftpExe) + }) + + t.Run("candidate that resolves to nothing returns empty", func(t *testing.T) { + sshExe := cygwinToolchain(t) + t.Setenv(fakeCygpathOutEnv, filepath.Join(filepath.Dir(sshExe), "sftp-server")) + + assert.Equal(t, SftpServerForSSH(ctx, SSHExe{Exe: sshExe}), "", + "unresolvable candidate -> caller falls back to sshocker auto-detect") + }) + + t.Run("cygpath failure returns empty", func(t *testing.T) { + sshExe := cygwinToolchain(t) + t.Setenv(fakeCygpathOutEnv, fakeCygpathFailure) + + assert.Equal(t, SftpServerForSSH(ctx, SSHExe{Exe: sshExe}), "") + }) +} + // TestCompanionForSSH: a companion tool is resolved beside the selected // ssh.exe, and falls back to the bare name when no sibling exists. func TestCompanionForSSH(t *testing.T) { @@ -153,6 +220,22 @@ func TestCompanionForSSH(t *testing.T) { }) } +// cygwinToolchain writes an ssh.exe into a fresh directory, plus a cygpath.exe +// that is a copy of this test binary, and returns the ssh.exe path. +func cygwinToolchain(t *testing.T) string { + t.Helper() + dir := resolvedTempDir(t) + sshExe := filepath.Join(dir, "ssh.exe") + assert.NilError(t, os.WriteFile(sshExe, nil, 0o644)) + + self, err := os.Executable() + assert.NilError(t, err) + binary, err := os.ReadFile(self) + assert.NilError(t, err) + assert.NilError(t, os.WriteFile(filepath.Join(dir, "cygpath.exe"), binary, 0o755)) + return sshExe +} + // resolvedTempDir is t.TempDir() run through EvalSymlinks, matching what // the production helpers compute. On GitHub Windows runners t.TempDir() // is an 8.3 short path (C:\Users\RUNNER~1\...) that the helpers expand, diff --git a/website/content/en/docs/config/mount.md b/website/content/en/docs/config/mount.md index 31f33c1abf5..767a7364997 100644 --- a/website/content/en/docs/config/mount.md +++ b/website/content/en/docs/config/mount.md @@ -59,6 +59,12 @@ The default value of `sftpDriver` has been set to "openssh-sftp-server" since Li such as `/usr/libexec/sftp-server` is detected on the host. Lima prior to v0.10 had used "builtin" as the SFTP driver. +On Windows, unless `sftpDriver` is "builtin", Lima first looks for an sftp-server in the toolchain of the +`ssh.exe` it selected, either `sftp-server.exe` beside a native OpenSSH or `/usr/lib/ssh/sftp-server` for a +Cygwin-based one. Only if that finds nothing does the generic search apply. +When the toolchain lookup finds one, the server and the host paths it serves come from the same install, which +matters when a Cygwin-based OpenSSH (Git for Windows, MSYS2) and the native Windows OpenSSH are both installed. + #### Caveats - A mount is disabled when the SSH connection was shut down. - A compromised `sshfs` process in the guest may have access to unexposed host directories. From 170dccc3fe7fb0ffb643d18b739c30b73804ffba Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Sun, 26 Jul 2026 14:54:12 -0700 Subject: [PATCH 4/6] shell: measure the --sync directory depth on the host's own path `--sync` refuses a directory too close to the filesystem root, but on Windows it measured a path already converted to the /c/... form while still splitting on backslashes. Every directory counted as depth 1, so `--sync` could not run there at all. Measuring the native path keeps one threshold meaning the same thing everywhere. Only Windows separates on a backslash. Elsewhere it is an ordinary filename character, and counting it as a separator would score a directory just below the root deep enough to pass, so the caller now says which form it holds rather than leaving it implied. That guard was also the only thing stopping `--sync` when the conversion failed or returned nothing, because the empty string scored 1. Both now need their own error. An empty directory reaches rsync as the toolchain root, and with stdout not a terminal the sync back runs `--delete` against it unprompted. A wsl2 guest already reaches the host directory through the /mnt automount, so `--sync` cannot isolate it from host files there. It now refuses that combination up front. Signed-off-by: Jan Dubois --- cmd/limactl/shell.go | 76 +++++++++++++++++++++----- cmd/limactl/shell_test.go | 58 ++++++++++++++++++++ website/content/en/docs/examples/ai.md | 1 + 3 files changed, 120 insertions(+), 15 deletions(-) diff --git a/cmd/limactl/shell.go b/cmd/limactl/shell.go index ef76378fb4e..d617ba6501c 100644 --- a/cmd/limactl/shell.go +++ b/cmd/limactl/shell.go @@ -213,23 +213,37 @@ func shellAction(cmd *cobra.Command, args []string) error { if syncHostWorkdir && len(inst.Config.Mounts) > 0 { return errors.New("cannot use `--sync` when the instance has host mounts configured, start the instance with `--mount-none` to disable mounts") } + // A wsl2 guest already reaches the host directory through the /mnt automount, + // so `--sync` cannot isolate it from host files the way it does elsewhere. + if syncHostWorkdir && inst.VMType == limatype.WSL2 { + return errors.New("cannot use `--sync` with a wsl2 instance, the host directory is already visible in the guest") + } // When workDir is explicitly set, the shell MUST have workDir as the cwd, or exit with an error. // // changeDirCmd := "cd workDir || exit 1" if workDir != "" // := "cd hostCurrentDir || cd hostHomeDir" if workDir == "" var changeDirCmd string - var hostCurrentDir string + // hostCurrentDirNative is the path as the host sees it. hostCurrentDir is the + // form the guest and the copy tool receive, which on Windows differs. + var hostCurrentDir, hostCurrentDirNative string if syncDirVal != "" { - hostCurrentDir, err = filepath.Abs(syncDirVal) - if err == nil && runtime.GOOS == "windows" { - hostCurrentDir, err = mountDirFromWindowsDir(ctx, inst, hostCurrentDir) - } + hostCurrentDirNative, err = filepath.Abs(syncDirVal) } else { - hostCurrentDir, err = hostCurrentDirectory(ctx, inst) + hostCurrentDirNative, err = os.Getwd() + } + if err == nil { + hostCurrentDir = hostCurrentDirNative + if runtime.GOOS == "windows" { + hostCurrentDir, err = mountDirFromWindowsDir(ctx, inst, hostCurrentDirNative) + } } if err != nil { + // An empty hostCurrentDir would reach rsync as the toolchain root. + if syncHostWorkdir { + return fmt.Errorf("failed to determine the host directory to sync: %w", err) + } changeDirCmd = "false" logrus.WithError(err).Warn("failed to get the current directory") } @@ -238,10 +252,19 @@ func shellAction(cmd *cobra.Command, args []string) error { return fmt.Errorf("rsync is required for `--sync` but not found: %w", err) } - srcWdDepth := len(strings.Split(hostCurrentDir, string(os.PathSeparator))) + // Measure the host's own path. The form hostCurrentDir carries on Windows + // adds one component (/c/...) or two (/cygdrive/c/...). + srcWdDepth := pathDepth(hostCurrentDirNative, runtime.GOOS == "windows") if srcWdDepth < rsyncMinimumSrcDirDepth { - return fmt.Errorf("expected the depth of the host working directory (%#q) to be more than %d, only got %d (Hint: %s)", - hostCurrentDir, rsyncMinimumSrcDirDepth, srcWdDepth, "cd to a deeper directory") + return fmt.Errorf("expected the depth of the host working directory (%#q) to be at least %d, only got %d (Hint: %s)", + hostCurrentDirNative, rsyncMinimumSrcDirDepth, srcWdDepth, "cd to a deeper directory") + } + // rsync acts on hostCurrentDir, so measure that too: cygpath can report + // success without writing a path, and an fstab can map a deep directory + // onto a shallow one. It is always POSIX form, even on Windows. + if dstWdDepth := pathDepth(hostCurrentDir, false); dstWdDepth < rsyncMinimumSrcDirDepth { + return fmt.Errorf("expected the depth of the converted host working directory (%#q) to be at least %d, only got %d", + hostCurrentDir, rsyncMinimumSrcDirDepth, dstWdDepth) } } @@ -744,12 +767,35 @@ func executeSSHForRsync(ctx context.Context, sshCmd exec.Cmd, sshLocalPort int, return nil } -func hostCurrentDirectory(ctx context.Context, inst *limatype.Instance) (string, error) { - hostCurrentDir, err := os.Getwd() - if err == nil && runtime.GOOS == "windows" { - hostCurrentDir, err = mountDirFromWindowsDir(ctx, inst, hostCurrentDir) - } - return hostCurrentDir, err +// pathDepth counts the separator-delimited fields of an absolute path, +// collapsing repeated separators. A trailing separator adds no field unless the +// path is a bare root. Pass windows for a path in Windows form: elsewhere a +// backslash is an ordinary filename character, and counting it as a separator +// would score a directory just below the root deep enough to pass the guard. +func pathDepth(path string, windows bool) int { + slashed := path + if windows { + slashed = strings.ReplaceAll(path, `\`, "/") + // An extended-length or device prefix spells a path the plain form also + // spells, so drop it before counting. The `UNC` token this leaves behind + // occupies the field the plain form's leading separator would. + if strings.HasPrefix(slashed, "//?/") || strings.HasPrefix(slashed, "//./") { + slashed = slashed[len("//?/"):] + } + } + // A separator run delimits one field. The two leading separators of a UNC + // path enclose one root, so counting both would clear the minimum depth for + // a share root. + for strings.Contains(slashed, "//") { + slashed = strings.ReplaceAll(slashed, "//", "/") + } + // filepath.Clean keeps the trailing separator of a root, so `\\server\share` + // and `\\server\share\` both reach here. Trim one that leaves a separator + // behind, which spares the bare roots "/" and `C:\`. + if trimmed := strings.TrimSuffix(slashed, "/"); strings.Contains(trimmed, "/") { + slashed = trimmed + } + return strings.Count(slashed, "/") + 1 } func rsyncVersion(ctx context.Context) (*semver.Version, error) { diff --git a/cmd/limactl/shell_test.go b/cmd/limactl/shell_test.go index eed76c53856..8bf6008dc06 100644 --- a/cmd/limactl/shell_test.go +++ b/cmd/limactl/shell_test.go @@ -9,6 +9,64 @@ import ( "gotest.tools/v3/assert" ) +// TestPathDepth covers both host path forms, because `limactl shell --sync` +// compares the depth against rsyncMinimumSrcDirDepth to refuse a directory too +// close to the root. A Windows path must count the same as its Unix equivalent, +// or the same threshold means two different things. The windows column is what +// the call site passes for runtime.GOOS, so both platforms are covered wherever +// this runs. +func TestPathDepth(t *testing.T) { + tests := []struct { + path string + windows bool + expected int + }{ + // A home directory sits at 3 on either platform, below the minimum of 4. + {"/Users/jan", false, 3}, + {`C:\Users\jan`, true, 3}, + {"/Users/jan/proj", false, 4}, + {`C:\Users\jan\proj`, true, 4}, + {"/", false, 2}, + {`C:\`, true, 2}, + // filepath.Abs normalises this form away before Lima ever sees it. + // Counting it anyway keeps the helper independent of the host separator. + {"C:/Users/jan/proj", true, 4}, + // On Unix a backslash is an ordinary filename character. Counting it as + // a separator would score this single directory just below the root at + // 4 and pass the guard. + {`/x\y\z`, false, 2}, + {`/foo\..\..\..\bar`, false, 2}, + // The guard must refuse a share root and allow a directory inside it; + // counting both leading separators would pass the root. + {`\\server\share`, true, 3}, + {`\\server\share\proj`, true, 4}, + // Go's Clean keeps a separator run inside a UNC volume, and whether + // Windows collapses it earlier is unverified, so the guard collapses + // the run itself. + {`\\server\\share`, true, 3}, + {`\\\server\share`, true, 3}, + // An extended-length or device prefix spells a path the plain form + // also spells. + {`\\?\C:\`, true, 2}, + {`\\?\C:\Users\jan\proj`, true, 4}, + {`\\.\C:\`, true, 2}, + {`\\?\UNC\server\share`, true, 3}, + // filepath.Clean keeps a root's trailing separator and strips one below + // a root, so only the first form reaches Lima. Counting both alike + // keeps the trim from changing an ordinary path's depth. + {`\\server\share\`, true, 3}, + {`C:\Users\jan\proj\`, true, 4}, + // An empty path returns 1, so the guard refuses one if it ever reaches + // here. + {"", false, 1}, + } + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + assert.Equal(t, pathDepth(tt.path, tt.windows), tt.expected) + }) + } +} + func TestParseRsyncStats(t *testing.T) { tests := []struct { name string diff --git a/website/content/en/docs/examples/ai.md b/website/content/en/docs/examples/ai.md index 2a076d8c9dd..2d8b985e529 100644 --- a/website/content/en/docs/examples/ai.md +++ b/website/content/en/docs/examples/ai.md @@ -190,6 +190,7 @@ limactl shell --sync . default - **rsync** must be installed on both host and guest - The host working directory must be at least 4 levels deep (e.g., `/Users/username/projects/myproject`) - The instance must not have any host mounts configured (use `--mount-none` when creating) +- The instance must not use the `wsl2` VM type ## See also From 1fe47ffdf7c7ddeacbe012725f73771902b5c935 Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Sun, 17 May 2026 12:44:47 -0700 Subject: [PATCH 5/6] docs: document the Windows host SSH toolchain Lima now supports native Windows OpenSSH alongside the Cygwin-based toolchains, and the two differ in which binaries they ship, which path form they produce, and where their sftp-server lives. Document what to install, how Lima picks between them, and what changes for reverse-sshfs mounts. Also replace the Known Issues bullet that claimed Windows ships no ssh.exe. Lima unpacks a .tar.gz rootfs itself, so only xz, bzip2, and zstd archives still need an external binary. These pages describe the state after the copytool and reverse-sshfs support lands, so this should merge after those. Signed-off-by: Jan Dubois --- .../en/docs/config/environment-variables.md | 27 +++++++ website/content/en/docs/config/vmtype/qemu.md | 7 ++ website/content/en/docs/config/vmtype/wsl2.md | 78 ++++++++++++++++++- .../content/en/docs/installation/_index.md | 3 + 4 files changed, 114 insertions(+), 1 deletion(-) diff --git a/website/content/en/docs/config/environment-variables.md b/website/content/en/docs/config/environment-variables.md index 373c261e0a8..ee034a120b5 100644 --- a/website/content/en/docs/config/environment-variables.md +++ b/website/content/en/docs/config/environment-variables.md @@ -214,3 +214,30 @@ This page documents the environment variables used in Lima. ```sh export QEMU_SYSTEM_X86_64=/usr/local/bin/qemu-system-x86_64 ``` + +### `SSH` + +- **Description**: Command to run in place of the `ssh` executable. The value is + split into shell tokens, so it can carry arguments as well as a path. Lima + sets its own `-F` on most invocations, and `limactl copy` drops the arguments + on its scp backend, so a path with no arguments is the reliable form. + Tokenization also reads `\` as an escape and splits on spaces. A Windows + path therefore needs quotes, and its backslashes survive only inside single + quotes; unquoted, `C:\Program Files\Git\usr\bin\ssh.exe` arrives as + `C:Program`. +- **Default**: unset. Lima then searches `$PATH`, and on Windows falls back to + `%SystemRoot%\System32\OpenSSH`. +- **Usage**: + ```sh + export SSH=/opt/homebrew/bin/ssh + ``` + ```powershell + $env:SSH = "'C:\Program Files\Git\usr\bin\ssh.exe'" + ``` +- **Note**: On Windows this overrides the toolchain detection described under + [Windows toolchain]({{< ref "/docs/config/vmtype/wsl2#windows-toolchain" >}}). + Lima pairs the path form and the `sftp-server` binary with whichever `ssh` + this names, but some paths still follow `PATH`, among them the guest mount + point and `limactl shell`'s working directory. Point `SSH` at the same + install that comes first on `PATH` to keep them consistent. + `limactl guest-install` ignores this variable altogether. diff --git a/website/content/en/docs/config/vmtype/qemu.md b/website/content/en/docs/config/vmtype/qemu.md index d1a638362b6..5f6654e3c31 100644 --- a/website/content/en/docs/config/vmtype/qemu.md +++ b/website/content/en/docs/config/vmtype/qemu.md @@ -11,6 +11,13 @@ Recommended QEMU version: - v8.2.1 or later (macOS) - v6.2.0 or later (Linux) +On a Windows host, "qemu" needs an OpenSSH installation for its own `ssh`, +`scp`, and `ssh-keygen`. Mounts there must be reverse-sshfs, because QEMU +for Windows has no 9p support, and reverse-sshfs is the only thing in Lima +that uses `sftp-server`. See +[Windows toolchain]({{< ref "/docs/config/vmtype/wsl2#windows-toolchain" >}}) +for which binaries to install and how Lima chooses between them. + An example configuration: {{< tabpane text=true >}} {{% tab header="CLI" %}} diff --git a/website/content/en/docs/config/vmtype/wsl2.md b/website/content/en/docs/config/vmtype/wsl2.md index 8f33fc9c77c..d88ef470255 100644 --- a/website/content/en/docs/config/vmtype/wsl2.md +++ b/website/content/en/docs/config/vmtype/wsl2.md @@ -42,7 +42,7 @@ containerd: - "wsl2" currently doesn't support many of Lima's options. See [this file](https://github.com/lima-vm/lima/blob/master/pkg/wsl2/wsl_driver_windows.go#L19) for the latest supported options. - When running lima using "wsl2", `${LIMA_HOME}//serial.log` will not contain kernel boot logs - WSL2 requires a `tar` formatted rootfs archive instead of a VM image. Standard VM disk images (like `.qcow2`, `.raw`, etc.) or `.squashfs` images cannot be natively imported by WSL2. -- Windows doesn't ship with ssh.exe, gzip.exe, etc. which are used by Lima at various points. The easiest way around this is to run `winget install -e --id Git.MinGit` (winget is now built in to Windows as well), and add the resulting `C:\Program Files\Git\usr\bin\` directory to your path. +- Lima unpacks a `.tar.gz` rootfs on its own, but a `.tar.xz`, `.tar.bz2`, or `.tar.zst` one needs the matching `xz`, `bzip2`, or `zstd` binary on your `PATH`, and Windows ships none of them. ### Rootfs Image Requirements & Building Custom Images @@ -76,3 +76,79 @@ If you want to build and use your own custom rootfs, you can build it from a sta docker build -o type=tar,dest=custom-rootfs.tar . ``` +### Windows toolchain + +Lima uses an OpenSSH installation on the host. On a default Windows +install that is the native binaries in `C:\Windows\System32\OpenSSH\`: + +- **OpenSSH Client** (`ssh.exe`, `scp.exe`, `ssh-keygen.exe`) ships by default + on Windows 10 and Windows 11, and covers the WSL2 driver. +- **`sftp-server.exe`** is part of OpenSSH Server, an [optional Feature on Demand](https://learn.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse). + Only the QEMU driver's reverse-sshfs mounts use it, and they treat it as a + preference: with no `sftp-server` on the host, `sshocker` serves the mount + in-process instead. Pinning `sftpDriver: openssh-sftp-server` drops that + in-process fallback, so the mount fails if the search below turns up + nothing either. Install it via + `Add-WindowsCapability -Online -Name OpenSSH.Server~~~~0.0.1.0` from an + elevated PowerShell, or via Settings → Apps → Optional features. Setting + `sftpDriver: builtin` under a mount's + [`sshfs`]({{< ref "/docs/config/mount#reverse-sshfs" >}}) skips the search. + +Installing [Git for Windows](https://gitforwindows.org/) +(`winget install -e --id Git.Git`) remains supported as an alternative +to the native binaries. Use the full Git for Windows installer, not +MinGit. MinGit omits `scp.exe`, `ssh-keygen.exe`, and `cygpath.exe`, +all of which Lima needs. Add `C:\Program Files\Git\usr\bin\` to your +`PATH` ahead of any other directory holding an `ssh.exe`. The Git +installer's default setting adds only `cmd\`, which holds none of those +binaries, and `usr\bin` also carries `find.exe` and `sort.exe`, which +shadow the Windows commands of those names. + +Order matters because Lima takes the first directory on `PATH` that holds +`ssh.exe`, `scp.exe`, and `ssh-keygen.exe` together. When no directory +qualifies it tries `%SystemRoot%\System32\OpenSSH`, and failing that +whatever `ssh` `PATH` yields, complete or not. + +The `SSH` environment variable overrides that search, but it does not +replace it. The hostagent still runs the `ssh` that `PATH` resolves, while +the key and socket paths it is handed take the form of whichever toolchain +`SSH` names, so the two must name the same install. Naming a Cygwin +toolchain in `SSH` while native OpenSSH leads `PATH` fails `limactl start` +at its readiness check, before any mount. Other helpers convert +paths with the `cygpath` that `PATH` resolves, regardless of `SSH`. Fixing +`PATH` is the reliable route. + +Lima detects which ssh toolchain is in use on each `limactl start` and +takes both the path form and the `sftp-server` binary from that +toolchain, so the path and the server that reads it match: + +- **Cygwin-based ssh** (Git for Windows, MSYS2, stock Cygwin): paths are + converted by `cygpath` to a POSIX form, `/c/Users/USER` under MSYS2 and + Git for Windows or `/cygdrive/c/Users/USER` under stock Cygwin, + respecting any custom fstab the user has configured. `sftp-server` is + resolved from the same toolchain. +- **Native Windows OpenSSH**: paths are returned with forward slashes, + like `C:/Users/USER`. Native `ssh`, `ssh-keygen`, and `scp` accept + this form directly. `sftp-server.exe` is picked from the same + directory as `ssh.exe`. + +When no matching `sftp-server` is found, the hostagent logs an info +message at start and falls back to `sshocker`'s own detection; a missing +OpenSSH.Server install on a native-only host typically shows up there. + +Note: Lima converts the host path with the selected toolchain on every +start, and `sshocker` maps it back to a drive letter from the leading +`/x/` segment without consulting any `fstab`. A custom MSYS2 `fstab` that +remaps drive prefixes (rare) therefore survives the round trip only by +coincidence, so avoid changing toolchains between starts of an existing +QEMU instance with a reverse-sshfs mount on such a host. A mount pinned to +`sftpDriver: builtin` is exempt, because `sshocker` serves it from the +native path. + +Separately, a mount without an explicit `mountPoint` takes its guest path +from whichever `cygpath` `PATH` resolves, re-derived on every load rather +than stored. Stock Cygwin yields `/cygdrive/c/Users/USER` where MSYS2, +Git for Windows, and Lima's own fallback all yield `/c/Users/USER`, so +adding or removing stock Cygwin ahead of the others on `PATH` moves an +existing instance's mount inside the guest. Set `mountPoint` explicitly +to pin it. diff --git a/website/content/en/docs/installation/_index.md b/website/content/en/docs/installation/_index.md index dc5e208f8b2..13b51018d66 100644 --- a/website/content/en/docs/installation/_index.md +++ b/website/content/en/docs/installation/_index.md @@ -11,6 +11,9 @@ Supported host OS: Prerequisite: - QEMU (Required, only if [QEMU]({{< ref "/docs/config/vmtype#qemu" >}}) driver is used) +- An OpenSSH client on Windows hosts (Windows 10 and 11 ship one); see + [Windows toolchain]({{< ref "/docs/config/vmtype/wsl2#windows-toolchain" >}}) + for when Lima needs more than the default install {{< tabpane text=true >}} From b8016ef30c886705495212cff3bd08156c6b7be3 Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Sun, 26 Jul 2026 23:45:30 -0700 Subject: [PATCH 6/6] CI: factor Windows setup into composite actions; add plain-Windows jobs Move the Windows runner setup out of test.yml into reusable composite actions, and add windows-plain-{wsl2,qemu} jobs that run Lima on a host with Git for Windows, MSYS2, and Cygwin removed. The plain-host action checks the toolchain is gone across filesystem, PATH, known binaries, and registry, and that the native OpenSSH client is present, so a runner-image change in either direction fails loudly instead of masking a regression. The plain-host check can prove the OpenSSH binaries exist but not which ssh limactl will resolve, so the selection rules gain unit tests next to the code that implements them. With test.yml no longer setting _LIMA_WINDOWS_EXTRA_PATH, that env var loses its last user, so drop the implementation and its docs entry. It prepended C:\Program Files\Git\usr\bin to limactl's PATH, which is how the MSYS2 jobs got a Cygwin ssh. The MSYS2 jobs install openssh in its place, so they keep exercising the Cygwin toolchain while the plain jobs exercise the native one. They also gain rsync. Without it hack/test-templates.sh skips its rsync copy backend instead of failing, so the path conversion that backend performs on Windows never runs. The "windows" job id becomes "windows-wsl2". Its name: is unchanged, so required status checks are unaffected. Signed-off-by: Jan Dubois --- .github/actions/windows_msys2_prep/action.yml | 24 +++ .../actions/windows_plain_build/action.yml | 26 +++ .github/actions/windows_plain_host/action.yml | 183 ++++++++++++++++++ .../windows_plain_smoke_test/action.yml | 89 +++++++++ .../windows_plain_templates/action.yml | 81 ++++++++ .../actions/windows_qemu_install/action.yml | 11 ++ .github/actions/windows_wsl2_setup/action.yml | 62 ++++++ .github/workflows/test.yml | 120 ++++++++---- cmd/limactl/main.go | 10 - pkg/sshutil/sshutil_windows_test.go | 36 ++++ .../en/docs/config/environment-variables.md | 14 -- website/content/en/docs/releases/breaking.md | 5 + 12 files changed, 600 insertions(+), 61 deletions(-) create mode 100644 .github/actions/windows_msys2_prep/action.yml create mode 100644 .github/actions/windows_plain_build/action.yml create mode 100644 .github/actions/windows_plain_host/action.yml create mode 100644 .github/actions/windows_plain_smoke_test/action.yml create mode 100644 .github/actions/windows_plain_templates/action.yml create mode 100644 .github/actions/windows_qemu_install/action.yml create mode 100644 .github/actions/windows_wsl2_setup/action.yml diff --git a/.github/actions/windows_msys2_prep/action.yml b/.github/actions/windows_msys2_prep/action.yml new file mode 100644 index 00000000000..9802879e974 --- /dev/null +++ b/.github/actions/windows_msys2_prep/action.yml @@ -0,0 +1,24 @@ +name: 'windows msys2 prep' +description: > + Adds C:\msys64\usr\bin to PATH and installs the MSYS2 packages the Windows + integration jobs need. The PATH prepend persists into subsequent job steps + via $GITHUB_PATH, so the test step can call cygpath / bash / pacman tools + directly without re-prepending. The Make step inherits the prepend too, so + make's $(shell find ...) finds GNU find instead of System32\FIND.EXE. +runs: + using: composite + steps: + - name: Prepend MSYS2 to PATH and install test dependencies + shell: pwsh + # The value written to $GITHUB_PATH is the static literal C:\msys64\usr\bin, + # which is the canonical pattern for adding a directory to subsequent + # steps' PATH. No PR-controlled data reaches the env file. + run: | # zizmor: ignore[github-env] + $ErrorActionPreference = 'Stop' + Add-Content -Path $env:GITHUB_PATH -Value 'C:\msys64\usr\bin' + # rsync: without it the copy test skips its rsync backend, so the Windows + # path conversion that backend performs goes unexercised. + # openssh: keeps a Cygwin ssh ahead of %SystemRoot%\System32\OpenSSH, so + # these jobs cover the Cygwin toolchain and the plain jobs the native one. + & 'C:\msys64\usr\bin\pacman.exe' -Sy --noconfirm openbsd-netcat diffutils openssh rsync socat w3m + if ($LASTEXITCODE -ne 0) { throw "pacman failed: $LASTEXITCODE" } diff --git a/.github/actions/windows_plain_build/action.yml b/.github/actions/windows_plain_build/action.yml new file mode 100644 index 00000000000..0630fccf456 --- /dev/null +++ b/.github/actions/windows_plain_build/action.yml @@ -0,0 +1,26 @@ +name: 'windows plain build' +description: > + Builds limactl.exe and the Linux/amd64 guest agent with `go build` on a + vanilla Windows host (no MSYS2 make / bash). The guest agent must be built + separately because limactl looks it up at + _output/share/lima/lima-guestagent.Linux-x86_64 when starting an instance, + and `go build ./cmd/limactl` does not produce that file. + + Without make's -X ldflag pkg/version.Version keeps its "" default. + Being unparsable, that passes every version comparison, which is what lets + default.yaml's minimumLimaVersion accept these builds. +runs: + using: composite + steps: + - name: Build limactl and Linux/amd64 guest agent + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + go build -o _output\bin\limactl.exe .\cmd\limactl + if ($LASTEXITCODE -ne 0) { throw "limactl build failed: $LASTEXITCODE" } + New-Item -ItemType Directory -Force -Path _output\share\lima | Out-Null + $env:CGO_ENABLED = '0' + $env:GOOS = 'linux' + $env:GOARCH = 'amd64' + go build -o _output\share\lima\lima-guestagent.Linux-x86_64 .\cmd\lima-guestagent + if ($LASTEXITCODE -ne 0) { throw "lima-guestagent build failed: $LASTEXITCODE" } diff --git a/.github/actions/windows_plain_host/action.yml b/.github/actions/windows_plain_host/action.yml new file mode 100644 index 00000000000..d37a73ca1b7 --- /dev/null +++ b/.github/actions/windows_plain_host/action.yml @@ -0,0 +1,183 @@ +name: 'windows plain host' +description: > + Uninstalls MSYS2 and Git for Windows from the windows-2025 runner so the + job runs against a vanilla Windows toolchain: native OpenSSH from + %SystemRoot%\System32\OpenSSH, wsl.exe, and tar. Then verifies they are + gone across four layers (filesystem, PATH, binaries, registry) and that the + native OpenSSH client is there, so a runner-image change in either direction + fails the job loudly. + + Destructive and irreversible: it also rewrites the Machine and User PATH in + the registry, so run it only on an ephemeral runner. + + Run AFTER steps that need git CLI (checkout, windows_plain_templates) and + BEFORE the build / smoke test steps. +runs: + using: composite + steps: + - name: Uninstall Git for Windows + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + $uninstaller = 'C:\Program Files\Git\unins000.exe' + if (Test-Path $uninstaller) { + Write-Host "Running Git for Windows Inno Setup uninstaller..." + & $uninstaller /VERYSILENT /NORESTART /SUPPRESSMSGBOXES + # The parent exits at once and Inno Setup finishes the work in a copy + # of itself under %TEMP%, named _iu*.tmp rather than unins000, so wait + # on both before wiping the leftover dir. + $deadline = (Get-Date).AddSeconds(120) + while ((Get-Date) -lt $deadline -and + (Get-Process -Name 'unins000', '_iu*' -ErrorAction SilentlyContinue)) { + Start-Sleep -Seconds 2 + } + } else { + Write-Host "No Git for Windows uninstaller at $uninstaller; skipping." + } + foreach ($d in 'C:\Program Files\Git', 'C:\Program Files (x86)\Git') { + if (Test-Path $d) { + Write-Host "Removing leftover $d" + Remove-Item $d -Recurse -Force -ErrorAction SilentlyContinue + } + } + # The uninstaller forks, so its own exit status says nothing about the + # result. Discard it and let the verify step below be the gate. + exit 0 + + - name: Uninstall MSYS2 and standalone mingw + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + # MSYS2 on this runner image is an extracted tree, not a package, so + # "uninstall" means killing any process still mapping msys-2.0.dll + # then removing the dir. C:\mingw64 and C:\mingw32 are the same shape. + $procs = Get-Process | Where-Object { + try { $_.Modules.ModuleName -contains 'msys-2.0.dll' } catch { $false } + } + if ($procs) { + Write-Host "Stopping $($procs.Count) process(es) still mapping msys-2.0.dll" + $procs | Stop-Process -Force -ErrorAction SilentlyContinue + Start-Sleep -Seconds 1 + } + foreach ($d in 'C:\msys64', 'C:\tools\msys64', 'C:\msys32', + 'C:\mingw64', 'C:\mingw32') { + if (Test-Path $d) { + Write-Host "Removing $d" + Remove-Item $d -Recurse -Force -ErrorAction SilentlyContinue + } + } + + - name: Scrub forbidden entries from Machine + User PATH + shell: pwsh + # The PATH= we publish here is the runner's own Machine + User PATH + # with a static deny-list of toolchain prefixes removed — no + # PR-controlled data reaches the env file. + run: | # zizmor: ignore[github-env] + $ErrorActionPreference = 'Stop' + # The uninstall removes the dirs, but the Machine/User PATH still has + # stale entries (C:\mingw64\bin stays registered). Rewrite both scopes + # so the verify step sees a clean host, then publish the combined PATH + # via $GITHUB_ENV, because registry changes do not reach the running + # runner process. $GITHUB_PATH entries from earlier steps are still + # prepended on top of this value, so the scrub cannot reach them. + $forbidden = '(?i)(msys|mingw|cygwin|\\Git\\(cmd|bin|usr))' + foreach ($scope in @('Machine','User')) { + $orig = [Environment]::GetEnvironmentVariable('Path', $scope) + if (-not $orig) { continue } + $cleaned = ($orig -split ';' | Where-Object { $_ -and ($_ -notmatch $forbidden) }) -join ';' + if ($cleaned -ne $orig) { + Write-Host "Scrubbing $scope PATH" + [Environment]::SetEnvironmentVariable('Path', $cleaned, $scope) + } + } + $machine = [Environment]::GetEnvironmentVariable('Path','Machine') + $user = [Environment]::GetEnvironmentVariable('Path','User') + $combined = (@($machine, $user) | Where-Object { $_ }) -join ';' + Add-Content -Path $env:GITHUB_ENV -Value "PATH=$combined" + + - name: Verify host is vanilla Windows (no MSYS2 / Git for Windows / Cygwin) + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + # The PATH= published by the scrub step already applies to this step, so + # the inherited PATH is what later steps see. Check it as inherited + # rather than rebuilding it, which would only re-read the scrub's input. + $fail = @() + + # --- Layer 1: filesystem --- + $fsForbidden = @( + 'C:\msys64', 'C:\tools\msys64', 'C:\msys32', + 'C:\Program Files\Git', 'C:\Program Files (x86)\Git', + 'C:\mingw64', 'C:\mingw32', + 'C:\cygwin', 'C:\cygwin64', 'C:\tools\cygwin' + ) + foreach ($d in $fsForbidden) { + if (Test-Path $d) { $fail += "filesystem: $d still exists" } + } + + # --- Layer 2: PATH (process + machine + user) --- + $pathRegex = '(?i)(msys|mingw|cygwin|\\Git\\(cmd|bin|usr))' + foreach ($scope in @('Process','Machine','User')) { + $p = [Environment]::GetEnvironmentVariable('Path', $scope) + if (-not $p) { continue } + foreach ($entry in $p -split ';') { + if ($entry -and ($entry -match $pathRegex)) { + $fail += ("PATH ({0}): {1}" -f $scope, $entry) + } + } + } + + # --- Layer 3: smoking-gun binaries --- + # Any of these resolving on PATH means a toolchain is still reachable. + # bash is special: WSL ships %SystemRoot%\System32\bash.exe which is + # legitimately present on the wsl2 plain job, so allow only that path. + $forbiddenCmds = @('cygpath','pacman','mintty','git','git-bash','git-cmd') + foreach ($cmd in $forbiddenCmds) { + $g = Get-Command $cmd -ErrorAction SilentlyContinue + if ($g) { $fail += ("binary: {0} -> {1}" -f $cmd, $g.Source) } + } + $allowedBash = Join-Path $env:SystemRoot 'System32\bash.exe' + $bash = Get-Command bash -ErrorAction SilentlyContinue + if ($bash -and ($bash.Source -ne $allowedBash)) { + $fail += "binary: bash -> $($bash.Source) (only $allowedBash is allowed)" + } + $sh = Get-Command sh -ErrorAction SilentlyContinue + if ($sh) { $fail += "binary: sh -> $($sh.Source)" } + + # --- Layer 4: registry uninstall keys --- + $uninstallRoots = @( + 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall', + 'HKLM:\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall', + 'HKCU:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall' + ) + foreach ($root in $uninstallRoots) { + if (-not (Test-Path $root)) { continue } + $keys = Get-ChildItem $root -ErrorAction SilentlyContinue + foreach ($k in $keys) { + $props = Get-ItemProperty $k.PSPath -ErrorAction SilentlyContinue + if ($props -and $props.DisplayName -and + ($props.DisplayName -match '(?i)(msys|git for windows|cygwin|mingw)')) { + $fail += ("registry: {0} -> DisplayName='{1}'" -f $k.PSPath, $props.DisplayName) + } + } + } + + # --- Layer 5: the toolchain these jobs do need --- + # Missing ssh.exe fails the job with an opaque `exec: "ssh": executable + # file not found`; missing scp.exe or ssh-keygen.exe makes Lima fall + # back to a bare ssh lookup and fail later elsewhere; missing + # sftp-server.exe drops reverse-sshfs to the builtin driver, so the job + # stops covering what it was added to cover. + $opensshDir = Join-Path $env:SystemRoot 'System32\OpenSSH' + foreach ($exe in 'ssh.exe', 'scp.exe', 'ssh-keygen.exe', 'sftp-server.exe') { + $exePath = Join-Path $opensshDir $exe + if (-not (Test-Path $exePath)) { $fail += "missing: $exePath" } + } + + if ($fail.Count -gt 0) { + Write-Host "Plain-Windows verification FAILED:" -ForegroundColor Red + $fail | ForEach-Object { Write-Host " $_" } + Write-Error "Runner is not in the expected plain-Windows state; see the list above." + exit 1 + } + Write-Host "Plain-Windows verification PASSED." diff --git a/.github/actions/windows_plain_smoke_test/action.yml b/.github/actions/windows_plain_smoke_test/action.yml new file mode 100644 index 00000000000..180f7bc9999 --- /dev/null +++ b/.github/actions/windows_plain_smoke_test/action.yml @@ -0,0 +1,89 @@ +name: 'windows plain smoke test' +description: > + Runs Lima's create / start / shell / copy / stop / delete cycle against a + given template on a plain Windows host. Each limactl invocation runs with + --debug so traces land in the workflow log, and on failure the workflow + dumps the instance's hostagent / serial logs so the run carries enough + context to diagnose without re-running. +inputs: + template: + description: Path to the Lima template (e.g. .\templates\experimental\wsl2.yaml). + required: true + instance-name: + description: Name to use for the test instance. + required: false + default: plain + lima-home-suffix: + description: > + Suffix appended to runner.temp for LIMA_HOME (e.g. "plain-wsl2"). Must be + unique per job sharing a runner so concurrent jobs cannot collide. + required: true + vm-type: + description: > + Lima vmType to force via `--vm-type` (e.g. "qemu" or "wsl2"). When empty, + `limactl create` auto-selects, which on Windows prefers wsl2 over qemu + even when the template ships a disk image incompatible with wsl2. + required: false + default: '' +runs: + using: composite + steps: + - name: Smoke test (create / start / shell / copy / stop / delete) + shell: pwsh + env: + LIMA_HOME: ${{ runner.temp }}\lima-${{ inputs.lima-home-suffix }} + INSTANCE: ${{ inputs.instance-name }} + TEMPLATE: ${{ inputs.template }} + VM_TYPE: ${{ inputs.vm-type }} + run: | + $ErrorActionPreference = 'Stop' + # $ErrorActionPreference does not propagate to native commands; wrap + # limactl so a non-zero exit aborts the whole script instead of + # silently continuing past the failure. + function Invoke-Limactl { + & .\_output\bin\limactl.exe --debug @args + if ($LASTEXITCODE -ne 0) { throw "limactl $($args -join ' ') exited $LASTEXITCODE" } + } + if (Test-Path $env:LIMA_HOME) { Remove-Item $env:LIMA_HOME -Recurse -Force } + New-Item -ItemType Directory -Path $env:LIMA_HOME | Out-Null + $createArgs = @('--tty=false', "--name=$env:INSTANCE") + if ($env:VM_TYPE) { $createArgs += "--vm-type=$env:VM_TYPE" } + $createArgs += $env:TEMPLATE + Invoke-Limactl create @createArgs + Invoke-Limactl start $env:INSTANCE + Invoke-Limactl shell --tty=false $env:INSTANCE -- uname -srm + 'roundtrip' | Out-File -Encoding ascii "$env:LIMA_HOME\rt.txt" + Invoke-Limactl copy "$env:LIMA_HOME\rt.txt" "${env:INSTANCE}:/tmp/rt.txt" + Invoke-Limactl shell --tty=false $env:INSTANCE -- cat /tmp/rt.txt + # Cover the guest -> host direction too: it runs through different + # parseCopyPaths plumbing than the upload above. + Invoke-Limactl copy "${env:INSTANCE}:/tmp/rt.txt" "$env:LIMA_HOME\rt-back.txt" + # -Raw yields $null for an empty file, and $null.Trim() would throw + # before the mismatch message a zero-byte copy is meant to produce. + $back = "$(Get-Content "$env:LIMA_HOME\rt-back.txt" -Raw)".Trim() + if ($back -ne 'roundtrip') { + throw "round-trip mismatch: expected 'roundtrip', got '$back'" + } + Invoke-Limactl stop $env:INSTANCE + Invoke-Limactl delete --force $env:INSTANCE + + - name: Dump Lima logs on failure + if: failure() + shell: pwsh + env: + LIMA_HOME: ${{ runner.temp }}\lima-${{ inputs.lima-home-suffix }} + INSTANCE: ${{ inputs.instance-name }} + run: | + $instDir = Join-Path $env:LIMA_HOME $env:INSTANCE + if (-not (Test-Path $instDir)) { + Write-Host "No instance directory at $instDir, nothing to dump." + exit 0 + } + Get-ChildItem $instDir | Format-Table Name, Length, LastWriteTime + foreach ($name in 'ha.stdout.log','ha.stderr.log','serial.log','lima.yaml','ssh.config') { + $p = Join-Path $instDir $name + if (Test-Path $p) { + Write-Host ("===== {0} =====" -f $p) + Get-Content $p + } + } diff --git a/.github/actions/windows_plain_templates/action.yml b/.github/actions/windows_plain_templates/action.yml new file mode 100644 index 00000000000..231180121aa --- /dev/null +++ b/.github/actions/windows_plain_templates/action.yml @@ -0,0 +1,81 @@ +name: 'windows plain templates' +description: > + Resolves Lima's template symlinks and copies templates/ to + _output/share/lima/templates/, replicating `make`'s TEMPLATES target without + needing MSYS2 / Git for Windows. + + Several files under templates/ are git symlinks (mode 120000) pointing at + sibling files, and some chain (opensuse.yaml -> opensuse-leap.yaml -> + opensuse-leap-16.yaml). The working-tree representation varies: a runner + with Developer Mode (or admin) preserves them as NTFS symlinks; a plain + Windows checkout with core.symlinks=false writes 17-byte plaintext stubs. + Reading targets directly from git's object store makes resolution + independent of which representation the checkout used. + + This action MUST run before windows_plain_host (which uninstalls Git for + Windows), because it shells out to `git ls-tree` and `git cat-file`. +runs: + using: composite + steps: + - name: Install templates (replicating make's TEMPLATES target) + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + # windows_plain_host uninstalls git, so a job that runs this action after + # it would fail here with a bare not-found. Name the contract instead. + if (-not (Get-Command git -ErrorAction SilentlyContinue)) { + throw "git not found; this action must run before windows_plain_host" + } + $srcRoot = 'templates' + $dstRoot = '_output\share\lima\templates' + if (Test-Path $dstRoot) { Remove-Item $dstRoot -Recurse -Force } + New-Item -ItemType Directory -Force -Path $dstRoot | Out-Null + Copy-Item -Path "$srcRoot\*" -Destination $dstRoot -Recurse -Force + + $linkTargets = @{} + # Capture before iterating: a partial `git ls-tree` failure would let + # foreach run on truncated data, and later `git cat-file` calls would + # overwrite $LASTEXITCODE before the failure could be seen. + $lsTreeOutput = git ls-tree -r HEAD $srcRoot + if ($LASTEXITCODE -ne 0) { throw "git ls-tree failed: $LASTEXITCODE" } + foreach ($line in $lsTreeOutput) { + if ($line -notmatch '^120000\s') { continue } + $parts = $line -split "`t", 2 + $sha = ($parts[0] -split '\s+')[2] + $posixPath = $parts[1] + # A failed cat-file writes only to stderr, so keep the value a string: + # $null.Trim() would throw ahead of the check below. + $target = "$(git cat-file blob $sha)".Trim() + if ($LASTEXITCODE -ne 0) { throw "git cat-file $sha failed: $LASTEXITCODE" } + $linkTargets[$posixPath] = $target + } + + foreach ($posixPath in $linkTargets.Keys) { + $resolved = $posixPath + $depth = 0 + while ($linkTargets.ContainsKey($resolved)) { + if ($depth++ -gt 16) { throw "Symlink chain too deep starting from $posixPath" } + $target = $linkTargets[$resolved] + $slash = $resolved.LastIndexOf('/') + $resolved = if ($slash -ge 0) { "$($resolved.Substring(0, $slash))/$target" } else { $target } + $segs = New-Object System.Collections.ArrayList + foreach ($s in ($resolved -split '/')) { + if ($s -eq '' -or $s -eq '.') { continue } + if ($s -eq '..') { + if ($segs.Count -gt 0) { $segs.RemoveAt($segs.Count - 1) | Out-Null } + continue + } + $segs.Add($s) | Out-Null + } + $resolved = $segs -join '/' + } + # Defensive: the '..' handling above silently underflows. Guard against + # a future symlink edit whose chain resolves outside templates/. + if (-not $resolved.StartsWith("$srcRoot/")) { + throw "Symlink $posixPath resolved to $resolved, which escapes $srcRoot/" + } + $srcFile = $resolved -replace '/', '\' + $dstFile = Join-Path '_output\share\lima' ($posixPath -replace '/', '\') + Write-Host "Resolving symlink: $posixPath -> $resolved" + Copy-Item -Force -Path $srcFile -Destination $dstFile + } diff --git a/.github/actions/windows_qemu_install/action.yml b/.github/actions/windows_qemu_install/action.yml new file mode 100644 index 00000000000..c20ba165830 --- /dev/null +++ b/.github/actions/windows_qemu_install/action.yml @@ -0,0 +1,11 @@ +name: 'windows qemu install' +description: Installs QEMU on the Windows runner via winget. +runs: + using: composite + steps: + - name: Install QEMU + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + winget install --silent --accept-source-agreements --accept-package-agreements --disable-interactivity SoftwareFreedomConservancy.QEMU + if ($LASTEXITCODE -ne 0) { throw "winget install QEMU failed: $LASTEXITCODE" } diff --git a/.github/actions/windows_wsl2_setup/action.yml b/.github/actions/windows_wsl2_setup/action.yml new file mode 100644 index 00000000000..563a5d1ef50 --- /dev/null +++ b/.github/actions/windows_wsl2_setup/action.yml @@ -0,0 +1,62 @@ +name: 'windows wsl2 setup' +description: > + Enables WSL2 on the Windows runner and imports a dummy distro. The dummy + distro is required because `wsl --list --verbose` (called from Lima) fails + when no distro is installed -- see lima-vm/lima#1826. Starting with WSL2 + 2.5.7.0 the imported tarball must contain /bin/sh and /etc, hence the + scaffolding here. +runs: + using: composite + steps: + - name: Enable WSL2 + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + # PowerShell's $ErrorActionPreference does not propagate to native + # commands; check $LASTEXITCODE after every wsl invocation that + # gates the runner state. + wsl --set-default-version 2 + if ($LASTEXITCODE -ne 0) { throw "wsl --set-default-version failed: $LASTEXITCODE" } + wsl --shutdown + if ($LASTEXITCODE -ne 0) { throw "wsl --shutdown failed: $LASTEXITCODE" } + # --update reaches a Microsoft update service that returns + # transient 403s; treat it as informational, like --status / + # --version / --list --online below. + wsl --update + wsl --status + wsl --version + wsl --list --online + # pwsh exits with the last command's status, and --list --online reaches + # that same service, so discard it to keep the four calls informational. + exit 0 + - name: Install dummy WSL2 distro + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + # Keep the scratch tarball and the distro disk under $RUNNER_TEMP so no + # untracked files land in the checked-out workspace. + $scratch = Join-Path $env:RUNNER_TEMP 'wsl2-dummy' + New-Item -ItemType Directory -Force -Path "$scratch\bin","$scratch\etc" | Out-Null + Set-Content -Path "$scratch\bin\sh" -Value '' + $tarball = Join-Path $env:RUNNER_TEMP 'wsl2-dummy.tar' + tar -cf $tarball --format ustar -C $scratch . + if ($LASTEXITCODE -ne 0) { throw "tar -cf $tarball failed: $LASTEXITCODE" } + # A fresh runner has no `dummy` to unregister; not checking $LASTEXITCODE + # is what makes that failure harmless. The call matters on a developer + # machine, where the import would otherwise fail on the second run. + wsl --unregister dummy + $distroDir = Join-Path $env:RUNNER_TEMP 'wsl2-dummy-distro' + New-Item -ItemType Directory -Force -Path $distroDir | Out-Null + # A composite action step cannot carry timeout-minutes, so bound the + # import here; it is the call that has hung on runners before. + # -ArgumentList joins its elements unquoted, so quote the two paths. + $p = Start-Process -FilePath wsl.exe -PassThru -NoNewWindow ` + -ArgumentList '--import', 'dummy', "`"$distroDir`"", "`"$tarball`"" + if (-not $p.WaitForExit(60000)) { + $p.Kill() + throw "wsl --import dummy timed out after 60s" + } + if ($p.ExitCode -ne 0) { throw "wsl --import dummy failed: $($p.ExitCode)" } + wsl --list --verbose + # Informational, like the listings in the step above. + exit 0 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 67b6cc8c82c..a491a47a374 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -145,36 +145,11 @@ jobs: - name: Uninstall run: sudo make uninstall - windows: + windows-wsl2: name: "Windows tests (WSL2)" runs-on: windows-2025 timeout-minutes: 30 steps: - - name: Enable WSL2 - run: | - wsl --set-default-version 2 - wsl --shutdown - wsl --update - wsl --status - wsl --version - wsl --list --online - - name: Install WSL2 distro - timeout-minutes: 1 - run: | - # FIXME: At least one distro has to be installed here, - # otherwise `wsl --list --verbose` (called from Lima) fails: - # https://github.com/lima-vm/lima/pull/1826#issuecomment-1729993334 - # The distro image itself is not consumed by Lima. - # Starting with WSL2 version 2.5.7.0 the distro will be rejected - # if it doesn't contain /bin/sh and /etc. - # ------------------------------------------------------------------ - mkdir dummy - mkdir dummy\bin - mkdir dummy\etc - echo "" >dummy\bin\sh - tar -cf dummy.tar --format ustar -C dummy . - wsl --import dummy $env:TEMP dummy.tar - wsl --list --verbose - name: Set gitconfig run: | git config --global core.autocrlf false @@ -186,19 +161,19 @@ jobs: with: go-version: stable cache: false + - uses: ./.github/actions/windows_wsl2_setup + - uses: ./.github/actions/windows_msys2_prep - name: Unit tests run: go test -v ./... - name: Make run: make - name: Integration tests (WSL2, Windows host) run: | - $env:PATH = "$pwd\_output\bin;" + 'C:\msys64\usr\bin;' + $env:PATH - pacman -Sy --noconfirm openbsd-netcat diffutils socat w3m - $env:MSYS2_ENV_CONV_EXCL = 'HOME_HOST;HOME_GUEST;_LIMA_WINDOWS_EXTRA_PATH' + $env:PATH = "$pwd\_output\bin;" + $env:PATH + $env:MSYS2_ENV_CONV_EXCL = 'HOME_HOST;HOME_GUEST' $env:HOME_HOST = $(cygpath.exe "$env:USERPROFILE") $env:HOME_GUEST = "/mnt$env:HOME_HOST" $env:LIMACTL_CREATE_ARGS = '--vm-type=wsl2 --mount-type=wsl2 --containerd=system' - $env:_LIMA_WINDOWS_EXTRA_PATH = 'C:\Program Files\Git\usr\bin' bash.exe -c "./hack/test-templates.sh templates/experimental/wsl2.yaml" windows-qemu: @@ -217,22 +192,93 @@ jobs: with: go-version: stable cache: false + - uses: ./.github/actions/windows_qemu_install + - uses: ./.github/actions/windows_msys2_prep - name: Make run: make - - name: Install QEMU - run: | - winget install --silent --accept-source-agreements --accept-package-agreements --disable-interactivity SoftwareFreedomConservancy.QEMU - name: Integration tests (QEMU, Windows host) run: | - $env:PATH = "$pwd\_output\bin;" + 'C:\msys64\usr\bin;' + 'C:\Program Files\QEMU;' + $env:PATH - pacman -Sy --noconfirm openbsd-netcat diffutils socat w3m - $env:MSYS2_ENV_CONV_EXCL = 'HOME_HOST;HOME_GUEST;_LIMA_WINDOWS_EXTRA_PATH' + $env:PATH = "$pwd\_output\bin;" + 'C:\Program Files\QEMU;' + $env:PATH + $env:MSYS2_ENV_CONV_EXCL = 'HOME_HOST;HOME_GUEST' $env:HOME_HOST = $(cygpath.exe "$env:USERPROFILE") $env:HOME_GUEST = "$env:HOME_HOST" $env:LIMACTL_CREATE_ARGS = '--vm-type=qemu' - $env:_LIMA_WINDOWS_EXTRA_PATH = 'C:\Program Files\Git\usr\bin' bash.exe -c "./hack/test-templates.sh templates/default.yaml" + windows-plain-wsl2: + # Smoke test that Lima's WSL2 driver works on a Windows host with only + # the bits that ship in a default Windows 10/11 install (native OpenSSH, + # wsl.exe, tar). Notably: no Git for Windows, no MSYS2, no cygpath, no + # Cygwin/MSYS2 ssh. Builds with `go build` (instead of `make`) and + # exercises the main commands in PowerShell. If this job ever needs to + # add MSYS2 / Git for Windows back to make a step pass, that means a new + # external-binary dependency was introduced and should be evaluated. + name: "Windows tests (WSL2, plain Windows host)" + runs-on: windows-2025 + timeout-minutes: 30 + steps: + - name: Set gitconfig + run: | + git config --global core.autocrlf false + git config --global core.eol lf + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: stable + cache: false + - uses: ./.github/actions/windows_wsl2_setup + # No windows_plain_templates here: wsl2.yaml carries no `base:`, so this + # job never reads the resolved template store that action builds. + - uses: ./.github/actions/windows_plain_host + - uses: ./.github/actions/windows_plain_build + - uses: ./.github/actions/windows_plain_smoke_test + with: + template: .\templates\experimental\wsl2.yaml + lima-home-suffix: plain-wsl2 + + windows-plain-qemu: + # Same shape as windows-plain-wsl2 but exercises the QEMU driver. Covers + # native OpenSSH selection and cygpath-free path translation without + # MSYS2 / Git-for-Windows on PATH. The reverse-sshfs mount starts through + # the native sftp-server, but its confirmation step fails on the runner + # and sshocker treats that as non-fatal, so the mount itself is uncovered. + name: "Windows tests (QEMU, plain Windows host)" + runs-on: windows-2025 + timeout-minutes: 30 + steps: + - name: Set gitconfig + run: | + git config --global core.autocrlf false + git config --global core.eol lf + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: stable + cache: false + - uses: ./.github/actions/windows_qemu_install + - uses: ./.github/actions/windows_plain_templates + - uses: ./.github/actions/windows_plain_host + # winget's QEMU manifest does not register a PATH entry, so add the + # install dir here, extending the scrubbed PATH= that windows_plain_host + # published through $GITHUB_ENV. + - name: Add QEMU to PATH + shell: pwsh + run: | + Add-Content -Path $env:GITHUB_ENV -Value "PATH=C:\Program Files\QEMU;$env:PATH" + - uses: ./.github/actions/windows_plain_build + - uses: ./.github/actions/windows_plain_smoke_test + with: + template: .\templates\default.yaml + lima-home-suffix: plain-qemu + # Force qemu: on Windows, `limactl create` auto-selects wsl2 + # over qemu, and templates/default.yaml ships a cloud disk + # image that the wsl2 driver rejects. + vm-type: qemu + qemu: name: "Integration tests (QEMU, macOS host)" runs-on: macos-15-large # Intel diff --git a/cmd/limactl/main.go b/cmd/limactl/main.go index ba3761d8453..e625de64e8f 100644 --- a/cmd/limactl/main.go +++ b/cmd/limactl/main.go @@ -38,16 +38,6 @@ func main() { } yq.MaybeRunYQ() - if runtime.GOOS == "windows" { - extras, hasExtra := os.LookupEnv("_LIMA_WINDOWS_EXTRA_PATH") - if hasExtra && strings.TrimSpace(extras) != "" { - p := os.Getenv("PATH") - err := os.Setenv("PATH", strings.TrimSpace(extras)+string(filepath.ListSeparator)+p) - if err != nil { - logrus.Warning("Can't add extras to PATH, relying entirely on system PATH") - } - } - } err := newApp().Execute() server.Disconnect() osutil.HandleExitError(err) diff --git a/pkg/sshutil/sshutil_windows_test.go b/pkg/sshutil/sshutil_windows_test.go index 50e8d7627aa..eb331abcca1 100644 --- a/pkg/sshutil/sshutil_windows_test.go +++ b/pkg/sshutil/sshutil_windows_test.go @@ -83,6 +83,42 @@ func TestPickCompleteSSHOnWindows(t *testing.T) { t.Setenv("PATH", mingit) assert.Equal(t, pickCompleteSSHOnWindows(), nativeSSH) }) + + t.Run("incomplete native install disqualifies it too", func(t *testing.T) { + fakeRoot := resolvedTempDir(t) + nativeDir := filepath.Join(fakeRoot, "System32", "OpenSSH") + assert.NilError(t, os.MkdirAll(nativeDir, 0o755)) + assert.NilError(t, os.WriteFile(filepath.Join(nativeDir, "ssh.exe"), nil, 0o644)) + t.Setenv("SystemRoot", fakeRoot) + t.Setenv("PATH", "") + + assert.Equal(t, pickCompleteSSHOnWindows(), "", + "an ssh.exe without its companions never qualifies, wherever it lives") + }) +} + +// TestNewSSHExeFallsBackToPATH: when no directory holds a complete install, +// NewSSHExe stops being selective and takes whatever `ssh` PATH resolves. The +// binary it returns is therefore reachable only through PATH, which is why a +// host check that only proves the files exist cannot show which ssh Lima runs. +func TestNewSSHExeFallsBackToPATH(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows-only PATH walk") + } + + partial := resolvedTempDir(t) + sshExe := filepath.Join(partial, "ssh.exe") + assert.NilError(t, os.WriteFile(sshExe, nil, 0o644)) + + t.Setenv("SystemRoot", resolvedTempDir(t)) + t.Setenv("PATH", partial) + t.Setenv(EnvShellSSH, "") + + assert.Equal(t, pickCompleteSSHOnWindows(), "", "the partial install must not qualify") + + got, err := NewSSHExe() + assert.NilError(t, err) + assert.Equal(t, got.Exe, sshExe) } // TestCygpathForSSH: an ssh.exe next to cygpath.exe is Cygwin-based and diff --git a/website/content/en/docs/config/environment-variables.md b/website/content/en/docs/config/environment-variables.md index ee034a120b5..395b1e6c309 100644 --- a/website/content/en/docs/config/environment-variables.md +++ b/website/content/en/docs/config/environment-variables.md @@ -147,20 +147,6 @@ This page documents the environment variables used in Lima. export LIMA_USERNET_RESOLVE_IP_ADDRESS_TIMEOUT=5 ``` -### `_LIMA_WINDOWS_EXTRA_PATH` - -- **Description**: Additional directories which will be added to PATH by `limactl.exe` process to search for tools. - It is useful, when there is a need to prevent collisions between binaries available in active shell and ones - used by `limactl.exe` - injecting them only for the running process w/o altering PATH observed by user shell. - Is is Windows specific and does nothing for other platforms. -- **Default**: unset -- **Usage**: - ```bat - set _LIMA_WINDOWS_EXTRA_PATH=C:\Program Files\Git\usr\bin - ``` -- **Note**: It is an experimental setting and has no guarantees being ever promoted to stable. It may be removed - or changed at any stage of project development. - ### `QEMU_SYSTEM_AARCH64` - **Description**: Path to the `qemu-system-aarch64` binary. diff --git a/website/content/en/docs/releases/breaking.md b/website/content/en/docs/releases/breaking.md index 858a7876185..ee8550c9e79 100644 --- a/website/content/en/docs/releases/breaking.md +++ b/website/content/en/docs/releases/breaking.md @@ -3,6 +3,11 @@ title: Breaking changes weight: 20 --- +## v2.3.0 +- The experimental `_LIMA_WINDOWS_EXTRA_PATH` environment variable was removed. It prepended directories to + `limactl.exe`'s own PATH on Windows hosts, leaving the calling shell's PATH untouched. Put them on `PATH` + instead, or set `PATH` for the single call if you need to keep them out of your shell. + ## v2.2.0 - The default [`socket_vmnet` group](../config/network/vmnet.md) in `networks.yaml` was changed from `everyone` to `admin`. A non-admin user must set `group` to a group they belong to (e.g. `staff`).