From 39e7bc90673ec7b4fbd20068ade933d6054dd849 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 15:27:32 +0200 Subject: [PATCH] config: a bad number or true/false setting stops startup (closes #269) PORT and DNSWATCHER_TLS_EXPIRY_WARNING were read with viper.GetInt, and DNSWATCHER_DEBUG, DNSWATCHER_MAINTENANCE_MODE and DNSWATCHER_SEND_TEST_NOTIFICATION with viper.GetBool, which turn a value that does not parse into 0 or false with no error. Each is now parsed the way parseInterval parses an interval: PORT must be 1 to 65535, the expiry warning 1 or more days, the true/false settings a value strconv.ParseBool accepts. Anything else stops startup with an error naming the variable and the value; empty still means the default. Reading the config file moved, unchanged, from buildConfig into readConfigFile, so buildConfig stays within the linter's length limit. Model: opus-5-5 --- README.md | 10 +++ TODO.md | 2 + internal/config/config.go | 129 +++++++++++++++++++++++++++++---- internal/config/config_test.go | 95 +++++++++++++++++++++++- 4 files changed, 220 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 089ef95..7386c61 100644 --- a/README.md +++ b/README.md @@ -463,6 +463,16 @@ or empty variable (`DNSWATCHER_DNS_INTERVAL=`) means the default. If either is set to anything else, including a bare number or a zero or negative duration, dnswatcher refuses to start with an error naming the variable and the value. +**`PORT` and `DNSWATCHER_TLS_EXPIRY_WARNING`** take a whole number: 1 to 65535 +for `PORT`, and a number of days, 1 or more, for +`DNSWATCHER_TLS_EXPIRY_WARNING`. `DNSWATCHER_DEBUG`, +`DNSWATCHER_MAINTENANCE_MODE` and `DNSWATCHER_SEND_TEST_NOTIFICATION` take +`true` or `false`; `1`, `t`, `T`, `TRUE` and `True` also mean true, and `0`, +`f`, `F`, `FALSE` and `False` also mean false. An unset or empty variable means +the default. Any other value, such as `PORT=http`, +`DNSWATCHER_TLS_EXPIRY_WARNING=7d` or `DNSWATCHER_DEBUG=yes`, makes dnswatcher +refuse to start with an error naming the variable and the value. + **`DNSWATCHER_SENTRY_DSN` reports crashes in HTTP requests to Sentry.** When it is set, a panic in an HTTP request handler is sent to Sentry, and the request still gets a `500 Internal Server Error` answer. Nothing else is sent to Sentry: diff --git a/TODO.md b/TODO.md index 5c3c05e..368b81f 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false + setting stops startup instead of being read as 0 or false (closes #269). - 2026-10-13: a check saving the state while the dashboard or `/api/v1/status` is read cannot crash the process (closes #261). - 2026-10-06: canonical files re-fetched from `sneak/prompts` at `dd4027b`; lint diff --git a/internal/config/config.go b/internal/config/config.go index 2f82f40..5e7b30b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "slices" + "strconv" "strings" "time" @@ -24,6 +25,9 @@ const ( defaultTLSExpiryWarning = 7 ) +// maxPort is the highest TCP port number. +const maxPort = 65535 + // ErrNoTargets is returned when no monitoring targets are configured. var ErrNoTargets = errors.New( "no monitoring targets configured: set DNSWATCHER_TARGETS environment variable", @@ -40,6 +44,23 @@ var ErrInvalidInterval = errors.New( // lists a name that is not in DNSWATCHER_TARGETS. var ErrNotInTargets = errors.New("name is not in DNSWATCHER_TARGETS") +// ErrInvalidPort is returned when PORT is set but is not a whole number +// from 1 to 65535. An empty value counts as unset and means the default. +var ErrInvalidPort = errors.New("port must be a whole number from 1 to 65535") + +// ErrInvalidTLSExpiryWarning is returned when DNSWATCHER_TLS_EXPIRY_WARNING +// is set but is not a whole number, 1 or more. An empty value counts as unset +// and means the default. +var ErrInvalidTLSExpiryWarning = errors.New( + "TLS expiry warning must be a whole number of days, 1 or more", +) + +// ErrInvalidBool is returned when DNSWATCHER_DEBUG, +// DNSWATCHER_MAINTENANCE_MODE or DNSWATCHER_SEND_TEST_NOTIFICATION is set +// but is not a value strconv.ParseBool accepts. An empty value counts as +// unset and means the default. +var ErrInvalidBool = errors.New("value must be true or false") + // Params contains dependencies for Config. type Params struct { fx.In @@ -127,16 +148,9 @@ func buildConfig( log *slog.Logger, params *Params, ) (*Config, error) { - err := viper.ReadInConfig() + err := readConfigFile(log) if err != nil { - var notFound viper.ConfigFileNotFoundError - if !errors.As(err, ¬Found) { - log.Error("config file malformed", "error", err) - - return nil, fmt.Errorf( - "config file malformed: %w", err, - ) - } + return nil, err } dnsInterval, err := parseInterval("DNS_INTERVAL") @@ -161,9 +175,34 @@ func buildConfig( return nil, err } + port, err := parsePort() + if err != nil { + return nil, err + } + + tlsExpiryWarning, err := parseTLSExpiryWarning() + if err != nil { + return nil, err + } + + debug, err := parseBool("DEBUG") + if err != nil { + return nil, err + } + + maintenanceMode, err := parseBool("MAINTENANCE_MODE") + if err != nil { + return nil, err + } + + sendTestNotification, err := parseBool("SEND_TEST_NOTIFICATION") + if err != nil { + return nil, err + } + cfg := &Config{ - Port: viper.GetInt("PORT"), - Debug: viper.GetBool("DEBUG"), + Port: port, + Debug: debug, DataDir: viper.GetString("DATA_DIR"), Domains: domains, Hostnames: hostnames, @@ -173,12 +212,12 @@ func buildConfig( NtfyTopic: viper.GetString("NTFY_TOPIC"), DNSInterval: dnsInterval, TLSInterval: tlsInterval, - TLSExpiryWarning: viper.GetInt("TLS_EXPIRY_WARNING"), + TLSExpiryWarning: tlsExpiryWarning, SentryDSN: viper.GetString("SENTRY_DSN"), - MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"), + MaintenanceMode: maintenanceMode, MetricsUsername: viper.GetString("METRICS_USERNAME"), MetricsPassword: viper.GetString("METRICS_PASSWORD"), - SendTestNotification: viper.GetBool("SEND_TEST_NOTIFICATION"), + SendTestNotification: sendTestNotification, params: params, log: log, } @@ -186,6 +225,21 @@ func buildConfig( return cfg, nil } +// readConfigFile reads the config file, if there is one. +func readConfigFile(log *slog.Logger) error { + err := viper.ReadInConfig() + if err != nil { + var notFound viper.ConfigFileNotFoundError + if !errors.As(err, ¬Found) { + log.Error("config file malformed", "error", err) + + return fmt.Errorf("config file malformed: %w", err) + } + } + + return nil +} + // parseInterval reads the DNSWATCHER_-prefixed setting key as a duration. A // value that does not parse, or is zero or negative, is an error naming the // variable and the value; an unset variable has its default from setupViper. @@ -202,6 +256,53 @@ func parseInterval(key string) (time.Duration, error) { return interval, nil } +// parsePort reads PORT, which has no DNSWATCHER_ prefix, as a port number. A +// value that is not a whole number from 1 to 65535 is an error naming the +// variable and the value; an unset variable has its default from setupViper. +func parsePort() (int, error) { + value := viper.GetString("PORT") + + port, err := strconv.Atoi(value) + if err != nil || port < 1 || port > maxPort { + return 0, fmt.Errorf("invalid PORT %q: %w", value, ErrInvalidPort) + } + + return port, nil +} + +// parseTLSExpiryWarning reads DNSWATCHER_TLS_EXPIRY_WARNING as a number of +// days. A value that is not a whole number, 1 or more, is an error naming the +// variable and the value; an unset variable has its default from setupViper. +func parseTLSExpiryWarning() (int, error) { + value := viper.GetString("TLS_EXPIRY_WARNING") + + days, err := strconv.Atoi(value) + if err != nil || days < 1 { + return 0, fmt.Errorf( + "invalid DNSWATCHER_TLS_EXPIRY_WARNING %q: %w", + value, ErrInvalidTLSExpiryWarning, + ) + } + + return days, nil +} + +// parseBool reads the DNSWATCHER_-prefixed setting key as true or false. A +// value strconv.ParseBool does not accept is an error naming the variable and +// the value; an unset variable has its default from setupViper. +func parseBool(key string) (bool, error) { + value := viper.GetString(key) + + result, err := strconv.ParseBool(value) + if err != nil { + return false, fmt.Errorf( + "invalid DNSWATCHER_%s %q: %w", key, value, ErrInvalidBool, + ) + } + + return result, nil +} + func parseAndValidateTargets() ([]string, []string, error) { domains, hostnames, err := ClassifyTargets( parseCSV(viper.GetString("TARGETS")), diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ffb93ab..0c44726 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -116,7 +116,7 @@ func TestNew_OnlyEmptyCSVSegments(t *testing.T) { } // TestNew_InvalidIntervalStopsStartup checks values that must stop startup; -// TestNew_DefaultValues and TestNew_EmptyIntervalMeansDefault check that an +// TestNew_DefaultValues and TestNew_EmptyValueMeansDefault check that an // unset or empty interval means the default. func TestNew_InvalidIntervalStopsStartup(t *testing.T) { variables := []string{"DNSWATCHER_DNS_INTERVAL", "DNSWATCHER_TLS_INTERVAL"} @@ -144,16 +144,107 @@ func TestNew_InvalidIntervalStopsStartup(t *testing.T) { } } -func TestNew_EmptyIntervalMeansDefault(t *testing.T) { +func TestNew_EmptyValueMeansDefault(t *testing.T) { viper.Reset() t.Setenv("DNSWATCHER_TARGETS", "example.com") t.Setenv("DNSWATCHER_DNS_INTERVAL", "") t.Setenv("DNSWATCHER_TLS_INTERVAL", "") + t.Setenv("PORT", "") + t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "") + t.Setenv("DNSWATCHER_DEBUG", "") + t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "") + t.Setenv("DNSWATCHER_SEND_TEST_NOTIFICATION", "") cfg, err := config.New(nil, newTestParams(t)) require.NoError(t, err) assert.Equal(t, time.Hour, cfg.DNSInterval) assert.Equal(t, 12*time.Hour, cfg.TLSInterval) + assert.Equal(t, 8080, cfg.Port) + assert.Equal(t, 7, cfg.TLSExpiryWarning) + assert.False(t, cfg.Debug) + assert.False(t, cfg.MaintenanceMode) + assert.False(t, cfg.SendTestNotification) +} + +// TestNew_InvalidNumberOrBoolStopsStartup checks values that must stop +// startup; TestNew_EnvironmentOverrides, the two tests after this one and +// TestNew_EmptyValueMeansDefault check values that must not. +func TestNew_InvalidNumberOrBoolStopsStartup(t *testing.T) { + notBools := []string{"yes", "on", "2", "tRUE"} + tests := []struct { + variable string + values []string + want error + }{ + { + variable: "PORT", + values: []string{"notaport", "0", "-1", "65536", "8080.5"}, + want: config.ErrInvalidPort, + }, + { + variable: "DNSWATCHER_TLS_EXPIRY_WARNING", + values: []string{"abc", "7d", "0", "-5"}, + want: config.ErrInvalidTLSExpiryWarning, + }, + { + variable: "DNSWATCHER_DEBUG", + values: notBools, + want: config.ErrInvalidBool, + }, + { + variable: "DNSWATCHER_MAINTENANCE_MODE", + values: notBools, + want: config.ErrInvalidBool, + }, + { + variable: "DNSWATCHER_SEND_TEST_NOTIFICATION", + values: notBools, + want: config.ErrInvalidBool, + }, + } + + for _, tt := range tests { + for _, value := range tt.values { + t.Run(tt.variable+"="+value, func(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.com") + t.Setenv(tt.variable, value) + + _, err := config.New(nil, newTestParams(t)) + require.ErrorIs(t, err, tt.want) + require.ErrorContains(t, err, tt.variable) + require.ErrorContains(t, err, strconv.Quote(value)) + }) + } + } +} + +func TestNew_LowestNumbersAndOtherSpellingsOfTrue(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.com") + t.Setenv("PORT", "1") + t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "1") + t.Setenv("DNSWATCHER_DEBUG", "1") + t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "T") + t.Setenv("DNSWATCHER_SEND_TEST_NOTIFICATION", "True") + + cfg, err := config.New(nil, newTestParams(t)) + require.NoError(t, err) + assert.Equal(t, 1, cfg.Port) + assert.Equal(t, 1, cfg.TLSExpiryWarning) + assert.True(t, cfg.Debug) + assert.True(t, cfg.MaintenanceMode) + assert.True(t, cfg.SendTestNotification) +} + +func TestNew_HighestPort(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.com") + t.Setenv("PORT", "65535") + + cfg, err := config.New(nil, newTestParams(t)) + require.NoError(t, err) + assert.Equal(t, 65535, cfg.Port) } func TestNew_DebugEnablesDebugLogging(t *testing.T) {