feat: warn and continue on config type errors

Author: caidaoli

internal/config/auth_load_workers_test.go

@@ -1,11 +1,141 @@
 package config
 
 import (
+	"io"
 	"os"
 	"path/filepath"
+	"reflect"
+	"strings"
 	"testing"
+
+	log "github.com/sirupsen/logrus"
+	logtest "github.com/sirupsen/logrus/hooks/test"
 )
 
+func TestConfigLoadersWarnAndContinueOnTypeErrors(t *testing.T) {
+	raw := []byte("port: 9001\n" +
+		"oauth-excluded-models:\n" +
+		"  codex:\n" +
+		"    - valid-model\n" +
+		"    - name: invalid-model-secret\n" +
+		"auth-load-workers: worker-secret\n" +
+		"debug: true\n")
+
+	loaders := []struct {
+		name string
+		load func(*testing.T, []byte) (*Config, error)
+	}{
+		{
+			name: "bytes",
+			load: func(_ *testing.T, data []byte) (*Config, error) {
+				return ParseConfigBytes(data)
+			},
+		},
+		{
+			name: "file",
+			load: func(t *testing.T, data []byte) (*Config, error) {
+				configPath := filepath.Join(t.TempDir(), "config.yaml")
+				if errWrite := os.WriteFile(configPath, data, 0o600); errWrite != nil {
+					t.Fatal(errWrite)
+				}
+				return LoadConfig(configPath)
+			},
+		},
+	}
+
+	for _, loader := range loaders {
+		t.Run(loader.name, func(t *testing.T) {
+			hook := captureConfigWarnings(t)
+			cfg, errLoad := loader.load(t, raw)
+			if errLoad != nil {
+				t.Fatalf("load config error = %v", errLoad)
+			}
+			if cfg.Port != 9001 || !cfg.Debug {
+				t.Fatalf("valid config fields were not preserved: port=%d debug=%t", cfg.Port, cfg.Debug)
+			}
+			if cfg.AuthLoadWorkers != DefaultAuthLoadWorkers {
+				t.Fatalf("AuthLoadWorkers = %d, want default %d", cfg.AuthLoadWorkers, DefaultAuthLoadWorkers)
+			}
+			if got := cfg.OAuthExcludedModels["codex"]; !reflect.DeepEqual(got, []string{"valid-model"}) {
+				t.Fatalf("OAuthExcludedModels[codex] = %#v, want valid entry only", got)
+			}
+
+			entries := hook.AllEntries()
+			if len(entries) != 2 {
+				t.Fatalf("warning count = %d, want 2", len(entries))
+			}
+			wantContent := map[int]string{
+				5: "- name: <redacted>",
+				6: "auth-load-workers: <redacted>",
+			}
+			wantDetail := map[int]string{
+				5: "cannot unmarshal !!map into string",
+				6: "cannot unmarshal !!str into int",
+			}
+			for _, entry := range entries {
+				if entry.Level != log.WarnLevel || entry.Message != "ignoring invalid config value" {
+					t.Fatalf("unexpected log entry: level=%s message=%q", entry.Level, entry.Message)
+				}
+				line, okLine := entry.Data["line"].(int)
+				if !okLine {
+					t.Fatalf("warning line = %#v, want int", entry.Data["line"])
+				}
+				content, okContent := entry.Data["content"].(string)
+				if !okContent || content != wantContent[line] {
+					t.Fatalf("warning content at line %d = %#v, want %q", line, entry.Data["content"], wantContent[line])
+				}
+				detail, okDetail := entry.Data["error"].(string)
+				if !okDetail || detail != wantDetail[line] {
+					t.Fatalf("warning error at line %d = %#v, want %q", line, entry.Data["error"], wantDetail[line])
+				}
+				for _, secret := range []string{"invalid-model-secret", "worker-secret", "worker-"} {
+					if strings.Contains(content, secret) || strings.Contains(detail, secret) {
+						t.Fatalf("warning leaked %q: content=%q error=%q", secret, content, detail)
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestConfigLoadersRejectNonTypeMismatchYAMLErrors(t *testing.T) {
+	tests := []struct {
+		name string
+		raw  []byte
+	}{
+		{name: "malformed collection", raw: []byte("auth-load-workers: [\n")},
+		{name: "duplicate key", raw: []byte("port: 9001\nport: 9002\n")},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if _, errParse := ParseConfigBytes(test.raw); errParse == nil {
+				t.Fatal("ParseConfigBytes() error = nil")
+			}
+			configPath := filepath.Join(t.TempDir(), "config.yaml")
+			if errWrite := os.WriteFile(configPath, test.raw, 0o600); errWrite != nil {
+				t.Fatal(errWrite)
+			}
+			if _, errLoad := LoadConfig(configPath); errLoad == nil {
+				t.Fatal("LoadConfig() error = nil")
+			}
+		})
+	}
+}
+
+func captureConfigWarnings(t *testing.T) *logtest.Hook {
+	t.Helper()
+	logger := log.StandardLogger()
+	previousHooks := logger.ReplaceHooks(make(log.LevelHooks))
+	previousOutput := logger.Out
+	logger.SetOutput(io.Discard)
+	hook := logtest.NewLocal(logger)
+	t.Cleanup(func() {
+		logger.ReplaceHooks(previousHooks)
+		logger.SetOutput(previousOutput)
+	})
+	return hook
+}
+
 func TestParseConfigBytesAuthLoadWorkers(t *testing.T) {
 	tests := []struct {
 		name string

internal/config/config_decode.go

@@ -0,0 +1,144 @@
+package config
+
+import (
+	"bytes"
+	"errors"
+	"strconv"
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+	"gopkg.in/yaml.v3"
+)
+
+const maxConfigWarningContentRunes = 256
+
+type configYAMLWarning struct {
+	line   int
+	detail string
+}
+
+// unmarshalConfigYAML keeps yaml.v3's partially decoded result for type mismatches.
+// Syntax and structural errors remain fatal because they do not produce a trustworthy document.
+func unmarshalConfigYAML(data []byte, cfg *Config) error {
+	errUnmarshal := yaml.Unmarshal(data, cfg)
+	if errUnmarshal == nil {
+		return nil
+	}
+
+	var typeError *yaml.TypeError
+	if !errors.As(errUnmarshal, &typeError) {
+		return errUnmarshal
+	}
+
+	warnings := make([]configYAMLWarning, 0, len(typeError.Errors))
+	for _, issue := range typeError.Errors {
+		warning, okMismatch := configTypeMismatchWarning(issue)
+		if !okMismatch {
+			return errUnmarshal
+		}
+		warnings = append(warnings, warning)
+	}
+	for _, warning := range warnings {
+		log.WithFields(log.Fields{
+			"content": redactedConfigSourceLine(data, warning.line),
+			"error":   warning.detail,
+			"line":    warning.line,
+		}).Warn("ignoring invalid config value")
+	}
+	return nil
+}
+
+func configTypeMismatchWarning(issue string) (configYAMLWarning, bool) {
+	line := 0
+	detail := strings.TrimSpace(issue)
+	if remainder, okPrefix := strings.CutPrefix(detail, "line "); okPrefix {
+		if lineText, message, okLine := strings.Cut(remainder, ": "); okLine {
+			if parsedLine, errLine := strconv.Atoi(lineText); errLine == nil {
+				line = parsedLine
+				detail = message
+			}
+		}
+	}
+	const prefix = "cannot unmarshal "
+	if !strings.HasPrefix(detail, prefix) {
+		return configYAMLWarning{}, false
+	}
+	sourceAndValue, target, okTarget := strings.Cut(strings.TrimPrefix(detail, prefix), " into ")
+	if !okTarget {
+		return configYAMLWarning{}, false
+	}
+	sourceType := sourceAndValue
+	if valueAt := strings.IndexByte(sourceType, ' '); valueAt >= 0 {
+		sourceType = sourceType[:valueAt]
+	}
+	return configYAMLWarning{
+		line:   line,
+		detail: prefix + sourceType + " into " + target,
+	}, true
+}
+
+func redactedConfigSourceLine(data []byte, line int) string {
+	lines := bytes.Split(data, []byte("\n"))
+	if line < 1 || line > len(lines) {
+		return "<unavailable>"
+	}
+	raw := strings.TrimSpace(strings.TrimSuffix(string(lines[line-1]), "\r"))
+	if raw == "" {
+		return "<empty>"
+	}
+
+	var document yaml.Node
+	if errParse := yaml.Unmarshal([]byte(raw), &document); errParse != nil {
+		return "<redacted>"
+	}
+	redactConfigLogNode(&document, false)
+	rendered, errMarshal := yaml.Marshal(&document)
+	if errMarshal != nil {
+		return "<redacted>"
+	}
+	content := strings.Join(strings.Fields(string(rendered)), " ")
+	if content == "" {
+		return "<empty>"
+	}
+	runes := []rune(content)
+	if len(runes) > maxConfigWarningContentRunes {
+		content = string(runes[:maxConfigWarningContentRunes]) + "..."
+	}
+	return content
+}
+
+func redactConfigLogNode(node *yaml.Node, mappingKey bool) {
+	if node == nil {
+		return
+	}
+	node.Anchor = ""
+	node.HeadComment = ""
+	node.LineComment = ""
+	node.FootComment = ""
+
+	switch node.Kind {
+	case yaml.DocumentNode:
+		for _, child := range node.Content {
+			redactConfigLogNode(child, false)
+		}
+	case yaml.MappingNode:
+		for index := 0; index+1 < len(node.Content); index += 2 {
+			redactConfigLogNode(node.Content[index], true)
+			redactConfigLogNode(node.Content[index+1], false)
+		}
+	case yaml.SequenceNode:
+		for _, child := range node.Content {
+			redactConfigLogNode(child, false)
+		}
+	case yaml.AliasNode:
+		node.Value = "<redacted>"
+		node.Alias = nil
+	case yaml.ScalarNode:
+		if mappingKey || node.ShortTag() == "!!null" {
+			return
+		}
+		node.Tag = "!!str"
+		node.Value = "<redacted>"
+		node.Style = 0
+	}
+}

internal/config/config_load.go

@@ -9,7 +9,6 @@ import (
 	"syscall"
 
 	log "github.com/sirupsen/logrus"
-	"gopkg.in/yaml.v3"
 )
 
 // LoadConfig reads a YAML configuration file from the given path,
@@ -28,7 +27,8 @@ func LoadConfig(configFile string) (*Config, error) {
 
 // LoadConfigOptional reads YAML from configFile.
 // If optional is true and the file is missing, it returns an empty Config.
-// If optional is true and the file is empty or invalid, it returns an empty Config.
+// If optional is true and the file is empty or structurally invalid, it returns an empty Config.
+// Field type mismatches are always logged and skipped while valid values are retained.
 func LoadConfigOptional(configFile string, optional bool) (*Config, error) {
 	// Read the entire configuration file into memory.
 	data, err := os.ReadFile(configFile)
@@ -81,9 +81,9 @@ func LoadConfigOptional(configFile string, optional bool) (*Config, error) {
 	cfg.Pprof.Addr = DefaultPprofAddr
 	cfg.AuthLoadWorkers = DefaultAuthLoadWorkers
 	cfg.CredentialInFlight = DefaultCredentialInFlightConfig()
-	if err = yaml.Unmarshal(data, &cfg); err != nil {
+	if err = unmarshalConfigYAML(data, &cfg); err != nil {
 		if optional {
-			// In cloud deploy mode, if YAML parsing fails, return empty config instead of error.
+			// In cloud deploy mode, if YAML syntax or structure parsing fails, return empty config instead of error.
 			cfgOptional := &Config{CredentialInFlight: DefaultCredentialInFlightConfig()}
 			cfgOptional.normalizeAuthLoadWorkers()
 			cfgOptional.NormalizePluginsConfig()

internal/config/parse.go

@@ -6,7 +6,6 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/crypto/bcrypt"
-	"gopkg.in/yaml.v3"
 )
 
 // ParseConfigBytes parses a YAML configuration payload into Config and applies the same
@@ -38,7 +37,7 @@ func ParseConfigBytes(data []byte) (*Config, error) {
 	cfg.AuthLoadWorkers = DefaultAuthLoadWorkers
 	cfg.CredentialInFlight = DefaultCredentialInFlightConfig()
 
-	if err := yaml.Unmarshal(data, &cfg); err != nil {
+	if err := unmarshalConfigYAML(data, &cfg); err != nil {
 		return nil, fmt.Errorf("parse config payload: %w", err)
 	}