feat(web): support list editing for channel array fields (#2595)

Add reusable channel array list controls and parsing utilities for channel forms. Normalize channel string-array payloads in the backend, including pasted values, numeric IDs, hidden characters, duplicates, and empty clears. Also allow FlexibleStringSlice to unmarshal null values and cover the new behavior with backend and config tests.
2026-06-12 18:08:54 +00:00 · 2026-04-21 16:04:28 +08:00
parent dcb4b67e00
commit ba6992234f
15 changed files with 1025 additions and 195 deletions
@@ -56,13 +56,22 @@ func (h *Handler) handleUpdateConfig(w http.ResponseWriter, r *http.Request) {
 	}
 	defer r.Body.Close()

-	var cfg config.Config
-	if err = json.Unmarshal(body, &cfg); err != nil {
+	var raw map[string]any
+	if err = json.Unmarshal(body, &raw); err != nil {
 		http.Error(w, fmt.Sprintf("Invalid JSON: %v", err), http.StatusBadRequest)
 		return
 	}
-	var raw map[string]any
-	if err = json.Unmarshal(body, &raw); err != nil {
+	if err = normalizeChannelArrayFields(raw); err != nil {
+		http.Error(w, fmt.Sprintf("Invalid channel array field: %v", err), http.StatusBadRequest)
+		return
+	}
+	normalizedBody, err := json.Marshal(raw)
+	if err != nil {
+		http.Error(w, "Failed to normalize config payload", http.StatusBadRequest)
+		return
+	}
+	var cfg config.Config
+	if err = json.Unmarshal(normalizedBody, &cfg); err != nil {
 		http.Error(w, fmt.Sprintf("Invalid JSON: %v", err), http.StatusBadRequest)
 		return
 	}
@@ -154,6 +163,10 @@ func (h *Handler) handlePatchConfig(w http.ResponseWriter, r *http.Request) {

 	// Recursively merge patch into base
 	mergeMap(base, patch)
+	if err = normalizeChannelArrayFields(base); err != nil {
+		http.Error(w, fmt.Sprintf("Invalid channel array field: %v", err), http.StatusBadRequest)
+		return
+	}

 	// Convert merged map back to Config struct
 	merged, err := json.Marshal(base)
@@ -382,6 +395,184 @@ func asMapField(value map[string]any, key string) (map[string]any, bool) {
 	return m, isMap
 }

+var (
+	allowFromHiddenCharsRe = regexp.MustCompile("[\u200B\u200C\u200D\u200E\u200F\u202A-\u202E\u2060-\u2069\uFEFF]")
+	allowFromSplitRe       = regexp.MustCompile("[,\uFF0C、;；\r\n\t]+")
+	conservativeSplitRe    = regexp.MustCompile("[,\uFF0C\r\n\t]+")
+)
+
+type stringArrayParserOptions struct {
+	stripHiddenChars bool
+}
+
+func normalizeChannelArrayFields(raw map[string]any) error {
+	channelsMap, hasChannels := asMapField(raw, "channel_list")
+	if !hasChannels {
+		return nil
+	}
+
+	defaultCfg := config.DefaultConfig()
+	for channelName, rawChannel := range channelsMap {
+		chMap, ok := rawChannel.(map[string]any)
+		if !ok {
+			continue
+		}
+
+		if rawAllowFrom, exists := chMap["allow_from"]; exists {
+			normalized, err := normalizeStringArrayValue(rawAllowFrom, stringArrayParserOptions{
+				stripHiddenChars: true,
+			})
+			if err != nil {
+				return fmt.Errorf("channel_list.%s.allow_from: %w", channelName, err)
+			}
+			chMap["allow_from"] = normalized
+		}
+
+		if groupTrigger, ok := asMapField(chMap, "group_trigger"); ok {
+			if rawPrefixes, exists := groupTrigger["prefixes"]; exists {
+				normalized, err := normalizeStringArrayValue(rawPrefixes, stringArrayParserOptions{})
+				if err != nil {
+					return fmt.Errorf("channel_list.%s.group_trigger.prefixes: %w", channelName, err)
+				}
+				groupTrigger["prefixes"] = normalized
+			}
+		}
+
+		settingsMap, hasSettings := asMapField(chMap, "settings")
+		if !hasSettings {
+			continue
+		}
+
+		settingsType := channelSettingsType(defaultCfg, channelName, chMap)
+		if settingsType == nil {
+			continue
+		}
+
+		for i := range settingsType.NumField() {
+			field := settingsType.Field(i)
+			if !field.IsExported() || !isStringSliceType(field.Type) {
+				continue
+			}
+			jsonKey := strings.Split(field.Tag.Get("json"), ",")[0]
+			if jsonKey == "" || jsonKey == "-" {
+				continue
+			}
+			rawValue, exists := settingsMap[jsonKey]
+			if !exists {
+				continue
+			}
+
+			options := stringArrayParserOptions{}
+			if jsonKey == "allow_from" {
+				options.stripHiddenChars = true
+			}
+			normalized, err := normalizeStringArrayValue(rawValue, options)
+			if err != nil {
+				return fmt.Errorf("channel_list.%s.settings.%s: %w", channelName, jsonKey, err)
+			}
+			settingsMap[jsonKey] = normalized
+		}
+	}
+	return nil
+}
+
+func channelSettingsType(
+	defaultCfg *config.Config,
+	channelName string,
+	channelMap map[string]any,
+) reflect.Type {
+	if channelType, _ := channelMap["type"].(string); channelType != "" {
+		if bc := defaultCfg.Channels.GetByType(channelType); bc != nil {
+			if decoded, err := bc.GetDecoded(); err == nil && decoded != nil {
+				return derefType(reflect.TypeOf(decoded))
+			}
+		}
+	}
+
+	if bc := defaultCfg.Channels.Get(channelName); bc != nil {
+		if decoded, err := bc.GetDecoded(); err == nil && decoded != nil {
+			return derefType(reflect.TypeOf(decoded))
+		}
+	}
+
+	return nil
+}
+
+func derefType(typ reflect.Type) reflect.Type {
+	for typ != nil && typ.Kind() == reflect.Ptr {
+		typ = typ.Elem()
+	}
+	return typ
+}
+
+func isStringSliceType(typ reflect.Type) bool {
+	typ = derefType(typ)
+	return typ != nil && typ.Kind() == reflect.Slice && typ.Elem().Kind() == reflect.String
+}
+
+func normalizeStringArrayValue(value any, options stringArrayParserOptions) ([]string, error) {
+	switch typed := value.(type) {
+	case nil:
+		return nil, nil
+	case string:
+		return parseStringArrayValue(typed, options), nil
+	case float64:
+		return normalizeStringArrayItems([]string{fmt.Sprintf("%.0f", typed)}, options), nil
+	case []string:
+		return normalizeStringArrayItems(typed, options), nil
+	case []any:
+		items := make([]string, 0, len(typed))
+		for _, item := range typed {
+			switch raw := item.(type) {
+			case string:
+				items = append(items, raw)
+			case float64:
+				items = append(items, fmt.Sprintf("%.0f", raw))
+			default:
+				return nil, fmt.Errorf("unsupported list item type %T", item)
+			}
+		}
+		return normalizeStringArrayItems(items, options), nil
+	default:
+		return nil, fmt.Errorf("unsupported list field type %T", value)
+	}
+}
+
+func parseStringArrayValue(raw string, options stringArrayParserOptions) []string {
+	if strings.TrimSpace(raw) == "" {
+		return []string{}
+	}
+	splitRe := conservativeSplitRe
+	if options.stripHiddenChars {
+		splitRe = allowFromSplitRe
+	}
+	return normalizeStringArrayItems(splitRe.Split(raw, -1), options)
+}
+
+func normalizeStringArrayItems(items []string, options stringArrayParserOptions) []string {
+	result := make([]string, 0, len(items))
+	seen := make(map[string]struct{}, len(items))
+	for _, item := range items {
+		normalized := item
+		if options.stripHiddenChars {
+			normalized = allowFromHiddenCharsRe.ReplaceAllString(normalized, "")
+		}
+		normalized = strings.TrimSpace(normalized)
+		if normalized == "" {
+			continue
+		}
+		if _, exists := seen[normalized]; exists {
+			continue
+		}
+		seen[normalized] = struct{}{}
+		result = append(result, normalized)
+	}
+	if len(result) == 0 {
+		return []string{}
+	}
+	return result
+}
+
 func getSecretString(m map[string]any, key string) (string, bool) {
 	if raw, exists := m[key]; exists {
 		s, isString := raw.(string)
@@ -230,6 +230,285 @@ func TestHandlePatchConfig_SavesChannelListSettingsPatch(t *testing.T) {
 	}
 }

+func TestHandlePatchConfig_NormalizesStringChannelArrayFields(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": {
+			"pico": {
+				"type": "pico",
+				"allow_from": " ou_a\u200b，\u2060ou_b\tou_c\u202e，ou_a ",
+				"group_trigger": {
+					"prefixes": "/，!;\n?，/"
+				},
+				"settings": {
+					"allow_origins": "https://a.example.com，http://localhost:5173，https://a.example.com"
+				}
+			},
+			"irc": {
+				"type": "irc",
+				"settings": {
+					"channels": "#ops,\n#dev,\n#ops",
+					"request_caps": "multi-prefix，echo-message\tbatch，multi-prefix"
+				}
+			}
+		}
+	}`))
+	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)
+	}
+
+	picoChannel := cfg.Channels[config.ChannelPico]
+	if len(picoChannel.AllowFrom) != 3 ||
+		picoChannel.AllowFrom[0] != "ou_a" ||
+		picoChannel.AllowFrom[1] != "ou_b" ||
+		picoChannel.AllowFrom[2] != "ou_c" {
+		t.Fatalf("pico allow_from = %#v, want [\"ou_a\", \"ou_b\", \"ou_c\"]", picoChannel.AllowFrom)
+	}
+	if len(picoChannel.GroupTrigger.Prefixes) != 3 ||
+		picoChannel.GroupTrigger.Prefixes[0] != "/" ||
+		picoChannel.GroupTrigger.Prefixes[1] != "!;" ||
+		picoChannel.GroupTrigger.Prefixes[2] != "?" {
+		t.Fatalf(
+			"pico group_trigger.prefixes = %#v, want [\"/\", \"!;\", \"?\"]",
+			picoChannel.GroupTrigger.Prefixes,
+		)
+	}
+
+	decoded, err := picoChannel.GetDecoded()
+	if err != nil {
+		t.Fatalf("GetDecoded() pico error = %v", err)
+	}
+	picoCfg := decoded.(*config.PicoSettings)
+	if len(picoCfg.AllowOrigins) != 2 ||
+		picoCfg.AllowOrigins[0] != "https://a.example.com" ||
+		picoCfg.AllowOrigins[1] != "http://localhost:5173" {
+		t.Fatalf(
+			"pico allow_origins = %#v, want [\"https://a.example.com\", \"http://localhost:5173\"]",
+			picoCfg.AllowOrigins,
+		)
+	}
+
+	ircChannel := cfg.Channels[config.ChannelIRC]
+	decoded, err = ircChannel.GetDecoded()
+	if err != nil {
+		t.Fatalf("GetDecoded() irc error = %v", err)
+	}
+	ircCfg := decoded.(*config.IRCSettings)
+	if len(ircCfg.Channels) != 2 ||
+		ircCfg.Channels[0] != "#ops" ||
+		ircCfg.Channels[1] != "#dev" {
+		t.Fatalf("irc channels = %#v, want [\"#ops\", \"#dev\"]", ircCfg.Channels)
+	}
+	if len(ircCfg.RequestCaps) != 3 ||
+		ircCfg.RequestCaps[0] != "multi-prefix" ||
+		ircCfg.RequestCaps[1] != "echo-message" ||
+		ircCfg.RequestCaps[2] != "batch" {
+		t.Fatalf(
+			"irc request_caps = %#v, want [\"multi-prefix\", \"echo-message\", \"batch\"]",
+			ircCfg.RequestCaps,
+		)
+	}
+}
+
+func TestHandlePatchConfig_NormalizesSingleNumericAllowFrom(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": {
+			"telegram": {
+				"type": "telegram",
+				"allow_from": 123456
+			}
+		}
+	}`))
+	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)
+	}
+	telegramChannel := cfg.Channels[config.ChannelTelegram]
+	if len(telegramChannel.AllowFrom) != 1 || telegramChannel.AllowFrom[0] != "123456" {
+		t.Fatalf("telegram allow_from = %#v, want [\"123456\"]", telegramChannel.AllowFrom)
+	}
+}
+
+func TestHandlePatchConfig_RejectsInvalidChannelArrayFields(t *testing.T) {
+	configPath, cleanup := setupOAuthTestEnv(t)
+	defer cleanup()
+
+	cfg, err := config.LoadConfig(configPath)
+	if err != nil {
+		t.Fatalf("LoadConfig() error = %v", err)
+	}
+	telegramChannel := cfg.Channels[config.ChannelTelegram]
+	telegramChannel.AllowFrom = config.FlexibleStringSlice{"existing-user"}
+	if err := config.SaveConfig(configPath, cfg); err != nil {
+		t.Fatalf("SaveConfig() error = %v", err)
+	}
+
+	tests := []struct {
+		name string
+		body string
+	}{
+		{
+			name: "object allow_from",
+			body: `{
+				"channel_list": {
+					"telegram": {
+						"type": "telegram",
+						"allow_from": {"id": "bad"}
+					}
+				}
+			}`,
+		},
+		{
+			name: "boolean allow_from",
+			body: `{
+				"channel_list": {
+					"telegram": {
+						"type": "telegram",
+						"allow_from": true
+					}
+				}
+			}`,
+		},
+		{
+			name: "object settings array",
+			body: `{
+				"channel_list": {
+					"irc": {
+						"type": "irc",
+						"settings": {
+							"channels": {"name": "#ops"}
+						}
+					}
+				}
+			}`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			h := NewHandler(configPath)
+			mux := http.NewServeMux()
+			h.RegisterRoutes(mux)
+
+			req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(tt.body))
+			req.Header.Set("Content-Type", "application/json")
+
+			rec := httptest.NewRecorder()
+			mux.ServeHTTP(rec, req)
+			if rec.Code != http.StatusBadRequest {
+				t.Fatalf(
+					"PATCH /api/config status = %d, want %d, body=%s",
+					rec.Code,
+					http.StatusBadRequest,
+					rec.Body.String(),
+				)
+			}
+
+			cfg, err := config.LoadConfig(configPath)
+			if err != nil {
+				t.Fatalf("LoadConfig() error = %v", err)
+			}
+			telegramChannel := cfg.Channels[config.ChannelTelegram]
+			if len(telegramChannel.AllowFrom) != 1 || telegramChannel.AllowFrom[0] != "existing-user" {
+				t.Fatalf("telegram allow_from = %#v, want unchanged [\"existing-user\"]", telegramChannel.AllowFrom)
+			}
+		})
+	}
+}
+
+func TestHandlePatchConfig_ClearingAllowFromDoesNotLeaveEmptyStringItem(t *testing.T) {
+	configPath, cleanup := setupOAuthTestEnv(t)
+	defer cleanup()
+
+	cfg, err := config.LoadConfig(configPath)
+	if err != nil {
+		t.Fatalf("LoadConfig() error = %v", err)
+	}
+	feishuChannel := cfg.Channels[config.ChannelFeishu]
+	feishuChannel.Enabled = true
+	feishuChannel.AllowFrom = config.FlexibleStringSlice{"ou_existing_user"}
+	decoded, err := feishuChannel.GetDecoded()
+	if err != nil {
+		t.Fatalf("GetDecoded() error = %v", err)
+	}
+	feishuCfg := decoded.(*config.FeishuSettings)
+	feishuCfg.AppID = "cli_existing_app"
+	feishuCfg.AppSecret = *config.NewSecureString("existing-secret")
+	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": {
+			"feishu": {
+				"enabled": true,
+				"allow_from": "",
+				"settings": {
+					"app_id": "cli_existing_app"
+				}
+			}
+		}
+	}`))
+	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)
+	}
+	feishuChannel = cfg.Channels[config.ChannelFeishu]
+	if len(feishuChannel.AllowFrom) != 0 {
+		t.Fatalf("feishu allow_from = %#v, want empty slice", feishuChannel.AllowFrom)
+	}
+
+	configData, err := os.ReadFile(configPath)
+	if err != nil {
+		t.Fatalf("ReadFile(configPath) error = %v", err)
+	}
+	if strings.Contains(string(configData), `"allow_from": [""]`) {
+		t.Fatalf("config file should not contain empty-string allow_from item: %s", string(configData))
+	}
+}
+
 func TestHandlePatchConfig_CreatesMissingChannelWithTypeAndSecret(t *testing.T) {
 	configPath, cleanup := setupOAuthTestEnv(t)
 	defer cleanup()