fix: Plugin version path traversal

Signed-off-by: George Jenkins <gvjenkins@gmail.com>

internal/plugin/loader_test.go

@@ -337,6 +337,7 @@ runtime: subprocess
 		"correct name field": {
 			yaml: `apiVersion: v1
 name: my-plugin
+version: 1.0.0
 type: cli/v1
 runtime: subprocess
 `,

internal/plugin/metadata.go

@@ -19,9 +19,17 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/Masterminds/semver/v3"
+
 	"helm.sh/helm/v4/internal/plugin/schema"
 )
 
+// isValidSemver checks if the given string is a valid semantic version.
+func isValidSemver(v string) bool {
+	_, err := semver.NewVersion(v)
+	return err == nil
+}
+
 // Metadata of a plugin, converted from the "on-disk" legacy or v1 plugin.yaml
 // Specifically, Config and RuntimeConfig are converted to their respective types based on the plugin type and runtime
 type Metadata struct {
@@ -57,6 +65,10 @@ func (m Metadata) Validate() error {
 		errs = append(errs, fmt.Errorf("invalid plugin name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name))
 	}
 
+	if m.Version != "" && !isValidSemver(m.Version) {
+		errs = append(errs, fmt.Errorf("invalid plugin version %q: must be valid semver", m.Version))
+	}
+
 	if m.APIVersion == "" {
 		errs = append(errs, errors.New("empty APIVersion"))
 	}

internal/plugin/metadata_legacy.go

@@ -72,6 +72,11 @@ func (m *MetadataLegacy) Validate() error {
 	if !validPluginName.MatchString(m.Name) {
 		return fmt.Errorf("invalid plugin name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name)
 	}
+
+	if m.Version != "" && !isValidSemver(m.Version) {
+		return fmt.Errorf("invalid plugin version %q: must be valid semver", m.Version)
+	}
+
 	m.Usage = sanitizeString(m.Usage)
 
 	if len(m.PlatformCommand) > 0 && len(m.Command) > 0 {

internal/plugin/metadata_legacy_test.go

@@ -59,6 +59,13 @@ func TestMetadataLegacyValidate(t *testing.T) {
 				},
 			},
 		},
+		"valid with version": {
+			Name:    "myplugin",
+			Version: "1.0.0",
+		},
+		"valid with empty version": {
+			Name: "myplugin",
+		},
 	}
 
 	for testName, metadata := range testsValid {
@@ -116,6 +123,14 @@ func TestMetadataLegacyValidate(t *testing.T) {
 				},
 			},
 		},
+		"path traversal version": {
+			Name:    "myplugin",
+			Version: "../../../../tmp/evil",
+		},
+		"invalid version": {
+			Name:    "myplugin",
+			Version: "not-a-version",
+		},
 	}
 
 	for testName, metadata := range testsInvalid {

internal/plugin/metadata_test.go

@@ -72,6 +72,40 @@ func TestValidatePluginData(t *testing.T) {
 	}
 }
 
+func TestMetadataValidateVersionPathTraversal(t *testing.T) {
+	tests := map[string]struct {
+		version string
+		valid   bool
+	}{
+		"valid semver":                {version: "1.0.0", valid: true},
+		"valid semver with v prefix":  {version: "v1.0.0", valid: true},
+		"valid semver with prerelease": {version: "1.2.3-alpha.1+build.123", valid: true},
+		"empty version":               {version: "", valid: true},
+		"path traversal":              {version: "../../../../tmp/evil", valid: false},
+		"path traversal etc":          {version: "../../../etc/passwd", valid: false},
+		"path traversal in version":   {version: "1.0.0/../../etc", valid: false},
+		"not a version":               {version: "not-a-version", valid: false},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := mockSubprocessCLIPlugin(t, "testplugin")
+			m.metadata.Version = tc.version
+			err := m.Metadata().Validate()
+			if tc.valid && err != nil {
+				t.Errorf("expected valid, got error: %s", err)
+			}
+			if !tc.valid {
+				if err == nil {
+					t.Errorf("expected error for version %q, got nil", tc.version)
+				} else if !strings.Contains(err.Error(), "invalid plugin version") {
+					t.Errorf("expected 'invalid plugin version' error, got: %s", err)
+				}
+			}
+		})
+	}
+}
+
 func TestMetadataValidateMultipleErrors(t *testing.T) {
 	// Create metadata with multiple validation issues
 	metadata := Metadata{

internal/plugin/metadata_v1.go

@@ -52,6 +52,13 @@ func (m *MetadataV1) Validate() error {
 		return errors.New("invalid plugin `name`")
 	}
 
+	if m.Version == "" {
+		return errors.New("plugin `version` is required")
+	}
+	if !isValidSemver(m.Version) {
+		return fmt.Errorf("invalid plugin `version` %q: must be valid semver", m.Version)
+	}
+
 	if m.APIVersion != "v1" {
 		return fmt.Errorf("invalid `apiVersion`: %q", m.APIVersion)
 	}

internal/plugin/metadata_v1_test.go

@@ -0,0 +1,82 @@
+/*
+Copyright The Helm Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package plugin
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMetadataV1ValidateVersion(t *testing.T) {
+	base := func() MetadataV1 {
+		return MetadataV1{
+			APIVersion: "v1",
+			Name:       "myplugin",
+			Type:       "cli/v1",
+			Runtime:    "subprocess",
+			Version:    "1.0.0",
+		}
+	}
+
+	testsValid := map[string]string{
+		"simple version":    "1.0.0",
+		"v prefix":          "v1.0.0",
+		"with prerelease":   "1.2.3-alpha.1",
+		"with build meta":   "1.2.3+build.123",
+		"full prerelease":   "1.2.3-alpha.1+build.123",
+	}
+
+	for name, version := range testsValid {
+		t.Run("valid/"+name, func(t *testing.T) {
+			m := base()
+			m.Version = version
+			assert.NoError(t, m.Validate())
+		})
+	}
+
+	testsInvalid := map[string]struct {
+		version string
+		errMsg  string
+	}{
+		"empty version": {
+			version: "",
+			errMsg:  "plugin `version` is required",
+		},
+		"path traversal": {
+			version: "../../../../tmp/evil",
+			errMsg:  "invalid plugin `version`",
+		},
+		"path traversal etc": {
+			version: "../../../etc/passwd",
+			errMsg:  "invalid plugin `version`",
+		},
+		"not a version": {
+			version: "not-a-version",
+			errMsg:  "invalid plugin `version`",
+		},
+	}
+
+	for name, tc := range testsInvalid {
+		t.Run("invalid/"+name, func(t *testing.T) {
+			m := base()
+			m.Version = tc.version
+			err := m.Validate()
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), tc.errMsg)
+		})
+	}
+}

pkg/cmd/testdata/helm home with space/helm/plugins/fullenv/plugin.yaml

@@ -1,6 +1,7 @@
 ---
 apiVersion: v1
 name: fullenv
+version: 0.1.0
 type: cli/v1
 runtime: subprocess
 config:

pkg/cmd/testdata/helmhome/helm/plugins/args/plugin.yaml

@@ -1,4 +1,5 @@
 name: args
+version: 0.1.0
 type: cli/v1
 apiVersion: v1
 runtime: subprocess

pkg/cmd/testdata/helmhome/helm/plugins/echo/plugin.yaml

@@ -1,4 +1,5 @@
 name: echo
+version: 0.1.0
 type: cli/v1
 apiVersion: v1
 runtime: subprocess

pkg/cmd/testdata/helmhome/helm/plugins/exitwith/plugin.yaml

@@ -1,6 +1,7 @@
 ---
 apiVersion: v1
 name: exitwith
+version: 0.1.0
 type: cli/v1
 runtime: subprocess
 config:

pkg/cmd/testdata/helmhome/helm/plugins/fullenv/plugin.yaml

@@ -1,6 +1,7 @@
 ---
 apiVersion: v1
 name: fullenv
+version: 0.1.0
 type: cli/v1
 runtime: subprocess
 config:

pkg/cmd/testdata/helmhome/helm/plugins/shortenv/plugin.yaml

@@ -1,6 +1,7 @@
 ---
 apiVersion: v1
 name: shortenv
+version: 0.1.0
 type: cli/v1
 runtime: subprocess
 config:

pkg/cmd/testdata/testplugin/plugin.yaml

@@ -1,6 +1,7 @@
 ---
 apiVersion: v1
 name: testplugin
+version: 0.1.0
 type: cli/v1
 runtime: subprocess
 config: