fix: improve error handling in security configuration and update logging messages

This commit is contained in:
Kristjan Kruus 2026-03-23 14:50:23 +02:00
parent 0aa8c95cb9
commit f09b36d1f9
3 changed files with 120 additions and 2 deletions

View file

@ -1943,7 +1943,9 @@ func (c *Config) ValidateModelList() error {
func (c *Config) SecurityCopyFrom(cfg *Config) { func (c *Config) SecurityCopyFrom(cfg *Config) {
c.security = cfg.security c.security = cfg.security
if c.security != nil { if c.security != nil {
_ = applySecurityConfig(c, c.security) if err := applySecurityConfig(c, c.security); err != nil {
logger.Errorf("failed to apply security config in SecurityCopyFrom: %v", err)
}
} }
} }

View file

@ -73,7 +73,7 @@ func (h *Handler) handleUpdateConfig(w http.ResponseWriter, r *http.Request) {
return return
} }
logger.Infof("new config: %+v", cfg) logger.Infof("configuration updated successfully")
if err := config.SaveConfig(h.configPath, &cfg); err != nil { if err := config.SaveConfig(h.configPath, &cfg); err != nil {
http.Error(w, fmt.Sprintf("Failed to save config: %v", err), http.StatusInternalServerError) http.Error(w, fmt.Sprintf("Failed to save config: %v", err), http.StatusInternalServerError)

View file

@ -4,6 +4,8 @@ import (
"bytes" "bytes"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"os"
"path/filepath"
"testing" "testing"
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
@ -141,6 +143,120 @@ func TestHandlePatchConfig_AllowsInvalidExecRegexPatternsWhenExecDisabled(t *tes
} }
} }
// setupPicoEnabledEnv creates a test environment with Pico channel enabled and
// its token stored only in .security.yml (not in the JSON payload).
func setupPicoEnabledEnv(t *testing.T) (string, func()) {
t.Helper()
tmp := t.TempDir()
oldHome := os.Getenv("HOME")
oldPicoHome := os.Getenv("PICOCLAW_HOME")
if err := os.Setenv("HOME", tmp); err != nil {
t.Fatalf("set HOME: %v", err)
}
if err := os.Setenv("PICOCLAW_HOME", filepath.Join(tmp, ".picoclaw")); err != nil {
t.Fatalf("set PICOCLAW_HOME: %v", err)
}
cfg := config.DefaultConfig()
cfg.ModelList = []*config.ModelConfig{{
ModelName: "custom-default",
Model: "openai/gpt-4o",
}}
cfg.Agents.Defaults.ModelName = "custom-default"
cfg.Channels.Pico.Enabled = true
cfg.WithSecurity(&config.SecurityConfig{
ModelList: map[string]config.ModelSecurityEntry{
"custom-default": {APIKeys: []string{"sk-default"}},
},
Channels: config.ChannelsSecurity{
Pico: &config.PicoSecurity{Token: "test-pico-token"},
},
})
configPath := filepath.Join(tmp, "config.json")
if err := config.SaveConfig(configPath, cfg); err != nil {
t.Fatalf("SaveConfig error: %v", err)
}
cleanup := func() {
_ = os.Setenv("HOME", oldHome)
if oldPicoHome == "" {
_ = os.Unsetenv("PICOCLAW_HOME")
} else {
_ = os.Setenv("PICOCLAW_HOME", oldPicoHome)
}
}
return configPath, cleanup
}
func TestHandleUpdateConfig_SucceedsWhenPicoTokenInSecurityOnly(t *testing.T) {
configPath, cleanup := setupPicoEnabledEnv(t)
defer cleanup()
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
// PUT request with pico enabled but no token in JSON — token is in .security.yml
req := httptest.NewRequest(http.MethodPut, "/api/config", bytes.NewBufferString(`{
"version": 1,
"agents": {
"defaults": {
"workspace": "~/.picoclaw/workspace",
"model_name": "custom-default"
}
},
"channels": {
"pico": {
"enabled": true,
"ping_interval": 30,
"read_timeout": 60,
"write_timeout": 10,
"max_connections": 100
}
},
"model_list": [
{
"model_name": "custom-default",
"model": "openai/gpt-4o",
"api_keys": ["sk-default"]
}
]
}`))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("PUT /api/config status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
}
}
func TestHandlePatchConfig_SucceedsWhenPicoTokenInSecurityOnly(t *testing.T) {
configPath, cleanup := setupPicoEnabledEnv(t)
defer cleanup()
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
// PATCH request changing an unrelated field — pico token still in .security.yml
req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(`{
"gateway": {
"log_level": "info"
}
}`))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("PATCH /api/config status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
}
}
func TestHandlePatchConfig_AllowsInvalidDenyRegexPatternsWhenDenyPatternsDisabled(t *testing.T) { func TestHandlePatchConfig_AllowsInvalidDenyRegexPatternsWhenDenyPatternsDisabled(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t) configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup() defer cleanup()