Compare commits

3 Commits

Author SHA1 Message Date
1828d99e0d Rate-limit the public webhook receiver endpoint (closes #64)
All checks were successful
check / check (push) Successful in 3m49s
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.
2026-08-11 12:41:25 +00:00
d51cd0fd29 Enforce the body size limit before CSRF parses the form (closes #90)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
CSRF ran before MaxBodySize, so the CSRF middleware parsed the form body
before any cap applied and an oversized request was read in full before
being rejected. MaxBodySize is now the first middleware in all four route
groups that parse forms, ahead of CSRF and RequireAuth.

An oversize request therefore gets 413 without the handler running and
without state changing, including the password-change route.

Note the ordering trade: an unauthenticated client now receives 413 rather
than an auth redirect on /user/{username}/password.
2026-08-11 14:37:38 +02:00
15a61173fc Mask target config on the source detail page (closes #113)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
The page rendered the stored target config verbatim, exposing the Slack
incoming-webhook URL, which is a bearer credential: anyone holding it can
post to the channel indefinitely, and it cannot be scoped or revoked
per-holder.

Target config now reaches the template only as a TargetView carrying
labelled fields, so no code path can render the raw blob. maskURL keeps
scheme and host and elides the path, and drops query, fragment and
userinfo; every parse failure yields a neutral placeholder rather than
falling back to the stored string. HTTP header values are never rendered,
only a count.

Rendering change only: the stored config format and the delivery path are
unchanged.
2026-08-11 14:37:09 +02:00
13 changed files with 1381 additions and 53 deletions

View File

@@ -972,9 +972,17 @@ Applied to all routes in this order:
8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set;
configured with `Repanic: true` so panics still reach Recoverer)
Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a
**MaxBodySize** middleware that limits POST/PUT/PATCH request bodies to
1 MB using `http.MaxBytesReader`, preventing oversized form submissions.
Additionally, form endpoints (`/pages`, `/user/*`, `/sources`,
`/source/*`) apply a **MaxBodySize** middleware that limits
POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
CSRF middleware in every one of those route groups, because
gorilla/csrf parses the form; if the cap were installed after it, form
parsing would run under net/http's 10 MB default and the 1 MB limit
would never apply. A request that declares a `Content-Length` over the
limit is answered with `413 Request Entity Too Large` before any other
middleware or handler runs; a chunked request, or one that lies about
its length, is hard-capped by `http.MaxBytesReader` and fails
downstream at form-parse time.
### Authentication
@@ -996,7 +1004,8 @@ Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a
- Production security headers on all responses: HSTS, X-Content-Type-Options
(`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy,
and Permissions-Policy
- Request body size limits (1 MB) on all form POST endpoints
- Request body size limits (1 MB) on all form POST endpoints, enforced
by middleware that runs before CSRF parses the form
- **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf)
on all state-changing forms (cookie-based double-submit tokens with
HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and

View File

@@ -0,0 +1,224 @@
package delivery
import (
"encoding/json"
"fmt"
"net/url"
"strconv"
"sneak.berlin/go/webhooker/internal/database"
)
// configUnavailable is what a target's configuration renders
// as when it is absent, of an unknown type, or does not
// parse. The stored blob is never shown as a fallback: it can
// hold a credential (a Slack incoming webhook URL is a bearer
// token) and a UI that prints it leaks that credential into
// browser history, screenshots and screen shares.
const configUnavailable = "(unavailable)"
// urlPathElision stands in for a URL's elided path.
const urlPathElision = "/..."
// ConfigField is one labelled, display-safe value derived
// from a target's stored configuration.
type ConfigField struct {
Label string
Value string
}
// TargetView is the display-safe projection of a target for
// the UI. It deliberately has no raw configuration field, so
// no template — present or future — can render the stored
// blob.
type TargetView struct {
ID string
Name string
Type database.TargetType
Active bool
Config []ConfigField
}
// NewTargetViews projects targets for rendering, replacing
// each stored configuration blob with named, display-safe
// fields.
func NewTargetViews(
targets []database.Target,
) []TargetView {
views := make([]TargetView, 0, len(targets))
for i := range targets {
t := &targets[i]
views = append(views, TargetView{
ID: t.ID,
Name: t.Name,
Type: t.Type,
Active: t.Active,
Config: targetConfigFields(t),
})
}
return views
}
// targetConfigFields returns the display-safe fields for a
// target's configuration. Anything it cannot parse becomes
// the neutral placeholder.
func targetConfigFields(
t *database.Target,
) []ConfigField {
switch t.Type {
case database.TargetTypeSlack:
return slackConfigFields(t.Config)
case database.TargetTypeHTTP:
return httpConfigFields(t)
case database.TargetTypeDatabase:
return databaseConfigFields(t.Config)
case database.TargetTypeLog:
// The log target takes no configuration.
return nil
default:
return unavailableConfigFields()
}
}
// unavailableConfigFields is the neutral placeholder shown
// for a configuration that could not be presented.
func unavailableConfigFields() []ConfigField {
return []ConfigField{{
Label: "Configuration",
Value: configUnavailable,
}}
}
// slackConfigFields describes a Slack target. Only the masked
// webhook URL is shown; the full URL is the credential.
func slackConfigFields(configJSON string) []ConfigField {
cfg, err := parseSlackConfig(configJSON)
if err != nil {
return unavailableConfigFields()
}
return []ConfigField{{
Label: "Webhook URL",
Value: cfg.MaskedWebhookURL(),
}}
}
// httpConfigFields describes an HTTP target: its destination
// and its retry settings. Header values are not shown — they
// routinely carry authorization tokens — only how many are
// configured.
func httpConfigFields(t *database.Target) []ConfigField {
cfg, err := parseHTTPConfig(t.Config)
if err != nil {
return unavailableConfigFields()
}
fields := []ConfigField{{
Label: "Destination URL",
Value: cfg.URL,
}}
if cfg.Timeout > 0 {
fields = append(fields, ConfigField{
Label: "Timeout",
Value: strconv.Itoa(cfg.Timeout) + "s",
})
}
if len(cfg.Headers) > 0 {
fields = append(fields, ConfigField{
Label: "Headers",
Value: fmt.Sprintf(
"%d configured", len(cfg.Headers),
),
})
}
return append(fields, retryFields(t)...)
}
// retryFields describes a target's retry settings, which live
// on the target row rather than in its configuration blob.
func retryFields(t *database.Target) []ConfigField {
retries := strconv.Itoa(t.MaxRetries)
if t.MaxRetries == 0 {
retries += " (fire-and-forget)"
}
fields := []ConfigField{{
Label: "Max Retries",
Value: retries,
}}
if t.MaxQueueSize > 0 {
fields = append(fields, ConfigField{
Label: "Max Queue Size",
Value: strconv.Itoa(t.MaxQueueSize),
})
}
return fields
}
// databaseConfigFields describes an archive target. Its
// configuration is optional, and an absent or empty expiry
// means the archive is kept forever. An expiry that is set
// but not a valid duration is reported as unavailable rather
// than echoed back.
func databaseConfigFields(configJSON string) []ConfigField {
expiry := archiveExpiryNever
if configJSON != "" {
var cfg databaseTargetConfig
err := json.Unmarshal([]byte(configJSON), &cfg)
if err != nil {
return unavailableConfigFields()
}
if cfg.Expiry != "" {
if ValidateArchiveExpiry(cfg.Expiry) != nil {
return unavailableConfigFields()
}
expiry = cfg.Expiry
}
}
return []ConfigField{{
Label: "Archive Expiry",
Value: expiry,
}}
}
// MaskedWebhookURL returns the Slack webhook URL reduced to
// its scheme and host, with the path, query and any userinfo
// elided. The path segments are the credential, so none of
// them is shown: the field accepts an arbitrary URL, so no
// segment can be assumed non-secret. A URL that does not
// parse into a scheme and host yields the neutral
// placeholder, never the raw string.
func (c *SlackTargetConfig) MaskedWebhookURL() string {
return maskURL(c.WebhookURL)
}
// maskURL renders a URL as scheme plus host with everything
// that can carry a secret removed.
func maskURL(raw string) string {
parsed, err := url.Parse(raw)
if err != nil || parsed.Scheme == "" ||
parsed.Host == "" {
return configUnavailable
}
masked := parsed.Scheme + "://" + parsed.Host
if parsed.Path != "" && parsed.Path != "/" {
masked += urlPathElision
}
return masked
}

View File

@@ -0,0 +1,299 @@
package delivery_test
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
const (
// slackSecretPath is the credential-bearing part of a
// Slack incoming webhook URL: everything after the host.
slackSecretPath = "/services/T00000000/B00000000/" +
"XXXXXXXXXXXXXXXXXXXXXXXX"
slackWebhookURL = "https://hooks.slack.com" +
slackSecretPath
viewExampleOrigin = "https://example.com"
viewExampleHook = viewExampleOrigin + "/hook"
viewUnavailable = "(unavailable)"
viewExpiryNever = "never"
)
func TestMaskedWebhookURL(t *testing.T) {
t.Parallel()
tests := map[string]struct {
url string
want string
}{
"slack webhook": {
url: slackWebhookURL,
want: "https://hooks.slack.com/...",
},
"query string dropped": {
url: viewExampleOrigin + "/a?token=secret",
want: viewExampleOrigin + "/...",
},
// Fabricated userinfo in a test URL, not a real
// credential.
//nolint:gosec // G101
"userinfo dropped": {
url: "https://user:pw@example.com/a/b",
want: viewExampleOrigin + "/...",
},
"no path": {
url: viewExampleOrigin,
want: viewExampleOrigin,
},
"root path": {
url: viewExampleOrigin + "/",
want: viewExampleOrigin,
},
"not a url": {
url: "definitely not a url",
want: viewUnavailable,
},
"empty": {
url: "",
want: viewUnavailable,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()
cfg := &delivery.SlackTargetConfig{
WebhookURL: tc.url,
}
assert.Equal(
t, tc.want, cfg.MaskedWebhookURL(),
)
})
}
}
// TestMaskedWebhookURL_NeverLeaksPath is the direct
// expression of the rule: whatever the input, the masked
// value never contains a path segment of it.
func TestMaskedWebhookURL_NeverLeaksPath(t *testing.T) {
t.Parallel()
cfg := &delivery.SlackTargetConfig{
WebhookURL: slackWebhookURL,
}
masked := cfg.MaskedWebhookURL()
assert.NotContains(t, masked, "T00000000")
assert.NotContains(t, masked, "B00000000")
assert.NotContains(
t, masked, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
assert.NotContains(t, masked, slackSecretPath)
}
// fieldMap turns a view's config fields into a lookup so
// assertions read by label.
func fieldMap(fields []delivery.ConfigField) map[string]string {
out := make(map[string]string, len(fields))
for _, f := range fields {
out[f.Label] = f.Value
}
return out
}
// viewFor projects a single target and returns its view.
func viewFor(
t *testing.T,
target database.Target,
) delivery.TargetView {
t.Helper()
views := delivery.NewTargetViews(
[]database.Target{target},
)
require.Len(t, views, 1)
return views[0]
}
func TestNewTargetViews_Slack(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Name: "slack-target",
Type: database.TargetTypeSlack,
Active: true,
Config: `{"webhookUrl":"` +
slackWebhookURL + `"}`,
})
assert.Equal(t, "slack-target", view.Name)
assert.Equal(
t,
map[string]string{
"Webhook URL": "https://hooks.slack.com/...",
},
fieldMap(view.Config),
)
}
func TestNewTargetViews_HTTP(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + viewExampleHook + `",` +
`"timeout":30,` +
`"headers":{"Authorization":"Bearer sekrit"}}`,
MaxRetries: 5,
MaxQueueSize: 100,
})
fields := fieldMap(view.Config)
assert.Equal(
t,
map[string]string{
"Destination URL": viewExampleHook,
"Timeout": "30s",
"Headers": "1 configured",
"Max Retries": "5",
"Max Queue Size": "100",
},
fields,
)
// Header values can be credentials and are never shown.
for _, v := range fields {
assert.NotContains(t, v, "sekrit")
}
}
func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + viewExampleHook + `"}`,
})
assert.Equal(
t,
map[string]string{
"Destination URL": viewExampleHook,
"Max Retries": "0 (fire-and-forget)",
},
fieldMap(view.Config),
)
}
func TestNewTargetViews_Database(t *testing.T) {
t.Parallel()
tests := map[string]struct {
config string
want string
}{
"empty config": {config: "", want: viewExpiryNever},
"empty expiry": {config: `{}`, want: viewExpiryNever},
"explicit": {
config: `{"expiry":"720h"}`,
want: "720h",
},
"never literal": {
config: `{"expiry":"` + viewExpiryNever + `"}`,
want: viewExpiryNever,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeDatabase,
Config: tc.config,
})
assert.Equal(
t,
map[string]string{"Archive Expiry": tc.want},
fieldMap(view.Config),
)
})
}
}
func TestNewTargetViews_Log(t *testing.T) {
t.Parallel()
view := viewFor(t, database.Target{
Type: database.TargetTypeLog,
Config: "",
})
assert.Empty(t, view.Config)
}
// TestNewTargetViews_Unpresentable proves that no config the
// view cannot present falls back to the stored blob.
func TestNewTargetViews_Unpresentable(t *testing.T) {
t.Parallel()
const blob = `{"webhookUrl":"https://hooks.slack.com` +
slackSecretPath + `"`
tests := map[string]database.Target{
"unknown target type": {
Type: database.TargetType("carrier-pigeon"),
Config: blob,
},
"unparseable json": {
Type: database.TargetTypeSlack,
Config: blob,
},
"empty slack config": {
Type: database.TargetTypeSlack,
},
"slack config without url": {
Type: database.TargetTypeSlack,
Config: `{}`,
},
"unparseable http json": {
Type: database.TargetTypeHTTP,
Config: `{"url":`,
},
"unparseable archive json": {
Type: database.TargetTypeDatabase,
Config: `{"expiry":`,
},
"invalid archive expiry": {
Type: database.TargetTypeDatabase,
Config: `{"expiry":"a fortnight"}`,
},
}
for name, target := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()
view := viewFor(t, target)
assert.Equal(
t,
map[string]string{
"Configuration": viewUnavailable,
},
fieldMap(view.Config),
)
})
}
}

