1 Commits

Author SHA1 Message Date
08c9c1a5d8 Enforce the body size limit before CSRF parses the form (closes #90)
All checks were successful
check / check (push) Successful in 3m6s
chi runs Use middleware in registration order, and every form route
group registered CSRF() before MaxBodySize(). gorilla/csrf calls
r.PostFormValue, so the form was parsed under net/http's 10 MB default
and the intended 1 MB cap never applied to form fields. The
/user/{username} group, which carries POST /password, had no
MaxBodySize registration at all.

- Register MaxBodySize ahead of CSRF in /pages, /sources, and
  /source/{sourceID}, and add it to /user/{username}.
- Reject a declared-oversize body up front with 413. Reordering alone
  cannot produce one: http.MaxBytesReader surfaces its error on Read,
  so the form parse fails and gorilla/csrf answers 403 "no token" for
  what is really an oversized body. MaxBytesReader is still installed
  afterwards so chunked or length-lying clients stay hard-capped.
- Drop the handler-local MaxBytesReader calls in auth.go, profile.go,
  and source_management.go now that the middleware is the single
  enforcement point. maxBodyShift stays; webhook.go still uses it.

The /webhook/{uuid} receiver is untouched: it bounds itself with
io.LimitReader in readWebhookBody and is neither CSRF-protected nor
form-parsed.

Tests cover the middleware in isolation (declared oversize is rejected
without reaching a sentinel handler; at-limit and under-limit bodies
pass through intact; GET is unaffected; an undeclared oversize body is
truncated at the cap) and the real router built by SetupRoutes, so the
registration order itself is guarded: an oversized POST to
/pages/login returns 413 with no gorilla/csrf cookie issued, an
oversized POST /password with a valid session and CSRF token returns
413 and leaves the stored hash unchanged, and under-limit requests
still complete through the normal CSRF path.
2026-08-09 01:53:34 +00:00
21 changed files with 713 additions and 980 deletions

View File

@@ -307,20 +307,13 @@ event routing.
| `user_id` | UUID | Foreign key → User | | `user_id` | UUID | Foreign key → User |
| `name` | string | Human-readable name | | `name` | string | Human-readable name |
| `description` | string | Optional description | | `description` | string | Optional description |
| `retention_days` | integer | Days to retain events (default: 30; 0 means retain forever) | | `retention_days` | integer | Days to retain events (default: 30) |
**Relations:** Belongs to User. Has many Entrypoints. Has many Targets. **Relations:** Belongs to User. Has many Entrypoints. Has many Targets.
The `retention_days` field controls how long event data is kept in the The `retention_days` field controls how long event data is kept in the
webhook's dedicated database before automatic cleanup. webhook's dedicated database before automatic cleanup.
Setting `retention_days` to `0` means "retain events forever". Because
the column carries a default of 30, a literal zero cannot survive an
insert, so a zero is rewritten on save to a sentinel of `365 * 1000`
days (`database.RetentionForeverDays`). The retention reaper recognises
that sentinel and skips the webhook entirely, and the web UI displays
such a webhook's retention as "forever" rather than as a day count.
#### Entrypoint #### Entrypoint
A receiver URL where external services POST webhook events. Each A receiver URL where external services POST webhook events. Each
@@ -516,7 +509,7 @@ This separation provides:
DB; the event database file is hard-deleted (permanently removed). DB; the event database file is hard-deleted (permanently removed).
- **Per-webhook retention** — the `retention_days` field on each webhook - **Per-webhook retention** — the `retention_days` field on each webhook
controls automatic cleanup of old events in that webhook's database controls automatic cleanup of old events in that webhook's database
only, or disables cleanup entirely when set to `0` (retain forever). only.
- **Performance** — each webhook's database has its own WAL, its own - **Performance** — each webhook's database has its own WAL, its own
page cache, and its own lock, so concurrent event ingestion across page cache, and its own lock, so concurrent event ingestion across
webhooks won't contend. webhooks won't contend.
@@ -874,9 +867,17 @@ Applied to all routes in this order:
8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set; 8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set;
configured with `Repanic: true` so panics still reach Recoverer) configured with `Repanic: true` so panics still reach Recoverer)
Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a Additionally, form endpoints (`/pages`, `/user/*`, `/sources`,
**MaxBodySize** middleware that limits POST/PUT/PATCH request bodies to `/source/*`) apply a **MaxBodySize** middleware that limits
1 MB using `http.MaxBytesReader`, preventing oversized form submissions. 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 ### Authentication
@@ -898,7 +899,8 @@ Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a
- Production security headers on all responses: HSTS, X-Content-Type-Options - Production security headers on all responses: HSTS, X-Content-Type-Options
(`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy, (`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy,
and Permissions-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) - **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf)
on all state-changing forms (cookie-based double-submit tokens with on all state-changing forms (cookie-based double-submit tokens with
HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and

13
TODO.md
View File

@@ -28,12 +28,13 @@ databases currently grow without bound.
# Completed Steps # Completed Steps
- 2026-08-09 Make retain-forever reachable from the normal create and - 2026-08-09 Enforce the request body size limit before the CSRF
edit flows (#79): a `RetentionForeverDays = 365 * 1000` sentinel, a middleware parses the form (#90): `MaxBodySize` is now registered
`Webhook.BeforeSave` hook rewriting any non-positive `retention_days` ahead of `CSRF()` in every form route group, the `/user/{username}`
to it ahead of GORM's own column defaulting, a reaper that skips such group gained the cap it never had (which is where `POST /password`
webhooks outright, form validation that honours `0` and rejects lives), the middleware rejects a declared-oversize body with a real
garbage with a 400, and a retention UI that says "forever" 413 up front, and the redundant handler-local
`http.MaxBytesReader` calls were removed
- 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in - 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
`Dockerfile`, release-archive sha256 pins in `script/bootstrap`), `Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so

View File

@@ -18,11 +18,6 @@ const (
testVersion = "test" testVersion = "test"
// testContentType is the event content type used in tests. // testContentType is the event content type used in tests.
testContentType = "application/json" testContentType = "application/json"
// testWebhookName is the Webhook.Name used in tests.
testWebhookName = "test-webhook"
// testForeverLabel is Webhook.RetentionLabel for a retain-forever
// webhook.
testForeverLabel = "forever"
) )
func setupTestDB( func setupTestDB(

View File

@@ -1,39 +1,6 @@
package database package database
import (
"strconv"
"gorm.io/gorm"
)
const (
// DefaultRetentionDays is the event retention period applied to a
// webhook created without an explicit retention value. It is the
// single source of truth for that policy and must stay in sync
// with the `gorm:"default:30"` column default on
// Webhook.RetentionDays below; a struct tag cannot reference a
// constant, so a test asserts the two agree.
DefaultRetentionDays = 30
// RetentionForeverDays is the sentinel RetentionDays value meaning
// "retain events forever". Users express that intent as 0, which
// Webhook.BeforeSave rewrites to this value: the column default
// substitutes DefaultRetentionDays for a zero value at insert
// time, so a zero can never survive a round trip to the database.
// Nothing outside this file may hardcode the number.
RetentionForeverDays = 365 * 1000
)
// Webhook represents a webhook processing unit that groups entrypoints and targets // Webhook represents a webhook processing unit that groups entrypoints and targets
//
// The receiver kinds below are deliberately mixed. BeforeSave has to
// take a pointer because it mutates the record, and GORM only invokes
// hooks declared that way. RetainsForever and RetentionLabel have to
// take values because html/template calls them on webhooks held in a
// template data map, which reflection cannot address; a pointer
// receiver there fails at render time rather than at compile time.
//
//nolint:recvcheck // GORM needs a pointer hook; templates need values.
type Webhook struct { type Webhook struct {
BaseModel BaseModel
@@ -41,9 +8,7 @@ type Webhook struct {
Name string `gorm:"not null" json:"name"` Name string `gorm:"not null" json:"name"`
Description string `json:"description"` Description string `json:"description"`
// RetentionDays is the number of days to retain events. A value of // RetentionDays is the number of days to retain events.
// RetentionForeverDays means retain forever. The column default
// must equal DefaultRetentionDays.
RetentionDays int `gorm:"default:30" json:"retentionDays"` RetentionDays int `gorm:"default:30" json:"retentionDays"`
// Relations // Relations
@@ -51,44 +16,3 @@ type Webhook struct {
Entrypoints []Entrypoint `json:"entrypoints,omitempty"` Entrypoints []Entrypoint `json:"entrypoints,omitempty"`
Targets []Target `json:"targets,omitempty"` Targets []Target `json:"targets,omitempty"`
} }
// BeforeSave normalises RetentionDays on every insert and update. A
// non-positive value is the user's way of asking for "retain forever",
// which is stored as the RetentionForeverDays sentinel.
//
// This has to happen in a hook rather than at the call sites. GORM
// substitutes the column default (DefaultRetentionDays) for a zero
// value while building the insert statement, which runs after
// BeforeSave; rewriting any later than this loses that race and the
// row lands at 30 days. Living on the model also means a future call
// site — a REST API, a fixture, a migration — cannot bypass it.
func (w *Webhook) BeforeSave(_ *gorm.DB) error {
if w.RetentionDays <= 0 {
w.RetentionDays = RetentionForeverDays
}
return nil
}
// RetainsForever reports whether this webhook's events are kept
// indefinitely. It accepts the RetentionForeverDays sentinel written by
// BeforeSave and, defensively, the non-positive values that rows
// written before the sentinel existed may still carry.
func (w Webhook) RetainsForever() bool {
return w.RetentionDays <= 0 ||
w.RetentionDays >= RetentionForeverDays
}
// RetentionLabel returns the webhook's retention policy as display
// text, so that no template has to know about the sentinel value.
func (w Webhook) RetentionLabel() string {
if w.RetainsForever() {
return "forever"
}
if w.RetentionDays == 1 {
return "1 day"
}
return strconv.Itoa(w.RetentionDays) + " days"
}

View File

@@ -1,184 +0,0 @@
package database_test
import (
"context"
"reflect"
"strconv"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
)
// startedTestDB returns a started main database for model-level tests.
func startedTestDB(t *testing.T) *gorm.DB {
t.Helper()
db, lc := setupTestDB(t)
ctx := context.Background()
require.NoError(t, lc.Start(ctx))
t.Cleanup(func() { require.NoError(t, lc.Stop(ctx)) })
return db.DB()
}
// storedRetention reads the retention_days column straight out of the
// row, so the assertion is about what was persisted rather than about
// whatever the in-memory struct happens to hold.
func storedRetention(t *testing.T, db *gorm.DB, id string) int {
t.Helper()
var got int
require.NoError(
t,
db.Model(&database.Webhook{}).
Where("id = ?", id).
Pluck("retention_days", &got).Error,
)
return got
}
// newWebhookWithRetention creates a webhook through the ordinary Create
// path, so the BeforeSave hook and the GORM column default both apply
// exactly as they do in production.
func newWebhookWithRetention(
t *testing.T,
db *gorm.DB,
wh *database.Webhook,
) string {
t.Helper()
wh.UserID = uuid.New().String()
wh.Name = testWebhookName
require.NoError(
t,
db.Omit(clause.Associations).Create(wh).Error,
)
return wh.ID
}
func TestWebhookBeforeSave_ZeroBecomesForeverSentinel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: 0}
id := newWebhookWithRetention(t, db, wh)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetention(t, db, id),
"a zero retention must be stored as the sentinel, "+
"not replaced by the column default",
)
}
func TestWebhookBeforeSave_NegativeBecomesForeverSentinel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: -5}
id := newWebhookWithRetention(t, db, wh)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetention(t, db, id),
)
}
func TestWebhookBeforeSave_PositiveIsPreserved(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: 7}
id := newWebhookWithRetention(t, db, wh)
assert.Equal(t, 7, storedRetention(t, db, id))
}
// TestWebhookBeforeSave_UpdateToZeroBecomesSentinel proves the hook
// fires on update as well as insert, via the same Save call the edit
// handler makes.
func TestWebhookBeforeSave_UpdateToZeroBecomesSentinel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: 30}
id := newWebhookWithRetention(t, db, wh)
require.Equal(t, 30, storedRetention(t, db, id))
wh.RetentionDays = 0
require.NoError(t, db.Omit(clause.Associations).Save(wh).Error)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetention(t, db, id),
)
}
// TestWebhookRetentionColumnDefaultMatchesConstant guards the one place
// the default lives twice: a struct tag cannot reference a constant, so
// this asserts the tag and DefaultRetentionDays agree.
func TestWebhookRetentionColumnDefaultMatchesConstant(t *testing.T) {
t.Parallel()
field, ok := reflect.TypeFor[database.Webhook]().
FieldByName("RetentionDays")
require.True(t, ok, "Webhook.RetentionDays must exist")
assert.Equal(
t,
"default:"+strconv.Itoa(database.DefaultRetentionDays),
field.Tag.Get("gorm"),
)
}
func TestWebhookRetainsForeverAndLabel(t *testing.T) {
t.Parallel()
cases := []struct {
name string
days int
forever bool
label string
}{
{
"sentinel",
database.RetentionForeverDays, true, testForeverLabel,
},
{
"above sentinel",
database.RetentionForeverDays + 1, true, testForeverLabel,
},
{"legacy zero", 0, true, testForeverLabel},
{"legacy negative", -1, true, testForeverLabel},
{"default", database.DefaultRetentionDays, false, "30 days"},
{"one day", 1, false, "1 day"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
wh := database.Webhook{RetentionDays: tc.days}
assert.Equal(t, tc.forever, wh.RetainsForever())
assert.Equal(t, tc.label, wh.RetentionLabel())
})
}
}

View File

@@ -114,8 +114,7 @@ func (r *RetentionReaper) run(ctx context.Context) {
} }
// sweep lists every webhook from the main database and reaps expired // sweep lists every webhook from the main database and reaps expired
// rows from each per-webhook database that has a finite retention // rows from each per-webhook database whose RetentionDays is positive.
// policy. Webhooks set to retain forever are skipped entirely.
func (r *RetentionReaper) sweep(ctx context.Context) { func (r *RetentionReaper) sweep(ctx context.Context) {
var webhooks []Webhook var webhooks []Webhook
@@ -140,13 +139,8 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
wh := webhooks[i] wh := webhooks[i]
// Skip retain-forever webhooks before building any query. // RetentionDays of zero or less means retain forever.
// RetainsForever covers both the RetentionForeverDays if wh.RetentionDays <= 0 {
// sentinel and the non-positive values that predate it: the
// sentinel is a positive number, so without this the reaper
// would compute a cutoff a thousand years in the past and
// issue a DELETE matching nothing on every single sweep.
if wh.RetainsForever() {
continue continue
} }

View File

@@ -77,7 +77,7 @@ func createWebhook(
wh := &database.Webhook{ wh := &database.Webhook{
UserID: uuid.New().String(), UserID: uuid.New().String(),
Name: testWebhookName, Name: "test-webhook",
RetentionDays: retentionDays, RetentionDays: retentionDays,
} }
require.NoError( require.NoError(
@@ -85,11 +85,10 @@ func createWebhook(
db.Omit(clause.Associations).Create(wh).Error, db.Omit(clause.Associations).Create(wh).Error,
) )
// Webhook.BeforeSave rewrites a non-positive RetentionDays to the // The RetentionDays column carries a GORM default of 30, so a
// retain-forever sentinel, and the column's GORM default would // zero (or negative) value passed to Create is replaced by that
// otherwise substitute 30. Force the requested value with a // default. Force the requested value explicitly so the
// column-level update so tests can plant legacy rows that predate // retain-forever (<= 0) path can be exercised.
// the sentinel and still carry a literal 0 or negative value.
require.NoError( require.NoError(
t, t,
db.Model(wh). db.Model(wh).
@@ -99,30 +98,6 @@ func createWebhook(
return wh.ID return wh.ID
} }
// createWebhookNormally inserts a webhook through the ordinary Create
// path, with no column-level forcing, so Webhook.BeforeSave applies
// exactly as it does in production. Passing 0 therefore yields a row
// holding the RetentionForeverDays sentinel.
func createWebhookNormally(
t *testing.T,
db *gorm.DB,
retentionDays int,
) string {
t.Helper()
wh := &database.Webhook{
UserID: uuid.New().String(),
Name: testWebhookName,
RetentionDays: retentionDays,
}
require.NoError(
t,
db.Omit(clause.Associations).Create(wh).Error,
)
return wh.ID
}
// eventChain is the set of row IDs seeded for a single event. // eventChain is the set of row IDs seeded for a single event.
type eventChain struct { type eventChain struct {
eventID string eventID string
@@ -281,59 +256,12 @@ func TestRetentionReaper_ReapsExpiredKeepsRecent(t *testing.T) {
assertChainPresent(t, db, recent) assertChainPresent(t, db, recent)
} }
// TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep covers the
// end-to-end retain-forever path: a webhook created the normal way with
// a requested retention of 0 lands on the RetentionForeverDays
// sentinel, and the reaper leaves its ancient events alone while still
// reaping a finite-retention webhook in the very same sweep.
func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
t *testing.T,
) {
t.Parallel()
env := setupRetentionTest(t)
foreverID := createWebhookNormally(t, env.mainDB.DB(), 0)
var stored database.Webhook
require.NoError(
t,
env.mainDB.DB().Where("id = ?", foreverID).
First(&stored).Error,
)
require.Equal(
t,
database.RetentionForeverDays,
stored.RetentionDays,
"a requested retention of 0 must persist as the sentinel",
)
finiteID := createWebhookNormally(t, env.mainDB.DB(), 30)
foreverDB, err := env.mgr.GetDB(foreverID)
require.NoError(t, err)
finiteDB, err := env.mgr.GetDB(finiteID)
require.NoError(t, err)
ancient := time.Now().Add(-365 * 24 * time.Hour)
kept := seedEventChain(t, foreverDB, foreverID, ancient)
doomed := seedEventChain(t, finiteDB, finiteID, ancient)
env.reaper.ExportSweep(context.Background())
assertChainPresent(t, foreverDB, kept)
assertChainGone(t, finiteDB, doomed)
}
func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) { func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
t.Parallel() t.Parallel()
env := setupRetentionTest(t) env := setupRetentionTest(t)
// A legacy row written before the sentinel existed still carries a // RetentionDays of zero means retain forever.
// literal 0; the <= 0 guard must keep honouring it.
webhookID := createWebhook(t, env.mainDB.DB(), 0) webhookID := createWebhook(t, env.mainDB.DB(), 0)
db, err := env.mgr.GetDB(webhookID) db, err := env.mgr.GetDB(webhookID)

View File

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

View File

@@ -26,6 +26,8 @@ const (
maxBodyShift = 20 maxBodyShift = 20
// recentEventLimit is the number of recent events to show. // recentEventLimit is the number of recent events to show.
recentEventLimit = 20 recentEventLimit = 20
// defaultRetentionDays is the default event retention period.
defaultRetentionDays = 30
// paginationPerPage is the number of items per page. // paginationPerPage is the number of items per page.
paginationPerPage = 25 paginationPerPage = 25

View File

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

View File

@@ -25,37 +25,6 @@ type WebhookListItem struct {
// errMissingURL signals that a required URL was not provided. // errMissingURL signals that a required URL was not provided.
var errMissingURL = errors.New("missing URL") var errMissingURL = errors.New("missing URL")
// errInvalidRetention signals a retention_days form value that is not
// a non-negative whole number.
var errInvalidRetention = errors.New("invalid retention days")
// retentionErrorMessage is what the create and edit forms show the user
// when parseRetentionDays returns errInvalidRetention.
const retentionErrorMessage = "Retention must be a whole number of " +
"days, or 0 to retain events forever."
// parseRetentionDays interprets a retention_days form value.
//
// An empty value yields fallback, which lets the create path apply the
// default and the edit path leave the stored value unchanged. A value
// of 0 is returned as 0 and is rewritten to the retain-forever
// sentinel by database.Webhook's BeforeSave hook, so no handler needs
// to know the sentinel. Anything unparseable or negative is an error
// rather than a silently substituted default.
func parseRetentionDays(raw string, fallback int) (int, error) {
raw = strings.TrimSpace(raw)
if raw == "" {
return fallback, nil
}
v, err := strconv.Atoi(raw)
if err != nil || v < 0 {
return 0, errInvalidRetention
}
return v, nil
}
// EventWithDeliveries holds an event and its deliveries. // EventWithDeliveries holds an event and its deliveries.
type EventWithDeliveries struct { type EventWithDeliveries struct {
database.Event database.Event
@@ -137,20 +106,11 @@ func (h *Handlers) buildWebhookListItems(
// HandleSourceCreate shows the form to create a new webhook. // HandleSourceCreate shows the form to create a new webhook.
func (h *Handlers) HandleSourceCreate() http.HandlerFunc { func (h *Handlers) HandleSourceCreate() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
h.renderTemplate( data := map[string]any{
w, r, "sources_new.html", newSourceFormData(""), tmplKeyError: "",
) }
}
}
// newSourceFormData builds the template data for the webhook creation h.renderTemplate(w, r, "sources_new.html", data)
// form, carrying the retention default so the pre-filled value comes
// from database.DefaultRetentionDays rather than being a third
// hardcoded copy of the same policy.
func newSourceFormData(errMsg string) map[string]any {
return map[string]any{
tmplKeyError: errMsg,
"DefaultRetentionDays": database.DefaultRetentionDays,
} }
} }
@@ -167,10 +127,8 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
return return
} }
r.Body = http.MaxBytesReader( // The body size cap is enforced by the MaxBodySize
w, r.Body, 1<<maxBodyShift, // middleware, which runs before CSRF parses the form.
)
err := r.ParseForm() err := r.ParseForm()
if err != nil { if err != nil {
http.Error( http.Error(
@@ -185,26 +143,23 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
retentionStr := r.FormValue("retention_days") retentionStr := r.FormValue("retention_days")
if name == "" { if name == "" {
data := map[string]any{
tmplKeyError: "Name is required",
}
w.WriteHeader(http.StatusBadRequest) w.WriteHeader(http.StatusBadRequest)
h.renderTemplate( h.renderTemplate(w, r, "sources_new.html", data)
w, r, "sources_new.html",
newSourceFormData("Name is required"),
)
return return
} }
retentionDays, retErr := parseRetentionDays( retentionDays := defaultRetentionDays
retentionStr, database.DefaultRetentionDays,
)
if retErr != nil {
w.WriteHeader(http.StatusBadRequest)
h.renderTemplate(
w, r, "sources_new.html",
newSourceFormData(retentionErrorMessage),
)
return if retentionStr != "" {
v, convErr := strconv.Atoi(retentionStr)
if convErr == nil && v > 0 {
retentionDays = v
}
} }
h.createWebhookWithEntrypoint( h.createWebhookWithEntrypoint(
@@ -429,10 +384,8 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
return return
} }
r.Body = http.MaxBytesReader( // The body size cap is enforced by the MaxBodySize
w, r.Body, 1<<maxBodyShift, // middleware, which runs before CSRF parses the form.
)
err = r.ParseForm() err = r.ParseForm()
if err != nil { if err != nil {
http.Error( http.Error(
@@ -452,10 +405,8 @@ func (h *Handlers) applyWebhookEdit(
r *http.Request, r *http.Request,
webhook *database.Webhook, webhook *database.Webhook,
) { ) {
r.Body = http.MaxBytesReader( // The body size cap is enforced by the MaxBodySize middleware,
w, r.Body, 1<<maxBodyShift, // which runs before CSRF parses the form.
)
name := r.FormValue("name") name := r.FormValue("name")
if name == "" { if name == "" {
data := map[string]any{ data := map[string]any{
@@ -471,25 +422,7 @@ func (h *Handlers) applyWebhookEdit(
webhook.Name = name webhook.Name = name
webhook.Description = r.FormValue("description") webhook.Description = r.FormValue("description")
h.parseRetention(r, webhook)
// An empty field falls back to the stored value, so submitting the
// form without touching retention leaves the policy alone.
retentionDays, retErr := parseRetentionDays(
r.FormValue("retention_days"), webhook.RetentionDays,
)
if retErr != nil {
data := map[string]any{
tmplKeyWebhook: *webhook,
tmplKeyError: retentionErrorMessage,
}
w.WriteHeader(http.StatusBadRequest)
h.renderTemplate(w, r, "source_edit.html", data)
return
}
webhook.RetentionDays = retentionDays
err := h.db.DB().Save(webhook).Error err := h.db.DB().Save(webhook).Error
if err != nil { if err != nil {
@@ -503,6 +436,23 @@ func (h *Handlers) applyWebhookEdit(
) )
} }
// parseRetention parses and applies retention_days from the
// form.
func (h *Handlers) parseRetention(
r *http.Request,
webhook *database.Webhook,
) {
retStr := r.FormValue("retention_days")
if retStr == "" {
return
}
v, err := strconv.Atoi(retStr)
if err == nil && v > 0 {
webhook.RetentionDays = v
}
}
// HandleSourceDelete handles webhook deletion. // HandleSourceDelete handles webhook deletion.
func (h *Handlers) HandleSourceDelete() http.HandlerFunc { func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
@@ -769,10 +719,8 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
return return
} }
r.Body = http.MaxBytesReader( // The body size cap is enforced by the MaxBodySize
w, r.Body, 1<<maxBodyShift, // middleware, which runs before CSRF parses the form.
)
err = r.ParseForm() err = r.ParseForm()
if err != nil { if err != nil {
http.Error( http.Error(
@@ -829,10 +777,8 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
return return
} }
r.Body = http.MaxBytesReader( // The body size cap is enforced by the MaxBodySize
w, r.Body, 1<<maxBodyShift, // middleware, which runs before CSRF parses the form.
)
err = r.ParseForm() err = r.ParseForm()
if err != nil { if err != nil {
http.Error( http.Error(
@@ -852,10 +798,8 @@ func (h *Handlers) processTargetCreate(
r *http.Request, r *http.Request,
webhook database.Webhook, webhook database.Webhook,
) { ) {
r.Body = http.MaxBytesReader( // The body size cap is enforced by the MaxBodySize middleware,
w, r.Body, 1<<maxBodyShift, // which runs before CSRF parses the form.
)
name := r.FormValue("name") name := r.FormValue("name")
targetType := database.TargetType(r.FormValue("type")) targetType := database.TargetType(r.FormValue("type"))
targetURL := r.FormValue("url") targetURL := r.FormValue("url")

View File

@@ -1,481 +0,0 @@
package handlers_test
import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"strconv"
"strings"
"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"
)
const (
// sourceTestUserID is the session user id used by the webhook
// management tests.
sourceTestUserID = "source-test-user"
// sourceIDParam is the chi URL parameter naming a webhook.
sourceIDParam = "sourceID"
)
// formRequest builds an urlencoded POST to path carrying the given
// cookies, plus any chi URL parameters the handler reads.
func formRequest(
path string,
cookies []*http.Cookie,
form url.Values,
urlParams map[string]string,
) *http.Request {
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)
}
rctx := chi.NewRouteContext()
for k, v := range urlParams {
rctx.URLParams.Add(k, v)
}
return req.WithContext(
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
)
}
// getRequest builds a GET to path carrying the given cookies, plus any
// chi URL parameters the handler reads.
func getRequest(
t *testing.T,
path string,
cookies []*http.Cookie,
urlParams map[string]string,
) *http.Request {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, path, nil,
)
for _, c := range cookies {
req.AddCookie(c)
}
rctx := chi.NewRouteContext()
for k, v := range urlParams {
rctx.URLParams.Add(k, v)
}
return req.WithContext(
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
)
}
// submitCreate posts the webhook creation form with the given
// retention_days value (omitted entirely when retention is nil) and
// returns the recorder.
func submitCreate(
t *testing.T,
h *handlers.Handlers,
cookies []*http.Cookie,
name string,
retention *string,
) *httptest.ResponseRecorder {
t.Helper()
form := url.Values{}
form.Set("name", name)
if retention != nil {
form.Set("retention_days", *retention)
}
req := formRequest("/sources/new", cookies, form, nil)
w := httptest.NewRecorder()
h.HandleSourceCreateSubmit().ServeHTTP(w, req)
return w
}
// onlyWebhook loads the single webhook belonging to the test user.
func onlyWebhook(
t *testing.T,
db *database.Database,
) database.Webhook {
t.Helper()
var webhooks []database.Webhook
require.NoError(
t,
db.DB().Where("user_id = ?", sourceTestUserID).
Find(&webhooks).Error,
)
require.Len(t, webhooks, 1)
return webhooks[0]
}
// seedWebhook inserts a webhook owned by the test user with an exact
// stored retention value, bypassing Webhook.BeforeSave via a
// column-level update so that legacy rows can be planted too.
func seedWebhook(
t *testing.T,
db *database.Database,
retentionDays int,
) database.Webhook {
t.Helper()
wh := &database.Webhook{
UserID: sourceTestUserID,
Name: "seeded",
RetentionDays: retentionDays,
}
require.NoError(
t,
db.DB().Omit(clause.Associations).Create(wh).Error,
)
require.NoError(
t,
db.DB().Model(wh).
Update("retention_days", retentionDays).Error,
)
wh.RetentionDays = retentionDays
return *wh
}
// storedRetentionDays reads the retention_days column for a webhook.
func storedRetentionDays(
t *testing.T,
db *database.Database,
id string,
) int {
t.Helper()
var got int
require.NoError(
t,
db.DB().Model(&database.Webhook{}).
Where("id = ?", id).
Pluck("retention_days", &got).Error,
)
return got
}
// sourceTestEnv bundles the handler, session, and database a webhook
// management test drives.
type sourceTestEnv struct {
handlers *handlers.Handlers
db *database.Database
cookies []*http.Cookie
}
func setupSourceTest(t *testing.T) *sourceTestEnv {
t.Helper()
var h *handlers.Handlers
var sess *session.Session
var db *database.Database
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
return &sourceTestEnv{
handlers: h,
db: db,
cookies: authenticatedCookies(
t, sess, sourceTestUserID, "sourceuser",
),
}
}
// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
// regression test for the bug: the create form's 0 must reach the
// database as the retain-forever sentinel rather than being replaced by
// the column's default of 30.
func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
zero := "0"
w := submitCreate(t, env.handlers, env.cookies, "forever", &zero)
require.Equal(t, http.StatusSeeOther, w.Code)
wh := onlyWebhook(t, env.db)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetentionDays(t, env.db, wh.ID),
)
assert.True(t, wh.RetainsForever())
}
func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
require.Equal(t, http.StatusSeeOther, w.Code)
wh := onlyWebhook(t, env.db)
assert.Equal(
t,
database.DefaultRetentionDays,
storedRetentionDays(t, env.db, wh.ID),
)
}
// TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
// form's pre-filled retention from becoming a third hardcoded copy of
// the 30-day policy.
func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
w := httptest.NewRecorder()
env.handlers.HandleSourceCreate().ServeHTTP(
w, getRequest(t, "/sources/new", env.cookies, nil),
)
require.Equal(t, http.StatusOK, w.Code)
body := w.Body.String()
assert.Contains(
t, body,
`value="`+strconv.Itoa(database.DefaultRetentionDays)+`"`,
)
assert.NotContains(
t, body, `max="365"`,
"a max below the sentinel would block retain-forever",
)
assert.Contains(t, body, `min="0"`)
}
func TestHandleSourceCreateSubmit_InvalidRetentionIsRejected(
t *testing.T,
) {
t.Parallel()
for _, raw := range []string{"abc", "-1", "3.5"} {
t.Run(raw, func(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
w := submitCreate(
t, env.handlers, env.cookies, "bad", &raw,
)
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(
t, w.Body.String(), "Retention must be",
)
var count int64
require.NoError(
t,
env.db.DB().Model(&database.Webhook{}).
Where("user_id = ?", sourceTestUserID).
Count(&count).Error,
)
assert.Zero(
t, count,
"no webhook may be created from a rejected form",
)
})
}
}
// submitEdit posts the webhook edit form for the given webhook.
func submitEdit(
t *testing.T,
env *sourceTestEnv,
wh database.Webhook,
retention string,
) *httptest.ResponseRecorder {
t.Helper()
form := url.Values{}
form.Set("name", wh.Name)
form.Set("description", wh.Description)
form.Set("retention_days", retention)
req := formRequest(
"/source/"+wh.ID+"/edit",
env.cookies,
form,
map[string]string{sourceIDParam: wh.ID},
)
w := httptest.NewRecorder()
env.handlers.HandleSourceEditSubmit().ServeHTTP(w, req)
return w
}
func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
w := submitEdit(t, env, wh, "0")
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetentionDays(t, env.db, wh.ID),
)
}
func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
w := submitEdit(t, env, wh, "not-a-number")
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(t, w.Body.String(), "Retention must be")
assert.Equal(
t,
database.DefaultRetentionDays,
storedRetentionDays(t, env.db, wh.ID),
"a rejected form must not change the stored retention",
)
}
func TestHandleSourceEditSubmit_EmptyRetentionLeavesValueUnchanged(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, 7)
w := submitEdit(t, env, wh, "")
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(t, 7, storedRetentionDays(t, env.db, wh.ID))
}
// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
// the removed max="365" cap used to break: render the edit form for a
// retain-forever webhook, confirm the pre-filled sentinel is not capped
// by browser validation, then submit that pre-filled value straight
// back and confirm the retention policy survives untouched.
func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.RetentionForeverDays)
req := getRequest(
t, "/source/"+wh.ID+"/edit", env.cookies,
map[string]string{sourceIDParam: wh.ID},
)
w := httptest.NewRecorder()
env.handlers.HandleSourceEdit().ServeHTTP(w, req)
require.Equal(t, http.StatusOK, w.Code)
sentinel := strconv.Itoa(database.RetentionForeverDays)
body := w.Body.String()
assert.Contains(
t, body, `value="`+sentinel+`"`,
"the edit form pre-fills the stored retention",
)
assert.NotContains(
t, body, `max="365"`,
"a max below the sentinel would block saving any edit",
)
assert.Contains(
t, body, "forever",
"the form explains what the sentinel means",
)
// Submit the pre-filled value back, exactly as a browser would.
post := submitEdit(t, env, wh, sentinel)
require.Equal(t, http.StatusSeeOther, post.Code)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetentionDays(t, env.db, wh.ID),
)
}
// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
// the retain-forever value is never rendered to the user as a raw day
// count on either read-only view.
func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.RetentionForeverDays)
sentinel := strconv.Itoa(database.RetentionForeverDays)
listW := httptest.NewRecorder()
env.handlers.HandleSourceList().ServeHTTP(
listW, getRequest(t, "/sources", env.cookies, nil),
)
require.Equal(t, http.StatusOK, listW.Code)
assert.Contains(t, listW.Body.String(), "Retention: forever")
assert.NotContains(t, listW.Body.String(), sentinel)
detailW := httptest.NewRecorder()
env.handlers.HandleSourceDetail().ServeHTTP(
detailW,
getRequest(
t, "/source/"+wh.ID, env.cookies,
map[string]string{sourceIDParam: wh.ID},
),
)
require.Equal(t, http.StatusOK, detailW.Code)
assert.Contains(t, detailW.Body.String(), "Retention: forever")
assert.NotContains(t, detailW.Body.String(), sentinel)
}

View File

@@ -285,10 +285,36 @@ func (s *Middleware) NoCache() func(http.Handler) http.Handler {
} }
} }
// MaxBodySize returns middleware that limits the request body size // bodyLimitedMethod reports whether the request method carries a
// for POST requests. If the body exceeds the given limit in // body that the MaxBodySize middleware should cap.
// bytes, the server returns 413 Request Entity Too Large. This func bodyLimitedMethod(method string) bool {
// prevents clients from sending arbitrarily large form bodies. 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( func (s *Middleware) MaxBodySize(
maxBytes int64, maxBytes int64,
) func(http.Handler) http.Handler { ) func(http.Handler) http.Handler {
@@ -297,14 +323,31 @@ func (s *Middleware) MaxBodySize(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
) { ) {
if r.Method == http.MethodPost || if !bodyLimitedMethod(r.Method) {
r.Method == http.MethodPut || next.ServeHTTP(w, r)
r.Method == http.MethodPatch {
r.Body = http.MaxBytesReader( return
w, r.Body, maxBytes,
)
} }
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) next.ServeHTTP(w, r)
}) })
} }

View File

@@ -3,10 +3,12 @@ package middleware_test
import ( import (
"context" "context"
"encoding/base64" "encoding/base64"
"io"
"log/slog" "log/slog"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"os" "os"
"strings"
"testing" "testing"
"github.com/gorilla/sessions" "github.com/gorilla/sessions"
@@ -426,6 +428,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 --- // --- Helper Tests ---
func TestIpFromHostPort(t *testing.T) { 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() { func (s *Server) setupPageRoutes() {
s.router.Route("/pages", func(r chi.Router) { 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.CSRF())
r.Use(s.mw.NoCache()) r.Use(s.mw.NoCache())
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Group(func(r chi.Router) { r.Group(func(r chi.Router) {
r.Use(s.mw.LoginRateLimit()) r.Use(s.mw.LoginRateLimit())
@@ -106,6 +108,9 @@ func (s *Server) setupPageRoutes() {
func (s *Server) setupUserRoutes() { func (s *Server) setupUserRoutes() {
s.router.Route("/user/{username}", func(r chi.Router) { 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.CSRF())
r.Use(s.mw.NoCache()) r.Use(s.mw.NoCache())
r.Use(s.mw.RequireAuth()) r.Use(s.mw.RequireAuth())
@@ -118,20 +123,24 @@ func (s *Server) setupUserRoutes() {
func (s *Server) setupSourceRoutes() { func (s *Server) setupSourceRoutes() {
s.router.Route("/sources", func(r chi.Router) { 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.CSRF())
r.Use(s.mw.NoCache()) r.Use(s.mw.NoCache())
r.Use(s.mw.RequireAuth()) r.Use(s.mw.RequireAuth())
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Get("/", s.h.HandleSourceList()) r.Get("/", s.h.HandleSourceList())
r.Get("/new", s.h.HandleSourceCreate()) r.Get("/new", s.h.HandleSourceCreate())
r.Post("/new", s.h.HandleSourceCreateSubmit()) r.Post("/new", s.h.HandleSourceCreateSubmit())
}) })
s.router.Route("/source/{sourceID}", func(r chi.Router) { 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.CSRF())
r.Use(s.mw.NoCache()) r.Use(s.mw.NoCache())
r.Use(s.mw.RequireAuth()) r.Use(s.mw.RequireAuth())
r.Use(s.mw.MaxBodySize(maxFormBodySize))
r.Get("/", s.h.HandleSourceDetail()) r.Get("/", s.h.HandleSourceDetail())
r.Get("/edit", s.h.HandleSourceEdit()) r.Get("/edit", s.h.HandleSourceEdit())
r.Post("/edit", s.h.HandleSourceEditSubmit()) r.Post("/edit", s.h.HandleSourceEditSubmit())

View File

@@ -0,0 +1,375 @@
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) {}
// 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{} },
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

@@ -181,7 +181,7 @@
<!-- Info --> <!-- Info -->
<div class="mt-4 text-sm text-gray-400"> <div class="mt-4 text-sm text-gray-400">
<p>Retention: {{.Webhook.RetentionLabel}} &middot; Created: {{.Webhook.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</p> <p>Retention: {{.Webhook.RetentionDays}} days &middot; Created: {{.Webhook.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</p>
</div> </div>
</div> </div>
{{end}} {{end}}

View File

@@ -28,8 +28,7 @@
<div class="form-group"> <div class="form-group">
<label for="retention_days" class="label">Retention (days)</label> <label for="retention_days" class="label">Retention (days)</label>
<input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input"> <input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="1" max="365" class="input">
<p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}. Enter 0 to retain events forever.</p>
</div> </div>
<div class="flex gap-3"> <div class="flex gap-3">

View File

@@ -25,7 +25,7 @@
<p class="text-sm text-gray-500 mt-1">{{.Description}}</p> <p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
{{end}} {{end}}
</div> </div>
<span class="badge-info">Retention: {{.RetentionLabel}}</span> <span class="badge-info">{{.RetentionDays}}d retention</span>
</div> </div>
<div class="flex gap-6 mt-4 text-sm text-gray-500"> <div class="flex gap-6 mt-4 text-sm text-gray-500">
<span>{{.EntrypointCount}} entrypoint{{if ne .EntrypointCount 1}}s{{end}}</span> <span>{{.EntrypointCount}} entrypoint{{if ne .EntrypointCount 1}}s{{end}}</span>

View File

@@ -28,8 +28,8 @@
<div class="form-group"> <div class="form-group">
<label for="retention_days" class="label">Retention (days)</label> <label for="retention_days" class="label">Retention (days)</label>
<input type="number" id="retention_days" name="retention_days" value="{{.DefaultRetentionDays}}" min="0" class="input"> <input type="number" id="retention_days" name="retention_days" value="30" min="1" max="365" class="input">
<p class="text-xs text-gray-500 mt-1">How long to keep event data. Enter 0 to retain events forever.</p> <p class="text-xs text-gray-500 mt-1">How long to keep event data.</p>
</div> </div>
<div class="flex gap-3"> <div class="flex gap-3">