Repository: lima-vm/lima

Issue:
Mounts: prohibit relative paths in YAML (while allowing them in `limactl create --mount` CLI)
The current YAML surprisingly allows relative paths (maybe a regression):
```
mounts:
- location: .
```

The actual path changes depending on the cwd of the `limactl start` command.

In Lima v2.0, these relative paths should be prohibited to make the command deterministic.

`limactl create --mount DIR` should continue to support relative paths.
A relative path should be resolved into an absolute path at the time of `limactl create`.


Discussion:
@AkihiroSuda: The current code seems already prohibiting relative paths, but the validation seems not working

https://github.com/lima-vm/lima/blob/11a81a8f55a609dbd8a3e77968a752696d53ab13/pkg/limayaml/validate.go#L108-L111


This is not a v2 regression. The issue is reproducible with v1.2.1 too.

Solution patch:
diff --git a/cmd/limactl/editflags/editflags.go b/cmd/limactl/editflags/editflags.go
index d6b87547..2a11e394 100644
--- a/cmd/limactl/editflags/editflags.go
+++ b/cmd/limactl/editflags/editflags.go
@@ -16,6 +16,7 @@ import (
 	"github.com/spf13/cobra"
 	flag "github.com/spf13/pflag"
 
+	"github.com/lima-vm/lima/v2/pkg/localpathutil"
 	"github.com/lima-vm/lima/v2/pkg/registry"
 )
 
@@ -174,6 +175,10 @@ func buildMountListExpression(ss []string) (string, error) {
 	for i, s := range ss {
 		writable := strings.HasSuffix(s, ":w")
 		loc := strings.TrimSuffix(s, ":w")
+		loc, err := localpathutil.Expand(loc)
+		if err != nil {
+			return "", err
+		}
 		expr += fmt.Sprintf(`{"location": %q, "writable": %v}`, loc, writable)
 		if i < len(ss)-1 {
 			expr += ","
diff --git a/pkg/limayaml/defaults.go b/pkg/limayaml/defaults.go
index ae915b5b..1c96ff8e 100644
--- a/pkg/limayaml/defaults.go
+++ b/pkg/limayaml/defaults.go
@@ -713,10 +713,14 @@ func FillDefault(ctx context.Context, y, d, o *limatype.LimaYAML, filePath strin
 				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)
+
+		// Expand a path that begins with `~`. Relative paths are not modified, and rejected by Validate() later.
+		if localpathutil.IsTildePath(mount.Location) {
+			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 {
 			mountLocation := mounts[i].Location



Test patch:
diff --git a/cmd/limactl/editflags/editflags_test.go b/cmd/limactl/editflags/editflags_test.go
index 7c6c2908..ae9e178e 100644
--- a/cmd/limactl/editflags/editflags_test.go
+++ b/cmd/limactl/editflags/editflags_test.go
@@ -4,10 +4,13 @@
 package editflags
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
 	"gotest.tools/v3/assert"
+
+	"github.com/lima-vm/lima/v2/pkg/localpathutil"
 )
 
 func TestCompleteCPUs(t *testing.T) {
@@ -160,6 +163,13 @@ func TestParsePortForward(t *testing.T) {
 }
 
 func TestYQExpressions(t *testing.T) {
+	expand := func(s string) string {
+		s, err := localpathutil.Expand(s)
+		assert.NilError(t, err)
+		// `D:\foo` -> `D:\\foo` (appears in YAML)
+		s = strings.ReplaceAll(s, "\\", "\\\\")
+		return s
+	}
 	tests := []struct {
 		name        string
 		args        []string
@@ -169,15 +179,15 @@ func TestYQExpressions(t *testing.T) {
 	}{
 		{
 			name:        "mount",
-			args:        []string{"--mount", "/foo", "--mount", "/bar:w"},
+			args:        []string{"--mount", "/foo", "--mount", "./bar:w"},
 			newInstance: false,
-			expected:    []string{`.mounts += [{"location": "/foo", "writable": false},{"location": "/bar", "writable": true}] | .mounts |= unique_by(.location)`},
+			expected:    []string{`.mounts += [{"location": "` + expand("/foo") + `", "writable": false},{"location": "` + expand("./bar") + `", "writable": true}] | .mounts |= unique_by(.location)`},
 		},
 		{
 			name:        "mount-only",
 			args:        []string{"--mount-only", "/foo", "--mount-only", "/bar:w"},
 			newInstance: false,
-			expected:    []string{`.mounts = [{"location": "/foo", "writable": false},{"location": "/bar", "writable": true}]`},
+			expected:    []string{`.mounts = [{"location": "` + expand("/foo") + `", "writable": false},{"location": "` + expand("/bar") + `", "writable": true}]`},
 		},
 		{
 			name:        "mixture of mount and mount-only",
diff --git a/pkg/limayaml/validate_unix_test.go b/pkg/limayaml/validate_unix_test.go
new file mode 100644
index 00000000..97c8cff3
--- /dev/null
+++ b/pkg/limayaml/validate_unix_test.go
@@ -0,0 +1,48 @@
+//go:build !windows
+
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package limayaml
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestValidateMounts(t *testing.T) {
+	yBase := `images: [{"location": "/dummy"}]`
+	tests := []struct {
+		name          string
+		mounts        string
+		skipOnWindows bool
+		wantErr       string
+	}{
+		{
+			name:    "Valid",
+			mounts:  `mounts: [{location: "/foo", writable: false}, {location: "~/foo", writable: true}]`,
+			wantErr: "",
+		},
+		{
+			name:   "Invalid (relative)",
+			mounts: `mounts: [{location: ".", writable: false}]`,
+			wantErr: func() string {
+				return "must be an absolute path"
+			}(),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			y, err := Load(t.Context(), []byte(yBase+"\n"+tt.mounts), "lima.yaml")
+			assert.NilError(t, err)
+			err = Validate(y, false)
+			if tt.wantErr != "" {
+				assert.ErrorContains(t, err, tt.wantErr)
+			} else {
+				assert.NilError(t, err)
+			}
+		})
+	}
+}