Fix security config precedence during migration (#1984)

* Fix security config precedence during migration

* add doc

* fix ci

* add baidu search

pkg/config/config.go

@@ -1314,17 +1314,29 @@ func LoadConfig(path string) (*Config, error) {
 		if err != nil {
 			return nil, err
 		}
-		// Load security configuration
+
-		securityPath := securityPath(path)
+		// Legacy config (no version field)
-		sec, err := loadSecurityConfig(securityPath)
+		tmpCfg, e := loadConfigV0(data)
+		if e != nil {
+			return nil, e
+		}
+
+		tmpCfgMigrated, e := tmpCfg.Migrate()
+		if e != nil {
+			logger.ErrorF("config migrate fail", map[string]any{"from": versionInfo.Version, "to": CurrentVersion})
+			return nil, e
+		}
+
+		// Load security configuration from .security.yml
+		secPath := securityPath(path)
+		sec, err := loadSecurityConfig(secPath)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load security config: %w", err)
 		}
 
-		// Apply security references from .security.yml BEFORE resolveAPIKeys
+		// Merge security configs: config.json takes precedence over .security.yml
-		// This resolves ref: references to actual values
+		if err := applySecurityConfigWithPrecedence(cfg, tmpCfgMigrated, sec); err != nil {
-		if err := applySecurityConfig(cfg, sec); err != nil {
+			return nil, fmt.Errorf("failed to merge security config: %w", err)
-			return nil, fmt.Errorf("failed to apply security config: %w", err)
 		}
 	default:
 		return nil, fmt.Errorf("unsupported config version: %d", versionInfo.Version)
@@ -1557,6 +1569,28 @@ func applySecurityConfig(cfg *Config, sec *SecurityConfig) error {
 	return nil
 }
 
+// applySecurityConfigWithPrecedence merges security config from tmpCfg (migrated from configV0) and sec (SecurityConfig),
+// with tmpCfg taking precedence. It then applies the merged security config to cfg.
+func applySecurityConfigWithPrecedence(cfg *Config, tmpCfg *Config, sec *SecurityConfig) error {
+	// Get security config from tmpCfg (already extracted during migration)
+	var tmpSec *SecurityConfig
+	if tmpCfg != nil {
+		tmpSec = tmpCfg.security
+	}
+
+	// If tmpCfg has no security config, just apply sec directly
+	if tmpSec == nil {
+		return applySecurityConfig(cfg, sec)
+	}
+
+	// Merge sec and tmpSec, with tmpSec (from config.json) taking precedence
+	// mergeSecurityConfig(existing, newer) - newer takes precedence
+	mergedSec := mergeSecurityConfig(sec, tmpSec)
+
+	// Apply the merged security config to cfg
+	return applySecurityConfig(cfg, mergedSec)
+}
+
 func toNameIndex(list []*ModelConfig) []string {
 	nameList := make([]string, 0, len(list))
 	countMap := make(map[string]int)

pkg/config/config_old.go

@@ -833,6 +833,7 @@ type webToolsConfigV0 struct {
 	Perplexity           perplexityConfigV0  `                                json:"perplexity"`
 	SearXNG              SearXNGConfig       `                                json:"searxng"`
 	GLMSearch            glmSearchConfigV0   `                                json:"glm_search"`
+	BaiduSearch          baiduSearchConfigV0 `                                json:"baidu_search"`
 	PreferNative         bool                `                                json:"prefer_native"                    env:"PICOCLAW_TOOLS_WEB_PREFER_NATIVE"`
 	Proxy                string              `                                json:"proxy,omitempty"                  env:"PICOCLAW_TOOLS_WEB_PROXY"`
 	FetchLimitBytes      int64               `                                json:"fetch_limit_bytes,omitempty"      env:"PICOCLAW_TOOLS_WEB_FETCH_LIMIT_BYTES"`
@@ -924,11 +925,34 @@ func (v *glmSearchConfigV0) ToGLMSearchConfig() (GLMSearchConfig, *GLMSearchSecu
 	}, sec
 }
 
+type baiduSearchConfigV0 struct {
+	Enabled    bool   `json:"enabled"     env:"PICOCLAW_TOOLS_WEB_BAIDU_ENABLED"`
+	APIKey     string `json:"api_key"     env:"PICOCLAW_TOOLS_WEB_BAIDU_API_KEY"`
+	BaseURL    string `json:"base_url"    env:"PICOCLAW_TOOLS_WEB_BAIDU_BASE_URL"`
+	MaxResults int    `json:"max_results" env:"PICOCLAW_TOOLS_WEB_BAIDU_MAX_RESULTS"`
+}
+
+func (v *baiduSearchConfigV0) ToBaiduSearchConfig() (BaiduSearchConfig, *BaiduSearchSecurity) {
+	var sec *BaiduSearchSecurity
+	if v.APIKey != "" {
+		sec = &BaiduSearchSecurity{
+			APIKey: v.APIKey,
+		}
+	}
+	return BaiduSearchConfig{
+		Enabled:    v.Enabled,
+		apiKey:     v.APIKey,
+		BaseURL:    v.BaseURL,
+		MaxResults: v.MaxResults,
+	}, sec
+}
+
 func (v *webToolsConfigV0) ToWebToolsConfig() (WebToolsConfig, WebToolsSecurity) {
 	brave, braveSecurity := v.Brave.ToBraveConfig()
 	tavily, tavilySecurity := v.Tavily.ToTavilyConfig()
 	perplexity, perplexitySecurity := v.Perplexity.ToPerplexityConfig()
 	glmSearch, glmSearchSecurity := v.GLMSearch.ToGLMSearchConfig()
+	baiduSearch, baiduSearchSecurity := v.BaiduSearch.ToBaiduSearchConfig()
 
 	return WebToolsConfig{
 			ToolConfig:           v.ToolConfig,
@@ -938,16 +962,18 @@ func (v *webToolsConfigV0) ToWebToolsConfig() (WebToolsConfig, WebToolsSecurity)
 			Perplexity:           perplexity,
 			SearXNG:              v.SearXNG,
 			GLMSearch:            glmSearch,
+			BaiduSearch:          baiduSearch,
 			PreferNative:         v.PreferNative,
 			Proxy:                v.Proxy,
 			FetchLimitBytes:      v.FetchLimitBytes,
 			Format:               v.Format,
 			PrivateHostWhitelist: v.PrivateHostWhitelist,
 		}, WebToolsSecurity{
-			Brave:      braveSecurity,
+			Brave:       braveSecurity,
-			Tavily:     tavilySecurity,
+			Tavily:      tavilySecurity,
-			Perplexity: perplexitySecurity,
+			Perplexity:  perplexitySecurity,
-			GLMSearch:  glmSearchSecurity,
+			GLMSearch:   glmSearchSecurity,
+			BaiduSearch: baiduSearchSecurity,
 		}
 }
 

pkg/config/security_integration_test.go

@@ -43,7 +43,8 @@ func TestSecurityConfigIntegration(t *testing.T) {
 	t.Run("Full workflow with security references", func(t *testing.T) {
 		tmpDir := t.TempDir()
 
-		// Create config.json with references
+		// Create config.json with direct security values (not ref: references)
+		// These values should take precedence over .security.yml
 		configPath := filepath.Join(tmpDir, "config.json")
 		configContent := `{
   "version": 1,
@@ -52,25 +53,25 @@ func TestSecurityConfigIntegration(t *testing.T) {
       "model_name": "test-model",
       "model": "openai/test-model",
       "api_base": "https://api.openai.com/v1",
-      "api_key": "ref:model_list.test-model.api_key"
+      "api_key": "sk-from-config-json-direct"
     }
   ],
   "channels": {
     "telegram": {
       "enabled": true,
-      "token": "ref:channels.telegram.token"
+      "token": "token-from-config-json-direct"
     }
   },
   "tools": {
     "web": {
       "brave": {
         "enabled": true,
-        "api_key": "ref:web.brave.api_key"
+        "api_key": "BSA-from-config-json-direct"
       }
     },
     "skills": {
       "github": {
-        "token": "ref:skills.github.token"
+        "token": "ghp-from-config-json-direct"
       }
     }
   }
@@ -78,46 +79,47 @@ func TestSecurityConfigIntegration(t *testing.T) {
 		err := os.WriteFile(configPath, []byte(configContent), 0o644)
 		require.NoError(t, err)
 
-		// Create .security.yml with actual values
+		// Create .security.yml with different values
+		// These should be overridden by config.json values
 		securityPath := filepath.Join(tmpDir, SecurityConfigFile)
 		securityContent := `model_list:
   test-model:
     api_keys:
-      - "sk-test-api-key-12345"
+      - "sk-from-security-yml"
 
 channels:
   telegram:
-    token: "123456789:ABCdefGHIjklMNOpqrsTUVwxyz"
+    token: "token-from-security-yml"
 
 web:
   brave:
     api_keys:
-      - "BSAbrave-api-key-67890"
+      - "BSA-from-security-yml"
 
 skills:
   github:
-    token: "ghp_github-token-abc123"`
+    token: "ghp-from-security-yml"`
 		err = os.WriteFile(securityPath, []byte(securityContent), 0o600)
 		require.NoError(t, err)
 
-		// Load config and verify references are resolved
+		// Load config and verify config.json values take precedence
 		cfg, err := LoadConfig(configPath)
 		require.NoError(t, err)
 		require.NotNil(t, cfg)
 
-		// Verify model API key is resolved
+		// Verify model API key from config.json takes precedence
 		assert.Equal(t, 1, len(cfg.ModelList))
 		assert.Equal(t, "test-model", cfg.ModelList[0].ModelName)
-		assert.Equal(t, "sk-test-api-key-12345", cfg.ModelList[0].apiKeys[0])
+		assert.Equal(t, "sk-from-config-json-direct", cfg.ModelList[0].apiKeys[0])
 
-		// Verify channel token is resolved
+		// Verify channel token from config.json takes precedence
-		assert.Equal(t, "123456789:ABCdefGHIjklMNOpqrsTUVwxyz", cfg.Channels.Telegram.token)
+		assert.Equal(t, "token-from-config-json-direct", cfg.Channels.Telegram.token)
 
-		// Verify web tool API key is resolved
+		// Verify web tool API key from config.json takes precedence
-		assert.Equal(t, "BSAbrave-api-key-67890", cfg.Tools.Web.Brave.APIKey())
+		assert.Equal(t, "BSA-from-config-json-direct", cfg.Tools.Web.Brave.APIKey())
 
-		// Verify skills token is resolved
+		// Verify skills token from config.json takes precedence
-		assert.Equal(t, "ghp_github-token-abc123", cfg.Tools.Skills.Github.token)
+		assert.Equal(t, "ghp-from-config-json-direct", cfg.Tools.Skills.Github.token)
 	})
 }