Align session codec max-age with the 7-day cap (closes #108)

sessions.NewCookieStore gives its securecookie codecs a 30-day max
age, and assigning store.Options never touches Codecs. The store
therefore decoded a cookie up to 30 days old while the cookie
attribute and the server-side expiry check both said 7 days, so a
refactor of that check -- or a second decode path that skips
IsAuthenticated -- would silently reopen a 30-day window.

Build the store through store.MaxAge, which sets Options.MaxAge and
propagates to every codec. The store is now built by newStore, the
one place a store is constructed; Regenerate shares its cookie
attributes via cookieOptions.

Tests:

- Two codec tests decode a re-stamped cookie an hour inside and an
  hour outside the cap. They only exercise Session.Get, so they pin
  the codec: reverting the fix fails the rejection test whether or
  not the server-side expiry check is present.
- The lazy-refresh bound is pinned two-sidedly, so the documented
  "expires up to 10% early, never late" guarantee now fails the
  suite if idleRefreshDivisor drifts in either direction.

Also scopes the RequireAuth save error to a distinct saveErr
variable instead of reusing the outer err. It is a plain assignment
rather than an inline "if saveErr := ...; saveErr != nil", because
the repo's noinlineerr linter rejects that form. Behaviour is
unchanged; this is scoping hygiene, not a bug fix. Two README claims
are corrected as well: the idle timeout is disabled by any
non-positive value, not only 0, and deploying the two-clock expiry
logs existing sessions out once because they carry no timestamps.

#108

README.md

@@ -93,9 +93,8 @@ TTY detection, and security headers are always applied.
 | `METRICS_USERNAME`      | Basic auth username for `/metrics`  | `""`     |
 | `METRICS_PASSWORD`      | Basic auth password for `/metrics`  | `""`     |
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
-| `RETENTION_SWEEP_INTERVAL` | Retention reaper period (Go duration, must be positive) | `1h` |
 | `SESSION_IDLE_TIMEOUT`  | Idle session timeout (Go duration)  | `24h`    |
-| `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
+| `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint | `120` |
 | `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted | `""` (none) |
 
 #### Trusted proxies
@@ -151,9 +150,10 @@ one runs out first:
 - **Idle expiry** (`SESSION_IDLE_TIMEOUT`, default `24h`) is a sliding
   window. Every authenticated request pushes it forward, so a session
   in continuous use never hits it, while an abandoned one expires a day
-  after its last use. Set it to `0` to disable idle expiry entirely;
+  after its last use. Any non-positive value (`0`, or a negative
-  the absolute cap below still applies. A set-but-unparseable value
+  duration such as `-1s`) disables idle expiry entirely; the absolute
-  aborts startup rather than silently falling back to the default.
+  cap below still applies. A set-but-unparseable value aborts startup
+  rather than silently falling back to the default.
 - **Absolute expiry** is a fixed 7 days from login. Activity does
   **not** extend it: after a week, every session ends and the user
   authenticates again.
@@ -165,6 +165,11 @@ idle window rather than on every request, which means a session may
 expire up to 10% early relative to the user's true last request, but
 never late.
 
+Both clocks are anchored by timestamps stored in the session cookie.
+Sessions issued before this feature existed carry neither, so they are
+treated as expired: upgrading to a build that has it logs every
+existing session out once, and those users sign in again.
+
 #### Invalid values abort startup
 
 The defaults above apply **only** to variables that are unset (or set
@@ -174,12 +179,8 @@ its value and refuses to start, rather than silently running with a
 substituted default. `PORT=eighty`, `DEBUG=ture`, and
 `RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
 additionally be a number in the range 1–65535,
-`RECEIVER_RATE_LIMIT` must be at least 1,
+`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
-`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
+`TRUSTED_PROXIES` must be a CIDR block or a bare IP address.
-period, so `0s` or a negative value would crash the reaper after
-startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or
-a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a
-non-positive value there means idle expiry is disabled, not invalid.
 
 Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
 spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
@@ -856,26 +857,6 @@ legitimate webhook senders). Requests over the limit receive HTTP 429
 with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
 value aborts startup rather than silently falling back to the default.
 
-A second limit sits in front of that one, keyed on the client IP alone
-and covering the whole route at ten times `RECEIVER_RATE_LIMIT` requests
-per minute (default 1200). The per-entrypoint limit needs it: the route
-pattern matches any single path segment, so a client that invents a
-fresh path per request gets a fresh per-entrypoint bucket every time and
-would otherwise have no aggregate limit at all — while each of those
-requests still costs an entrypoint lookup before it 404s. The aggregate
-limit leaves room for one address to drive several entrypoints at their
-full rate, and it is not configurable separately.
-
-What that aggregate limit bounds is the database work an invented path
-costs, not the number of log lines it produces. The path is
-attacker-controlled, so nothing on this route writes it to the log
-above `DEBUG`: a path that names no entrypoint is recorded by the
-handler at `DEBUG`, and the aggregate limiter logs its rejections at
-`DEBUG` and without the path. Every request is still recorded once by
-the access log, at `INFO`, with its full URL, whether it was served or
-rejected — so a flood of invented paths still writes one `INFO` line
-per request.
-
 Every limiter here — receiver, login, and password change — identifies
 the client the same way, through one shared key function: the
 connection's own address, unless the peer is listed in