View File

@@ -29,10 +29,8 @@ func (h *Handlers) HandleLoginPage() http.HandlerFunc {
// HandleLoginSubmit handles the login form submission (POST)
func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
// Limit request body to prevent memory exhaustion
r.Body = http.MaxBytesReader(w, r.Body, 1<<maxBodyShift)
// Parse form data
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
h.log.Error("failed to parse form", "error", err)

View File

@@ -31,9 +31,8 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
return
}
// Limit request body to prevent memory exhaustion.
r.Body = http.MaxBytesReader(w, r.Body, 1<<maxBodyShift)
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
h.log.Error("failed to parse form", "error", err)

View File

@@ -0,0 +1,185 @@
package handlers_test
import (
"context"
"net/http"
"net/http/httptest"
"testing"
"github.com/go-chi/chi"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
// The secret path segments of a Slack incoming webhook URL.
// Holding them is enough to post to the channel forever, so
// they must never reach the rendered page.
const (
slackSecretPath = "/services/T00000000/B00000000/" +
"XXXXXXXXXXXXXXXXXXXXXXXX"
slackWebhookURL = "https://hooks.slack.com" +
slackSecretPath
)
// seedConfiguredTarget inserts a target with a stored config
// blob.
func seedConfiguredTarget(
t *testing.T,
db *database.Database,
webhookID string,
targetType database.TargetType,
config string,
) {
t.Helper()
tgt := &database.Target{
WebhookID: webhookID,
Name: "t-" + string(targetType),
Type: targetType,
Active: true,
Config: config,
}
require.NoError(
t,
db.DB().Omit(clause.Associations).Create(tgt).Error,
)
}
// renderSourceDetailPage runs the real source detail handler
// for a webhook and returns the rendered HTML.
func renderSourceDetailPage(
t *testing.T,
h *handlers.Handlers,
sess *session.Session,
webhookID string,
) string {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodGet,
"/source/"+webhookID,
nil,
)
for _, c := range authenticatedCookies(
t, sess, deleteTestUserID, deleteTestUsername,
) {
req.AddCookie(c)
}
rctx := chi.NewRouteContext()
rctx.URLParams.Add(paramSourceID, webhookID)
req = req.WithContext(
context.WithValue(
req.Context(), chi.RouteCtxKey, rctx,
),
)
w := httptest.NewRecorder()
h.HandleSourceDetail().ServeHTTP(w, req)
require.Equal(t, http.StatusOK, w.Code)
return w.Body.String()
}
// TestHandleSourceDetail_MasksSlackWebhookURL is the
// load-bearing regression test for the credential leak: the
// rendered page must show the Slack target without any of the
// secret path segments of its webhook URL.
func TestHandleSourceDetail_MasksSlackWebhookURL(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.NotContains(
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
)
assert.NotContains(t, body, "webhookUrl")
assert.Contains(t, body, "Webhook URL")
assert.Contains(t, body, "https://hooks.slack.com/...")
}
// TestHandleSourceDetail_RendersNamedTargetFields proves the
// other target types render labelled fields rather than the
// stored blob.
func TestHandleSourceDetail_RendersNamedTargetFields(
t *testing.T,
) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook","timeout":30,`+
`"headers":{"Authorization":"Bearer sekrit"}}`,
)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetTypeDatabase,
`{"expiry":"720h"}`,
)
seedConfiguredTarget(
t, db, wh.ID,
database.TargetType("carrier-pigeon"),
`{"beak":"sharp"}`,
)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.Contains(t, body, "Destination URL")
assert.Contains(t, body, "https://example.com/hook")
assert.Contains(t, body, "Timeout")
assert.Contains(t, body, "1 configured")
assert.NotContains(t, body, "sekrit")
assert.Contains(t, body, "Archive Expiry")
assert.Contains(t, body, "720h")
// An unknown type gets the neutral placeholder, never the
// stored blob.
assert.Contains(t, body, "(unavailable)")
assert.NotContains(t, body, "beak")
}

