Isolate config tests from the shell; any out-of-range PORT is ErrInvalidPort (closes #94)

Config tests, and the first-boot debug log test that builds a Config,
start from an empty environment: config.ClearEnvForTest unsets every
variable the process has and restores each when the test ends, so
nothing exported in the developer's shell changes a result.
TestEnvPositiveInt and TestEnvPort share one table runner, and
TestEnvPort checks that the bad value appears in each error.

Every out-of-range PORT now wraps ErrInvalidPort: zero, negatives,
above 65535, and numbers too large or too small for an int.

The README configuration table and the Settings page say that a
RETENTION_SWEEP_INTERVAL that does not parse, or is zero or negative,
fails startup.

Model: opus-5-5

README.md

@@ -2924,7 +2924,8 @@ webhooker/
 │   ├── resetpw/
 │   │   └── resetpw.go         # `webhooker resetpw`: set an account's password, stopped deployments only
 │   ├── config/
-│   │   └── config.go          # Configuration loading from environment variables
+│   │   ├── config.go          # Configuration loading from environment variables
+│   │   └── testing.go         # ClearEnvForTest: an empty environment for one test
 │   ├── database/
 │   │   ├── base_model.go      # BaseModel with UUID primary keys
 │   │   ├── database.go        # GORM connection, migrations, admin seed

internal/config/config.go

@@ -362,22 +362,24 @@ func envPositiveInt(
 // envPort returns the value of the named environment variable parsed
 // as a TCP port number. Returns defaultValue if not set. A set value
 // that is unparseable, below 1, or above maxPort is a hard error
-// naming the key and the bad value; the two out-of-range cases both
-// wrap ErrInvalidPort.
+// naming the key and the bad value; every out-of-range value wraps
+// ErrInvalidPort, including one too large or too small for an int.
 func envPort(key string, defaultValue int) (int, error) {
 	v := os.Getenv(key)
 	if v == "" {
 		return defaultValue, nil
 	}
 
+	// strconv.ErrRange means a number too large or too small for an
+	// int, which is outside the port range as well.
 	port, err := strconv.Atoi(v)
-	if err != nil {
+	if err != nil && !errors.Is(err, strconv.ErrRange) {
 		return 0, fmt.Errorf(
 			"invalid integer for %s: %q: %w", key, v, err,
 		)
 	}
 
-	if port < 1 || port > maxPort {
+	if err != nil || port < 1 || port > maxPort {
 		return 0, fmt.Errorf(
 			"%w: %s must be from 1 to %d, got %q",
 			ErrInvalidPort, key, maxPort, v,

internal/config/config_test.go

@@ -70,7 +70,7 @@ func TestEnvironmentConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 
 			if tt.envValue != "" {
 				t.Setenv(
@@ -196,7 +196,7 @@ func TestRetentionSweepInterval(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 
 			if tt.set {
@@ -335,7 +335,7 @@ func TestSessionIdleTimeout(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 
 			if tt.set {
@@ -388,7 +388,7 @@ func TestDefaultDataDir(t *testing.T) {
 		t.Run("env="+name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 
 			if env != "" {
 				t.Setenv("WEBHOOKER_ENVIRONMENT", env)
@@ -433,9 +433,9 @@ func TestDataDirHelper(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			if set == "" {
-				unsetEnv(t, "DATA_DIR")
-			} else {
+			config.ClearEnvForTest(t)
+
+			if set != "" {
 				t.Setenv("DATA_DIR", set)
 			}
 
@@ -498,7 +498,7 @@ func TestReceiverRateLimit(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 
 			if tt.set {
@@ -614,7 +614,7 @@ func TestTrustedProxies(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 
 			if tt.set {
@@ -725,7 +725,7 @@ func TestAllowedEgressCIDRs(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 
 			if tt.set {
@@ -797,7 +797,7 @@ func TestEgressAllowlistWarning(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", config.EnvironmentDev)
 
 			if tt.allowed != "" {
@@ -933,7 +933,7 @@ func TestMetricsAuthConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 
 			if tt.username.set {
 				t.Setenv("METRICS_USERNAME", tt.username.value)

internal/config/dotenv_test.go

@@ -39,9 +39,9 @@ func writeDotEnv(t *testing.T, contents string) string {
 // normally rather than be refused for a file it was never meant to
 // have.
 //
-//nolint:paralleltest // unsetEnv uses t.Setenv.
+//nolint:paralleltest // ClearEnvForTest uses t.Setenv.
 func TestLoadDotEnv_MissingFileIsFine(t *testing.T) {
-	unsetEnv(t, dotEnvKey)
+	config.ClearEnvForTest(t)
 
 	absent := filepath.Join(t.TempDir(), config.DotEnvPath)
 	require.NoError(t, config.LoadDotEnvFileForTest(absent))
@@ -54,9 +54,9 @@ func TestLoadDotEnv_MissingFileIsFine(t *testing.T) {
 // reaches the environment, which is the whole reason the file is read
 // at all.
 //
-//nolint:paralleltest // unsetEnv uses t.Setenv.
+//nolint:paralleltest // ClearEnvForTest uses t.Setenv.
 func TestLoadDotEnv_AppliesValues(t *testing.T) {
-	unsetEnv(t, dotEnvKey)
+	config.ClearEnvForTest(t)
 
 	path := writeDotEnv(t, "# a comment\n"+dotEnvKey+"=from-dot-env\n")
 
@@ -82,9 +82,9 @@ func TestLoadDotEnv_RealEnvironmentWins(t *testing.T) {
 // reverts to its default; the process used to start that way with no
 // log line naming the file at all.
 //
-//nolint:paralleltest // unsetEnv uses t.Setenv.
+//nolint:paralleltest // ClearEnvForTest uses t.Setenv.
 func TestLoadDotEnv_MalformedFileAborts(t *testing.T) {
-	unsetEnv(t, dotEnvKey)
+	config.ClearEnvForTest(t)
 
 	path := writeDotEnv(
 		t, malformedDotEnv+dotEnvKey+"=from-dot-env\n",
@@ -132,7 +132,7 @@ func TestLoadDotEnv_UnreadableFileAborts(t *testing.T) {
 //
 //nolint:paralleltest // t.Chdir moves the whole process.
 func TestLoadDotEnv_ReadsTheWorkingDirectory(t *testing.T) {
-	unsetEnv(t, dotEnvKey)
+	config.ClearEnvForTest(t)
 
 	dir := t.TempDir()
 	require.NoError(t, os.WriteFile(

internal/config/env_test.go

@@ -1,7 +1,6 @@
 package config_test
 
 import (
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -38,41 +37,6 @@ const (
 	bindAddressSample = "10.1.2.3"
 )
 
-// configEnvKeys is every variable config.New reads. Its tests unset
-// all of them before setting the ones under test, so a variable
-// exported in the developer's shell cannot change their outcome.
-func configEnvKeys() []string {
-	return []string{
-		"WEBHOOKER_ENVIRONMENT",
-		envKeyPort,
-		envKeyBindAddress,
-		"DATA_DIR",
-		envKeyDebug,
-		"METRICS_USERNAME",
-		"METRICS_PASSWORD",
-		envKeySentryDSN,
-		"RETENTION_SWEEP_INTERVAL",
-		"SESSION_IDLE_TIMEOUT",
-		"RECEIVER_RATE_LIMIT",
-		"TRUSTED_PROXIES",
-		"ALLOWED_EGRESS_CIDRS",
-	}
-}
-
-// unsetEnv makes each key absent for the rest of the test and puts
-// back whatever it held when the test ends. t.Setenv registers that
-// restore; the Unsetenv after it is what makes the key absent, since
-// a key set to the empty string is still present, and godotenv will
-// not overwrite a present key.
-func unsetEnv(t *testing.T, keys ...string) {
-	t.Helper()
-
-	for _, key := range keys {
-		t.Setenv(key, "")
-		require.NoError(t, os.Unsetenv(key))
-	}
-}
-
 // envBoolCase is one row of the envBool table.
 type envBoolCase struct {
 	name         string
@@ -156,10 +120,10 @@ func TestEnvBool(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
+			config.ClearEnvForTest(t)
+
 			if tt.set {
 				t.Setenv(testEnvKey, tt.value)
-			} else {
-				unsetEnv(t, testEnvKey)
 			}
 
 			got, err := config.EnvBoolForTest(
@@ -180,17 +144,62 @@ func TestEnvBool(t *testing.T) {
 	}
 }
 
+// envIntCase is one row of the envPositiveInt and envPort tables.
+type envIntCase struct {
+	name        string
+	set         bool
+	value       string
+	expectError bool
+	errIs       error
+	expected    int
+}
+
+// runEnvIntCases runs each row through parse, which is
+// envPositiveInt or envPort, with testEnvKey set to the row's value
+// or left unset.
+func runEnvIntCases(
+	t *testing.T,
+	parse func(key string, defaultValue int) (int, error),
+	defaultValue int,
+	tests []envIntCase,
+) {
+	t.Helper()
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			config.ClearEnvForTest(t)
+
+			if tt.set {
+				t.Setenv(testEnvKey, tt.value)
+			}
+
+			got, err := parse(testEnvKey, defaultValue)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), testEnvKey)
+				assert.Contains(t, err.Error(), tt.value)
+
+				if tt.errIs != nil {
+					require.ErrorIs(t, err, tt.errIs)
+				}
+
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}
+
+//nolint:paralleltest // runEnvIntCases uses t.Setenv.
 func TestEnvPositiveInt(t *testing.T) {
 	const defaultValue = 7
 
-	tests := []struct {
-		name        string
-		set         bool
-		value       string
-		expectError bool
-		errIs       error
-		expected    int
-	}{
+	runEnvIntCases(t, config.EnvPositiveIntForTest, defaultValue, []envIntCase{
 		{
 			name:     "unset returns the default integer",
 			expected: defaultValue,
@@ -227,52 +236,14 @@ func TestEnvPositiveInt(t *testing.T) {
 			expectError: true,
 			errIs:       config.ErrNonPositiveValue,
 		},
-	}
-
-	//nolint:dupl // TestEnvPort makes the same checks, on purpose.
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			// Cannot use t.Parallel() here because t.Setenv
-			// is incompatible with parallel subtests.
-			if tt.set {
-				t.Setenv(testEnvKey, tt.value)
-			} else {
-				unsetEnv(t, testEnvKey)
-			}
-
-			got, err := config.EnvPositiveIntForTest(
-				testEnvKey, defaultValue,
-			)
-
-			if tt.expectError {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), testEnvKey)
-				assert.Contains(t, err.Error(), tt.value)
-
-				if tt.errIs != nil {
-					require.ErrorIs(t, err, tt.errIs)
-				}
-
-				return
-			}
-
-			require.NoError(t, err)
-			assert.Equal(t, tt.expected, got)
-		})
-	}
+	})
 }
 
+//nolint:paralleltest // runEnvIntCases uses t.Setenv.
 func TestEnvPort(t *testing.T) {
 	const defaultValue = 8080
 
-	tests := []struct {
-		name        string
-		set         bool
-		value       string
-		expectError bool
-		errIs       error
-		expected    int
-	}{
+	runEnvIntCases(t, config.EnvPortForTest, defaultValue, []envIntCase{
 		{
 			name:     "unset returns the default port",
 			expected: defaultValue,
@@ -302,6 +273,13 @@ func TestEnvPort(t *testing.T) {
 			expectError: true,
 			errIs:       config.ErrInvalidPort,
 		},
+		{
+			name:        "negative is rejected",
+			set:         true,
+			value:       "-1",
+			expectError: true,
+			errIs:       config.ErrInvalidPort,
+		},
 		{
 			name:        "above the port range is rejected",
 			set:         true,
@@ -309,39 +287,14 @@ func TestEnvPort(t *testing.T) {
 			expectError: true,
 			errIs:       config.ErrInvalidPort,
 		},
-	}
-
-	//nolint:dupl // TestEnvPositiveInt makes the same checks, on purpose.
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			// Cannot use t.Parallel() here because t.Setenv
-			// is incompatible with parallel subtests.
-			if tt.set {
-				t.Setenv(testEnvKey, tt.value)
-			} else {
-				unsetEnv(t, testEnvKey)
-			}
-
-			got, err := config.EnvPortForTest(
-				testEnvKey, defaultValue,
-			)
-
-			if tt.expectError {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), testEnvKey)
-				assert.Contains(t, err.Error(), tt.value)
-
-				if tt.errIs != nil {
-					require.ErrorIs(t, err, tt.errIs)
-				}
-
-				return
-			}
-
-			require.NoError(t, err)
-			assert.Equal(t, tt.expected, got)
-		})
-	}
+		{
+			name:        "too large for an int is rejected",
+			set:         true,
+			value:       "99999999999999999999",
+			expectError: true,
+			errIs:       config.ErrInvalidPort,
+		},
+	})
 }
 
 // TestEnvBindAddress covers BIND_ADDRESS parsing.
@@ -357,10 +310,10 @@ func TestEnvBindAddress(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
+			config.ClearEnvForTest(t)
+
 			if tt.set {
 				t.Setenv(testEnvKey, tt.value)
-			} else {
-				unsetEnv(t, testEnvKey)
 			}
 
 			got, err := config.EnvBindAddressForTest(
@@ -523,7 +476,7 @@ func TestNewRejectsBadEnvValues(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
-			unsetEnv(t, configEnvKeys()...)
+			config.ClearEnvForTest(t)
 			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 			t.Setenv(tt.key, tt.value)
 
@@ -685,7 +638,7 @@ func sentryEnvValueCases() []badEnvValueCase {
 // break the legitimate unset case: absent variables still get their
 // documented defaults.
 func TestNewUsesDefaultsWhenUnset(t *testing.T) {
-	unsetEnv(t, configEnvKeys()...)
+	config.ClearEnvForTest(t)
 	t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
 
 	cfg, err := buildConfig(t)

internal/config/sentry_test.go

@@ -100,10 +100,10 @@ func TestEnvSentryDSN(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// Cannot use t.Parallel() here because t.Setenv
 			// is incompatible with parallel subtests.
+			config.ClearEnvForTest(t)
+
 			if tt.set {
 				t.Setenv(envKeySentryDSN, tt.value)
-			} else {
-				unsetEnv(t, envKeySentryDSN)
 			}
 
 			got, err := config.EnvSentryDSNForTest(envKeySentryDSN)

internal/config/testing.go

@@ -0,0 +1,30 @@
+package config
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+// ClearEnvForTest unsets every variable in the process environment
+// for the rest of the test and puts each back when the test ends, so
+// a test sees only the variables it sets itself, not whatever the
+// developer's shell exports.
+func ClearEnvForTest(t *testing.T) {
+	t.Helper()
+
+	for _, entry := range os.Environ() {
+		key, _, _ := strings.Cut(entry, "=")
+
+		// t.Setenv registers the restore; the Unsetenv after it is
+		// what makes the key absent, since a key set to the empty
+		// string is still present, and godotenv will not overwrite a
+		// present key.
+		t.Setenv(key, "")
+
+		err := os.Unsetenv(key)
+		if err != nil {
+			t.Fatalf("unsetting %s: %v", key, err)
+		}
+	}
+}

internal/database/totals_test.go

@@ -102,6 +102,13 @@ func TestWebhookDBManager_TotalsSurviveReopen(t *testing.T) {
 // seedExpiredEvents stores count events created at the given time,
 // each with a delivered delivery to one target and a failed delivery
 // to the other, and one attempt for each delivery.
+//
+// It and seedBareEvents insert 50 rows per statement, not more. The
+// SQLite driver looks up each parameter's value by scanning the
+// statement's arguments from the first until it reaches that
+// parameter's, so the time to bind a statement grows with the square of
+// its parameter count: at 500 rows, several thousand parameters, the
+// seeding took most of these tests' time under -race.
 func seedExpiredEvents(
 	t *testing.T,
 	db *gorm.DB,
@@ -138,8 +145,8 @@ func seedExpiredEvents(
 		)
 	}
 
-	require.NoError(t, db.CreateInBatches(events, 500).Error)
-	require.NoError(t, db.CreateInBatches(deliveries, 500).Error)
+	require.NoError(t, db.CreateInBatches(events, 50).Error)
+	require.NoError(t, db.CreateInBatches(deliveries, 50).Error)
 
 	results := make([]database.DeliveryResult, len(deliveries))
 	for i := range deliveries {
@@ -148,7 +155,7 @@ func seedExpiredEvents(
 		}
 	}
 
-	require.NoError(t, db.CreateInBatches(results, 500).Error)
+	require.NoError(t, db.CreateInBatches(results, 50).Error)
 }
 
 // seedBareEvents stores count events created at the given time, with
@@ -172,7 +179,7 @@ func seedBareEvents(
 		events[i].CreatedAt = createdAt
 	}
 
-	require.NoError(t, db.CreateInBatches(events, 500).Error)
+	require.NoError(t, db.CreateInBatches(events, 50).Error)
 }
 
 // TestRetentionReaper_PrunesMoreThanOneBatch verifies that a prune

internal/gormlog/firstboot_test.go

@@ -117,8 +117,8 @@ func readFirstBootSecrets(
 }
 
 // bootAtDebug starts and stops the real application graph against
-// dataDir with DEBUG=true, and returns everything it wrote to standard
-// output.
+// dataDir with DEBUG=true and nothing else set, and returns everything
+// it wrote to standard output.
 //
 // config.New reads DEBUG from the environment exactly as the binary
 // does, internal/logger builds the handler it builds in production,
@@ -128,6 +128,7 @@ func readFirstBootSecrets(
 func bootAtDebug(t *testing.T, dataDir string) string {
 	t.Helper()
 
+	config.ClearEnvForTest(t)
 	t.Setenv("DEBUG", "true")
 	t.Setenv("DATA_DIR", dataDir)
 

script/test

@@ -27,7 +27,11 @@
 # Those figures predate tests hashing the admin password at 1 MB instead of
 # 64 MB (https://git.eeqj.de/sneak/webhooker/pulls/404). After that change, in
 # a cache-defeated build at host load 44-109 (2026-10-02), internal/handlers
-# took 8.5s and the slowest package was internal/database at 15.8s.
+# took 8.5s and the slowest package was internal/database at 15.8s. Once its
+# retention tests seeded 50 rows per insert instead of 500
+# (https://git.eeqj.de/sneak/webhooker/issues/198), internal/database took
+# 7.3s and the slowest package was internal/handlers at 8.1s to 10.0s, at host
+# load 25-48 (2026-10-02).
 #
 # -p 4 -parallel 8 keep the run under 2 GB of memory: at most four test
 # binaries build or run at once, each with at most eight parallel tests. Under