fix: apply security credentials before config validation in web handlers
- Move SecurityCopyFrom() before validateConfig() in PUT and PATCH handlers - Make SecurityCopyFrom() call applySecurityConfig() to populate private fields - Add tests for config save with security-only channel tokens Without this fix, saving config via the web UI fails with 'channels.pico.token is required' (and similar for Telegram/Discord) when tokens are stored in .security.yml, because the validation ran before security credentials were copied to the config struct.
This commit is contained in:
parent
96e312680d
commit
1f9d390a64
3 changed files with 135 additions and 9 deletions
|
|
@ -1942,6 +1942,11 @@ 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 err := applySecurityConfig(c, c.security); err != nil {
|
||||||
|
logger.Errorf("failed to apply security config in SecurityCopyFrom: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func MergeAPIKeys(apiKey string, apiKeys []string) []string {
|
func MergeAPIKeys(apiKey string, apiKeys []string) []string {
|
||||||
|
|
|
||||||
|
|
@ -54,6 +54,15 @@ func (h *Handler) handleUpdateConfig(w http.ResponseWriter, r *http.Request) {
|
||||||
cfg.Tools.Exec.AllowRemote = config.DefaultConfig().Tools.Exec.AllowRemote
|
cfg.Tools.Exec.AllowRemote = config.DefaultConfig().Tools.Exec.AllowRemote
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Load existing config and copy security credentials before validation,
|
||||||
|
// so that security-managed fields (e.g. pico token) are available.
|
||||||
|
oldCfg, err := config.LoadConfig(h.configPath)
|
||||||
|
if err != nil {
|
||||||
|
http.Error(w, fmt.Sprintf("Failed to load config: %v", err), http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
cfg.SecurityCopyFrom(oldCfg)
|
||||||
|
|
||||||
if errs := validateConfig(&cfg); len(errs) > 0 {
|
if errs := validateConfig(&cfg); len(errs) > 0 {
|
||||||
w.Header().Set("Content-Type", "application/json")
|
w.Header().Set("Content-Type", "application/json")
|
||||||
w.WriteHeader(http.StatusBadRequest)
|
w.WriteHeader(http.StatusBadRequest)
|
||||||
|
|
@ -64,13 +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")
|
||||||
oldCfg, err := config.LoadConfig(h.configPath)
|
|
||||||
if err != nil {
|
|
||||||
http.Error(w, fmt.Sprintf("Failed to load config: %v", err), http.StatusInternalServerError)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
cfg.SecurityCopyFrom(oldCfg)
|
|
||||||
|
|
||||||
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)
|
||||||
|
|
@ -149,6 +152,10 @@ func (h *Handler) handlePatchConfig(w http.ResponseWriter, r *http.Request) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Copy security credentials before validation so security-managed
|
||||||
|
// fields (e.g. pico token) are available for validation checks.
|
||||||
|
newCfg.SecurityCopyFrom(cfg)
|
||||||
|
|
||||||
if errs := validateConfig(&newCfg); len(errs) > 0 {
|
if errs := validateConfig(&newCfg); len(errs) > 0 {
|
||||||
w.Header().Set("Content-Type", "application/json")
|
w.Header().Set("Content-Type", "application/json")
|
||||||
w.WriteHeader(http.StatusBadRequest)
|
w.WriteHeader(http.StatusBadRequest)
|
||||||
|
|
@ -159,8 +166,6 @@ func (h *Handler) handlePatchConfig(w http.ResponseWriter, r *http.Request) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
newCfg.SecurityCopyFrom(cfg)
|
|
||||||
|
|
||||||
if err := config.SaveConfig(h.configPath, &newCfg); err != nil {
|
if err := config.SaveConfig(h.configPath, &newCfg); 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)
|
||||||
return
|
return
|
||||||
|
|
|
||||||
|
|
@ -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()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue