Fail loudly on set-but-unparseable env config values (closes #80 )

The config env helpers silently substituted the documented default whenever a variable was set but could not be parsed, so a typo in an operator-supplied value produced a running daemon with configuration nobody asked for instead of a startup failure. `PORT=eighty` quietly listened on 8080 and `DEBUG=ture` quietly disabled debug logging. Defaults now apply only to variables that are unset or empty. Any variable that is set but unparseable is a hard error that names the key and the offending value and aborts startup through fx. - add `envPositiveInt` with `ErrNonPositiveValue`, copied verbatim from the definition on the unmerged #87 so that rebasing after it lands is a delete-one-copy operation rather than a semantic merge - remove `envInt` entirely; `PORT` is parsed by a new `envPort`, which adds the TCP upper bound (`ErrInvalidPort`, 1..65535) - change `envBool` to return an error and parse with `strconv.ParseBool`, so `yes`, `on`, and typos are rejected rather than silently treated as false; callers are `DEBUG` and `MAINTENANCE_MODE` - move env loading into `loadFromEnv`, with the environment check extracted to `resolveEnvironment` (also as #87 defines it), keeping `New` within the funlen budget `envString` parses nothing and `envDuration` was already fail-loud, so both are unchanged. A repo-wide audit of `os.Getenv`/`os.LookupEnv` found no parse sites outside `internal/config`. Tests cover each helper with a table (unset, valid, set-but-invalid) plus `config.New`-level cases proving a bad `PORT`, `DEBUG`, or `MAINTENANCE_MODE` aborts startup while unset variables still get their defaults. README documents the fail-loud rule and the accepted boolean spellings.
2026-08-09 01:45:27 +00:00
17 changed files with 649 additions and 981 deletions
--- a/README.md
+++ b/README.md
@@ -89,10 +89,27 @@ TTY detection, and security headers are always applied.
 | `PORT`                  | HTTP listen port                    | `8080`   |
 | `DATA_DIR`              | Directory for all SQLite databases  | `/var/lib/webhooker` |
 | `DEBUG`                 | Enable debug logging                | `false`  |
+| `MAINTENANCE_MODE`      | Serve the maintenance page          | `false`  |
 | `METRICS_USERNAME`      | Basic auth username for `/metrics`  | `""`     |
 | `METRICS_PASSWORD`      | Basic auth password for `/metrics`  | `""`     |
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |

+#### Invalid values abort startup
+
+The defaults above apply **only** to variables that are unset (or set
+to an empty string). A variable that is set but cannot be parsed is a
+fatal configuration error: webhooker logs the offending variable and
+its value and refuses to start, rather than silently running with a
+substituted default. `PORT=eighty`, `DEBUG=ture`, and
+`RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
+additionally be a number in the range 1–65535.
+
+Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
+spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
+`true`, `True`, `0`, `f`, `F`, `FALSE`, `false`, `False` — and nothing
+else. `yes`, `on`, and `off` are rejected rather than quietly treated
+as false.
+
 On first startup, webhooker automatically generates a cryptographically
 secure session encryption key and stores it in the database. This key
 persists across restarts — no manual key management is needed.
@@ -307,20 +324,13 @@ event routing.
 | `user_id`        | UUID    | Foreign key → User |
 | `name`           | string  | Human-readable name |
 | `description`    | string  | Optional description |
-| `retention_days` | integer | Days to retain events (default: 30; 0 means retain forever) |
+| `retention_days` | integer | Days to retain events (default: 30) |

 **Relations:** Belongs to User. Has many Entrypoints. Has many Targets.

 The `retention_days` field controls how long event data is kept in the
 webhook's dedicated database before automatic cleanup.

-Setting `retention_days` to `0` means "retain events forever". Because
-the column carries a default of 30, a literal zero cannot survive an
-insert, so a zero is rewritten on save to a sentinel of `365 * 1000`
-days (`database.RetentionForeverDays`). The retention reaper recognises
-that sentinel and skips the webhook entirely, and the web UI displays
-such a webhook's retention as "forever" rather than as a day count.
-
 #### Entrypoint

 A receiver URL where external services POST webhook events. Each
@@ -516,7 +526,7 @@ This separation provides:
  DB; the event database file is hard-deleted (permanently removed).
 - **Per-webhook retention** — the `retention_days` field on each webhook
  controls automatic cleanup of old events in that webhook's database
-  only, or disables cleanup entirely when set to `0` (retain forever).
+  only.
 - **Performance** — each webhook's database has its own WAL, its own
  page cache, and its own lock, so concurrent event ingestion across
  webhooks won't contend.
--- a/TODO.md
+++ b/TODO.md
@@ -10,30 +10,28 @@

 # Status

-pre-1.0. No git tags exist. main (afe88c6) is a working webhook proxy
+pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
 with auth, CSRF/SSRF protections, login rate limiting, Slack target,
-policy compliance (#6), and pinned lint tooling (#55). Note: TODO.md was
-deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
-content was folded into the README TODO section, which this draft
-reconstructs as of 2026-07-06.
+policy compliance (#6), pinned lint tooling (#55), a per-webhook event
+retention reaper (#63), and fail-loud configuration parsing (#80). Note:
+TODO.md was deliberately deleted from this repo in f9a9569 (2026-03-01,
+#6); its content was folded into the README TODO section, which this
+draft reconstructs as of 2026-07-06.

 # Next Step

-Implement automatic event retention cleanup based on retention_days: a
-periodic maintenance job that deletes Events, Deliveries, and
-DeliveryResults older than the parent webhook's retention_days from each
-per-webhook event database. The field exists on the Webhook model and
-the README promises the behavior, but nothing enforces it, so event
-databases currently grow without bound.
+Manual event redelivery from the web UI (replay is a core promised
+capability in the README rationale).

 # Completed Steps

- 2026-08-09 Make retain-forever reachable from the normal create and
-  edit flows (#79): a `RetentionForeverDays = 365 * 1000` sentinel, a
-  `Webhook.BeforeSave` hook rewriting any non-positive `retention_days`
-  to it ahead of GORM's own column defaulting, a reaper that skips such
-  webhooks outright, form validation that honours `0` and rejects
-  garbage with a 400, and a retention UI that says "forever"
+- 2026-08-09 Configuration parsing fails loudly on set-but-unparseable
+  environment values: `envInt` removed in favour of `envPositiveInt`
+  plus a `PORT` range check, `envBool` now parses with
+  `strconv.ParseBool`, and defaults apply only to unset variables (#80)
+- 2026-08-07 Automatic event retention cleanup based on
+  `retention_days`, deleting expired events, deliveries, and delivery
+  results from each per-webhook event database (#63)
 - 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
  `Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
  adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so
@@ -62,8 +60,6 @@ databases currently grow without bound.

 # Future Steps

- Manual event redelivery from the web UI (replay is a core promised
-  capability in the README rationale)
 - Delivery status and retry management UI
 - Per-webhook rate limiting in the receiver handler (per-webhook config
  plus handler enforcement; global limits must not apply to receiver
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -7,7 +7,6 @@ import (
 	"log/slog"
 	"os"
 	"strconv"
-	"strings"
 	"time"

 	"go.uber.org/fx"
@@ -31,12 +30,24 @@ const (
 	// defaultRetentionSweepInterval is how often the retention
 	// reaper deletes events older than each webhook's RetentionDays.
 	defaultRetentionSweepInterval = time.Hour
+
+	// maxPort is the highest valid TCP port number. The lower
+	// bound (at least 1) is enforced by envPositiveInt.
+	maxPort = 65535
 )

 // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
 // contains an unrecognised value.
 var ErrInvalidEnvironment = errors.New("invalid environment")

+// ErrNonPositiveValue is returned when an environment variable that
+// requires a positive integer is set to zero or a negative number.
+var ErrNonPositiveValue = errors.New("value must be positive")
+
+// ErrInvalidPort is returned when an environment variable holding a
+// TCP port number is set above the valid port range.
+var ErrInvalidPort = errors.New("invalid port")
+
 //nolint:revive // ConfigParams is a standard fx naming convention.
 type ConfigParams struct {
 	fx.In
@@ -81,27 +92,81 @@ func envString(key string) string {
 }

 // envBool returns the value of the named environment variable
-// parsed as a boolean. Returns defaultValue if not set.
-func envBool(key string, defaultValue bool) bool {
-	if v := os.Getenv(key); v != "" {
-		return strings.EqualFold(v, "true") || v == "1"
+// parsed as a boolean. Returns defaultValue if not set. If the
+// variable is set but cannot be parsed, it returns a wrapped error
+// naming the key and the bad value, so startup fails loudly rather
+// than silently falling back to the default.
+//
+// Parsing is strconv.ParseBool, which accepts 1, t, T, TRUE, true,
+// True, 0, f, F, FALSE, false and False. Anything else — "yes",
+// "on", or a typo like "ture" — is an error rather than a silent
+// false.
+func envBool(key string, defaultValue bool) (bool, error) {
+	v := os.Getenv(key)
+	if v == "" {
+		return defaultValue, nil
 	}

-	return defaultValue
+	b, err := strconv.ParseBool(v)
+	if err != nil {
+		return false, fmt.Errorf(
+			"invalid boolean for %s: %q: %w", key, v, err,
+		)
+	}
+
+	return b, nil
 }

-// envInt returns the value of the named environment variable
-// parsed as an integer. Returns defaultValue if not set or
-// unparseable.
-func envInt(key string, defaultValue int) int {
-	if v := os.Getenv(key); v != "" {
-		i, err := strconv.Atoi(v)
-		if err == nil {
-			return i
-		}
+// envPositiveInt returns the value of the named environment variable
+// parsed as a positive integer. Returns defaultValue if not set. If
+// the variable is set but cannot be parsed, or parses to less than
+// one, it returns a wrapped error naming the key and the bad value,
+// so startup fails loudly rather than silently falling back to the
+// default.
+func envPositiveInt(
+	key string,
+	defaultValue int,
+) (int, error) {
+	v := os.Getenv(key)
+	if v == "" {
+		return defaultValue, nil
 	}

-	return defaultValue
+	i, err := strconv.Atoi(v)
+	if err != nil {
+		return 0, fmt.Errorf(
+			"invalid integer for %s: %q: %w", key, v, err,
+		)
+	}
+
+	if i < 1 {
+		return 0, fmt.Errorf(
+			"%w: %s must be at least 1, got %q",
+			ErrNonPositiveValue, key, v,
+		)
+	}
+
+	return i, nil
+}
+
+// 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.
+func envPort(key string, defaultValue int) (int, error) {
+	port, err := envPositiveInt(key, defaultValue)
+	if err != nil {
+		return 0, err
+	}
+
+	if port > maxPort {
+		return 0, fmt.Errorf(
+			"%w: %s must be at most %d, got %d",
+			ErrInvalidPort, key, maxPort, port,
+		)
+	}
+
+	return port, nil
 }

 // envDuration returns the value of the named environment variable
@@ -128,32 +193,52 @@ func envDuration(
 	return d, nil
 }

-// New creates a Config by reading environment variables.
-//
-//nolint:revive // lc parameter is required by fx even if unused.
-func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
-	log := params.Logger.Get()
-
-	// Determine environment from WEBHOOKER_ENVIRONMENT env var,
-	// default to dev
+// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
+// dev, and rejects unrecognised values.
+func resolveEnvironment() (string, error) {
 	environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
 	if environment == "" {
 		environment = EnvironmentDev
 	}

-	// Validate environment
 	if environment != EnvironmentDev &&
 		environment != EnvironmentProd {
-		return nil, fmt.Errorf(
+		return "", fmt.Errorf(
 			"%w: WEBHOOKER_ENVIRONMENT must be '%s' or '%s', got '%s'",
 			ErrInvalidEnvironment,
 			EnvironmentDev, EnvironmentProd, environment,
 		)
 	}

-	// Parse the retention sweep interval; a set-but-unparseable value
-	// is a hard error so fx aborts startup rather than silently using
-	// the default.
+	return environment, nil
+}
+
+// loadFromEnv builds a Config from the environment. Every value that
+// needs parsing fails loudly when it is set but unparseable: the
+// documented defaults apply only to variables that are unset (or
+// empty), never as a substitute for a value the operator actually
+// provided.
+func loadFromEnv() (*Config, error) {
+	environment, err := resolveEnvironment()
+	if err != nil {
+		return nil, err
+	}
+
+	port, err := envPort("PORT", defaultPort)
+	if err != nil {
+		return nil, err
+	}
+
+	debug, err := envBool("DEBUG", false)
+	if err != nil {
+		return nil, err
+	}
+
+	maintenanceMode, err := envBool("MAINTENANCE_MODE", false)
+	if err != nil {
+		return nil, err
+	}
+
 	retentionSweepInterval, err := envDuration(
 		"RETENTION_SWEEP_INTERVAL",
 		defaultRetentionSweepInterval,
@@ -162,21 +247,36 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		return nil, err
 	}

-	// Load configuration values from environment variables
-	s := &Config{
+	return &Config{
 		DataDir:                envString("DATA_DIR"),
-		Debug:                  envBool("DEBUG", false),
-		MaintenanceMode:        envBool("MAINTENANCE_MODE", false),
+		Debug:                  debug,
+		MaintenanceMode:        maintenanceMode,
 		Environment:            environment,
 		MetricsUsername:        envString("METRICS_USERNAME"),
 		MetricsPassword:        envString("METRICS_PASSWORD"),
-		Port:                   envInt("PORT", defaultPort),
+		Port:                   port,
 		SentryDSN:              envString("SENTRY_DSN"),
 		RetentionSweepInterval: retentionSweepInterval,
-		log:                    log,
-		params:                 &params,
+	}, nil
+}
+
+// New creates a Config by reading environment variables.
+//
+//nolint:revive // lc parameter is required by fx even if unused.
+func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
+	log := params.Logger.Get()
+
+	// A set-but-unparseable value anywhere in the environment is a
+	// hard error, so fx aborts startup rather than running with a
+	// silently substituted default.
+	s, err := loadFromEnv()
+	if err != nil {
+		return nil, err
 	}

+	s.log = log
+	s.params = &params
+
 	// Set default DataDir. All SQLite databases (main application
 	// DB and per-webhook event DBs) live here. The same default is
 	// used regardless of environment; override with DATA_DIR if
--- a/internal/config/env_test.go
+++ b/internal/config/env_test.go
@@ -0,0 +1,409 @@
+package config_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/logger"
+)
+
+// testEnvKey is a throwaway variable name used only by the helper
+// tables below, so they cannot disturb real configuration.
+const testEnvKey = "WEBHOOKER_TEST_VALUE"
+
+// Real configuration variables exercised by the config.New tests.
+const (
+	envKeyPort            = "PORT"
+	envKeyDebug           = "DEBUG"
+	envKeyMaintenanceMode = "MAINTENANCE_MODE"
+)
+
+// envBoolCase is one row of the envBool table.
+type envBoolCase struct {
+	name         string
+	set          bool
+	value        string
+	defaultValue bool
+	expectError  bool
+	expected     bool
+}
+
+// envBoolCases is the envBool table, kept out of the test body so
+// the test itself stays readable.
+func envBoolCases() []envBoolCase {
+	return []envBoolCase{
+		{
+			name:         "unset uses default false",
+			defaultValue: false,
+			expected:     false,
+		},
+		{
+			name:         "unset uses default true",
+			defaultValue: true,
+			expected:     true,
+		},
+		{
+			name:         "empty uses default true",
+			set:          true,
+			value:        "",
+			defaultValue: true,
+			expected:     true,
+		},
+		{
+			name:     "true is parsed",
+			set:      true,
+			value:    "true",
+			expected: true,
+		},
+		{
+			name:     "one is parsed",
+			set:      true,
+			value:    "1",
+			expected: true,
+		},
+		{
+			name:         "False is parsed",
+			set:          true,
+			value:        "False",
+			defaultValue: true,
+			expected:     false,
+		},
+		{
+			name:         "zero is parsed",
+			set:          true,
+			value:        "0",
+			defaultValue: true,
+			expected:     false,
+		},
+		{
+			name:        "yes is rejected",
+			set:         true,
+			value:       "yes",
+			expectError: true,
+		},
+		{
+			name:        "on is rejected",
+			set:         true,
+			value:       "on",
+			expectError: true,
+		},
+		{
+			name:        "typo is rejected",
+			set:         true,
+			value:       "ture",
+			expectError: true,
+		},
+	}
+}
+
+func TestEnvBool(t *testing.T) {
+	for _, tt := range envBoolCases() {
+		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 {
+				require.NoError(t, os.Unsetenv(testEnvKey))
+			}
+
+			got, err := config.EnvBoolForTest(
+				testEnvKey, tt.defaultValue,
+			)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), testEnvKey)
+				assert.Contains(t, err.Error(), tt.value)
+
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}
+
+func TestEnvPositiveInt(t *testing.T) {
+	const defaultValue = 7
+
+	tests := []struct {
+		name        string
+		set         bool
+		value       string
+		expectError bool
+		errIs       error
+		expected    int
+	}{
+		{
+			name:     "unset returns the default integer",
+			expected: defaultValue,
+		},
+		{
+			name:     "empty returns the default integer",
+			set:      true,
+			value:    "",
+			expected: defaultValue,
+		},
+		{
+			name:     "positive value is parsed",
+			set:      true,
+			value:    "42",
+			expected: 42,
+		},
+		{
+			name:        "unparseable value is rejected",
+			set:         true,
+			value:       "not-a-number",
+			expectError: true,
+		},
+		{
+			name:        "zero is rejected",
+			set:         true,
+			value:       "0",
+			expectError: true,
+			errIs:       config.ErrNonPositiveValue,
+		},
+		{
+			name:        "negative is rejected",
+			set:         true,
+			value:       "-5",
+			expectError: true,
+			errIs:       config.ErrNonPositiveValue,
+		},
+	}
+
+	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 {
+				require.NoError(t, os.Unsetenv(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)
+		})
+	}
+}
+
+func TestEnvPort(t *testing.T) {
+	const defaultValue = 8080
+
+	tests := []struct {
+		name        string
+		set         bool
+		value       string
+		expectError bool
+		errIs       error
+		expected    int
+	}{
+		{
+			name:     "unset returns the default port",
+			expected: defaultValue,
+		},
+		{
+			name:     "valid port is parsed",
+			set:      true,
+			value:    "9000",
+			expected: 9000,
+		},
+		{
+			name:     "highest port is accepted",
+			set:      true,
+			value:    "65535",
+			expected: 65535,
+		},
+		{
+			name:        "unparseable value is rejected",
+			set:         true,
+			value:       "not-a-port",
+			expectError: true,
+		},
+		{
+			name:        "zero is rejected",
+			set:         true,
+			value:       "0",
+			expectError: true,
+			errIs:       config.ErrNonPositiveValue,
+		},
+		{
+			name:        "above the port range is rejected",
+			set:         true,
+			value:       "65536",
+			expectError: true,
+			errIs:       config.ErrInvalidPort,
+		},
+	}
+
+	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 {
+				require.NoError(t, os.Unsetenv(testEnvKey))
+			}
+
+			got, err := config.EnvPortForTest(
+				testEnvKey, defaultValue,
+			)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), testEnvKey)
+
+				if tt.errIs != nil {
+					require.ErrorIs(t, err, tt.errIs)
+				}
+
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}
+
+// buildConfig constructs a Config through fx exactly as the
+// application does, returning the config and any construction error.
+func buildConfig(t *testing.T) (*config.Config, error) {
+	t.Helper()
+
+	var cfg *config.Config
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Provide(
+			globals.New,
+			logger.New,
+			config.New,
+		),
+		fx.Populate(&cfg),
+	)
+
+	return cfg, app.Err()
+}
+
+func TestNewRejectsBadEnvValues(t *testing.T) {
+	tests := []struct {
+		name        string
+		key         string
+		value       string
+		expectError bool
+		check       func(t *testing.T, cfg *config.Config)
+	}{
+		{
+			name:  "valid PORT is used",
+			key:   envKeyPort,
+			value: "9001",
+			check: func(t *testing.T, cfg *config.Config) {
+				t.Helper()
+				assert.Equal(t, 9001, cfg.Port)
+			},
+		},
+		{
+			name:        "unparseable PORT aborts startup",
+			key:         envKeyPort,
+			value:       "eighty-eighty",
+			expectError: true,
+		},
+		{
+			name:        "out-of-range PORT aborts startup",
+			key:         envKeyPort,
+			value:       "70000",
+			expectError: true,
+		},
+		{
+			name:  "valid DEBUG is used",
+			key:   envKeyDebug,
+			value: "true",
+			check: func(t *testing.T, cfg *config.Config) {
+				t.Helper()
+				assert.True(t, cfg.Debug)
+			},
+		},
+		{
+			name:        "unparseable DEBUG aborts startup",
+			key:         envKeyDebug,
+			value:       "ture",
+			expectError: true,
+		},
+		{
+			name:        "unparseable MAINTENANCE_MODE aborts startup",
+			key:         envKeyMaintenanceMode,
+			value:       "sometimes",
+			expectError: true,
+		},
+	}
+
+	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.
+			t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
+			t.Setenv(tt.key, tt.value)
+
+			cfg, err := buildConfig(t)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.key)
+				assert.Contains(t, err.Error(), tt.value)
+
+				return
+			}
+
+			require.NoError(t, err)
+			require.NotNil(t, cfg)
+			tt.check(t, cfg)
+		})
+	}
+}
+
+// TestNewUsesDefaultsWhenUnset proves the fail-loud behaviour did not
+// break the legitimate unset case: absent variables still get their
+// documented defaults.
+func TestNewUsesDefaultsWhenUnset(t *testing.T) {
+	t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
+
+	for _, key := range []string{
+		envKeyPort, envKeyDebug, envKeyMaintenanceMode,
+	} {
+		require.NoError(t, os.Unsetenv(key))
+	}
+
+	cfg, err := buildConfig(t)
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+
+	assert.Equal(t, 8080, cfg.Port)
+	assert.False(t, cfg.Debug)
+	assert.False(t, cfg.MaintenanceMode)
+}
--- a/internal/config/export_test.go
+++ b/internal/config/export_test.go
@@ -0,0 +1,20 @@
+package config
+
+// This file exposes the unexported environment parsing helpers to
+// the external config_test package so each helper can be covered by
+// its own table-driven test without weakening the package API.
+
+// EnvBoolForTest exposes envBool.
+func EnvBoolForTest(key string, defaultValue bool) (bool, error) {
+	return envBool(key, defaultValue)
+}
+
+// EnvPositiveIntForTest exposes envPositiveInt.
+func EnvPositiveIntForTest(key string, defaultValue int) (int, error) {
+	return envPositiveInt(key, defaultValue)
+}
+
+// EnvPortForTest exposes envPort.
+func EnvPortForTest(key string, defaultValue int) (int, error) {
+	return envPort(key, defaultValue)
+}
--- a/internal/database/database_test.go
+++ b/internal/database/database_test.go
@@ -18,11 +18,6 @@ const (
 	testVersion = "test"
 	// testContentType is the event content type used in tests.
 	testContentType = "application/json"
-	// testWebhookName is the Webhook.Name used in tests.
-	testWebhookName = "test-webhook"
-	// testForeverLabel is Webhook.RetentionLabel for a retain-forever
-	// webhook.
-	testForeverLabel = "forever"
 )

 func setupTestDB(
--- a/internal/database/model_webhook.go
+++ b/internal/database/model_webhook.go
@@ -1,39 +1,6 @@
 package database

-import (
-	"strconv"
-
-	"gorm.io/gorm"
-)
-
-const (
-	// DefaultRetentionDays is the event retention period applied to a
-	// webhook created without an explicit retention value. It is the
-	// single source of truth for that policy and must stay in sync
-	// with the `gorm:"default:30"` column default on
-	// Webhook.RetentionDays below; a struct tag cannot reference a
-	// constant, so a test asserts the two agree.
-	DefaultRetentionDays = 30
-
-	// RetentionForeverDays is the sentinel RetentionDays value meaning
-	// "retain events forever". Users express that intent as 0, which
-	// Webhook.BeforeSave rewrites to this value: the column default
-	// substitutes DefaultRetentionDays for a zero value at insert
-	// time, so a zero can never survive a round trip to the database.
-	// Nothing outside this file may hardcode the number.
-	RetentionForeverDays = 365 * 1000
-)
-
 // Webhook represents a webhook processing unit that groups entrypoints and targets
-//
-// The receiver kinds below are deliberately mixed. BeforeSave has to
-// take a pointer because it mutates the record, and GORM only invokes
-// hooks declared that way. RetainsForever and RetentionLabel have to
-// take values because html/template calls them on webhooks held in a
-// template data map, which reflection cannot address; a pointer
-// receiver there fails at render time rather than at compile time.
-//
-//nolint:recvcheck // GORM needs a pointer hook; templates need values.
 type Webhook struct {
 	BaseModel

@@ -41,9 +8,7 @@ type Webhook struct {
 	Name        string `gorm:"not null"           json:"name"`
 	Description string `json:"description"`

-	// RetentionDays is the number of days to retain events. A value of
-	// RetentionForeverDays means retain forever. The column default
-	// must equal DefaultRetentionDays.
+	// RetentionDays is the number of days to retain events.
 	RetentionDays int `gorm:"default:30" json:"retentionDays"`

 	// Relations
@@ -51,44 +16,3 @@ type Webhook struct {
 	Entrypoints []Entrypoint `json:"entrypoints,omitempty"`
 	Targets     []Target     `json:"targets,omitempty"`
 }
-
-// BeforeSave normalises RetentionDays on every insert and update. A
-// non-positive value is the user's way of asking for "retain forever",
-// which is stored as the RetentionForeverDays sentinel.
-//
-// This has to happen in a hook rather than at the call sites. GORM
-// substitutes the column default (DefaultRetentionDays) for a zero
-// value while building the insert statement, which runs after
-// BeforeSave; rewriting any later than this loses that race and the
-// row lands at 30 days. Living on the model also means a future call
-// site — a REST API, a fixture, a migration — cannot bypass it.
-func (w *Webhook) BeforeSave(_ *gorm.DB) error {
-	if w.RetentionDays <= 0 {
-		w.RetentionDays = RetentionForeverDays
-	}
-
-	return nil
-}
-
-// RetainsForever reports whether this webhook's events are kept
-// indefinitely. It accepts the RetentionForeverDays sentinel written by
-// BeforeSave and, defensively, the non-positive values that rows
-// written before the sentinel existed may still carry.
-func (w Webhook) RetainsForever() bool {
-	return w.RetentionDays <= 0 ||
-		w.RetentionDays >= RetentionForeverDays
-}
-
-// RetentionLabel returns the webhook's retention policy as display
-// text, so that no template has to know about the sentinel value.
-func (w Webhook) RetentionLabel() string {
-	if w.RetainsForever() {
-		return "forever"
-	}
-
-	if w.RetentionDays == 1 {
-		return "1 day"
-	}
-
-	return strconv.Itoa(w.RetentionDays) + " days"
-}
--- a/internal/database/model_webhook_test.go
+++ b/internal/database/model_webhook_test.go
@@ -1,184 +0,0 @@
-package database_test
-
-import (
-	"context"
-	"reflect"
-	"strconv"
-	"testing"
-
-	"github.com/google/uuid"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// startedTestDB returns a started main database for model-level tests.
-func startedTestDB(t *testing.T) *gorm.DB {
-	t.Helper()
-
-	db, lc := setupTestDB(t)
-
-	ctx := context.Background()
-	require.NoError(t, lc.Start(ctx))
-	t.Cleanup(func() { require.NoError(t, lc.Stop(ctx)) })
-
-	return db.DB()
-}
-
-// storedRetention reads the retention_days column straight out of the
-// row, so the assertion is about what was persisted rather than about
-// whatever the in-memory struct happens to hold.
-func storedRetention(t *testing.T, db *gorm.DB, id string) int {
-	t.Helper()
-
-	var got int
-
-	require.NoError(
-		t,
-		db.Model(&database.Webhook{}).
-			Where("id = ?", id).
-			Pluck("retention_days", &got).Error,
-	)
-
-	return got
-}
-
-// newWebhookWithRetention creates a webhook through the ordinary Create
-// path, so the BeforeSave hook and the GORM column default both apply
-// exactly as they do in production.
-func newWebhookWithRetention(
-	t *testing.T,
-	db *gorm.DB,
-	wh *database.Webhook,
-) string {
-	t.Helper()
-
-	wh.UserID = uuid.New().String()
-	wh.Name = testWebhookName
-
-	require.NoError(
-		t,
-		db.Omit(clause.Associations).Create(wh).Error,
-	)
-
-	return wh.ID
-}
-
-func TestWebhookBeforeSave_ZeroBecomesForeverSentinel(t *testing.T) {
-	t.Parallel()
-
-	db := startedTestDB(t)
-
-	wh := &database.Webhook{RetentionDays: 0}
-	id := newWebhookWithRetention(t, db, wh)
-
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetention(t, db, id),
-		"a zero retention must be stored as the sentinel, "+
-			"not replaced by the column default",
-	)
-}
-
-func TestWebhookBeforeSave_NegativeBecomesForeverSentinel(t *testing.T) {
-	t.Parallel()
-
-	db := startedTestDB(t)
-
-	wh := &database.Webhook{RetentionDays: -5}
-	id := newWebhookWithRetention(t, db, wh)
-
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetention(t, db, id),
-	)
-}
-
-func TestWebhookBeforeSave_PositiveIsPreserved(t *testing.T) {
-	t.Parallel()
-
-	db := startedTestDB(t)
-
-	wh := &database.Webhook{RetentionDays: 7}
-	id := newWebhookWithRetention(t, db, wh)
-
-	assert.Equal(t, 7, storedRetention(t, db, id))
-}
-
-// TestWebhookBeforeSave_UpdateToZeroBecomesSentinel proves the hook
-// fires on update as well as insert, via the same Save call the edit
-// handler makes.
-func TestWebhookBeforeSave_UpdateToZeroBecomesSentinel(t *testing.T) {
-	t.Parallel()
-
-	db := startedTestDB(t)
-
-	wh := &database.Webhook{RetentionDays: 30}
-	id := newWebhookWithRetention(t, db, wh)
-	require.Equal(t, 30, storedRetention(t, db, id))
-
-	wh.RetentionDays = 0
-	require.NoError(t, db.Omit(clause.Associations).Save(wh).Error)
-
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetention(t, db, id),
-	)
-}
-
-// TestWebhookRetentionColumnDefaultMatchesConstant guards the one place
-// the default lives twice: a struct tag cannot reference a constant, so
-// this asserts the tag and DefaultRetentionDays agree.
-func TestWebhookRetentionColumnDefaultMatchesConstant(t *testing.T) {
-	t.Parallel()
-
-	field, ok := reflect.TypeFor[database.Webhook]().
-		FieldByName("RetentionDays")
-	require.True(t, ok, "Webhook.RetentionDays must exist")
-
-	assert.Equal(
-		t,
-		"default:"+strconv.Itoa(database.DefaultRetentionDays),
-		field.Tag.Get("gorm"),
-	)
-}
-
-func TestWebhookRetainsForeverAndLabel(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name    string
-		days    int
-		forever bool
-		label   string
-	}{
-		{
-			"sentinel",
-			database.RetentionForeverDays, true, testForeverLabel,
-		},
-		{
-			"above sentinel",
-			database.RetentionForeverDays + 1, true, testForeverLabel,
-		},
-		{"legacy zero", 0, true, testForeverLabel},
-		{"legacy negative", -1, true, testForeverLabel},
-		{"default", database.DefaultRetentionDays, false, "30 days"},
-		{"one day", 1, false, "1 day"},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			wh := database.Webhook{RetentionDays: tc.days}
-
-			assert.Equal(t, tc.forever, wh.RetainsForever())
-			assert.Equal(t, tc.label, wh.RetentionLabel())
-		})
-	}
-}
--- a/internal/database/retention.go
+++ b/internal/database/retention.go
@@ -114,8 +114,7 @@ func (r *RetentionReaper) run(ctx context.Context) {
 }

 // sweep lists every webhook from the main database and reaps expired
