diff --git a/internal/plugin/loader_test.go b/internal/plugin/loader_test.go index e84905248..16b8581c3 100644 --- a/internal/plugin/loader_test.go +++ b/internal/plugin/loader_test.go @@ -338,6 +338,7 @@ runtime: subprocess "correct name field": { yaml: `apiVersion: v1 name: my-plugin +version: 1.0.0 type: cli/v1 runtime: subprocess `, diff --git a/internal/plugin/metadata.go b/internal/plugin/metadata.go index 4e019f0b3..337c73da6 100644 --- a/internal/plugin/metadata.go +++ b/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, fmt.Errorf("empty APIVersion")) } diff --git a/internal/plugin/metadata_legacy.go b/internal/plugin/metadata_legacy.go index 3cd1a50cd..212dccbd1 100644 --- a/internal/plugin/metadata_legacy.go +++ b/internal/plugin/metadata_legacy.go @@ -71,6 +71,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 { diff --git a/internal/plugin/metadata_legacy_test.go b/internal/plugin/metadata_legacy_test.go index 9421e98b5..24cc48a60 100644 --- a/internal/plugin/metadata_legacy_test.go +++ b/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 { diff --git a/internal/plugin/metadata_test.go b/internal/plugin/metadata_test.go index 145ef5101..e983d32ae 100644 --- a/internal/plugin/metadata_test.go +++ b/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{ diff --git a/internal/plugin/metadata_v1.go b/internal/plugin/metadata_v1.go index 81dbc2e20..1eb0ae610 100644 --- a/internal/plugin/metadata_v1.go +++ b/internal/plugin/metadata_v1.go @@ -51,6 +51,13 @@ func (m *MetadataV1) Validate() error { return fmt.Errorf("invalid plugin `name`") } + if m.Version == "" { + return fmt.Errorf("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) } diff --git a/internal/plugin/metadata_v1_test.go b/internal/plugin/metadata_v1_test.go new file mode 100644 index 000000000..ed9a2c671 --- /dev/null +++ b/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) + }) + } +} diff --git a/pkg/cmd/testdata/helm home with space/helm/plugins/fullenv/plugin.yaml b/pkg/cmd/testdata/helm home with space/helm/plugins/fullenv/plugin.yaml index a58544b03..c99e5122b 100644 --- a/pkg/cmd/testdata/helm home with space/helm/plugins/fullenv/plugin.yaml +++ b/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: diff --git a/pkg/cmd/testdata/helmhome/helm/plugins/args/plugin.yaml b/pkg/cmd/testdata/helmhome/helm/plugins/args/plugin.yaml index 4156e7f17..24d79ac7e 100644 --- a/pkg/cmd/testdata/helmhome/helm/plugins/args/plugin.yaml +++ b/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 diff --git a/pkg/cmd/testdata/helmhome/helm/plugins/echo/plugin.yaml b/pkg/cmd/testdata/helmhome/helm/plugins/echo/plugin.yaml index a0a0b5255..a707c3373 100644 --- a/pkg/cmd/testdata/helmhome/helm/plugins/echo/plugin.yaml +++ b/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 diff --git a/pkg/cmd/testdata/helmhome/helm/plugins/exitwith/plugin.yaml b/pkg/cmd/testdata/helmhome/helm/plugins/exitwith/plugin.yaml index ba9508255..93930219b 100644 --- a/pkg/cmd/testdata/helmhome/helm/plugins/exitwith/plugin.yaml +++ b/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: diff --git a/pkg/cmd/testdata/helmhome/helm/plugins/fullenv/plugin.yaml b/pkg/cmd/testdata/helmhome/helm/plugins/fullenv/plugin.yaml index a58544b03..c99e5122b 100644 --- a/pkg/cmd/testdata/helmhome/helm/plugins/fullenv/plugin.yaml +++ b/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: diff --git a/pkg/cmd/testdata/helmhome/helm/plugins/shortenv/plugin.yaml b/pkg/cmd/testdata/helmhome/helm/plugins/shortenv/plugin.yaml index 5fe053ed0..3f935db4b 100644 --- a/pkg/cmd/testdata/helmhome/helm/plugins/shortenv/plugin.yaml +++ b/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: diff --git a/pkg/cmd/testdata/testplugin/plugin.yaml b/pkg/cmd/testdata/testplugin/plugin.yaml index 3ee5d04f6..fb1d82062 100644 --- a/pkg/cmd/testdata/testplugin/plugin.yaml +++ b/pkg/cmd/testdata/testplugin/plugin.yaml @@ -1,6 +1,7 @@ --- apiVersion: v1 name: testplugin +version: 0.1.0 type: cli/v1 runtime: subprocess config: