Allow retention_days of 0 to mean retain forever (closes #79)

RetentionDays carried gorm:"default:30", so GORM substituted 30 for a
zero value while building the insert. A webhook could therefore never
be configured to keep its events indefinitely: the reaper's
retain-forever branch existed but was unreachable from the normal
create and edit flows.

Introduce database.RetentionForeverDays = 365 * 1000 as the sentinel
for "retain forever" and a Webhook.BeforeSave hook that rewrites any
non-positive RetentionDays to it. The rewrite has to live in the hook
rather than at the call sites: GORM applies the column default while
converting the model to insert values, which happens after BeforeSave,
so anything later loses that race. Putting it on the model also means
a future call site, such as the planned REST API, cannot bypass it.

The reaper now skips a webhook when Webhook.RetainsForever reports
true, which recognises the sentinel and keeps honouring the old <= 0
values for rows written before it existed. Without this the sentinel,
being positive, would have produced a cutoff a thousand years in the
past and a DELETE matching nothing on every sweep.

Form handling is shared by create and edit through parseRetentionDays
so the two cannot drift: an empty field keeps the previous behaviour
(default on create, unchanged on edit), 0 is honoured, and an
unparseable or negative value is a 400 that re-renders the form rather
than a silently substituted default.

The retention inputs drop max="365". That cap was not cosmetic: the
edit form pre-fills the stored value, so a retain-forever webhook
rendered 365000 into an input capped at 365 and browser validation
would have blocked saving any edit to it. min becomes 0 with a hint
explaining what 0 does, and the list and detail views render a
RetentionLabel of "forever" instead of a raw day count.

The 30-day default is consolidated into database.DefaultRetentionDays,
referenced from the handler and from the create form's pre-filled
value, with a test asserting it agrees with the struct tag that cannot
reference it.

README.md

@@ -321,15 +321,6 @@ 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.
 
-A *finite* retention is capped at `database.MaxFiniteRetentionDays`
-(106751 days, about 292 years), and a larger one is rejected with a
-400. The cap is not arbitrary: the reaper computes its cutoff as a
-`time.Duration`, an int64 nanosecond count, and a longer period
-overflows it. An overflowed cutoff lands in the future, where it
-matches every row, so the sweep would delete every event the webhook
-has instead of none. The reaper also clamps the value it is given, so a
-row written by an older version cannot trigger that either.
-
 #### Entrypoint
 
 A receiver URL where external services POST webhook events. Each

TODO.md

@@ -33,12 +33,7 @@ databases currently grow without bound.
   `Webhook.BeforeSave` hook rewriting any non-positive `retention_days`
   to it ahead of GORM's own column defaulting, a reaper that skips such
   webhooks outright, form validation that honours `0` and rejects
-  garbage with a 400, and a retention UI that says "forever". Also
-  closes the overflow the same code path exposed: a finite
-  `retention_days` above `MaxFiniteRetentionDays` (106751, derived from
-  what an int64 `time.Duration` can hold) wrapped the reaper's cutoff
-  into the future and deleted every event, so it is now rejected at the
-  form and clamped in the reaper
+  garbage with a 400, and a retention UI that says "forever"
 - 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
   `Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
   adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so

internal/database/model_webhook.go

@@ -1,9 +1,7 @@
 package database
 
 import (
-	"math"
 	"strconv"
-	"time"
 
 	"gorm.io/gorm"
 )
@@ -24,36 +22,18 @@ const (
 	// time, so a zero can never survive a round trip to the database.
 	// Nothing outside this file may hardcode the number.
 	RetentionForeverDays = 365 * 1000
-
-	// MaxFiniteRetentionDays is the largest finite retention period the
-	// reaper's cutoff arithmetic can represent, and therefore the
-	// largest one a caller may request. It is derived from that
-	// arithmetic rather than picked: retentionCutoff computes
-	// retentionDays * hoursPerDay * time.Hour, and a time.Duration is
-	// an int64 nanosecond count, so math.MaxInt64 nanoseconds divided
-	// by an hour and then by a day is the exact ceiling — 106751 days,
-	// a little over 292 years.
-	//
-	// One day more overflows int64, wraps the product negative, and
-	// turns the cutoff into a timestamp in the far future that matches
-	// every row in the webhook's database. That is why this bound is
-	// enforced on input and why retentionCutoff saturates underneath
-	// it. Note that RetentionForeverDays deliberately sits above this
-	// ceiling: such webhooks are skipped before any cutoff is
-	// computed, and never reach the arithmetic at all.
-	MaxFiniteRetentionDays = int(
-		math.MaxInt64 / int64(time.Hour) / hoursPerDay,
-	)
 )
 
 // Webhook represents a webhook processing unit that groups entrypoints and targets
 //
-// Every method below takes a pointer receiver. BeforeSave has to,
-// because it mutates the record and GORM only invokes hooks declared
-// that way; the display helpers follow suit so the receiver kinds do
-// not mix. Handlers therefore put a *Webhook into template data:
-// html/template cannot call a pointer method on a value held in a map,
-// because a map element is not addressable.
+// 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 {
 	BaseModel
 
@@ -90,29 +70,18 @@ func (w *Webhook) BeforeSave(_ *gorm.DB) error {
 	return nil
 }
 
-// retainsForever reports whether a stored RetentionDays value means
-// "keep events indefinitely". It is the single definition of that
-// question, shared by Webhook.RetainsForever and by the reaper's
-// cutoff computation so the two cannot disagree about which webhooks
-// are exempt from reaping.
-//
-// 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 retainsForever(retentionDays int) bool {
-	return retentionDays <= 0 ||
-		retentionDays >= RetentionForeverDays
-}
-
 // RetainsForever reports whether this webhook's events are kept
-// indefinitely.
-func (w *Webhook) RetainsForever() bool {
-	return retainsForever(w.RetentionDays)
+// 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 {
+func (w Webhook) RetentionLabel() string {
 	if w.RetainsForever() {
 		return "forever"
 	}

internal/database/model_webhook_test.go

@@ -5,7 +5,6 @@ import (
 	"reflect"
 	"strconv"
 	"testing"
-	"time"
 
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
@@ -149,43 +148,6 @@ func TestWebhookRetentionColumnDefaultMatchesConstant(t *testing.T) {
 	)
 }
 
-// TestMaxFiniteRetentionDaysIsTheOverflowCeiling asserts that the
-// constant is exactly where the cutoff arithmetic stops working, which
-// is what makes it a derived bound rather than a round number someone
-// liked. One day more wraps the int64 nanosecond count negative, and a
-// negative span is precisely what turned a cutoff into a future
-// timestamp that matched — and deleted — every row.
-//
-// The multiplications are done through variables on purpose: as
-// constant expressions the overflowing one would not compile.
-func TestMaxFiniteRetentionDaysIsTheOverflowCeiling(t *testing.T) {
-	t.Parallel()
-
-	const hoursPerDay = 24
-
-	atCeiling := database.MaxFiniteRetentionDays
-	overCeiling := database.MaxFiniteRetentionDays + 1
-
-	assert.Positive(
-		t,
-		time.Duration(atCeiling*hoursPerDay)*time.Hour,
-		"the ceiling itself must still be representable",
-	)
-	assert.Negative(
-		t,
-		time.Duration(overCeiling*hoursPerDay)*time.Hour,
-		"one day past the ceiling must overflow",
-	)
-
-	assert.Less(
-		t,
-		database.MaxFiniteRetentionDays,
-		database.RetentionForeverDays,
-		"the sentinel sits above the ceiling and is only safe "+
-			"because retain-forever webhooks skip the arithmetic",
-	)
-}
-
 func TestWebhookRetainsForeverAndLabel(t *testing.T) {
 	t.Parallel()
 

internal/database/retention.go

@@ -177,10 +177,9 @@ func (r *RetentionReaper) reapWebhook(
 		return
 	}
 
-	cutoff, ok := retentionCutoff(time.Now(), retentionDays)
-	if !ok {
-		return
-	}
+	cutoff := time.Now().Add(
+		-time.Duration(retentionDays*hoursPerDay) * time.Hour,
+	)
 
 	deleted, err := reapExpired(db, cutoff)
 	if err != nil {
@@ -203,37 +202,6 @@ func (r *RetentionReaper) reapWebhook(
 	}
 }
 
-// retentionCutoff returns the timestamp before which a webhook's
-// events have expired, and whether any cutoff applies at all. It
-// reports false for a retain-forever policy, so no DELETE is issued.
-//
-// The day count is clamped to MaxFiniteRetentionDays first. This is
-// defense in depth rather than decoration: a time.Duration is an int64
-// nanosecond count, so an unclamped multiplication overflows above
-// that ceiling and wraps the span negative. Subtracting a negative
-// span moves the cutoff into the far future, where it matches every
-// row in the database: the sweep then deletes every event, delivery,
-// and delivery result, including ones created seconds ago. Rejecting
-// out-of-range input at the form is the primary guard; saturating here
-// means an old row, a migration, or a future call site cannot turn a
-// too-large retention into total data loss.
-func retentionCutoff(
-	now time.Time,
-	retentionDays int,
-) (time.Time, bool) {
-	if retainsForever(retentionDays) {
-		return time.Time{}, false
-	}
-
-	if retentionDays > MaxFiniteRetentionDays {
-		retentionDays = MaxFiniteRetentionDays
-	}
-
-	return now.Add(
-		-time.Duration(retentionDays*hoursPerDay) * time.Hour,
-	), true
-}
-
 // reapExpired hard-deletes, in foreign-key-safe order, the delivery
 // results, deliveries, and events associated with events older than
 // cutoff. Deletes are unscoped so rows are physically removed rather

internal/database/retention_test.go

@@ -327,58 +327,6 @@ func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
 	assertChainGone(t, finiteDB, doomed)
 }
 
-// TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents pins the
-// overflow that made a large finite retention destroy everything.
-//
-// The cutoff is a time.Duration, an int64 nanosecond count. A day
-// count above MaxFiniteRetentionDays multiplied out unclamped wraps
-// negative, so subtracting it moves the cutoff into the far future,
-// where "created_at < cutoff" matches every row: an event created a
-// moment ago, and its delivery and delivery result, were all deleted
-// on the first sweep. 200000 is inside that band and below the
-// retain-forever sentinel, so it is treated as a finite policy and
-// really does reach the arithmetic.
-//
-// The row is planted at the column level because such a value can no
-// longer be submitted through the form; the point of the test is that
-// a row from an older version, or a future call site, still cannot
-// trigger the wipe.
-func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupRetentionTest(t)
-
-	const overflowingRetentionDays = 200000
-
-	require.Greater(
-		t,
-		overflowingRetentionDays,
-		database.MaxFiniteRetentionDays,
-		"the test value must exceed what the cutoff can represent",
-	)
-	require.Less(
-		t,
-		overflowingRetentionDays,
-		database.RetentionForeverDays,
-		"the test value must not be rescued by the forever skip",
-	)
-
-	webhookID := createWebhook(
-		t, env.mainDB.DB(), overflowingRetentionDays,
-	)
-
-	db, err := env.mgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	fresh := seedEventChain(t, db, webhookID, time.Now())
-
-	env.reaper.ExportSweep(context.Background())
-
-	assertChainPresent(t, db, fresh)
-}
-
 func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
 	t.Parallel()
 

internal/handlers/source_management.go

@@ -29,47 +29,19 @@ var errMissingURL = errors.New("missing URL")
 // a non-negative whole number.
 var errInvalidRetention = errors.New("invalid retention days")
 
-// errRetentionTooLarge signals a retention_days form value that is a
-// whole number but larger than the reaper's cutoff arithmetic can
-// represent. It is distinguished from errInvalidRetention so the form
-// can tell the user the actual ceiling instead of implying their input
-// was not a number.
-var errRetentionTooLarge = errors.New("retention days out of range")
-
-// retentionErrorMessage returns the message the create and edit forms
-// show the user for a rejected retention_days value. Any error other
-// than errRetentionTooLarge falls back to the generic wording, so an
-// unrecognised parse failure still produces a sensible 400 rather than
-// an empty alert.
-func retentionErrorMessage(err error) string {
-	if errors.Is(err, errRetentionTooLarge) {
-		return "Retention must be at most " +
-			strconv.Itoa(database.MaxFiniteRetentionDays) +
+// 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."
-	}
-
-	return "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. Anything unparseable
-// or negative is an error rather than a silently substituted default.
-//
-// The upper bound is not cosmetic. The reaper computes its cutoff as a
-// time.Duration, an int64 nanosecond count, so a day count above
-// database.MaxFiniteRetentionDays overflows, puts the cutoff in the
-// future, and deletes every event the webhook has. A finite value
-// above that ceiling is therefore a 400.
-//
-// A value at or above the retain-forever sentinel is not out of range:
-// it is what the edit form pre-fills for a retain-forever webhook, so
-// submitting the form back unchanged has to keep meaning "forever"
-// rather than being rejected.
+// 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 == "" {
@@ -81,14 +53,6 @@ func parseRetentionDays(raw string, fallback int) (int, error) {
 		return 0, errInvalidRetention
 	}
 
-	if v >= database.RetentionForeverDays {
-		return database.RetentionForeverDays, nil
-	}
-
-	if v > database.MaxFiniteRetentionDays {
-		return 0, errRetentionTooLarge
-	}
-
 	return v, nil
 }
 
@@ -174,28 +138,18 @@ func (h *Handlers) buildWebhookListItems(
 func (h *Handlers) HandleSourceCreate() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		h.renderTemplate(
-			w, r, "sources_new.html",
-			newSourceFormData("", "", ""),
+			w, r, "sources_new.html", newSourceFormData(""),
 		)
 	}
 }
 
 // newSourceFormData builds the template data for the webhook creation
-// form.
-//
-// It carries the retention default so the pre-filled value comes from
-// database.DefaultRetentionDays rather than being a third hardcoded
-// copy of the same policy, and it carries the submitted name and
-// description so that re-rendering the form after a validation failure
-// gives the user their input back instead of a blank form. The edit
-// form already behaves that way; create now matches it.
-func newSourceFormData(
-	errMsg, name, description string,
-) map[string]any {
+// 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,
-		"Name":                 name,
-		"Description":          description,
 		"DefaultRetentionDays": database.DefaultRetentionDays,
 	}
 }
@@ -234,9 +188,7 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
 			w.WriteHeader(http.StatusBadRequest)
 			h.renderTemplate(
 				w, r, "sources_new.html",
-				newSourceFormData(
-					"Name is required", name, description,
-				),
+				newSourceFormData("Name is required"),
 			)
 
 			return
@@ -249,10 +201,7 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
 			w.WriteHeader(http.StatusBadRequest)
 			h.renderTemplate(
 				w, r, "sources_new.html",
-				newSourceFormData(
-					retentionErrorMessage(retErr),
-					name, description,
-				),
+				newSourceFormData(retentionErrorMessage),
 			)
 
 			return
@@ -409,10 +358,8 @@ func (h *Handlers) renderSourceDetail(
 		scheme = fwdProto
 	}
 
-	// The template calls Webhook methods, which take pointer
-	// receivers; html/template cannot address a value stored in a map.
 	data := map[string]any{
-		tmplKeyWebhook: &webhook,
+		tmplKeyWebhook: webhook,
 		"Entrypoints":  entrypoints,
 		"Targets":      targets,
 		"Events":       events,
@@ -448,7 +395,7 @@ func (h *Handlers) HandleSourceEdit() http.HandlerFunc {
 		}
 
 		data := map[string]any{
-			tmplKeyWebhook: &webhook,
+			tmplKeyWebhook: webhook,
 			tmplKeyError:   "",
 		}
 
@@ -512,7 +459,7 @@ func (h *Handlers) applyWebhookEdit(
 	name := r.FormValue("name")
 	if name == "" {
 		data := map[string]any{
-			tmplKeyWebhook: webhook,
+			tmplKeyWebhook: *webhook,
 			tmplKeyError:   "Name is required",
 		}
 
@@ -532,8 +479,8 @@ func (h *Handlers) applyWebhookEdit(
 	)
 	if retErr != nil {
 		data := map[string]any{
-			tmplKeyWebhook: webhook,
-			tmplKeyError:   retentionErrorMessage(retErr),
+			tmplKeyWebhook: *webhook,
+			tmplKeyError:   retentionErrorMessage,
 		}
 
 		w.WriteHeader(http.StatusBadRequest)
@@ -687,7 +634,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 		}
 
 		data := map[string]any{
-			tmplKeyWebhook: &webhook,
+			tmplKeyWebhook: webhook,
 			"Events":       evts,
 			"Page":         page,
 			"TotalPages":   totalPages,

internal/handlers/source_management_test.go

@@ -319,102 +319,6 @@ func TestHandleSourceCreateSubmit_InvalidRetentionIsRejected(
 	}
 }
 
-// TestHandleSourceCreateSubmit_OverflowingRetentionIsRejected covers
-// the data-loss path directly: a finite retention above the largest one
-// the reaper's cutoff arithmetic can represent must never reach the
-// database, because the sweep would compute a future cutoff and delete
-// every event the webhook has.
-func TestHandleSourceCreateSubmit_OverflowingRetentionIsRejected(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	tooBig := strconv.Itoa(database.MaxFiniteRetentionDays + 1)
-
-	env := setupSourceTest(t)
-
-	w := submitCreate(t, env.handlers, env.cookies, "huge", &tooBig)
-
-	assert.Equal(t, http.StatusBadRequest, w.Code)
-	assert.Contains(
-		t, w.Body.String(),
-		strconv.Itoa(database.MaxFiniteRetentionDays),
-		"the form tells the user the actual ceiling",
-	)
-
-	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",
-	)
-}
-
-// TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever guards the
-// boundary between "too large to represent" and "retain forever": the
-// sentinel is above MaxFiniteRetentionDays, but it is the value the
-// edit form pre-fills, so it must be accepted rather than rejected as
-// out of range.
-func TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
-
-	w := submitCreate(t, env.handlers, env.cookies, "forever", &sentinel)
-	require.Equal(t, http.StatusSeeOther, w.Code)
-
-	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
-}
-
-// TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput checks that a
-// validation failure hands the user's typing back, matching what the
-// edit form already does. Losing a long description to a mistyped
-// retention value is the kind of thing that makes people give up on a
-// form.
-func TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	const (
-		name        = "kept-name"
-		description = "a description worth not losing"
-	)
-
-	form := url.Values{}
-	form.Set("name", name)
-	form.Set("description", description)
-	form.Set("retention_days", "nonsense")
-
-	req := formRequest("/sources/new", env.cookies, form, nil)
-	w := httptest.NewRecorder()
-
-	env.handlers.HandleSourceCreateSubmit().ServeHTTP(w, req)
-
-	require.Equal(t, http.StatusBadRequest, w.Code)
-
-	body := w.Body.String()
-
-	assert.Contains(t, body, `value="`+name+`"`)
-	assert.Contains(t, body, description)
-}
-
 // submitEdit posts the webhook edit form for the given webhook.
 func submitEdit(
 	t *testing.T,
@@ -525,13 +429,9 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 		t, body, `max="365"`,
 		"a max below the sentinel would block saving any edit",
 	)
-	// "Currently forever." is the rendered RetentionLabel, not the
-	// static hint below the input, which says "Enter 0 to retain events
-	// forever." A bare Contains of "forever" would pass for any
-	// webhook and would assert nothing about this one.
 	assert.Contains(
-		t, body, "Currently forever.",
-		"the form reports this webhook's policy as forever",
+		t, body, "forever",
+		"the form explains what the sentinel means",
 	)
 
 	// Submit the pre-filled value back, exactly as a browser would.

templates/sources_new.html

@@ -18,12 +18,12 @@
             <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
             <div class="form-group">
                 <label for="name" class="label">Name</label>
-                <input type="text" id="name" name="name" value="{{.Name}}" required autofocus placeholder="My Webhook" class="input">
+                <input type="text" id="name" name="name" required autofocus placeholder="My Webhook" class="input">
             </div>
 
             <div class="form-group">
                 <label for="description" class="label">Description</label>
-                <textarea id="description" name="description" rows="3" placeholder="Optional description" class="input">{{.Description}}</textarea>
+                <textarea id="description" name="description" rows="3" placeholder="Optional description" class="input"></textarea>
             </div>
 
             <div class="form-group">