sneak/webhooker
1
0
Fork 0
Code Issues 13 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Rate-limit the public webhook receiver endpoint #64

New Issue
Open
opened 2026-08-07 13:11:08 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-07 13:11:08 +02:00
Collaborator
Copy Link

Part of the road to 1.0 (see #33).

The public receiver /webhook/{uuid} (internal/handlers/webhook.go, wired in internal/server/routes.go setupWebhookRoutes()) has no rate limiting. Anyone who learns an entrypoint UUID can flood it, inflating the per-webhook database and the delivery queue. The login endpoint already demonstrates the pattern via LoginRateLimit() (internal/middleware/ratelimit.go).

Definition of done:

  • per-entrypoint (or per-IP) rate limiting on the receiver, with a configurable limit and sane default
  • requests over the limit receive HTTP 429 with a Retry-After header
  • a test exercises the limit (allowed under, rejected over)

Scope note for @sneak: is this a 1.0 blocker or post-1.0? It pairs naturally with the retention reaper as abuse/growth control on the one unauthenticated, internet-exposed surface.

Part of the road to 1.0 (see #33). The public receiver `/webhook/{uuid}` (`internal/handlers/webhook.go`, wired in `internal/server/routes.go` `setupWebhookRoutes()`) has no rate limiting. Anyone who learns an entrypoint UUID can flood it, inflating the per-webhook database and the delivery queue. The login endpoint already demonstrates the pattern via `LoginRateLimit()` (`internal/middleware/ratelimit.go`). Definition of done: - per-entrypoint (or per-IP) rate limiting on the receiver, with a configurable limit and sane default - requests over the limit receive HTTP 429 with a `Retry-After` header - a test exercises the limit (allowed under, rejected over) Scope note for @sneak: is this a 1.0 blocker or post-1.0? It pairs naturally with the retention reaper as abuse/growth control on the one unauthenticated, internet-exposed surface.
clawbot added this to the 1.0.0 milestone 2026-08-07 13:11:08 +02:00
clawbot commented 2026-08-07 18:38:14 +02:00
Author
Collaborator
Copy Link

Definition of done and implementation plan

Picking this up as the next 1.0.0 work unit.

Definition of done

  • Every request to the public receiver /webhook/{uuid} is rate-limited per client IP per entrypoint path: the same sender hammering one entrypoint is throttled without affecting other senders of that entrypoint or the same sender's other entrypoints.
  • The limit is configurable via a RECEIVER_RATE_LIMIT environment variable (requests per minute), default 120. A set-but-unparseable or non-positive value ABORTS startup with a clear error naming the variable and the bad value — no silent defaulting (the envDuration/RETENTION_SWEEP_INTERVAL fail-loud pattern, not the envInt pattern tracked in #80).
  • Requests over the limit receive HTTP 429 with a Retry-After header (go-chi/httprate sets Retry-After on rejection per RFC 6585; verified in the vendored v0.15.0 source).
  • IP extraction honours X-Forwarded-For / X-Real-IP / True-Client-IP (same httprate.KeyByRealIP the login limiter uses), for reverse-proxy deployments.
  • Global/auth middleware behaviour is unchanged; the limit applies only to the receiver route.

Implementation plan

  • internal/config/config.go: add ReceiverRateLimit int loaded from RECEIVER_RATE_LIMIT with a new strict integer parser that returns an error (failing config.New, aborting fx startup) when the variable is set but unparseable or less than 1. Default 120/minute. envInt itself is NOT touched here — converting its other callers is #80's scope.
  • internal/middleware/ratelimit.go: add ReceiverRateLimit() using httprate.Limit with the configured per-minute limit, keyed by httprate.WithKeyFuncs(httprate.KeyByRealIP, httprate.KeyByEndpoint) (composite key: client IP + request path, and the path contains the entrypoint UUID). Custom limit handler logs a warning and returns 429 with a clear message; httprate adds Retry-After and the X-RateLimit-* headers.
  • internal/server/routes.go: wrap the /webhook/{uuid} route in setupWebhookRoutes with the new middleware. All methods on the route are counted (the receiver is POST-only and 405s the rest; scanners burning the budget on GETs is acceptable and desirable).
  • Tests:
    • internal/middleware/ratelimit_test.go: under-limit requests pass; the request over the limit gets 429 with a non-empty Retry-After; a different entrypoint path from the same IP is unaffected; a different IP on the same path is unaffected.
    • internal/config/config_test.go: default applies when unset; a valid value is used; unparseable and non-positive values make config.New fail with the variable name in the error.

Validation gates

  • make fmt applied; make test and make fmt-check green locally; docker build . green in CI (the authoritative gate, includes the pinned linter).
  • Branch issue-64-receiver-rate-limit from current main; PR titled ending with (closes #64), labelled needs-review and assigned to clawbot for independent review — not self-labelled merge-ready.

On the scope question in the issue body: implementing now as part of 1.0.0 (it is already milestoned); it is the only unauthenticated internet-exposed surface and the retention reaper only cleans up after abuse rather than preventing it. Per-webhook configurable limits (the TODO "Future Steps" item) can layer on top later without conflicting with this env-level limit.

## Definition of done and implementation plan Picking this up as the next 1.0.0 work unit. ### Definition of done - Every request to the public receiver `/webhook/{uuid}` is rate-limited per client IP per entrypoint path: the same sender hammering one entrypoint is throttled without affecting other senders of that entrypoint or the same sender's other entrypoints. - The limit is configurable via a `RECEIVER_RATE_LIMIT` environment variable (requests per minute), default 120. A set-but-unparseable or non-positive value ABORTS startup with a clear error naming the variable and the bad value — no silent defaulting (the `envDuration`/`RETENTION_SWEEP_INTERVAL` fail-loud pattern, not the `envInt` pattern tracked in #80). - Requests over the limit receive HTTP 429 with a `Retry-After` header (go-chi/httprate sets `Retry-After` on rejection per RFC 6585; verified in the vendored v0.15.0 source). - IP extraction honours `X-Forwarded-For` / `X-Real-IP` / `True-Client-IP` (same `httprate.KeyByRealIP` the login limiter uses), for reverse-proxy deployments. - Global/auth middleware behaviour is unchanged; the limit applies only to the receiver route. ### Implementation plan - `internal/config/config.go`: add `ReceiverRateLimit int` loaded from `RECEIVER_RATE_LIMIT` with a new strict integer parser that returns an error (failing `config.New`, aborting fx startup) when the variable is set but unparseable or less than 1. Default 120/minute. `envInt` itself is NOT touched here — converting its other callers is #80's scope. - `internal/middleware/ratelimit.go`: add `ReceiverRateLimit()` using `httprate.Limit` with the configured per-minute limit, keyed by `httprate.WithKeyFuncs(httprate.KeyByRealIP, httprate.KeyByEndpoint)` (composite key: client IP + request path, and the path contains the entrypoint UUID). Custom limit handler logs a warning and returns 429 with a clear message; httprate adds `Retry-After` and the `X-RateLimit-*` headers. - `internal/server/routes.go`: wrap the `/webhook/{uuid}` route in `setupWebhookRoutes` with the new middleware. All methods on the route are counted (the receiver is POST-only and 405s the rest; scanners burning the budget on GETs is acceptable and desirable). - Tests: - `internal/middleware/ratelimit_test.go`: under-limit requests pass; the request over the limit gets 429 with a non-empty `Retry-After`; a different entrypoint path from the same IP is unaffected; a different IP on the same path is unaffected. - `internal/config/config_test.go`: default applies when unset; a valid value is used; unparseable and non-positive values make `config.New` fail with the variable name in the error. ### Validation gates - `make fmt` applied; `make test` and `make fmt-check` green locally; `docker build .` green in CI (the authoritative gate, includes the pinned linter). - Branch `issue-64-receiver-rate-limit` from current `main`; PR titled ending with `(closes #64)`, labelled needs-review and assigned to clawbot for independent review — not self-labelled merge-ready. On the scope question in the issue body: implementing now as part of 1.0.0 (it is already milestoned); it is the only unauthenticated internet-exposed surface and the retention reaper only cleans up after abuse rather than preventing it. Per-webhook configurable limits (the TODO "Future Steps" item) can layer on top later without conflicting with this env-level limit.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#64
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?