3 Commits

Author SHA1 Message Date
618b07ca0f Align session codec max-age with the 7-day cap (closes #108)
All checks were successful
check / check (push) Successful in 3m5s
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
2026-08-12 10:00:50 +00:00
d19e33671c Gate forwarded-header trust behind trusted-proxy config (closes #88)
All checks were successful
check / check (push) Successful in 6s
All three rate limiters (receiver, login, password change) now key on the connection's own address unless the direct peer is inside the new TRUSTED_PROXIES CIDR list, in which case X-Forwarded-For is walked right to left for the first non-proxy hop. Default is the empty list, which trusts nothing. A set-but-unparseable value aborts startup.
2026-08-12 11:36:10 +02:00
aab448b076 Clarify web UI terminology, copy, and the entrypoint URL (closes #57)
All checks were successful
check / check (push) Successful in 4s
Unifies user-visible copy on "Webhook" (routes and URLs unchanged), drops
the placeholder Profile settings section, and adds a copy-to-clipboard
affordance for the entrypoint URL as progressive enhancement — the button
stays hidden unless both the target element and the Clipboard API resolve,
so no dead control appears without JavaScript and the URL stays selectable.

Retention copy now matches what the code does: deletion is permanent, 0
retains forever, and a blank field means the default on create or the
current value on edit. The permanent-deletion sentence is suppressed for a
retain-forever webhook, which the reaper exempts before computing a cutoff.

Template tests gained a render-completed assertion. Without it, a page that
aborted mid-render still satisfied assertions matching the already-flushed
prefix, because renderTemplate streams to the ResponseWriter (#123).
2026-08-11 15:42:08 +02:00
19 changed files with 1381 additions and 122 deletions

View File

@@ -95,6 +95,54 @@ TTY detection, and security headers are always applied.
| `SENTRY_DSN` | Sentry error reporting DSN | `""` | | `SENTRY_DSN` | Sentry error reporting DSN | `""` |
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | | `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` | | `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
#### Trusted proxies
`TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
address such as `192.168.1.7` is accepted and treated as a single
host), for example `192.168.1.7, 2001:db8::5`. It decides whose
`X-Forwarded-For` header the rate limiters believe, so it should name
the addresses of your reverse proxies and nothing else.
`X-Forwarded-For` is honoured **only** when the connecting peer is
inside one of these blocks; for every other peer the client identity is
the connection's own address and the header is ignored. The default is
the empty list, which trusts nobody — anything else would let any
client pick its own rate limit bucket, minting a fresh one per request
or draining someone else's. Set it to the address of your reverse
proxy, and to nothing wider. A set but unparseable value aborts
startup.
`X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
Reverse proxies append to `X-Forwarded-For` but forward other client
headers verbatim, so a single-valued header is client-controlled even
behind a trusted proxy.
Within a trusted request, `X-Forwarded-For` is read right to left,
because the rightmost entry is the one the nearest proxy appended and
everything left of it may have been written by the client. The first
hop that is not itself a trusted proxy is taken as the client. A hop
that is not a bare IP address — `ip:port`, a bracketed IPv6 literal,
the token `unknown` — ends the walk and the peer address is used
instead, since past such an entry the chain is not the shape assumed
here. The peer address is likewise used when the header is absent or
every hop in it is a trusted proxy.
Two operator requirements follow:
- Your proxy must **append** the peer address to `X-Forwarded-For`
(nginx `$proxy_add_x_forwarded_for`, HAProxy `option forwardfor`,
Caddy and AWS ALB by default), and must append a bare address with
no port.
- List proxy hosts **only**. Any address inside `TRUSTED_PROXIES`
chooses its own rate-limit key: its `X-Forwarded-For` is walked, so
it can name a different address on every request to get a fresh
bucket each time, or name another client's address to drain that
client's bucket. Never list a block that also covers clients — a
broad `10.0.0.0/8` on a network where clients live in the same range
makes all three limits, including the unauthenticated webhook
receiver, silently bypassable by every client in the block.
Sessions are bounded by two independent clocks, and end at whichever Sessions are bounded by two independent clocks, and end at whichever
one runs out first: one runs out first:
@@ -102,9 +150,10 @@ one runs out first:
- **Idle expiry** (`SESSION_IDLE_TIMEOUT`, default `24h`) is a sliding - **Idle expiry** (`SESSION_IDLE_TIMEOUT`, default `24h`) is a sliding
window. Every authenticated request pushes it forward, so a session window. Every authenticated request pushes it forward, so a session
in continuous use never hits it, while an abandoned one expires a day 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 - **Absolute expiry** is a fixed 7 days from login. Activity does
**not** extend it: after a week, every session ends and the user **not** extend it: after a week, every session ends and the user
authenticates again. authenticates again.
@@ -116,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 expire up to 10% early relative to the user's true last request, but
never late. 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 #### Invalid values abort startup
The defaults above apply **only** to variables that are unset (or set The defaults above apply **only** to variables that are unset (or set
@@ -124,8 +178,9 @@ fatal configuration error: webhooker logs the offending variable and
its value and refuses to start, rather than silently running with a its value and refuses to start, rather than silently running with a
substituted default. `PORT=eighty`, `DEBUG=ture`, and substituted default. `PORT=eighty`, `DEBUG=ture`, and
`RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must `RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
additionally be a number in the range 165535, and additionally be a number in the range 165535,
`RECEIVER_RATE_LIMIT` must be at least 1. `RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
`TRUSTED_PROXIES` must be a CIDR block or a bare IP address.
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`, spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
@@ -802,6 +857,16 @@ legitimate webhook senders). Requests over the limit receive HTTP 429
with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT` with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
value aborts startup rather than silently falling back to the default. value aborts startup rather than silently falling back to the default.
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
`TRUSTED_PROXIES`, in which case the forwarded client address is used
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.
Finer-grained per-webhook rate limits (configured in the web UI and 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 enforced in the webhook handler) can layer on top of this env-level
abuse limit later; they are tracked as future work. abuse limit later; they are tracked as future work.

View File

@@ -26,6 +26,10 @@ capability in the README rationale).
# Completed Steps # Completed Steps
- 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
actual policy (deletion by the reaper, 0 retains forever) (#57)
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry - 2026-08-09 Inactivity-based session timeout: sliding idle expiry
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
requests, with the 7-day absolute cap kept as an independent requests, with the 7-day absolute cap kept as an independent

View File

@@ -5,8 +5,10 @@ import (
"errors" "errors"
"fmt" "fmt"
"log/slog" "log/slog"
"net/netip"
"os" "os"
"strconv" "strconv"
"strings"
"time" "time"
"go.uber.org/fx" "go.uber.org/fx"
@@ -45,6 +47,11 @@ const (
// maxPort is the highest valid TCP port number. The lower // maxPort is the highest valid TCP port number. The lower
// bound (at least 1) is enforced by envPositiveInt. // bound (at least 1) is enforced by envPositiveInt.
maxPort = 65535 maxPort = 65535
// mappedV4Offset is the number of leading bits an IPv4-mapped
// IPv6 prefix spends on the ::ffff:0:0/96 wrapper, so a /104
// covers the same addresses as an IPv4 /8.
mappedV4Offset = 96
) )
// ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
@@ -59,6 +66,11 @@ var ErrNonPositiveValue = errors.New("value must be positive")
// TCP port number is set above the valid port range. // TCP port number is set above the valid port range.
var ErrInvalidPort = errors.New("invalid port") var ErrInvalidPort = errors.New("invalid port")
// ErrInvalidCIDR is returned when an environment variable holding a
// list of CIDR blocks contains an entry that is neither a CIDR block
// nor a bare IP address.
var ErrInvalidCIDR = errors.New("invalid CIDR")
//nolint:revive // ConfigParams is a standard fx naming convention. //nolint:revive // ConfigParams is a standard fx naming convention.
type ConfigParams struct { type ConfigParams struct {
fx.In fx.In
@@ -90,6 +102,17 @@ type Config struct {
// client IP may send to a single webhook receiver entrypoint. // client IP may send to a single webhook receiver entrypoint.
ReceiverRateLimit int ReceiverRateLimit int
// TrustedProxies is the set of networks whose members are
// allowed to speak for the client with X-Forwarded-For, the
// only forwarded header read. It is empty unless
// TRUSTED_PROXIES is set, and empty means no peer is
// trusted: forwarded headers are then ignored entirely and
// clients are identified by the connection's own address.
// Members can choose their own rate-limit key, so this must
// name proxy hosts only, never a block that also covers
// clients.
TrustedProxies []netip.Prefix
params *ConfigParams params *ConfigParams
log *slog.Logger log *slog.Logger
} }
@@ -212,6 +235,71 @@ func envDuration(
return d, nil 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).
//
// Both forms are unmapped, because peer addresses are unmapped
// before they are matched against the list: an IPv4-mapped prefix
// left in that form would silently never match.
func parseCIDR(entry string) (netip.Prefix, error) {
if strings.Contains(entry, "/") {
prefix, err := netip.ParsePrefix(entry)
if err != nil {
return netip.Prefix{}, err //nolint:wrapcheck // wrapped by caller
}
if addr := prefix.Addr(); addr.Is4In6() &&
prefix.Bits() >= mappedV4Offset {
prefix = netip.PrefixFrom(
addr.Unmap(), prefix.Bits()-mappedV4Offset,
)
}
return prefix.Masked(), nil
}
addr, err := netip.ParseAddr(entry)
if err != nil {
return netip.Prefix{}, err //nolint:wrapcheck // wrapped by caller
}
return netip.PrefixFrom(addr.Unmap(), addr.Unmap().BitLen()), nil
}
// envPrefixList returns the value of the named environment variable
// parsed as a comma-separated list of CIDR blocks (bare addresses
// allowed). An unset, empty, or blank value yields an empty list. A
// set value containing an unparseable entry is a hard error naming
// the key and the bad entry, so startup fails loudly rather than
// silently running with a list the operator did not intend.
func envPrefixList(key string) ([]netip.Prefix, error) {
v := strings.TrimSpace(os.Getenv(key))
if v == "" {
return nil, nil
}
var prefixes []netip.Prefix
for entry := range strings.SplitSeq(v, ",") {
entry = strings.TrimSpace(entry)
if entry == "" {
continue
}
prefix, err := parseCIDR(entry)
if err != nil {
return nil, fmt.Errorf(
"%w: %s: %q: %w", ErrInvalidCIDR, key, entry, err,
)
}
prefixes = append(prefixes, prefix)
}
return prefixes, nil
}
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to // resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
// dev, and rejects unrecognised values. // dev, and rejects unrecognised values.
func resolveEnvironment() (string, error) { func resolveEnvironment() (string, error) {
@@ -282,6 +370,11 @@ func loadFromEnv() (*Config, error) {
return nil, err return nil, err
} }
trustedProxies, err := envPrefixList("TRUSTED_PROXIES")
if err != nil {
return nil, err
}
return &Config{ return &Config{
DataDir: envString("DATA_DIR"), DataDir: envString("DATA_DIR"),
Debug: debug, Debug: debug,
@@ -294,6 +387,7 @@ func loadFromEnv() (*Config, error) {
RetentionSweepInterval: retentionSweepInterval, RetentionSweepInterval: retentionSweepInterval,
SessionIdleTimeout: sessionIdleTimeout, SessionIdleTimeout: sessionIdleTimeout,
ReceiverRateLimit: receiverRateLimit, ReceiverRateLimit: receiverRateLimit,
TrustedProxies: trustedProxies,
}, nil }, nil
} }
@@ -335,6 +429,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"dataDir", s.DataDir, "dataDir", s.DataDir,
"retentionSweepInterval", s.RetentionSweepInterval.String(), "retentionSweepInterval", s.RetentionSweepInterval.String(),
"receiverRateLimit", s.ReceiverRateLimit, "receiverRateLimit", s.ReceiverRateLimit,
"trustedProxies", len(s.TrustedProxies),
"hasSentryDSN", s.SentryDSN != "", "hasSentryDSN", s.SentryDSN != "",
"hasMetricsAuth", "hasMetricsAuth",
s.MetricsUsername != "" && s.MetricsPassword != "", s.MetricsUsername != "" && s.MetricsPassword != "",

View File

@@ -20,6 +20,10 @@ const (
caseUnsetUsesDefault = "unset uses default" caseUnsetUsesDefault = "unset uses default"
caseValidValueParsed = "valid value is parsed" caseValidValueParsed = "valid value is parsed"
caseUnparseableFails = "unparseable value fails startup" caseUnparseableFails = "unparseable value fails startup"
// cidrPrivateV4 is the sample trusted-proxy block the
// TRUSTED_PROXIES cases are built from.
cidrPrivateV4 = "10.0.0.0/8"
) )
func TestEnvironmentConfig(t *testing.T) { func TestEnvironmentConfig(t *testing.T) {
@@ -179,9 +183,10 @@ func TestRetentionSweepInterval(t *testing.T) {
} }
} }
// expectStartupError asserts that fx refuses to build the app, // startupError builds the app config.New belongs to and returns
// which is what a set-but-invalid environment value must cause. // the error fx reports, which is non-nil whenever an environment
func expectStartupError(t *testing.T) { // value is set but invalid.
func startupError(t *testing.T) error {
t.Helper() t.Helper()
var cfg *config.Config var cfg *config.Config
@@ -196,7 +201,33 @@ func expectStartupError(t *testing.T) {
fx.Populate(&cfg), fx.Populate(&cfg),
) )
assert.Error(t, app.Err()) return app.Err()
}
// expectStartupError asserts that fx refuses to build the app,
// which is what a set-but-invalid environment value must cause.
func expectStartupError(t *testing.T) {
t.Helper()
assert.Error(t, startupError(t))
}
// expectStartupErrorFor asserts that startup fails, that the error
// names the offending variable so an operator can find it, and,
// when sentinel is non-nil, that it wraps that sentinel.
func expectStartupErrorFor(
t *testing.T,
key string,
sentinel error,
) {
t.Helper()
err := startupError(t)
require.ErrorContains(t, err, key)
if sentinel != nil {
require.ErrorIs(t, err, sentinel)
}
} }
func testRetentionSweepIntervalSuccess( func testRetentionSweepIntervalSuccess(
@@ -351,6 +382,10 @@ func TestReceiverRateLimit(t *testing.T) {
set bool set bool
value string value string
expectError bool expectError bool
// sentinel, when set, must be wrapped by the startup
// error; every error case must additionally name the
// variable in its message.
sentinel error
expected int expected int
}{ }{
{ {
@@ -375,12 +410,14 @@ func TestReceiverRateLimit(t *testing.T) {
set: true, set: true,
value: "0", value: "0",
expectError: true, expectError: true,
sentinel: config.ErrNonPositiveValue,
}, },
{ {
name: "negative fails startup", name: "negative fails startup",
set: true, set: true,
value: "-5", value: "-5",
expectError: true, expectError: true,
sentinel: config.ErrNonPositiveValue,
}, },
} }
@@ -399,7 +436,9 @@ func TestReceiverRateLimit(t *testing.T) {
} }
if tt.expectError { if tt.expectError {
expectStartupError(t) expectStartupErrorFor(
t, "RECEIVER_RATE_LIMIT", tt.sentinel,
)
} else { } else {
testReceiverRateLimitSuccess(t, tt.expected) testReceiverRateLimitSuccess(t, tt.expected)
} }
@@ -432,3 +471,116 @@ func testReceiverRateLimitSuccess(
assert.Equal(t, expected, cfg.ReceiverRateLimit) assert.Equal(t, expected, cfg.ReceiverRateLimit)
} }
func TestTrustedProxies(t *testing.T) {
tests := []struct {
name string
set bool
value string
expectError bool
expected []string
}{
{
// The default must be "trust nobody": an empty list
// means forwarded headers are ignored, never that
// every peer may speak for the client.
name: caseUnsetUsesDefault,
set: false,
expected: []string{},
},
{
name: "blank value trusts nothing",
set: true,
value: " ",
expected: []string{},
},
{
name: caseValidValueParsed,
set: true,
value: cidrPrivateV4 + ", 192.168.1.7 ,2001:db8::/32",
expected: []string{
cidrPrivateV4, "192.168.1.7/32", "2001:db8::/32",
},
},
{
name: "host bits are masked off",
set: true,
value: "10.1.2.3/8",
expected: []string{cidrPrivateV4},
},
{
// Peer addresses are unmapped before they are
// matched, so an IPv4-mapped prefix kept in that
// form could never match anything.
name: "IPv4-mapped prefix is unmapped",
set: true,
value: "::ffff:10.0.0.0/104",
expected: []string{cidrPrivateV4},
},
{
name: caseUnparseableFails,
set: true,
value: cidrPrivateV4 + ",not-an-address",
expectError: true,
},
{
name: "out-of-range prefix length fails startup",
set: true,
value: "10.0.0.0/33",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set {
t.Setenv("TRUSTED_PROXIES", tt.value)
} else {
require.NoError(t, os.Unsetenv("TRUSTED_PROXIES"))
}
if tt.expectError {
expectStartupErrorFor(
t, "TRUSTED_PROXIES", config.ErrInvalidCIDR,
)
} else {
testTrustedProxiesSuccess(t, tt.expected)
}
})
}
}
func testTrustedProxiesSuccess(
t *testing.T,
expected []string,
) {
t.Helper()
var cfg *config.Config
app := fxtest.New(
t,
fx.Provide(
globals.New,
logger.New,
config.New,
),
fx.Populate(&cfg),
)
require.NoError(t, app.Err())
app.RequireStart()
defer app.RequireStop()
got := make([]string, 0, len(cfg.TrustedProxies))
for _, prefix := range cfg.TrustedProxies {
got = append(got, prefix.String())
}
assert.Equal(t, expected, got)
}

