Warn when production shares one rate-limit bucket (closes #149)
All checks were successful
check / check (push) Successful in 3m7s

With TRUSTED_PROXIES empty, every rate limiter keys on the connecting
peer. Production runs behind a TLS-terminating reverse proxy, so the
peer is that proxy for every request and all clients share one bucket
per limit. For the login limiter that means any remote client sending
five POSTs a minute holds the only administrative login at HTTP 429.

The empty default is correct — trusting forwarded headers from
arbitrary peers lets any client choose its own bucket — so this makes
the consequence visible rather than changing the keying, the limits or
the default:

- config logs a WARN at startup when the environment is prod and
  TRUSTED_PROXIES is empty, naming the variable, the shared bucket and
  the deniable admin login.
- The security-feature bullet's "per IP" login claim is now conditional
  on TRUSTED_PROXIES, which is the only case where it holds.
- The rate-limiting section separates the receiver case (sharing costs
  throughput, the safe direction) from the login case (sharing costs
  availability of the only admin path, not safe).
- The trusted-proxies configuration section states the consequence and
  names TRUSTED_PROXIES as the remedy.
This commit is contained in:
2026-08-12 11:42:04 +00:00
parent 339548d794
commit 0beeddd475
4 changed files with 175 additions and 13 deletions

View File

@@ -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")
})
}
}