Rate-limit the public webhook receiver endpoint (closes #64)
All checks were successful
check / check (push) Successful in 4m6s
All checks were successful
check / check (push) Successful in 4m6s
The public receiver /webhook/{uuid} had no rate limiting: anyone
who learns an entrypoint UUID can flood it, inflating the
per-webhook database and the delivery queue.
Add a dedicated limit scoped to the receiver route, keyed per
client IP per request path (the path contains the entrypoint
UUID), so one misbehaving sender is throttled without affecting
other senders of the same entrypoint or other entrypoints.
Requests over the limit get a 429; httprate adds the Retry-After
header per RFC 6585. IP extraction honours X-Forwarded-For,
X-Real-IP, and True-Client-IP for reverse-proxy deployments.
The limit is RECEIVER_RATE_LIMIT requests per minute, default
120, parsed with the existing envPositiveInt strict parser: a
set-but-unparseable or non-positive value aborts startup rather
than silently falling back to the default.
Also update the README env table and Rate Limiting design
section.
This commit is contained in:
@@ -35,6 +35,13 @@ const (
|
||||
// authenticated activity before it expires.
|
||||
defaultSessionIdleTimeout = 24 * time.Hour
|
||||
|
||||
// defaultReceiverRateLimit is the default number of requests
|
||||
// per minute each client IP may send to a single webhook
|
||||
// receiver entrypoint. Generous for legitimate webhook
|
||||
// senders while bounding abuse of the one unauthenticated,
|
||||
// internet-exposed endpoint.
|
||||
defaultReceiverRateLimit = 120
|
||||
|
||||
// maxPort is the highest valid TCP port number. The lower
|
||||
// bound (at least 1) is enforced by envPositiveInt.
|
||||
maxPort = 65535
|
||||
@@ -79,6 +86,10 @@ type Config struct {
|
||||
// which a session expires. Non-positive disables idle expiry.
|
||||
SessionIdleTimeout time.Duration
|
||||
|
||||
// ReceiverRateLimit is the number of requests per minute each
|
||||
// client IP may send to a single webhook receiver entrypoint.
|
||||
ReceiverRateLimit int
|
||||
|
||||
params *ConfigParams
|
||||
log *slog.Logger
|
||||
}
|
||||
@@ -263,6 +274,14 @@ func loadFromEnv() (*Config, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
receiverRateLimit, err := envPositiveInt(
|
||||
"RECEIVER_RATE_LIMIT",
|
||||
defaultReceiverRateLimit,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return &Config{
|
||||
DataDir: envString("DATA_DIR"),
|
||||
Debug: debug,
|
||||
@@ -274,6 +293,7 @@ func loadFromEnv() (*Config, error) {
|
||||
SentryDSN: envString("SENTRY_DSN"),
|
||||
RetentionSweepInterval: retentionSweepInterval,
|
||||
SessionIdleTimeout: sessionIdleTimeout,
|
||||
ReceiverRateLimit: receiverRateLimit,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -314,6 +334,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
||||
"maintenanceMode", s.MaintenanceMode,
|
||||
"dataDir", s.DataDir,
|
||||
"retentionSweepInterval", s.RetentionSweepInterval.String(),
|
||||
"receiverRateLimit", s.ReceiverRateLimit,
|
||||
"hasSentryDSN", s.SentryDSN != "",
|
||||
"hasMetricsAuth",
|
||||
s.MetricsUsername != "" && s.MetricsPassword != "",
|
||||
|
||||
@@ -14,6 +14,14 @@ import (
|
||||
"sneak.berlin/go/webhooker/internal/logger"
|
||||
)
|
||||
|
||||
// Shared subtest names for the env-parsing tables below, which all
|
||||
// exercise the same three cases against different variables.
|
||||
const (
|
||||
caseUnsetUsesDefault = "unset uses default"
|
||||
caseValidValueParsed = "valid value is parsed"
|
||||
caseUnparseableFails = "unparseable value fails startup"
|
||||
)
|
||||
|
||||
func TestEnvironmentConfig(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -130,18 +138,18 @@ func TestRetentionSweepInterval(t *testing.T) {
|
||||
expected time.Duration
|
||||
}{
|
||||
{
|
||||
name: "unset uses default",
|
||||
name: caseUnsetUsesDefault,
|
||||
set: false,
|
||||
expected: time.Hour,
|
||||
},
|
||||
{
|
||||
name: "valid value is parsed",
|
||||
name: caseValidValueParsed,
|
||||
set: true,
|
||||
value: "15m",
|
||||
expected: 15 * time.Minute,
|
||||
},
|
||||
{
|
||||
name: "unparseable value fails startup",
|
||||
name: caseUnparseableFails,
|
||||
set: true,
|
||||
value: "not-a-duration",
|
||||
expectError: true,
|
||||
@@ -172,7 +180,7 @@ func TestRetentionSweepInterval(t *testing.T) {
|
||||
}
|
||||
|
||||
// expectStartupError asserts that fx refuses to build the app,
|
||||
// which is what a set-but-unparseable duration must cause.
|
||||
// which is what a set-but-invalid environment value must cause.
|
||||
func expectStartupError(t *testing.T) {
|
||||
t.Helper()
|
||||
|
||||
@@ -226,18 +234,18 @@ func TestSessionIdleTimeout(t *testing.T) {
|
||||
expected time.Duration
|
||||
}{
|
||||
{
|
||||
name: "unset uses default",
|
||||
name: caseUnsetUsesDefault,
|
||||
set: false,
|
||||
expected: 24 * time.Hour,
|
||||
},
|
||||
{
|
||||
name: "valid value is parsed",
|
||||
name: caseValidValueParsed,
|
||||
set: true,
|
||||
value: "30m",
|
||||
expected: 30 * time.Minute,
|
||||
},
|
||||
{
|
||||
name: "unparseable value fails startup",
|
||||
name: caseUnparseableFails,
|
||||
set: true,
|
||||
value: "not-a-duration",
|
||||
expectError: true,
|
||||
@@ -336,3 +344,91 @@ func TestDefaultDataDir(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestReceiverRateLimit(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
set bool
|
||||
value string
|
||||
expectError bool
|
||||
expected int
|
||||
}{
|
||||
{
|
||||
name: caseUnsetUsesDefault,
|
||||
set: false,
|
||||
expected: 120,
|
||||
},
|
||||
{
|
||||
name: caseValidValueParsed,
|
||||
set: true,
|
||||
value: "30",
|
||||
expected: 30,
|
||||
},
|
||||
{
|
||||
name: caseUnparseableFails,
|
||||
set: true,
|
||||
value: "not-a-number",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "zero fails startup",
|
||||
set: true,
|
||||
value: "0",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "negative fails startup",
|
||||
set: true,
|
||||
value: "-5",
|
||||
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")
|
||||
|
||||
if tt.set {
|
||||
t.Setenv("RECEIVER_RATE_LIMIT", tt.value)
|
||||
} else {
|
||||
require.NoError(t, os.Unsetenv(
|
||||
"RECEIVER_RATE_LIMIT",
|
||||
))
|
||||
}
|
||||
|
||||
if tt.expectError {
|
||||
expectStartupError(t)
|
||||
} else {
|
||||
testReceiverRateLimitSuccess(t, tt.expected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func testReceiverRateLimitSuccess(
|
||||
t *testing.T,
|
||||
expected int,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
var cfg *config.Config
|
||||
|
||||
app := fxtest.New(
|
||||
t,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
config.New,
|
||||
),
|
||||
fx.Populate(&cfg),
|
||||
)
|
||||
require.NoError(t, app.Err())
|
||||
|
||||
app.RequireStart()
|
||||
|
||||
defer app.RequireStop()
|
||||
|
||||
assert.Equal(t, expected, cfg.ReceiverRateLimit)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user