Warn when TRUSTED_PROXIES is empty in production (closes #149)

With no trusted proxies configured, every client behind the reverse proxy production requires shares one rate-limit bucket per limit, so five POSTs per minute from anywhere holds the login limit full and denies the admin login until restart. The default is still correct; it was the consequence that was invisible. Startup now warns, and the README no longer claims the login limit is per-IP unconditionally.

internal/config/config.go

@@ -422,6 +422,38 @@ func loadFromEnv() (*Config, error) {
 	}, nil
 }
 
+// warnSharedRateLimitBucket logs a startup warning when a production
+// deployment leaves TRUSTED_PROXIES empty.
+//
+// With no trusted proxies every rate limiter keys on the connecting
+// peer's address. A production deployment is required to run behind a
+// TLS-terminating reverse proxy, and the peer is then that proxy for
+// every request, so all clients share one bucket per limiter. The
+// login limiter's bucket is the dangerous one: any remote client can
+// keep it full, which denies the only administrative login to
+// everyone until the process restarts.
+//
+// The default of trusting nobody is deliberate — trusting forwarded
+// headers from arbitrary peers lets any client choose its own bucket —
+// so this warns rather than failing startup or changing the key.
+func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) {
+	if !c.IsProd() || len(c.TrustedProxies) > 0 {
+		return
+	}
+
+	log.Warn(
+		"TRUSTED_PROXIES is empty: rate limits key on the "+
+			"connecting peer, so behind the reverse proxy a "+
+			"production deployment runs behind, every client "+
+			"shares one bucket per limit. Any remote client can "+
+			"then keep the login limit full and deny the admin "+
+			"login, the only administrative path, until restart. "+
+			"Set TRUSTED_PROXIES to your reverse proxy's address.",
+		"environment", c.Environment,
+		"trustedProxies", len(c.TrustedProxies),
+	)
+}
+
 // New creates a Config by reading environment variables.
 //
 //nolint:revive // lc parameter is required by fx even if unused.
@@ -466,5 +498,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		s.MetricsUsername != "" && s.MetricsPassword != "",
 	)
 
+	s.warnSharedRateLimitBucket(log)
+
 	return s, nil
 }

internal/config/config_test.go

@@ -1,6 +1,8 @@
 package config_test
 
 import (
+	"bytes"
+	"log/slog"
 	"os"
 	"testing"
 	"time"
@@ -624,3 +626,79 @@ func testTrustedProxiesSuccess(
 
 	assert.Equal(t, expected, got)
 }
+
+// TestSharedRateLimitBucketWarning covers the startup warning that
+// tells an operator their production deployment shares one rate-limit
+// bucket between every client, which makes the admin login remotely
+// deniable. It must fire when TRUSTED_PROXIES is empty in production
+// and stay quiet otherwise.
+func TestSharedRateLimitBucketWarning(t *testing.T) {
+	tests := []struct {
+		name           string
+		environment    string
+		trustedProxies string
+		expectWarning  bool
+	}{
+		{
+			name:          "prod without trusted proxies warns",
+			environment:   config.EnvironmentProd,
+			expectWarning: true,
+		},
+		{
+			name:           "prod with trusted proxies is quiet",
+			environment:    config.EnvironmentProd,
+			trustedProxies: cidrPrivateV4,
+			expectWarning:  false,
+		},
+		{
+			// Development is not required to run behind a
+			// reverse proxy, so the shared bucket the warning
+			// describes is not the expected shape there.
+			name:          "dev without trusted proxies is quiet",
+			environment:   config.EnvironmentDev,
+			expectWarning: false,
+		},
+	}
+
+	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", tt.environment)
+
+			if tt.trustedProxies == "" {
+				require.NoError(
+					t, os.Unsetenv("TRUSTED_PROXIES"),
+				)
+			} else {
+				t.Setenv("TRUSTED_PROXIES", tt.trustedProxies)
+			}
+
+			var buf bytes.Buffer
+
+			log := slog.New(slog.NewJSONHandler(
+				&buf, &slog.HandlerOptions{
+					Level: slog.LevelDebug,
+				},
+			))
+
+			require.NoError(
+				t,
+				config.WarnSharedRateLimitBucketForTest(log),
+			)
+
+			if !tt.expectWarning {
+				assert.Empty(t, buf.String())
+
+				return
+			}
+
+			logged := buf.String()
+
+			assert.Contains(t, logged, `"level":"WARN"`)
+			assert.Contains(t, logged, "TRUSTED_PROXIES")
+			assert.Contains(t, logged, "shares one bucket")
+			assert.Contains(t, logged, "deny the admin login")
+		})
+	}
+}

internal/config/export_test.go

@@ -1,9 +1,26 @@
 package config
 
+import "log/slog"
+
 // 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.
 
+// WarnSharedRateLimitBucketForTest loads a Config from the current
+// environment and emits its startup warnings to log. The real logger
+// writes to stdout, so this lets the warning's firing condition be
+// asserted against a handler the test controls.
+func WarnSharedRateLimitBucketForTest(log *slog.Logger) error {
+	c, err := loadFromEnv()
+	if err != nil {
+		return err
+	}
+
+	c.warnSharedRateLimitBucket(log)
+
+	return nil
+}
+
 // EnvBoolForTest exposes envBool.
 func EnvBoolForTest(key string, defaultValue bool) (bool, error) {
 	return envBool(key, defaultValue)