-// rows from each per-webhook database that has a finite retention
-// policy. Webhooks set to retain forever are skipped entirely.
+// rows from each per-webhook database whose RetentionDays is positive.
 func (r *RetentionReaper) sweep(ctx context.Context) {
 	var webhooks []Webhook

@@ -140,13 +139,8 @@ func (r *RetentionReaper) sweep(ctx context.Context) {

 		wh := webhooks[i]

-		// Skip retain-forever webhooks before building any query.
-		// RetainsForever covers both the RetentionForeverDays
-		// sentinel and the non-positive values that predate it: the
-		// sentinel is a positive number, so without this the reaper
-		// would compute a cutoff a thousand years in the past and
-		// issue a DELETE matching nothing on every single sweep.
-		if wh.RetainsForever() {
+		// RetentionDays of zero or less means retain forever.
+		if wh.RetentionDays <= 0 {
 			continue
 		}

--- a/internal/database/retention_test.go
+++ b/internal/database/retention_test.go
@@ -77,7 +77,7 @@ func createWebhook(

 	wh := &database.Webhook{
 		UserID:        uuid.New().String(),
-		Name:          testWebhookName,
+		Name:          "test-webhook",
 		RetentionDays: retentionDays,
 	}
 	require.NoError(
@@ -85,11 +85,10 @@ func createWebhook(
 		db.Omit(clause.Associations).Create(wh).Error,
 	)

-	// Webhook.BeforeSave rewrites a non-positive RetentionDays to the
-	// retain-forever sentinel, and the column's GORM default would
-	// otherwise substitute 30. Force the requested value with a
-	// column-level update so tests can plant legacy rows that predate
-	// the sentinel and still carry a literal 0 or negative value.
+	// The RetentionDays column carries a GORM default of 30, so a
+	// zero (or negative) value passed to Create is replaced by that
+	// default. Force the requested value explicitly so the
+	// retain-forever (<= 0) path can be exercised.
 	require.NoError(
 		t,
 		db.Model(wh).
@@ -99,30 +98,6 @@ func createWebhook(
 	return wh.ID
 }

-// createWebhookNormally inserts a webhook through the ordinary Create
-// path, with no column-level forcing, so Webhook.BeforeSave applies
-// exactly as it does in production. Passing 0 therefore yields a row
-// holding the RetentionForeverDays sentinel.
-func createWebhookNormally(
-	t *testing.T,
-	db *gorm.DB,
-	retentionDays int,
-) string {
-	t.Helper()
-
-	wh := &database.Webhook{
-		UserID:        uuid.New().String(),
-		Name:          testWebhookName,
-		RetentionDays: retentionDays,
-	}
-	require.NoError(
-		t,
-		db.Omit(clause.Associations).Create(wh).Error,
-	)
-
-	return wh.ID
-}
-
 // eventChain is the set of row IDs seeded for a single event.
 type eventChain struct {
 	eventID    string
@@ -281,59 +256,12 @@ func TestRetentionReaper_ReapsExpiredKeepsRecent(t *testing.T) {
 	assertChainPresent(t, db, recent)
 }

-// TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep covers the
-// end-to-end retain-forever path: a webhook created the normal way with
-// a requested retention of 0 lands on the RetentionForeverDays
-// sentinel, and the reaper leaves its ancient events alone while still
-// reaping a finite-retention webhook in the very same sweep.
-func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupRetentionTest(t)
-
-	foreverID := createWebhookNormally(t, env.mainDB.DB(), 0)
-
-	var stored database.Webhook
-
-	require.NoError(
-		t,
-		env.mainDB.DB().Where("id = ?", foreverID).
-			First(&stored).Error,
-	)
-	require.Equal(
-		t,
-		database.RetentionForeverDays,
-		stored.RetentionDays,
-		"a requested retention of 0 must persist as the sentinel",
-	)
-
-	finiteID := createWebhookNormally(t, env.mainDB.DB(), 30)
-
-	foreverDB, err := env.mgr.GetDB(foreverID)
-	require.NoError(t, err)
-
-	finiteDB, err := env.mgr.GetDB(finiteID)
-	require.NoError(t, err)
-
-	ancient := time.Now().Add(-365 * 24 * time.Hour)
-	kept := seedEventChain(t, foreverDB, foreverID, ancient)
-	doomed := seedEventChain(t, finiteDB, finiteID, ancient)
-
-	env.reaper.ExportSweep(context.Background())
-
-	assertChainPresent(t, foreverDB, kept)
-	assertChainGone(t, finiteDB, doomed)
-}
-
 func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
 	t.Parallel()

 	env := setupRetentionTest(t)

-	// A legacy row written before the sentinel existed still carries a
-	// literal 0; the <= 0 guard must keep honouring it.
+	// RetentionDays of zero means retain forever.
 	webhookID := createWebhook(t, env.mainDB.DB(), 0)

 	db, err := env.mgr.GetDB(webhookID)
--- a/internal/handlers/handlers.go
+++ b/internal/handlers/handlers.go
@@ -26,6 +26,8 @@ const (
 	maxBodyShift = 20
 	// recentEventLimit is the number of recent events to show.
 	recentEventLimit = 20
+	// defaultRetentionDays is the default event retention period.
+	defaultRetentionDays = 30
 	// paginationPerPage is the number of items per page.
 	paginationPerPage = 25

--- a/internal/handlers/source_management.go
+++ b/internal/handlers/source_management.go
@@ -25,37 +25,6 @@ type WebhookListItem struct {
 // errMissingURL signals that a required URL was not provided.
 var errMissingURL = errors.New("missing URL")

-// errInvalidRetention signals a retention_days form value that is not
-// a non-negative whole number.
-var errInvalidRetention = errors.New("invalid retention days")
-
-// retentionErrorMessage is what the create and edit forms show the user
-// when parseRetentionDays returns errInvalidRetention.
-const retentionErrorMessage = "Retention must be a whole number of " +
-	"days, or 0 to retain events forever."
-
-// parseRetentionDays interprets a retention_days form value.
-//
-// An empty value yields fallback, which lets the create path apply the
-// default and the edit path leave the stored value unchanged. A value
-// of 0 is returned as 0 and is rewritten to the retain-forever
-// sentinel by database.Webhook's BeforeSave hook, so no handler needs
-// to know the sentinel. Anything unparseable or negative is an error
-// rather than a silently substituted default.
-func parseRetentionDays(raw string, fallback int) (int, error) {
-	raw = strings.TrimSpace(raw)
-	if raw == "" {
-		return fallback, nil
-	}
-
-	v, err := strconv.Atoi(raw)
-	if err != nil || v < 0 {
-		return 0, errInvalidRetention
-	}
-
-	return v, nil
-}
-
 // EventWithDeliveries holds an event and its deliveries.
 type EventWithDeliveries struct {
 	database.Event
@@ -137,20 +106,11 @@ func (h *Handlers) buildWebhookListItems(
 // HandleSourceCreate shows the form to create a new webhook.
 func (h *Handlers) HandleSourceCreate() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		h.renderTemplate(
-			w, r, "sources_new.html", newSourceFormData(""),
-		)
-	}
-}
+		data := map[string]any{
+			tmplKeyError: "",
+		}

-// newSourceFormData builds the template data for the webhook creation
-// form, carrying the retention default so the pre-filled value comes
-// from database.DefaultRetentionDays rather than being a third
-// hardcoded copy of the same policy.
-func newSourceFormData(errMsg string) map[string]any {
-	return map[string]any{
-		tmplKeyError:           errMsg,
-		"DefaultRetentionDays": database.DefaultRetentionDays,
+		h.renderTemplate(w, r, "sources_new.html", data)
 	}
 }

@@ -185,26 +145,23 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
 		retentionStr := r.FormValue("retention_days")

 		if name == "" {
+			data := map[string]any{
+				tmplKeyError: "Name is required",
+			}
+
 			w.WriteHeader(http.StatusBadRequest)
-			h.renderTemplate(
-				w, r, "sources_new.html",
-				newSourceFormData("Name is required"),
-			)
+			h.renderTemplate(w, r, "sources_new.html", data)

 			return
 		}

-		retentionDays, retErr := parseRetentionDays(
-			retentionStr, database.DefaultRetentionDays,
-		)
-		if retErr != nil {
-			w.WriteHeader(http.StatusBadRequest)
-			h.renderTemplate(
-				w, r, "sources_new.html",
-				newSourceFormData(retentionErrorMessage),
-			)
+		retentionDays := defaultRetentionDays

-			return
+		if retentionStr != "" {
+			v, convErr := strconv.Atoi(retentionStr)
+			if convErr == nil && v > 0 {
+				retentionDays = v
+			}
 		}

 		h.createWebhookWithEntrypoint(
@@ -471,25 +428,7 @@ func (h *Handlers) applyWebhookEdit(

 	webhook.Name = name
 	webhook.Description = r.FormValue("description")
-
-	// An empty field falls back to the stored value, so submitting the
-	// form without touching retention leaves the policy alone.
-	retentionDays, retErr := parseRetentionDays(
-		r.FormValue("retention_days"), webhook.RetentionDays,
-	)
-	if retErr != nil {
-		data := map[string]any{
-			tmplKeyWebhook: *webhook,
-			tmplKeyError:   retentionErrorMessage,
-		}
-
-		w.WriteHeader(http.StatusBadRequest)
-		h.renderTemplate(w, r, "source_edit.html", data)
-
-		return
-	}
-
-	webhook.RetentionDays = retentionDays
+	h.parseRetention(r, webhook)

 	err := h.db.DB().Save(webhook).Error
 	if err != nil {
@@ -503,6 +442,23 @@ func (h *Handlers) applyWebhookEdit(
 	)
 }

+// parseRetention parses and applies retention_days from the
+// form.
+func (h *Handlers) parseRetention(
+	r *http.Request,
+	webhook *database.Webhook,
+) {
+	retStr := r.FormValue("retention_days")
+	if retStr == "" {
+		return
+	}
+
+	v, err := strconv.Atoi(retStr)
+	if err == nil && v > 0 {
+		webhook.RetentionDays = v
+	}
+}
+
 // HandleSourceDelete handles webhook deletion.
 func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
--- a/internal/handlers/source_management_test.go
+++ b/internal/handlers/source_management_test.go
@@ -1,481 +0,0 @@
-package handlers_test
-
-import (
-	"context"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"strconv"
-	"strings"
-	"testing"
-
-	"github.com/go-chi/chi"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-const (
-	// sourceTestUserID is the session user id used by the webhook
-	// management tests.
-	sourceTestUserID = "source-test-user"
-	// sourceIDParam is the chi URL parameter naming a webhook.
-	sourceIDParam = "sourceID"
-)
-
-// formRequest builds an urlencoded POST to path carrying the given
-// cookies, plus any chi URL parameters the handler reads.
-func formRequest(
-	path string,
-	cookies []*http.Cookie,
-	form url.Values,
-	urlParams map[string]string,
-) *http.Request {
-	req := httptest.NewRequestWithContext(
-		context.Background(),
-		http.MethodPost,
-		path,
-		strings.NewReader(form.Encode()),
-	)
-	req.Header.Set(
-		"Content-Type", "application/x-www-form-urlencoded",
-	)
-
-	for _, c := range cookies {
-		req.AddCookie(c)
-	}
-
-	rctx := chi.NewRouteContext()
-	for k, v := range urlParams {
-		rctx.URLParams.Add(k, v)
-	}
-
-	return req.WithContext(
-		context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
-	)
-}
-
-// getRequest builds a GET to path carrying the given cookies, plus any
-// chi URL parameters the handler reads.
-func getRequest(
-	t *testing.T,
-	path string,
-	cookies []*http.Cookie,
-	urlParams map[string]string,
-) *http.Request {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, path, nil,
-	)
-
-	for _, c := range cookies {
-		req.AddCookie(c)
-	}
-
-	rctx := chi.NewRouteContext()
-	for k, v := range urlParams {
-		rctx.URLParams.Add(k, v)
-	}
-
-	return req.WithContext(
-		context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
-	)
-}
-
-// submitCreate posts the webhook creation form with the given
-// retention_days value (omitted entirely when retention is nil) and
-// returns the recorder.
-func submitCreate(
-	t *testing.T,
-	h *handlers.Handlers,
-	cookies []*http.Cookie,
-	name string,
-	retention *string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	form := url.Values{}
-	form.Set("name", name)
-
-	if retention != nil {
-		form.Set("retention_days", *retention)
-	}
-
-	req := formRequest("/sources/new", cookies, form, nil)
-	w := httptest.NewRecorder()
-
-	h.HandleSourceCreateSubmit().ServeHTTP(w, req)
-
-	return w
-}
-
-// onlyWebhook loads the single webhook belonging to the test user.
-func onlyWebhook(
-	t *testing.T,
-	db *database.Database,
-) database.Webhook {
-	t.Helper()
-
-	var webhooks []database.Webhook
-
-	require.NoError(
-		t,
-		db.DB().Where("user_id = ?", sourceTestUserID).
-			Find(&webhooks).Error,
-	)
-	require.Len(t, webhooks, 1)
-
-	return webhooks[0]
-}
-
-// seedWebhook inserts a webhook owned by the test user with an exact
-// stored retention value, bypassing Webhook.BeforeSave via a
-// column-level update so that legacy rows can be planted too.
-func seedWebhook(
-	t *testing.T,
-	db *database.Database,
-	retentionDays int,
-) database.Webhook {
-	t.Helper()
-
-	wh := &database.Webhook{
-		UserID:        sourceTestUserID,
-		Name:          "seeded",
-		RetentionDays: retentionDays,
-	}
-	require.NoError(
-		t,
-		db.DB().Omit(clause.Associations).Create(wh).Error,
-	)
-	require.NoError(
-		t,
-		db.DB().Model(wh).
-			Update("retention_days", retentionDays).Error,
-	)
-
-	wh.RetentionDays = retentionDays
-
-	return *wh
-}
-
-// storedRetentionDays reads the retention_days column for a webhook.
-func storedRetentionDays(
-	t *testing.T,
-	db *database.Database,
-	id string,
-) int {
-	t.Helper()
-
-	var got int
-
-	require.NoError(
-		t,
-		db.DB().Model(&database.Webhook{}).
-			Where("id = ?", id).
-			Pluck("retention_days", &got).Error,
-	)
-
-	return got
-}
-
-// sourceTestEnv bundles the handler, session, and database a webhook
-// management test drives.
-type sourceTestEnv struct {
-	handlers *handlers.Handlers
-	db       *database.Database
-	cookies  []*http.Cookie
-}
-
-func setupSourceTest(t *testing.T) *sourceTestEnv {
-	t.Helper()
-
-	var h *handlers.Handlers
-
-	var sess *session.Session
-
-	var db *database.Database
-
-	app := newTestApp(t, &h, &sess, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	return &sourceTestEnv{
-		handlers: h,
-		db:       db,
-		cookies: authenticatedCookies(
-			t, sess, sourceTestUserID, "sourceuser",
-		),
-	}
-}
-
-// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
-// regression test for the bug: the create form's 0 must reach the
-// database as the retain-forever sentinel rather than being replaced by
-// the column's default of 30.
-func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	zero := "0"
-
-	w := submitCreate(t, env.handlers, env.cookies, "forever", &zero)
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
-	assert.True(t, wh.RetainsForever())
-}
-
-func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.DefaultRetentionDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
-}
-
-// TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
-// form's pre-filled retention from becoming a third hardcoded copy of
-// the 30-day policy.
-func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	w := httptest.NewRecorder()
-	env.handlers.HandleSourceCreate().ServeHTTP(
-		w, getRequest(t, "/sources/new", env.cookies, nil),
-	)
-
-	require.Equal(t, http.StatusOK, w.Code)
-
-	body := w.Body.String()
-
-	assert.Contains(
-		t, body,
-		`value="`+strconv.Itoa(database.DefaultRetentionDays)+`"`,
-	)
-	assert.NotContains(
-		t, body, `max="365"`,
-		"a max below the sentinel would block retain-forever",
-	)
-	assert.Contains(t, body, `min="0"`)
-}
-
-func TestHandleSourceCreateSubmit_InvalidRetentionIsRejected(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	for _, raw := range []string{"abc", "-1", "3.5"} {
-		t.Run(raw, func(t *testing.T) {
-			t.Parallel()
-
-			env := setupSourceTest(t)
-
-			w := submitCreate(
-				t, env.handlers, env.cookies, "bad", &raw,
-			)
-
-			assert.Equal(t, http.StatusBadRequest, w.Code)
-			assert.Contains(
-				t, w.Body.String(), "Retention must be",
-			)
-
-			var count int64
-
-			require.NoError(
-				t,
-				env.db.DB().Model(&database.Webhook{}).
-					Where("user_id = ?", sourceTestUserID).
-					Count(&count).Error,
-			)
-			assert.Zero(
-				t, count,
-				"no webhook may be created from a rejected form",
-			)
-		})
-	}
-}
-
-// submitEdit posts the webhook edit form for the given webhook.
-func submitEdit(
-	t *testing.T,
-	env *sourceTestEnv,
-	wh database.Webhook,
-	retention string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	form := url.Values{}
-	form.Set("name", wh.Name)
-	form.Set("description", wh.Description)
-	form.Set("retention_days", retention)
-
-	req := formRequest(
-		"/source/"+wh.ID+"/edit",
-		env.cookies,
-		form,
-		map[string]string{sourceIDParam: wh.ID},
-	)
-	w := httptest.NewRecorder()
-
-	env.handlers.HandleSourceEditSubmit().ServeHTTP(w, req)
-
-	return w
-}
-
-func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
-
-	w := submitEdit(t, env, wh, "0")
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
-}
-
-func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
-
-	w := submitEdit(t, env, wh, "not-a-number")
-
-	assert.Equal(t, http.StatusBadRequest, w.Code)
-	assert.Contains(t, w.Body.String(), "Retention must be")
-	assert.Equal(
-		t,
-		database.DefaultRetentionDays,
-		storedRetentionDays(t, env.db, wh.ID),
-		"a rejected form must not change the stored retention",
-	)
-}
-
-func TestHandleSourceEditSubmit_EmptyRetentionLeavesValueUnchanged(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhook(t, env.db, 7)
-
-	w := submitEdit(t, env, wh, "")
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	assert.Equal(t, 7, storedRetentionDays(t, env.db, wh.ID))
-}
-
-// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
-// the removed max="365" cap used to break: render the edit form for a
-// retain-forever webhook, confirm the pre-filled sentinel is not capped
-// by browser validation, then submit that pre-filled value straight
-// back and confirm the retention policy survives untouched.
-func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhook(t, env.db, database.RetentionForeverDays)
-
-	req := getRequest(
-		t, "/source/"+wh.ID+"/edit", env.cookies,
-		map[string]string{sourceIDParam: wh.ID},
-	)
-	w := httptest.NewRecorder()
-	env.handlers.HandleSourceEdit().ServeHTTP(w, req)
-
-	require.Equal(t, http.StatusOK, w.Code)
-
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
-	body := w.Body.String()
-
-	assert.Contains(
-		t, body, `value="`+sentinel+`"`,
-		"the edit form pre-fills the stored retention",
-	)
-	assert.NotContains(
-		t, body, `max="365"`,
-		"a max below the sentinel would block saving any edit",
-	)
-	assert.Contains(
-		t, body, "forever",
-		"the form explains what the sentinel means",
-	)
-
-	// Submit the pre-filled value back, exactly as a browser would.
-	post := submitEdit(t, env, wh, sentinel)
-	require.Equal(t, http.StatusSeeOther, post.Code)
-
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
-}
-
-// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
-// the retain-forever value is never rendered to the user as a raw day
-// count on either read-only view.
-func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhook(t, env.db, database.RetentionForeverDays)
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
-
-	listW := httptest.NewRecorder()
-	env.handlers.HandleSourceList().ServeHTTP(
-		listW, getRequest(t, "/sources", env.cookies, nil),
-	)
-
-	require.Equal(t, http.StatusOK, listW.Code)
-	assert.Contains(t, listW.Body.String(), "Retention: forever")
-	assert.NotContains(t, listW.Body.String(), sentinel)
-
-	detailW := httptest.NewRecorder()
-	env.handlers.HandleSourceDetail().ServeHTTP(
-		detailW,
-		getRequest(
-			t, "/source/"+wh.ID, env.cookies,
-			map[string]string{sourceIDParam: wh.ID},
-		),
-	)
-
-	require.Equal(t, http.StatusOK, detailW.Code)
-	assert.Contains(t, detailW.Body.String(), "Retention: forever")
-	assert.NotContains(t, detailW.Body.String(), sentinel)
-}
--- a/templates/source_detail.html
+++ b/templates/source_detail.html
@@ -181,7 +181,7 @@

    <!-- Info -->
    <div class="mt-4 text-sm text-gray-400">
-        <p>Retention: {{.Webhook.RetentionLabel}} &middot; Created: {{.Webhook.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</p>
+        <p>Retention: {{.Webhook.RetentionDays}} days &middot; Created: {{.Webhook.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</p>
    </div>
 </div>
 {{end}}
--- a/templates/source_edit.html
+++ b/templates/source_edit.html
@@ -28,8 +28,7 @@

            <div class="form-group">
                <label for="retention_days" class="label">Retention (days)</label>
-                <input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input">
-                <p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}. Enter 0 to retain events forever.</p>
+                <input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="1" max="365" class="input">
            </div>

            <div class="flex gap-3">
--- a/templates/sources_list.html
+++ b/templates/sources_list.html
@@ -25,7 +25,7 @@
                    <p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
                    {{end}}
                </div>
-                <span class="badge-info">Retention: {{.RetentionLabel}}</span>
+                <span class="badge-info">{{.RetentionDays}}d retention</span>
            </div>
            <div class="flex gap-6 mt-4 text-sm text-gray-500">
                <span>{{.EntrypointCount}} entrypoint{{if ne .EntrypointCount 1}}s{{end}}</span>
--- a/templates/sources_new.html
+++ b/templates/sources_new.html
@@ -28,8 +28,8 @@

            <div class="form-group">
                <label for="retention_days" class="label">Retention (days)</label>
-                <input type="number" id="retention_days" name="retention_days" value="{{.DefaultRetentionDays}}" min="0" class="input">
-                <p class="text-xs text-gray-500 mt-1">How long to keep event data. Enter 0 to retain events forever.</p>
+                <input type="number" id="retention_days" name="retention_days" value="30" min="1" max="365" class="input">
+                <p class="text-xs text-gray-500 mt-1">How long to keep event data.</p>
            </div>

            <div class="flex gap-3">