diff --git a/TODO.md b/TODO.md index 4c9ba84..eb7dec6 100644 --- a/TODO.md +++ b/TODO.md @@ -34,6 +34,12 @@ fill up `testpackage` — white-box test files renamed to `*_internal_test.go`); three `//nolint:tagliatelle` directives keep the snake_case JSON wire/disk formats unchanged; `make check` green +- 2026-08-07 validate configuration on startup, fail fast on bad + config (closes #52): a config value that is set but unparseable or + invalid aborts startup naming the key and value (defaults apply only + to omitted keys), unknown config keys abort startup, a malformed + config file aborts instead of being skipped, and `state_dir` is + verified creatable and writable before the listener binds - 2026-08-07 manual test pass of the auth and encrypted URL flows against a locally built and running `pixad` (built from `main` at `6573b9d`, port 18099, local throwaway config); all six checks @@ -83,7 +89,6 @@ fill up # Future Steps -- P0: validate configuration on startup, fail fast on bad config - P1: implement blocked networks configuration to extend SSRF protection - P1: rate limit global concurrent upstream fetches to prevent diff --git a/config.example.yml b/config.example.yml index d4295f6..900a0b6 100644 --- a/config.example.yml +++ b/config.example.yml @@ -9,7 +9,7 @@ maintenance_mode: false state_dir: ./data # Image proxy settings -# HMAC signing key for URL signatures (leave empty to require allowlist for all requests) +# HMAC signing key for URL signatures (required, at least 32 characters) # Generate with: openssl rand -base64 32 signing_key: "CHANGE_ME_generate_with_openssl_rand_base64_32" diff --git a/internal/config/config.go b/internal/config/config.go index 26d1762..f94e0f6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,11 +2,14 @@ package config import ( - "errors" "fmt" "log/slog" + "math" + "net/url" "os" "path/filepath" + "sort" + "strconv" "strings" "git.eeqj.de/sneak/smartconfig" @@ -25,17 +28,10 @@ const ( // Params defines dependencies for Config. type Params struct { fx.In - Globals *globals.Globals Logger *logger.Logger } -// Static validation errors. -var ( - errSigningKeyRequired = errors.New("signing_key is required") - errSigningKeyTooShort = errors.New("signing_key too short") -) - // Config holds application configuration values. type Config struct { Debug bool @@ -68,60 +64,277 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) { log.Info("no config file found, using defaults") } - c := &Config{ - Debug: getBool(sc, "debug", false), - MaintenanceMode: getBool(sc, "maintenance_mode", false), - Port: getInt(sc, "port", DefaultPort), - StateDir: getString(sc, "state_dir", DefaultStateDir), - SentryDSN: getString(sc, "sentry_dsn", ""), - MetricsUsername: getString(sc, "metrics.username", ""), - MetricsPassword: getString(sc, "metrics.password", ""), - SigningKey: getString(sc, "signing_key", ""), - AllowlistHosts: getStringSlice(sc), - AllowHTTP: getBool(sc, "allow_http", false), - UpstreamConnectionsPerHost: getInt( - sc, "upstream_connections_per_host", DefaultUpstreamConnectionsPerHost, - ), + c, err := newFromSmartConfig(sc) + if err != nil { + return nil, err } - // Build DBURL from StateDir if not explicitly set - c.DBURL = getString(sc, "db_url", "") - if c.DBURL == "" { - c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_journal_mode=WAL", c.StateDir) + if err := c.ensureStateDirWritable(); err != nil { + return nil, err } if c.Debug { params.Logger.EnableDebugLogging() } - // Validate required configuration - err = c.validate() - if err != nil { + return c, nil +} + +// newFromSmartConfig constructs a Config from a loaded smartconfig +// instance and validates it. A nil sc means no config file was found, +// in which case every option takes its default value. A key that is +// present but unparseable or invalid is an error: defaults apply only +// to omitted keys, never to invalid explicit values. +func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { + if sc != nil { + if err := validateKnownKeys(sc); err != nil { + return nil, err + } + + if err := validateAllowlistHostsValue(sc); err != nil { + return nil, err + } + } + + loader := &strictLoader{sc: sc} + + c := &Config{ + Debug: loader.boolVal("debug", false), + MaintenanceMode: loader.boolVal("maintenance_mode", false), + Port: loader.intVal("port", DefaultPort), + StateDir: loader.stringVal("state_dir", DefaultStateDir), + SentryDSN: loader.stringVal("sentry_dsn", ""), + MetricsUsername: loader.stringVal("metrics.username", ""), + MetricsPassword: loader.stringVal("metrics.password", ""), + SigningKey: loader.stringVal("signing_key", ""), + AllowlistHosts: getStringSlice(sc), + AllowHTTP: loader.boolVal("allow_http", false), + UpstreamConnectionsPerHost: loader.intVal( + "upstream_connections_per_host", DefaultUpstreamConnectionsPerHost), + } + + // Build DBURL from StateDir if not explicitly set. The derived URL + // is a default: it applies only when db_url is omitted, never to an + // explicitly empty value. + c.DBURL = loader.stringVal("db_url", "") + if c.DBURL == "" && loader.err == nil { + if sc != nil { + if _, present := sc.Get("db_url"); present { + return nil, fmt.Errorf( + "config key %q: value must not be empty; omit the key to derive it from state_dir", + "db_url") + } + } + + c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_journal_mode=WAL", c.StateDir) + } + + if loader.err != nil { + return nil, loader.err + } + + if err := c.validate(); err != nil { return nil, err } return c, nil } -// validate checks that all required configuration values are set. -func (c *Config) validate() error { - if c.SigningKey == "" { - return errSigningKeyRequired +// validateKnownKeys rejects configuration files containing keys the +// application does not understand, so typos fail at startup instead of +// being silently ignored, and rejects keys that are explicitly set to +// null: a null is a SET value, never an omission, so it must not +// silently take the default. The env section is permitted because +// smartconfig consumes it for environment variable injection. +func validateKnownKeys(sc *smartconfig.Config) error { + var unknown, nullKeys []string + + for key, value := range sc.Data() { + if !isKnownConfigKey(key) { + unknown = append(unknown, key) + + continue + } + + if value == nil { + nullKeys = append(nullKeys, key) + + continue + } + + if key == "metrics" { + metricsMap, ok := value.(map[string]interface{}) + if !ok { + return fmt.Errorf( + "config key %q: value %v is not a map of metrics settings", + "metrics", value) + } + + for subkey, subvalue := range metricsMap { + if subkey != "username" && subkey != "password" { + unknown = append(unknown, "metrics."+subkey) + + continue + } + + if subvalue == nil { + nullKeys = append(nullKeys, "metrics."+subkey) + } + } + } } - // Minimum key length for security (32 bytes = 256 bits) - const minKeyLength = 32 - if len(c.SigningKey) < minKeyLength { + if len(unknown) > 0 { + sort.Strings(unknown) + + return fmt.Errorf("unknown config keys: %s", strings.Join(unknown, ", ")) + } + + if len(nullKeys) > 0 { + sort.Strings(nullKeys) + + if len(nullKeys) == 1 { + return errNullConfigValue(nullKeys[0]) + } + return fmt.Errorf( - "%w: must be at least %d characters", errSigningKeyTooShort, minKeyLength, - ) + "config keys %s: value is null; omit a key entirely to use its default", + strings.Join(nullKeys, ", ")) } return nil } -// loadConfigFile loads configuration from the PIXA_CONFIG_PATH env var -// or standard locations. +// errNullConfigValue reports a config key that is explicitly set to +// null (including the bare "key:" form and the "~" alias). Silently +// applying the default would mask a truncated or typo'd config entry. +func errNullConfigValue(key string) error { + return fmt.Errorf( + "config key %q: value is null; omit the key entirely to use the default", key) +} + +// isKnownConfigKey reports whether key is a permitted top-level +// configuration key. +func isKnownConfigKey(key string) bool { + switch key { + case "debug", "maintenance_mode", "port", "state_dir", "sentry_dsn", + "db_url", "metrics", "signing_key", "allowlist_hosts", "allow_http", + "upstream_connections_per_host", "env": + return true + } + + return false +} + +// ensureStateDirWritable verifies at startup that StateDir can be +// created and written to, so a misconfigured path aborts startup +// instead of failing later at first use. +func (c *Config) ensureStateDirWritable() error { + const stateDirPerms = 0o750 + + if err := os.MkdirAll(c.StateDir, stateDirPerms); err != nil { + return fmt.Errorf("config key %q: cannot create directory %q: %w", + "state_dir", c.StateDir, err) + } + + probe, err := os.CreateTemp(c.StateDir, ".startup-write-probe-*") + if err != nil { + return fmt.Errorf("config key %q: directory %q is not writable: %w", + "state_dir", c.StateDir, err) + } + + probePath := probe.Name() + + if err := probe.Close(); err != nil { + return fmt.Errorf("config key %q: cannot close probe file %q: %w", + "state_dir", probePath, err) + } + + //nolint:gosec // G703: probePath comes from os.CreateTemp inside the just-validated StateDir + if err := os.Remove(probePath); err != nil { + return fmt.Errorf("config key %q: cannot remove probe file %q: %w", + "state_dir", probePath, err) + } + + return nil +} + +// validate checks that all required configuration values are set and +// that every value is within its valid range. +func (c *Config) validate() error { + // The signing key value is never echoed in error messages. + if c.SigningKey == "" { + return fmt.Errorf("config key %q: a value is required", "signing_key") + } + + // Minimum key length for security (32 bytes = 256 bits) + const minKeyLength = 32 + if len(c.SigningKey) < minKeyLength { + return fmt.Errorf("config key %q: value must be at least %d characters, got %d", + "signing_key", minKeyLength, len(c.SigningKey)) + } + + const maxPort = 65535 + if c.Port < 1 || c.Port > maxPort { + return fmt.Errorf("config key %q: value %d is outside the valid port range 1-%d", + "port", c.Port, maxPort) + } + + if c.UpstreamConnectionsPerHost < 1 { + return fmt.Errorf("config key %q: value %d must be at least 1", + "upstream_connections_per_host", c.UpstreamConnectionsPerHost) + } + + if c.StateDir == "" { + return fmt.Errorf("config key %q: value must not be empty", "state_dir") + } + + for _, host := range c.AllowlistHosts { + if err := validateAllowlistHost(host); err != nil { + return err + } + } + + if c.SentryDSN != "" { + parsed, err := url.Parse(c.SentryDSN) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return fmt.Errorf("config key %q: value %q is not a valid URL", + "sentry_dsn", c.SentryDSN) + } + } + + if (c.MetricsUsername == "") != (c.MetricsPassword == "") { + return fmt.Errorf("config keys %q and %q must be set together", + "metrics.username", "metrics.password") + } + + return nil +} + +// validateAllowlistHost checks that an allowlist_hosts entry is a bare +// hostname, optionally with a leading dot for suffix matching. URLs, +// paths, and whitespace indicate a misconfigured entry. An entry with +// no hostname labels (such as ".") is rejected: the allowlist matcher +// treats a leading dot as a suffix pattern, so a bare "." would match +// any upstream host written in FQDN trailing-dot form and effectively +// disable URL signing. +func validateAllowlistHost(host string) error { + if strings.Contains(host, "://") || strings.ContainsAny(host, "/ \t") { + return fmt.Errorf( + "config key %q: entry %q must be a bare hostname without scheme, path, or whitespace", + "allowlist_hosts", host) + } + + if strings.Trim(host, ".") == "" { + return fmt.Errorf( + "config key %q: entry %q contains no hostname labels", + "allowlist_hosts", host) + } + + return nil +} + +// loadConfigFile loads configuration from PIXA_CONFIG_PATH env var or standard locations. func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, error) { // Check for explicit config path from environment if envPath := os.Getenv("PIXA_CONFIG_PATH"); envPath != "" { @@ -147,14 +360,13 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro for _, path := range configPaths { cleanPath := filepath.Clean(path) - - _, statErr := os.Stat(cleanPath) - if statErr == nil { + //nolint:gosec // G703: paths are hardcoded config locations + if _, statErr := os.Stat(cleanPath); statErr == nil { + // A config file that exists but does not parse is a fatal + // startup error, never something to skip over. sc, err := smartconfig.NewFromConfigPath(path) if err != nil { - log.Warn("failed to parse config file", "path", path, "error", err) - - continue + return nil, fmt.Errorf("failed to parse config file %s: %w", path, err) } log.Info("loaded config file", "path", path) @@ -166,45 +378,207 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro return nil, nil //nolint:nilnil // nil config is valid (use defaults) } -func getString(sc *smartconfig.Config, key, defaultVal string) string { - if sc == nil { - return defaultVal +// strictLoader accumulates the first error encountered while reading +// typed values out of a smartconfig instance, so Config construction +// can stay a single struct literal. +type strictLoader struct { + sc *smartconfig.Config + err error +} + +func (l *strictLoader) stringVal(key, defaultVal string) string { + if l.err != nil { + return "" } - val, err := sc.GetString(key) + val, err := getString(l.sc, key, defaultVal) if err != nil { - return defaultVal + l.err = err } return val } -func getInt(sc *smartconfig.Config, key string, defaultVal int) int { - if sc == nil { - return defaultVal +func (l *strictLoader) intVal(key string, defaultVal int) int { + if l.err != nil { + return 0 } - val, err := sc.GetInt(key) + val, err := getInt(l.sc, key, defaultVal) if err != nil { - return defaultVal + l.err = err } return val } -func getBool(sc *smartconfig.Config, key string, defaultVal bool) bool { - if sc == nil { - return defaultVal +func (l *strictLoader) boolVal(key string, defaultVal bool) bool { + if l.err != nil { + return false } - val, err := sc.GetBool(key) + val, err := getBool(l.sc, key, defaultVal) if err != nil { - return defaultVal + l.err = err } return val } +// getString returns the string value for key, or defaultVal if the key +// is omitted. A present value that is not a string, or is explicitly +// null, is an error. +func getString(sc *smartconfig.Config, key, defaultVal string) (string, error) { + if sc == nil { + return defaultVal, nil + } + + raw, ok := sc.Get(key) + if !ok { + return defaultVal, nil + } + + if raw == nil { + return "", errNullConfigValue(key) + } + + str, ok := raw.(string) + if !ok { + return "", fmt.Errorf("config key %q: value %v (%T) is not a string", + key, raw, raw) + } + + return str, nil +} + +// getInt returns the integer value for key, or defaultVal if the key is +// omitted. A present value that is not a whole number, or is explicitly +// null, is an error; fractional values are never truncated. +func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) { + if sc == nil { + return defaultVal, nil + } + + raw, ok := sc.Get(key) + if !ok { + return defaultVal, nil + } + + if raw == nil { + return 0, errNullConfigValue(key) + } + + switch val := raw.(type) { + case int: + return val, nil + case int64: + return int(val), nil + case float64: + if val != math.Trunc(val) { + return 0, fmt.Errorf("config key %q: value %v is not an integer", key, val) + } + + return int(val), nil + case string: + parsed, err := strconv.Atoi(strings.TrimSpace(val)) + if err != nil { + return 0, fmt.Errorf("config key %q: value %q is not an integer", key, val) + } + + return parsed, nil + default: + return 0, fmt.Errorf("config key %q: value %v (%T) is not an integer", + key, raw, raw) + } +} + +// getBool returns the boolean value for key, or defaultVal if the key +// is omitted. A present value that is not a boolean (or a ParseBool-able +// string), or is explicitly null, is an error; numbers are not accepted +// as booleans. +func getBool(sc *smartconfig.Config, key string, defaultVal bool) (bool, error) { + if sc == nil { + return defaultVal, nil + } + + raw, ok := sc.Get(key) + if !ok { + return defaultVal, nil + } + + if raw == nil { + return false, errNullConfigValue(key) + } + + switch val := raw.(type) { + case bool: + return val, nil + case string: + parsed, err := strconv.ParseBool(strings.TrimSpace(val)) + if err != nil { + return false, fmt.Errorf("config key %q: value %q is not a boolean", key, val) + } + + return parsed, nil + default: + return false, fmt.Errorf("config key %q: value %v (%T) is not a boolean", + key, raw, raw) + } +} + +// validateAllowlistHostsValue checks the raw shape of the +// allowlist_hosts value before the lenient extraction in getStringSlice +// runs: an explicitly null value, a value that is not a list of strings +// (or a comma-separated string), a non-string entry, or an empty entry +// is an error, never silently skipped. +func validateAllowlistHostsValue(sc *smartconfig.Config) error { + const key = "allowlist_hosts" + + raw, ok := sc.Get(key) + if !ok { + return nil + } + + if raw == nil { + return errNullConfigValue(key) + } + + switch val := raw.(type) { + case []interface{}: + for _, item := range val { + str, ok := item.(string) + if !ok { + return fmt.Errorf( + "config key %q: list entry %v (%T) is not a string", key, item, item) + } + + if strings.TrimSpace(str) == "" { + return fmt.Errorf("config key %q: list contains an empty entry", key) + } + } + case string: + if strings.TrimSpace(val) == "" { + return nil + } + + for _, part := range strings.Split(val, ",") { + if strings.TrimSpace(part) == "" { + return fmt.Errorf( + "config key %q: value %q contains an empty entry", key, val) + } + } + default: + return fmt.Errorf("config key %q: value %v (%T) is not a list of strings", + key, raw, raw) + } + + return nil +} + +// getStringSlice returns the allowlist_hosts list of strings, or nil if +// the key is omitted. It accepts a YAML list of strings or a +// comma-separated string (backwards compatibility). Malformed entries +// are rejected beforehand by validateAllowlistHostsValue. func getStringSlice(sc *smartconfig.Config) []string { if sc == nil { return nil @@ -216,7 +590,7 @@ func getStringSlice(sc *smartconfig.Config) []string { } // Handle YAML list format - if slice, ok := val.([]any); ok { + if slice, ok := val.([]interface{}); ok { result := make([]string, 0, len(slice)) for _, item := range slice { if str, ok := item.(string); ok { diff --git a/internal/config/config_validation_test.go b/internal/config/config_validation_test.go new file mode 100644 index 0000000..127c0d7 --- /dev/null +++ b/internal/config/config_validation_test.go @@ -0,0 +1,547 @@ +package config + +import ( + "io" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "git.eeqj.de/sneak/smartconfig" +) + +// validTestSigningKey is a 32-character signing key that satisfies the +// minimum length requirement in validate(). +const validTestSigningKey = "0123456789abcdef0123456789abcdef" + +// configFromYAML writes yamlContent to a temporary config file, loads it +// via smartconfig, and constructs a Config from it using the same code +// path the server uses at startup. +func configFromYAML(t *testing.T, yamlContent string) (*Config, error) { + t.Helper() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + + if err := os.WriteFile(configPath, []byte(yamlContent), 0o600); err != nil { + t.Fatalf("failed to write test config: %v", err) + } + + sc, err := smartconfig.NewFromConfigPath(configPath) + if err != nil { + t.Fatalf("failed to load test config: %v", err) + } + + return newFromSmartConfig(sc) +} + +func TestOmittedValuesUseDefaults(t *testing.T) { + c, err := configFromYAML(t, "signing_key: "+validTestSigningKey+"\n") + if err != nil { + t.Fatalf("minimal config should be valid, got error: %v", err) + } + + if c.Port != DefaultPort { + t.Errorf("Port = %d, want default %d", c.Port, DefaultPort) + } + + if c.StateDir != DefaultStateDir { + t.Errorf("StateDir = %q, want default %q", c.StateDir, DefaultStateDir) + } + + if c.UpstreamConnectionsPerHost != DefaultUpstreamConnectionsPerHost { + t.Errorf("UpstreamConnectionsPerHost = %d, want default %d", + c.UpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost) + } + + if c.Debug { + t.Error("Debug = true, want default false") + } + + if c.MaintenanceMode { + t.Error("MaintenanceMode = true, want default false") + } + + if c.AllowHTTP { + t.Error("AllowHTTP = true, want default false") + } + + if len(c.AllowlistHosts) != 0 { + t.Errorf("AllowlistHosts = %v, want empty", c.AllowlistHosts) + } + + wantDBURL := "file:" + DefaultStateDir + "/state.sqlite3?_journal_mode=WAL" + if c.DBURL != wantDBURL { + t.Errorf("DBURL = %q, want derived default %q", c.DBURL, wantDBURL) + } +} + +func TestExplicitValidValuesAreUsed(t *testing.T) { + yamlContent := ` +port: 9090 +debug: true +maintenance_mode: true +state_dir: /tmp/pixa-test-state +db_url: "file:/tmp/pixa-test-state/other.sqlite3" +signing_key: ` + validTestSigningKey + ` +allowlist_hosts: + - s3.sneak.cloud + - .example.com +allow_http: true +upstream_connections_per_host: 5 +sentry_dsn: "https://abc123@sentry.example.com/42" +metrics: + username: metricsuser + password: metricspass +` + + c, err := configFromYAML(t, yamlContent) + if err != nil { + t.Fatalf("valid config should load, got error: %v", err) + } + + if c.Port != 9090 { + t.Errorf("Port = %d, want 9090", c.Port) + } + + if !c.Debug || !c.MaintenanceMode || !c.AllowHTTP { + t.Errorf("bool fields = debug %v maintenance %v allow_http %v, want all true", + c.Debug, c.MaintenanceMode, c.AllowHTTP) + } + + if c.StateDir != "/tmp/pixa-test-state" { + t.Errorf("StateDir = %q, want /tmp/pixa-test-state", c.StateDir) + } + + if c.DBURL != "file:/tmp/pixa-test-state/other.sqlite3" { + t.Errorf("DBURL = %q, want explicit value", c.DBURL) + } + + if len(c.AllowlistHosts) != 2 || c.AllowlistHosts[0] != "s3.sneak.cloud" || + c.AllowlistHosts[1] != ".example.com" { + t.Errorf("AllowlistHosts = %v, want [s3.sneak.cloud .example.com]", c.AllowlistHosts) + } + + if c.UpstreamConnectionsPerHost != 5 { + t.Errorf("UpstreamConnectionsPerHost = %d, want 5", c.UpstreamConnectionsPerHost) + } + + if c.SentryDSN != "https://abc123@sentry.example.com/42" { + t.Errorf("SentryDSN = %q, want explicit value", c.SentryDSN) + } + + if c.MetricsUsername != "metricsuser" || c.MetricsPassword != "metricspass" { + t.Errorf("metrics = %q/%q, want metricsuser/metricspass", + c.MetricsUsername, c.MetricsPassword) + } +} + +func TestCommaSeparatedAllowlistStillSupported(t *testing.T) { + yamlContent := `signing_key: ` + validTestSigningKey + ` +allowlist_hosts: "s3.sneak.cloud, sneak.berlin" +` + + c, err := configFromYAML(t, yamlContent) + if err != nil { + t.Fatalf("comma-separated allowlist should load, got error: %v", err) + } + + if len(c.AllowlistHosts) != 2 || c.AllowlistHosts[0] != "s3.sneak.cloud" || + c.AllowlistHosts[1] != "sneak.berlin" { + t.Errorf("AllowlistHosts = %v, want [s3.sneak.cloud sneak.berlin]", c.AllowlistHosts) + } +} + +// TestSetButInvalidValueAbortsStartup verifies the no-silent-fallback +// rule: a key that is explicitly set to an unparseable or out-of-range +// value must produce a startup error naming the offending key, never +// silently fall back to the default. +func TestSetButInvalidValueAbortsStartup(t *testing.T) { + signingKeyLine := "signing_key: " + validTestSigningKey + "\n" + + cases := []struct { + name string + yaml string + // wantErrSubstrings must all appear in the error message. + wantErrSubstrings []string + }{ + { + name: "port not a number", + yaml: signingKeyLine + "port: banana\n", + wantErrSubstrings: []string{"port", "banana"}, + }, + { + name: "port zero", + yaml: signingKeyLine + "port: 0\n", + wantErrSubstrings: []string{"port", "0"}, + }, + { + name: "port above 65535", + yaml: signingKeyLine + "port: 99999\n", + wantErrSubstrings: []string{"port", "99999"}, + }, + { + name: "port fractional", + yaml: signingKeyLine + "port: 8080.5\n", + wantErrSubstrings: []string{"port", "8080.5"}, + }, + { + name: "debug not a bool", + yaml: signingKeyLine + "debug: notabool\n", + wantErrSubstrings: []string{"debug", "notabool"}, + }, + { + name: "maintenance_mode not a bool", + yaml: signingKeyLine + "maintenance_mode: sometimes\n", + wantErrSubstrings: []string{"maintenance_mode", "sometimes"}, + }, + { + name: "allow_http numeric", + yaml: signingKeyLine + "allow_http: 2\n", + wantErrSubstrings: []string{"allow_http", "2"}, + }, + { + name: "upstream_connections_per_host zero", + yaml: signingKeyLine + "upstream_connections_per_host: 0\n", + wantErrSubstrings: []string{"upstream_connections_per_host", "0"}, + }, + { + name: "upstream_connections_per_host negative", + yaml: signingKeyLine + "upstream_connections_per_host: -3\n", + wantErrSubstrings: []string{"upstream_connections_per_host", "-3"}, + }, + { + name: "upstream_connections_per_host not a number", + yaml: signingKeyLine + "upstream_connections_per_host: many\n", + wantErrSubstrings: []string{"upstream_connections_per_host", "many"}, + }, + { + name: "allowlist host with scheme", + yaml: signingKeyLine + "allowlist_hosts:\n - https://example.com\n", + wantErrSubstrings: []string{ + "allowlist_hosts", "https://example.com", + }, + }, + { + name: "allowlist host with path", + yaml: signingKeyLine + "allowlist_hosts:\n - example.com/images\n", + wantErrSubstrings: []string{ + "allowlist_hosts", "example.com/images", + }, + }, + { + name: "allowlist host with whitespace", + yaml: signingKeyLine + "allowlist_hosts:\n - \"exa mple.com\"\n", + wantErrSubstrings: []string{"allowlist_hosts", "exa mple.com"}, + }, + { + name: "allowlist entry not a string", + yaml: signingKeyLine + "allowlist_hosts:\n - 123\n", + wantErrSubstrings: []string{"allowlist_hosts", "123"}, + }, + { + name: "allowlist not a list", + yaml: signingKeyLine + "allowlist_hosts:\n key: value\n", + wantErrSubstrings: []string{"allowlist_hosts"}, + }, + { + name: "signing_key too short", + yaml: "signing_key: short\n", + wantErrSubstrings: []string{"signing_key"}, + }, + { + name: "signing_key missing", + yaml: "port: 8080\n", + wantErrSubstrings: []string{"signing_key"}, + }, + { + name: "state_dir explicitly empty", + yaml: signingKeyLine + "state_dir: \"\"\n", + wantErrSubstrings: []string{"state_dir"}, + }, + { + name: "sentry_dsn not a URL", + yaml: signingKeyLine + "sentry_dsn: \"not a url\"\n", + wantErrSubstrings: []string{"sentry_dsn", "not a url"}, + }, + { + name: "metrics username without password", + yaml: signingKeyLine + "metrics:\n username: bob\n", + wantErrSubstrings: []string{"metrics"}, + }, + { + name: "metrics password without username", + yaml: signingKeyLine + "metrics:\n password: hunter2\n", + wantErrSubstrings: []string{"metrics"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := configFromYAML(t, tc.yaml) + if err == nil { + t.Fatalf("config with %s must abort startup, got config: %+v", tc.name, c) + } + + t.Logf("got expected error: %v", err) + + for _, want := range tc.wantErrSubstrings { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } + }) + } +} + +// TestExplicitNullValueAbortsStartup verifies that a key explicitly +// set to null (including the bare "key:" form and the "~" alias) aborts +// startup naming the key. An explicit null is a SET value: it must +// never silently fall back to the default the way an omitted key does. +func TestExplicitNullValueAbortsStartup(t *testing.T) { + signingKeyLine := "signing_key: " + validTestSigningKey + "\n" + + cases := []struct { + name string + yaml string + // wantErrSubstrings must all appear in the error message. + wantErrSubstrings []string + }{ + { + name: "port explicit null", + yaml: signingKeyLine + "port: null\n", + wantErrSubstrings: []string{"port", "null"}, + }, + { + name: "port bare key no value", + yaml: signingKeyLine + "port:\n", + wantErrSubstrings: []string{"port", "null"}, + }, + { + name: "debug tilde null", + yaml: signingKeyLine + "debug: ~\n", + wantErrSubstrings: []string{"debug", "null"}, + }, + { + name: "maintenance_mode null", + yaml: signingKeyLine + "maintenance_mode: null\n", + wantErrSubstrings: []string{"maintenance_mode", "null"}, + }, + { + name: "allow_http null", + yaml: signingKeyLine + "allow_http: null\n", + wantErrSubstrings: []string{"allow_http", "null"}, + }, + { + name: "state_dir null", + yaml: signingKeyLine + "state_dir: null\n", + wantErrSubstrings: []string{"state_dir", "null"}, + }, + { + name: "db_url null", + yaml: signingKeyLine + "db_url: null\n", + wantErrSubstrings: []string{"db_url", "null"}, + }, + { + name: "sentry_dsn null", + yaml: signingKeyLine + "sentry_dsn: null\n", + wantErrSubstrings: []string{"sentry_dsn", "null"}, + }, + { + name: "upstream_connections_per_host null", + yaml: signingKeyLine + "upstream_connections_per_host: null\n", + wantErrSubstrings: []string{"upstream_connections_per_host", "null"}, + }, + { + name: "allowlist_hosts null", + yaml: signingKeyLine + "allowlist_hosts: null\n", + wantErrSubstrings: []string{"allowlist_hosts", "null"}, + }, + { + name: "signing_key null", + yaml: "signing_key: null\n", + wantErrSubstrings: []string{"signing_key", "null"}, + }, + { + name: "metrics null", + yaml: signingKeyLine + "metrics: null\n", + wantErrSubstrings: []string{"metrics", "null"}, + }, + { + name: "metrics subkeys null", + yaml: signingKeyLine + "metrics:\n username: null\n password: null\n", + wantErrSubstrings: []string{ + "metrics.username", "metrics.password", "null", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := configFromYAML(t, tc.yaml) + if err == nil { + t.Fatalf("config with %s must abort startup, got config: %+v", tc.name, c) + } + + t.Logf("got expected error: %v", err) + + for _, want := range tc.wantErrSubstrings { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } + }) + } +} + +// TestExplicitlyEmptyDBURLAbortsStartup verifies that db_url set to an +// empty string aborts startup: the derived file:...state.sqlite3 URL is +// a default, and defaults apply only to omitted keys. This matches +// state_dir, where an explicitly empty value already aborts. +func TestExplicitlyEmptyDBURLAbortsStartup(t *testing.T) { + yamlContent := "signing_key: " + validTestSigningKey + "\ndb_url: \"\"\n" + + c, err := configFromYAML(t, yamlContent) + if err == nil { + t.Fatalf("explicitly empty db_url must abort startup, got config: %+v", c) + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), "db_url") { + t.Errorf("error %q does not name the offending key db_url", err.Error()) + } +} + +// TestAllowlistHostsRejectsDotOnlyEntries verifies that entries with no +// hostname labels are rejected. The allowlist matcher treats a leading +// dot as a suffix pattern, so a bare "." entry would match any upstream +// host written in FQDN trailing-dot form (e.g. evil.com.) and +// effectively disable URL signing with a single character. +func TestAllowlistHostsRejectsDotOnlyEntries(t *testing.T) { + signingKeyLine := "signing_key: " + validTestSigningKey + "\n" + + for _, entry := range []string{".", ".."} { + t.Run(entry, func(t *testing.T) { + yamlContent := signingKeyLine + + "allowlist_hosts:\n - \"" + entry + "\"\n" + + c, err := configFromYAML(t, yamlContent) + if err == nil { + t.Fatalf("allowlist entry %q must abort startup, got config: %+v", + entry, c) + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), "allowlist_hosts") { + t.Errorf("error %q does not name the offending key allowlist_hosts", + err.Error()) + } + }) + } +} + +func TestUnknownTopLevelKeyAbortsStartup(t *testing.T) { + yamlContent := `signing_key: ` + validTestSigningKey + ` +whitelist_hosts: + - example.com +` + + c, err := configFromYAML(t, yamlContent) + if err == nil { + t.Fatalf("config with unknown key must abort startup, got config: %+v", c) + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), "whitelist_hosts") { + t.Errorf("error %q does not name the unknown key whitelist_hosts", err.Error()) + } +} + +func TestUnknownMetricsSubkeyAbortsStartup(t *testing.T) { + yamlContent := `signing_key: ` + validTestSigningKey + ` +metrics: + username: bob + password: hunter2 + port: 9100 +` + + c, err := configFromYAML(t, yamlContent) + if err == nil { + t.Fatalf("config with unknown metrics subkey must abort startup, got config: %+v", c) + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), "metrics.port") { + t.Errorf("error %q does not name the unknown key metrics.port", err.Error()) + } +} + +func TestEnvSectionIsPermitted(t *testing.T) { + yamlContent := `signing_key: ` + validTestSigningKey + ` +env: + PIXA_TEST_ENV_INJECTION: injected +` + + if _, err := configFromYAML(t, yamlContent); err != nil { + t.Fatalf("env section must be permitted (smartconfig consumes it), got error: %v", err) + } +} + +func TestMalformedConfigFileAbortsStartup(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + + if err := os.WriteFile(configPath, []byte("port: [unclosed\n"), 0o600); err != nil { + t.Fatalf("failed to write malformed config: %v", err) + } + + // loadConfigFile falls through to the relative config.yml candidate; + // the appname is chosen so no /etc or $HOME candidate can exist. + t.Setenv("PIXA_CONFIG_PATH", "") + t.Chdir(tmpDir) + + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + + sc, err := loadConfigFile(log, "pixa-test-nonexistent-app") + if err == nil { + t.Fatalf("malformed config file must abort startup, got config: %v", sc) + } + + t.Logf("got expected error: %v", err) +} + +func TestEnsureStateDirCreatesDirectory(t *testing.T) { + stateDir := filepath.Join(t.TempDir(), "nested", "state") + + c := &Config{StateDir: stateDir} + if err := c.ensureStateDirWritable(); err != nil { + t.Fatalf("creatable state_dir must validate, got error: %v", err) + } + + info, err := os.Stat(stateDir) + if err != nil || !info.IsDir() { + t.Fatalf("state_dir was not created: info=%v err=%v", info, err) + } +} + +func TestEnsureStateDirFailsOnUncreatablePath(t *testing.T) { + // A path below /dev/null can never be created, even when running + // as root (as in the Docker build). + c := &Config{StateDir: "/dev/null/pixa-state"} + + err := c.ensureStateDirWritable() + if err == nil { + t.Fatal("uncreatable state_dir must abort startup, got nil error") + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), "state_dir") { + t.Errorf("error %q does not name the offending key state_dir", err.Error()) + } +}