View File

@@ -0,0 +1,299 @@
package handlers_test
import (
"context"
"net/http"
"net/http/httptest"
"strconv"
"testing"
"time"
"github.com/stretchr/testify/assert"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
// Template data keys the page templates read. The handlers package has
// its own unexported constants for these; this is the external test
// package, so it needs its own.
const (
dataKeyWebhook = "Webhook"
dataKeyError = "Error"
)
// testWebhookID is the identifier given to the webhook under test on
// pages that render one.
const testWebhookID = "wh-1"
// renderPage renders a page template through the real template set as
// an authenticated user and returns the resulting HTML.
func renderPage(
t *testing.T,
h *handlers.Handlers,
sess *session.Session,
page string,
data map[string]any,
) string {
t.Helper()
cookies := authenticatedCookies(t, sess, "test-user-id", "testuser")
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, "/", nil,
)
for _, c := range cookies {
req.AddCookie(c)
}
w := httptest.NewRecorder()
h.RenderTemplateForTest(w, req, page, data)
return w.Body.String()
}
// TestNavbarUsesWebhookTerminology pins the user-visible navigation
// label to "Webhooks". The /sources route is deliberately unchanged, so
// the assertion targets the link text rather than the href.
func TestNavbarUsesWebhookTerminology(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
// One item, so the list body renders too: it calls
// WebhookListItem.RetentionLabel, promoted from the embedded
// Webhook and therefore a pointer method. An empty list would
// skip that call and hide a template error behind the
// navigation assertions below.
item := handlers.WebhookListItem{}
item.Name = "wh"
item.ID = testWebhookID
item.RetentionDays = 14
body := renderPage(t, h, sess, "sources_list.html", map[string]any{
"Webhooks": []handlers.WebhookListItem{item},
})
assert.Contains(t, body, "Retention: 14 days")
assert.Contains(t, body, `class="btn-text">Webhooks</a>`)
assert.Contains(
t, body, `class="btn-text w-full text-left">Webhooks</a>`,
)
assert.Contains(
t, body,
`<h1 class="text-2xl font-medium text-gray-900">Webhooks</h1>`,
)
assert.NotContains(
t, body, ">Sources<",
"no user-visible element may still be labelled Sources",
)
assert.Contains(
t, body, `href="/sources"`,
"the /sources route itself must not change",
)
}
// TestEditPageUsesWebhookTerminology pins the edit page's heading and
// its back link. The link's href still points at /source/{id}, which is
// intentional: only user-visible copy changes.
func TestEditPageUsesWebhookTerminology(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
// The webhook goes in as a pointer because source_edit.html calls
// Webhook.RetentionLabel, a pointer method: a map element is not
// addressable, so a value here renders an error instead of the
// page.
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
webhook.ID = testWebhookID
body := renderPage(t, h, sess, "source_edit.html", map[string]any{
dataKeyWebhook: webhook,
dataKeyError: "",
})
assert.Contains(t, body, "Edit Webhook")
assert.NotContains(t, body, ">Sources<")
assert.Contains(t, body, `href="/source/wh-1"`)
}
// TestCreateFormRetentionCopyMatchesBehaviour pins the create form's
// retention copy to what the code does: the reaper permanently deletes
// events past the cutoff, an empty field falls back to
// DefaultRetentionDays, and 0 is rewritten to the retain-forever
// sentinel by Webhook.BeforeSave.
func TestCreateFormRetentionCopyMatchesBehaviour(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
body := renderPage(t, h, sess, "sources_new.html", map[string]any{
"Name": "",
"Description": "",
"DefaultRetentionDays": database.DefaultRetentionDays,
dataKeyError: "",
})
assert.Contains(
t, body,
"permanently deletes events older than this",
"the form must say retention is enforced by deletion",
)
assert.Contains(t, body, "Enter 0 to retain events forever")
assert.Contains(
t, body,
"leave blank to use the default of "+
strconv.Itoa(database.DefaultRetentionDays)+" days",
"blank means the default, not forever",
)
}
// TestEditFormRetentionCopyMatchesBehaviour pins the edit form's
// retention copy, including that it states the stored policy via
// RetentionLabel and that an empty field leaves that policy unchanged
// rather than meaning forever.
func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
finite := &database.Webhook{Name: "wh", RetentionDays: 14}
finite.ID = testWebhookID
body := renderPage(t, h, sess, "source_edit.html", map[string]any{
dataKeyWebhook: finite,
dataKeyError: "",
})
assert.Contains(t, body, "Currently 14 days.")
assert.Contains(
t, body,
"permanently deletes events older than this",
)
assert.Contains(t, body, "Enter 0 to retain events forever")
assert.Contains(
t, body,
"leave blank to keep the current setting",
"blank means unchanged, not forever",
)
forever := &database.Webhook{
Name: "wh",
RetentionDays: database.RetentionForeverDays,
}
forever.ID = "wh-2"
foreverBody := renderPage(
t, h, sess, "source_edit.html", map[string]any{
dataKeyWebhook: forever,
dataKeyError: "",
},
)
assert.Contains(
t, foreverBody, "Currently forever.",
"a retain-forever webhook must not read as a day count",
)
assert.Contains(
t, foreverBody,
"No events are deleted while retention is set to forever",
)
assert.NotContains(
t, foreverBody,
"permanently deletes events older than this",
"the reaper skips retain-forever webhooks, so the form "+
"must not claim it deletes their events",
)
}
// TestEntrypointCopyButtonIsProgressiveEnhancement proves the copy
// affordance degrades: the button ships with the hidden attribute, so a
// browser that never runs app.js shows no dead control, and the URL is
// rendered as ordinary selectable text either way.
func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
entrypoint := database.Entrypoint{Path: "abc123"}
entrypoint.ID = "ep-1"
// The webhook goes in as a pointer because source_detail.html
// calls Webhook.RetentionLabel, a pointer method: a map element
// is not addressable, so a value here aborts execution partway
// down the page, after the copy button has already been flushed
// to the response.
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
webhook.ID = testWebhookID
webhook.CreatedAt = time.Date(
2026, time.January, 2, 3, 4, 5, 0, time.UTC,
)
body := renderPage(t, h, sess, "source_detail.html", map[string]any{
dataKeyWebhook: webhook,
"Entrypoints": []database.Entrypoint{entrypoint},
// The handler passes delivery.NewTargetViews(targets), never
// raw targets, so the test data has to have that same shape.
"Targets": delivery.NewTargetViews(nil),
"Events": []database.Event{},
"BaseURL": "https://hooks.example.com",
})
assert.Contains(
t, body,
`<code id="entrypoint-url-ep-1"`,
)
assert.Contains(t, body, "https://hooks.example.com/webhook/abc123")
assert.Contains(
t, body,
`hidden data-copy-target="entrypoint-url-ep-1"`,
"the button must start hidden and be revealed by script",
)
// renderTemplate streams to the ResponseWriter, so an abort
// midway still leaves everything above it in the body. This pins
// content from the last line of the template, which is below the
// assertions above: without it, a page that renders the copy
// button and then 500s passes.
assert.Contains(
t, body, "Retention: 14 days",
"the page must render to completion, not abort partway",
)
}

