Do tilde-expansion only once in FillDefaults()

jandubois · jandubois · commit fc308733a4de · 2025-03-11T19:01:18.000-07:00
We need to do the expansion before we copy the Location
over as the default MountPoint because we can't do tilde
expansion for guest filenames (the existing call in
cidata.go was an error).

Signed-off-by: Jan Dubois &lt;jan.dubois@suse.com&gt;
diff --git a/pkg/cidata/cidata.go b/pkg/cidata/cidata.go
@@ -200,14 +200,6 @@ func templateArgs(bootScripts bool, instDir, name string, instConfig *limayaml.L
 	}
 	for i, f := range instConfig.Mounts {
 		tag := fmt.Sprintf("mount%d", i)
-		location, err := localpathutil.Expand(f.Location)
-		if err != nil {
-			return nil, err
-		}
-		mountPoint, err := localpathutil.Expand(*f.MountPoint)
-		if err != nil {
-			return nil, err
-		}
 		options := "defaults"
 		switch fstype {
 		case "9p", "virtiofs":
@@ -220,17 +212,17 @@ func templateArgs(bootScripts bool, instDir, name string, instConfig *limayaml.L
 				options += fmt.Sprintf(",version=%s", *f.NineP.ProtocolVersion)
 				msize, err := units.RAMInBytes(*f.NineP.Msize)
 				if err != nil {
-					return nil, fmt.Errorf("failed to parse msize for %q: %w", location, err)
+					return nil, fmt.Errorf("failed to parse msize for %q: %w", f.Location, err)
 				}
 				options += fmt.Sprintf(",msize=%d", msize)
 				options += fmt.Sprintf(",cache=%s", *f.NineP.Cache)
 			}
 			// don't fail the boot, if virtfs is not available
 			options += ",nofail"
 		}
-		args.Mounts = append(args.Mounts, Mount{Tag: tag, MountPoint: mountPoint, Type: fstype, Options: options})
-		if location == hostHome {
-			args.HostHomeMountPoint = mountPoint
+		args.Mounts = append(args.Mounts, Mount{Tag: tag, MountPoint: *f.MountPoint, Type: fstype, Options: options})
+		if f.Location == hostHome {
+			args.HostHomeMountPoint = *f.MountPoint
 		}
 	}
 
diff --git a/pkg/hostagent/inotify.go b/pkg/hostagent/inotify.go
@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	guestagentapi "github.com/lima-vm/lima/pkg/guestagent/api"
-	"github.com/lima-vm/lima/pkg/localpathutil"
 	"github.com/rjeczalik/notify"
 	"github.com/sirupsen/logrus"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -74,20 +73,16 @@ func (a *HostAgent) setupWatchers(events chan notify.EventInfo) error {
 		if !*m.Writable {
 			continue
 		}
-		location, err := localpathutil.Expand(m.Location)
+		symlink, err := filepath.EvalSymlinks(m.Location)
 		if err != nil {
 			return err
 		}
-		symlink, err := filepath.EvalSymlinks(location)
-		if err != nil {
-			return err
-		}
-		if location != symlink {
-			mountSymlinks[symlink] = location
+		if m.Location != symlink {
+			mountSymlinks[symlink] = m.Location
 		}
 
-		logrus.Infof("enable inotify for writable mount: %s", location)
-		err = notify.Watch(path.Join(location, "..."), events, GetNotifyEvent())
+		logrus.Infof("enable inotify for writable mount: %s", m.Location)
+		err = notify.Watch(path.Join(m.Location, "..."), events, GetNotifyEvent())
 		if err != nil {
 			return err
 		}
diff --git a/pkg/hostagent/mount.go b/pkg/hostagent/mount.go
@@ -9,7 +9,6 @@ import (
 	"os"
 
 	"github.com/lima-vm/lima/pkg/limayaml"
-	"github.com/lima-vm/lima/pkg/localpathutil"
 	"github.com/lima-vm/sshocker/pkg/reversesshfs"
 	"github.com/sirupsen/logrus"
 )
@@ -35,16 +34,7 @@ func (a *HostAgent) setupMounts() ([]*mount, error) {
 }
 
 func (a *HostAgent) setupMount(m limayaml.Mount) (*mount, error) {
-	location, err := localpathutil.Expand(m.Location)
-	if err != nil {
-		return nil, err
-	}
-
-	mountPoint, err := localpathutil.Expand(*m.MountPoint)
-	if err != nil {
-		return nil, err
-	}
-	if err := os.MkdirAll(location, 0o755); err != nil {
+	if err := os.MkdirAll(m.Location, 0o755); err != nil {
 		return nil, err
 	}
 	// NOTE: allow_other requires "user_allow_other" in /etc/fuse.conf
@@ -55,35 +45,35 @@ func (a *HostAgent) setupMount(m limayaml.Mount) (*mount, error) {
 	if *m.SSHFS.FollowSymlinks {
 		sshfsOptions += ",follow_symlinks"
 	}
-	logrus.Infof("Mounting %q on %q", location, mountPoint)
+	logrus.Infof("Mounting %q on %q", m.Location, *m.MountPoint)
 
 	rsf := &reversesshfs.ReverseSSHFS{
 		Driver:              *m.SSHFS.SFTPDriver,
 		SSHConfig:           a.sshConfig,
-		LocalPath:           location,
+		LocalPath:           m.Location,
 		Host:                "127.0.0.1",
 		Port:                a.sshLocalPort,
-		RemotePath:          mountPoint,
+		RemotePath:          *m.MountPoint,
 		Readonly:            !(*m.Writable),
 		SSHFSAdditionalArgs: []string{"-o", sshfsOptions},
 	}
 	if err := rsf.Prepare(); err != nil {
-		return nil, fmt.Errorf("failed to prepare reverse sshfs for %q on %q: %w", location, mountPoint, err)
+		return nil, fmt.Errorf("failed to prepare reverse sshfs for %q on %q: %w", m.Location, *m.MountPoint, err)
 	}
 	if err := rsf.Start(); err != nil {
-		logrus.WithError(err).Warnf("failed to mount reverse sshfs for %q on %q, retrying with `-o nonempty`", location, mountPoint)
+		logrus.WithError(err).Warnf("failed to mount reverse sshfs for %q on %q, retrying with `-o nonempty`", m.Location, *m.MountPoint)
 		// NOTE: nonempty is not supported for libfuse3: https://github.com/canonical/multipass/issues/1381
 		rsf.SSHFSAdditionalArgs = []string{"-o", "nonempty"}
 		if err := rsf.Start(); err != nil {
-			return nil, fmt.Errorf("failed to mount reverse sshfs for %q on %q: %w", location, mountPoint, err)
+			return nil, fmt.Errorf("failed to mount reverse sshfs for %q on %q: %w", m.Location, *m.MountPoint, err)
 		}
 	}
 
 	res := &mount{
 		close: func() error {
-			logrus.Infof("Unmounting %q", location)
-			if closeErr := rsf.Close(); closeErr != nil {
-				return fmt.Errorf("failed to unmount reverse sshfs for %q on %q: %w", location, mountPoint, err)
+			logrus.Infof("Unmounting %q", m.Location)
+			if err := rsf.Close(); err != nil {
+				return fmt.Errorf("failed to unmount reverse sshfs for %q on %q: %w", m.Location, *m.MountPoint, err)
 			}
 			return nil
 		},
diff --git a/pkg/limayaml/defaults.go b/pkg/limayaml/defaults.go
@@ -29,6 +29,7 @@ import (
 	"golang.org/x/sys/cpu"
 
 	"github.com/lima-vm/lima/pkg/identifierutil"
+	"github.com/lima-vm/lima/pkg/localpathutil"
 	. "github.com/lima-vm/lima/pkg/must"
 	"github.com/lima-vm/lima/pkg/networks"
 	"github.com/lima-vm/lima/pkg/osutil"
@@ -785,8 +786,13 @@ func FillDefault(y, d, o *LimaYAML, filePath string, warn bool) {
 				mounts[i].NineP.Cache = ptr.Of(Default9pCacheForRO)
 			}
 		}
+		if location, err := localpathutil.Expand(mount.Location); err == nil {
+			mounts[i].Location = location
+		} else {
+			logrus.WithError(err).Warnf("Couldn't expand location %q", mount.Location)
+		}
 		if mount.MountPoint == nil {
-			mounts[i].MountPoint = ptr.Of(mount.Location)
+			mounts[i].MountPoint = ptr.Of(mounts[i].Location)
 		}
 	}
 
diff --git a/pkg/limayaml/defaults_test.go b/pkg/limayaml/defaults_test.go
@@ -146,7 +146,9 @@ func TestFillDefault(t *testing.T) {
 			},
 		},
 		Mounts: []Mount{
-			{Location: "/tmp"},
+			// Location will be passed through localpathutil.Expand() which will normalize the name
+			// (add a drive letter). So we must start with a valid local path to match it again later.
+			{Location: filepath.Clean(os.TempDir())},
 			{Location: "{{.Dir}}/{{.Param.ONE}}", MountPoint: ptr.Of("/mnt/{{.Param.ONE}}")},
 		},
 		MountType: ptr.Of(NINEP),
@@ -231,7 +233,7 @@ func TestFillDefault(t *testing.T) {
 	expect.Mounts[0].NineP.Cache = ptr.Of(Default9pCacheForRO)
 	expect.Mounts[0].Virtiofs.QueueSize = nil
 	// Only missing Mounts field is Writable, and the default value is also the null value: false
-	expect.Mounts[1].Location = fmt.Sprintf("%s/%s", instDir, y.Param["ONE"])
+	expect.Mounts[1].Location = filepath.Join(instDir, y.Param["ONE"])
 	expect.Mounts[1].MountPoint = ptr.Of(fmt.Sprintf("/mnt/%s", y.Param["ONE"]))
 	expect.Mounts[1].Writable = ptr.Of(false)
 	expect.Mounts[1].SSHFS.Cache = ptr.Of(true)
@@ -385,7 +387,9 @@ func TestFillDefault(t *testing.T) {
 
 		Mounts: []Mount{
 			{
-				Location: "/var/log",
+				// Calling filepath.Abs() to add a drive letter on Windows
+				// Location: filepath.Abs("/var/log")
+				Location: func() string { varLog, _ := filepath.Abs("/var/log"); return varLog }(),
 				Writable: ptr.Of(false),
 			},
 		},
diff --git a/pkg/limayaml/validate.go b/pkg/limayaml/validate.go
@@ -154,6 +154,7 @@ func Validate(y *LimaYAML, warn bool) error {
 			return fmt.Errorf("field `mounts[%d].location` must be an absolute path, got %q",
 				i, f.Location)
 		}
+		// f.Location has already been expanded in FillDefaults(), but that function cannot return errors.
 		loc, err := localpathutil.Expand(f.Location)
 		if err != nil {
 			return fmt.Errorf("field `mounts[%d].location` refers to an unexpandable path: %q: %w", i, f.Location, err)
@@ -174,6 +175,10 @@ func Validate(y *LimaYAML, warn bool) error {
 		case *y.User.Home:
 			return fmt.Errorf("field `mounts[%d].mountPoint` is the reserved internal home directory %q", i, *y.User.Home)
 		}
+		// There is no tilde-expansion for guest filenames
+		if strings.HasPrefix(*f.MountPoint, "~") {
+			return fmt.Errorf("field `mounts[%d].mountPoint` must not start with \"~\"", i)
+		}
 
 		if _, err := units.RAMInBytes(*f.NineP.Msize); err != nil {
 			return fmt.Errorf("field `msize` has an invalid value: %w", err)
diff --git a/pkg/qemu/qemu.go b/pkg/qemu/qemu.go
@@ -31,7 +31,6 @@ import (
 	"github.com/lima-vm/lima/pkg/fileutils"
 	"github.com/lima-vm/lima/pkg/iso9660util"
 	"github.com/lima-vm/lima/pkg/limayaml"
-	"github.com/lima-vm/lima/pkg/localpathutil"
 	"github.com/lima-vm/lima/pkg/networks"
 	"github.com/lima-vm/lima/pkg/qemu/imgutil"
 	"github.com/lima-vm/lima/pkg/store"
@@ -935,19 +934,15 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er
 	if *y.MountType == limayaml.NINEP || *y.MountType == limayaml.VIRTIOFS {
 		for i, f := range y.Mounts {
 			tag := fmt.Sprintf("mount%d", i)
-			location, err := localpathutil.Expand(f.Location)
-			if err != nil {
-				return "", nil, err
-			}
-			if err := os.MkdirAll(location, 0o755); err != nil {
+			if err := os.MkdirAll(f.Location, 0o755); err != nil {
 				return "", nil, err
 			}
 
 			switch *y.MountType {
 			case limayaml.NINEP:
 				options := "local"
 				options += fmt.Sprintf(",mount_tag=%s", tag)
-				options += fmt.Sprintf(",path=%s", location)
+				options += fmt.Sprintf(",path=%s", f.Location)
 				options += fmt.Sprintf(",security_model=%s", *f.NineP.SecurityModel)
 				if !*f.Writable {
 					options += ",readonly"
@@ -1067,10 +1062,6 @@ func FindVirtiofsd(qemuExe string) (string, error) {
 
 func VirtiofsdCmdline(cfg Config, mountIndex int) ([]string, error) {
 	mount := cfg.LimaYAML.Mounts[mountIndex]
-	location, err := localpathutil.Expand(mount.Location)
-	if err != nil {
-		return nil, err
-	}
 
 	vhostSock := filepath.Join(cfg.InstanceDir, fmt.Sprintf(filenames.VhostSock, mountIndex))
 	// qemu_driver has to wait for the socket to appear, so make sure any old ones are removed here.
@@ -1080,7 +1071,7 @@ func VirtiofsdCmdline(cfg Config, mountIndex int) ([]string, error) {
 
 	return []string{
 		"--socket-path", vhostSock,
-		"--shared-dir", location,
+		"--shared-dir", mount.Location,
 	}, nil
 }
 
diff --git a/pkg/vz/vm_darwin.go b/pkg/vz/vm_darwin.go
@@ -25,7 +25,6 @@ import (
 	"github.com/lima-vm/lima/pkg/driver"
 	"github.com/lima-vm/lima/pkg/iso9660util"
 	"github.com/lima-vm/lima/pkg/limayaml"
-	"github.com/lima-vm/lima/pkg/localpathutil"
 	"github.com/lima-vm/lima/pkg/nativeimgutil"
 	"github.com/lima-vm/lima/pkg/networks"
 	"github.com/lima-vm/lima/pkg/networks/usernet"
@@ -548,18 +547,14 @@ func attachFolderMounts(driver *driver.BaseDriver, vmConfig *vz.VirtualMachineCo
 	var mounts []vz.DirectorySharingDeviceConfiguration
 	if *driver.Instance.Config.MountType == limayaml.VIRTIOFS {
 		for i, mount := range driver.Instance.Config.Mounts {
-			expandedPath, err := localpathutil.Expand(mount.Location)
-			if err != nil {
-				return err
-			}
-			if _, err := os.Stat(expandedPath); errors.Is(err, os.ErrNotExist) {
-				err := os.MkdirAll(expandedPath, 0o750)
+			if _, err := os.Stat(mount.Location); errors.Is(err, os.ErrNotExist) {
+				err := os.MkdirAll(mount.Location, 0o750)
 				if err != nil {
 					return err
 				}
 			}
 
-			directory, err := vz.NewSharedDirectory(expandedPath, !*mount.Writable)
+			directory, err := vz.NewSharedDirectory(mount.Location, !*mount.Writable)
 			if err != nil {
 				return err
 			}

Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@ import (`
`29`	`29`	`"golang.org/x/sys/cpu"`
`30`	`30`
`31`	`31`	`"github.com/lima-vm/lima/pkg/identifierutil"`
	`32`	`+ "github.com/lima-vm/lima/pkg/localpathutil"`
`32`	`33`	`. "github.com/lima-vm/lima/pkg/must"`
`33`	`34`	`"github.com/lima-vm/lima/pkg/networks"`
`34`	`35`	`"github.com/lima-vm/lima/pkg/osutil"`
`@@ -785,8 +786,13 @@ func FillDefault(y, d, o *LimaYAML, filePath string, warn bool) {`
`785`	`786`	`mounts[i].NineP.Cache = ptr.Of(Default9pCacheForRO)`
`786`	`787`	`}`
`787`	`788`	`}`
	`789`	`+ if location, err := localpathutil.Expand(mount.Location); err == nil {`
	`790`	`+ mounts[i].Location = location`
	`791`	`+ } else {`
	`792`	`+ logrus.WithError(err).Warnf("Couldn't expand location %q", mount.Location)`
	`793`	`+ }`
`788`	`794`	`if mount.MountPoint == nil {`
`789`		`- mounts[i].MountPoint = ptr.Of(mount.Location)`
	`795`	`+ mounts[i].MountPoint = ptr.Of(mounts[i].Location)`
`790`	`796`	`}`
`791`	`797`	`}`
`792`	`798`