diff --git a/internal/plugin/loader.go b/internal/plugin/loader.go index a58a84126..2f051b99e 100644 --- a/internal/plugin/loader.go +++ b/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 } diff --git a/internal/plugin/loader_test.go b/internal/plugin/loader_test.go index 47c214910..31fd8df78 100644 --- a/internal/plugin/loader_test.go +++ b/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) +} diff --git a/internal/plugin/metadata.go b/internal/plugin/metadata.go index 15a954037..b051c1223 100644 --- a/internal/plugin/metadata.go +++ b/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 == "" { diff --git a/internal/plugin/metadata_legacy.go b/internal/plugin/metadata_legacy.go index 1b80e6196..3cd1a50cd 100644 --- a/internal/plugin/metadata_legacy.go +++ b/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) diff --git a/internal/plugin/metadata_test.go b/internal/plugin/metadata_test.go index 0c699b00f..145ef5101 100644 --- a/internal/plugin/metadata_test.go +++ b/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 name"}, // Test leading chars - {false, mockSubprocessCLIPlugin(t, "foo -bar "), "invalid name"}, // Test trailing chars - {false, mockSubprocessCLIPlugin(t, "foo\nbar"), "invalid name"}, // Test newline + {false, mockSubprocessCLIPlugin(t, "foo -bar"), "invalid plugin name"}, + {false, mockSubprocessCLIPlugin(t, "$foo -bar"), "invalid plugin name"}, // Test leading chars + {false, mockSubprocessCLIPlugin(t, "foo -bar "), "invalid plugin name"}, // Test trailing chars + {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",