View File

@@ -214,11 +214,11 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
// handler runs, while the headers are still ours to // handler runs, while the headers are still ours to
// write. // write.
if s.session.Touch(sess) { 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( s.log.Error(
"auth middleware: failed to refresh session", "auth middleware: failed to refresh session",
"error", err, "error", saveErr,
) )
} }
} }

View File

@@ -2,6 +2,9 @@ package middleware
import ( import (
"net/http" "net/http"
"net/netip"
"slices"
"strings"
"time" "time"
"github.com/go-chi/httprate" "github.com/go-chi/httprate"
@@ -31,13 +34,120 @@ const (
receiverRateInterval = 1 * time.Minute receiverRateInterval = 1 * time.Minute
) )
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
// addr so that comparisons and bucket keys are canonical.
func normalizeAddr(addr netip.Addr) netip.Addr {
return addr.Unmap().WithZone("")
}
// isTrustedProxy reports whether addr belongs to a network the
// operator listed in TRUSTED_PROXIES. The list is empty by default,
// so by default nothing is trusted.
func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
for _, prefix := range m.params.Config.TrustedProxies {
if prefix.Contains(addr) {
return true
}
}
return false
}
// forwardedClientAddr returns the client address named by this
// request's X-Forwarded-For chain. It is consulted only for requests
// whose direct peer is a trusted proxy.
//
// X-Forwarded-For is the only header read. X-Real-IP and
// True-Client-IP are deliberately ignored: the reverse proxies in
// common use append to X-Forwarded-For and pass any other header the
// client sent through untouched, so believing a single-valued header
// would let a client behind the trusted proxy name its own bucket —
// the very bypass this gating exists to close.
//
// The chain is walked right to left, because the rightmost entry is
// the one the nearest proxy appended and everything to its left may
// have been written by the client. The first hop that is not itself
// 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.
func (m *Middleware) forwardedClientAddr(
r *http.Request,
) (netip.Addr, bool) {
hops := strings.Split(
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
)
for _, hop := range slices.Backward(hops) {
hop = strings.TrimSpace(hop)
if hop == "" {
continue
}
addr, err := netip.ParseAddr(hop)
if err != nil {
return netip.Addr{}, false
}
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
return addr, true
}
}
return netip.Addr{}, false
}
// rateLimitKey is the client identity every rate limiter in this
// package buckets on. Forwarded headers are honoured only when the
// direct peer (RemoteAddr) is inside the configured trusted-proxy
// set; otherwise the peer address itself is the key. Without that
// gate any client could mint a fresh bucket per request, or starve
// another client's bucket, by picking an X-Forwarded-For value —
// which makes every limit here decorative against a deliberate
// attacker.
func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
return m.clientKey(r), nil
}
// clientKey computes the bucket key described on rateLimitKey.
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 rather than collapsing such peers into one
// shared bucket.
return r.RemoteAddr
}
peer = normalizeAddr(peer)
if !m.isTrustedProxy(peer) {
return peer.String()
}
if addr, ok := m.forwardedClientAddr(r); ok {
return addr.String()
}
return peer.String()
}
// tooManyRequests returns the 429 handler shared by every limiter:
// it logs the rejection with logMessage and answers with
// responseMessage. httprate adds the Retry-After header (RFC 6585).
func (m *Middleware) tooManyRequests(
logMessage, responseMessage string,
) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
m.log.Warn(logMessage, "path", r.URL.Path)
http.Error(w, responseMessage, http.StatusTooManyRequests)
}
}
// LoginRateLimit returns middleware that enforces per-IP rate // LoginRateLimit returns middleware that enforces per-IP rate
// limiting on login attempts using go-chi/httprate. Only POST // limiting on login attempts using go-chi/httprate. Only POST
// requests are rate-limited; GET requests (rendering the login // requests are rate-limited; GET requests (rendering the login
// form) pass through unaffected. When the rate limit is exceeded, // form) pass through unaffected. When the rate limit is exceeded,
// a 429 Too Many Requests response is returned. IP extraction // a 429 Too Many Requests response is returned. Clients are
// honours X-Forwarded-For, X-Real-IP, and True-Client-IP headers // identified by rateLimitKey.
// for reverse-proxy setups.
func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler {
return m.postRateLimit( return m.postRateLimit(
loginRateLimit, loginRateLimit,
@@ -66,9 +176,7 @@ func (m *Middleware) PasswordChangeRateLimit() func(http.Handler) http.Handler {
// limit on POST requests only; all other methods pass through // limit on POST requests only; all other methods pass through
// unaffected. Requests over the limit receive a 429 with the // unaffected. Requests over the limit receive a 429 with the
// given response message, and each rejection is logged with the // given response message, and each rejection is logged with the
// given log message. IP extraction honours X-Forwarded-For, // given log message. Clients are identified by rateLimitKey.
// X-Real-IP, and True-Client-IP headers for reverse-proxy
// setups.
func (m *Middleware) postRateLimit( func (m *Middleware) postRateLimit(
limit int, limit int,
interval time.Duration, interval time.Duration,
@@ -77,19 +185,10 @@ func (m *Middleware) postRateLimit(
limiter := httprate.Limit( limiter := httprate.Limit(
limit, limit,
interval, interval,
httprate.WithKeyFuncs(httprate.KeyByRealIP), httprate.WithKeyFuncs(m.rateLimitKey),
httprate.WithLimitHandler(http.HandlerFunc( httprate.WithLimitHandler(
func(w http.ResponseWriter, r *http.Request) { m.tooManyRequests(logMessage, responseMessage),
m.log.Warn(logMessage, ),
"path", r.URL.Path,
)
http.Error(
w,
responseMessage,
http.StatusTooManyRequests,
)
},
)),
) )
return func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler {
@@ -116,31 +215,19 @@ func (m *Middleware) postRateLimit(
// path (the path contains the entrypoint UUID, so each sender // path (the path contains the entrypoint UUID, so each sender
// is limited per entrypoint without affecting other senders or // is limited per entrypoint without affecting other senders or
// other entrypoints). The limit is Config.ReceiverRateLimit // other entrypoints). The limit is Config.ReceiverRateLimit
// requests per minute. Requests over the limit receive a 429; // requests per minute. Requests over the limit receive a 429.
// httprate adds the Retry-After header (RFC 6585). IP // Clients are identified by rateLimitKey.
// extraction honours X-Forwarded-For, X-Real-IP, and
// True-Client-IP headers for reverse-proxy setups.
func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler { func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
return httprate.Limit( return httprate.Limit(
m.params.Config.ReceiverRateLimit, m.params.Config.ReceiverRateLimit,
receiverRateInterval, receiverRateInterval,
httprate.WithKeyFuncs( httprate.WithKeyFuncs(
httprate.KeyByRealIP, m.rateLimitKey,
httprate.KeyByEndpoint, httprate.KeyByEndpoint,
), ),
httprate.WithLimitHandler(http.HandlerFunc( httprate.WithLimitHandler(m.tooManyRequests(
func(w http.ResponseWriter, r *http.Request) {
m.log.Warn(
"webhook receiver rate limit exceeded", "webhook receiver rate limit exceeded",
"path", r.URL.Path, "Too many requests. Please slow down.",
)
http.Error(
w,
"Too many requests. "+
"Please slow down.",
http.StatusTooManyRequests,
)
},
)), )),
) )
} }

View File

@@ -2,9 +2,11 @@ package middleware_test
import ( import (
"context" "context"
"fmt"
"log/slog" "log/slog"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/netip"
"os" "os"
"testing" "testing"
@@ -182,11 +184,22 @@ func TestLoginRateLimit_IndependentPerIP(t *testing.T) {
) )
} }
// receiverLimitedHandler builds a ReceiverRateLimit-wrapped // okHandler is the terminal handler the limiter middleware wraps
// handler with the given per-minute limit. // in these tests: it answers 200 to anything that reaches it.
func receiverLimitedHandler( func okHandler() http.Handler {
t *testing.T, limit int, return http.HandlerFunc(
) http.Handler { func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
},
)
}
// rateLimitMiddleware builds a Middleware around cfg, whose
// TrustedProxies field is what the rate limit key function gates
// forwarded-header trust on.
func rateLimitMiddleware(
t *testing.T, cfg *config.Config,
) *middleware.Middleware {
t.Helper() t.Helper()
log := slog.New(slog.NewTextHandler( log := slog.New(slog.NewTextHandler(
@@ -194,17 +207,53 @@ func receiverLimitedHandler(
&slog.HandlerOptions{Level: slog.LevelDebug}, &slog.HandlerOptions{Level: slog.LevelDebug},
)) ))
m := middleware.NewForTest( return middleware.NewForTest(log, cfg, nil)
log, }
&config.Config{ReceiverRateLimit: limit},
nil, // trustedProxies parses CIDR strings for a test Config.
func trustedProxies(cidrs ...string) []netip.Prefix {
prefixes := make([]netip.Prefix, 0, len(cidrs))
for _, cidr := range cidrs {
prefixes = append(prefixes, netip.MustParsePrefix(cidr))
}
return prefixes
}
// postWithHeaders sends one POST to the handler from peer with the
// given headers set and returns the recorder.
func postWithHeaders(
handler http.Handler,
peer, path string,
headers map[string]string,
) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, path, nil,
)
req.RemoteAddr = peer
for name, value := range headers {
req.Header.Set(name, value)
}
w := httptest.NewRecorder()
handler.ServeHTTP(w, req)
return w
}
// receiverLimitedHandler builds a ReceiverRateLimit-wrapped
// handler with the given per-minute limit and no trusted proxies.
func receiverLimitedHandler(
t *testing.T, limit int,
) http.Handler {
t.Helper()
m := rateLimitMiddleware(
t, &config.Config{ReceiverRateLimit: limit},
) )
return m.ReceiverRateLimit()(http.HandlerFunc( return m.ReceiverRateLimit()(okHandler())
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
},
))
} }
// receiverPost sends one POST to the handler from the given IP // receiverPost sends one POST to the handler from the given IP
@@ -311,3 +360,252 @@ func TestReceiverRateLimit_CountsEveryMethod(t *testing.T) {
"a GET over the limit must be rate-limited", "a GET over the limit must be rate-limited",
) )
} }
const (
loginPath = "/pages/login"
headerXFF = "X-Forwarded-For"
headerReal = "X-Real-IP"
headerTrue = "True-Client-IP"
)
// assertSharedBucket drives the login limiter from peer with the
// trusted-proxy set proxies, sending one more request than the limit
// allows and varying the headers on each with headers(i). Every
// request must land in the same bucket, so the last one is rejected:
// if any of the varying header values reached the key, the run would
// have minted fresh buckets and nothing would be rejected.
func assertSharedBucket(
t *testing.T,
proxies []netip.Prefix,
peer string,
headers func(i int) map[string]string,
msg string,
) {
t.Helper()
m := rateLimitMiddleware(
t, &config.Config{TrustedProxies: proxies},
)
handler := m.LoginRateLimit()(okHandler())
for i := range middleware.LoginRateLimitConst {
w := postWithHeaders(handler, peer, loginPath, headers(i))
assert.Equal(
t, http.StatusOK, w.Code,
"request %d should pass", i,
)
}
w := postWithHeaders(
handler, peer, loginPath,
headers(middleware.LoginRateLimitConst),
)
assert.Equal(t, http.StatusTooManyRequests, w.Code, msg)
}
// TestRateLimitKey_SpoofedForwardedFromUntrustedPeer is the test
// this gating exists for: with no trusted proxies configured (the
// default), a client that rotates a forwarded header on every
// request must stay in one bucket. If forwarded headers were
// trusted unconditionally, each spoofed value would mint a fresh
// bucket and the limit would stop no one.
func TestRateLimitKey_SpoofedForwardedFromUntrustedPeer(
t *testing.T,
) {
t.Parallel()
for _, header := range []string{
headerXFF, headerReal, headerTrue,
} {
t.Run(header, func(t *testing.T) {
t.Parallel()
assertSharedBucket(
t, nil, "203.0.113.9:44444",
func(i int) map[string]string {
return map[string]string{
header: fmt.Sprintf(
"198.51.100.%d", i+1,
),
}
},
"a spoofed "+header+" from an untrusted peer "+
"must not mint a fresh bucket",
)
})
}
}
// TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer is the
// regression test for the bypass hiding inside the trusted case.
// Real reverse proxies (nginx, HAProxy, Caddy, ALB) set only
// X-Forwarded-For and pass every other client header through
// verbatim, so a client behind the configured proxy can send its own
// X-Real-IP or True-Client-IP. Reading either would hand that client
// a fresh bucket per request from inside exactly the deployment
// TRUSTED_PROXIES exists to serve, so neither header is read at all.
func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
t *testing.T,
) {
t.Parallel()
for _, header := range []string{headerReal, headerTrue} {
t.Run(header, func(t *testing.T) {
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"),
"10.0.0.1:44444",
func(i int) map[string]string {
return map[string]string{
header: fmt.Sprintf(
"198.51.100.%d", i+1,
),
}
},
header+" from a trusted peer must not mint a "+
"fresh bucket: only X-Forwarded-For is read",
)
})
}
}
// TestRateLimitKey_MalformedRightmostHopFallsBackToPeer covers the
// other end of the chain walk. The rightmost X-Forwarded-For entry
// is the one the trusted proxy appended; if it cannot be read as an
// address the chain is not the shape the walk assumes, and every
// entry to its left may have come from the client. The walk must
// stop and fall back to the peer rather than select one of them.
func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
t *testing.T,
) {
t.Parallel()
// Forms seen in the wild: host:port (Azure Application
// Gateway, IIS ARR), a bracketed IPv6 literal, and the
// RFC 7239 placeholder token.
for _, tail := range []string{
"198.51.100.7:1234", "[2001:db8::1]", "unknown",
} {
t.Run(tail, func(t *testing.T) {
t.Parallel()
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, tail,
),
}
},
"an unparseable rightmost hop must fall back "+
"to the peer address, not select a "+
"client-controlled entry",
)
})
}
}
// TestRateLimitKey_ForwardedHonouredFromTrustedPeer checks the
// other half: when the direct peer is a configured trusted proxy,
// the forwarded client address is what buckets are keyed on, so
// one sender behind the proxy cannot exhaust another's limit.
func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
t *testing.T,
) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies("10.0.0.0/8"),
})
handler := m.LoginRateLimit()(okHandler())
const peer = "10.0.0.1:44444"
first := map[string]string{headerXFF: "198.51.100.7"}
for range middleware.LoginRateLimitConst {
postWithHeaders(handler, peer, loginPath, first)
}
w := postWithHeaders(handler, peer, loginPath, first)
assert.Equal(
t, http.StatusTooManyRequests, w.Code,
"the forwarded client's own bucket must fill up",
)
w = postWithHeaders(
handler, peer, loginPath,
map[string]string{headerXFF: "198.51.100.8"},
)
assert.Equal(
t, http.StatusOK, w.Code,
"a forwarded header from a trusted peer must be honoured",
)
}
// TestRateLimitKey_ChainWalkSkipsClientPrepended covers the
// residual spoofing route behind a trusted proxy: the client
// controls the leftmost X-Forwarded-For entries, so the key is the
// rightmost hop that is not itself trusted. Rotating the prepended
// entry must not create new buckets.
func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
t.Parallel()
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, 198.51.100.7, 10.0.0.2", i+1,
),
}
},
"a client-prepended X-Forwarded-For entry must not "+
"mint a fresh bucket",
)
}
// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
// the receiver limiter uses the same gated key function as the
// POST limiters.
func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
t *testing.T,
) {
t.Parallel()
const (
limit = 3
peer = "203.0.113.10:44444"
path = "/webhook/uuid-d"
)
handler := receiverLimitedHandler(t, limit)
for i := range limit {
w := postWithHeaders(
handler, peer, path,
map[string]string{
headerXFF: fmt.Sprintf(
"198.51.100.%d", i+1,
),
},
)
assert.Equal(
t, http.StatusOK, w.Code,
"request %d should pass", i,
)
}
w := postWithHeaders(
handler, peer, path,
map[string]string{headerXFF: "198.51.100.200"},
)
assert.Equal(
t, http.StatusTooManyRequests, w.Code,
"a spoofed X-Forwarded-For from an untrusted peer must "+
"not mint a fresh receiver bucket",
)
}

