From b91f967b6616ac4625af9010fcd6cdaaee6ad49e Mon Sep 17 00:00:00 2001 From: "Jakub A. W" Date: Wed, 8 Jul 2026 16:34:55 +0200 Subject: [PATCH 1/4] fix(config): reject unknown config keys and log provider origins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A misindented `providers:` section parses as a null section plus unknown top-level keys. yaml.Unmarshal discarded those silently, so the gateway booted with none of the operator's providers and no error — the fault only surfaced once an env-provisioned provider was removed and startup failed with "no providers were successfully registered". Parse the YAML layer with KnownFields(true) so an unknown key fails startup, naming the file and every offending line. This also catches ordinary typos (`prot:` for `port:`, `bass_url:` for `base_url:`). The env layer declares the same structures as JSON and overrides YAML entry by entry, so a typo there would silently win over a correct YAML entry. Decode VIRTUAL_MODELS, SET_RATE_LIMIT_*, and SET_BUDGET_* with DisallowUnknownFields for the same reason. Two fixes fall out of that: - BudgetLimitConfig had no json tags, unlike its RateLimitRuleConfig neighbour which added them for exactly this env form, so `period_seconds` in a SET_BUDGET_* JSON array was silently dropped and produced a limit with no window. Add the tags. - `failover.overrides` is a removed key that old config files still carry and that strict parsing would now reject. Model it as an explicitly ignored field, upgrading it from silently ignored to a logged deprecation pointing at `disabled_models`. A config path that exists but cannot be read — a directory bind-mounted where a file was expected — was indistinguishable from a missing file and fell back to defaults. Report it instead. Finally, log one line at boot showing how many providers came from the config file versus environment discovery, with their names, so an operator can see at a glance that a config file contributed nothing. Closes #509 Co-Authored-By: Claude Opus 4.8 (1M context) --- config/budget.go | 9 +- config/budget_test.go | 34 +++++++ config/config.go | 92 ++++++++++++----- config/config_strict_test.go | 161 ++++++++++++++++++++++++++++++ config/failover.go | 11 ++ config/merge.go | 15 ++- config/ratelimit.go | 3 +- config/ratelimit_test.go | 10 ++ config/virtualmodels.go | 3 +- config/virtualmodels_test.go | 20 +++- docs/advanced/config-yaml.mdx | 51 ++++++++++ internal/providers/config.go | 18 ++++ internal/providers/config_test.go | 35 +++++++ internal/providers/init.go | 7 ++ 14 files changed, 435 insertions(+), 34 deletions(-) create mode 100644 config/budget_test.go create mode 100644 config/config_strict_test.go diff --git a/config/budget.go b/config/budget.go index aa93f298..7e5d1e6e 100644 --- a/config/budget.go +++ b/config/budget.go @@ -30,17 +30,18 @@ type BudgetUserPathConfig struct { } // BudgetLimitConfig declares one spend limit for a reset period. +// The json tags support the JSON-array form of SET_BUDGET_* env values. type BudgetLimitConfig struct { // Period accepts hourly, daily, weekly, or monthly. The resolved period is // persisted as PeriodSeconds in the database. - Period string `yaml:"period"` + Period string `yaml:"period" json:"period"` // PeriodSeconds can be set directly instead of Period. Standard values are // 3600, 86400, 604800, and 2592000. - PeriodSeconds int64 `yaml:"period_seconds"` + PeriodSeconds int64 `yaml:"period_seconds" json:"period_seconds"` // Amount is the maximum allowed tracked provider spend for the period. - Amount float64 `yaml:"amount"` + Amount float64 `yaml:"amount" json:"amount"` } func applyBudgetEnv(cfg *Config) error { @@ -89,7 +90,7 @@ func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) { } if strings.HasPrefix(raw, "[") { var limits []BudgetLimitConfig - if err := json.Unmarshal([]byte(raw), &limits); err != nil { + if err := decodeStrictJSON(raw, &limits); err != nil { return nil, err } return limits, nil diff --git a/config/budget_test.go b/config/budget_test.go new file mode 100644 index 00000000..76aad251 --- /dev/null +++ b/config/budget_test.go @@ -0,0 +1,34 @@ +package config + +import ( + "strings" + "testing" +) + +// The JSON-array form of SET_BUDGET_* decodes through json tags. Without them +// period_seconds was silently dropped, yielding a limit with no window. +func TestParseBudgetEnvLimits_JSONArray(t *testing.T) { + limits, err := parseBudgetEnvLimits(`[{"period_seconds":7200,"amount":5}]`) + if err != nil { + t.Fatalf("parseBudgetEnvLimits() error = %v", err) + } + if len(limits) != 1 { + t.Fatalf("len(limits) = %d, want 1", len(limits)) + } + if limits[0].PeriodSeconds != 7200 { + t.Fatalf("PeriodSeconds = %d, want 7200", limits[0].PeriodSeconds) + } + if limits[0].Amount != 5 { + t.Fatalf("Amount = %v, want 5", limits[0].Amount) + } +} + +func TestParseBudgetEnvLimits_RejectsUnknownField(t *testing.T) { + _, err := parseBudgetEnvLimits(`[{"period":"daily","ammount":5}]`) + if err == nil { + t.Fatal("parseBudgetEnvLimits() error = nil, want unknown-field error") + } + if !strings.Contains(err.Error(), "ammount") { + t.Fatalf("parseBudgetEnvLimits() error = %q, want it to name the unknown field", err) + } +} diff --git a/config/config.go b/config/config.go index 8cc63b14..eefb0482 100644 --- a/config/config.go +++ b/config/config.go @@ -2,8 +2,14 @@ package config import ( + "errors" "fmt" + "io" + "io/fs" + "log/slog" "os" + "regexp" + "strings" "time" "gopkg.in/yaml.v3" @@ -225,32 +231,30 @@ func Load() (*LoadResult, error) { }, nil } -// applyYAML reads an optional config.yaml and overlays it onto cfg. +// configFilePaths are searched in order; the first readable file wins. +var configFilePaths = []string{ + "config/config.yaml", + "config.yaml", +} + +// applyYAML reads an optional config file and overlays it onto cfg. // Returns the raw provider map parsed from the providers: YAML section. // If no config file is found, this is a no-op (not an error). +// +// Parsing is strict: an unknown key is an error rather than a silently ignored +// one. A misindented section — the classic `providers:` followed by entries at +// column zero — otherwise parses as a null section plus unknown top-level keys, +// and the gateway boots with none of the operator's providers. func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { - paths := []string{ - "config/config.yaml", - "config.yaml", - } - - var data []byte - for _, p := range paths { - raw, err := os.ReadFile(p) - if err == nil { - data = raw - break - } + path, data, err := readConfigFile() + if err != nil { + return nil, err } - - rawProviders := make(map[string]RawProviderConfig) - if data == nil { - return rawProviders, nil + slog.Info("no config file found; using defaults and environment", "searched", configFilePaths) + return map[string]RawProviderConfig{}, nil } - expanded := expandString(string(data)) - // yamlTarget is a local struct that mirrors Config for YAML unmarshaling, // using RawProviderConfig for providers so nullable resilience overrides are preserved. type yamlTarget struct { @@ -259,13 +263,53 @@ func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { } target := yamlTarget{Config: cfg} - if err := yaml.Unmarshal([]byte(expanded), &target); err != nil { - return nil, fmt.Errorf("failed to parse config.yaml: %w", err) + decoder := yaml.NewDecoder(strings.NewReader(expandString(string(data)))) + decoder.KnownFields(true) + // A file holding only comments decodes to nothing; that is an empty overlay, + // not a failure. + if err := decoder.Decode(&target); err != nil && !errors.Is(err, io.EOF) { + return nil, formatYAMLError(path, err) } - if target.RawProviders != nil { - rawProviders = target.RawProviders + slog.Info("config file loaded", "path", path, "providers", len(target.RawProviders)) + + if target.RawProviders == nil { + return map[string]RawProviderConfig{}, nil } + return target.RawProviders, nil +} + +// readConfigFile returns the first config file that exists and its contents, or an +// empty path and nil contents when none does. A file that exists but cannot be read +// — wrong permissions, or a directory mounted where a file was expected — is an +// error, not a missing file: silently falling back to defaults is how a +// misconfigured deployment boots with no providers. +func readConfigFile() (string, []byte, error) { + for _, path := range configFilePaths { + data, err := os.ReadFile(path) + switch { + case err == nil: + return path, data, nil + case errors.Is(err, fs.ErrNotExist): + continue + default: + return "", nil, fmt.Errorf("failed to read %s: %w", path, err) + } + } + return "", nil, nil +} + +// yamlTypeSuffix matches the Go type name yaml.v3 appends to unknown-field errors +// ("field foo not found in type config.yamlTarget"). It names an internal struct +// the operator cannot act on, so it is stripped. +var yamlTypeSuffix = regexp.MustCompile(` in type \S+`) - return rawProviders, nil +// formatYAMLError rewrites a yaml.v3 decode error into a single actionable line +// prefixed with the offending file. +func formatYAMLError(path string, err error) error { + msg := yamlTypeSuffix.ReplaceAllString(err.Error(), "") + msg = strings.TrimPrefix(msg, "yaml: unmarshal errors:\n") + msg = strings.TrimPrefix(msg, "yaml: ") + msg = strings.ReplaceAll(msg, "\n ", "; ") + return fmt.Errorf("failed to parse %s: %s", path, strings.TrimSpace(msg)) } diff --git a/config/config_strict_test.go b/config/config_strict_test.go new file mode 100644 index 00000000..5f021102 --- /dev/null +++ b/config/config_strict_test.go @@ -0,0 +1,161 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestLoad_RejectsUnknownYAMLFields locks in strict parsing: an unknown key is a +// startup error, never a silently dropped section. The misindented providers: +// block is the motivating case — it parses as a null section plus unknown +// top-level keys, so the gateway would otherwise boot with no providers at all. +func TestLoad_RejectsUnknownYAMLFields(t *testing.T) { + tests := []struct { + name string + yaml string + wantErr string + }{ + { + name: "misindented providers section", + yaml: ` +server: + port: "9999" + +providers: +uranium-geryon-9b: + type: vllm + base_url: "http://uranium-geryon-9b:8000/v1" +`, + wantErr: `field uranium-geryon-9b not found`, + }, + { + name: "unknown top-level key", + yaml: "bogus_section:\n a: 1\n", + // Reported against the file, without yaml.v3's internal Go type name. + wantErr: "failed to parse config.yaml: line 1: field bogus_section not found", + }, + { + name: "unknown nested key", + yaml: "server:\n prot: \"9999\"\n", + wantErr: "field prot not found", + }, + { + name: "unknown provider key", + yaml: "providers:\n local:\n type: vllm\n bass_url: \"http://x:8000/v1\"\n", + wantErr: "field bass_url not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearAllConfigEnvVars(t) + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, tt.yaml) + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want unknown-field error") + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Load() error = %q, want it to contain %q", err, tt.wantErr) + } + if strings.Contains(err.Error(), "in type") { + t.Fatalf("Load() error leaks an internal Go type name: %q", err) + } + }) + }) + } +} + +// TestLoad_AcceptsValidYAMLShapes guards the edges strict parsing must not break: +// an empty or comments-only file is an empty overlay, a correctly indented +// providers: block still loads, and the removed failover.overrides key is still +// tolerated so an old config file keeps booting. +func TestLoad_AcceptsValidYAMLShapes(t *testing.T) { + tests := []struct { + name string + yaml string + wantProviders int + }{ + {name: "empty file", yaml: ""}, + {name: "comments only", yaml: "# nothing to see here\n"}, + {name: "explicit null providers", yaml: "providers:\n"}, + {name: "document end marker", yaml: "server:\n port: \"9999\"\n...\n"}, + { + name: "correctly indented providers", + yaml: "providers:\n local:\n type: vllm\n base_url: \"http://x:8000/v1\"\n", + wantProviders: 1, + }, + { + name: "legacy failover overrides are tolerated", + yaml: "failover:\n overrides:\n \"gpt-4o\":\n mode: \"off\"\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearAllConfigEnvVars(t) + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, tt.yaml) + + result, err := Load() + if err != nil { + t.Fatalf("Load() failed: %v", err) + } + if got := len(result.RawProviders); got != tt.wantProviders { + t.Fatalf("len(RawProviders) = %d, want %d", got, tt.wantProviders) + } + }) + }) + } +} + +// TestApplyYAML_ExampleConfigParses keeps the shipped example honest: every key it +// documents must exist on Config, or operators who copy it cannot boot under strict +// parsing. It exercises the decode only — Load additionally resolves paths the +// example points at (failover rules), which do not exist relative to a temp dir. +func TestApplyYAML_ExampleConfigParses(t *testing.T) { + clearAllConfigEnvVars(t) + + example, err := os.ReadFile("config.example.yaml") + if err != nil { + t.Fatalf("Failed to read config.example.yaml: %v", err) + } + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, string(example)) + + if _, err := applyYAML(buildDefaultConfig()); err != nil { + t.Fatalf("config.example.yaml does not parse: %v", err) + } + }) +} + +// A path that exists but cannot be read — most often a directory bind-mounted where +// a file was expected — must not be mistaken for a missing config file. +func TestApplyYAML_UnreadableConfigFileIsAnError(t *testing.T) { + clearAllConfigEnvVars(t) + + withTempDir(t, func(dir string) { + if err := os.Mkdir(filepath.Join(dir, "config.yaml"), 0755); err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + + _, err := applyYAML(buildDefaultConfig()) + if err == nil { + t.Fatal("applyYAML() succeeded, want read error") + } + if !strings.Contains(err.Error(), "failed to read config.yaml") { + t.Fatalf("applyYAML() error = %q, want a read error naming the file", err) + } + }) +} + +func writeConfigYAML(t *testing.T, dir, contents string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(contents), 0644); err != nil { + t.Fatalf("Failed to write config.yaml: %v", err) + } +} diff --git a/config/failover.go b/config/failover.go index 8771f878..d1e6ff29 100644 --- a/config/failover.go +++ b/config/failover.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "os" "strings" ) @@ -68,6 +69,11 @@ type FailoverConfig struct { // env. It accepts either a JSON string array or object with boolean values. DisabledModelsJSON string `yaml:"disabled_models_json" env:"FAILOVER_DISABLED_MODELS_JSON"` + // Overrides is a removed compatibility field. Per-model failover modes are gone; + // operators migrate to DisabledModels. It is still parsed — and ignored, with a + // warning — so an old config file keeps booting under strict YAML validation. + Overrides map[string]any `yaml:"overrides"` + // Manual holds the parsed manual failover lists loaded from ManualRulesPath. Manual map[string][]string `yaml:"-"` @@ -80,6 +86,11 @@ func loadFailoverConfig(cfg *FailoverConfig) error { return nil } + if len(cfg.Overrides) > 0 { + slog.Warn("failover.overrides was removed and is ignored; use failover.disabled_models instead") + cfg.Overrides = nil + } + cfg.DefaultMode = ResolveFailoverDefaultMode(cfg.DefaultMode) manual := make(map[string][]string) diff --git a/config/merge.go b/config/merge.go index a1a75c4f..74e9d3cf 100644 --- a/config/merge.go +++ b/config/merge.go @@ -1,6 +1,19 @@ package config -import "strings" +import ( + "encoding/json" + "strings" +) + +// decodeStrictJSON decodes an infrastructure-as-code env var into target, rejecting +// unknown keys. The env layer declares the same structures as the YAML layer and +// overrides it entry by entry, so a typo must fail loudly here too — a silently +// ignored key would let a malformed env entry win over a correct YAML one. +func decodeStrictJSON(raw string, target any) error { + decoder := json.NewDecoder(strings.NewReader(raw)) + decoder.DisallowUnknownFields() + return decoder.Decode(target) +} // mergeByKey overlays override entries onto base, replacing matching base // entries in place and appending new entries. diff --git a/config/ratelimit.go b/config/ratelimit.go index 9a8472cc..202987a8 100644 --- a/config/ratelimit.go +++ b/config/ratelimit.go @@ -1,7 +1,6 @@ package config import ( - "encoding/json" "fmt" "strconv" "strings" @@ -136,7 +135,7 @@ func parseRateLimitEnvLimits(raw string) ([]RateLimitRuleConfig, error) { } if strings.HasPrefix(raw, "[") { var limits []RateLimitRuleConfig - if err := json.Unmarshal([]byte(raw), &limits); err != nil { + if err := decodeStrictJSON(raw, &limits); err != nil { return nil, err } return limits, nil diff --git a/config/ratelimit_test.go b/config/ratelimit_test.go index d3dde4c9..d7461034 100644 --- a/config/ratelimit_test.go +++ b/config/ratelimit_test.go @@ -406,3 +406,13 @@ func TestRateLimitsEnabledByDefaultAndTogglable(t *testing.T) { } }) } + +func TestParseRateLimitEnvLimits_RejectsUnknownField(t *testing.T) { + _, err := parseRateLimitEnvLimits(`[{"period":"minute","max_requsts":100}]`) + if err == nil { + t.Fatal("parseRateLimitEnvLimits() error = nil, want unknown-field error") + } + if !strings.Contains(err.Error(), "max_requsts") { + t.Fatalf("parseRateLimitEnvLimits() error = %q, want it to name the unknown field", err) + } +} diff --git a/config/virtualmodels.go b/config/virtualmodels.go index 35ae9dad..65cff8b7 100644 --- a/config/virtualmodels.go +++ b/config/virtualmodels.go @@ -1,7 +1,6 @@ package config import ( - "encoding/json" "fmt" "os" "strings" @@ -59,7 +58,7 @@ func applyVirtualModelsEnv(cfg *Config) error { return nil } var fromEnv []VirtualModelConfig - if err := json.Unmarshal([]byte(raw), &fromEnv); err != nil { + if err := decodeStrictJSON(raw, &fromEnv); err != nil { return fmt.Errorf("invalid %s: %w", envVirtualModels, err) } cfg.VirtualModels = mergeByKey(cfg.VirtualModels, fromEnv, func(model VirtualModelConfig) string { diff --git a/config/virtualmodels_test.go b/config/virtualmodels_test.go index 94da92ed..442ea093 100644 --- a/config/virtualmodels_test.go +++ b/config/virtualmodels_test.go @@ -1,6 +1,9 @@ package config -import "testing" +import ( + "strings" + "testing" +) func TestApplyVirtualModelsEnv_ParsesAndMerges(t *testing.T) { cfg := &Config{VirtualModels: []VirtualModelConfig{ @@ -37,6 +40,21 @@ func TestApplyVirtualModelsEnv_Invalid(t *testing.T) { } } +// The env layer overrides YAML entry by entry, so a typo must fail loudly rather +// than let a malformed env entry silently win over a correct YAML one. +func TestApplyVirtualModelsEnv_RejectsUnknownField(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, `[{"source":"smart","targts":[{"model":"openai/gpt-4o"}]}]`) + + err := applyVirtualModelsEnv(cfg) + if err == nil { + t.Fatal("applyVirtualModelsEnv() error = nil, want unknown-field error") + } + if !strings.Contains(err.Error(), "targts") { + t.Fatalf("applyVirtualModelsEnv() error = %q, want it to name the unknown field", err) + } +} + func TestApplyVirtualModelsEnv_Unset(t *testing.T) { cfg := &Config{VirtualModels: []VirtualModelConfig{{Source: "smart", Target: "openai/gpt-4o"}}} t.Setenv(envVirtualModels, "") diff --git a/docs/advanced/config-yaml.mdx b/docs/advanced/config-yaml.mdx index 4384d617..3cabb3a9 100644 --- a/docs/advanced/config-yaml.mdx +++ b/docs/advanced/config-yaml.mdx @@ -50,6 +50,32 @@ Effective precedence is: `.env` is not a separate priority layer. It is just a convenient way to load environment variables before startup. +Each layer is applied on top of the previous one, so a provider declared in +`config.yaml` and overlaid by env vars still counts as coming from the file. + +## Startup Validation + +Declarative config is parsed strictly: an unknown key is a startup error, not a +silently ignored one. This catches typos (`prot:` instead of `port:`) and, more +importantly, misindented sections — see the gotcha below. + +This applies to `config.yaml` and to the env vars that declare the same +structures as JSON: `VIRTUAL_MODELS`, `SET_RATE_LIMIT_*`, and `SET_BUDGET_*`. +Because env entries override YAML entries, a typo in one of them would otherwise +silently win over a correct YAML entry. + +To confirm which layer supplied your providers, GoModel logs one line at boot: + +```json +{"msg":"providers resolved","total":5,"from_config_file":4,"from_env":1, + "config_file_providers":["plutonium-qwen-dflash","uranium-gemma-embedding"], + "env_providers":["opencode-go"]} +``` + +`from_config_file: 0` while a file is loaded means the file contributed no +providers. GoModel also logs `config file loaded` with the resolved path, or +`no config file found` with the paths it searched. + ## Current Schema The current source of truth lives in the main codebase: @@ -94,6 +120,10 @@ services: wins. +If the mounted path exists but cannot be read — most often a directory +bind-mounted where a file was expected — startup fails rather than silently +falling back to defaults. + ## Gotchas ### Unresolved `${VAR}` placeholders drop the provider @@ -105,6 +135,27 @@ applies if `${VAR}` appears in the middle of the value, such as `prefix-${OPENAI_API_KEY}`. Always verify your env vars are exported before starting the process, or supply a default: `${OPENAI_API_KEY:-}`. +### A misindented `providers:` section is rejected + +YAML reads this as an empty `providers:` section plus three unrelated top-level +keys, not as three providers: + +```yaml +providers: +uranium-geryon-9b: # not indented under providers: + type: vllm + base_url: "http://uranium-geryon-9b:8000/v1" +``` + +Strict parsing rejects it at startup: + +```text +failed to parse config.yaml: line 2: field uranium-geryon-9b not found +``` + +Indent provider entries two spaces under `providers:`. Before strict parsing, +this booted with zero YAML providers and no error. + ### Per-provider resilience can only come from YAML The env-var override walk skips `map` fields. `RETRY_MAX_RETRIES` changes the diff --git a/internal/providers/config.go b/internal/providers/config.go index 4df6f4b5..df3ed77a 100644 --- a/internal/providers/config.go +++ b/internal/providers/config.go @@ -530,6 +530,24 @@ func isUnresolvedEnvPlaceholder(value string) bool { return inner != "" && !strings.ContainsAny(inner, "{}") } +// providerOrigins splits the resolved provider names by where they were declared: +// the config file, or environment-variable discovery. A provider named in the +// config file counts as fromFile even when env vars overlay its fields. Operators +// need the split to notice a config file that contributed nothing — a misindented +// providers: section reads as zero fromFile providers. +func providerOrigins(declared map[string]config.RawProviderConfig, resolved map[string]ProviderConfig) (fromFile, fromEnv []string) { + for name := range resolved { + if _, ok := declared[name]; ok { + fromFile = append(fromFile, name) + } else { + fromEnv = append(fromEnv, name) + } + } + sort.Strings(fromFile) + sort.Strings(fromEnv) + return fromFile, fromEnv +} + // skippedProviderNames lists the YAML-declared providers that did not survive // credential resolution, so operators can see why a configured provider is // absent instead of it disappearing silently. diff --git a/internal/providers/config_test.go b/internal/providers/config_test.go index 459fbb7f..6f12c7a1 100644 --- a/internal/providers/config_test.go +++ b/internal/providers/config_test.go @@ -1,6 +1,7 @@ package providers import ( + "slices" "testing" "time" @@ -361,6 +362,40 @@ func TestSkippedProviderNames_ListsDeclaredButUnresolved(t *testing.T) { } } +func TestProviderOrigins_SplitsConfigFileFromEnv(t *testing.T) { + // openai is declared in the config file and overlaid by env vars; it still + // counts as coming from the file. groq exists only because of env discovery. + declared := map[string]config.RawProviderConfig{ + "openai": {Type: "openai"}, + "vllm-b": {Type: "vllm", BaseURL: "http://b:8000/v1"}, + } + resolved := map[string]ProviderConfig{ + "openai": {Type: "openai"}, + "vllm-b": {Type: "vllm"}, + "groq": {Type: "groq"}, + } + + fromFile, fromEnv := providerOrigins(declared, resolved) + if want := []string{"openai", "vllm-b"}; !slices.Equal(fromFile, want) { + t.Fatalf("fromFile = %v, want %v", fromFile, want) + } + if want := []string{"groq"}; !slices.Equal(fromEnv, want) { + t.Fatalf("fromEnv = %v, want %v", fromEnv, want) + } +} + +// A misindented providers: section yields no config-file providers, which is the +// signal an operator needs to see at boot. +func TestProviderOrigins_NoDeclaredProviders(t *testing.T) { + fromFile, fromEnv := providerOrigins(nil, map[string]ProviderConfig{"openai": {Type: "openai"}}) + if len(fromFile) != 0 { + t.Fatalf("fromFile = %v, want empty", fromFile) + } + if want := []string{"openai"}; !slices.Equal(fromEnv, want) { + t.Fatalf("fromEnv = %v, want %v", fromEnv, want) + } +} + func TestFilterEmptyProviders_EmptyMap(t *testing.T) { got := filterEmptyProviders(map[string]config.RawProviderConfig{}, testDiscoveryConfigs) if len(got) != 0 { diff --git a/internal/providers/init.go b/internal/providers/init.go index 77ede269..3cc668cb 100644 --- a/internal/providers/init.go +++ b/internal/providers/init.go @@ -81,6 +81,13 @@ func Init(ctx context.Context, result *config.LoadResult, factory *ProviderFacto } providerMap, credentialResolved := resolveProviders(result.RawProviders, result.Config.Resilience, factory.discoveryConfigsSnapshot()) + fromFile, fromEnv := providerOrigins(result.RawProviders, providerMap) + slog.Info("providers resolved", + "total", len(providerMap), + "from_config_file", len(fromFile), + "from_env", len(fromEnv), + "config_file_providers", fromFile, + "env_providers", fromEnv) if skipped := skippedProviderNames(result.RawProviders, credentialResolved); len(skipped) > 0 { slog.Info("configured providers skipped: credentials or base_url did not resolve", "providers", skipped) From 1f0b33eaefa711c9a125a7e9b569488070b5c207 Mon Sep 17 00:00:00 2001 From: "Jakub A. W" Date: Wed, 8 Jul 2026 16:53:46 +0200 Subject: [PATCH 2/4] feat(config): add CONFIG_STRICT escape hatch for unknown keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strict parsing is the right default — a dropped providers, rate_limits, budgets, or guardrails entry silently changes routing, cost, or security — but it blocks one legitimate case: rolling a binary back under a config file written for a newer version, where an unknown key is expected rather than a typo. CONFIG_STRICT=false downgrades unknown keys to warnings that name the file, line, and field, then boots without them. The default stays true. Unknown keys are always detected: lax mode still decodes with KnownFields(true) and inspects the resulting TypeError, so it can report each ignored key instead of dropping it in silence the way yaml.Unmarshal did. The flag relaxes which keys are accepted, never whether a value makes sense. A malformed value (`port: [9999, 8080]`) and a YAML syntax error stay fatal in both modes, and a file mixing an unknown key with a malformed value still fails. The same rule applies to the JSON env layer, where only encoding/json's unknown-field error is downgraded. Entity-level validation is unaffected: a virtual model whose target names a provider that does not exist still fails startup even when the key that would have declared that provider was ignored with a warning. Co-Authored-By: Claude Opus 4.8 (1M context) --- .env.template | 8 +++ config/budget.go | 8 +-- config/budget_test.go | 4 +- config/config.go | 98 +++++++++++++++++++++++++++++++---- config/config_strict_test.go | 89 ++++++++++++++++++++++++++++++- config/config_test.go | 1 + config/merge.go | 27 ++++++++-- config/ratelimit.go | 13 +++-- config/ratelimit_test.go | 2 +- config/virtualmodels.go | 4 +- config/virtualmodels_test.go | 32 ++++++++++-- docs/advanced/config-yaml.mdx | 22 +++++++- 12 files changed, 273 insertions(+), 35 deletions(-) diff --git a/.env.template b/.env.template index 2c3c5b5d..68dd2048 100644 --- a/.env.template +++ b/.env.template @@ -5,6 +5,14 @@ # Header used to read/write request user_path values (default: X-GoModel-User-Path) # USER_PATH_HEADER=X-GoModel-User-Path +# Reject unknown keys in config.yaml and in the JSON env vars that declare the same +# structures (VIRTUAL_MODELS, SET_RATE_LIMIT_*, SET_BUDGET_*). Default: true, so a +# typo or a misindented section fails startup instead of silently dropping providers, +# rate limits, budgets, or guardrails. Set false to downgrade unknown keys to +# warnings — intended for rolling a binary back under a newer config file. Malformed +# values stay fatal in either mode. +# CONFIG_STRICT=true + # Tagging based on headers: label every request from the listed headers (numbered # from 1). Labels are recorded in usage tracking and audit logs. A header value can # carry several labels split by the delimiter (default: ","). The optional prefix is diff --git a/config/budget.go b/config/budget.go index 7e5d1e6e..d08eb3ad 100644 --- a/config/budget.go +++ b/config/budget.go @@ -44,7 +44,7 @@ type BudgetLimitConfig struct { Amount float64 `yaml:"amount" json:"amount"` } -func applyBudgetEnv(cfg *Config) error { +func applyBudgetEnv(cfg *Config, strict bool) error { if cfg == nil { return nil } @@ -55,7 +55,7 @@ func applyBudgetEnv(cfg *Config) error { cfg.Budgets.UserPaths, "SET_BUDGET_", func(entry BudgetUserPathConfig) string { return entry.Path }, - parseBudgetEnvLimits, + func(raw string) ([]BudgetLimitConfig, error) { return parseBudgetEnvLimits(raw, strict) }, func(path string, limits []BudgetLimitConfig) BudgetUserPathConfig { return BudgetUserPathConfig{Path: path, Limits: limits} }, @@ -67,7 +67,7 @@ func applyBudgetEnv(cfg *Config) error { return nil } -func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) { +func parseBudgetEnvLimits(raw string, strict bool) ([]BudgetLimitConfig, error) { raw = strings.TrimSpace(raw) if raw == "" { return nil, nil @@ -90,7 +90,7 @@ func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) { } if strings.HasPrefix(raw, "[") { var limits []BudgetLimitConfig - if err := decodeStrictJSON(raw, &limits); err != nil { + if err := decodeIaCJSON("SET_BUDGET_*", raw, &limits, strict); err != nil { return nil, err } return limits, nil diff --git a/config/budget_test.go b/config/budget_test.go index 76aad251..3e478028 100644 --- a/config/budget_test.go +++ b/config/budget_test.go @@ -8,7 +8,7 @@ import ( // The JSON-array form of SET_BUDGET_* decodes through json tags. Without them // period_seconds was silently dropped, yielding a limit with no window. func TestParseBudgetEnvLimits_JSONArray(t *testing.T) { - limits, err := parseBudgetEnvLimits(`[{"period_seconds":7200,"amount":5}]`) + limits, err := parseBudgetEnvLimits(`[{"period_seconds":7200,"amount":5}]`, true) if err != nil { t.Fatalf("parseBudgetEnvLimits() error = %v", err) } @@ -24,7 +24,7 @@ func TestParseBudgetEnvLimits_JSONArray(t *testing.T) { } func TestParseBudgetEnvLimits_RejectsUnknownField(t *testing.T) { - _, err := parseBudgetEnvLimits(`[{"period":"daily","ammount":5}]`) + _, err := parseBudgetEnvLimits(`[{"period":"daily","ammount":5}]`, true) if err == nil { t.Fatal("parseBudgetEnvLimits() error = nil, want unknown-field error") } diff --git a/config/config.go b/config/config.go index eefb0482..d25e1b1e 100644 --- a/config/config.go +++ b/config/config.go @@ -9,6 +9,7 @@ import ( "log/slog" "os" "regexp" + "strconv" "strings" "time" @@ -158,7 +159,12 @@ func buildDefaultConfig() *Config { func Load() (*LoadResult, error) { cfg := buildDefaultConfig() - rawProviders, err := applyYAML(cfg) + strict, err := resolveConfigStrict() + if err != nil { + return nil, err + } + + rawProviders, err := applyYAML(cfg, strict) if err != nil { return nil, err } @@ -174,7 +180,7 @@ func Load() (*LoadResult, error) { if err := applyEnvOverrides(cfg); err != nil { return nil, err } - if err := applyVirtualModelsEnv(cfg); err != nil { + if err := applyVirtualModelsEnv(cfg, strict); err != nil { return nil, err } if err := applyTaggingEnv(cfg); err != nil { @@ -184,13 +190,13 @@ func Load() (*LoadResult, error) { return nil, err } applyBudgetDependencies(cfg) - if err := applyBudgetEnv(cfg); err != nil { + if err := applyBudgetEnv(cfg, strict); err != nil { return nil, err } if err := validateBudgetConfig(&cfg.Budgets); err != nil { return nil, err } - if err := applyRateLimitEnv(cfg); err != nil { + if err := applyRateLimitEnv(cfg, strict); err != nil { return nil, err } if err := validateRateLimitConfig(&cfg.RateLimits); err != nil { @@ -237,15 +243,41 @@ var configFilePaths = []string{ "config.yaml", } +const envConfigStrict = "CONFIG_STRICT" + +// resolveConfigStrict reads CONFIG_STRICT, which defaults to true: an unknown key +// in declarative config aborts startup rather than being ignored, because a +// dropped providers, rate_limits, budgets, or guardrails entry silently changes +// routing, cost, or security. Set it to false to downgrade unknown keys to +// warnings — useful when rolling a binary back under a newer config file. +// +// It is read directly from the environment because it governs the parse of the +// YAML layer, which runs before the env-tag overrides are applied. +func resolveConfigStrict() (bool, error) { + raw := strings.TrimSpace(os.Getenv(envConfigStrict)) + if raw == "" { + return true, nil + } + strict, err := strconv.ParseBool(raw) + if err != nil { + return false, fmt.Errorf("invalid %s: %q is not a boolean", envConfigStrict, raw) + } + if !strict { + slog.Warn("CONFIG_STRICT=false: unknown config keys are ignored with a warning instead of aborting startup") + } + return strict, nil +} + // applyYAML reads an optional config file and overlays it onto cfg. // Returns the raw provider map parsed from the providers: YAML section. // If no config file is found, this is a no-op (not an error). // -// Parsing is strict: an unknown key is an error rather than a silently ignored -// one. A misindented section — the classic `providers:` followed by entries at -// column zero — otherwise parses as a null section plus unknown top-level keys, -// and the gateway boots with none of the operator's providers. -func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { +// When strict, an unknown key is an error rather than a silently ignored one. A +// misindented section — the classic `providers:` followed by entries at column +// zero — otherwise parses as a null section plus unknown top-level keys, and the +// gateway boots with none of the operator's providers. CONFIG_STRICT=false +// downgrades unknown keys to warnings; malformed values stay fatal either way. +func applyYAML(cfg *Config, strict bool) (map[string]RawProviderConfig, error) { path, data, err := readConfigFile() if err != nil { return nil, err @@ -264,11 +296,15 @@ func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { target := yamlTarget{Config: cfg} decoder := yaml.NewDecoder(strings.NewReader(expandString(string(data)))) + // Unknown keys are always detected. Whether they are fatal is decided below, + // so the lax mode can still name each one instead of dropping it in silence. decoder.KnownFields(true) // A file holding only comments decodes to nothing; that is an empty overlay, // not a failure. if err := decoder.Decode(&target); err != nil && !errors.Is(err, io.EOF) { - return nil, formatYAMLError(path, err) + if err := reportYAMLDecodeError(path, err, strict); err != nil { + return nil, err + } } slog.Info("config file loaded", "path", path, "providers", len(target.RawProviders)) @@ -279,6 +315,48 @@ func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { return target.RawProviders, nil } +// reportYAMLDecodeError decides the fate of a decode error. Unknown keys are fatal +// when strict and warnings otherwise; every other problem — a malformed value, a +// syntax error — is fatal regardless, because CONFIG_STRICT relaxes what the schema +// accepts, not whether the file makes sense. Returns nil when nothing is fatal. +func reportYAMLDecodeError(path string, err error, strict bool) error { + var typeErr *yaml.TypeError + if strict || !errors.As(err, &typeErr) { + return formatYAMLError(path, err) + } + + var fatal []string + for _, message := range typeErr.Errors { + line, field, ok := parseUnknownFieldMessage(message) + if !ok { + fatal = append(fatal, message) + continue + } + slog.Warn("unknown config key ignored; it has no effect", + "path", path, "line", line, "field", field) + } + if len(fatal) > 0 { + return formatYAMLError(path, &yaml.TypeError{Errors: fatal}) + } + return nil +} + +// unknownFieldMessage matches yaml.v3's unknown-key message, the only decode error +// CONFIG_STRICT=false is allowed to downgrade. +var unknownFieldMessage = regexp.MustCompile(`^line (\d+): field (\S+) not found in type \S+$`) + +func parseUnknownFieldMessage(message string) (line int, field string, ok bool) { + match := unknownFieldMessage.FindStringSubmatch(message) + if match == nil { + return 0, "", false + } + line, err := strconv.Atoi(match[1]) + if err != nil { + return 0, "", false + } + return line, match[2], true +} + // readConfigFile returns the first config file that exists and its contents, or an // empty path and nil contents when none does. A file that exists but cannot be read // — wrong permissions, or a directory mounted where a file was expected — is an diff --git a/config/config_strict_test.go b/config/config_strict_test.go index 5f021102..30396cfe 100644 --- a/config/config_strict_test.go +++ b/config/config_strict_test.go @@ -112,6 +112,91 @@ func TestLoad_AcceptsValidYAMLShapes(t *testing.T) { } } +// CONFIG_STRICT=false relaxes what the schema accepts, so an unknown key becomes a +// warning and the rest of the file still applies. +func TestLoad_ConfigStrictFalseDowngradesUnknownKeysToWarnings(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "false") + + withTempDir(t, func(dir string) { + // The reporter's misindented file: four keys that should have been providers, + // plus a server section that must still take effect. + writeConfigYAML(t, dir, ` +server: + port: "9999" + +providers: +uranium-geryon-9b: + type: vllm + base_url: "http://uranium-geryon-9b:8000/v1" +`) + + result, err := Load() + if err != nil { + t.Fatalf("Load() failed under CONFIG_STRICT=false: %v", err) + } + if len(result.RawProviders) != 0 { + t.Fatalf("len(RawProviders) = %d, want 0 (the keys are not providers)", len(result.RawProviders)) + } + if result.Config.Server.Port != "9999" { + t.Fatalf("Server.Port = %q, want the known keys to still apply", result.Config.Server.Port) + } + }) +} + +// CONFIG_STRICT relaxes which keys are accepted, never whether a value makes sense. +// A malformed value is a broken config in any mode. +func TestLoad_ConfigStrictFalseStillRejectsMalformedValues(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "false") + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, "server:\n port: [9999, 8080]\n") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want a type error even under CONFIG_STRICT=false") + } + if !strings.Contains(err.Error(), "cannot unmarshal") { + t.Fatalf("Load() error = %q, want a type error", err) + } + }) +} + +// A file that mixes an unknown key with a malformed value must still fail: the +// unknown key is downgraded, the type error is not. +func TestLoad_ConfigStrictFalseFailsWhenAnyErrorIsFatal(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "false") + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, "bogus_section:\n a: 1\nserver:\n port: [9999]\n") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want the type error to remain fatal") + } + if strings.Contains(err.Error(), "bogus_section") { + t.Fatalf("Load() error = %q, want only the fatal type error reported", err) + } + }) +} + +func TestLoad_ConfigStrictRejectsNonBoolean(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "yes-please") + + withTempDir(t, func(string) { + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want an invalid CONFIG_STRICT error") + } + if !strings.Contains(err.Error(), "invalid CONFIG_STRICT") { + t.Fatalf("Load() error = %q, want it to name CONFIG_STRICT", err) + } + }) +} + // TestApplyYAML_ExampleConfigParses keeps the shipped example honest: every key it // documents must exist on Config, or operators who copy it cannot boot under strict // parsing. It exercises the decode only — Load additionally resolves paths the @@ -127,7 +212,7 @@ func TestApplyYAML_ExampleConfigParses(t *testing.T) { withTempDir(t, func(dir string) { writeConfigYAML(t, dir, string(example)) - if _, err := applyYAML(buildDefaultConfig()); err != nil { + if _, err := applyYAML(buildDefaultConfig(), true); err != nil { t.Fatalf("config.example.yaml does not parse: %v", err) } }) @@ -143,7 +228,7 @@ func TestApplyYAML_UnreadableConfigFileIsAnError(t *testing.T) { t.Fatalf("Failed to create directory: %v", err) } - _, err := applyYAML(buildDefaultConfig()) + _, err := applyYAML(buildDefaultConfig(), true) if err == nil { t.Fatal("applyYAML() succeeded, want read error") } diff --git a/config/config_test.go b/config/config_test.go index 676f43cb..cc792792 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -37,6 +37,7 @@ func clearProviderEnvVars(t *testing.T) { func clearAllConfigEnvVars(t *testing.T) { t.Helper() for _, key := range []string{ + "CONFIG_STRICT", "PORT", "BASE_PATH", "GOMODEL_MASTER_KEY", "BODY_SIZE_LIMIT", "SWAGGER_ENABLED", "PPROF_ENABLED", "ENABLE_PASSTHROUGH_ROUTES", "ALLOW_PASSTHROUGH_V1_ALIAS", "USER_PATH_HEADER", "ENABLED_PASSTHROUGH_PROVIDERS", "GOMODEL_CACHE_DIR", "CACHE_REFRESH_INTERVAL", "REDIS_URL", "REDIS_KEY_MODELS", "REDIS_KEY_RESPONSES", "REDIS_TTL_MODELS", "REDIS_TTL_RESPONSES", diff --git a/config/merge.go b/config/merge.go index 74e9d3cf..27f3118f 100644 --- a/config/merge.go +++ b/config/merge.go @@ -2,19 +2,38 @@ package config import ( "encoding/json" + "log/slog" "strings" ) -// decodeStrictJSON decodes an infrastructure-as-code env var into target, rejecting -// unknown keys. The env layer declares the same structures as the YAML layer and -// overrides it entry by entry, so a typo must fail loudly here too — a silently -// ignored key would let a malformed env entry win over a correct YAML one. +// decodeIaCJSON decodes an infrastructure-as-code env var into target. The env layer +// declares the same structures as the YAML layer and overrides it entry by entry, so +// a typo must fail as loudly here as it does there — a silently ignored key would let +// a malformed env entry win over a correct YAML one. CONFIG_STRICT=false downgrades +// unknown keys to warnings, matching the YAML layer; malformed values stay fatal. +func decodeIaCJSON(source, raw string, target any, strict bool) error { + err := decodeStrictJSON(raw, target) + if err == nil || strict || !isUnknownFieldJSONError(err) { + return err + } + slog.Warn("unknown config key ignored; it has no effect", "source", source, "detail", err.Error()) + // The strict decode stopped at the unknown key, so re-decode leniently to fill + // target from the whole value. + return json.Unmarshal([]byte(raw), target) +} + func decodeStrictJSON(raw string, target any) error { decoder := json.NewDecoder(strings.NewReader(raw)) decoder.DisallowUnknownFields() return decoder.Decode(target) } +// isUnknownFieldJSONError reports whether err is encoding/json's unknown-key error, +// the only decode error CONFIG_STRICT=false is allowed to downgrade. +func isUnknownFieldJSONError(err error) bool { + return strings.HasPrefix(err.Error(), "json: unknown field ") +} + // mergeByKey overlays override entries onto base, replacing matching base // entries in place and appending new entries. func mergeByKey[T any](base, override []T, key func(T) string) []T { diff --git a/config/ratelimit.go b/config/ratelimit.go index 202987a8..67af07a9 100644 --- a/config/ratelimit.go +++ b/config/ratelimit.go @@ -68,18 +68,21 @@ type RateLimitRuleConfig struct { MaxTokens *int64 `yaml:"max_tokens" json:"max_tokens"` } -func applyRateLimitEnv(cfg *Config) error { +func applyRateLimitEnv(cfg *Config, strict bool) error { if cfg == nil { return nil } if !cfg.RateLimits.Enabled { return nil } + parseLimits := func(raw string) ([]RateLimitRuleConfig, error) { + return parseRateLimitEnvLimits(raw, strict) + } entries, err := applyUserPathLimitEnv( cfg.RateLimits.UserPaths, "SET_RATE_LIMIT_", func(entry RateLimitUserPathConfig) string { return entry.Path }, - parseRateLimitEnvLimits, + parseLimits, func(path string, limits []RateLimitRuleConfig) RateLimitUserPathConfig { return RateLimitUserPathConfig{Path: path, Limits: limits} }, @@ -99,7 +102,7 @@ func applyRateLimitEnv(cfg *Config) error { rateLimitProviderNameFromEnvSuffix, normalizeRateLimitProviderName, func(entry RateLimitProviderConfig) string { return entry.Name }, - parseRateLimitEnvLimits, + parseLimits, func(name string, limits []RateLimitRuleConfig) RateLimitProviderConfig { return RateLimitProviderConfig{Name: name, Limits: limits} }, @@ -128,14 +131,14 @@ func normalizeRateLimitProviderName(raw string) (string, error) { // parseRateLimitEnvLimits parses either a JSON array of rule objects or the // compact "rpm=100,tpm=50000,rpd=1000,concurrent=10" syntax. -func parseRateLimitEnvLimits(raw string) ([]RateLimitRuleConfig, error) { +func parseRateLimitEnvLimits(raw string, strict bool) ([]RateLimitRuleConfig, error) { raw = strings.TrimSpace(raw) if raw == "" { return nil, nil } if strings.HasPrefix(raw, "[") { var limits []RateLimitRuleConfig - if err := decodeStrictJSON(raw, &limits); err != nil { + if err := decodeIaCJSON("SET_RATE_LIMIT_*", raw, &limits, strict); err != nil { return nil, err } return limits, nil diff --git a/config/ratelimit_test.go b/config/ratelimit_test.go index d7461034..ce7ca55b 100644 --- a/config/ratelimit_test.go +++ b/config/ratelimit_test.go @@ -408,7 +408,7 @@ func TestRateLimitsEnabledByDefaultAndTogglable(t *testing.T) { } func TestParseRateLimitEnvLimits_RejectsUnknownField(t *testing.T) { - _, err := parseRateLimitEnvLimits(`[{"period":"minute","max_requsts":100}]`) + _, err := parseRateLimitEnvLimits(`[{"period":"minute","max_requsts":100}]`, true) if err == nil { t.Fatal("parseRateLimitEnvLimits() error = nil, want unknown-field error") } diff --git a/config/virtualmodels.go b/config/virtualmodels.go index 65cff8b7..347427cb 100644 --- a/config/virtualmodels.go +++ b/config/virtualmodels.go @@ -52,13 +52,13 @@ const envVirtualModels = "VIRTUAL_MODELS" // virtual model definitions — and merges it over the YAML-declared list. Env // entries override YAML entries with the same source, consistent with the rest of // the config pipeline where env always wins. -func applyVirtualModelsEnv(cfg *Config) error { +func applyVirtualModelsEnv(cfg *Config, strict bool) error { raw := strings.TrimSpace(os.Getenv(envVirtualModels)) if raw == "" { return nil } var fromEnv []VirtualModelConfig - if err := decodeStrictJSON(raw, &fromEnv); err != nil { + if err := decodeIaCJSON(envVirtualModels, raw, &fromEnv, strict); err != nil { return fmt.Errorf("invalid %s: %w", envVirtualModels, err) } cfg.VirtualModels = mergeByKey(cfg.VirtualModels, fromEnv, func(model VirtualModelConfig) string { diff --git a/config/virtualmodels_test.go b/config/virtualmodels_test.go index 442ea093..60888bc1 100644 --- a/config/virtualmodels_test.go +++ b/config/virtualmodels_test.go @@ -15,7 +15,7 @@ func TestApplyVirtualModelsEnv_ParsesAndMerges(t *testing.T) { {"source":"new","target":"anthropic/claude"} ]`) - if err := applyVirtualModelsEnv(cfg); err != nil { + if err := applyVirtualModelsEnv(cfg, true); err != nil { t.Fatalf("applyVirtualModelsEnv() error = %v", err) } if len(cfg.VirtualModels) != 3 { @@ -35,7 +35,7 @@ func TestApplyVirtualModelsEnv_ParsesAndMerges(t *testing.T) { func TestApplyVirtualModelsEnv_Invalid(t *testing.T) { cfg := &Config{} t.Setenv(envVirtualModels, `{not valid json`) - if err := applyVirtualModelsEnv(cfg); err == nil { + if err := applyVirtualModelsEnv(cfg, true); err == nil { t.Fatalf("applyVirtualModelsEnv() error = nil, want parse error") } } @@ -46,7 +46,7 @@ func TestApplyVirtualModelsEnv_RejectsUnknownField(t *testing.T) { cfg := &Config{} t.Setenv(envVirtualModels, `[{"source":"smart","targts":[{"model":"openai/gpt-4o"}]}]`) - err := applyVirtualModelsEnv(cfg) + err := applyVirtualModelsEnv(cfg, true) if err == nil { t.Fatal("applyVirtualModelsEnv() error = nil, want unknown-field error") } @@ -55,10 +55,34 @@ func TestApplyVirtualModelsEnv_RejectsUnknownField(t *testing.T) { } } +// CONFIG_STRICT=false applies to the env layer too: the unknown key is warned about +// and the rest of the entry still loads. +func TestApplyVirtualModelsEnv_LaxIgnoresUnknownField(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, `[{"source":"smart","targts":[],"target":"openai/gpt-4o"}]`) + + if err := applyVirtualModelsEnv(cfg, false); err != nil { + t.Fatalf("applyVirtualModelsEnv() error = %v, want the unknown key ignored", err) + } + if len(cfg.VirtualModels) != 1 || cfg.VirtualModels[0].Target != "openai/gpt-4o" { + t.Fatalf("lax decode dropped the known fields: %#v", cfg.VirtualModels) + } +} + +// Even lax, a value of the wrong type is fatal. +func TestApplyVirtualModelsEnv_LaxStillRejectsMalformedValues(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, `[{"source":123}]`) + + if err := applyVirtualModelsEnv(cfg, false); err == nil { + t.Fatal("applyVirtualModelsEnv() error = nil, want a type error") + } +} + func TestApplyVirtualModelsEnv_Unset(t *testing.T) { cfg := &Config{VirtualModels: []VirtualModelConfig{{Source: "smart", Target: "openai/gpt-4o"}}} t.Setenv(envVirtualModels, "") - if err := applyVirtualModelsEnv(cfg); err != nil { + if err := applyVirtualModelsEnv(cfg, true); err != nil { t.Fatalf("applyVirtualModelsEnv() error = %v", err) } if len(cfg.VirtualModels) != 1 { diff --git a/docs/advanced/config-yaml.mdx b/docs/advanced/config-yaml.mdx index 3cabb3a9..330f02df 100644 --- a/docs/advanced/config-yaml.mdx +++ b/docs/advanced/config-yaml.mdx @@ -64,6 +64,25 @@ structures as JSON: `VIRTUAL_MODELS`, `SET_RATE_LIMIT_*`, and `SET_BUDGET_*`. Because env entries override YAML entries, a typo in one of them would otherwise silently win over a correct YAML entry. +The default is strict because a dropped `providers`, `rate_limits`, `budgets`, +or `guardrails` entry silently changes routing, cost, or security — a cost cap +that never applies looks exactly like a cost cap that does. + +Set `CONFIG_STRICT=false` to downgrade unknown keys to warnings and boot without +them. This is for rolling a binary back under a newer config file, not for +everyday use: + +```text +WARN unknown config key ignored; it has no effect path=config.yaml line=23 field=uranium-geryon-9b +INFO providers resolved total=1 from_config_file=0 from_env=1 +``` + +`CONFIG_STRICT` relaxes which *keys* are accepted, never whether a *value* makes +sense. A malformed value (`port: [9999, 8080]`) aborts startup in either mode. +Entity-level validation also still applies: a virtual model whose target names a +provider that does not exist fails startup even when the key that would have +declared that provider was ignored with a warning. + To confirm which layer supplied your providers, GoModel logs one line at boot: ```json @@ -154,7 +173,8 @@ failed to parse config.yaml: line 2: field uranium-geryon-9b not found ``` Indent provider entries two spaces under `providers:`. Before strict parsing, -this booted with zero YAML providers and no error. +this booted with zero YAML providers and no error. With `CONFIG_STRICT=false` it +boots with zero YAML providers and four warnings. ### Per-provider resilience can only come from YAML From 7835f6a6ba319757c2c43b5454714d8bb3b15b97 Mon Sep 17 00:00:00 2001 From: "Jakub A. W" Date: Wed, 8 Jul 2026 18:53:18 +0200 Subject: [PATCH 3/4] fix(config): reject trailing data after declarative JSON and YAML values Swapping json.Unmarshal for a json.Decoder to get DisallowUnknownFields regressed a check nobody asked for but everybody relied on: Decode stops after the first JSON value and leaves the rest unread, so VIRTUAL_MODELS='[{"source":"a"}] {"targts":[]}' booted happily while ignoring the suffix. json.Unmarshal rejects trailing data; silently applying half an env var is precisely the failure this path exists to prevent. Require io.EOF after the decoded value. The YAML layer had the same hole, pre-dating this branch: both yaml.Unmarshal and yaml.Decoder read one document, so every key after a `---` separator was applied nowhere. Reject a second document. Both are structural faults, not unknown keys, so they stay fatal under CONFIG_STRICT=false. Also correct the misindentation gotcha in the docs, whose prose counted keys the sample did not contain. The sample now carries two providers and the quoted error is the binary's verbatim output. Reported by Greptile and CodeRabbit on #511. Co-Authored-By: Claude Opus 4.8 (1M context) --- config/config.go | 26 ++++++++++++++++++++++++-- config/config_strict_test.go | 23 +++++++++++++++++++++++ config/merge.go | 14 +++++++++++++- config/virtualmodels_test.go | 28 ++++++++++++++++++++++++++++ docs/advanced/config-yaml.mdx | 15 +++++++++------ 5 files changed, 97 insertions(+), 9 deletions(-) diff --git a/config/config.go b/config/config.go index d25e1b1e..91244fed 100644 --- a/config/config.go +++ b/config/config.go @@ -301,11 +301,15 @@ func applyYAML(cfg *Config, strict bool) (map[string]RawProviderConfig, error) { decoder.KnownFields(true) // A file holding only comments decodes to nothing; that is an empty overlay, // not a failure. - if err := decoder.Decode(&target); err != nil && !errors.Is(err, io.EOF) { - if err := reportYAMLDecodeError(path, err, strict); err != nil { + decodeErr := decoder.Decode(&target) + if decodeErr != nil && !errors.Is(decodeErr, io.EOF) { + if err := reportYAMLDecodeError(path, decodeErr, strict); err != nil { return nil, err } } + if err := ensureSingleDocument(path, decoder); err != nil { + return nil, err + } slog.Info("config file loaded", "path", path, "providers", len(target.RawProviders)) @@ -315,6 +319,24 @@ func applyYAML(cfg *Config, strict bool) (map[string]RawProviderConfig, error) { return target.RawProviders, nil } +// ensureSingleDocument rejects a config file holding more than one YAML document. +// The decoder reads only the first, so everything after a `---` separator would be +// applied nowhere — the same silent loss a misindented section causes. Decoding into +// a yaml.Node accepts any shape, so this detects a second document without +// re-triggering the unknown-key check. A structural fault, fatal regardless of +// CONFIG_STRICT. +func ensureSingleDocument(path string, decoder *yaml.Decoder) error { + var extra yaml.Node + err := decoder.Decode(&extra) + if errors.Is(err, io.EOF) { + return nil + } + if err != nil { + return formatYAMLError(path, err) + } + return fmt.Errorf("failed to parse %s: only one YAML document is supported, found another after a '---' separator", path) +} + // reportYAMLDecodeError decides the fate of a decode error. Unknown keys are fatal // when strict and warnings otherwise; every other problem — a malformed value, a // syntax error — is fatal regardless, because CONFIG_STRICT relaxes what the schema diff --git a/config/config_strict_test.go b/config/config_strict_test.go index 30396cfe..5d30f6fc 100644 --- a/config/config_strict_test.go +++ b/config/config_strict_test.go @@ -112,6 +112,29 @@ func TestLoad_AcceptsValidYAMLShapes(t *testing.T) { } } +// A second YAML document is never applied, so accepting one silently drops every key +// it declares. Structural, therefore fatal regardless of CONFIG_STRICT. +func TestLoad_RejectsMultipleYAMLDocuments(t *testing.T) { + for _, strict := range []string{"true", "false"} { + t.Run("CONFIG_STRICT="+strict, func(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, strict) + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, "providers:\n a:\n type: vllm\n base_url: \"http://a:8000/v1\"\n---\nproviders:\n b:\n type: vllm\n base_url: \"http://b:8000/v1\"\n") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want a multi-document error") + } + if !strings.Contains(err.Error(), "only one YAML document is supported") { + t.Fatalf("Load() error = %q, want a multi-document error", err) + } + }) + }) + } +} + // CONFIG_STRICT=false relaxes what the schema accepts, so an unknown key becomes a // warning and the rest of the file still applies. func TestLoad_ConfigStrictFalseDowngradesUnknownKeysToWarnings(t *testing.T) { diff --git a/config/merge.go b/config/merge.go index 27f3118f..1483f594 100644 --- a/config/merge.go +++ b/config/merge.go @@ -2,6 +2,9 @@ package config import ( "encoding/json" + "errors" + "fmt" + "io" "log/slog" "strings" ) @@ -25,7 +28,16 @@ func decodeIaCJSON(source, raw string, target any, strict bool) error { func decodeStrictJSON(raw string, target any) error { decoder := json.NewDecoder(strings.NewReader(raw)) decoder.DisallowUnknownFields() - return decoder.Decode(target) + if err := decoder.Decode(target); err != nil { + return err + } + // Decode stops after the first JSON value and leaves the rest unread, so + // `[{...}] junk` would pass. json.Unmarshal rejects trailing data and so must we: + // silently ignoring half an env var is the bug this whole path exists to prevent. + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + return fmt.Errorf("unexpected data after the JSON value") + } + return nil } // isUnknownFieldJSONError reports whether err is encoding/json's unknown-key error, diff --git a/config/virtualmodels_test.go b/config/virtualmodels_test.go index 60888bc1..4b89ca9e 100644 --- a/config/virtualmodels_test.go +++ b/config/virtualmodels_test.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "strings" "testing" ) @@ -55,6 +56,33 @@ func TestApplyVirtualModelsEnv_RejectsUnknownField(t *testing.T) { } } +// json.Decoder stops after the first value and leaves the rest unread, so trailing +// data must be rejected explicitly — silently applying half an env var is the failure +// this path exists to prevent. Structural, therefore fatal in both modes. +func TestApplyVirtualModelsEnv_RejectsTrailingData(t *testing.T) { + trailing := map[string]string{ + "garbage suffix": `[{"source":"smart","target":"openai/gpt-4o"}] and then some junk`, + "second JSON value": `[{"source":"smart","target":"openai/gpt-4o"}] {"targts":[]}`, + "second JSON on a line": "[{\"source\":\"smart\",\"target\":\"openai/gpt-4o\"}]\n{\"x\":1}", + } + for name, raw := range trailing { + for _, strict := range []bool{true, false} { + t.Run(fmt.Sprintf("%s/strict=%v", name, strict), func(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, raw) + + err := applyVirtualModelsEnv(cfg, strict) + if err == nil { + t.Fatal("applyVirtualModelsEnv() error = nil, want trailing-data error") + } + if !strings.Contains(err.Error(), "unexpected data after the JSON value") { + t.Fatalf("applyVirtualModelsEnv() error = %q, want a trailing-data error", err) + } + }) + } + } +} + // CONFIG_STRICT=false applies to the env layer too: the unknown key is warned about // and the rest of the entry still loads. func TestApplyVirtualModelsEnv_LaxIgnoresUnknownField(t *testing.T) { diff --git a/docs/advanced/config-yaml.mdx b/docs/advanced/config-yaml.mdx index 330f02df..75837895 100644 --- a/docs/advanced/config-yaml.mdx +++ b/docs/advanced/config-yaml.mdx @@ -156,25 +156,28 @@ starting the process, or supply a default: `${OPENAI_API_KEY:-}`. ### A misindented `providers:` section is rejected -YAML reads this as an empty `providers:` section plus three unrelated top-level -keys, not as three providers: +YAML reads this as an empty `providers:` section plus two unrelated top-level +keys, not as two providers: ```yaml providers: -uranium-geryon-9b: # not indented under providers: +uranium-geryon-9b: # not indented under providers: type: vllm base_url: "http://uranium-geryon-9b:8000/v1" +uranium-gemma-embedding: # likewise + type: vllm + base_url: "http://uranium-gemma-embedding:8000/v1" ``` -Strict parsing rejects it at startup: +Strict parsing rejects it at startup, naming every offending line: ```text -failed to parse config.yaml: line 2: field uranium-geryon-9b not found +failed to parse config.yaml: line 2: field uranium-geryon-9b not found; line 5: field uranium-gemma-embedding not found ``` Indent provider entries two spaces under `providers:`. Before strict parsing, this booted with zero YAML providers and no error. With `CONFIG_STRICT=false` it -boots with zero YAML providers and four warnings. +boots with zero YAML providers and one warning per unindented key. ### Per-provider resilience can only come from YAML From 91db16b8a1daa90a81eae2f034134285bdff4a31 Mon Sep 17 00:00:00 2001 From: "Jakub A. W" Date: Wed, 8 Jul 2026 19:10:10 +0200 Subject: [PATCH 4/4] build: bump Go to 1.26.5 for GO-2026-5856 govulncheck reports the Encrypted Client Hello privacy leak in crypto/tls (GO-2026-5856), fixed in go1.26.5. It reproduces on pristine main at e0eb0eef, so CI is red independently of this branch; the reachable traces are the dashboard, redis, and mongo TLS paths, none of them new. Bump every pin so the shipped binary is patched, not just CI: .github/workflows/test.yml GO_VERSION 1.26.4 -> 1.26.5 .github/workflows/release.yml go-version 1.26.4 -> 1.26.5 Dockerfile golang:1.26.4-alpine3.23 -> 1.26.5 go.mod go 1.26.4 -> go 1.26.5 Verified: govulncheck reports no vulnerabilities with and without the swagger tag; unit, contract, e2e, and integration suites pass; golangci-lint is clean across all build tags; and the release image builds and reports go1.26.5. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/release.yml | 2 +- .github/workflows/test.yml | 2 +- Dockerfile | 2 +- go.mod | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ac2d8715..b04cc59d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -155,7 +155,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: 1.26.4 + go-version: 1.26.5 cache: true - name: Run GoReleaser diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 32e694b3..b457ab0f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,7 +12,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.26.4" + GO_VERSION: "1.26.5" # Restrict token permissions to minimum required permissions: diff --git a/Dockerfile b/Dockerfile index ae621fb0..e3918eb4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build stage — run on the build host's native arch for speed, cross-compile for target -FROM --platform=$BUILDPLATFORM golang:1.26.4-alpine3.23 AS builder +FROM --platform=$BUILDPLATFORM golang:1.26.5-alpine3.23 AS builder ARG TARGETOS ARG TARGETARCH diff --git a/go.mod b/go.mod index d609f92d..f1648ec8 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module gomodel -go 1.26.4 +go 1.26.5 require ( github.com/andybalholm/brotli v1.2.2