View File

@@ -213,10 +213,8 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
return
}
r.Body = http.MaxBytesReader(
w, r.Body, 1<<maxBodyShift,
)
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
http.Error(
@@ -414,9 +412,12 @@ func (h *Handlers) renderSourceDetail(
data := map[string]any{
tmplKeyWebhook: &webhook,
"Entrypoints": entrypoints,
"Targets": targets,
"Events": events,
"BaseURL": scheme + "://" + host,
// Targets are projected to a display-safe view: the
// stored config blob holds credentials and must never
// reach a template.
"Targets": delivery.NewTargetViews(targets),
"Events": events,
"BaseURL": scheme + "://" + host,
}
h.renderTemplate(w, r, "source_detail.html", data)
@@ -482,10 +483,8 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
return
}
r.Body = http.MaxBytesReader(
w, r.Body, 1<<maxBodyShift,
)
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err = r.ParseForm()
if err != nil {
http.Error(
@@ -505,10 +504,8 @@ func (h *Handlers) applyWebhookEdit(
r *http.Request,
webhook *database.Webhook,
) {
r.Body = http.MaxBytesReader(
w, r.Body, 1<<maxBodyShift,
)
// The body size cap is enforced by the MaxBodySize middleware,
// which runs before CSRF parses the form.
name := r.FormValue("name")
if name == "" {
data := map[string]any{
@@ -887,10 +884,8 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
return
}
r.Body = http.MaxBytesReader(
w, r.Body, 1<<maxBodyShift,
)
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err = r.ParseForm()
if err != nil {
http.Error(
@@ -947,10 +942,8 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
return
}
r.Body = http.MaxBytesReader(
w, r.Body, 1<<maxBodyShift,
)
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err = r.ParseForm()
if err != nil {
http.Error(
@@ -970,10 +963,8 @@ func (h *Handlers) processTargetCreate(
r *http.Request,
webhook database.Webhook,
) {
r.Body = http.MaxBytesReader(
w, r.Body, 1<<maxBodyShift,
)
// The body size cap is enforced by the MaxBodySize middleware,
// which runs before CSRF parses the form.
name := r.FormValue("name")
targetType := database.TargetType(r.FormValue("type"))
targetURL := r.FormValue("url")

View File

@@ -309,10 +309,36 @@ func (s *Middleware) NoCache() func(http.Handler) http.Handler {
}
}
// MaxBodySize returns middleware that limits the request body size
// for POST requests. If the body exceeds the given limit in
// bytes, the server returns 413 Request Entity Too Large. This
// prevents clients from sending arbitrarily large form bodies.
// bodyLimitedMethod reports whether the request method carries a
// body that the MaxBodySize middleware should cap.
func bodyLimitedMethod(method string) bool {
return method == http.MethodPost ||
method == http.MethodPut ||
method == http.MethodPatch
}
// MaxBodySize returns middleware that limits the size of
// POST/PUT/PATCH request bodies to maxBytes. It must be registered
// before any middleware that parses the body — notably CSRF, which
// calls r.PostFormValue — so that form parsing happens under this
// cap rather than net/http's 10 MB default.
//
// Two enforcement paths exist, because http.MaxBytesReader alone
// cannot produce a 413: it reports the overflow as an error from
// Read, by which point the body parser downstream has already
// converted that error into its own response.
//
// - Declared oversize: the request announces a Content-Length
// greater than maxBytes. The middleware answers 413 Request
// Entity Too Large immediately and does not call the next
// handler, so neither CSRF nor the endpoint handler runs.
// - Undeclared oversize: the request is chunked (Content-Length
// of -1) or lies about its Content-Length. There is nothing to
// check up front, so http.MaxBytesReader hard-caps the body at
// maxBytes and the request fails downstream — the form parse
// errors out and CSRF rejects it with 403. The response is less
// precise than a 413, but the body is still never buffered
// beyond the cap, which is the property that matters.
func (s *Middleware) MaxBodySize(
maxBytes int64,
) func(http.Handler) http.Handler {
@@ -321,14 +347,31 @@ func (s *Middleware) MaxBodySize(
w http.ResponseWriter,
r *http.Request,
) {
if r.Method == http.MethodPost ||
r.Method == http.MethodPut ||
r.Method == http.MethodPatch {
r.Body = http.MaxBytesReader(
w, r.Body, maxBytes,
)
if !bodyLimitedMethod(r.Method) {
next.ServeHTTP(w, r)
return
}
if r.ContentLength > maxBytes {
s.log.Warn(
"request body exceeds limit",
"method", r.Method,
"path", r.URL.Path,
"content_length", r.ContentLength,
"limit", maxBytes,
)
http.Error(
w,
"Request Entity Too Large",
http.StatusRequestEntityTooLarge,
)
return
}
r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
next.ServeHTTP(w, r)
})
}

View File

@@ -3,10 +3,12 @@ package middleware_test
import (
"context"
"encoding/base64"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"time"
@@ -648,6 +650,153 @@ func TestNoCache_SetsHeaders(t *testing.T) {
)
}
// --- MaxBodySize Middleware Tests ---
const testBodyLimit int64 = 64
// maxBodySizeHandler wraps a sentinel handler in MaxBodySize with
// testBodyLimit. The sentinel records whether it ran and how much of
// the body it managed to read, so tests can distinguish "never
// reached" from "reached but truncated".
type maxBodySizeResult struct {
called bool
read int
readErr error
response *httptest.ResponseRecorder
}
func runMaxBodySize(
t *testing.T,
req *http.Request,
) *maxBodySizeResult {
t.Helper()
m, _ := testMiddleware(t, config.EnvironmentDev)
res := &maxBodySizeResult{response: httptest.NewRecorder()}
handler := m.MaxBodySize(testBodyLimit)(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
res.called = true
body, err := io.ReadAll(r.Body)
res.read = len(body)
res.readErr = err
w.WriteHeader(http.StatusOK)
},
))
handler.ServeHTTP(res.response, req)
return res
}
// postWithBody builds a POST request whose Content-Length is
// accurate for the given payload size.
func postWithBody(size int) *http.Request {
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodPost, "/pages/login",
strings.NewReader(strings.Repeat("a", size)),
)
req.Header.Set(
"Content-Type", "application/x-www-form-urlencoded",
)
return req
}
func TestMaxBodySize_DeclaredOversize_413AndHandlerNotReached(
t *testing.T,
) {
t.Parallel()
res := runMaxBodySize(t, postWithBody(int(testBodyLimit)+1))
assert.False(
t, res.called,
"handler must not be reached for an oversized body",
)
assert.Equal(
t, http.StatusRequestEntityTooLarge, res.response.Code,
)
}
func TestMaxBodySize_AtLimit_PassesThrough(t *testing.T) {
t.Parallel()
res := runMaxBodySize(t, postWithBody(int(testBodyLimit)))
assert.True(
t, res.called,
"handler should be reached for a body at the limit",
)
require.NoError(t, res.readErr)
assert.Equal(t, int(testBodyLimit), res.read)
assert.Equal(t, http.StatusOK, res.response.Code)
}
func TestMaxBodySize_UnderLimit_PassesThrough(t *testing.T) {
t.Parallel()
res := runMaxBodySize(t, postWithBody(1))
assert.True(t, res.called)
require.NoError(t, res.readErr)
assert.Equal(t, 1, res.read)
assert.Equal(t, http.StatusOK, res.response.Code)
}
func TestMaxBodySize_GetWithOversizeBody_NotCapped(t *testing.T) {
t.Parallel()
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodGet, "/pages/login",
strings.NewReader(
strings.Repeat("a", int(testBodyLimit)+1),
),
)
res := runMaxBodySize(t, req)
assert.True(
t, res.called,
"GET requests are not subject to the POST body cap",
)
require.NoError(t, res.readErr)
assert.Equal(t, int(testBodyLimit)+1, res.read)
}
// TestMaxBodySize_UndeclaredOversize_TruncatedAtCap covers the
// chunked / lying-Content-Length case: there is nothing to check up
// front, so the request reaches the handler but MaxBytesReader
// hard-caps the body and the read fails at the limit.
func TestMaxBodySize_UndeclaredOversize_TruncatedAtCap(
t *testing.T,
) {
t.Parallel()
req := postWithBody(int(testBodyLimit) + 1)
// Simulate a chunked request: no declared length.
req.ContentLength = -1
res := runMaxBodySize(t, req)
assert.True(
t, res.called,
"an undeclared oversize body cannot be rejected up front",
)
require.Error(
t, res.readErr,
"reading past the cap must fail",
)
assert.Equal(
t, int(testBodyLimit), res.read,
"the handler must not see more than the cap",
)
}
// --- Helper Tests ---
func TestIpFromHostPort(t *testing.T) {

View File

@@ -0,0 +1,36 @@
package server
import (
"log/slog"
"net/http"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/middleware"
)
// MaxFormBodySizeForTest exposes the form body cap so tests can
// build requests that sit exactly at, below, and above it.
const MaxFormBodySizeForTest = maxFormBodySize
// NewRouterForTest builds the real route tree via SetupRoutes with
// the supplied middleware and handlers, bypassing the fx lifecycle
// and the HTTP listener. Tests use it so that route-group middleware
// registration order is exercised exactly as it ships, rather than
// against a hand-rebuilt chain that could drift from routes.go.
func NewRouterForTest(
log *slog.Logger,
cfg *config.Config,
mw *middleware.Middleware,
h *handlers.Handlers,
) http.Handler {
s := &Server{
log: log,
mw: mw,
h: h,
params: ServerParams{Config: cfg},
}
s.SetupRoutes()
return s.router
}

View File

@@ -90,9 +90,11 @@ func (s *Server) setupRoutes() {
func (s *Server) setupPageRoutes() {
s.router.Route("/pages", func(r chi.Router) {
// MaxBodySize must precede CSRF: gorilla/csrf parses the
// form, so the cap has to be installed before it runs.
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Use(s.mw.CSRF())
r.Use(s.mw.NoCache())
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Group(func(r chi.Router) {
r.Use(s.mw.LoginRateLimit())
@@ -106,6 +108,9 @@ func (s *Server) setupPageRoutes() {
func (s *Server) setupUserRoutes() {
s.router.Route("/user/{username}", func(r chi.Router) {
// MaxBodySize must precede CSRF: gorilla/csrf parses the
// form, so the cap has to be installed before it runs.
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Use(s.mw.CSRF())
r.Use(s.mw.NoCache())
r.Use(s.mw.RequireAuth())
@@ -118,20 +123,24 @@ func (s *Server) setupUserRoutes() {
func (s *Server) setupSourceRoutes() {
s.router.Route("/sources", func(r chi.Router) {
// MaxBodySize must precede CSRF: gorilla/csrf parses the
// form, so the cap has to be installed before it runs.
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Use(s.mw.CSRF())
r.Use(s.mw.NoCache())
r.Use(s.mw.RequireAuth())
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Get("/", s.h.HandleSourceList())
r.Get("/new", s.h.HandleSourceCreate())
r.Post("/new", s.h.HandleSourceCreateSubmit())
})
s.router.Route("/source/{sourceID}", func(r chi.Router) {
// MaxBodySize must precede CSRF: gorilla/csrf parses the
// form, so the cap has to be installed before it runs.
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Use(s.mw.CSRF())
r.Use(s.mw.NoCache())
r.Use(s.mw.RequireAuth())
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Get("/", s.h.HandleSourceDetail())
r.Get("/edit", s.h.HandleSourceEdit())
r.Post("/edit", s.h.HandleSourceEditSubmit())

View File

@@ -0,0 +1,383 @@
package server_test
import (
"context"
"html"
"net/http"
"net/http/httptest"
"net/url"
"regexp"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/server"
"sneak.berlin/go/webhooker/internal/session"
)
// csrfCookieName is the cookie gorilla/csrf issues when it runs. Its
// presence or absence on a response is how these tests tell whether
// the CSRF middleware executed.
const csrfCookieName = "_gorilla_csrf"
type noopNotifier struct{}
func (n *noopNotifier) Notify([]delivery.Task) {}
// noopEvictor satisfies handlers.New's delivery.WebhookEvictor
// dependency. These tests never delete a webhook, so there is
// nothing to record.
type noopEvictor struct{}
func (e *noopEvictor) EvictWebhook(string) {}
// testEnv is the real router from routes.go plus the collaborators
// tests need to seed users and forge sessions.
type testEnv struct {
router http.Handler
sess *session.Session
db *database.Database
}
// newTestEnv wires the dependency graph with fx and builds the
// production route tree, so middleware registration order is
// exercised exactly as it ships.
func newTestEnv(t *testing.T) *testEnv {
t.Helper()
var (
log *logger.Logger
cfg *config.Config
mw *middleware.Middleware
hnd *handlers.Handlers
sess *session.Session
db *database.Database
)
app := fxtest.New(
t,
fx.Provide(
globals.New,
logger.New,
func() *config.Config {
return &config.Config{
DataDir: t.TempDir(),
Environment: config.EnvironmentDev,
}
},
database.New,
database.NewWebhookDBManager,
healthcheck.New,
session.New,
func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} },
middleware.New,
handlers.New,
),
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db),
)
app.RequireStart()
t.Cleanup(app.RequireStop)
return &testEnv{
router: server.NewRouterForTest(log.Get(), cfg, mw, hnd),
sess: sess,
db: db,
}
}
// oversizeValue returns a form value one byte past the route-group
// body cap, so an encoded form containing it is guaranteed oversize.
func oversizeValue() string {
return strings.Repeat("a", int(server.MaxFormBodySizeForTest)+1)
}
// csrfCookieSet reports whether the response issued a gorilla/csrf
// cookie, which only happens if the CSRF middleware ran.
func csrfCookieSet(w *httptest.ResponseRecorder) bool {
for _, c := range w.Result().Cookies() {
if c.Name == csrfCookieName {
return true
}
}
return false
}
// get issues a GET through the router with the supplied cookies.
func (e *testEnv) get(
path string,
cookies []*http.Cookie,
) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, path, nil,
)
for _, c := range cookies {
req.AddCookie(c)
}
w := httptest.NewRecorder()
e.router.ServeHTTP(w, req)
return w
}
// post issues a urlencoded form POST through the router. The body is
// a strings.Reader, so the request carries an accurate
// Content-Length — the signal MaxBodySize checks up front.
func (e *testEnv) post(
path string,
form url.Values,
cookies []*http.Cookie,
) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, path,
strings.NewReader(form.Encode()),
)
req.Header.Set(
"Content-Type", "application/x-www-form-urlencoded",
)
for _, c := range cookies {
req.AddCookie(c)
}
w := httptest.NewRecorder()
e.router.ServeHTTP(w, req)
return w
}
// csrfFrom renders the page at path and returns the CSRF token from
// its form together with every cookie needed for the follow-up POST.
func (e *testEnv) csrfFrom(
t *testing.T,
path string,
cookies []*http.Cookie,
) (string, []*http.Cookie) {
t.Helper()
w := e.get(path, cookies)
require.Equal(t, http.StatusOK, w.Code)
pattern := regexp.MustCompile(
`name="csrf_token" value="([^"]+)"`,
)
match := pattern.FindStringSubmatch(w.Body.String())
require.Len(t, match, 2, "form must embed a CSRF token")
// html/template escapes "+" and "=" in attribute values, and
// gorilla/csrf tokens are standard base64, so the value read
// out of the markup has to be unescaped before it is submitted.
token := html.UnescapeString(match[1])
combined := make([]*http.Cookie, 0, len(cookies))
combined = append(combined, cookies...)
combined = append(combined, w.Result().Cookies()...)
return token, combined
}
// authCookies forges an authenticated session for the given user.
func (e *testEnv) authCookies(
t *testing.T,
userID, username string,
) []*http.Cookie {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, "/setup", nil,
)
w := httptest.NewRecorder()
s, err := e.sess.Get(req)
require.NoError(t, err)
e.sess.SetUser(s, userID, username)
require.NoError(t, e.sess.Save(req, w, s))
cookies := w.Result().Cookies()
require.NotEmpty(t, cookies, "session cookie should be set")
return cookies
}
// seedUser creates a user with the given password and returns the
// stored hash so tests can assert whether it later changed.
func (e *testEnv) seedUser(
t *testing.T,
username, password string,
) (string, string) {
t.Helper()
hash, err := database.HashPassword(password)
require.NoError(t, err)
user := &database.User{Username: username, Password: hash}
require.NoError(t, e.db.DB().Create(user).Error)
return user.ID, hash
}
// storedHash reads the current password hash for a username.
func (e *testEnv) storedHash(t *testing.T, username string) string {
t.Helper()
var user database.User
require.NoError(t,
e.db.DB().Where("username = ?", username).
First(&user).Error,
)
return user.Password
}
// --- /pages group ---
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs
// ahead of gorilla/csrf: the response is a clean 413 and no CSRF
// cookie was issued, so neither the CSRF middleware nor the login
// handler ran.
func TestPagesLogin_OversizeBody_RejectedBeforeCSRF(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
form := url.Values{}
form.Set("username", oversizeValue())
form.Set("password", "irrelevant")
w := env.post("/pages/login", form, nil)
assert.Equal(
t, http.StatusRequestEntityTooLarge, w.Code,
)
assert.False(
t, csrfCookieSet(w),
"CSRF middleware must not run for an oversized body",
)
}
// TestPagesLogin_UnderLimit_NoToken_CSRFRejects is the control for
// the test above: an identically shaped but under-limit POST does
// reach gorilla/csrf, which rejects it and issues its cookie. Without
// this, the missing-cookie assertion above would prove nothing.
func TestPagesLogin_UnderLimit_NoToken_CSRFRejects(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
form := url.Values{}
form.Set("username", "someone")
form.Set("password", "irrelevant")
w := env.post("/pages/login", form, nil)
assert.Equal(t, http.StatusForbidden, w.Code)
assert.True(
t, csrfCookieSet(w),
"CSRF middleware should run for an under-limit body",
)
}
// TestPagesLogin_UnderLimit_ValidToken_ReachesHandler proves the
// reorder did not break CSRF token handling: a token harvested from
// the rendered login form is still accepted and the request lands in
// the handler.
func TestPagesLogin_UnderLimit_ValidToken_ReachesHandler(
t *testing.T,
) {
t.Parallel()
env := newTestEnv(t)
token, cookies := env.csrfFrom(t, "/pages/login", nil)
form := url.Values{}
form.Set("csrf_token", token)
form.Set("username", "nosuchuser")
form.Set("password", "wrongpassword")
w := env.post("/pages/login", form, cookies)
assert.Equal(t, http.StatusUnauthorized, w.Code)
assert.Contains(
t, w.Body.String(), "Invalid username or password",
"request should reach the login handler",
)
}
// --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
// covers the route that previously had no middleware body cap at
// all. The request carries a valid session and a valid CSRF token,
// so the only thing that can stop it is the size cap; the unchanged
// password hash is the observable proof the handler never ran.
func TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged(
t *testing.T,
) {
t.Parallel()
env := newTestEnv(t)
userID, originalHash := env.seedUser(t, "pwuser", "oldpassword")
cookies := env.authCookies(t, userID, "pwuser")
token, cookies := env.csrfFrom(t, "/user/pwuser/", cookies)
form := url.Values{}
form.Set("csrf_token", token)
form.Set("current_password", "oldpassword")
form.Set("new_password", oversizeValue())
form.Set("confirm_password", oversizeValue())
w := env.post("/user/pwuser/password", form, cookies)
assert.Equal(
t, http.StatusRequestEntityTooLarge, w.Code,
)
assert.Equal(
t, originalHash, env.storedHash(t, "pwuser"),
"handler must not run, so the password must be unchanged",
)
}
// TestPasswordChange_UnderLimit_Succeeds proves that adding the cap
// to the /user/{username} group did not break the route it guards.
func TestPasswordChange_UnderLimit_Succeeds(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
userID, originalHash := env.seedUser(t, "okuser", "oldpassword")
cookies := env.authCookies(t, userID, "okuser")
token, cookies := env.csrfFrom(t, "/user/okuser/", cookies)
form := url.Values{}
form.Set("csrf_token", token)
form.Set("current_password", "oldpassword")
form.Set("new_password", "brandnewpassword")
form.Set("confirm_password", "brandnewpassword")
w := env.post("/user/okuser/password", form, cookies)
assert.Equal(t, http.StatusOK, w.Code)
assert.NotEqual(
t, originalHash, env.storedHash(t, "okuser"),
"an under-limit password change should still apply",
)
}

View File

@@ -145,8 +145,11 @@
</form>
</div>
</div>
{{if .Config}}
<code class="text-xs text-gray-500 break-all block mt-1">{{.Config}}</code>
{{range .Config}}
<div class="text-xs text-gray-500 break-all mt-1">
<span class="font-medium text-gray-700">{{.Label}}:</span>
<span>{{.Value}}</span>
</div>
{{end}}
</div>
{{else}}