Skip to content

Commit 8e53f67

Browse files
fix(config): reject unknown config keys and log provider origins (#511)
* fix(config): reject unknown config keys and log provider origins 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) <noreply@anthropic.com> * feat(config): add CONFIG_STRICT escape hatch for unknown keys 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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 e0eb0ee, 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 23e3113 commit 8e53f67

20 files changed

Lines changed: 781 additions & 54 deletions

.env.template

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@
55
# Header used to read/write request user_path values (default: X-GoModel-User-Path)
66
# USER_PATH_HEADER=X-GoModel-User-Path
77

8+
# Reject unknown keys in config.yaml and in the JSON env vars that declare the same
9+
# structures (VIRTUAL_MODELS, SET_RATE_LIMIT_*, SET_BUDGET_*). Default: true, so a
10+
# typo or a misindented section fails startup instead of silently dropping providers,
11+
# rate limits, budgets, or guardrails. Set false to downgrade unknown keys to
12+
# warnings — intended for rolling a binary back under a newer config file. Malformed
13+
# values stay fatal in either mode.
14+
# CONFIG_STRICT=true
15+
816
# Tagging based on headers: label every request from the listed headers (numbered
917
# from 1). Labels are recorded in usage tracking and audit logs. A header value can
1018
# carry several labels split by the delimiter (default: ","). The optional prefix is

.github/workflows/release.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ jobs:
155155
- name: Set up Go
156156
uses: actions/setup-go@v6
157157
with:
158-
go-version: 1.26.4
158+
go-version: 1.26.5
159159
cache: true
160160

161161
- name: Run GoReleaser

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ concurrency:
1212
cancel-in-progress: true
1313

1414
env:
15-
GO_VERSION: "1.26.4"
15+
GO_VERSION: "1.26.5"
1616

1717
# Restrict token permissions to minimum required
1818
permissions:

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Build stage — run on the build host's native arch for speed, cross-compile for target
2-
FROM --platform=$BUILDPLATFORM golang:1.26.4-alpine3.23 AS builder
2+
FROM --platform=$BUILDPLATFORM golang:1.26.5-alpine3.23 AS builder
33

44
ARG TARGETOS
55
ARG TARGETARCH

config/budget.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,21 @@ type BudgetUserPathConfig struct {
3030
}
3131

3232
// BudgetLimitConfig declares one spend limit for a reset period.
33+
// The json tags support the JSON-array form of SET_BUDGET_* env values.
3334
type BudgetLimitConfig struct {
3435
// Period accepts hourly, daily, weekly, or monthly. The resolved period is
3536
// persisted as PeriodSeconds in the database.
36-
Period string `yaml:"period"`
37+
Period string `yaml:"period" json:"period"`
3738

3839
// PeriodSeconds can be set directly instead of Period. Standard values are
3940
// 3600, 86400, 604800, and 2592000.
40-
PeriodSeconds int64 `yaml:"period_seconds"`
41+
PeriodSeconds int64 `yaml:"period_seconds" json:"period_seconds"`
4142

4243
// Amount is the maximum allowed tracked provider spend for the period.
43-
Amount float64 `yaml:"amount"`
44+
Amount float64 `yaml:"amount" json:"amount"`
4445
}
4546

