fix: improve plugin name validation err messages early via unmarshalling

- Add strict YAML unmarshalling for v1 plugins (d.KnownFields)
- Add comprehensive test coverage for validPluginName regex
- Maintain backwards compatibility for legacy plugins

Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>

internal/plugin/loader.go

@@ -47,6 +47,7 @@ func loadMetadataLegacy(metadataData []byte) (*Metadata, error) {
 
 	var ml MetadataLegacy
 	d := yaml.NewDecoder(bytes.NewReader(metadataData))
+	// NOTE: No strict unmarshalling for legacy plugins - maintain backwards compatibility
 	if err := d.Decode(&ml); err != nil {
 		return nil, err
 	}
@@ -66,6 +67,7 @@ func loadMetadataV1(metadataData []byte) (*Metadata, error) {
 
 	var mv1 MetadataV1
 	d := yaml.NewDecoder(bytes.NewReader(metadataData))
+	d.KnownFields(true)
 	if err := d.Decode(&mv1); err != nil {
 		return nil, err
 	}

internal/plugin/loader_test.go

@@ -268,3 +268,61 @@ func TestFindPlugins(t *testing.T) {
 		})
 	}
 }
+
+func TestLoadMetadataLegacy_CapitalNameField(t *testing.T) {
+	yamlWithCapitalName := `Name: my-plugin
+version: 1.0.0
+usage: test plugin
+description: test description
+command: echo test`
+
+	_, err := loadMetadataLegacy([]byte(yamlWithCapitalName))
+
+	// Legacy plugins: No strict unmarshalling (backwards compatibility)
+	// YAML decoder silently ignores "Name:", then validation catches empty name
+	require.Error(t, err, "expected error for capital 'Name:' field")
+	assert.Contains(t, err.Error(), "invalid plugin name \"\": must contain only a-z, A-Z, 0-9, _ and -")
+
+	t.Logf("Legacy error (validation catches empty name): %v", err)
+	t.Log("NOTE: V1 plugins use strict unmarshalling and would get: yaml: field Name not found")
+}
+
+func TestLoadMetadataLegacy_CorrectField(t *testing.T) {
+	yamlWithCorrectName := `name: my-plugin
+version: 1.0.0
+usage: test plugin
+description: test description
+command: echo test`
+
+	m, err := loadMetadataLegacy([]byte(yamlWithCorrectName))
+
+	require.NoError(t, err, "expected success for correct 'name:' field")
+	assert.Equal(t, "my-plugin", m.Name)
+}
+
+func TestLoadMetadataV1_CapitalNameField(t *testing.T) {
+	yamlWithCapitalName := `apiVersion: v1
+Name: my-plugin
+type: cli/v1
+runtime: subprocess
+`
+
+	_, err := loadMetadataV1([]byte(yamlWithCapitalName))
+
+	require.Error(t, err, "expected error for capital 'Name:' field")
+	assert.Contains(t, err.Error(), "field Name not found in type plugin.MetadataV1")
+
+	t.Logf("V1 error (strict unmarshalling): %v", err)
+}
+
+func TestLoadMetadataV1_CorrectField(t *testing.T) {
+	yamlWithCorrectName := `apiVersion: v1
+name: my-plugin
+type: cli/v1
+runtime: subprocess
+`
+	m, err := loadMetadataV1([]byte(yamlWithCorrectName))
+
+	require.NoError(t, err, "expected success for correct 'name:' field")
+	assert.Equal(t, "my-plugin", m.Name)
+}

internal/plugin/metadata.go

@@ -54,7 +54,7 @@ func (m Metadata) Validate() error {
 	var errs []error
 
 	if !validPluginName.MatchString(m.Name) {
-		errs = append(errs, fmt.Errorf("invalid name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name))
+		errs = append(errs, fmt.Errorf("invalid plugin name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name))
 	}
 
 	if m.APIVersion == "" {

internal/plugin/metadata_legacy.go

@@ -69,7 +69,7 @@ type MetadataLegacy struct {
 
 func (m *MetadataLegacy) Validate() error {
 	if !validPluginName.MatchString(m.Name) {
-		return fmt.Errorf("invalid name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name)
+		return fmt.Errorf("invalid plugin name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name)
 	}
 	m.Usage = sanitizeString(m.Usage)
 

internal/plugin/metadata_test.go

@@ -53,10 +53,10 @@ func TestValidatePluginData(t *testing.T) {
 	}{
 		{true, mockSubprocessCLIPlugin(t, "abcdefghijklmnopqrstuvwxyz0123456789_-ABC"), ""},
 		{true, mockSubprocessCLIPlugin(t, "foo-bar-FOO-BAR_1234"), ""},
-		{false, mockSubprocessCLIPlugin(t, "foo -bar"), "invalid name"},
+		{false, mockSubprocessCLIPlugin(t, "foo -bar"), "invalid plugin name"},
-		{false, mockSubprocessCLIPlugin(t, "$foo -bar"), "invalid name"}, // Test leading chars
+		{false, mockSubprocessCLIPlugin(t, "$foo -bar"), "invalid plugin name"}, // Test leading chars
-		{false, mockSubprocessCLIPlugin(t, "foo -bar "), "invalid name"}, // Test trailing chars
+		{false, mockSubprocessCLIPlugin(t, "foo -bar "), "invalid plugin name"}, // Test trailing chars
-		{false, mockSubprocessCLIPlugin(t, "foo\nbar"), "invalid name"},  // Test newline
+		{false, mockSubprocessCLIPlugin(t, "foo\nbar"), "invalid plugin name"},  // Test newline
 		{true, mockNoCommand, ""},     // Test no command metadata works
 		{true, mockLegacyCommand, ""}, // Test legacy command metadata works
 	} {
@@ -92,7 +92,7 @@ func TestMetadataValidateMultipleErrors(t *testing.T) {
 
 	// Check that all expected errors are present in the joined error
 	expectedErrors := []string{
-		"invalid name",
+		"invalid plugin name",
 		"empty APIVersion",
 		"empty type field",
 		"empty runtime field",