View File

@@ -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",
)
}

View File

@@ -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)
}

View File

@@ -100,6 +100,35 @@ type Session struct {
now func() time.Time 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 // New creates a new session manager. The cookie store is
// initialized during the fx OnStart phase after the database is // initialized during the fx OnStart phase after the database is
// connected, using a session key that is auto-generated and stored // 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.key = keyBytes
s.store = store s.store = newStore(keyBytes, !params.Config.IsDev())
s.log.Info("session manager initialized") s.log.Info("session manager initialized")
return nil return nil
@@ -350,13 +368,8 @@ func (s *Session) Regenerate(
// Apply the standard session options (the destroyed old // Apply the standard session options (the destroyed old
// session had MaxAge = -1, which store.New might inherit // session had MaxAge = -1, which store.New might inherit
// from the cookie). // 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 return newSess, nil
} }

View File

@@ -39,6 +39,19 @@ func (c *fakeClock) Advance(d time.Duration) {
c.t = c.t.Add(d) 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 // testSession creates a Session with a real cookie store and the
// real clock. // real clock.
func testSession(t *testing.T) *session.Session { func testSession(t *testing.T) *session.Session {
@@ -59,20 +72,8 @@ func testSessionWithClock(
) (*session.Session, *fakeClock) { ) (*session.Session, *fakeClock) {
t.Helper() 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{ cfg := &config.Config{
Environment: config.EnvironmentDev, 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) { func TestTouch_UnauthenticatedSessionIsNotRefreshed(t *testing.T) {
t.Parallel() t.Parallel()

View File

@@ -1,2 +1,60 @@
// Webhooker client-side JavaScript // Webhooker client-side JavaScript
console.log("Webhooker loaded"); console.log("Webhooker loaded");
// Copy-to-clipboard, as progressive enhancement.
//
// Markup renders each copy button with the `hidden` attribute and a
// `data-copy-target` pointing at the id of the element holding the
// text. This script reveals a button only once it has both a resolvable
// target and a usable Clipboard API, so a browser without either shows
// no button at all and the text stays selectable.
(function () {
"use strict";
const revertDelayMs = 2000;
function flash(button, message) {
const original = button.getAttribute("data-copy-label");
button.textContent = message;
window.setTimeout(function () {
button.textContent = original;
}, revertDelayMs);
}
function wire(button) {
const target = document.getElementById(
button.getAttribute("data-copy-target")
);
if (!target) {
return;
}
button.setAttribute("data-copy-label", button.textContent);
button.addEventListener("click", function () {
navigator.clipboard.writeText(target.textContent.trim()).then(
function () {
flash(button, "Copied");
},
function () {
flash(button, "Copy failed");
}
);
});
button.removeAttribute("hidden");
}
function init() {
if (!navigator.clipboard || !navigator.clipboard.writeText) {
return;
}
const buttons = document.querySelectorAll("[data-copy-target]");
buttons.forEach(wire);
}
if (document.readyState === "loading") {
document.addEventListener("DOMContentLoaded", init);
} else {
init();
}
})();

View File

@@ -16,7 +16,7 @@
<!-- Desktop navigation --> <!-- Desktop navigation -->
<div class="hidden md:flex items-center gap-4"> <div class="hidden md:flex items-center gap-4">
{{if .User}} {{if .User}}
<a href="/sources" class="btn-text">Sources</a> <a href="/sources" class="btn-text">Webhooks</a>
<a href="/user/{{.User.Username}}" class="btn-text"> <a href="/user/{{.User.Username}}" class="btn-text">
<svg class="w-5 h-5 mr-1" fill="currentColor" viewBox="0 0 16 16"> <svg class="w-5 h-5 mr-1" fill="currentColor" viewBox="0 0 16 16">
<path d="M11 6a3 3 0 1 1-6 0 3 3 0 0 1 6 0z"/> <path d="M11 6a3 3 0 1 1-6 0 3 3 0 0 1 6 0z"/>
@@ -38,7 +38,7 @@
<div x-show="open" x-cloak x-transition class="md:hidden mt-4 pt-4 border-t border-gray-200"> <div x-show="open" x-cloak x-transition class="md:hidden mt-4 pt-4 border-t border-gray-200">
<div class="flex flex-col gap-2"> <div class="flex flex-col gap-2">
{{if .User}} {{if .User}}
<a href="/sources" class="btn-text w-full text-left">Sources</a> <a href="/sources" class="btn-text w-full text-left">Webhooks</a>
<a href="/user/{{.User.Username}}" class="btn-text w-full text-left">Profile</a> <a href="/user/{{.User.Username}}" class="btn-text w-full text-left">Profile</a>
<form method="POST" action="/pages/logout"> <form method="POST" action="/pages/logout">
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}"> <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">

View File

@@ -34,7 +34,6 @@
<hr class="border-gray-200 mb-6"> <hr class="border-gray-200 mb-6">
<div class="grid grid-cols-1 md:grid-cols-2 gap-8">
<div> <div>
<h3 class="text-lg font-medium text-gray-900 mb-3">Account Information</h3> <h3 class="text-lg font-medium text-gray-900 mb-3">Account Information</h3>
<dl class="space-y-3"> <dl class="space-y-3">
@@ -48,11 +47,6 @@
</div> </div>
</dl> </dl>
</div> </div>
<div>
<h3 class="text-lg font-medium text-gray-900 mb-3">Settings</h3>
<p class="text-sm text-gray-500">Profile settings and preferences will be available here.</p>
</div>
</div>
</div> </div>
<div class="card p-6 mt-6"> <div class="card p-6 mt-6">

View File

@@ -69,7 +69,12 @@
</form> </form>
</div> </div>
</div> </div>
<code class="text-xs text-gray-500 break-all block mt-1">{{$.BaseURL}}/webhook/{{.Path}}</code> <div class="flex items-start gap-2 mt-1">
<code id="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 break-all block flex-1">{{$.BaseURL}}/webhook/{{.Path}}</code>
<!-- Hidden until app.js reveals it; without the
script the URL above stays selectable. -->
<button type="button" hidden data-copy-target="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 hover:text-primary-600">Copy</button>
</div>
</div> </div>
{{else}} {{else}}
<div class="p-4 text-sm text-gray-500">No entrypoints configured.</div> <div class="p-4 text-sm text-gray-500">No entrypoints configured.</div>

View File

@@ -29,7 +29,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="0" class="input">
<p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}. Enter 0 to retain events forever.</p> <p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}.{{if .Webhook.RetainsForever}} No events are deleted while retention is set to forever.{{else}} A periodic cleanup permanently deletes events older than this, along with their delivery records.{{end}} Enter 0 to retain events forever; leave blank to keep the current setting.</p>
</div> </div>
<div class="flex gap-3"> <div class="flex gap-3">

View File

@@ -1,6 +1,6 @@
{{template "base" .}} {{template "base" .}}
{{define "title"}}Sources - Webhooker{{end}} {{define "title"}}Webhooks - Webhooker{{end}}
{{define "content"}} {{define "content"}}
<div class="max-w-6xl mx-auto px-6 py-8"> <div class="max-w-6xl mx-auto px-6 py-8">

View File

@@ -29,7 +29,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="{{.DefaultRetentionDays}}" min="0" class="input"> <input type="number" id="retention_days" name="retention_days" value="{{.DefaultRetentionDays}}" min="0" 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">A periodic cleanup permanently deletes events older than this, along with their delivery records. Enter 0 to retain events forever; leave blank to use the default of {{.DefaultRetentionDays}} days.</p>
</div> </div>
<div class="flex gap-3"> <div class="flex gap-3">