Repository: lima-vm/lima

Issue:
VM names with dots are not recognised as names when using with `limactl start` on macOS 15.5
### Description

With lima v1.1.1, on macOS Sonoma 15.5, I cannot start a VM using `limactl start example.com` because it thinks 'example.com' is a path. I must use `limactl start --name=example.com` to have it work correctly. This is a change from the previous version's behaviour.

I am not certain if this bug is specific to lima or macOS, but  Lima is likely using heuristics to determine if an argument is a file path or instance name, and "example.com" triggers the file path logic incorrectly.

Discussion:

Solution patch:
diff --git a/cmd/limactl/edit.go b/cmd/limactl/edit.go
index b24e6141..a411df71 100644
--- a/cmd/limactl/edit.go
+++ b/cmd/limactl/edit.go
@@ -13,7 +13,6 @@ import (
 	"github.com/lima-vm/lima/cmd/limactl/editflags"
 	"github.com/lima-vm/lima/pkg/editutil"
 	"github.com/lima-vm/lima/pkg/instance"
-	"github.com/lima-vm/lima/pkg/limatmpl"
 	"github.com/lima-vm/lima/pkg/limayaml"
 	networks "github.com/lima-vm/lima/pkg/networks/reconcile"
 	"github.com/lima-vm/lima/pkg/store"
@@ -46,33 +45,28 @@ func editAction(cmd *cobra.Command, args []string) error {
 	var filePath string
 	var err error
 	var inst *store.Instance
-	switch {
-	case limatmpl.SeemsYAMLPath(arg):
-		// absolute path is required for `limayaml.Validate`
-		filePath, err = filepath.Abs(arg)
-		if err != nil {
-			return err
-		}
-	default:
-		var instName string
-		if arg != "" {
-			instName = arg
-		} else {
-			instName = DefaultInstanceName
-		}
 
-		inst, err = store.Inspect(instName)
+	if arg == "" {
+		arg = DefaultInstanceName
+	}
+	if err := store.ValidateInstName(arg); err == nil {
+		inst, err = store.Inspect(arg)
 		if err != nil {
 			if errors.Is(err, os.ErrNotExist) {
-				return fmt.Errorf("instance %q not found", instName)
+				return fmt.Errorf("instance %q not found", arg)
 			}
 			return err
 		}
-
 		if inst.Status == store.StatusRunning {
 			return errors.New("cannot edit a running instance")
 		}
 		filePath = filepath.Join(inst.Dir, filenames.LimaYAML)
+	} else {
+		// absolute path is required for `limayaml.Validate`
+		filePath, err = filepath.Abs(arg)
+		if err != nil {
+			return err
+		}
 	}
 
 	yContent, err := os.ReadFile(filePath)
diff --git a/cmd/limactl/start.go b/cmd/limactl/start.go
index c09aaa85..5187de60 100644
--- a/cmd/limactl/start.go
+++ b/cmd/limactl/start.go
@@ -7,12 +7,10 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
 
-	"github.com/containerd/containerd/identifiers"
 	"github.com/lima-vm/lima/cmd/limactl/editflags"
 	"github.com/lima-vm/lima/pkg/editutil"
 	"github.com/lima-vm/lima/pkg/instance"
@@ -118,8 +116,13 @@ func loadOrCreateInstance(cmd *cobra.Command, args []string, createOnly bool) (*
 	if err != nil {
 		return nil, err
 	}
-	if isTemplateURL, templateURL := limatmpl.SeemsTemplateURL(arg); isTemplateURL {
-		templateName := path.Join(templateURL.Host, templateURL.Path)
+	if name != "" {
+		err := store.ValidateInstName(name)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if isTemplateURL, templateName := limatmpl.SeemsTemplateURL(arg); isTemplateURL {
 		switch templateName {
 		case "experimental/vz":
 			logrus.Warn("template://experimental/vz was merged into the default template in Lima v1.0. See also <https://lima-vm.io/docs/config/vmtype/>.")
@@ -144,35 +147,25 @@ func loadOrCreateInstance(cmd *cobra.Command, args []string, createOnly bool) (*
 		// see if the tty was set explicitly or not
 		ttySet := cmd.Flags().Changed("tty")
 		if ttySet && tty {
-			return nil, errors.New("cannot use --tty=true and read template from stdin together")
+			return nil, errors.New("cannot use --tty=true when reading template from stdin")
 		}
 		tty = false
 	}
-	tmpl, err := limatmpl.Read(cmd.Context(), name, arg)
-	if err != nil {
-		return nil, err
-	}
-	if len(tmpl.Bytes) > 0 {
-		if createOnly {
-			if _, err := store.Inspect(tmpl.Name); err == nil {
-				return nil, fmt.Errorf("instance %q already exists", tmpl.Name)
-			}
-		}
-	} else {
+	var tmpl *limatmpl.Template
+	if err := store.ValidateInstName(arg); arg == "" || err == nil {
+		tmpl = &limatmpl.Template{Name: name}
 		if arg == "" {
-			if tmpl.Name == "" {
+			if name == "" {
 				tmpl.Name = DefaultInstanceName
 			}
 		} else {
 			logrus.Debugf("interpreting argument %q as an instance name", arg)
-			if tmpl.Name != "" && tmpl.Name != arg {
+			if name != "" && name != arg {
 				return nil, fmt.Errorf("instance name %q and CLI flag --name=%q cannot be specified together", arg, tmpl.Name)
 			}
 			tmpl.Name = arg
 		}
-		if err := identifiers.Validate(tmpl.Name); err != nil {
-			return nil, fmt.Errorf("argument must be either an instance name, a YAML file path, or a URL, got %q: %w", tmpl.Name, err)
-		}
+		// store.Inspect() will validate the template name (in case it has been set to arg)
 		inst, err := store.Inspect(tmpl.Name)
 		if err == nil {
 			if createOnly {
@@ -204,7 +197,21 @@ func loadOrCreateInstance(cmd *cobra.Command, args []string, createOnly bool) (*
 		if err != nil {
 			return nil, err
 		}
+	} else {
+		tmpl, err = limatmpl.Read(cmd.Context(), name, arg)
+		if err != nil {
+			return nil, err
+		}
+		if createOnly {
+			// store.Inspect() will also validate the instance name
+			if _, err := store.Inspect(tmpl.Name); err == nil {
+				return nil, fmt.Errorf("instance %q already exists", tmpl.Name)
+			}
+		} else if err := store.ValidateInstName(tmpl.Name); err != nil {
+			return nil, err
+		}
 	}
+
 	if err := tmpl.Embed(cmd.Context(), true, true); err != nil {
 		return nil, err
 	}
diff --git a/go.mod b/go.mod
index ad48c336..9faca1a4 100644
--- a/go.mod
+++ b/go.mod
@@ -23,7 +23,7 @@ require (
 	github.com/foxcpp/go-mockdns v1.1.0
 	github.com/goccy/go-yaml v1.17.1
 	github.com/google/go-cmp v0.7.0
-	github.com/google/yamlfmt v0.16.0
+	github.com/google/yamlfmt v0.17.0
 	github.com/invopop/jsonschema v0.13.0
 	github.com/lima-vm/go-qcow2reader v0.6.0
 	github.com/lima-vm/sshocker v0.3.8 // gomodjail:unconfined
@@ -36,7 +36,7 @@ require (
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58
 	github.com/rjeczalik/notify v0.9.3
-	github.com/santhosh-tekuri/jsonschema/v6 v6.0.1
+	github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
 	github.com/sethvargo/go-password v0.3.1
 	github.com/sirupsen/logrus v1.9.4-0.20230606125235-dd1b4c2e81af
 	github.com/spf13/cobra v1.9.1 // gomodjail:unconfined
@@ -62,7 +62,6 @@ require (
 	github.com/alecthomas/participle/v2 v2.1.4 // indirect
 	github.com/bahlo/generic-list-go v0.2.0 // indirect
 	github.com/bmatcuk/doublestar/v4 v4.7.1 // indirect
-	github.com/braydonk/yaml v0.9.0 // indirect
 	github.com/buger/jsonparser v1.1.1 // indirect
 	github.com/containerd/errdefs v1.0.0 // indirect
 	github.com/containerd/log v0.1.0 // indirect
diff --git a/go.sum b/go.sum
index 30c2aadf..73daa18e 100644
--- a/go.sum
+++ b/go.sum
@@ -28,8 +28,6 @@ github.com/balajiv113/fd v0.0.0-20230330094840-143eec500f3e h1:IdMhFPEfTZQU971tI
 github.com/balajiv113/fd v0.0.0-20230330094840-143eec500f3e/go.mod h1:aXGMJsd3XrnUFTuyf/pTGg5jG6CY8JMZ5juywvShjgQ=
 github.com/bmatcuk/doublestar/v4 v4.7.1 h1:fdDeAqgT47acgwd9bd9HxJRDmc9UAmPpc+2m0CXv75Q=
 github.com/bmatcuk/doublestar/v4 v4.7.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
-github.com/braydonk/yaml v0.9.0 h1:ewGMrVmEVpsm3VwXQDR388sLg5+aQ8Yihp6/hc4m+h4=
-github.com/braydonk/yaml v0.9.0/go.mod h1:hcm3h581tudlirk8XEUPDBAimBPbmnL0Y45hCRl47N4=
 github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMUs=
 github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
 github.com/cheggaaa/pb/v3 v3.1.7 h1:2FsIW307kt7A/rz/ZI2lvPO+v3wKazzE4K/0LtTWsOI=
@@ -129,8 +127,8 @@ github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaU
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/google/yamlfmt v0.16.0 h1:5auoxqdx2CxOb022XGBElFFVH8uE/lAJDCWKRMq4mT8=
-github.com/google/yamlfmt v0.16.0/go.mod h1:/fF8jQmFopG3InQoWYG3gTORPXqLwNkcBqAT4UA4ab0=
+github.com/google/yamlfmt v0.17.0 h1:/tdp01rIlvLz3LgJ2NtMLnqgAadZm33P7GcPU680b+w=
+github.com/google/yamlfmt v0.17.0/go.mod h1:gs0UEklJOYkUJ+OOCG0hg9n+DzucKDPlJElTUasVNK8=
 github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM=
 github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg=
 github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec h1:qv2VnGeEQHchGaZ/u7lxST/RaJw+cv273q79D81Xbog=
@@ -241,8 +239,8 @@ github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 h1:OkMGxebDjyw0ULyrTYWeN0UNCCkmCWfjPnIA2W6oviI=
 github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06/go.mod h1:+ePHsJ1keEjQtpvf9HHw0f4ZeJ0TLRsxhunSI2hYJSs=
-github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 h1:PKK9DyHxif4LZo+uQSgXNqs0jj5+xZwwfKHgph2lxBw=
-github.com/santhosh-tekuri/jsonschema/v6 v6.0.1/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
+github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ=
+github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
 github.com/sethvargo/go-password v0.3.1 h1:WqrLTjo7X6AcVYfC6R7GtSyuUQR9hGyAj/f1PYQZCJU=
 github.com/sethvargo/go-password v0.3.1/go.mod h1:rXofC1zT54N7R8K/h1WDUdkf9BOx5OptoxrMBcrXzvs=
 github.com/sirupsen/logrus v1.9.4-0.20230606125235-dd1b4c2e81af h1:Sp5TG9f7K39yfB+If0vjp97vuT74F72r8hfRpP8jLU0=
diff --git a/pkg/limatmpl/locator.go b/pkg/limatmpl/locator.go
index 3f2f2674..218311af 100644
--- a/pkg/limatmpl/locator.go
+++ b/pkg/limatmpl/locator.go
@@ -15,7 +15,6 @@ import (
 	"regexp"
 	"runtime"
 	"strings"
-	"unicode"
 
 	"github.com/containerd/containerd/identifiers"
 	"github.com/lima-vm/lima/pkg/ioutilx"
@@ -37,10 +36,9 @@ func Read(ctx context.Context, name, locator string) (*Template, error) {
 		return tmpl, nil
 	}
 
-	isTemplateURL, templateURL := SeemsTemplateURL(locator)
+	isTemplateURL, templateName := SeemsTemplateURL(locator)
 	switch {
 	case isTemplateURL:
-		templateName := path.Join(templateURL.Host, templateURL.Path)
 		logrus.Debugf("interpreting argument %q as a template name %q", locator, templateName)
 		if tmpl.Name == "" {
 			// e.g., templateName = "deprecated/centos-7.yaml" , tmpl.Name = "centos-7"
@@ -95,7 +93,12 @@ func Read(ctx context.Context, name, locator string) (*Template, error) {
 		if err != nil {
 			return nil, err
 		}
-	case SeemsFilePath(locator):
+	case locator == "-":
+		tmpl.Bytes, err = io.ReadAll(os.Stdin)
+		if err != nil {
+			return nil, fmt.Errorf("unexpected error reading stdin: %w", err)
+		}
+	default:
 		if tmpl.Name == "" {
 			tmpl.Name, err = InstNameFromYAMLPath(locator)
 			if err != nil {
@@ -116,11 +119,6 @@ func Read(ctx context.Context, name, locator string) (*Template, error) {
 		if err != nil {
 			return nil, err
 		}
-	case locator == "-":
-		tmpl.Bytes, err = io.ReadAll(os.Stdin)
-		if err != nil {
-			return nil, fmt.Errorf("unexpected error reading stdin: %w", err)
-		}
 	}
 	// The only reason not to call tmpl.UseAbsLocators() here is that `limactl tmpl copy --verbatim …`
 	// should create an unmodified copy of the template.
@@ -233,14 +231,20 @@ func InstNameFromImageURL(locator, imageArch string) string {
 	return name
 }
 
-func SeemsTemplateURL(arg string) (bool, *url.URL) {
+// SeemsTemplateURL returns true if the arg is a URL using the template scheme.
+// When it returns true, it also returns the template name.
+func SeemsTemplateURL(arg string) (isTemplate bool, templateName string) {
 	u, err := url.Parse(arg)
 	if err != nil {
-		return false, u
+		return false, ""
 	}
-	return u.Scheme == "template", u
+	if u.Scheme == "template" {
+		return true, path.Join(u.Host, u.Path)
+	}
+	return false, ""
 }
 
+// SeemsHTTPURL returns true if the arg is a URL using the http or https scheme.
 func SeemsHTTPURL(arg string) bool {
 	u, err := url.Parse(arg)
 	if err != nil {
@@ -252,6 +256,7 @@ func SeemsHTTPURL(arg string) bool {
 	return true
 }
 
+// SeemsFileURL returns true if the arg is a URL using the file scheme.
 func SeemsFileURL(arg string) bool {
 	u, err := url.Parse(arg)
 	if err != nil {
@@ -260,28 +265,6 @@ func SeemsFileURL(arg string) bool {
 	return u.Scheme == "file"
 }
 
-func SeemsYAMLPath(arg string) bool {
-	if strings.Contains(arg, "/") {
-		return true
-	}
-	lower := strings.ToLower(arg)
-	return strings.HasSuffix(lower, ".yml") || strings.HasSuffix(lower, ".yaml")
-}
-
-// SeemsFilePath returns true if arg either contains a path separator or has a file extension that
-// does not start with a digit. `my.yaml` is a file path, `ubuntu-20.10` is not.
-func SeemsFilePath(arg string) bool {
-	// Single-letter schemes will be drive names on Windows, e.g. "c:/foo.yaml"
-	if u, err := url.Parse(arg); err == nil && len(u.Scheme) > 1 {
-		return false
-	}
-	if strings.ContainsRune(arg, '/') || strings.ContainsRune(arg, filepath.Separator) {
-		return true
-	}
-	ext := filepath.Ext(arg)
-	return len(ext) > 1 && !unicode.IsDigit(rune(ext[1]))
-}
-
 func InstNameFromURL(urlStr string) (string, error) {
 	u, err := url.Parse(urlStr)
 	if err != nil {
diff --git a/pkg/store/store.go b/pkg/store/store.go
index 60732d10..58669dd7 100644
--- a/pkg/store/store.go
+++ b/pkg/store/store.go
@@ -5,6 +5,7 @@ package store
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -45,6 +46,19 @@ func Validate() error {
 	return nil
 }
 
+// ValidateInstName checks if the name is a valid instance name. For this it needs to
+// be a valid identifier, and not end in .yml or .yaml (case insensitively).
+func ValidateInstName(name string) error {
+	if err := identifiers.Validate(name); err != nil {
+		return fmt.Errorf("instance name %q is not a valid identifier: %w", name, err)
+	}
+	lower := strings.ToLower(name)
+	if strings.HasSuffix(lower, ".yml") || strings.HasSuffix(lower, ".yaml") {
+		return fmt.Errorf("instance name %q must not end with .yml or .yaml suffix", name)
+	}
+	return nil
+}
+
 // Instances returns the names of the instances under LimaDir.
 func Instances() ([]string, error) {
 	limaDir, err := dirnames.LimaDir()
@@ -93,7 +107,7 @@ func Disks() ([]string, error) {
 // InstanceDir returns the instance dir.
 // InstanceDir does not check whether the instance exists.
 func InstanceDir(name string) (string, error) {
-	if err := identifiers.Validate(name); err != nil {
+	if err := ValidateInstName(name); err != nil {
 		return "", err
 	}
 	limaDir, err := dirnames.LimaDir()



Test patch:
diff --git a/pkg/limatmpl/locator_test.go b/pkg/limatmpl/locator_test.go
index dcf510c2..6c2c03d8 100644
--- a/pkg/limatmpl/locator_test.go
+++ b/pkg/limatmpl/locator_test.go
@@ -61,3 +61,64 @@ func TestInstNameFromImageURL(t *testing.T) {
 		assert.Equal(t, name, "rocky-8-9.10")
 	})
 }
+
+func TestSeemsTemplateURL(t *testing.T) {
+	arg := "template://foo/bar"
+	t.Run(arg, func(t *testing.T) {
+		is, name := limatmpl.SeemsTemplateURL(arg)
+		assert.Equal(t, is, true)
+		assert.Equal(t, name, "foo/bar")
+	})
+	notTemplateURLs := []string{
+		"file:///foo",
+		"http://foo",
+		"https://foo",
+		"foo",
+	}
+	for _, arg := range notTemplateURLs {
+		t.Run(arg, func(t *testing.T) {
+			is, _ := limatmpl.SeemsTemplateURL(arg)
+			assert.Equal(t, is, false)
+		})
+	}
+}
+
+func TestSeemsHTTPURL(t *testing.T) {
+	httpURLs := []string{
+		"http://foo/",
+		"https://foo/",
+	}
+	for _, arg := range httpURLs {
+		t.Run(arg, func(t *testing.T) {
+			assert.Equal(t, limatmpl.SeemsHTTPURL(arg), true)
+		})
+	}
+	notHTTPURLs := []string{
+		"file:///foo",
+		"template://foo",
+		"foo",
+	}
+	for _, arg := range notHTTPURLs {
+		t.Run(arg, func(t *testing.T) {
+			assert.Equal(t, limatmpl.SeemsHTTPURL(arg), false)
+		})
+	}
+}
+
+func TestSeemsFileURL(t *testing.T) {
+	arg := "file:///foo"
+	t.Run(arg, func(t *testing.T) {
+		assert.Equal(t, limatmpl.SeemsFileURL(arg), true)
+	})
+	notFileURLs := []string{
+		"http://foo",
+		"https://foo",
+		"template://foo",
+		"foo",
+	}
+	for _, arg := range notFileURLs {
+		t.Run(arg, func(t *testing.T) {
+			assert.Equal(t, limatmpl.SeemsFileURL(arg), false)
+		})
+	}
+}
diff --git a/pkg/store/store_test.go b/pkg/store/store_test.go
new file mode 100644
index 00000000..4dfe1eea
--- /dev/null
+++ b/pkg/store/store_test.go
@@ -0,0 +1,61 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package store
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestValidateInstName(t *testing.T) {
+	instNames := []string{
+		"default",
+		"Ubuntu-20.04",
+		"example.com",
+		"under_score",
+		"1-2_3.4",
+		"yml",
+		"yaml",
+		"foo.yaml.com",
+	}
+	for _, arg := range instNames {
+		t.Run(arg, func(t *testing.T) {
+			err := ValidateInstName(arg)
+			assert.NilError(t, err)
+		})
+	}
+	invalidIdentifiers := []string{
+		"",
+		"my/instance",
+		"my\\instance",
+		"c:default",
+		"dot.",
+		".dot",
+		"dot..dot",
+		"underscore_",
+		"_underscore",
+		"underscore__underscore",
+		"dash-",
+		"-dash",
+		"dash--dash",
+	}
+	for _, arg := range invalidIdentifiers {
+		t.Run(arg, func(t *testing.T) {
+			err := ValidateInstName(arg)
+			assert.ErrorContains(t, err, "not a valid identifier")
+		})
+	}
+	yamlNames := []string{
+		"default.yaml",
+		"MY.YAML",
+		"My.YmL",
+	}
+	for _, arg := range yamlNames {
+		t.Run(arg, func(t *testing.T) {
+			err := ValidateInstName(arg)
+			assert.ErrorContains(t, err, "must not end with .y")
+		})
+	}
+}