TODO.md

@@ -27,6 +27,12 @@ returns 410; logout redirects back to login
 
 # Completed Steps
 
+- 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 fix the two remaining gosec findings (G124 in
   internal/session): session cookies now always carry
   Secure/HttpOnly/SameSite=Strict on both the set and clear paths;
@@ -55,7 +61,6 @@ returns 410; logout redirects back to login
 
 - P0: implement cache size management and eviction so the disk cannot
   fill up
-- 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

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"
 

internal/config/config.go

@@ -4,8 +4,12 @@ package config
 import (
 	"fmt"
 	"log/slog"
+	"math"
+	"net/url"
 	"os"
 	"path/filepath"
+	"sort"
+	"strconv"
 	"strings"
 
 	"git.eeqj.de/sneak/smartconfig"
@@ -78,29 +82,47 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 
 // 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.
+// 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:                      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, "allowlist_hosts"),
-		AllowHTTP:                  getBool(sc, "allow_http", false),
-		UpstreamConnectionsPerHost: getInt(sc, "upstream_connections_per_host", DefaultUpstreamConnectionsPerHost),
+		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, "allowlist_hosts"),
+		AllowHTTP:       loader.boolVal("allow_http", false),
+		UpstreamConnectionsPerHost: loader.intVal(
+			"upstream_connections_per_host", DefaultUpstreamConnectionsPerHost),
 	}
 
 	// Build DBURL from StateDir if not explicitly set
-	c.DBURL = getString(sc, "db_url", "")
+	c.DBURL = loader.stringVal("db_url", "")
 	if c.DBURL == "" {
 		c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_journal_mode=WAL", c.StateDir)
 	}
 
-	// Validate required configuration
+	if loader.err != nil {
+		return nil, loader.err
+	}
+
 	if err := c.validate(); err != nil {
 		return nil, err
 	}
@@ -108,14 +130,92 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 	return c, nil
 }
 
+// validateKnownKeys rejects configuration files containing keys the
+// application does not understand, so typos fail at startup instead of
+// being silently ignored. The env section is permitted because
+// smartconfig consumes it for environment variable injection.
+func validateKnownKeys(sc *smartconfig.Config) error {
+	var unknown []string
+
+	for key, value := range sc.Data() {
+		if !isKnownConfigKey(key) {
+			unknown = append(unknown, 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 := range metricsMap {
+				if subkey != "username" && subkey != "password" {
+					unknown = append(unknown, "metrics."+subkey)
+				}
+			}
+		}
+	}
+
+	if len(unknown) > 0 {
+		sort.Strings(unknown)
+
+		return fmt.Errorf("unknown config keys: %s", strings.Join(unknown, ", "))
+	}
+
+	return nil
+}
+
+// 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)
+	}
+
+	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.
+// validate checks that all required configuration values are set and
+// that every value is within its valid range.
 func (c *Config) validate() error {
 	if c.SigningKey == "" {
 		return fmt.Errorf("signing_key is required")
@@ -124,7 +224,55 @@ func (c *Config) validate() error {
 	// Minimum key length for security (32 bytes = 256 bits)
 	const minKeyLength = 32
 	if len(c.SigningKey) < minKeyLength {
-		return fmt.Errorf("signing_key must be at least %d characters", minKeyLength)
+		return fmt.Errorf("signing_key must be at least %d characters, got %d",
+			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.
+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)
 	}
 
 	return nil
@@ -158,11 +306,11 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro
 		cleanPath := filepath.Clean(path)
 		//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)
@@ -174,45 +322,189 @@ 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 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 || raw == nil {
+		return defaultVal, nil
+	}
+
+	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 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 || raw == nil {
+		return defaultVal, nil
+	}
+
+	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) 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 || raw == nil {
+		return defaultVal, nil
+	}
+
+	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: 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 || raw == nil {
+		return nil
+	}
+
+	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 list of strings for key, 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, key string) []string {
 	if sc == nil {
 		return nil

internal/config/config_validation_test.go

@@ -231,8 +231,8 @@ func TestSetButInvalidValueAbortsStartup(t *testing.T) {
 			},
 		},
 		{
-			name: "allowlist host with whitespace",
-			yaml: signingKeyLine + "allowlist_hosts:\n  - \"exa mple.com\"\n",
+			name:              "allowlist host with whitespace",
+			yaml:              signingKeyLine + "allowlist_hosts:\n  - \"exa mple.com\"\n",
 			wantErrSubstrings: []string{"allowlist_hosts", "exa mple.com"},
 		},
 		{