From 67aa44e8756c09630737bd0e6734b21fd930ed86 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 25 Apr 2026 22:25:35 +0300 Subject: [PATCH] feat(config): viper-loaded config with fail-fast validation --- go.mod | 2 +- internal/config/config.go | 232 ++++++++++++++++++++ internal/config/config_test.go | 381 +++++++++++++++++++++++++++++++++ internal/deps/deps.go | 16 +- 4 files changed, 621 insertions(+), 10 deletions(-) create mode 100644 internal/config/config.go create mode 100644 internal/config/config_test.go diff --git a/go.mod b/go.mod index b5e5a9b272a76a482404880fd366a3b87fd34794..305ac937a3714ecf0f73703533531a33333d4bae 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/coreos/go-oidc/v3 v3.18.0 github.com/go-chi/chi/v5 v5.2.5 github.com/go-playground/validator/v10 v10.30.2 + github.com/go-viper/mapstructure/v2 v2.4.0 github.com/golang-migrate/migrate/v4 v4.19.1 github.com/jmoiron/sqlx v1.4.0 github.com/prometheus/client_golang v1.23.2 @@ -24,7 +25,6 @@ require ( github.com/go-jose/go-jose/v4 v4.1.4 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect - github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/leodido/go-urn v1.4.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 0000000000000000000000000000000000000000..b154cd6048f3d696df93c7b25baeb095d338931c --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,232 @@ +// Package config loads and validates the lethe server configuration from a +// YAML file plus LETHE_* environment overrides. +// +// The Config struct is split into substructs (Server, Database, Auth, Logging, +// Ingest); each substruct is tagged with `config-section:""` so that steward +// (Phase 4+) can inject them by type into individual services. +// +// Validation runs in fail-fast mode: any unknown YAML key, missing required +// field, or out-of-range value rejects the load with a culpa-wrapped error +// carrying a machine-readable code (CONFIG_NOT_FOUND, CONFIG_PARSE, +// CONFIG_VALIDATE). There are no silent fallbacks: the only defaults applied +// are the ones explicitly listed in registerDefaults. +package config + +import ( + "errors" + "io/fs" + "os" + "regexp" + "strings" + "time" + + "github.com/go-playground/validator/v10" + "github.com/go-viper/mapstructure/v2" + "github.com/spf13/viper" + "go.bigb.es/auxilia/culpa" +) + +// Config is the root configuration object. Each substruct is exposed to +// steward as a config section via the `config-section:""` tag. +type Config struct { + Server ServerConfig `mapstructure:"server" config-section:""` + Database DatabaseConfig `mapstructure:"database" config-section:""` + Auth AuthConfig `mapstructure:"auth" config-section:""` + Logging LoggingConfig `mapstructure:"logging" config-section:""` + Ingest IngestConfig `mapstructure:"ingest" config-section:""` +} + +// ServerConfig controls the HTTP listener. +type ServerConfig struct { + Bind string `mapstructure:"bind" validate:"required,loopback_bind"` + ShutdownGrace time.Duration `mapstructure:"shutdown_grace" validate:"gt=0"` +} + +// DatabaseConfig points at the SQLite file and tunes its busy timeout. +type DatabaseConfig struct { + Path string `mapstructure:"path" validate:"required"` + BusyTimeout time.Duration `mapstructure:"busy_timeout" validate:"gt=0"` +} + +// AuthConfig is the authentication policy. At least one of ForwardAuth or +// OIDC must be enabled (enforced by the auth_at_least_one struct-level +// validator) and Admins must be a subset of AllowedUsers (enforced by +// admins_subset_of_allowed). +type AuthConfig struct { + AllowedUsers []string `mapstructure:"allowed_users" validate:"min=1,dive,required"` + Admins []string `mapstructure:"admins" validate:"dive,required"` + ForwardAuth ForwardAuthConfig `mapstructure:"forward_auth"` + OIDC OIDCConfig `mapstructure:"oidc"` +} + +// ForwardAuthConfig consumes a trusted reverse-proxy header. +type ForwardAuthConfig struct { + Enabled bool `mapstructure:"enabled"` + UserHeader string `mapstructure:"user_header"` +} + +// OIDCConfig validates bearer tokens against an OIDC issuer. +type OIDCConfig struct { + Enabled bool `mapstructure:"enabled"` + Issuer string `mapstructure:"issuer" validate:"required_if=Enabled true,omitempty,url"` + Audience string `mapstructure:"audience" validate:"required_if=Enabled true"` + UsernameClaim string `mapstructure:"username_claim"` +} + +// LoggingConfig selects log level and formatter. +type LoggingConfig struct { + Level string `mapstructure:"level" validate:"required,oneof=debug info warn error"` + Format string `mapstructure:"format" validate:"required,oneof=tint json"` +} + +// IngestConfig caps payload sizes and chunking on the ingest path. +type IngestConfig struct { + MaxBodyBytes int64 `mapstructure:"max_body_bytes" validate:"gt=0"` + MaxTurnContentBytes int64 `mapstructure:"max_turn_content_bytes" validate:"gt=0,ltefield=MaxBodyBytes"` + ChunkSize int `mapstructure:"chunk_size" validate:"gt=0"` +} + +// loopbackBindRe matches `127.0.0.1` or `127.0.0.1:` (1-5 digits). +// Other interfaces are rejected so the listener never binds publicly. +var loopbackBindRe = regexp.MustCompile(`^127\.0\.0\.1(?::\d{1,5})?$`) + +// Load reads YAML from path, applies env overrides, fills documented +// defaults, and validates. Errors carry a culpa code. +func Load(path string) (*Config, error) { + v := viper.New() + v.SetConfigFile(path) + v.SetEnvPrefix("LETHE") + v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + v.AutomaticEnv() + + registerDefaults(v) + + if err := v.ReadInConfig(); err != nil { + // viper returns *fs.PathError for missing files; treat any read + // failure as CONFIG_NOT_FOUND vs CONFIG_PARSE based on type. + if _, ok := err.(viper.ConfigFileNotFoundError); ok { + return nil, culpa.WithCode(culpa.Wrap(err, "config file not found"), "CONFIG_NOT_FOUND") + } + // Distinguish "missing file" (os PathError) from a parse failure. + if isFileMissing(err) { + return nil, culpa.WithCode(culpa.Wrap(err, "config file not found"), "CONFIG_NOT_FOUND") + } + return nil, culpa.WithCode(culpa.Wrap(err, "parse config"), "CONFIG_PARSE") + } + + // Stitch comma-separated env overrides for slice fields. viper does + // not split string env values into []string by itself, so we do it + // here for the known slice fields under auth. + splitCSVEnv(v, "auth.allowed_users", "LETHE_AUTH_ALLOWED_USERS") + splitCSVEnv(v, "auth.admins", "LETHE_AUTH_ADMINS") + + var cfg Config + if err := v.UnmarshalExact( + &cfg, + viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( + mapstructure.StringToTimeDurationHookFunc(), + mapstructure.StringToSliceHookFunc(","), + )), + func(c *mapstructure.DecoderConfig) { + c.ErrorUnused = true + }, + ); err != nil { + return nil, culpa.WithCode(culpa.Wrap(err, "decode config"), "CONFIG_PARSE") + } + + val := newValidator() + if err := val.Struct(&cfg); err != nil { + return nil, culpa.WithCode(culpa.Wrap(err, "validate config"), "CONFIG_VALIDATE") + } + return &cfg, nil +} + +// MustLoad calls Load and panics on error. Used by main.go where a bad +// config means the process cannot start anyway. +func MustLoad(path string) *Config { + cfg, err := Load(path) + if err != nil { + panic(err) + } + return cfg +} + +// registerDefaults sets the documented defaults. Notably, neither +// auth.forward_auth.enabled nor auth.oidc.enabled has a default — the +// operator must explicitly enable at least one mode. +func registerDefaults(v *viper.Viper) { + v.SetDefault("server.shutdown_grace", 10*time.Second) + v.SetDefault("database.busy_timeout", 5*time.Second) + v.SetDefault("auth.forward_auth.user_header", "Remote-User") + v.SetDefault("auth.oidc.username_claim", "preferred_username") + v.SetDefault("ingest.max_body_bytes", int64(16*1024*1024)) + v.SetDefault("ingest.max_turn_content_bytes", int64(4*1024*1024)) + v.SetDefault("ingest.chunk_size", 500) +} + +// splitCSVEnv converts a comma-separated environment variable into a +// []string and stores it on viper under key. We cannot rely on viper's +// AutomaticEnv to do this for slice destinations because the env value +// arrives as a single string after viper resolves it. +func splitCSVEnv(v *viper.Viper, key, envName string) { + raw, ok := os.LookupEnv(envName) + if !ok { + return + } + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p == "" { + continue + } + out = append(out, p) + } + v.Set(key, out) +} + +// newValidator builds a *validator.Validate with our custom rules +// registered. +func newValidator() *validator.Validate { + val := validator.New(validator.WithRequiredStructEnabled()) + // "loopback_bind" is a field-level rule so it can be expressed via the + // validate tag on ServerConfig.Bind alongside required. + _ = val.RegisterValidation("loopback_bind", func(fl validator.FieldLevel) bool { + return loopbackBindRe.MatchString(fl.Field().String()) + }) + val.RegisterStructValidation(authStructValidator, AuthConfig{}) + return val +} + +// authStructValidator enforces both auth_at_least_one and +// admins_subset_of_allowed. Struct-level is the right level: both rules +// straddle multiple fields of AuthConfig. +func authStructValidator(sl validator.StructLevel) { + auth, ok := sl.Current().Interface().(AuthConfig) + if !ok { + return + } + if !auth.ForwardAuth.Enabled && !auth.OIDC.Enabled { + sl.ReportError(auth.ForwardAuth.Enabled, "ForwardAuth.Enabled", "forward_auth.enabled", "auth_at_least_one", "") + } + allowed := make(map[string]struct{}, len(auth.AllowedUsers)) + for _, u := range auth.AllowedUsers { + allowed[u] = struct{}{} + } + for _, admin := range auth.Admins { + if _, found := allowed[admin]; !found { + sl.ReportError(auth.Admins, "Admins", "admins", "admins_subset_of_allowed", "") + return + } + } +} + +// isFileMissing reports whether err describes a missing-file condition +// from viper.ReadInConfig. viper wraps the underlying os error. +func isFileMissing(err error) bool { + var pe *fs.PathError + if errors.As(err, &pe) { + return errors.Is(pe.Err, fs.ErrNotExist) + } + return errors.Is(err, fs.ErrNotExist) +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000000000000000000000000000000000000..ea790e6e5adda35475f52abc680df0671b395361 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,381 @@ +package config_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "sourcecraft.dev/bigbes/lethe/internal/config" +) + +// validForwardAuthYAML is a baseline valid configuration with forward-auth +// enabled. Tests typically clone and tweak this string for negative cases. +const validForwardAuthYAML = ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice", "bob"] + admins: ["alice"] + forward_auth: + enabled: true + oidc: + enabled: false +logging: + level: "info" + format: "tint" +ingest: {} +` + +const validOIDCYAML = ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: false + oidc: + enabled: true + issuer: "https://auth.example.com" + audience: "lethe" +logging: + level: "info" + format: "tint" +ingest: {} +` + +const validBothEnabledYAML = ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: true + oidc: + enabled: true + issuer: "https://auth.example.com" + audience: "lethe" +logging: + level: "info" + format: "tint" +ingest: {} +` + +func writeYAML(t *testing.T, body string) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write yaml: %v", err) + } + return path +} + +func TestLoad_ValidForwardAuth(t *testing.T) { + path := writeYAML(t, validForwardAuthYAML) + cfg, err := config.Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Server.Bind != "127.0.0.1:8080" { + t.Errorf("Server.Bind = %q", cfg.Server.Bind) + } + if !cfg.Auth.ForwardAuth.Enabled { + t.Error("ForwardAuth.Enabled = false, want true") + } +} + +func TestLoad_ValidOIDC(t *testing.T) { + path := writeYAML(t, validOIDCYAML) + cfg, err := config.Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if !cfg.Auth.OIDC.Enabled { + t.Error("OIDC.Enabled = false, want true") + } + if cfg.Auth.OIDC.Issuer != "https://auth.example.com" { + t.Errorf("OIDC.Issuer = %q", cfg.Auth.OIDC.Issuer) + } +} + +func TestLoad_ValidBothEnabled(t *testing.T) { + path := writeYAML(t, validBothEnabledYAML) + if _, err := config.Load(path); err != nil { + t.Fatalf("Load: %v", err) + } +} + +func TestLoad_Defaults(t *testing.T) { + path := writeYAML(t, validForwardAuthYAML) + cfg, err := config.Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Auth.ForwardAuth.UserHeader != "Remote-User" { + t.Errorf("ForwardAuth.UserHeader = %q, want Remote-User", cfg.Auth.ForwardAuth.UserHeader) + } + if cfg.Auth.OIDC.UsernameClaim != "preferred_username" { + t.Errorf("OIDC.UsernameClaim = %q, want preferred_username", cfg.Auth.OIDC.UsernameClaim) + } + if cfg.Ingest.MaxBodyBytes != 16*1024*1024 { + t.Errorf("Ingest.MaxBodyBytes = %d, want %d", cfg.Ingest.MaxBodyBytes, 16*1024*1024) + } + if cfg.Ingest.MaxTurnContentBytes != 4*1024*1024 { + t.Errorf("Ingest.MaxTurnContentBytes = %d, want %d", cfg.Ingest.MaxTurnContentBytes, 4*1024*1024) + } + if cfg.Ingest.ChunkSize != 500 { + t.Errorf("Ingest.ChunkSize = %d, want 500", cfg.Ingest.ChunkSize) + } + if cfg.Database.BusyTimeout != 5*time.Second { + t.Errorf("Database.BusyTimeout = %s, want 5s", cfg.Database.BusyTimeout) + } + if cfg.Server.ShutdownGrace != 10*time.Second { + t.Errorf("Server.ShutdownGrace = %s, want 10s", cfg.Server.ShutdownGrace) + } +} + +func TestLoad_EmptyAllowlistRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: [] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error for empty allowed_users, got nil") + } +} + +func TestLoad_NonLoopbackBindRejected(t *testing.T) { + body := strings.Replace(validForwardAuthYAML, `"127.0.0.1:8080"`, `"0.0.0.0:8080"`, 1) + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error for non-loopback bind, got nil") + } +} + +func TestLoad_BothAuthModesDisabledRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: false + oidc: + enabled: false +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error when both auth modes are disabled, got nil") + } +} + +func TestLoad_OIDCEnabledWithoutIssuerRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: false + oidc: + enabled: true + audience: "lethe" +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error for OIDC enabled without issuer, got nil") + } +} + +func TestLoad_OIDCEnabledWithNonURLIssuerRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: false + oidc: + enabled: true + issuer: "not a url" + audience: "lethe" +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error for non-URL issuer, got nil") + } +} + +func TestLoad_AdminNotInAllowedUsersRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + admins: ["mallory"] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error when admin is not in allowed_users, got nil") + } +} + +func TestLoad_EmptyAdminsAccepted(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + admins: [] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err != nil { + t.Fatalf("expected empty admins to be accepted, got %v", err) + } +} + +func TestLoad_MissingDatabasePathRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: {} +auth: + allowed_users: ["alice"] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error for missing database.path, got nil") + } +} + +func TestLoad_MaxTurnContentBytesGreaterThanMaxBodyBytesRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: + max_body_bytes: 1024 + max_turn_content_bytes: 4096 +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error when max_turn_content_bytes > max_body_bytes, got nil") + } +} + +func TestLoad_UnknownYAMLKeyRejected(t *testing.T) { + body := ` +server: + bind: "127.0.0.1:8080" + totally_unknown_key: 42 +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: {} +` + if _, err := config.Load(writeYAML(t, body)); err == nil { + t.Fatal("expected error for unknown YAML key, got nil") + } +} + +func TestLoad_EnvOverride(t *testing.T) { + // Use a YAML body without admins so the override test only exercises + // the slice-decoding path (LETHE_AUTH_ALLOWED_USERS overrides YAML). + body := ` +server: + bind: "127.0.0.1:8080" +database: + path: "./lethe.db" +auth: + allowed_users: ["alice"] + forward_auth: + enabled: true +logging: + level: "info" + format: "tint" +ingest: {} +` + path := writeYAML(t, body) + t.Setenv("LETHE_AUTH_ALLOWED_USERS", "carol,dave") + cfg, err := config.Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + got := cfg.Auth.AllowedUsers + if len(got) != 2 || got[0] != "carol" || got[1] != "dave" { + t.Errorf("AllowedUsers = %v, want [carol dave]", got) + } +} + +func TestMustLoad_PanicsOnError(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Fatal("expected MustLoad to panic on missing file") + } + }() + config.MustLoad("/nonexistent/path/that/should/not/exist.yaml") +} diff --git a/internal/deps/deps.go b/internal/deps/deps.go index c8356c927c83e7bd22c884287484a27e3718df30..c3de7e04a6d0783ef757503eaff796ed44a0cf3e 100644 --- a/internal/deps/deps.go +++ b/internal/deps/deps.go @@ -1,22 +1,20 @@ // Package deps records the locked set of direct dependencies for the lethe -// server during early scaffolding (Phase 1). Real packages adopt these as -// they come online (config — viper/validator; server — chi/prometheus; -// database — sqlx/modernc.org/sqlite/golang-migrate; auth — go-oidc; -// platform — auxilia steward/culpa/scribe). +// server during early scaffolding. Real packages adopt these as they come +// online (server — chi/prometheus; database — sqlx/modernc.org/sqlite/ +// golang-migrate; auth — go-oidc; platform — auxilia steward/scribe). // -// Once every dep below has at least one real importer, this file is -// expected to disappear in the same commit that completes the migration. +// Phase 2 promoted viper, validator/v10, and culpa to real imports under +// internal/config; they no longer appear here. Once every dep below has at +// least one real importer, this file is expected to disappear in the same +// commit that completes the migration. package deps import ( _ "github.com/coreos/go-oidc/v3/oidc" _ "github.com/go-chi/chi/v5" - _ "github.com/go-playground/validator/v10" _ "github.com/golang-migrate/migrate/v4" _ "github.com/jmoiron/sqlx" _ "github.com/prometheus/client_golang/prometheus" - _ "github.com/spf13/viper" - _ "go.bigb.es/auxilia/culpa" _ "go.bigb.es/auxilia/scribe" _ "go.bigb.es/auxilia/steward" _ "modernc.org/sqlite"