internal/config/config.go

@@ -60,6 +60,26 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 		log.Info("no config file found, using defaults")
 	}
 
+	c, err := newFromSmartConfig(sc)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := c.ensureStateDirWritable(); err != nil {
+		return nil, err
+	}
+
+	if c.Debug {
+		params.Logger.EnableDebugLogging()
+	}
+
+	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.
+func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 	c := &Config{
 		Debug:                      getBool(sc, "debug", false),
 		MaintenanceMode:            getBool(sc, "maintenance_mode", false),
@@ -80,10 +100,6 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 		c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_journal_mode=WAL", c.StateDir)
 	}
 
-	if c.Debug {
-		params.Logger.EnableDebugLogging()
-	}
-
 	// Validate required configuration
 	if err := c.validate(); err != nil {
 		return nil, err
@@ -92,6 +108,13 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 	return c, nil
 }
 
+// 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 {
+	return nil
+}
+
 // validate checks that all required configuration values are set.
 func (c *Config) validate() error {
 	if c.SigningKey == "" {

internal/config/config_validation_test.go

@@ -0,0 +1,399 @@
+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)
+				}
+			}
+		})
+	}
+}
+
+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())
+	}
+}