fix(web): save channel configs with nested channel_list patches (#2530)

Persist channel settings through the current channel_list schema, keeping common
channel fields at the top level and channel-specific fields under settings.
Return common fields and default config shapes from channel config endpoints, and
add coverage for nested patches, missing channel defaults, and secret handling.
This commit is contained in:
wenjie 2026-04-16 10:30:16 +08:00 committed by GitHub
parent f32b303d2a
commit a8d0b03515
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 295 additions and 14 deletions

View file

@ -514,6 +514,14 @@ func defaultChannels() ChannelsConfig {
"max_connections": 100, "max_connections": 100,
}, },
}, },
"irc": map[string]any{
"settings": map[string]any{
"server": "",
"tls": true,
"nick": "picoclaw",
"channels": []string{},
},
},
} }
channels := make(ChannelsConfig, len(defs)) channels := make(ChannelsConfig, len(defs))

View file

@ -116,31 +116,60 @@ func buildChannelConfigResponse(cfg *config.Config, item channelCatalogItem) cha
} }
bc := cfg.Channels.Get(item.ConfigKey) bc := cfg.Channels.Get(item.ConfigKey)
if bc == nil {
bc = defaultChannelConfig(item.ConfigKey)
if bc == nil { if bc == nil {
resp.Config = map[string]any{} resp.Config = map[string]any{}
return resp return resp
} }
}
// Detect configured secrets by checking the raw Settings JSON // Detect configured secrets by checking the raw Settings JSON
secrets := detectConfiguredSecrets(bc.Settings, item.Name) secrets := detectConfiguredSecrets(bc.Settings, item.Name)
resp.ConfiguredSecrets = secrets resp.ConfiguredSecrets = secrets
// Parse settings into a generic map for JSON response // Parse settings into a generic map for JSON response
var settings map[string]any settings := map[string]any{}
if len(bc.Settings) > 0 {
if err := json.Unmarshal(bc.Settings, &settings); err != nil { if err := json.Unmarshal(bc.Settings, &settings); err != nil {
resp.Config = map[string]any{} resp.Config = map[string]any{}
return resp return resp
} }
}
// Remove secure fields from response // Remove secure fields from response
for _, key := range secrets { for _, key := range secrets {
delete(settings, key) delete(settings, key)
} }
addChannelCommonConfig(settings, bc)
resp.Config = settings resp.Config = settings
return resp return resp
} }
func defaultChannelConfig(configKey string) *config.Channel {
return config.DefaultConfig().Channels.Get(configKey)
}
func addChannelCommonConfig(settings map[string]any, bc *config.Channel) {
settings["enabled"] = bc.Enabled
if len(bc.AllowFrom) > 0 {
settings["allow_from"] = []string(bc.AllowFrom)
}
if bc.ReasoningChannelID != "" {
settings["reasoning_channel_id"] = bc.ReasoningChannelID
}
if bc.GroupTrigger.MentionOnly || len(bc.GroupTrigger.Prefixes) > 0 {
settings["group_trigger"] = bc.GroupTrigger
}
if bc.Typing.Enabled {
settings["typing"] = bc.Typing
}
if bc.Placeholder.Enabled || len(bc.Placeholder.Text) > 0 {
settings["placeholder"] = bc.Placeholder
}
}
func detectConfiguredSecrets(settings config.RawNode, channelName string) []string { func detectConfiguredSecrets(settings config.RawNode, channelName string) []string {
var m map[string]any var m map[string]any
if err := json.Unmarshal(settings, &m); err != nil { if err := json.Unmarshal(settings, &m); err != nil {

View file

@ -27,6 +27,7 @@ func TestHandleGetChannelConfig_ReturnsSecretPresenceWithoutLeakingSecrets(t *te
bcfg := decoded.(*config.FeishuSettings) bcfg := decoded.(*config.FeishuSettings)
bcfg.AppID = "cli_test_app" bcfg.AppID = "cli_test_app"
bcfg.AppSecret = *config.NewSecureString("feishu-secret-from-security") bcfg.AppSecret = *config.NewSecureString("feishu-secret-from-security")
bc.AllowFrom = config.FlexibleStringSlice{"ou_test_user"}
if err := config.SaveConfig(configPath, cfg); err != nil { if err := config.SaveConfig(configPath, cfg); err != nil {
t.Fatalf("SaveConfig() error = %v", err) t.Fatalf("SaveConfig() error = %v", err)
} }
@ -67,6 +68,13 @@ func TestHandleGetChannelConfig_ReturnsSecretPresenceWithoutLeakingSecrets(t *te
if got := resp.Config["app_id"]; got != "cli_test_app" { if got := resp.Config["app_id"]; got != "cli_test_app" {
t.Fatalf("config.app_id = %#v, want %q", got, "cli_test_app") t.Fatalf("config.app_id = %#v, want %q", got, "cli_test_app")
} }
if got := resp.Config["enabled"]; got != true {
t.Fatalf("config.enabled = %#v, want true", got)
}
allowFrom, ok := resp.Config["allow_from"].([]any)
if !ok || len(allowFrom) != 1 || allowFrom[0] != "ou_test_user" {
t.Fatalf("config.allow_from = %#v, want [\"ou_test_user\"]", resp.Config["allow_from"])
}
if _, exists := resp.Config["app_secret"]; exists { if _, exists := resp.Config["app_secret"]; exists {
t.Fatalf("config should omit app_secret, got %#v", resp.Config["app_secret"]) t.Fatalf("config should omit app_secret, got %#v", resp.Config["app_secret"])
} }
@ -91,3 +99,97 @@ func TestHandleGetChannelConfig_ReturnsNotFoundForUnknownChannel(t *testing.T) {
t.Fatalf("GET /api/channels/not-a-channel/config status = %d, want %d", rec.Code, http.StatusNotFound) t.Fatalf("GET /api/channels/not-a-channel/config status = %d, want %d", rec.Code, http.StatusNotFound)
} }
} }
func TestHandleGetChannelConfig_ReturnsCommonFieldsWhenSettingsEmpty(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
bc := cfg.Channels[config.ChannelFeishu]
bc.Enabled = true
bc.AllowFrom = config.FlexibleStringSlice{"ou_common_user"}
if err := config.SaveConfig(configPath, cfg); err != nil {
t.Fatalf("SaveConfig() error = %v", err)
}
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
req := httptest.NewRequest(http.MethodGet, "/api/channels/feishu/config", nil)
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf(
"GET /api/channels/feishu/config status = %d, want %d, body=%s",
rec.Code,
http.StatusOK,
rec.Body.String(),
)
}
var resp struct {
Config map[string]any `json:"config"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
t.Fatalf("json.Unmarshal() error = %v", err)
}
if got := resp.Config["enabled"]; got != true {
t.Fatalf("config.enabled = %#v, want true", got)
}
allowFrom, ok := resp.Config["allow_from"].([]any)
if !ok || len(allowFrom) != 1 || allowFrom[0] != "ou_common_user" {
t.Fatalf("config.allow_from = %#v, want [\"ou_common_user\"]", resp.Config["allow_from"])
}
}
func TestHandleGetChannelConfig_ReturnsDefaultShapeForMissingChannel(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
delete(cfg.Channels, config.ChannelIRC)
if err := config.SaveConfig(configPath, cfg); err != nil {
t.Fatalf("SaveConfig() error = %v", err)
}
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
req := httptest.NewRequest(http.MethodGet, "/api/channels/irc/config", nil)
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf(
"GET /api/channels/irc/config status = %d, want %d, body=%s",
rec.Code,
http.StatusOK,
rec.Body.String(),
)
}
var resp struct {
Config map[string]any `json:"config"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
t.Fatalf("json.Unmarshal() error = %v", err)
}
if got := resp.Config["server"]; got != "" {
t.Fatalf("config.server = %#v, want empty string", got)
}
if got := resp.Config["nick"]; got != "picoclaw" {
t.Fatalf("config.nick = %#v, want %q", got, "picoclaw")
}
if got := resp.Config["enabled"]; got != false {
t.Fatalf("config.enabled = %#v, want false", got)
}
}

View file

@ -174,6 +174,130 @@ func TestHandlePatchConfig_AllowsInvalidExecRegexPatternsWhenExecDisabled(t *tes
} }
} }
func TestHandlePatchConfig_SavesChannelListSettingsPatch(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(`{
"channel_list": {
"feishu": {
"enabled": true,
"allow_from": ["ou_patch_user"],
"settings": {
"app_id": "cli_patch_app",
"app_secret": "patch-secret",
"is_lark": true
}
}
}
}`))
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())
}
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
bc := cfg.Channels[config.ChannelFeishu]
if !bc.Enabled {
t.Fatal("feishu should be enabled after PATCH")
}
if len(bc.AllowFrom) != 1 || bc.AllowFrom[0] != "ou_patch_user" {
t.Fatalf("feishu allow_from = %#v, want [\"ou_patch_user\"]", bc.AllowFrom)
}
decoded, err := bc.GetDecoded()
if err != nil {
t.Fatalf("GetDecoded() error = %v", err)
}
feishuCfg := decoded.(*config.FeishuSettings)
if got := feishuCfg.AppID; got != "cli_patch_app" {
t.Fatalf("feishu app_id = %q, want %q", got, "cli_patch_app")
}
if got := feishuCfg.AppSecret.String(); got != "patch-secret" {
t.Fatalf("feishu app_secret = %q, want %q", got, "patch-secret")
}
if !feishuCfg.IsLark {
t.Fatal("feishu is_lark should be true after PATCH")
}
}
func TestHandlePatchConfig_CreatesMissingChannelWithTypeAndSecret(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
delete(cfg.Channels, config.ChannelIRC)
if err = config.SaveConfig(configPath, cfg); err != nil {
t.Fatalf("SaveConfig() error = %v", err)
}
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(`{
"channel_list": {
"irc": {
"enabled": true,
"type": "irc",
"settings": {
"server": "irc.example.com",
"password": "irc-patch-password"
}
}
}
}`))
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())
}
cfg, err = config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
bc := cfg.Channels[config.ChannelIRC]
if bc == nil {
t.Fatal("irc channel should exist after PATCH")
}
if got := bc.Type; got != config.ChannelIRC {
t.Fatalf("irc type = %q, want %q", got, config.ChannelIRC)
}
decoded, err := bc.GetDecoded()
if err != nil {
t.Fatalf("GetDecoded() error = %v", err)
}
ircCfg := decoded.(*config.IRCSettings)
if got := ircCfg.Server; got != "irc.example.com" {
t.Fatalf("irc server = %q, want %q", got, "irc.example.com")
}
if got := ircCfg.Password.String(); got != "irc-patch-password" {
t.Fatalf("irc password = %q, want %q", got, "irc-patch-password")
}
configData, err := os.ReadFile(configPath)
if err != nil {
t.Fatalf("ReadFile(configPath) error = %v", err)
}
if bytes.Contains(configData, []byte("irc-patch-password")) {
t.Fatalf("config file leaked irc password: %s", string(configData))
}
}
// setupPicoEnabledEnv creates a test environment with Pico channel enabled and // setupPicoEnabledEnv creates a test environment with Pico channel enabled and
// its token stored only in .security.yml (not in the JSON payload). // its token stored only in .security.yml (not in the JSON payload).
func setupPicoEnabledEnv(t *testing.T) (string, func()) { func setupPicoEnabledEnv(t *testing.T) (string, func()) {

View file

@ -48,6 +48,14 @@ function asBool(value: unknown): boolean {
return value === true return value === true
} }
const CHANNEL_COMMON_CONFIG_KEYS = new Set([
"allow_from",
"group_trigger",
"placeholder",
"reasoning_channel_id",
"typing",
])
function normalizeConfig( function normalizeConfig(
channel: SupportedChannel, channel: SupportedChannel,
rawConfig: ChannelConfig, rawConfig: ChannelConfig,
@ -67,33 +75,42 @@ function buildSavePayload(
editConfig: ChannelConfig, editConfig: ChannelConfig,
enabled: boolean, enabled: boolean,
): ChannelConfig { ): ChannelConfig {
const payload: ChannelConfig = { enabled } const payload: ChannelConfig = { enabled, type: channel.config_key }
const settings: ChannelConfig = {}
for (const [key, value] of Object.entries(editConfig)) { for (const [key, value] of Object.entries(editConfig)) {
if (key.startsWith("_")) continue if (key.startsWith("_")) continue
if (key === "enabled") continue if (key === "enabled") continue
if (CHANNEL_COMMON_CONFIG_KEYS.has(key)) {
payload[key] = value
continue
}
if (isSecretField(key)) continue if (isSecretField(key)) continue
payload[key] = value settings[key] = value
} }
for (const [secretKey, editKey] of Object.entries(SECRET_FIELD_MAP)) { for (const [secretKey, editKey] of Object.entries(SECRET_FIELD_MAP)) {
const incoming = asString(editConfig[editKey]) const incoming = asString(editConfig[editKey])
if (incoming !== "") { if (incoming !== "") {
payload[secretKey] = incoming settings[secretKey] = incoming
continue continue
} }
const existing = asString(editConfig[secretKey]).trim() const existing = asString(editConfig[secretKey]).trim()
if (existing !== "") { if (existing !== "") {
payload[secretKey] = existing settings[secretKey] = existing
} }
} }
if (channel.name === "whatsapp_native") { if (channel.name === "whatsapp_native") {
payload.use_native = true settings.use_native = true
} }
if (channel.name === "whatsapp") { if (channel.name === "whatsapp") {
payload.use_native = false settings.use_native = false
}
if (Object.keys(settings).length > 0) {
payload.settings = settings
} }
return payload return payload
@ -377,7 +394,7 @@ export function ChannelConfigPage({ channelName }: ChannelConfigPageProps) {
setFieldErrors({}) setFieldErrors({})
try { try {
await patchAppConfig({ await patchAppConfig({
channels: { channel_list: {
[channel.config_key]: savePayload, [channel.config_key]: savePayload,
}, },
}) })

View file

@ -130,9 +130,10 @@ export function WecomForm({
setToggleError("") setToggleError("")
try { try {
await patchAppConfig({ await patchAppConfig({
channels: { channel_list: {
wecom: { wecom: {
enabled: checked, enabled: checked,
type: "wecom",
}, },
}, },
}) })