diff --git a/internal/config/config.go b/internal/config/config.go index a2c3778..a36e8ad 100644 --- a/internal/config/config.go +++ b/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 == "" { diff --git a/internal/config/config_validation_test.go b/internal/config/config_validation_test.go new file mode 100644 index 0000000..7471c92 --- /dev/null +++ b/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()) + } +}