1 Commits

Author SHA1 Message Date
f32284a39f Rate-limit the public webhook receiver endpoint (closes #64)
All checks were successful
check / check (push) Successful in 2m37s
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. A set-but-unparseable or non-positive value aborts startup
via the new envPositiveInt strict parser rather than silently
falling back to the default. envInt's other callers are
unchanged; converting them is tracked in #80.

Also update the README env table and Rate Limiting design
section, and sync TODO.md.
2026-08-07 16:51:52 +00:00
7 changed files with 356 additions and 41 deletions

View File

@@ -92,6 +92,7 @@ TTY detection, and security headers are always applied.
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` | | `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` | | `SENTRY_DSN` | Sentry error reporting DSN | `""` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
On first startup, webhooker automatically generates a cryptographically On first startup, webhooker automatically generates a cryptographically
secure session encryption key and stores it in the database. This key secure session encryption key and stores it in the database. This key
@@ -676,17 +677,24 @@ just delayed until the target is healthy again.
### Rate Limiting ### Rate Limiting
Global rate limiting middleware (e.g., per-IP throttling applied at the Global blanket rate limiting middleware (e.g., a per-IP throttle shared
router level) **must not** apply to webhook receiver endpoints. Webhook with the web UI) **must not** apply to webhook receiver endpoints.
endpoints receive automated traffic from external services at Webhook endpoints receive automated traffic from external services at
unpredictable rates, and blanket rate limits would cause legitimate unpredictable rates, and blanket limits shared with other routes would
deliveries to be dropped. cause legitimate deliveries to be dropped.
Instead, each webhook has its own individually configurable rate limit, The receiver instead has its own dedicated abuse limit, scoped to the
applied within the webhook handler itself. By default, no rate limit is `/webhook/{uuid}` route only and keyed per client IP per entrypoint: one
applied — webhook endpoints accept traffic as fast as it arrives. Rate misbehaving sender is throttled without affecting other senders of the
limits can be configured per-webhook when needed (e.g., to protect same entrypoint or the same sender's other entrypoints. The limit is
against a misbehaving sender). `RECEIVER_RATE_LIMIT` requests per minute (default 120, generous for
legitimate webhook senders). Requests over the limit receive HTTP 429
with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
value aborts startup rather than silently falling back to the default.
Finer-grained per-webhook rate limits (configured in the web UI and
enforced in the webhook handler) can layer on top of this env-level
abuse limit later; they are tracked as future work.
### API Endpoints ### API Endpoints

31
TODO.md
View File

@@ -10,24 +10,27 @@
# Status # Status
pre-1.0. No git tags exist. main (afe88c6) is a working webhook proxy pre-1.0. No git tags exist. main (81413c5) is a working webhook proxy
with auth, CSRF/SSRF protections, login rate limiting, Slack target, with auth, CSRF/SSRF protections, login rate limiting, Slack target,
policy compliance (#6), and pinned lint tooling (#55). Note: TODO.md was policy compliance (#6), pinned lint tooling (#55), a per-webhook event
deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its retention reaper (#63), and delivery targets behind a Target interface
content was folded into the README TODO section, which this draft (#77). Work is tracked as Gitea issues (the authoritative TODO); this
reconstructs as of 2026-07-06. file is a summary. 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 # Next Step
Implement automatic event retention cleanup based on retention_days: a Manual event redelivery from the web UI (replay is a core promised
periodic maintenance job that deletes Events, Deliveries, and capability in the README rationale).
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.
# Completed Steps # Completed Steps
- 2026-08-07 Rate-limit the public webhook receiver per client IP per
entrypoint, env-configurable with fail-loud parsing (#64)
- 2026-08-07 Per-webhook event retention reaper (#63); NoCache
middleware for authenticated pages (#61); Target interface refactor
(#77)
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section Makefile shims, README Entrypoints section
- 2026-03-25 pin golangci-lint Docker image for linting (#55) - 2026-03-25 pin golangci-lint Docker image for linting (#55)
@@ -51,12 +54,10 @@ databases currently grow without bound.
# Future Steps # 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 - Delivery status and retry management UI
- Per-webhook rate limiting in the receiver handler (per-webhook config - Per-webhook rate limiting in the receiver handler (per-webhook config
plus handler enforcement; global limits must not apply to receiver plus handler enforcement, layered on the env-level receiver limit
endpoints) from #64; global limits must not apply to receiver endpoints)
- Webhook signature verification for GitHub and Stripe HMAC formats - Webhook signature verification for GitHub and Stripe HMAC formats
- API key authentication for programmatic access (APIKey model exists; - API key authentication for programmatic access (APIKey model exists;
Bearer token middleware does not) Bearer token middleware does not)

View File

@@ -31,12 +31,23 @@ const (
// defaultRetentionSweepInterval is how often the retention // defaultRetentionSweepInterval is how often the retention
// reaper deletes events older than each webhook's RetentionDays. // reaper deletes events older than each webhook's RetentionDays.
defaultRetentionSweepInterval = time.Hour defaultRetentionSweepInterval = 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
) )
// ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
// contains an unrecognised value. // contains an unrecognised value.
var ErrInvalidEnvironment = errors.New("invalid environment") 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")
//nolint:revive // ConfigParams is a standard fx naming convention. //nolint:revive // ConfigParams is a standard fx naming convention.
type ConfigParams struct { type ConfigParams struct {
fx.In fx.In
@@ -60,6 +71,10 @@ type Config struct {
// RetentionSweepInterval is how often the retention reaper runs. // RetentionSweepInterval is how often the retention reaper runs.
RetentionSweepInterval time.Duration RetentionSweepInterval 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 params *ConfigParams
log *slog.Logger log *slog.Logger
} }
@@ -104,6 +119,38 @@ func envInt(key string, defaultValue int) int {
return defaultValue return defaultValue
} }
// 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
}
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
}
// envDuration returns the value of the named environment variable // envDuration returns the value of the named environment variable
// parsed as a Go duration (e.g. "1h", "30m"). Returns defaultValue if // parsed as a Go duration (e.g. "1h", "30m"). Returns defaultValue if
// not set. If the variable is set but cannot be parsed, it returns a // not set. If the variable is set but cannot be parsed, it returns a
@@ -128,27 +175,35 @@ func envDuration(
return d, nil return d, nil
} }
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
// dev, and rejects unrecognised values.
func resolveEnvironment() (string, error) {
environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
if environment == "" {
environment = EnvironmentDev
}
if environment != EnvironmentDev &&
environment != EnvironmentProd {
return "", fmt.Errorf(
"%w: WEBHOOKER_ENVIRONMENT must be '%s' or '%s', got '%s'",
ErrInvalidEnvironment,
EnvironmentDev, EnvironmentProd, environment,
)
}
return environment, nil
}
// New creates a Config by reading environment variables. // New creates a Config by reading environment variables.
// //
//nolint:revive // lc parameter is required by fx even if unused. //nolint:revive // lc parameter is required by fx even if unused.
func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
log := params.Logger.Get() log := params.Logger.Get()
// Determine environment from WEBHOOKER_ENVIRONMENT env var, environment, err := resolveEnvironment()
// default to dev if err != nil {
environment := os.Getenv("WEBHOOKER_ENVIRONMENT") return nil, err
if environment == "" {
environment = EnvironmentDev
}
// Validate environment
if environment != EnvironmentDev &&
environment != EnvironmentProd {
return nil, 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 // Parse the retention sweep interval; a set-but-unparseable value
@@ -162,6 +217,17 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
return nil, err return nil, err
} }
// Parse the receiver rate limit; a set-but-unparseable or
// non-positive value is a hard error so fx aborts startup
// rather than silently using the default.
receiverRateLimit, err := envPositiveInt(
"RECEIVER_RATE_LIMIT",
defaultReceiverRateLimit,
)
if err != nil {
return nil, err
}
// Load configuration values from environment variables // Load configuration values from environment variables
s := &Config{ s := &Config{
DataDir: envString("DATA_DIR"), DataDir: envString("DATA_DIR"),
@@ -173,6 +239,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
Port: envInt("PORT", defaultPort), Port: envInt("PORT", defaultPort),
SentryDSN: envString("SENTRY_DSN"), SentryDSN: envString("SENTRY_DSN"),
RetentionSweepInterval: retentionSweepInterval, RetentionSweepInterval: retentionSweepInterval,
ReceiverRateLimit: receiverRateLimit,
log: log, log: log,
params: &params, params: &params,
} }
@@ -197,6 +264,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"maintenanceMode", s.MaintenanceMode, "maintenanceMode", s.MaintenanceMode,
"dataDir", s.DataDir, "dataDir", s.DataDir,
"retentionSweepInterval", s.RetentionSweepInterval.String(), "retentionSweepInterval", s.RetentionSweepInterval.String(),
"receiverRateLimit", s.ReceiverRateLimit,
"hasSentryDSN", s.SentryDSN != "", "hasSentryDSN", s.SentryDSN != "",
"hasMetricsAuth", "hasMetricsAuth",
s.MetricsUsername != "" && s.MetricsPassword != "", s.MetricsUsername != "" && s.MetricsPassword != "",

View File

@@ -258,3 +258,109 @@ func TestDefaultDataDir(t *testing.T) {
}) })
} }
} }
func TestReceiverRateLimit(t *testing.T) {
tests := []struct {
name string
set bool
value string
expectError bool
expected int
}{
{
name: "unset uses default",
set: false,
expected: 120,
},
{
name: "valid value is parsed",
set: true,
value: "30",
expected: 30,
},
{
name: "unparseable value fails startup",
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 {
testReceiverRateLimitError(t)
} else {
testReceiverRateLimitSuccess(t, tt.expected)
}
})
}
}
func testReceiverRateLimitError(t *testing.T) {
t.Helper()
var cfg *config.Config
app := fx.New(
fx.NopLogger,
fx.Provide(
globals.New,
logger.New,
config.New,
),
fx.Populate(&cfg),
)
assert.Error(t, app.Err())
}
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)
}

View File

@@ -14,6 +14,11 @@ const (
// loginRateInterval is the time window for the rate limit. // loginRateInterval is the time window for the rate limit.
loginRateInterval = 1 * time.Minute loginRateInterval = 1 * time.Minute
// receiverRateInterval is the time window for the webhook
// receiver rate limit. The configured limit is expressed in
// requests per minute.
receiverRateInterval = 1 * time.Minute
) )
// LoginRateLimit returns middleware that enforces per-IP rate // LoginRateLimit returns middleware that enforces per-IP rate
@@ -62,3 +67,37 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler {
}) })
} }
} }
// ReceiverRateLimit returns middleware that rate-limits the
// public webhook receiver endpoint per client IP per request
// path (the path contains the entrypoint UUID, so each sender
// is limited per entrypoint without affecting other senders or
// other entrypoints). The limit is Config.ReceiverRateLimit
// requests per minute. Requests over the limit receive a 429;
// httprate adds the Retry-After header (RFC 6585). IP
// extraction honours X-Forwarded-For, X-Real-IP, and
// True-Client-IP headers for reverse-proxy setups.
func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
return httprate.Limit(
m.params.Config.ReceiverRateLimit,
receiverRateInterval,
httprate.WithKeyFuncs(
httprate.KeyByRealIP,
httprate.KeyByEndpoint,
),
httprate.WithLimitHandler(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
m.log.Warn(
"webhook receiver rate limit exceeded",
"path", r.URL.Path,
)
http.Error(
w,
"Too many requests. "+
"Please slow down.",
http.StatusTooManyRequests,
)
},
)),
)
}

View File

@@ -2,8 +2,10 @@ package middleware_test
import ( import (
"context" "context"
"log/slog"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"os"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -145,3 +147,94 @@ func TestLoginRateLimit_IndependentPerIP(t *testing.T) {
"different IP should not be affected", "different IP should not be affected",
) )
} }
// receiverLimitedHandler builds a ReceiverRateLimit-wrapped
// handler with the given per-minute limit.
func receiverLimitedHandler(
t *testing.T, limit int,
) http.Handler {
t.Helper()
log := slog.New(slog.NewTextHandler(
os.Stderr,
&slog.HandlerOptions{Level: slog.LevelDebug},
))
m := middleware.NewForTest(
log,
&config.Config{ReceiverRateLimit: limit},
nil,
)
return m.ReceiverRateLimit()(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
},
))
}
// receiverPost sends one POST to the handler from the given IP
// and path and returns the recorder.
func receiverPost(
handler http.Handler, ip, path string,
) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodPost, path, nil,
)
req.RemoteAddr = ip
w := httptest.NewRecorder()
handler.ServeHTTP(w, req)
return w
}
func TestReceiverRateLimit_LimitsPerIPAndPath(t *testing.T) {
t.Parallel()
const limit = 3
handler := receiverLimitedHandler(t, limit)
// The first limit requests from one IP to one entrypoint
// pass.
for i := range limit {
w := receiverPost(
handler, "9.9.9.9:1234", "/webhook/uuid-a",
)
assert.Equal(
t, http.StatusOK, w.Code,
"request %d should pass", i,
)
}
// The next request over the limit is rejected with a 429
// carrying a Retry-After header.
w := receiverPost(
handler, "9.9.9.9:1234", "/webhook/uuid-a",
)
assert.Equal(t, http.StatusTooManyRequests, w.Code)
assert.NotEmpty(
t, w.Header().Get("Retry-After"),
"429 must carry a Retry-After header",
)
// The same IP is not limited on a different entrypoint.
w = receiverPost(
handler, "9.9.9.9:1234", "/webhook/uuid-b",
)
assert.Equal(
t, http.StatusOK, w.Code,
"a different entrypoint must not be affected",
)
// A different IP is not limited on the same entrypoint.
w = receiverPost(
handler, "8.8.8.8:1234", "/webhook/uuid-a",
)
assert.Equal(
t, http.StatusOK, w.Code,
"a different client IP must not be affected",
)
}

View File

@@ -159,7 +159,7 @@ func (s *Server) setupSourceRoutes() {
} }
func (s *Server) setupWebhookRoutes() { func (s *Server) setupWebhookRoutes() {
s.router.HandleFunc( s.router.With(s.mw.ReceiverRateLimit()).HandleFunc(
"/webhook/{uuid}", "/webhook/{uuid}",
s.h.HandleWebhook(), s.h.HandleWebhook(),
) )