46-
func applyBudgetEnv(cfg *Config) error {
47+
func applyBudgetEnv(cfg *Config, strict bool) error {
4748
if cfg == nil {
4849
return nil
4950
}
@@ -54,7 +55,7 @@ func applyBudgetEnv(cfg *Config) error {
5455
cfg.Budgets.UserPaths,
5556
"SET_BUDGET_",
5657
func(entry BudgetUserPathConfig) string { return entry.Path },
57-
parseBudgetEnvLimits,
58+
func(raw string) ([]BudgetLimitConfig, error) { return parseBudgetEnvLimits(raw, strict) },
5859
func(path string, limits []BudgetLimitConfig) BudgetUserPathConfig {
5960
return BudgetUserPathConfig{Path: path, Limits: limits}
6061
},
@@ -66,7 +67,7 @@ func applyBudgetEnv(cfg *Config) error {
6667
return nil
6768
}
6869

69-
func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) {
70+
func parseBudgetEnvLimits(raw string, strict bool) ([]BudgetLimitConfig, error) {
7071
raw = strings.TrimSpace(raw)
7172
if raw == "" {
7273
return nil, nil
@@ -89,7 +90,7 @@ func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) {
8990
}
9091
if strings.HasPrefix(raw, "[") {
9192
var limits []BudgetLimitConfig
92-
if err := json.Unmarshal([]byte(raw), &limits); err != nil {
93+
if err := decodeIaCJSON("SET_BUDGET_*", raw, &limits, strict); err != nil {
9394
return nil, err
9495
}
9596
return limits, nil

config/budget_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package config
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
// The JSON-array form of SET_BUDGET_* decodes through json tags. Without them
9+
// period_seconds was silently dropped, yielding a limit with no window.
10+
func TestParseBudgetEnvLimits_JSONArray(t *testing.T) {
11+
limits, err := parseBudgetEnvLimits(`[{"period_seconds":7200,"amount":5}]`, true)
12+
if err != nil {
13+
t.Fatalf("parseBudgetEnvLimits() error = %v", err)
14+
}
15+
if len(limits) != 1 {
16+
t.Fatalf("len(limits) = %d, want 1", len(limits))
17+
}
18+
if limits[0].PeriodSeconds != 7200 {
19+
t.Fatalf("PeriodSeconds = %d, want 7200", limits[0].PeriodSeconds)
20+
}
21+
if limits[0].Amount != 5 {
22+
t.Fatalf("Amount = %v, want 5", limits[0].Amount)
23+
}
24+
}
25+
26+
func TestParseBudgetEnvLimits_RejectsUnknownField(t *testing.T) {
27+
_, err := parseBudgetEnvLimits(`[{"period":"daily","ammount":5}]`, true)
28+
if err == nil {
29+
t.Fatal("parseBudgetEnvLimits() error = nil, want unknown-field error")
30+
}
31+
if !strings.Contains(err.Error(), "ammount") {
32+
t.Fatalf("parseBudgetEnvLimits() error = %q, want it to name the unknown field", err)
33+
}
34+
}

config/config.go

Lines changed: 173 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@
22
package config
33

44
import (
5+
"errors"
56
"fmt"
7+
"io"
8+
"io/fs"
9+
"log/slog"
610
"os"
11+
"regexp"
12+
"strconv"
13+
"strings"
714
"time"
815

916
"gopkg.in/yaml.v3"
@@ -152,7 +159,12 @@ func buildDefaultConfig() *Config {
152159
func Load() (*LoadResult, error) {
153160
cfg := buildDefaultConfig()
154161

155-
rawProviders, err := applyYAML(cfg)
162+
strict, err := resolveConfigStrict()
163+
if err != nil {
164+
return nil, err
165+
}
166+
167+
rawProviders, err := applyYAML(cfg, strict)
156168
if err != nil {
157169
return nil, err
158170
}
@@ -168,7 +180,7 @@ func Load() (*LoadResult, error) {
168180
if err := applyEnvOverrides(cfg); err != nil {
169181
return nil, err
170182
}
171-
if err := applyVirtualModelsEnv(cfg); err != nil {
183+
if err := applyVirtualModelsEnv(cfg, strict); err != nil {
172184
return nil, err
173185
}
174186
if err := applyTaggingEnv(cfg); err != nil {
@@ -178,13 +190,13 @@ func Load() (*LoadResult, error) {
178190
return nil, err
179191
}
180192
applyBudgetDependencies(cfg)
181-
if err := applyBudgetEnv(cfg); err != nil {
193+
if err := applyBudgetEnv(cfg, strict); err != nil {
182194
return nil, err
183195
}
184196
if err := validateBudgetConfig(&cfg.Budgets); err != nil {
185197
return nil, err
186198
}
187-
if err := applyRateLimitEnv(cfg); err != nil {
199+
if err := applyRateLimitEnv(cfg, strict); err != nil {
188200
return nil, err
189201
}
190202
if err := validateRateLimitConfig(&cfg.RateLimits); err != nil {
@@ -225,32 +237,56 @@ func Load() (*LoadResult, error) {
225237
}, nil
226238
}
227239

228-
// applyYAML reads an optional config.yaml and overlays it onto cfg.
240+
// configFilePaths are searched in order; the first readable file wins.
241+
var configFilePaths = []string{
242+
"config/config.yaml",
243+
"config.yaml",
244+
}
245+
246+
const envConfigStrict = "CONFIG_STRICT"
247+
248+
// resolveConfigStrict reads CONFIG_STRICT, which defaults to true: an unknown key
249+
// in declarative config aborts startup rather than being ignored, because a
250+
// dropped providers, rate_limits, budgets, or guardrails entry silently changes
251+
// routing, cost, or security. Set it to false to downgrade unknown keys to
252+
// warnings — useful when rolling a binary back under a newer config file.
253+
//
254+
// It is read directly from the environment because it governs the parse of the
255+
// YAML layer, which runs before the env-tag overrides are applied.
256+
func resolveConfigStrict() (bool, error) {
257+
raw := strings.TrimSpace(os.Getenv(envConfigStrict))
258+
if raw == "" {
259+
return true, nil
260+
}
261+
strict, err := strconv.ParseBool(raw)
262+
if err != nil {
263+
return false, fmt.Errorf("invalid %s: %q is not a boolean", envConfigStrict, raw)
264+
}
265+
if !strict {
266+
slog.Warn("CONFIG_STRICT=false: unknown config keys are ignored with a warning instead of aborting startup")
267+
}
268+
return strict, nil
269+
}
270+
271+
// applyYAML reads an optional config file and overlays it onto cfg.
229272
// Returns the raw provider map parsed from the providers: YAML section.
230273
// If no config file is found, this is a no-op (not an error).
231-
func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) {
232-
paths := []string{
233-
"config/config.yaml",
234-
"config.yaml",
235-
}
236-
237-
var data []byte
238-
for _, p := range paths {
239-
raw, err := os.ReadFile(p)
240-
if err == nil {
241-
data = raw
242-
break
243-
}
274+
//
275+
// When strict, an unknown key is an error rather than a silently ignored one. A
276+
// misindented section — the classic `providers:` followed by entries at column
277+
// zero — otherwise parses as a null section plus unknown top-level keys, and the
278+
// gateway boots with none of the operator's providers. CONFIG_STRICT=false
279+
// downgrades unknown keys to warnings; malformed values stay fatal either way.
280+
func applyYAML(cfg *Config, strict bool) (map[string]RawProviderConfig, error) {
281+
path, data, err := readConfigFile()
282+
if err != nil {
283+
return nil, err
244284
}
245-
246-
rawProviders := make(map[string]RawProviderConfig)
247-
248285
if data == nil {
249-
return rawProviders, nil
286+
slog.Info("no config file found; using defaults and environment", "searched", configFilePaths)
287+
return map[string]RawProviderConfig{}, nil
250288
}
251289

252-
expanded := expandString(string(data))
253-
254290
// yamlTarget is a local struct that mirrors Config for YAML unmarshaling,
255291
// using RawProviderConfig for providers so nullable resilience overrides are preserved.
256292
type yamlTarget struct {
@@ -259,13 +295,121 @@ func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) {
259295
}
260296

261297
target := yamlTarget{Config: cfg}
262-
if err := yaml.Unmarshal([]byte(expanded), &target); err != nil {
263-
return nil, fmt.Errorf("failed to parse config.yaml: %w", err)
298+
decoder := yaml.NewDecoder(strings.NewReader(expandString(string(data))))
299+
// Unknown keys are always detected. Whether they are fatal is decided below,
300+
// so the lax mode can still name each one instead of dropping it in silence.
301+
decoder.KnownFields(true)
302+
// A file holding only comments decodes to nothing; that is an empty overlay,
303+
// not a failure.
304+
decodeErr := decoder.Decode(&target)
305+
if decodeErr != nil && !errors.Is(decodeErr, io.EOF) {
306+
if err := reportYAMLDecodeError(path, decodeErr, strict); err != nil {
307+
return nil, err
308+
}
264309
}
310+
if err := ensureSingleDocument(path, decoder); err != nil {
311+
return nil, err
312+
}
313+
314+
slog.Info("config file loaded", "path", path, "providers", len(target.RawProviders))
265315

266-
if target.RawProviders != nil {
267-
rawProviders = target.RawProviders
316+
if target.RawProviders == nil {
317+
return map[string]RawProviderConfig{}, nil
268318
}
319+
return target.RawProviders, nil
320+
}
321+
322+
// ensureSingleDocument rejects a config file holding more than one YAML document.
323+
// The decoder reads only the first, so everything after a `---` separator would be
324+
// applied nowhere — the same silent loss a misindented section causes. Decoding into
325+
// a yaml.Node accepts any shape, so this detects a second document without
326+
// re-triggering the unknown-key check. A structural fault, fatal regardless of
327+
// CONFIG_STRICT.
328+
func ensureSingleDocument(path string, decoder *yaml.Decoder) error {
329+
var extra yaml.Node
330+
err := decoder.Decode(&extra)
331+
if errors.Is(err, io.EOF) {
332+
return nil
333+
}
334+
if err != nil {
335+
return formatYAMLError(path, err)
336+
}
337+
return fmt.Errorf("failed to parse %s: only one YAML document is supported, found another after a '---' separator", path)
338+
}
339+
340+
// reportYAMLDecodeError decides the fate of a decode error. Unknown keys are fatal
341+
// when strict and warnings otherwise; every other problem — a malformed value, a
342+
// syntax error — is fatal regardless, because CONFIG_STRICT relaxes what the schema
343+
// accepts, not whether the file makes sense. Returns nil when nothing is fatal.
344+
func reportYAMLDecodeError(path string, err error, strict bool) error {
345+
var typeErr *yaml.TypeError
346+
if strict || !errors.As(err, &typeErr) {
347+
return formatYAMLError(path, err)
348+
}
349+
350+
var fatal []string
351+
for _, message := range typeErr.Errors {
352+
line, field, ok := parseUnknownFieldMessage(message)
353+
if !ok {
354+
fatal = append(fatal, message)
355+
continue
356+
}
357+
slog.Warn("unknown config key ignored; it has no effect",
358+
"path", path, "line", line, "field", field)
359+
}
360+
if len(fatal) > 0 {
361+
return formatYAMLError(path, &yaml.TypeError{Errors: fatal})
362+
}
363+
return nil
364+
}
365+
366+
// unknownFieldMessage matches yaml.v3's unknown-key message, the only decode error
367+
// CONFIG_STRICT=false is allowed to downgrade.
368+
var unknownFieldMessage = regexp.MustCompile(`^line (\d+): field (\S+) not found in type \S+$`)
369+
370+
func parseUnknownFieldMessage(message string) (line int, field string, ok bool) {
371+
match := unknownFieldMessage.FindStringSubmatch(message)
372+
if match == nil {
373+
return 0, "", false
374+
}
375+
line, err := strconv.Atoi(match[1])
376+
if err != nil {
377+
return 0, "", false
378+
}
379+
return line, match[2], true
380+
}
381+
382+
// readConfigFile returns the first config file that exists and its contents, or an
383+
// empty path and nil contents when none does. A file that exists but cannot be read
384+
// — wrong permissions, or a directory mounted where a file was expected — is an
385+
// error, not a missing file: silently falling back to defaults is how a
386+
// misconfigured deployment boots with no providers.
387+
func readConfigFile() (string, []byte, error) {
388+
for _, path := range configFilePaths {
389+
data, err := os.ReadFile(path)
390+
switch {
391+
case err == nil:
392+
return path, data, nil
393+
case errors.Is(err, fs.ErrNotExist):
394+
continue
395+
default:
396+
return "", nil, fmt.Errorf("failed to read %s: %w", path, err)
397+
}
398+
}
399+
return "", nil, nil
400+
}
401+
402+
// yamlTypeSuffix matches the Go type name yaml.v3 appends to unknown-field errors
403+
// ("field foo not found in type config.yamlTarget"). It names an internal struct
404+
// the operator cannot act on, so it is stripped.
405+
var yamlTypeSuffix = regexp.MustCompile(` in type \S+`)
269406

270-
return rawProviders, nil
407+
// formatYAMLError rewrites a yaml.v3 decode error into a single actionable line
408+
// prefixed with the offending file.
409+
func formatYAMLError(path string, err error) error {
410+
msg := yamlTypeSuffix.ReplaceAllString(err.Error(), "")
411+
msg = strings.TrimPrefix(msg, "yaml: unmarshal errors:\n")
412+
msg = strings.TrimPrefix(msg, "yaml: ")
413+
msg = strings.ReplaceAll(msg, "\n ", "; ")
414+
return fmt.Errorf("failed to parse %s: %s", path, strings.TrimSpace(msg))
271415
}

0 commit comments

Comments
 (0)