mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix(config): normalize empty security config before save/load (#1956)
Normalize missing security sections when attaching, loading, and saving security config so existing config files without `.security.yml` can still be updated safely. This fixes Pico channel setup for legacy/existing configs and adds coverage for the missing security file path and unexported JSON field behavior.
This commit is contained in:
@@ -106,6 +106,7 @@ func (c *Config) WithSecurity(sec *SecurityConfig) *Config {
|
||||
c.security = sec
|
||||
return c
|
||||
}
|
||||
sec = normalizeSecurityConfig(sec)
|
||||
err := applySecurityConfig(c, sec)
|
||||
if err != nil {
|
||||
return nil
|
||||
@@ -1768,6 +1769,7 @@ func SaveConfig(path string, cfg *Config) error {
|
||||
logger.ErrorC("config", "security is nil")
|
||||
return fmt.Errorf("security is nil")
|
||||
}
|
||||
cfg.security = normalizeSecurityConfig(cfg.security)
|
||||
// Ensure version is always set when saving
|
||||
if cfg.Version == 0 {
|
||||
cfg.Version = CurrentVersion
|
||||
|
||||
+21
-2
@@ -25,6 +25,25 @@ const (
|
||||
SecurityConfigFile = ".security.yml"
|
||||
)
|
||||
|
||||
func normalizeSecurityConfig(sec *SecurityConfig) *SecurityConfig {
|
||||
if sec == nil {
|
||||
sec = &SecurityConfig{}
|
||||
}
|
||||
if sec.ModelList == nil {
|
||||
sec.ModelList = map[string]ModelSecurityEntry{}
|
||||
}
|
||||
if sec.Channels == nil {
|
||||
sec.Channels = &ChannelsSecurity{}
|
||||
}
|
||||
if sec.Web == nil {
|
||||
sec.Web = &WebToolsSecurity{}
|
||||
}
|
||||
if sec.Skills == nil {
|
||||
sec.Skills = &SkillsSecurity{}
|
||||
}
|
||||
return sec
|
||||
}
|
||||
|
||||
// SecurityConfig stores all sensitive data (API keys, tokens, secrets, passwords)
|
||||
// This data is loaded from security.yml and kept separate from the main config
|
||||
type SecurityConfig struct {
|
||||
@@ -191,7 +210,7 @@ func loadSecurityConfig(securityPath string) (*SecurityConfig, error) {
|
||||
data, err := os.ReadFile(securityPath)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
return &SecurityConfig{}, nil
|
||||
return normalizeSecurityConfig(nil), nil
|
||||
}
|
||||
return nil, fmt.Errorf("failed to read security config: %w", err)
|
||||
}
|
||||
@@ -210,7 +229,7 @@ func loadSecurityConfig(securityPath string) (*SecurityConfig, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return &sec, nil
|
||||
return normalizeSecurityConfig(&sec), nil
|
||||
}
|
||||
|
||||
// saveSecurityConfig saves the security configuration to security.yml
|
||||
|
||||
@@ -17,13 +17,12 @@ import (
|
||||
|
||||
// Test JSON unmarshal of private fields
|
||||
func TestJSONUnmarshalPrivateFields(t *testing.T) {
|
||||
//nolint: govet
|
||||
type testStruct struct {
|
||||
PublicField string `json:"public"`
|
||||
privateField string `json:"private"`
|
||||
privateField string
|
||||
}
|
||||
|
||||
data := `{"public": "pub", "private": "priv"}`
|
||||
data := `{"public": "pub", "privateField": "priv"}`
|
||||
var s testStruct
|
||||
if err := json.Unmarshal([]byte(data), &s); err != nil {
|
||||
t.Fatalf("JSON unmarshal failed: %v", err)
|
||||
@@ -35,9 +34,8 @@ func TestJSONUnmarshalPrivateFields(t *testing.T) {
|
||||
if s.PublicField != "pub" {
|
||||
t.Errorf("PublicField = %q, want 'pub'", s.PublicField)
|
||||
}
|
||||
// This should fail because privateField is unexported
|
||||
if s.privateField != "priv" {
|
||||
t.Logf("privateField = %q, want 'priv' - THIS IS EXPECTED TO FAIL", s.privateField)
|
||||
if s.privateField != "" {
|
||||
t.Errorf("privateField = %q, want empty because unexported fields are ignored", s.privateField)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20,6 +20,9 @@ func TestSecurityConfig(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.NotNil(t, sec)
|
||||
assert.Empty(t, sec.ModelList)
|
||||
assert.NotNil(t, sec.Channels)
|
||||
assert.NotNil(t, sec.Web)
|
||||
assert.NotNil(t, sec.Skills)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -170,7 +170,7 @@ func setupPicoEnabledEnv(t *testing.T) (string, func()) {
|
||||
ModelList: map[string]config.ModelSecurityEntry{
|
||||
"custom-default": {APIKeys: []string{"sk-default"}},
|
||||
},
|
||||
Channels: config.ChannelsSecurity{
|
||||
Channels: &config.ChannelsSecurity{
|
||||
Pico: &config.PicoSecurity{Token: "test-pico-token"},
|
||||
},
|
||||
})
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"testing"
|
||||
@@ -154,6 +155,44 @@ func TestEnsurePicoChannel_PreservesUserSettings(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsurePicoChannel_ExistingConfigWithoutSecurityFile(t *testing.T) {
|
||||
configPath := filepath.Join(t.TempDir(), "config.json")
|
||||
|
||||
cfg := config.DefaultConfig()
|
||||
raw, err := json.Marshal(cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("Marshal() error = %v", err)
|
||||
}
|
||||
if err = os.WriteFile(configPath, raw, 0o600); err != nil {
|
||||
t.Fatalf("WriteFile() error = %v", err)
|
||||
}
|
||||
|
||||
h := NewHandler(configPath)
|
||||
|
||||
changed, err := h.ensurePicoChannel("")
|
||||
if err != nil {
|
||||
t.Fatalf("ensurePicoChannel() error = %v", err)
|
||||
}
|
||||
if !changed {
|
||||
t.Fatal("ensurePicoChannel() should report changed when pico is missing")
|
||||
}
|
||||
|
||||
cfg, err = config.LoadConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", err)
|
||||
}
|
||||
|
||||
if !cfg.Channels.Pico.Enabled {
|
||||
t.Error("expected Pico to be enabled after setup")
|
||||
}
|
||||
if cfg.Channels.Pico.Token() == "" {
|
||||
t.Error("expected a non-empty token after setup")
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(filepath.Dir(configPath), config.SecurityConfigFile)); err != nil {
|
||||
t.Fatalf("expected .security.yml to be created: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsurePicoChannel_Idempotent(t *testing.T) {
|
||||
configPath := filepath.Join(t.TempDir(), "config.json")
|
||||
h := NewHandler(configPath)
|
||||
|
||||
Reference in New Issue
Block a user