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

@@ -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:

TODO.md

@@ -19,6 +19,10 @@ 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
   and test are phases of the `Dockerfile`, every scripted build uncached (closes
   #257).

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, &notFound) {
-			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, &notFound) {
+			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")),

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) {

internal/state/state.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"maps"
 	"os"
 	"path/filepath"
 	"slices"
@@ -270,12 +271,21 @@ func (s *State) Save() error {
 	return nil
 }
 
-// GetSnapshot returns a copy of the current snapshot.
+// GetSnapshot returns a copy of the current snapshot, with copies of
+// its maps, so that a caller can range over them while the state is
+// written. The entries in the maps are not copied: an entry saved in the
+// state must be replaced, never changed in place.
 func (s *State) GetSnapshot() Snapshot {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 
-	return *s.snapshot
+	snap := *s.snapshot
+	snap.Domains = maps.Clone(snap.Domains)
+	snap.Hostnames = maps.Clone(snap.Hostnames)
+	snap.Ports = maps.Clone(snap.Ports)
+	snap.Certificates = maps.Clone(snap.Certificates)
+
+	return snap
 }
 
 // SetDomainState updates the state for a domain.

internal/state/state_test.go

@@ -995,7 +995,8 @@ func TestPortStateUnmarshalJSON_BothFormats(t *testing.T) {
 	}
 }
 
-// TestGetSnapshot_ReturnsCopy verifies that GetSnapshot returns a value copy.
+// TestGetSnapshot_ReturnsCopy verifies that the maps in the snapshot
+// GetSnapshot returns are not the state's own.
 func TestGetSnapshot_ReturnsCopy(t *testing.T) {
 	t.Parallel()
 
@@ -1005,15 +1006,67 @@ func TestGetSnapshot_ReturnsCopy(t *testing.T) {
 
 	snap := s.GetSnapshot()
 
-	// Mutate the returned snapshot's map.
-	snap.Domains["mutated.example.com"] = &state.DomainState{
-		Nameservers: []string{"ns1.mutated.com."},
+	const added = "added.example.com"
+
+	snap.Domains[added] = &state.DomainState{}
+	snap.Hostnames[added] = &state.HostnameState{}
+	snap.Ports[added] = &state.PortState{}
+	snap.Certificates[added] = &state.CertificateState{}
+
+	if _, ok := s.GetDomainState(added); ok {
+		t.Error("the snapshot's Domains map is the state's own")
 	}
 
-	// The original domain data should still be accessible.
-	_, ok := s.GetDomainState("example.com")
-	if !ok {
-		t.Error("original domain should still exist")
+	if _, ok := s.GetHostnameState(added); ok {
+		t.Error("the snapshot's Hostnames map is the state's own")
+	}
+
+	if _, ok := s.GetPortState(added); ok {
+		t.Error("the snapshot's Ports map is the state's own")
+	}
+
+	if _, ok := s.GetCertificateState(added); ok {
+		t.Error("the snapshot's Certificates map is the state's own")
+	}
+}
+
+// TestGetSnapshot_WhileWritten ranges over snapshots while another
+// goroutine writes the state, as the dashboard does while a check runs.
+// Under -race it fails when a snapshot's maps are the state's own.
+func TestGetSnapshot_WhileWritten(t *testing.T) {
+	t.Parallel()
+
+	s := state.NewForTestWithDataDir(t.TempDir())
+
+	const writes = 1000
+
+	written := make(chan struct{})
+
+	go func() {
+		defer close(written)
+
+		for range writes {
+			s.SetDomainState("example.com", &state.DomainState{})
+			s.SetHostnameState(testHostname, &state.HostnameState{})
+			s.SetPortState(testIPv4+":443", &state.PortState{})
+			s.SetCertificateState(
+				testIPv4+":443:"+testHostname, &state.CertificateState{},
+			)
+		}
+	}()
+
+	for {
+		select {
+		case <-written:
+			return
+		default:
+		}
+
+		// json.Marshal ranges over each of the snapshot's maps.
+		_, err := json.Marshal(s.GetSnapshot())
+		if err != nil {
+			t.Fatal(err)
+		}
 	}
 }