Merge pull request #3950 from AkihiroSuda/fix-3948

mounts: prohibit relative paths in YAML

Author: AkihiroSuda

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 += ","

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",

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

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)
+			}
+		})
+	}
+}