@@ -884,14 +865,7 @@ instead. See [Trusted proxies](#trusted-proxies). Deployed without that
 variable set, a client behind a reverse proxy shares one bucket with
 every other client behind the same proxy, which is the safe direction
 to be wrong in: set `TRUSTED_PROXIES` to the proxy's address to get
-per-client limits back. That shared bucket matters more for the
+per-client limits back.
-aggregate limit than for the per-entrypoint one: with `TRUSTED_PROXIES`
-unset behind the reverse proxy a production deployment is required to
-run behind, every request keys on the proxy, so the aggregate limit
-becomes a service-wide ceiling of 1200 requests per minute across all
-senders and all entrypoints, where the per-entrypoint limit's capacity
-still grows with the number of entrypoints. Any deployment with more
-than a handful of busy entrypoints must set `TRUSTED_PROXIES`.
 
 Finer-grained per-webhook rate limits (configured in the web UI and
 enforced in the webhook handler) can layer on top of this env-level

TODO.md

@@ -1,62 +1,31 @@
 # Workflow
 
-One issue per unit of work, one branch and one PR per issue:
+* branch (from `main`)
-
+* do the work in Next Step
-* ensure a tracked issue exists with a definition of done
+* move Next Step to the top of Completed Steps
-* branch from `next` (never from `main`)
+* move the top item of Future Steps into Next Step
-* do the work; open a PR based on `next` (never on `main`)
+* commit (`TODO.md` changes in the same commit as the work)
-* pass an independent review, then the manager squash-merges into `next`
+* merge to `main` if the branch is not protected, otherwise open a PR
-* push; nothing stays local-only
+* push
-
-`next` is the branch for the next milestone and must stay green and
-mergeable to `main` without notice. One `next` -> `main` PR accumulates
-the milestone; releases are cut from `main` separately.
-
-Issue branches do NOT touch this file — the manager maintains it on
-`next`. Every branch editing `TODO.md` conflicts with every other
-(#112).
 
 # Status
 
-pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
+pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
 with auth, CSRF/SSRF protections, login rate limiting, Slack target,
 event retention (#63), the database archiving target (#43), the admin
 password change flow (#65), policy compliance (#6), pinned lint tooling
-(#55), and fail-loud configuration parsing (#80).
+(#55), and fail-loud configuration parsing (#80). Note: TODO.md was
-
+deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
-`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
+content was folded into the README TODO section, which this draft
-is closed, and it is verified green by cache-defeated container runs
+reconstructs as of 2026-07-06.
-rather than by the CI badge, which can pass without executing anything
-(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
-(2026-03-01, #6); its content was folded into the README TODO section,
-which this draft reconstructs as of 2026-07-06.
 
 # Next Step
 
-Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
+Manual event redelivery from the web UI (replay is a core promised
-gate (#119) before the next cycle's work lands — a gate that can report
+capability in the README rationale).
-success without running is the one thing every other guarantee here
-rests on.
 
 # Completed Steps
 
-- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
-  cap: the reverse walk cuts entries with `strings.LastIndexByte`
-  instead of joining and splitting, so a 1 MB header allocates 16 bytes
-  rather than 1.6 MB per request on the unauthenticated receiver.
-  Semantics proven unchanged by differential testing against the
-  previous implementation (#133)
-- 2026-08-12 Cap the `X-Forwarded-For` hop walk at 64 entries, so an
-  attacker-supplied chain cannot burn unbounded CPU in the rate-limit
-  key function; running off the end falls back to the peer address
-  (#124)
-- 2026-08-12 Gate forwarded-header trust behind a `TRUSTED_PROXIES` CIDR
-  list: all three rate limiters key on the connection's own address
-  unless the direct peer is a configured proxy, in which case
-  `X-Forwarded-For` is walked right to left for the first non-proxy hop.
-  Default trusts nothing, and a set-but-unparseable value aborts
-  startup. Before this, any client could mint a fresh bucket or drain
-  another's by rotating a spoofed header (#88)
 - 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
   Profile settings placeholder removed, a progressive-enhancement copy
   button for the entrypoint URL, and retention form copy that states the

internal/config/config.go

@@ -92,7 +92,6 @@ type Config struct {
 	SentryDSN       string
 
 	// RetentionSweepInterval is how often the retention reaper runs.
-	// Always positive: it becomes a time.NewTicker period.
 	RetentionSweepInterval time.Duration
 
 	// SessionIdleTimeout is the sliding inactivity window after
@@ -236,34 +235,6 @@ func envDuration(
 	return d, nil
 }
 
-// envPositiveDuration returns the value of the named environment
-// variable parsed as a Go duration that must be greater than zero.
-// Returns defaultValue if not set. A set value that is unparseable or
-// non-positive is a hard error naming the key and the bad value.
-//
-// This is for durations that reach time.NewTicker, which panics on a
-// non-positive period, in a goroutine started after startup has
-// already reported success. It is deliberately not used for durations
-// where non-positive means "disabled" (SESSION_IDLE_TIMEOUT).
-func envPositiveDuration(
-	key string,
-	defaultValue time.Duration,
-) (time.Duration, error) {
-	d, err := envDuration(key, defaultValue)
-	if err != nil {
-		return 0, err
-	}
-
-	if d <= 0 {
-		return 0, fmt.Errorf(
-			"%w: %s must be greater than zero, got %s",
-			ErrNonPositiveValue, key, d,
-		)
-	}
-
-	return d, nil
-}
-
 // parseCIDR parses one trusted-proxy list entry, which may be a
 // CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated
 // as a single-host block).
@@ -375,7 +346,7 @@ func loadFromEnv() (*Config, error) {
 		return nil, err
 	}
 
-	retentionSweepInterval, err := envPositiveDuration(
+	retentionSweepInterval, err := envDuration(
 		"RETENTION_SWEEP_INTERVAL",
 		defaultRetentionSweepInterval,
 	)
@@ -383,8 +354,6 @@ func loadFromEnv() (*Config, error) {
 		return nil, err
 	}
 
-	// Non-positive is "disabled" here, not invalid, so this stays on
-	// envDuration.
 	sessionIdleTimeout, err := envDuration(
 		"SESSION_IDLE_TIMEOUT",
 		defaultSessionIdleTimeout,

internal/config/config_test.go

@@ -139,11 +139,7 @@ func TestRetentionSweepInterval(t *testing.T) {
 		set         bool
 		value       string
 		expectError bool
-		// sentinel, when set, must be wrapped by the startup
+		expected    time.Duration
-		// error; every error case must additionally name the
-		// variable in its message.
-		sentinel error
-		expected time.Duration
 	}{
 		{
 			name:     caseUnsetUsesDefault,
@@ -162,24 +158,6 @@ func TestRetentionSweepInterval(t *testing.T) {
 			value:       "not-a-duration",
 			expectError: true,
 		},
-		{
-			// A non-positive period panics the ticker in the
-			// reaper and archive-sweeper goroutines, long after
-			// startup has reported success, so it has to fail
-			// here instead.
-			name:        "zero fails startup",
-			set:         true,
-			value:       "0s",
-			expectError: true,
-			sentinel:    config.ErrNonPositiveValue,
-		},
-		{
-			name:        "negative fails startup",
-			set:         true,
-			value:       "-1h",
-			expectError: true,
-			sentinel:    config.ErrNonPositiveValue,
-		},
 	}
 
 	for _, tt := range tests {
@@ -197,9 +175,7 @@ func TestRetentionSweepInterval(t *testing.T) {
 			}
 
 			if tt.expectError {
-				expectStartupErrorFor(
+				expectStartupError(t)
-					t, "RETENTION_SWEEP_INTERVAL", tt.sentinel,
-				)
 			} else {
 				testRetentionSweepIntervalSuccess(t, tt.expected)
 			}
@@ -305,22 +281,6 @@ func TestSessionIdleTimeout(t *testing.T) {
 			value:       "not-a-duration",
 			expectError: true,
 		},
-		{
-			// Non-positive is "idle expiry disabled" for this
-			// variable, not a configuration error: unlike
-			// RETENTION_SWEEP_INTERVAL it never becomes a ticker
-			// period.
-			name:     "zero disables idle expiry",
-			set:      true,
-			value:    "0s",
-			expected: 0,
-		},
-		{
-			name:     "negative disables idle expiry",
-			set:      true,
-			value:    "-1h",
-			expected: -time.Hour,
-		},
 	}
 
 	for _, tt := range tests {

internal/handlers/webhook.go

@@ -39,6 +39,12 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
 			return
 		}
 
+		h.log.Info("webhook request received",
+			"entrypoint_uuid", entrypointUUID,
+			"method", r.Method,
+			"remote_addr", r.RemoteAddr,
+		)
+
 		entrypoint, ok := h.lookupEntrypoint(
 			w, r, entrypointUUID,
 		)
@@ -46,18 +52,6 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
 			return
 		}
 
-		// Logged only once the UUID is known to name a real
-		// entrypoint. The UUID comes straight out of the path on
-		// the one unauthenticated endpoint, so logging it before
-		// the lookup let a client write an INFO line per invented
-		// path; the request itself is already in the access log
-		// and a miss is already logged at DEBUG.
-		h.log.Info("webhook request received",
-			"entrypoint_uuid", entrypointUUID,
-			"method", r.Method,
-			"remote_addr", r.RemoteAddr,
-		)
-
 		if !entrypoint.Active {
 			http.Error(w, "Gone", http.StatusGone)
 

internal/middleware/export_test.go

@@ -25,11 +25,6 @@ func IPFromHostPort(hp string) string {
 	return ipFromHostPort(hp)
 }
 
-// ClientKeyForTest exposes clientKey for testing.
-func ClientKeyForTest(m *Middleware, r *http.Request) string {
-	return m.clientKey(r)
-}
-
 // IsClientTLS exposes isClientTLS for testing.
 func IsClientTLS(r *http.Request) bool {
 	return isClientTLS(r)
@@ -41,13 +36,3 @@ const LoginRateLimitConst = loginRateLimit
 // PasswordChangeRateLimitConst exposes the
 // passwordChangeRateLimit constant.
 const PasswordChangeRateLimitConst = passwordChangeRateLimit
-
-// ReceiverAggregateMultiplierConst exposes the
-// receiverAggregateMultiplier constant.
-const ReceiverAggregateMultiplierConst = receiverAggregateMultiplier
-
-// ReceiverAggregateLimitForTest exposes receiverAggregateLimit for
-// testing.
-func ReceiverAggregateLimitForTest(perEntrypoint int) int {
-	return receiverAggregateLimit(perEntrypoint)
-}

internal/middleware/middleware.go

@@ -214,11 +214,11 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
 			// handler runs, while the headers are still ours to
 			// write.
 			if s.session.Touch(sess) {
-				err = s.session.Save(r, w, sess)
+				saveErr := s.session.Save(r, w, sess)
-				if err != nil {
+				if saveErr != nil {
 					s.log.Error(
 						"auth middleware: failed to refresh session",
-						"error", err,
+						"error", saveErr,
 					)
 				}
 			}

internal/middleware/ratelimit.go

@@ -1,7 +1,6 @@
 package middleware
 
 import (
-	"math"
 	"net/http"
 	"net/netip"
 	"slices"
@@ -33,21 +32,6 @@ const (
 	// receiver rate limit. The configured limit is expressed in
 	// requests per minute.
 	receiverRateInterval = 1 * time.Minute
-
-	// receiverAggregateMultiplier scales the configured
-	// per-entrypoint receiver limit into the aggregate limit one
-	// client IP may spend across the whole /webhook/* route. Ten
-	// entrypoints' worth lets a single sender address drive several
-	// entrypoints at their full rate, while still capping what one
-	// address costs the unauthenticated receiver.
-	receiverAggregateMultiplier = 10
-
-	// maxForwardedHops bounds how many X-Forwarded-For entries the
-	// chain walk examines. Real chains are one to three hops, but a
-	// client can pad the header up to MaxHeaderBytes, so without a
-	// bound every request pays a walk proportional to whatever the
-	// client sent.
-	maxForwardedHops = 64
 )
 
 // normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
@@ -86,49 +70,26 @@ func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
 // a trusted proxy is the client. A hop that cannot be read as a bare
 // address ends the walk: past it the chain is not the shape assumed
 // here, so the caller falls back to the peer address.
-//
-// Only the last maxForwardedHops entries are examined. A longer chain
-// is padding, and running out of hops falls back to the peer address
-// the same way an unreadable hop does.
-//
-// The entries are cut off the right end of each header value in place
-// rather than split out of it: the receiver is unauthenticated and a
-// client can pad the header up to MaxHeaderBytes, so splitting would
-// allocate in proportion to the padding (about 8 MB for a 1 MB
-// header) before the cap could discard any of it. Multiple header
-// values are walked in reverse for the same reason, since joining
-// them copies the whole chain.
 func (m *Middleware) forwardedClientAddr(
 	r *http.Request,
 ) (netip.Addr, bool) {
-	seen := 0
+	hops := strings.Split(
+		strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
+	)
 
-	for _, value := range slices.Backward(
+	for _, hop := range slices.Backward(hops) {
-		r.Header.Values("X-Forwarded-For"),
+		hop = strings.TrimSpace(hop)
-	) {
+		if hop == "" {
-		for last := false; !last && seen < maxForwardedHops; seen++ {
+			continue
-			hop := value
+		}
 
-			comma := strings.LastIndexByte(value, ',')
+		addr, err := netip.ParseAddr(hop)
-			if comma < 0 {
+		if err != nil {
-				last = true
+			return netip.Addr{}, false
-			} else {
+		}
-				hop, value = value[comma+1:], value[:comma]
-			}
 
-			hop = strings.TrimSpace(hop)
+		if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
-			if hop == "" {
+			return addr, true
-				continue
-			}
-
-			addr, err := netip.ParseAddr(hop)
-			if err != nil {
-				return netip.Addr{}, false
-			}
-
-			if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
-				return addr, true
-			}
 		}
 	}
 
@@ -152,10 +113,8 @@ func (m *Middleware) clientKey(r *http.Request) string {
 	peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
 	if err != nil {
 		// Not an address we can reason about; key on the raw
-		// value, the most specific identity left. On a
+		// value rather than collapsing such peers into one
-		// Unix-socket listener every peer carries the same
+		// shared bucket.
-		// RemoteAddr and so shares one bucket, which is the
-		// fail-closed direction.
 		return r.RemoteAddr
 	}
 
@@ -183,31 +142,6 @@ func (m *Middleware) tooManyRequests(
 	}
 }
 
-// floodTooManyRequests returns the 429 handler for a limiter whose
-// rejections are themselves the flood: it logs at DEBUG and without
-// the path, then answers with responseMessage.
-//
-// The aggregate receiver limiter trips exactly when one address is
-// sending faster than the receiver wants to serve, so its rejection
-// log is one line per request of that flood. At WARN with "path" that
-// hands a client a way to write its own text into the operator's log,
-// at a level that trips alerting, once per request — the log-volume
-// problem this limiter exists to bound. DEBUG is off in production by
-// default, so a flood costs nothing here; the path is dropped so that
-// turning DEBUG on to diagnose one does not restore the problem.
-//
-// This limiter bounds the database work an invented path costs, not
-// the number of log lines it produces: the access log in
-// middleware.go still records every request, served or rejected.
-func (m *Middleware) floodTooManyRequests(
-	logMessage, responseMessage string,
-) http.HandlerFunc {
-	return func(w http.ResponseWriter, _ *http.Request) {
-		m.log.Debug(logMessage)
-		http.Error(w, responseMessage, http.StatusTooManyRequests)
-	}
-}
-
 // LoginRateLimit returns middleware that enforces per-IP rate
 // limiting on login attempts using go-chi/httprate. Only POST
 // requests are rate-limited; GET requests (rendering the login
@@ -276,26 +210,15 @@ func (m *Middleware) postRateLimit(
 	}
 }
 
-// ReceiverRateLimit returns middleware that rate-limits the public
+// ReceiverRateLimit returns middleware that rate-limits the
-// webhook receiver endpoint with two limits in series.
+// public webhook receiver endpoint per client IP per request
-//
+// path (the path contains the entrypoint UUID, so each sender
-// The inner limit is per client IP per request path: the path
+// is limited per entrypoint without affecting other senders or
-// contains the entrypoint UUID, so each sender is limited per
+// other entrypoints). The limit is Config.ReceiverRateLimit
-// entrypoint without affecting other senders or other entrypoints.
+// requests per minute. Requests over the limit receive a 429.
-// It is Config.ReceiverRateLimit requests per minute.
+// Clients are identified by rateLimitKey.
-//
-// That limit alone bounds nothing in aggregate. The route pattern
-// /webhook/{uuid} matches any single segment, so a client that
-// invents a fresh path per request mints a fresh bucket per request
-// and never refills one — and every such request still reaches the
-// handler's entrypoint lookup before it 404s. The outer limit is
-// therefore keyed on the client IP alone, capping what one address
-// can spend across the whole route however it varies the path.
-//
-// Requests over either limit receive a 429. Clients are identified
-// by rateLimitKey.
 func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
-	perEntrypoint := httprate.Limit(
+	return httprate.Limit(
 		m.params.Config.ReceiverRateLimit,
 		receiverRateInterval,
 		httprate.WithKeyFuncs(
@@ -307,31 +230,4 @@ func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
 			"Too many requests. Please slow down.",
 		)),
 	)
-
-	aggregate := httprate.Limit(
-		receiverAggregateLimit(m.params.Config.ReceiverRateLimit),
-		receiverRateInterval,
-		httprate.WithKeyFuncs(m.rateLimitKey),
-		httprate.WithLimitHandler(m.floodTooManyRequests(
-			"webhook receiver aggregate rate limit exceeded",
-			"Too many requests. Please slow down.",
-		)),
-	)
-
-	return func(next http.Handler) http.Handler {
-		return aggregate(perEntrypoint(next))
-	}
-}
-
-// receiverAggregateLimit is the per-IP aggregate limit derived from
-// the configured per-entrypoint limit. The operator sets the latter
-// and nothing bounds it from above, so the multiplication is
-// saturated rather than allowed to wrap into a negative limit that
-// would reject every request.
-func receiverAggregateLimit(perEntrypoint int) int {
-	if perEntrypoint > math.MaxInt/receiverAggregateMultiplier {
-		return math.MaxInt
-	}
-
-	return perEntrypoint * receiverAggregateMultiplier
 }

internal/middleware/ratelimit_test.go

@@ -4,15 +4,11 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
-	"math"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
 	"os"
-	"runtime"
-	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"sneak.berlin/go/webhooker/internal/config"
@@ -572,228 +568,6 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
 	)
 }
 
-// TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer covers the
-// hop-walk cap. A client behind the trusted proxy can pad
-// X-Forwarded-For with tens of thousands of trusted-looking hops,
-// which costs a walk proportional to the padding and, once the walk
-// runs off the left end of the chain, reaches the entry the client
-// put there. Capping the walk stops both: the key falls back to the
-// peer address, so rotating the head of the chain mints no bucket,
-// and the run does not scale with the chain length.
-func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	// 50k hops is roughly 0.9 MB, within the default
-	// MaxHeaderBytes.
-	const hops = 50000
-
-	padding := strings.Repeat(", 10.0.0.2", hops-1)
-
-	start := time.Now()
-
-	assertSharedBucket(
-		t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
-		func(i int) map[string]string {
-			return map[string]string{
-				headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
-			}
-		},
-		"a padded X-Forwarded-For chain must fall back to the "+
-			"peer address, not reach the client-controlled entry "+
-			"at the head of the chain",
-	)
-
-	assert.Less(
-		t, time.Since(start), 2*time.Second,
-		"the capped walk must not scale with the chain length",
-	)
-}
-
-// TestRateLimitKey_LongChainAllocationIsBounded is the allocation
-// half of the hop cap. Capping the walk still left every request
-// paying for the whole header the client sent, because the chain was
-// split before it was capped: about 8 MB of []string for the 1 MB a
-// default MaxHeaderBytes allows, on the unauthenticated receiver.
-//
-// Bytes are the measurement, not allocation count: strings.Split of a
-// 1 MB chain is a single allocation, so testing.AllocsPerRun scores
-// it as cheap. The test is deliberately sequential — it reads
-// process-wide counters, and Go runs this package's parallel tests
-// only after the sequential ones finish.
-//
-//nolint:paralleltest // reads process-wide allocation counters
-func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
-	// 100k hops of ", 10.0.0.2" is roughly 1 MB.
-	const (
-		hops            = 100000
-		iterations      = 50
-		maxBytesPerCall = 4096
-	)
-
-	m := rateLimitMiddleware(t, &config.Config{
-		TrustedProxies: trustedProxies("10.0.0.0/8"),
-	})
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodPost, loginPath, nil,
-	)
-	req.RemoteAddr = "10.0.0.1:44444"
-	req.Header.Set(
-		headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
-	)
-
-	var before, after runtime.MemStats
-
-	var key string
-
-	runtime.ReadMemStats(&before)
-
-	for range iterations {
-		key = middleware.ClientKeyForTest(m, req)
-	}
-
-	runtime.ReadMemStats(&after)
-
-	perCall := (after.TotalAlloc - before.TotalAlloc) / iterations
-
-	assert.Less(
-		t, perCall, uint64(maxBytesPerCall),
-		"a %d-byte X-Forwarded-For must not allocate in proportion "+
-			"to its length, but cost %d bytes per call",
-		len(req.Header.Get(headerXFF)), perCall,
-	)
-
-	assert.Equal(
-		t, "10.0.0.1", key,
-		"the padded chain must still fall back to the peer address",
-	)
-}
-
-// TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths is the
-// regression test for the per-path bucket key. The route pattern
-// matches any single segment, so a client that never reuses a path
-// never reuses a per-entrypoint bucket either, and its aggregate
-// rate against the receiver is whatever it likes — with every
-// request reaching an entrypoint lookup before it 404s. The IP-only
-// aggregate limiter is what bounds that, so this must fail if the
-// aggregate limiter is removed.
-func TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	const (
-		limit = 3
-		ip    = "6.6.6.6:1234"
-	)
-
-	aggregate := limit * middleware.ReceiverAggregateMultiplierConst
-
-	handler := receiverLimitedHandler(t, limit)
-
-	// Every request goes to a path this client has never used, so
-	// none of them shares a per-entrypoint bucket with another.
-	for i := range aggregate {
-		w := receiverPost(
-			handler, ip, fmt.Sprintf("/webhook/invented-%d", i),
-		)
-		assert.Equal(
-			t, http.StatusOK, w.Code,
-			"request %d to a distinct path should pass", i,
-		)
-	}
-
-	w := receiverPost(
-		handler, ip, fmt.Sprintf("/webhook/invented-%d", aggregate),
-	)
-	assert.Equal(
-		t, http.StatusTooManyRequests, w.Code,
-		"a client must not be able to raise its aggregate rate "+
-			"against /webhook/* by varying the path",
-	)
-
-	// The aggregate limit is still per client IP: exhausting one
-	// address must not throttle another.
-	w = receiverPost(handler, "6.6.6.7:1234", "/webhook/invented-0")
-	assert.Equal(
-		t, http.StatusOK, w.Code,
-		"a different client IP must not be affected",
-	)
-}
-
-// TestReceiverRateLimit_RejectedRequestsCountTowardAggregate pins the
-// order the two limiters are chained in. The aggregate limiter has to
-// be the outer one, so that it counts requests the per-entrypoint
-// limiter rejects: those requests still arrive, and the aggregate
-// limit exists to bound what one address can make the receiver do.
-//
-// One path is hammered past the per-entrypoint limit, which alone
-// would leave the aggregate budget almost untouched; then a path the
-// client has never used must be rejected, which only the aggregate
-// limiter can do. Swap the two limiters and that last request is
-// served, because the rejected ones never reached the aggregate
-// limiter to be counted.
-func TestReceiverRateLimit_RejectedRequestsCountTowardAggregate(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	const (
-		limit = 3
-		ip    = "6.6.6.8:1234"
-	)
-
-	aggregate := limit * middleware.ReceiverAggregateMultiplierConst
-
-	handler := receiverLimitedHandler(t, limit)
-
-	// Spend the whole aggregate budget on one path. Only the first
-	// limit requests are served; the rest are rejected by the
-	// per-entrypoint limiter but still count against the aggregate.
-	for i := range aggregate {
-		w := receiverPost(handler, ip, "/webhook/exhausted")
-
-		want := http.StatusTooManyRequests
-		if i < limit {
-			want = http.StatusOK
-		}
-
-		assert.Equal(
-			t, want, w.Code,
-			"request %d to the exhausted path", i,
-		)
-	}
-
-	w := receiverPost(handler, ip, "/webhook/never-used")
-	assert.Equal(
-		t, http.StatusTooManyRequests, w.Code,
-		"requests rejected per entrypoint must still count "+
-			"toward the aggregate limit, so the aggregate "+
-			"limiter has to run first",
-	)
-}
-
-// TestReceiverAggregateLimit_SaturatesOnOverflow covers the derived
-// aggregate limit for a configured per-entrypoint limit large enough
-// that multiplying it would wrap negative, which httprate would read
-// as a limit that rejects every request.
-func TestReceiverAggregateLimit_SaturatesOnOverflow(t *testing.T) {
-	t.Parallel()
-
-	assert.Equal(
-		t, 1200,
-		middleware.ReceiverAggregateLimitForTest(120),
-		"the default limit scales by the multiplier",
-	)
-	assert.Equal(
-		t, math.MaxInt,
-		middleware.ReceiverAggregateLimitForTest(math.MaxInt),
-		"an overflowing limit saturates instead of wrapping",
-	)
-}
-
 // TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
 // the receiver limiter uses the same gated key function as the
 // POST limiters.

internal/session/codec_test.go

@@ -0,0 +1,150 @@
+package session_test
+
+import (
+	"context"
+	"crypto/hmac"
+	"crypto/sha256"
+	"encoding/base64"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gorilla/sessions"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// The tests below exercise the securecookie codecs underneath the
+// store and nothing else: Session.Get only decodes, so no server-side
+// expiry check takes part in the result. They exist because
+// NewCookieStore gives its codecs a 30-day max age that assigning
+// store.Options does not override, which would let the codec accept a
+// cookie weeks past the cap the cookie attribute advertises.
+
+// issuedCookie returns a session cookie the store itself wrote.
+func issuedCookie(t *testing.T, s *session.Session) string {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/", nil)
+	w := httptest.NewRecorder()
+
+	sess, err := s.Get(req)
+	require.NoError(t, err)
+
+	sess.Values["probe"] = "value"
+	require.NoError(t, s.Save(req, w, sess))
+
+	cookies := w.Result().Cookies()
+	require.Len(t, cookies, 1)
+
+	return cookies[0].Value
+}
+
+// restamp rewrites the timestamp inside an encoded session cookie and
+// re-signs it, yielding the cookie the store would have written at
+// that instant. securecookie stamps the encoding time itself and
+// exposes no seam to move it, so its wire format is reproduced here:
+// the base64url payload is "date|value|mac", where mac is HMAC-SHA256
+// of "name|date|value" under the store's key.
+func restamp(
+	t *testing.T,
+	encoded string,
+	at time.Time,
+) string {
+	t.Helper()
+
+	raw, err := base64.URLEncoding.DecodeString(encoded)
+	require.NoError(t, err)
+
+	parts := strings.SplitN(string(raw), "|", 3)
+	require.Len(t, parts, 3)
+
+	stamped := fmt.Sprintf("%d|%s", at.Unix(), parts[1])
+
+	mac := hmac.New(sha256.New, testKey())
+	_, err = mac.Write([]byte(session.SessionName + "|" + stamped))
+	require.NoError(t, err)
+
+	payload := append([]byte(stamped+"|"), mac.Sum(nil)...)
+
+	return base64.URLEncoding.EncodeToString(payload)
+}
+
+// decodeCookie feeds value back through the store's decode path.
+func decodeCookie(
+	t *testing.T,
+	s *session.Session,
+	value string,
+) (*sessions.Session, error) {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/", nil)
+	req.AddCookie(&http.Cookie{
+		Name:     session.SessionName,
+		Value:    value,
+		Path:     "/",
+		HttpOnly: true,
+		Secure:   true,
+		SameSite: http.SameSiteLaxMode,
+	})
+
+	sess, err := s.Get(req)
+	require.NotNil(t, sess)
+
+	return sess, err
+}
+
+func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) {
+	t.Parallel()
+
+	s := testSession(t)
+
+	sess, err := decodeCookie(t, s, restamp(
+		t,
+		issuedCookie(t, s),
+		time.Now().Add(-(testAbsoluteMaxAge-time.Hour)),
+	))
+	require.NoError(t, err)
+	assert.False(
+		t, sess.IsNew,
+		"a cookie inside the cap must still decode",
+	)
+	assert.Equal(
+		t, "value", sess.Values["probe"],
+		"decoding must yield the values that were saved",
+	)
+}
+
+func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) {
+	t.Parallel()
+
+	s := testSession(t)
+
+	sess, err := decodeCookie(t, s, restamp(
+		t,
+		issuedCookie(t, s),
+		time.Now().Add(-(testAbsoluteMaxAge+time.Hour)),
+	))
+	require.Error(
+		t, err,
+		"the codec must refuse a cookie older than the cap",
+	)
+	assert.Contains(
+		t, err.Error(), "expired timestamp",
+		"rejection must come from the codec's age check",
+	)
+	assert.True(
+		t, sess.IsNew,
+		"a cookie past the cap must not populate a session",
+	)
+	assert.Nil(
+		t, sess.Values["probe"],
+		"a cookie past the cap must not yield its values",
+	)
+}

internal/session/export_test.go

@@ -0,0 +1,10 @@
+package session
+
+import "github.com/gorilla/sessions"
+
+// NewStore exposes the production cookie-store constructor so tests
+// exercise the store the application actually runs with, rather than a
+// lookalike assembled in the test.
+func NewStore(key []byte, secure bool) *sessions.CookieStore {
+	return newStore(key, secure)
+}

internal/session/session.go

@@ -100,6 +100,35 @@ type Session struct {
 	now func() time.Time
 }
 
+// cookieOptions returns the cookie attributes used for every session
+// cookie. MaxAge is deliberately left at its zero value: for a store
+// it is set through CookieStore.MaxAge (see newStore), and for a
+// single session it is copied from the store's options.
+func cookieOptions(secure bool) *sessions.Options {
+	return &sessions.Options{
+		Path:     "/",
+		HttpOnly: true,
+		Secure:   secure,
+		SameSite: http.SameSiteLaxMode,
+	}
+}
+
+// newStore builds the session cookie store.
+//
+// The absolute cap MUST be applied with store.MaxAge and not by
+// assigning store.Options.MaxAge. NewCookieStore gives the underlying
+// securecookie codecs a 30-day max age of their own, and assigning
+// Options never touches Codecs -- so a store configured that way still
+// decodes a 30-day-old cookie, leaving the cookie attribute and the
+// codec disagreeing about the same policy. store.MaxAge sets both.
+func newStore(key []byte, secure bool) *sessions.CookieStore {
+	store := sessions.NewCookieStore(key)
+	store.Options = cookieOptions(secure)
+	store.MaxAge(secondsPerDay * sessionMaxAgeDays)
+
+	return store
+}
+
 // New creates a new session manager. The cookie store is
 // initialized during the fx OnStart phase after the database is
 // connected, using a session key that is auto-generated and stored
@@ -142,19 +171,8 @@ func New(
 				)
 			}
 
-			store := sessions.NewCookieStore(keyBytes)
-
-			// Configure cookie options for security
-			store.Options = &sessions.Options{
-				Path:     "/",
-				MaxAge:   secondsPerDay * sessionMaxAgeDays,
-				HttpOnly: true,
-				Secure:   !params.Config.IsDev(),
-				SameSite: http.SameSiteLaxMode,
-			}
-
 			s.key = keyBytes
-			s.store = store
+			s.store = newStore(keyBytes, !params.Config.IsDev())
 			s.log.Info("session manager initialized")
 
 			return nil
@@ -350,13 +368,8 @@ func (s *Session) Regenerate(
 	// Apply the standard session options (the destroyed old
 	// session had MaxAge = -1, which store.New might inherit
 	// from the cookie).
-	newSess.Options = &sessions.Options{
+	newSess.Options = cookieOptions(!s.config.IsDev())
-		Path:     "/",
+	newSess.Options.MaxAge = secondsPerDay * sessionMaxAgeDays
-		MaxAge:   secondsPerDay * sessionMaxAgeDays,
-		HttpOnly: true,
-		Secure:   !s.config.IsDev(),
-		SameSite: http.SameSiteLaxMode,
-	}
 
 	return newSess, nil
 }

internal/session/session_test.go

@@ -39,6 +39,19 @@ func (c *fakeClock) Advance(d time.Duration) {
 	c.t = c.t.Add(d)
 }
 
+// testKey returns the fixed session key the tests sign with. The
+// codec tests re-sign cookies with it, so it must be the same key the
+// store was built from.
+func testKey() []byte {
+	key := make([]byte, testKeySize)
+
+	for i := range key {
+		key[i] = byte(i + 42)
+	}
+
+	return key
+}
+
 // testSession creates a Session with a real cookie store and the
 // real clock.
 func testSession(t *testing.T) *session.Session {
@@ -59,20 +72,8 @@ func testSessionWithClock(
 ) (*session.Session, *fakeClock) {
 	t.Helper()
 
-	key := make([]byte, testKeySize)
+	key := testKey()
-
+	store := session.NewStore(key, false)
-	for i := range key {
-		key[i] = byte(i + 42)
-	}
-
-	store := sessions.NewCookieStore(key)
-	store.Options = &sessions.Options{
-		Path:     "/",
-		MaxAge:   86400 * 7,
-		HttpOnly: true,
-		Secure:   false,
-		SameSite: http.SameSiteLaxMode,
-	}
 
 	cfg := &config.Config{
 		Environment:        config.EnvironmentDev,
@@ -645,6 +646,34 @@ func TestTouch_LazyBelowRefreshThreshold(t *testing.T) {
 	)
 }
 
+func TestTouch_RefreshThresholdIsOneTenthOfIdleWindow(t *testing.T) {
+	t.Parallel()
+
+	// testRefreshDivisor restates the documented bound independently
+	// of the implementation constant: the idle timestamp is rewritten
+	// once it is a tenth of the idle window old, which is what makes
+	// "expires up to 10% early, never late" true. Both assertions are
+	// needed to pin it -- a larger divisor fails the first, a smaller
+	// one fails the second.
+	const testRefreshDivisor = 10
+
+	threshold := testIdleTimeout / testRefreshDivisor
+
+	s, sess, clock := authenticatedSession(t, testIdleTimeout)
+
+	clock.Advance(threshold - time.Second)
+	assert.False(
+		t, s.Touch(sess),
+		"Touch must not rewrite the session below a tenth of the window",
+	)
+
+	clock.Advance(time.Second)
+	assert.True(
+		t, s.Touch(sess),
+		"Touch must rewrite the session at a tenth of the window",
+	)
+}
+
 func TestTouch_UnauthenticatedSessionIsNotRefreshed(t *testing.T) {
 	t.Parallel()