From 70c200e4d3cfebfdf5d9bbbcf60a2182a63486cb Mon Sep 17 00:00:00 2001 From: lxowalle Date: Fri, 10 Apr 2026 16:29:07 +0800 Subject: [PATCH] fix skills registry config merge compatibility --- pkg/config/config_struct.go | 176 ++++++++++++++++++------ pkg/config/config_struct_test.go | 41 ++++++ pkg/config/security_integration_test.go | 46 +++++++ 3 files changed, 220 insertions(+), 43 deletions(-) diff --git a/pkg/config/config_struct.go b/pkg/config/config_struct.go index 3ce1ff3f5..c7b6f2f1a 100644 --- a/pkg/config/config_struct.go +++ b/pkg/config/config_struct.go @@ -334,20 +334,41 @@ func (v *SkillsRegistriesConfig) UnmarshalJSON(data []byte) error { return nil } - legacy := map[string]*SkillRegistryConfig{} + legacy := map[string]json.RawMessage{} if err := json.Unmarshal(data, &legacy); err != nil { return err } - list = make([]*SkillRegistryConfig, 0, len(legacy)) - for name, registry := range legacy { + if len(*v) == 0 { + keys := make([]string, 0, len(legacy)) + for name := range legacy { + keys = append(keys, name) + } + sort.Strings(keys) + list = make([]*SkillRegistryConfig, 0, len(keys)) + for _, name := range keys { + var registry SkillRegistryConfig + if err := json.Unmarshal(legacy[name], ®istry); err != nil { + return err + } + registry.Name = name + list = append(list, ®istry) + } + *v = list + return nil + } + + for _, name := range sortedRegistryNamesFromJSON(legacy) { + registry := cloneRegistryConfig(findRegistryConfigByName(*v, name)) if registry == nil { - continue + registry = &SkillRegistryConfig{Name: name} + } + if err := json.Unmarshal(legacy[name], registry); err != nil { + return err } registry.Name = name - list = append(list, registry) + v.Set(name, *registry) } - *v = list return nil } @@ -366,24 +387,42 @@ func (v SkillsRegistriesConfig) MarshalJSON() ([]byte, error) { } func (c *SkillRegistryConfig) UnmarshalJSON(data []byte) error { - type alias struct { - Name string `json:"name,omitempty"` - Enabled bool `json:"enabled"` - BaseURL string `json:"base_url"` - AuthToken SecureString `json:"auth_token,omitzero"` - Param map[string]any `json:"param,omitempty"` - } var raw map[string]json.RawMessage if err := json.Unmarshal(data, &raw); err != nil { return err } - var parsed alias - if err := json.Unmarshal(data, &parsed); err != nil { - return err + params := cloneRegistryParams(c.Param) + if params == nil { + params = map[string]any{} } - params := map[string]any{} - for key, value := range parsed.Param { - params[key] = value + if value, ok := raw["name"]; ok { + if err := json.Unmarshal(value, &c.Name); err != nil { + return err + } + } + if value, ok := raw["enabled"]; ok { + if err := json.Unmarshal(value, &c.Enabled); err != nil { + return err + } + } + if value, ok := raw["base_url"]; ok { + if err := json.Unmarshal(value, &c.BaseURL); err != nil { + return err + } + } + if value, ok := raw["auth_token"]; ok { + if err := json.Unmarshal(value, &c.AuthToken); err != nil { + return err + } + } + if value, ok := raw["param"]; ok { + var nested map[string]any + if err := json.Unmarshal(value, &nested); err != nil { + return err + } + for key, nestedValue := range nested { + params[key] = nestedValue + } } for key, value := range raw { switch key { @@ -397,10 +436,6 @@ func (c *SkillRegistryConfig) UnmarshalJSON(data []byte) error { params[key] = decoded } } - c.Name = parsed.Name - c.Enabled = parsed.Enabled - c.BaseURL = parsed.BaseURL - c.AuthToken = parsed.AuthToken c.Param = params return nil } @@ -433,7 +468,10 @@ func (c *SkillRegistryConfig) UnmarshalYAML(value *yaml.Node) error { if err := value.Decode(&raw); err != nil { return err } - params := map[string]any{} + params := cloneRegistryParams(c.Param) + if params == nil { + params = map[string]any{} + } if nested, ok := raw["param"].(map[string]any); ok { for k, v := range nested { params[k] = v @@ -523,33 +561,85 @@ func (v *SkillsRegistriesConfig) UnmarshalYAML(value *yaml.Node) error { *v = list return nil } - for _, registry := range *v { - if registry == nil { + for _, name := range sortedRegistryNames(mm) { + sec := mm[name] + if sec == nil { continue } - sec := mm[registry.Name] - if sec != nil { - registry.AuthToken = sec.AuthToken - if registry.BaseURL == "" { - registry.BaseURL = sec.BaseURL - } - if !registry.Enabled { - registry.Enabled = sec.Enabled - } - if registry.Param == nil { - registry.Param = map[string]any{} - } - for key, value := range sec.Param { - if _, ok := registry.Param[key]; ok { - continue - } - registry.Param[key] = value + sec.Name = name + registry := findRegistryConfigByName(*v, name) + if registry == nil { + *v = append(*v, cloneRegistryConfig(sec)) + continue + } + registry.AuthToken = sec.AuthToken + if registry.BaseURL == "" { + registry.BaseURL = sec.BaseURL + } + if !registry.Enabled { + registry.Enabled = sec.Enabled + } + if registry.Param == nil { + registry.Param = map[string]any{} + } + for key, value := range sec.Param { + if _, ok := registry.Param[key]; ok { + continue } + registry.Param[key] = value } } return nil } +func cloneRegistryParams(src map[string]any) map[string]any { + if src == nil { + return nil + } + cloned := make(map[string]any, len(src)) + for key, value := range src { + cloned[key] = value + } + return cloned +} + +func cloneRegistryConfig(src *SkillRegistryConfig) *SkillRegistryConfig { + if src == nil { + return nil + } + cloned := *src + cloned.Param = cloneRegistryParams(src.Param) + return &cloned +} + +func findRegistryConfigByName(registries SkillsRegistriesConfig, name string) *SkillRegistryConfig { + for _, registry := range registries { + if registry == nil || registry.Name != name { + continue + } + return registry + } + return nil +} + +func sortedRegistryNames(mm map[string]*SkillRegistryConfig) []string { + keys := make([]string, 0, len(mm)) + for name := range mm { + keys = append(keys, name) + } + sort.Strings(keys) + return keys +} + +func sortedRegistryNamesFromJSON(mm map[string]json.RawMessage) []string { + keys := make([]string, 0, len(mm)) + for name := range mm { + keys = append(keys, name) + } + sort.Strings(keys) + return keys +} + func (v SkillsRegistriesConfig) MarshalYAML() (any, error) { type onlySecureRegistryData struct { AuthToken SecureString `yaml:"auth_token,omitempty"` diff --git a/pkg/config/config_struct_test.go b/pkg/config/config_struct_test.go index 9b27d117b..97d6eefab 100644 --- a/pkg/config/config_struct_test.go +++ b/pkg/config/config_struct_test.go @@ -263,6 +263,47 @@ func TestSkillsRegistriesConfigMarshalJSONPreservesObjectShape(t *testing.T) { assert.Equal(t, "https://clawhub.ai", clawhub.BaseURL) } +func TestSkillsRegistriesConfigUnmarshalJSONPreservesDefaultRegistries(t *testing.T) { + registries := DefaultConfig().Tools.Skills.Registries + + err := json.Unmarshal([]byte(`{ + "clawhub": { + "base_url": "https://clawhub.example.com" + } + }`), ®istries) + assert.NoError(t, err) + + clawhub, ok := registries.Get("clawhub") + assert.True(t, ok) + assert.True(t, clawhub.Enabled) + assert.Equal(t, "https://clawhub.example.com", clawhub.BaseURL) + + github, ok := registries.Get("github") + assert.True(t, ok) + assert.True(t, github.Enabled) + assert.Equal(t, "https://github.com", github.BaseURL) + assert.Empty(t, github.Param) +} + +func TestSkillsRegistriesConfigUnmarshalYAMLAppendsNewRegistryToExistingSlice(t *testing.T) { + registries := DefaultConfig().Tools.Skills.Registries + + err := yaml.Unmarshal([]byte(`custom: + base_url: https://skills.example.com + auth_token: custom-token +`), ®istries) + assert.NoError(t, err) + + custom, ok := registries.Get("custom") + assert.True(t, ok) + assert.Equal(t, "https://skills.example.com", custom.BaseURL) + assert.Equal(t, "custom-token", custom.AuthToken.String()) + + github, ok := registries.Get("github") + assert.True(t, ok) + assert.Equal(t, "https://github.com", github.BaseURL) +} + func TestSkillsGithubConfigV0ToSkillsGithubConfigPreservesBaseURL(t *testing.T) { legacy := skillsGithubConfigV0{ BaseURL: "https://ghe.example.com/git", diff --git a/pkg/config/security_integration_test.go b/pkg/config/security_integration_test.go index 821732c5c..c97955adb 100644 --- a/pkg/config/security_integration_test.go +++ b/pkg/config/security_integration_test.go @@ -481,4 +481,50 @@ skills: assert.Equal(t, "ghp-github-registry-token-from-file", githubRegistry.AuthToken.String()) assert.Equal(t, "http://127.0.0.1:7890", githubRegistry.Param["proxy"]) }) + + t.Run("Custom registry token supports security overlay", func(t *testing.T) { + tmpDir := t.TempDir() + + customTokenFile := filepath.Join(tmpDir, "custom_registry_token.txt") + err := os.WriteFile(customTokenFile, []byte("custom-registry-token-from-file"), 0o600) + require.NoError(t, err) + + configPath := filepath.Join(tmpDir, "config.json") + configContent := `{ + "version": 1, + "tools": { + "skills": { + "registries": { + "custom": { + "enabled": true, + "base_url": "https://skills.example.com" + } + } + } + } +}` + err = os.WriteFile(configPath, []byte(configContent), 0o644) + require.NoError(t, err) + + securityPath := filepath.Join(tmpDir, SecurityConfigFile) + securityContent := `skills: + registries: + custom: + auth_token: "file://custom_registry_token.txt" +` + err = os.WriteFile(securityPath, []byte(securityContent), 0o600) + require.NoError(t, err) + + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + + customRegistry, ok := cfg.Tools.Skills.Registries.Get("custom") + require.True(t, ok) + assert.Equal(t, "https://skills.example.com", customRegistry.BaseURL) + assert.Equal(t, "custom-registry-token-from-file", customRegistry.AuthToken.String()) + + githubRegistry, ok := cfg.Tools.Skills.Registries.Get("github") + require.True(t, ok) + assert.Equal(t, "https://github.com", githubRegistry.BaseURL) + }) }