Compare commits
4 Commits
c4022c0834
...
next
| Author | SHA1 | Date | |
|---|---|---|---|
| d8f9d149b5 | |||
| 339548d794 | |||
| 95161c7768 | |||
| 0e397b3174 |
98
README.md
98
README.md
@@ -93,10 +93,10 @@ TTY detection, and security headers are always applied.
|
||||
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
|
||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
||||
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
||||
| `RETENTION_SWEEP_INTERVAL` | Retention reaper period (Go duration, must be positive) | `1h` |
|
||||
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
|
||||
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
|
||||
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
|
||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
|
||||
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket) | `""` (none) |
|
||||
|
||||
#### Trusted proxies
|
||||
|
||||
@@ -115,6 +115,23 @@ or draining someone else's. Set it to the address of your reverse
|
||||
proxy, and to nothing wider. A set but unparseable value aborts
|
||||
startup.
|
||||
|
||||
That default is safe against forged headers, but leaving it unset in
|
||||
production has a cost you must know about. Production runs behind a
|
||||
TLS-terminating reverse proxy, so with `TRUSTED_PROXIES` unset every
|
||||
request keys on the proxy's own address and all clients share a single
|
||||
bucket per limit. For the login and password-change limits that is a
|
||||
denial of service anyone can perform: a steady five POSTs per minute
|
||||
from any address on the internet keeps the shared login bucket full,
|
||||
and the operator's own login then returns HTTP 429 for as long as the
|
||||
trickle continues. There is no second administrative path and no
|
||||
bypass. Restarting the service clears the in-memory buckets, but a
|
||||
sustained trickle re-locks them immediately.
|
||||
|
||||
The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
|
||||
address, which restores per-client buckets. webhooker logs a warning
|
||||
at startup when `WEBHOOKER_ENVIRONMENT=prod` and `TRUSTED_PROXIES` is
|
||||
empty. See [Rate Limiting](#rate-limiting) for what each limit shares.
|
||||
|
||||
`X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
|
||||
Reverse proxies append to `X-Forwarded-For` but forward other client
|
||||
headers verbatim, so a single-valued header is client-controlled even
|
||||
@@ -265,9 +282,10 @@ webhooker solves this by acting as a durable intermediary:
|
||||
targets simultaneously. This enables patterns like forwarding a
|
||||
GitHub webhook to both a deployment service and a Slack channel.
|
||||
|
||||
5. **Replay** — Stored events can be manually redelivered for debugging
|
||||
or testing, without requiring the original sender to fire the webhook
|
||||
again.
|
||||
5. **Replay** (not yet implemented) — Every received event is stored in
|
||||
full, which is what manual redelivery for debugging or testing will
|
||||
be built on. No redelivery exists today, in the web UI or the API;
|
||||
see [TODO.md](TODO.md).
|
||||
|
||||
### Use Cases
|
||||
|
||||
@@ -277,6 +295,7 @@ webhooker solves this by acting as a durable intermediary:
|
||||
size, and delivery performance
|
||||
- **Debugging** and introspection of webhook payloads in the web UI
|
||||
- **Replay** of webhook events for application testing and development
|
||||
(planned; not yet implemented)
|
||||
- **Fan-out** delivery of a single webhook to multiple downstream
|
||||
targets
|
||||
- **High-availability ingestion** for delivery to less reliable backend
|
||||
@@ -502,7 +521,7 @@ A programmatic access credential for API authentication.
|
||||
#### Event
|
||||
|
||||
A captured incoming webhook request. Stores the complete HTTP request
|
||||
data for replay and auditing.
|
||||
data for auditing and for the planned replay capability.
|
||||
|
||||
| Field | Type | Description |
|
||||
| -------------- | ------ | ----------- |
|
||||
@@ -784,9 +803,10 @@ unknown) one while one of its deliveries is still `retrying`, both
|
||||
recovery paths above terminally mark that delivery `failed` and record a
|
||||
`DeliveryResult` naming the current target type as the reason, logging it
|
||||
at warn level. The delivery is not re-dispatched under the new type — the
|
||||
operator never asked for that delivery — and the event itself remains
|
||||
stored in the per-webhook event database, so it can be redelivered
|
||||
manually.
|
||||
operator never asked for that delivery — and while the event itself
|
||||
remains stored in the per-webhook event database, there is no way to
|
||||
redeliver it: manual redelivery is planned, not implemented (see
|
||||
[TODO.md](TODO.md)).
|
||||
|
||||
### Circuit Breaker (HTTP Targets with Retries)
|
||||
|
||||
@@ -856,15 +876,60 @@ 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.
|
||||
|
||||
A second limit sits in front of that one, keyed on the client IP alone
|
||||
and covering the whole route at ten times `RECEIVER_RATE_LIMIT` requests
|
||||
per minute (default 1200). The per-entrypoint limit needs it: the route
|
||||
pattern matches any single path segment, so a client that invents a
|
||||
fresh path per request gets a fresh per-entrypoint bucket every time and
|
||||
would otherwise have no aggregate limit at all — while each of those
|
||||
requests still costs an entrypoint lookup before it 404s. The aggregate
|
||||
limit leaves room for one address to drive several entrypoints at their
|
||||
full rate, and it is not configurable separately.
|
||||
|
||||
What that aggregate limit bounds is the database work an invented path
|
||||
costs; log volume it caps rather than eliminates. A path that names no
|
||||
entrypoint is recorded by the handler at `DEBUG`, and the aggregate
|
||||
limiter logs its own rejections at `DEBUG` and without the path, so
|
||||
neither appears at all under the default level. The per-entrypoint
|
||||
limiter is the loud one: it still logs every rejection at `WARN` with
|
||||
the request path, which on this route is attacker-controlled text. A
|
||||
client hammering a single invented path is served `RECEIVER_RATE_LIMIT`
|
||||
requests and has the rest of its aggregate budget rejected there, so
|
||||
the aggregate limit is what bounds those `WARN` lines — to under ten
|
||||
times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
|
||||
defaults, where before it there was no bound at all. The access log is
|
||||
bounded by neither limit: every request is recorded once at `INFO` with
|
||||
its full URL, served or rejected alike.
|
||||
|
||||
Every limiter here — receiver, login, and password change — identifies
|
||||
the client the same way, through one shared key function: the
|
||||
connection's own address, unless the peer is listed in
|
||||
`TRUSTED_PROXIES`, in which case the forwarded client address is used
|
||||
instead. See [Trusted proxies](#trusted-proxies). Deployed without that
|
||||
variable set, a client behind a reverse proxy shares one bucket with
|
||||
every other client behind the same proxy, which is the safe direction
|
||||
to be wrong in: set `TRUSTED_PROXIES` to the proxy's address to get
|
||||
per-client limits back.
|
||||
every other client behind the same proxy. Set `TRUSTED_PROXIES` to the
|
||||
proxy's address to get per-client limits back. What the shared bucket
|
||||
costs is not the same for every limiter, and the two cases pull in
|
||||
opposite directions:
|
||||
|
||||
- For the **receiver** limits it costs throughput, which is the safe
|
||||
direction to be wrong in: sharing can only make a limit bind sooner,
|
||||
never let a sender past it. It matters more for the aggregate limit
|
||||
than for the per-entrypoint one: with `TRUSTED_PROXIES` unset behind
|
||||
the reverse proxy a production deployment is required to run behind,
|
||||
every request keys on the proxy, so the aggregate limit becomes a
|
||||
service-wide ceiling of 1200 requests per minute across all senders
|
||||
and all entrypoints, where the per-entrypoint limit's capacity still
|
||||
grows with the number of entrypoints. Any deployment with more than a
|
||||
handful of busy entrypoints must set `TRUSTED_PROXIES`.
|
||||
- For the **login and password-change** limits it costs availability of
|
||||
the only administrative path, which is not safe at all. Five POSTs
|
||||
per minute from any address on the internet keeps the single shared
|
||||
login bucket full, and the operator's own login returns HTTP 429 for
|
||||
as long as that trickle continues. A restart clears the in-memory
|
||||
buckets and a resumed trickle re-locks them. Production deployments
|
||||
must set `TRUSTED_PROXIES`; webhooker warns at startup when it is
|
||||
empty in `prod`.
|
||||
|
||||
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
|
||||
@@ -1083,8 +1148,11 @@ downstream at form-parse time.
|
||||
(custom HTTP transport with SSRF-safe dialer that validates resolved
|
||||
IPs before connecting, preventing DNS rebinding attacks)
|
||||
- **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
|
||||
per-IP sliding-window rate limiter on the login endpoint (5 POST
|
||||
attempts per minute per IP) to prevent brute-force attacks
|
||||
sliding-window rate limiter on the login endpoint, 5 POST attempts
|
||||
per minute per bucket, to slow brute-force attacks. The bucket is per
|
||||
client IP only when `TRUSTED_PROXIES` names the reverse proxy;
|
||||
unset, every client shares one bucket and the login becomes remotely
|
||||
deniable (see [Rate Limiting](#rate-limiting))
|
||||
- Prometheus metrics behind basic auth
|
||||
- Static assets embedded in binary (no filesystem access needed at
|
||||
runtime)
|
||||
|
||||
81
TODO.md
81
TODO.md
@@ -24,22 +24,52 @@ event retention (#63), the database archiving target (#43), the admin
|
||||
password change flow (#65), policy compliance (#6), pinned lint tooling
|
||||
(#55), and fail-loud configuration parsing (#80).
|
||||
|
||||
`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
|
||||
is closed, and it is verified green by cache-defeated container runs
|
||||
rather than by the CI badge, which can pass without executing anything
|
||||
(#119). 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` holds the completed 1.0.0 milestone: every issue in it is closed,
|
||||
and it is verified green both by CI and by cache-defeated container
|
||||
runs. The two were only made to mean the same thing this cycle — before
|
||||
#119, a warm layer cache let the gate report success without executing
|
||||
anything, and replayed the previous build's console log so the lie
|
||||
looked like a real run. 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
|
||||
|
||||
Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
|
||||
gate (#119) before the next cycle's work lands — a gate that can report
|
||||
success without running is the one thing every other guarantee here
|
||||
rests on.
|
||||
Merge the milestone PR to `main` and tag 1.0.0 from it.
|
||||
|
||||
Two decisions are open and belong to the owner, neither blocking the
|
||||
tag: #115 (mask the `http` target's destination URL, implemented
|
||||
speculatively and awaiting a yes or no) and #125 (whether IPv6
|
||||
rate-limit keys should bucket by `/64`).
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-12 Bound the receiver rate limit per client IP across the
|
||||
whole `/webhook/*` route. The existing limiter keyed on the request
|
||||
path and `/webhook/{uuid}` matches any single segment, so a client
|
||||
that invented a fresh path per request minted a fresh bucket per
|
||||
request: the limit on the only unauthenticated endpoint bounded
|
||||
nothing in aggregate, and every request still cost an entrypoint
|
||||
lookup before it 404ed. An outer limiter keyed on the client address
|
||||
alone now bounds that, chained in front of the unchanged
|
||||
per-entrypoint limiter (#139)
|
||||
- 2026-08-12 Correct release-blocking documentation inaccuracies: the
|
||||
README promised manual redelivery in the present tense in three
|
||||
places when nothing implements it (the same false claim also sat in
|
||||
the doc comment that was its source text), the env table omitted
|
||||
`RETENTION_SWEEP_INTERVAL`, and `TODO.md` itself omitted five landed
|
||||
units (#141)
|
||||
- 2026-08-12 Make the CI gate execute the checks it reports on. The
|
||||
workflow now writes a build-context fingerprint before calling
|
||||
`script/cibuild`, so a code commit invalidates the `COPY` layer of
|
||||
the lint and builder stages while a docs-only commit still replays
|
||||
from cache; a superseding run also rewrites the `failure` status
|
||||
Gitea leaves on commits it cancelled and never tested. Verified by
|
||||
pushing a deliberately broken test and watching CI go red (#119)
|
||||
- 2026-08-12 Require a positive `RETENTION_SWEEP_INTERVAL`: a
|
||||
non-positive value reached `time.NewTicker` in both the retention
|
||||
reaper and the archive sweeper, panicking two goroutines with no
|
||||
recover after startup had already reported success (#140)
|
||||
- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
|
||||
cap: the reverse walk cuts entries with `strings.LastIndexByte`
|
||||
instead of joining and splitting, so a 1 MB header allocates 16 bytes
|
||||
@@ -61,6 +91,34 @@ rests on.
|
||||
Profile settings placeholder removed, a progressive-enhancement copy
|
||||
button for the entrypoint URL, and retention form copy that states the
|
||||
actual policy (deletion by the reaper, 0 retains forever) (#57)
|
||||
- 2026-08-11 Mask the webhook credential in delivery errors and logs:
|
||||
Go embeds the request URL in `*url.Error`, so every transport failure
|
||||
persisted the full Slack webhook URL into the per-webhook event
|
||||
database via `DeliveryResult.Error`, a field a future REST API would
|
||||
have served. `maskURLError` drops path, query and userinfo while
|
||||
preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
|
||||
still work and DNS, TLS and timeout failures still read differently
|
||||
(#118)
|
||||
- 2026-08-11 Rate-limit the public webhook receiver endpoint
|
||||
(`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
|
||||
entrypoint path so one entrypoint cannot exhaust another's budget;
|
||||
over-limit requests get 429 with `Retry-After`. It was the one
|
||||
unauthenticated, internet-facing endpoint with no limit at all (#64)
|
||||
- 2026-08-11 Enforce the body size limit before CSRF parses the form:
|
||||
`MaxBodySize` is now first in all four form-parsing route groups, so
|
||||
an oversized request is rejected with 413 instead of being read in
|
||||
full by the CSRF middleware before any cap applied (#90)
|
||||
- 2026-08-11 Mask target config on the source detail page, which
|
||||
rendered the stored blob verbatim and so exposed the Slack
|
||||
incoming-webhook URL — a bearer credential that cannot be revoked
|
||||
per-holder. Config reaches the template only as a `TargetView` of
|
||||
labelled fields, and header values are rendered as a count (#113)
|
||||
- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
|
||||
sentinel written in `BeforeSave` so the GORM column default cannot
|
||||
win the race. Also bounds the reaper's cutoff arithmetic: day counts
|
||||
above 106751 overflowed `time.Duration` and wrapped the cutoff into
|
||||
the future, where every row matched and the sweep deleted everything
|
||||
(#79)
|
||||
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry
|
||||
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
|
||||
requests, with the 7-day absolute cap kept as an independent
|
||||
@@ -115,6 +173,9 @@ rests on.
|
||||
|
||||
# Future Steps
|
||||
|
||||
- Manual event redelivery from the web UI — the "Replay" capability the
|
||||
README describes as planned. No redelivery code exists anywhere in the
|
||||
tree; events are stored in full, which is all it would be built on
|
||||
- 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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -785,9 +785,9 @@ func (e *Engine) sweepSingleRetry(
|
||||
// status retrying themselves. Re-dispatching under the new type
|
||||
// would be a delivery the operator never asked for, and leaving
|
||||
// the row retrying strands it forever, so the delivery is
|
||||
// failed with a recorded reason and can be redelivered
|
||||
// manually. Logged at warn, not error: this is operator-caused
|
||||
// state, not a system fault.
|
||||
// failed with a recorded reason. The event stays stored, but
|
||||
// nothing redelivers it today. Logged at warn, not error: this
|
||||
// is operator-caused state, not a system fault.
|
||||
func (e *Engine) failUnretryableRetry(
|
||||
webhookDB *gorm.DB,
|
||||
webhookID string,
|
||||
|
||||
@@ -39,12 +39,6 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
h.log.Info("webhook request received",
|
||||
"entrypoint_uuid", entrypointUUID,
|
||||
"method", r.Method,
|
||||
"remote_addr", r.RemoteAddr,
|
||||
)
|
||||
|
||||
entrypoint, ok := h.lookupEntrypoint(
|
||||
w, r, entrypointUUID,
|
||||
)
|
||||
@@ -52,6 +46,18 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Logged only once the UUID is known to name a real
|
||||
// entrypoint. The UUID comes straight out of the path on
|
||||
// the one unauthenticated endpoint, so logging it before
|
||||
// the lookup let a client write an INFO line per invented
|
||||
// path; the request itself is already in the access log
|
||||
// and a miss is already logged at DEBUG.
|
||||
h.log.Info("webhook request received",
|
||||
"entrypoint_uuid", entrypointUUID,
|
||||
"method", r.Method,
|
||||
"remote_addr", r.RemoteAddr,
|
||||
)
|
||||
|
||||
if !entrypoint.Active {
|
||||
http.Error(w, "Gone", http.StatusGone)
|
||||
|
||||
|
||||
@@ -41,3 +41,13 @@ const LoginRateLimitConst = loginRateLimit
|
||||
// PasswordChangeRateLimitConst exposes the
|
||||
// passwordChangeRateLimit constant.
|
||||
const PasswordChangeRateLimitConst = passwordChangeRateLimit
|
||||
|
||||
// ReceiverAggregateMultiplierConst exposes the
|
||||
// receiverAggregateMultiplier constant.
|
||||
const ReceiverAggregateMultiplierConst = receiverAggregateMultiplier
|
||||
|
||||
// ReceiverAggregateLimitForTest exposes receiverAggregateLimit for
|
||||
// testing.
|
||||
func ReceiverAggregateLimitForTest(perEntrypoint int) int {
|
||||
return receiverAggregateLimit(perEntrypoint)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"math"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"slices"
|
||||
@@ -33,6 +34,14 @@ const (
|
||||
// requests per minute.
|
||||
receiverRateInterval = 1 * time.Minute
|
||||
|
||||
// receiverAggregateMultiplier scales the configured
|
||||
// per-entrypoint receiver limit into the aggregate limit one
|
||||
// client IP may spend across the whole /webhook/* route. Ten
|
||||
// entrypoints' worth lets a single sender address drive several
|
||||
// entrypoints at their full rate, while still capping what one
|
||||
// address costs the unauthenticated receiver.
|
||||
receiverAggregateMultiplier = 10
|
||||
|
||||
// maxForwardedHops bounds how many X-Forwarded-For entries the
|
||||
// chain walk examines. Real chains are one to three hops, but a
|
||||
// client can pad the header up to MaxHeaderBytes, so without a
|
||||
@@ -162,9 +171,11 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
||||
return peer.String()
|
||||
}
|
||||
|
||||
// tooManyRequests returns the 429 handler shared by every limiter:
|
||||
// it logs the rejection with logMessage and answers with
|
||||
// responseMessage. httprate adds the Retry-After header (RFC 6585).
|
||||
// tooManyRequests returns the 429 handler used by the login,
|
||||
// password-change and per-entrypoint receiver limiters: it logs the
|
||||
// rejection with logMessage and answers with responseMessage.
|
||||
// httprate adds the Retry-After header (RFC 6585). The aggregate
|
||||
// receiver limiter uses floodTooManyRequests instead.
|
||||
func (m *Middleware) tooManyRequests(
|
||||
logMessage, responseMessage string,
|
||||
) http.HandlerFunc {
|
||||
@@ -174,6 +185,31 @@ func (m *Middleware) tooManyRequests(
|
||||
}
|
||||
}
|
||||
|
||||
// floodTooManyRequests returns the 429 handler for a limiter whose
|
||||
// rejections are themselves the flood: it logs at DEBUG and without
|
||||
// the path, then answers with responseMessage.
|
||||
//
|
||||
// The aggregate receiver limiter trips exactly when one address is
|
||||
// sending faster than the receiver wants to serve, so its rejection
|
||||
// log is one line per request of that flood. At WARN with "path" that
|
||||
// hands a client a way to write its own text into the operator's log,
|
||||
// at a level that trips alerting, once per request — the log-volume
|
||||
// problem this limiter exists to bound. DEBUG is off in production by
|
||||
// default, so a flood costs nothing here; the path is dropped so that
|
||||
// turning DEBUG on to diagnose one does not restore the problem.
|
||||
//
|
||||
// This limiter bounds the database work an invented path costs, not
|
||||
// the number of log lines it produces: the access log in
|
||||
// middleware.go still records every request, served or rejected.
|
||||
func (m *Middleware) floodTooManyRequests(
|
||||
logMessage, responseMessage string,
|
||||
) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, _ *http.Request) {
|
||||
m.log.Debug(logMessage)
|
||||
http.Error(w, responseMessage, http.StatusTooManyRequests)
|
||||
}
|
||||
}
|
||||
|
||||
// LoginRateLimit returns middleware that enforces per-IP rate
|
||||
// limiting on login attempts using go-chi/httprate. Only POST
|
||||
// requests are rate-limited; GET requests (rendering the login
|
||||
@@ -242,15 +278,26 @@ func (m *Middleware) postRateLimit(
|
||||
}
|
||||
}
|
||||
|
||||
// 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.
|
||||
// Clients are identified by rateLimitKey.
|
||||
// ReceiverRateLimit returns middleware that rate-limits the public
|
||||
// webhook receiver endpoint with two limits in series.
|
||||
//
|
||||
// The inner limit is 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.
|
||||
// It is Config.ReceiverRateLimit requests per minute.
|
||||
//
|
||||
// That limit alone bounds nothing in aggregate. The route pattern
|
||||
// /webhook/{uuid} matches any single segment, so a client that
|
||||
// invents a fresh path per request mints a fresh bucket per request
|
||||
// and never refills one — and every such request still reaches the
|
||||
// handler's entrypoint lookup before it 404s. The outer limit is
|
||||
// therefore keyed on the client IP alone, capping what one address
|
||||
// can spend across the whole route however it varies the path.
|
||||
//
|
||||
// Requests over either limit receive a 429. Clients are identified
|
||||
// by rateLimitKey.
|
||||
func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
|
||||
return httprate.Limit(
|
||||
perEntrypoint := httprate.Limit(
|
||||
m.params.Config.ReceiverRateLimit,
|
||||
receiverRateInterval,
|
||||
httprate.WithKeyFuncs(
|
||||
@@ -262,4 +309,31 @@ func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
|
||||
"Too many requests. Please slow down.",
|
||||
)),
|
||||
)
|
||||
|
||||
aggregate := httprate.Limit(
|
||||
receiverAggregateLimit(m.params.Config.ReceiverRateLimit),
|
||||
receiverRateInterval,
|
||||
httprate.WithKeyFuncs(m.rateLimitKey),
|
||||
httprate.WithLimitHandler(m.floodTooManyRequests(
|
||||
"webhook receiver aggregate rate limit exceeded",
|
||||
"Too many requests. Please slow down.",
|
||||
)),
|
||||
)
|
||||
|
||||
return func(next http.Handler) http.Handler {
|
||||
return aggregate(perEntrypoint(next))
|
||||
}
|
||||
}
|
||||
|
||||
// receiverAggregateLimit is the per-IP aggregate limit derived from
|
||||
// the configured per-entrypoint limit. The operator sets the latter
|
||||
// and nothing bounds it from above, so the multiplication is
|
||||
// saturated rather than allowed to wrap into a negative limit that
|
||||
// would reject every request.
|
||||
func receiverAggregateLimit(perEntrypoint int) int {
|
||||
if perEntrypoint > math.MaxInt/receiverAggregateMultiplier {
|
||||
return math.MaxInt
|
||||
}
|
||||
|
||||
return perEntrypoint * receiverAggregateMultiplier
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"math"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/netip"
|
||||
@@ -670,6 +671,129 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths is the
|
||||
// regression test for the per-path bucket key. The route pattern
|
||||
// matches any single segment, so a client that never reuses a path
|
||||
// never reuses a per-entrypoint bucket either, and its aggregate
|
||||
// rate against the receiver is whatever it likes — with every
|
||||
// request reaching an entrypoint lookup before it 404s. The IP-only
|
||||
// aggregate limiter is what bounds that, so this must fail if the
|
||||
// aggregate limiter is removed.
|
||||
func TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
limit = 3
|
||||
ip = "6.6.6.6:1234"
|
||||
)
|
||||
|
||||
aggregate := limit * middleware.ReceiverAggregateMultiplierConst
|
||||
|
||||
handler := receiverLimitedHandler(t, limit)
|
||||
|
||||
// Every request goes to a path this client has never used, so
|
||||
// none of them shares a per-entrypoint bucket with another.
|
||||
for i := range aggregate {
|
||||
w := receiverPost(
|
||||
handler, ip, fmt.Sprintf("/webhook/invented-%d", i),
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"request %d to a distinct path should pass", i,
|
||||
)
|
||||
}
|
||||
|
||||
w := receiverPost(
|
||||
handler, ip, fmt.Sprintf("/webhook/invented-%d", aggregate),
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"a client must not be able to raise its aggregate rate "+
|
||||
"against /webhook/* by varying the path",
|
||||
)
|
||||
|
||||
// The aggregate limit is still per client IP: exhausting one
|
||||
// address must not throttle another.
|
||||
w = receiverPost(handler, "6.6.6.7:1234", "/webhook/invented-0")
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a different client IP must not be affected",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverRateLimit_RejectedRequestsCountTowardAggregate pins the
|
||||
// order the two limiters are chained in. The aggregate limiter has to
|
||||
// be the outer one, so that it counts requests the per-entrypoint
|
||||
// limiter rejects: those requests still arrive, and the aggregate
|
||||
// limit exists to bound what one address can make the receiver do.
|
||||
//
|
||||
// One path is hammered past the per-entrypoint limit, which alone
|
||||
// would leave the aggregate budget almost untouched; then a path the
|
||||
// client has never used must be rejected, which only the aggregate
|
||||
// limiter can do. Swap the two limiters and that last request is
|
||||
// served, because the rejected ones never reached the aggregate
|
||||
// limiter to be counted.
|
||||
func TestReceiverRateLimit_RejectedRequestsCountTowardAggregate(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
limit = 3
|
||||
ip = "6.6.6.8:1234"
|
||||
)
|
||||
|
||||
aggregate := limit * middleware.ReceiverAggregateMultiplierConst
|
||||
|
||||
handler := receiverLimitedHandler(t, limit)
|
||||
|
||||
// Spend the whole aggregate budget on one path. Only the first
|
||||
// limit requests are served; the rest are rejected by the
|
||||
// per-entrypoint limiter but still count against the aggregate.
|
||||
for i := range aggregate {
|
||||
w := receiverPost(handler, ip, "/webhook/exhausted")
|
||||
|
||||
want := http.StatusTooManyRequests
|
||||
if i < limit {
|
||||
want = http.StatusOK
|
||||
}
|
||||
|
||||
assert.Equal(
|
||||
t, want, w.Code,
|
||||
"request %d to the exhausted path", i,
|
||||
)
|
||||
}
|
||||
|
||||
w := receiverPost(handler, ip, "/webhook/never-used")
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"requests rejected per entrypoint must still count "+
|
||||
"toward the aggregate limit, so the aggregate "+
|
||||
"limiter has to run first",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverAggregateLimit_SaturatesOnOverflow covers the derived
|
||||
// aggregate limit for a configured per-entrypoint limit large enough
|
||||
// that multiplying it would wrap negative, which httprate would read
|
||||
// as a limit that rejects every request.
|
||||
func TestReceiverAggregateLimit_SaturatesOnOverflow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, 1200,
|
||||
middleware.ReceiverAggregateLimitForTest(120),
|
||||
"the default limit scales by the multiplier",
|
||||
)
|
||||
assert.Equal(
|
||||
t, math.MaxInt,
|
||||
middleware.ReceiverAggregateLimitForTest(math.MaxInt),
|
||||
"an overflowing limit saturates instead of wrapping",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
|
||||
// the receiver limiter uses the same gated key function as the
|
||||
// POST limiters.
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
// Webhooker client-side JavaScript
|
||||
console.log("Webhooker loaded");
|
||||
|
||||
// Copy-to-clipboard, as progressive enhancement.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user