Gate forwarded-header trust behind trusted-proxy config (closes #88)

Every rate limiter keyed on httprate.KeyByRealIP, which believes
True-Client-IP, X-Real-IP and the first X-Forwarded-For entry from
any peer. A client could therefore mint a fresh bucket per request
by rotating a spoofed header, or drain another client's bucket by
claiming its address, which left the receiver, login and password
change limits with no value against a deliberate attacker.

The receiver, login and password change limiters now share one key
function: the connection's own address, unless the direct peer is
inside a network listed in the new TRUSTED_PROXIES CIDR list, in
which case the forwarded client address is used. The list is empty
by default, so nothing is trusted until an operator names their
proxy; a set-but-unparseable value aborts startup, matching the
handling of the other parsed variables.

X-Forwarded-For is the only forwarded header read, from any peer.
Reverse proxies append to it but pass other client headers through
verbatim, so believing a single-valued X-Real-IP or True-Client-IP
would hand a client behind the trusted proxy a fresh bucket per
request - the same bypass, inside the deployment TRUSTED_PROXIES
exists to serve. Within a trusted request the chain is walked right
to left, since the rightmost entry is the one the nearest proxy
appended, and the first hop that is not itself a trusted proxy is
taken as the client. A hop that is not a bare address - ip:port, a
bracketed IPv6 literal, the token unknown - ends the walk and the
peer address is used, rather than continuing left into entries the
client controls.

Trusted-proxy prefixes written in IPv4-mapped form are unmapped at
parse time, since peer addresses are unmapped before matching and
such a prefix would otherwise silently never match.

Also folds in two cleanups from the same review: the 429 responder
shared by all three limiters is extracted, and the
RECEIVER_RATE_LIMIT error-path tests now assert that the failure
names the variable and wraps ErrNonPositiveValue rather than only
that some error occurred.

README.md

@@ -100,9 +100,10 @@ TTY detection, and security headers are always applied.
 #### Trusted proxies
 
 `TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
-address such as `10.0.0.1` is accepted and treated as a single host),
-for example `10.0.0.0/8, 192.168.1.7, 2001:db8::/32`. It decides whose
-`X-Forwarded-For` header the rate limiters believe.
+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
@@ -125,7 +126,8 @@ 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.
+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:
 
@@ -133,12 +135,14 @@ Two operator requirements follow:
   (nginx `$proxy_add_x_forwarded_for`, HAProxy `option forwardfor`,
   Caddy and AWS ALB by default), and must append a bare address with
   no port.
-- Keep the list narrow. A client whose own address falls inside a
-  broad block such as `10.0.0.0/8` is treated as a proxy: its address
-  is skipped during the walk, so it shares the bucket of whatever lies
-  further left rather than getting one of its own. That is safe — the
-  block is trusted by definition — but surprising if the block covers
-  ordinary clients as well as proxies.
+- 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
 one runs out first:

TODO.md

@@ -26,6 +26,10 @@ capability in the README rationale).
 
 # 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
   (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
   requests, with the 7-day absolute cap kept as an independent

internal/config/config.go

@@ -103,11 +103,14 @@ type Config struct {
 	ReceiverRateLimit int
 
 	// TrustedProxies is the set of networks whose members are
-	// allowed to speak for the client with forwarded headers
-	// (X-Forwarded-For, X-Real-IP, True-Client-IP). It is empty
-	// unless TRUSTED_PROXIES is set, and empty means no peer is
+	// 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

internal/handlers/ui_copy_test.go

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

static/js/app.js

@@ -1,2 +1,60 @@
 // Webhooker client-side JavaScript
 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();
+    }
+})();

templates/navbar.html

@@ -16,7 +16,7 @@
         <!-- Desktop navigation -->
         <div class="hidden md:flex items-center gap-4">
             {{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">
                 <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"/>
@@ -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 class="flex flex-col gap-2">
             {{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>
             <form method="POST" action="/pages/logout">
                 <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">

templates/profile.html

@@ -34,24 +34,18 @@
 
         <hr class="border-gray-200 mb-6">
 
-        <div class="grid grid-cols-1 md:grid-cols-2 gap-8">
-            <div>
-                <h3 class="text-lg font-medium text-gray-900 mb-3">Account Information</h3>
-                <dl class="space-y-3">
-                    <div class="flex">
-                        <dt class="w-32 text-sm font-medium text-gray-500">Username</dt>
-                        <dd class="text-sm text-gray-900">{{.User.Username}}</dd>
-                    </div>
-                    <div class="flex">
-                        <dt class="w-32 text-sm font-medium text-gray-500">Account Type</dt>
-                        <dd class="text-sm text-gray-900">Standard User</dd>
-                    </div>
-                </dl>
-            </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>
+            <h3 class="text-lg font-medium text-gray-900 mb-3">Account Information</h3>
+            <dl class="space-y-3">
+                <div class="flex">
+                    <dt class="w-32 text-sm font-medium text-gray-500">Username</dt>
+                    <dd class="text-sm text-gray-900">{{.User.Username}}</dd>
+                </div>
+                <div class="flex">
+                    <dt class="w-32 text-sm font-medium text-gray-500">Account Type</dt>
+                    <dd class="text-sm text-gray-900">Standard User</dd>
+                </div>
+            </dl>
         </div>
     </div>
 

templates/source_detail.html

@@ -69,7 +69,12 @@
                             </form>
                         </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>
                 {{else}}
                 <div class="p-4 text-sm text-gray-500">No entrypoints configured.</div>

templates/source_edit.html

@@ -29,7 +29,7 @@
             <div class="form-group">
                 <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">
-                <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 class="flex gap-3">

templates/sources_list.html

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

templates/sources_new.html

@@ -29,7 +29,7 @@
             <div class="form-group">
                 <label for="retention_days" class="label">Retention (days)</label>
                 <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 class="flex gap-3">