diff --git a/plugin/manifest.go b/plugin/manifest.go index 3b64a679..aa2fb28b 100644 --- a/plugin/manifest.go +++ b/plugin/manifest.go @@ -93,7 +93,8 @@ type Dependency struct { // // When the legacy object format is detected, its type lists are merged into the // top-level ModuleTypes, StepTypes, and TriggerTypes fields so callers always -// find types in a consistent location. +// find types in a consistent location. Any other JSON type (string, number, +// bool) is rejected with a descriptive error. func (m *PluginManifest) UnmarshalJSON(data []byte) error { // rawManifest breaks the recursion: it is the same layout as PluginManifest // but without the custom UnmarshalJSON method. @@ -114,32 +115,70 @@ func (m *PluginManifest) UnmarshalJSON(data []byte) error { if len(raw.Capabilities) == 0 { return nil } - switch raw.Capabilities[0] { + + // Peek at the first non-whitespace byte to decide which branch to take. + // This avoids silently ignoring genuinely invalid capability values. + switch firstNonSpace(raw.Capabilities) { + case 0, 'n': + // Empty or JSON null – treat as absent. + case '[': - // New format: array of CapabilityDecl + // New format: array of CapabilityDecl. var caps []CapabilityDecl if err := json.Unmarshal(raw.Capabilities, &caps); err != nil { return fmt.Errorf("invalid capabilities array: %w", err) } m.Capabilities = caps + case '{': // Legacy format: object with configProvider, moduleTypes, stepTypes, triggerTypes. // Merge type lists into the top-level fields so callers see them consistently. var legacyCaps struct { - ModuleTypes []string `json:"moduleTypes"` - StepTypes []string `json:"stepTypes"` - TriggerTypes []string `json:"triggerTypes"` + ModuleTypes []string `json:"moduleTypes"` + StepTypes []string `json:"stepTypes"` + TriggerTypes []string `json:"triggerTypes"` + WorkflowTypes []string `json:"workflowTypes"` } if err := json.Unmarshal(raw.Capabilities, &legacyCaps); err != nil { return fmt.Errorf("invalid capabilities object: %w", err) } - m.ModuleTypes = append(m.ModuleTypes, legacyCaps.ModuleTypes...) - m.StepTypes = append(m.StepTypes, legacyCaps.StepTypes...) - m.TriggerTypes = append(m.TriggerTypes, legacyCaps.TriggerTypes...) + m.ModuleTypes = appendUnique(m.ModuleTypes, legacyCaps.ModuleTypes...) + m.StepTypes = appendUnique(m.StepTypes, legacyCaps.StepTypes...) + m.TriggerTypes = appendUnique(m.TriggerTypes, legacyCaps.TriggerTypes...) + m.WorkflowTypes = appendUnique(m.WorkflowTypes, legacyCaps.WorkflowTypes...) + + default: + return fmt.Errorf("capabilities: unsupported JSON type (expected array or object, got %q)", string(raw.Capabilities)) } + return nil } +// firstNonSpace returns the first non-whitespace byte in b, or 0 if b is empty/all-whitespace. +func firstNonSpace(b []byte) byte { + for _, c := range b { + if c != ' ' && c != '\t' && c != '\r' && c != '\n' { + return c + } + } + return 0 +} + +// appendUnique appends values to dst, skipping any that are already present. +func appendUnique(dst []string, values ...string) []string { + existing := make(map[string]struct{}, len(dst)) + for _, v := range dst { + existing[v] = struct{}{} + } + for _, v := range values { + if _, ok := existing[v]; !ok { + dst = append(dst, v) + existing[v] = struct{}{} + } + } + return dst +} + // Validate checks that a manifest has all required fields and valid semver. func (m *PluginManifest) Validate() error { if m.Name == "" { diff --git a/plugin/manifest_test.go b/plugin/manifest_test.go index 0ad8507c..b6755e50 100644 --- a/plugin/manifest_test.go +++ b/plugin/manifest_test.go @@ -426,86 +426,136 @@ func TestManifestEngineFieldsLoadFromFile(t *testing.T) { } } -func TestPluginManifest_LegacyCapabilities(t *testing.T) { - // Legacy format: capabilities is a JSON object with configProvider, moduleTypes, etc. - legacyJSON := `{ - "name": "legacy-plugin", +// TestManifestLegacyCapabilitiesObject verifies that a plugin.json whose +// "capabilities" field is a plain JSON object (the format used by external +// plugins such as workflow-plugin-authz) is parsed without error and that the +// type lists nested inside the object are promoted to the manifest's top-level +// ModuleTypes/StepTypes/TriggerTypes fields. +func TestManifestLegacyCapabilitiesObject(t *testing.T) { + const legacyJSON = `{ + "name": "workflow-plugin-authz", "version": "1.0.0", - "author": "Test", - "description": "Legacy capabilities test", + "description": "RBAC authorization plugin using Casbin", + "author": "GoCodeAlone", + "license": "MIT", + "type": "external", + "tier": "core", + "minEngineVersion": "0.3.11", + "keywords": ["authz", "rbac", "casbin", "authorization", "policy"], + "homepage": "https://github.com/GoCodeAlone/workflow-plugin-authz", + "repository": "https://github.com/GoCodeAlone/workflow-plugin-authz", "capabilities": { - "configProvider": true, - "moduleTypes": ["test.module"], - "stepTypes": ["step.test"], - "triggerTypes": ["trigger.test"] + "configProvider": false, + "moduleTypes": ["authz.casbin"], + "stepTypes": [ + "step.authz_check_casbin", + "step.authz_add_policy", + "step.authz_remove_policy", + "step.authz_role_assign" + ], + "triggerTypes": [] } }` var m PluginManifest if err := json.Unmarshal([]byte(legacyJSON), &m); err != nil { - t.Fatalf("Unmarshal legacy capabilities: %v", err) + t.Fatalf("unexpected unmarshal error for legacy capabilities object: %v", err) } - if len(m.ModuleTypes) != 1 || m.ModuleTypes[0] != "test.module" { - t.Errorf("ModuleTypes = %v, want [test.module]", m.ModuleTypes) - } - if len(m.StepTypes) != 1 || m.StepTypes[0] != "step.test" { - t.Errorf("StepTypes = %v, want [step.test]", m.StepTypes) - } - if len(m.TriggerTypes) != 1 || m.TriggerTypes[0] != "trigger.test" { - t.Errorf("TriggerTypes = %v, want [trigger.test]", m.TriggerTypes) - } - // Legacy object format should not populate Capabilities slice + + // Capabilities array should be nil / empty – the object format has no CapabilityDecl items. if len(m.Capabilities) != 0 { - t.Errorf("Capabilities = %v, want empty for legacy object format", m.Capabilities) + t.Errorf("Capabilities = %v, want empty", m.Capabilities) } -} -func TestPluginManifest_NewCapabilitiesArrayFormat(t *testing.T) { - // New format: capabilities is a JSON array of CapabilityDecl - newJSON := `{ - "name": "new-plugin", - "version": "1.0.0", - "author": "Test", - "description": "New capabilities test", - "moduleTypes": ["test.module"], - "stepTypes": ["step.test"], - "capabilities": [{"name": "step.test", "role": "provider"}] - }` + // moduleTypes from the nested object should be promoted to the top level. + if len(m.ModuleTypes) != 1 || m.ModuleTypes[0] != "authz.casbin" { + t.Errorf("ModuleTypes = %v, want [authz.casbin]", m.ModuleTypes) + } - var m PluginManifest - if err := json.Unmarshal([]byte(newJSON), &m); err != nil { - t.Fatalf("Unmarshal new capabilities: %v", err) + // stepTypes should be promoted. + wantSteps := []string{ + "step.authz_check_casbin", + "step.authz_add_policy", + "step.authz_remove_policy", + "step.authz_role_assign", } - if len(m.Capabilities) != 1 || m.Capabilities[0].Name != "step.test" { - t.Errorf("Capabilities = %v, want [{step.test provider 0}]", m.Capabilities) + if len(m.StepTypes) != len(wantSteps) { + t.Errorf("StepTypes len = %d, want %d; got %v", len(m.StepTypes), len(wantSteps), m.StepTypes) + } else { + for i, want := range wantSteps { + if m.StepTypes[i] != want { + t.Errorf("StepTypes[%d] = %q, want %q", i, m.StepTypes[i], want) + } + } } - if len(m.ModuleTypes) != 1 || m.ModuleTypes[0] != "test.module" { - t.Errorf("ModuleTypes = %v, want [test.module]", m.ModuleTypes) + + // triggerTypes is an empty array – TriggerTypes should remain nil/empty. + if len(m.TriggerTypes) != 0 { + t.Errorf("TriggerTypes = %v, want empty", m.TriggerTypes) } } -func TestPluginManifest_LegacyCapabilitiesMergesWithTopLevel(t *testing.T) { - // Top-level fields should be merged with types from legacy capabilities object - legacyJSON := `{ - "name": "merged-plugin", +// TestManifestLegacyCapabilitiesObjectFile verifies that LoadManifest succeeds +// for a plugin.json that uses the legacy object-style capabilities field. +func TestManifestLegacyCapabilitiesObjectFile(t *testing.T) { + const legacyJSON = `{ + "name": "workflow-plugin-authz", "version": "1.0.0", - "author": "Test", - "description": "Merge test", - "moduleTypes": ["existing.module"], + "description": "RBAC authorization plugin", + "author": "GoCodeAlone", "capabilities": { - "moduleTypes": ["caps.module"], - "stepTypes": ["step.caps"] + "moduleTypes": ["authz.casbin"], + "stepTypes": ["step.authz_check"], + "triggerTypes": [] } }` - var m PluginManifest - if err := json.Unmarshal([]byte(legacyJSON), &m); err != nil { - t.Fatalf("Unmarshal: %v", err) + dir := t.TempDir() + path := filepath.Join(dir, "plugin.json") + if err := os.WriteFile(path, []byte(legacyJSON), 0644); err != nil { + t.Fatal(err) + } + + m, err := LoadManifest(path) + if err != nil { + t.Fatalf("LoadManifest error: %v", err) + } + if len(m.ModuleTypes) != 1 || m.ModuleTypes[0] != "authz.casbin" { + t.Errorf("ModuleTypes = %v, want [authz.casbin]", m.ModuleTypes) } - if len(m.ModuleTypes) != 2 { - t.Errorf("ModuleTypes = %v, want [existing.module caps.module]", m.ModuleTypes) + if len(m.StepTypes) != 1 || m.StepTypes[0] != "step.authz_check" { + t.Errorf("StepTypes = %v, want [step.authz_check]", m.StepTypes) } - if len(m.StepTypes) != 1 || m.StepTypes[0] != "step.caps" { - t.Errorf("StepTypes = %v, want [step.caps]", m.StepTypes) +} + +// TestManifestCapabilitiesInvalidFormat verifies that a plugin.json whose +// "capabilities" field is neither an array nor an object (e.g. a bare string) +// is rejected with a descriptive error. +func TestManifestCapabilitiesInvalidFormat(t *testing.T) { + cases := []struct { + name string + json string + }{ + { + name: "string value", + json: `{"name":"p","version":"1.0.0","author":"A","description":"D","capabilities":"oops"}`, + }, + { + name: "numeric value", + json: `{"name":"p","version":"1.0.0","author":"A","description":"D","capabilities":42}`, + }, + { + name: "boolean value", + json: `{"name":"p","version":"1.0.0","author":"A","description":"D","capabilities":true}`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var m PluginManifest + err := json.Unmarshal([]byte(tc.json), &m) + if err == nil { + t.Errorf("expected error for capabilities %s, got nil", tc.name) + } + }) } }