Name each database target's archive for its webhook and target (closes #376)

Each database target now has its own archive file,
archive-WEBHOOKNAME-TARGETNAME-TARGETID.db, instead of one
archive-WEBHOOKID.db per webhook. delivery.ArchiveFileName builds the
name.

A change of webhook or target name renames its archive files under the
archive writer's lock, before the new name is saved, and back again if
the save fails. Webhook edits, target edits and target creation run one
at a time, so no two of them interleave. A rename never replaces a
file, and one that fails part way moves back what it moved. Deleting a
target evicts only that target's writer. Archive files are never
deleted, and nothing looks for files under the old name.

Model: opus-5-5

README.md

@@ -145,6 +145,11 @@ TTY detection, and security headers are always applied.
 | `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted. A set value replaces the default. If any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set it to the proxy's address alone. See [Trusted proxies](#trusted-proxies) | `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16` (RFC 1918) |
 | `ALLOWED_EGRESS_CIDRS`  | CIDRs that delivery targets may reach despite the SSRF blocklist. Read [Allowing egress to your own network](#allowing-egress-to-your-own-network) before setting it | `""` (none) |
 
+The Settings page of the web UI (`/settings`, behind the login) lists
+every one of these with the value the running server loaded. It is
+read-only, and it shows `METRICS_PASSWORD` and `SENTRY_DSN` only as
+set or not set, never their values.
+
 #### Allowing egress to your own network
 
 By default every delivery target must resolve to a public address. The
@@ -1931,8 +1936,10 @@ when nothing is left. The target UUID keeps the file name unique. A
 webhook named `Orders (EU)` with a target named `Long-term archive`
 archives into `archive-orders-eu-long-term-archive-{target_uuid}.db`.
 Renaming the webhook or the target renames the file, under the same
-lock the archive writes and the archive sweeper take, so the name on
-disk matches the UI. A rename never replaces a file: if one already has
+lock the archive writes and the archive sweeper take. Webhook edits,
+target edits and target creation run one at a time, so no edit can
+rename the file between another's rename and save, and the name on disk
+matches the UI. A rename never replaces a file: if one already has
 the new name, the edit is refused with an error naming that file, and
 the stored name stays. If the archive is not there (the operator moved
 it away), the rename is not an error, and the next write creates the
@@ -2822,6 +2829,7 @@ returns to the page that was asked for.
 | ------ | ------------------------ | ----------- |
 | `GET`  | `/user/{username}`       | User profile page |
 | `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then `429`; `503` if no verification slot frees up within 5s, or immediately if 16 requests are already queued for one) |
+| `GET`  | `/settings`              | Read-only list of the configuration the server is running with; `METRICS_PASSWORD` and `SENTRY_DSN` show only as set or not set |
 | `GET`  | `/hooks`                 | List user's webhooks |
 | `GET`  | `/hooks/new`             | Create webhook form |
 | `POST` | `/hooks/new`             | Create webhook submission |
@@ -2935,6 +2943,7 @@ webhooker/
 │   │   ├── healthcheck.go     # Health check handler
 │   │   ├── index.go           # Index page handler
 │   │   ├── profile.go         # User profile handler
+│   │   ├── settings.go        # Read-only Settings page handler
 │   │   ├── source_management.go  # Webhook CRUD handlers
 │   │   └── webhook.go         # Webhook receiver handler
 │   ├── healthcheck/
@@ -3044,14 +3053,14 @@ local record instead of nothing. What that placement gives up is
 recovery of a panic in the six entries above it, none of which does
 more than set a header or start a timer.
 
-Each admin page route group (`/pages`, `/user/*`, `/hooks`,
-`/hook/*`) starts with its own **Recoverer** and, if `SENTRY_DSN` is
-set, its own **Sentry** error reporting. That Recoverer answers a panic
-with the `500` error page in the normal layout; the global one keeps
-the plain-text `500` for every other route.
+Each admin page route group (`/pages`, `/user/*`, `/settings`, `/hooks`,
+`/hook/*`) starts with its own **Recoverer** and, if `SENTRY_DSN` is set, its
+own **Sentry** error reporting. That Recoverer answers a panic with the `500`
+error page in the normal layout; the global one keeps the plain-text `500` for
+every other route.
 
-Additionally, form endpoints (`/pages`, `/user/*`, `/hooks`,
-`/hook/*`) apply a **MaxBodySize** middleware that limits
+Additionally, form endpoints (`/pages`, `/user/*`, `/settings`,
+`/hooks`, `/hook/*`) apply a **MaxBodySize** middleware that limits
 POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
 CSRF middleware in every one of those route groups, because
 gorilla/csrf parses the form; if the cap were installed after it, form
@@ -3070,7 +3079,7 @@ declared length. A chunked request, or
 one that lies about its length, is hard-capped by
 `http.MaxBytesReader` and fails downstream at form-parse time.
 
-Those same four route groups then apply **CSRF** and **NoCache**
+Those same five route groups then apply **CSRF** and **NoCache**
 (`Cache-Control: no-store`, `Pragma: no-cache`), and every group except
 `/pages` applies **RequireAuth**. The rate limiters are per-route
 rather than global: **PasswordChangeRateLimit** on
@@ -3116,12 +3125,12 @@ check, see [The login endpoint](#the-login-endpoint).
   by middleware that runs before CSRF parses the form
 - **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf)
   on all state-changing forms (cookie-based double-submit tokens with
-  HMAC authentication). Applied to `/pages`, `/hooks`, `/hook`, and
-  `/user` routes. Excluded from `/h` (inbound webhook POSTs) and
-  `/api` (stateless API). The middleware detects TLS per-request through
-  `internal/reqtls.IsTLS` — the same predicate the session cookie uses —
-  to set appropriate cookie security flags and Origin/Referer validation
-  mode
+  HMAC authentication). Applied to `/pages`, `/hooks`, `/hook`,
+  `/settings`, and `/user` routes. Excluded from `/h` (inbound webhook
+  POSTs) and `/api` (stateless API). The middleware detects TLS
+  per-request through `internal/reqtls.IsTLS` — the same predicate the
+  session cookie uses — to set appropriate cookie security flags and
+  Origin/Referer validation mode
 - **The entrypoint URL is the receiver's only credential.** Nothing
   about an inbound request is verified; possession of the UUID
   authorises submission, and no shared secret or signature check will

internal/handlers/handlers.go

@@ -10,10 +10,12 @@ import (
 	"html/template"
 	"log/slog"
 	"net/http"
+	"sync"
 	"sync/atomic"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
+	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
 	"sneak.berlin/go/webhooker/internal/globals"
@@ -57,6 +59,7 @@ type HandlersParams struct {
 
 	Logger       *logger.Logger
 	Globals      *globals.Globals
+	Config       *config.Config
 	Database     *database.Database
 	WebhookDBMgr *database.WebhookDBManager
 	Healthcheck  *healthcheck.Healthcheck
@@ -89,6 +92,14 @@ type Handlers struct {
 	// is one delivery will actually attempt.
 	ssrf *delivery.Guard
 
+	// renameMu makes the webhook edit, the target edit and target
+	// creation run one at a time, each held from loading the stored
+	// names through the archive rename, the save and any move back.
+	// Interleaved, one could rename an archive between another's
+	// rename and save, leaving the file named for one edit and the
+	// stored names from the other.
+	renameMu sync.Mutex
+
 	// dummyVerifications counts the equivalent-cost verifications
 	// charged for usernames that do not exist. It exists so a test
 	// can prove that path runs without measuring wall-clock time.
@@ -140,6 +151,7 @@ func New(
 	s.templates = map[string]*template.Template{
 		"login.html":         parsePageTemplate("login.html"),
 		"profile.html":       parsePageTemplate("profile.html"),
+		"settings.html":      parsePageTemplate("settings.html"),
 		"sources_list.html":  parsePageTemplate("sources_list.html"),
 		"sources_new.html":   parsePageTemplate("sources_new.html"),
 		"source_detail.html": parsePageTemplate("source_detail.html", "webhook_stats.html"),

internal/handlers/handlers_test.go

@@ -56,13 +56,16 @@ func (n *recordingNotifier) Tasks() []delivery.Task {
 // recordingArchives is a delivery.Archives that records what it
 // was asked to do, so a test can prove that a deletion or rename
 // path reached the delivery engine. After FailRenames, every
-// rename of that target fails with the given error.
+// rename of that target fails with the given error. After
+// BlockNextRename, the next rename is recorded and then waits.
 type recordingArchives struct {
 	mu             sync.Mutex
 	evicted        []string
 	evictedTargets []string
 	renames        []archiveRename
 	renameErrs     map[string]error
+	entered        chan struct{}
+	release        chan struct{}
 }
 
 // errInjectedRename is the failure a test hands FailRenames.
@@ -99,15 +102,38 @@ func (r *recordingArchives) Rename(
 	targetID, webhookName, targetName string,
 ) error {
 	r.mu.Lock()
-	defer r.mu.Unlock()
 
 	r.renames = append(r.renames, archiveRename{
 		TargetID:    targetID,
 		WebhookName: webhookName,
 		TargetName:  targetName,
 	})
+	err := r.renameErrs[targetID]
+	entered, release := r.entered, r.release
+	r.entered, r.release = nil, nil
 
-	return r.renameErrs[targetID]
+	r.mu.Unlock()
+
+	if entered != nil {
+		close(entered)
+		<-release
+	}
+
+	return err
+}
+
+// BlockNextRename makes the next rename, once recorded, wait until
+// the returned release is called. The returned channel is closed
+// when that rename starts waiting.
+func (r *recordingArchives) BlockNextRename() (<-chan struct{}, func()) {
+	entered := make(chan struct{})
+	release := make(chan struct{})
+
+	r.mu.Lock()
+	r.entered, r.release = entered, release
+	r.mu.Unlock()
+
+	return entered, func() { close(release) }
 }
 
 // FailRenames makes every later rename of targetID fail with err.
@@ -161,6 +187,19 @@ func newTestApp(
 ) *fxtest.App {
 	t.Helper()
 
+	return newTestAppWithConfig(
+		t, &config.Config{DataDir: t.TempDir()}, targets...,
+	)
+}
+
+// newTestAppWithConfig is newTestApp over a caller-supplied Config.
+func newTestAppWithConfig(
+	t *testing.T,
+	cfg *config.Config,
+	targets ...any,
+) *fxtest.App {
+	t.Helper()
+
 	return fxtest.New(
 		t,
 		// fx's own log is discarded, not sent to t.Logf: a hook still
@@ -170,11 +209,7 @@ func newTestApp(
 		fx.Provide(
 			globals.New,
 			logger.New,
-			func() *config.Config {
-				return &config.Config{
-					DataDir: t.TempDir(),
-				}
-			},
+			func() *config.Config { return cfg },
 			database.New,
 			database.NewWebhookDBManager,
 			healthcheck.New,

internal/handlers/settings.go

@@ -0,0 +1,135 @@
+package handlers
+
+import (
+	"net/http"
+	"net/netip"
+	"strconv"
+	"strings"
+
+	"sneak.berlin/go/webhooker/internal/config"
+)
+
+// notSet is what the Settings page shows for a value that is empty.
+const notSet = "not set"
+
+// settingRow is one line of the Settings page: an environment
+// variable, what it controls, and the value the server loaded for it.
+type settingRow struct {
+	Name        string
+	Description string
+	Value       string
+}
+
+// HandleSettings returns a handler for the read-only Settings page,
+// which lists the configuration the server started with.
+func (h *Handlers) HandleSettings() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		h.renderTemplate(w, r, "settings.html", map[string]any{
+			"Settings": settingRows(h.params.Config),
+		})
+	}
+}
+
+// settingRows lists every field of cfg under the environment variable
+// it is read from, with the description the README's configuration
+// table gives it (less its pointers to other README sections), in the
+// table's order. METRICS_PASSWORD and SENTRY_DSN are credentials, so
+// their values never reach the page: only whether they are set.
+func settingRows(cfg *config.Config) []settingRow {
+	metricsUsername := cfg.MetricsUsername
+	if metricsUsername == "" {
+		metricsUsername = notSet
+	}
+
+	return []settingRow{
+		{"WEBHOOKER_ENVIRONMENT", "dev or prod", cfg.Environment},
+		{"PORT", "HTTP listen port", strconv.Itoa(cfg.Port)},
+		{
+			"BIND_ADDRESS",
+			"IP address the HTTP listener binds. Loopback by default, " +
+				"so the cleartext listener is not published on every " +
+				"interface. The Docker image ships 0.0.0.0 instead",
+			cfg.BindAddress,
+		},
+		{"DATA_DIR", "Directory for all SQLite databases", cfg.DataDir},
+		{"DEBUG", "Enable debug logging", strconv.FormatBool(cfg.Debug)},
+		{
+			"MAINTENANCE_MODE",
+			"Report maintenanceMode: true in the healthcheck JSON. " +
+				"It does not change how any request is served — no " +
+				"maintenance page exists",
+			strconv.FormatBool(cfg.MaintenanceMode),
+		},
+		{
+			"METRICS_USERNAME",
+			"Basic auth username for /metrics. Must be set together " +
+				"with METRICS_PASSWORD; one without the other fails " +
+				"startup",
+			metricsUsername,
+		},
+		{
+			"METRICS_PASSWORD",
+			"Basic auth password for /metrics. Must be set together " +
+				"with METRICS_USERNAME; one without the other fails " +
+				"startup",
+			setOrNotSet(cfg.MetricsPassword),
+		},
+		{
+			"SENTRY_DSN",
+			"Sentry error reporting DSN. Unset leaves error reporting " +
+				"off; a value the Sentry SDK cannot parse fails startup " +
+				"rather than serving with reporting silently off",
+			setOrNotSet(cfg.SentryDSN),
+		},
+		{
+			"RETENTION_SWEEP_INTERVAL",
+			"How often the retention reaper and archive sweeper run " +
+				"(Go duration, must be positive)",
+			cfg.RetentionSweepInterval.String(),
+		},
+		{
+			"SESSION_IDLE_TIMEOUT",
+			"Idle session timeout (Go duration)",
+			cfg.SessionIdleTimeout.String(),
+		},
+		{
+			"RECEIVER_RATE_LIMIT",
+			"Receiver requests/minute per IP per entrypoint " +
+				"(10x that per IP across the route)",
+			strconv.Itoa(cfg.ReceiverRateLimit),
+		},
+		{
+			"TRUSTED_PROXIES",
+			"CIDRs whose forwarded headers are trusted. A set value " +
+				"replaces the default. If any client can reach webhooker, " +
+				"or the proxy in front of it, from an RFC 1918 source " +
+				"address, set it to the proxy's address alone",
+			cidrList(cfg.TrustedProxies),
+		},
+		{
+			"ALLOWED_EGRESS_CIDRS",
+			"CIDRs that delivery targets may reach despite the " +
+				"SSRF blocklist",
+			cidrList(cfg.AllowedEgressCIDRs),
+		},
+	}
+}
+
+// setOrNotSet is how the Settings page shows a credential: whether it
+// has a value, never the value itself.
+func setOrNotSet(value string) string {
+	if value == "" {
+		return notSet
+	}
+
+	return "set"
+}
+
+// cidrList renders a CIDR list setting for the Settings page.
+func cidrList(prefixes []netip.Prefix) string {
+	if len(prefixes) == 0 {
+		return "none"
+	}
+
+	return strings.Join(config.PrefixStrings(prefixes), ", ")
+}

internal/handlers/settings_test.go

@@ -0,0 +1,151 @@
+package handlers_test
+
+import (
+	"context"
+	"html"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"regexp"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/handlers"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// settingsShown renders the Settings page over cfg as a logged-in user
+// and returns the value it shows for each variable name, plus the
+// whole page.
+func settingsShown(
+	t *testing.T, cfg *config.Config,
+) (map[string]string, string) {
+	t.Helper()
+
+	var h *handlers.Handlers
+
+	var sess *session.Session
+
+	app := newTestAppWithConfig(t, cfg, &h, &sess)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	req := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/settings", nil,
+	)
+	for _, c := range authenticatedCookies(t, sess, "id", "admin") {
+		req.AddCookie(c)
+	}
+
+	w := httptest.NewRecorder()
+	h.HandleSettings().ServeHTTP(w, req)
+	require.Equal(t, http.StatusOK, w.Code)
+
+	body := w.Body.String()
+
+	row := regexp.MustCompile(
+		`<code[^>]*>([A-Z_]+)</code>\s*<code[^>]*>([^<]*)</code>`,
+	)
+
+	shown := map[string]string{}
+	for _, match := range row.FindAllStringSubmatch(body, -1) {
+		shown[match[1]] = html.UnescapeString(match[2])
+	}
+
+	return shown, body
+}
+
+func TestSettingsPageShowsLoadedConfiguration(t *testing.T) {
+	t.Parallel()
+
+	// DEBUG and MAINTENANCE_MODE get opposite values, and each of
+	// METRICS_USERNAME, METRICS_PASSWORD and SENTRY_DSN is the only one
+	// of the three set in one of the content tests, so each row is
+	// checked against its own field.
+	cfg := &config.Config{
+		DataDir:                t.TempDir(),
+		Debug:                  true,
+		MaintenanceMode:        false,
+		Environment:            config.EnvironmentDev,
+		MetricsUsername:        "scraper",
+		MetricsPassword:        "",
+		Port:                   9123,
+		SentryDSN:              "",
+		BindAddress:            "192.0.2.10",
+		RetentionSweepInterval: 17 * time.Minute,
+		SessionIdleTimeout:     3 * time.Hour,
+		ReceiverRateLimit:      77,
+		TrustedProxies: []netip.Prefix{
+			netip.MustParsePrefix("10.1.0.0/16"),
+		},
+		AllowedEgressCIDRs: []netip.Prefix{
+			netip.MustParsePrefix("192.168.5.0/24"),
+			netip.MustParsePrefix("fd00::/8"),
+		},
+	}
+
+	shown, body := settingsShown(t, cfg)
+
+	assert.Equal(t, map[string]string{
+		"WEBHOOKER_ENVIRONMENT":    "dev",
+		"PORT":                     "9123",
+		"BIND_ADDRESS":             "192.0.2.10",
+		"DATA_DIR":                 cfg.DataDir,
+		"DEBUG":                    "true",
+		"MAINTENANCE_MODE":         "false",
+		"METRICS_USERNAME":         "scraper",
+		"METRICS_PASSWORD":         "not set",
+		"SENTRY_DSN":               "not set",
+		"RETENTION_SWEEP_INTERVAL": "17m0s",
+		"SESSION_IDLE_TIMEOUT":     "3h0m0s",
+		"RECEIVER_RATE_LIMIT":      "77",
+		"TRUSTED_PROXIES":          "10.1.0.0/16",
+		"ALLOWED_EGRESS_CIDRS":     "192.168.5.0/24, fd00::/8",
+	}, shown)
+
+	assert.Contains(
+		t, body, `href="/settings"`,
+		"the navigation bar links to the page",
+	)
+}
+
+func TestSettingsPageShowsUnsetValues(t *testing.T) {
+	t.Parallel()
+
+	const metricsPassword = "metrics-password-1f9a"
+
+	shown, body := settingsShown(t, &config.Config{
+		DataDir:         t.TempDir(),
+		MetricsPassword: metricsPassword,
+	})
+
+	assert.Equal(t, "not set", shown["METRICS_USERNAME"])
+	assert.Equal(t, "set", shown["METRICS_PASSWORD"])
+	assert.Equal(t, "not set", shown["SENTRY_DSN"])
+	assert.NotContains(t, body, metricsPassword)
+	assert.Equal(t, "none", shown["TRUSTED_PROXIES"])
+	assert.Equal(t, "none", shown["ALLOWED_EGRESS_CIDRS"])
+}
+
+func TestSettingsPageShowsSentryDSNOnlyAsSet(t *testing.T) {
+	t.Parallel()
+
+	const (
+		sentryKey = "dsnkey7c2e"
+		sentryDSN = "https://" + sentryKey + "@errors.example.com/42"
+	)
+
+	shown, body := settingsShown(t, &config.Config{
+		DataDir:   t.TempDir(),
+		SentryDSN: sentryDSN,
+	})
+
+	assert.Equal(t, "not set", shown["METRICS_USERNAME"])
+	assert.Equal(t, "not set", shown["METRICS_PASSWORD"])
+	assert.Equal(t, "set", shown["SENTRY_DSN"])
+	assert.NotContains(t, body, sentryKey)
+}

internal/handlers/source_management.go

@@ -505,6 +505,9 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 
 		sourceID := chi.URLParam(r, "sourceID")
 
+		h.renameMu.Lock()
+		defer h.renameMu.Unlock()
+
 		var webhook database.Webhook
 
 		err := h.db.DB().Where(
@@ -1320,6 +1323,9 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 
 		sourceID := chi.URLParam(r, "sourceID")
 
+		h.renameMu.Lock()
+		defer h.renameMu.Unlock()
+
 		var webhook database.Webhook
 
 		err := h.db.DB().Where(

internal/handlers/source_management_test.go

@@ -2,16 +2,20 @@ package handlers_test
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/go-chi/chi"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
 	"gorm.io/gorm/clause"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/handlers"
@@ -611,6 +615,118 @@ func TestHandleSourceEditSubmit_FailedSaveRenamesBack(t *testing.T) {
 	)
 }
 
+// errInjectedRead is the failure a test makes reads of the main
+// database report.
+var errInjectedRead = errors.New("injected read failure")
+
+// TestHandleSourceEditSubmit_FailedSaveRenamesBackWithoutReading
+// proves that when the save fails and every later read of the main
+// database fails too, each archive the rename moved is still renamed
+// back: the move back needs no second read of the webhook's targets.
+func TestHandleSourceEditSubmit_FailedSaveRenamesBackWithoutReading(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	wh := seedWebhookWithRetention(t, env.db, 7)
+	first := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
+	second := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
+
+	var saveFailed atomic.Bool
+
+	require.NoError(t, env.db.DB().Callback().Update().
+		Before("gorm:update").
+		Register("test:fail_save", func(tx *gorm.DB) {
+			saveFailed.Store(true)
+
+			_ = tx.AddError(errInjectedSave)
+		}),
+	)
+	require.NoError(t, env.db.DB().Callback().Query().
+		Before("gorm:query").
+		Register("test:fail_reads_after_save", func(tx *gorm.DB) {
+			if saveFailed.Load() {
+				_ = tx.AddError(errInjectedRead)
+			}
+		}),
+	)
+
+	oldName := wh.Name
+	wh.Name = renamedWebhookName
+
+	w := submitEdit(t, env, wh, "")
+	require.Equal(t, http.StatusInternalServerError, w.Code)
+
+	assert.Equal(
+		t,
+		[]archiveRename{
+			{first.ID, renamedWebhookName, first.Name},
+			{second.ID, renamedWebhookName, second.Name},
+			{first.ID, oldName, first.Name},
+			{second.ID, oldName, second.Name},
+		},
+		env.archives.Renames(),
+	)
+}
+
+// TestHandleSourceEditSubmit_EditsDoNotInterleave proves that a second
+// webhook edit submitted while the first is inside its archive rename
+// does not run until the first is saved, so afterwards the stored
+// names are the ones the archive was last renamed to. The stand-in's
+// last rename is the name the file has on disk.
+func TestHandleSourceEditSubmit_EditsDoNotInterleave(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	wh := seedWebhookWithRetention(t, env.db, 7)
+	tgt := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
+
+	entered, release := env.archives.BlockNextRename()
+
+	firstEdit, secondEdit := wh, wh
+	firstEdit.Name = "First"
+	secondEdit.Name = "Second"
+
+	firstCode := make(chan int, 1)
+
+	go func() { firstCode <- submitEdit(t, env, firstEdit, "").Code }()
+
+	<-entered
+
+	secondCode := make(chan int, 1)
+
+	go func() { secondCode <- submitEdit(t, env, secondEdit, "").Code }()
+
+	// Were the edits not ordered, the second would run to its end in
+	// this time, while the first is still inside its rename.
+	time.Sleep(200 * time.Millisecond)
+	release()
+
+	assert.Equal(t, http.StatusSeeOther, <-firstCode)
+	assert.Equal(t, http.StatusSeeOther, <-secondCode)
+
+	var (
+		storedWebhook database.Webhook
+		storedTarget  database.Target
+	)
+
+	require.NoError(
+		t, env.db.DB().First(&storedWebhook, "id = ?", wh.ID).Error,
+	)
+	require.NoError(
+		t, env.db.DB().First(&storedTarget, "id = ?", tgt.ID).Error,
+	)
+
+	renames := env.archives.Renames()
+	require.NotEmpty(t, renames)
+	assert.Equal(
+		t,
+		archiveRename{tgt.ID, storedWebhook.Name, storedTarget.Name},
+		renames[len(renames)-1],
+	)
+}
+
 // TestHandleSourceEditSubmit_FailedRenameRenamesTheOthersBack proves
 // that when a webhook has three database targets and only the middle
 // one's archive cannot be renamed, the stored name stays and both

internal/handlers/target_edit.go

@@ -80,6 +80,9 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
 // HandleTargetEditSubmit handles the target edit form submission.
 func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
+		h.renameMu.Lock()
+		defer h.renameMu.Unlock()
+
 		webhook, target, ok := h.ownedTarget(w, r)
 		if !ok {
 			return

internal/metrics/metrics.go

@@ -384,6 +384,7 @@ func (s *Set) initSeries() {
 		s.deliveriesFailed.WithLabelValues(label)
 		s.deliveryRetries.WithLabelValues(label)
 		s.deliveryReplays.WithLabelValues(label)
+		s.deliveryDuration.WithLabelValues(label)
 		s.deliveriesPending.WithLabelValues(label)
 		s.deliveriesRetrying.WithLabelValues(label)
 		s.circuitBreakersOpen.WithLabelValues(label)

internal/metrics/metrics_test.go

@@ -167,6 +167,7 @@ func TestKnownSeriesExistBeforeAnyDelivery(t *testing.T) {
 		"webhooker_deliveries_succeeded_total",
 		"webhooker_deliveries_failed_total",
 		"webhooker_delivery_retries_total",
+		"webhooker_delivery_duration_seconds",
 		"webhooker_circuit_breakers_open",
 	} {
 		assert.ElementsMatch(t,

internal/server/routes.go

@@ -14,10 +14,11 @@ import (
 // bytes) for form POST endpoints. 1 MB is generous for any form
 // submission while preventing abuse from oversized payloads.
 //
-// The four admin page route groups below (/pages, /user/{username},
-// /hooks and /hook/{sourceID}) install MaxBodySize(maxFormBodySize)
-// right after their recoverer and error reporting, ahead of both CSRF
-// and RequireAuth. Both orderings are deliberate.
+// The five admin page route groups below (/pages, /user/{username},
+// /settings, /hooks and /hook/{sourceID}) install
+// MaxBodySize(maxFormBodySize) right after their recoverer and error
+// reporting, ahead of both CSRF and RequireAuth. Both orderings are
+// deliberate.
 //
 // Ahead of CSRF because gorilla/csrf parses the form. The cap has to
 // be installed before anything reads the body, or the parse runs
@@ -154,6 +155,7 @@ func (s *Server) setupRoutes() {
 
 	s.setupPageRoutes()
 	s.setupUserRoutes()
+	s.setupSettingsRoutes()
 	s.setupSourceRoutes()
 	s.setupWebhookRoutes()
 }
@@ -201,6 +203,24 @@ func (s *Server) setupUserRoutes() {
 	})
 }
 
+// setupSettingsRoutes serves the Settings page. It is GET only:
+// configuration comes from the environment and nothing here changes
+// it.
+func (s *Server) setupSettingsRoutes() {
+	s.router.Route("/settings", func(r chi.Router) {
+		s.recoverPanics(
+			r, s.h.HandleErrorPage(http.StatusInternalServerError),
+		)
+		// MaxBodySize precedes CSRF and RequireAuth deliberately;
+		// see maxFormBodySize for why, and for what it costs.
+		r.Use(s.mw.MaxBodySize(maxFormBodySize))
+		r.Use(s.mw.CSRF(s.h.HandleErrorPage(http.StatusForbidden)))
+		r.Use(s.mw.NoCache())
+		r.Use(s.mw.RequireAuth())
+		r.Get("/", s.h.HandleSettings())
+	})
+}
+
 func (s *Server) setupSourceRoutes() {
 	s.router.Route("/hooks", func(r chi.Router) {
 		s.recoverPanics(

internal/server/routes_test.go

@@ -1586,3 +1586,31 @@ func TestTwoMetricsRoutersInOneProcess(t *testing.T) {
 		}
 	}
 }
+
+// TestMetricsScrapeBeforeAnyDelivery pins
+// https://git.eeqj.de/sneak/webhooker/issues/267: an instance that
+// has delivered nothing must still serve the delivery duration
+// histogram, at zero, for every target type.
+func TestMetricsScrapeBeforeAnyDelivery(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnvWithConfig(
+		t, metricsConfig(t, metricsUser, metricsAuthValue),
+	)
+
+	scrape := env.metricsRequest(metricsUser, metricsAuthValue)
+	require.Equal(t, http.StatusOK, scrape.Code)
+
+	for _, targetType := range []database.TargetType{
+		database.TargetTypeHTTP,
+		database.TargetTypeDatabase,
+		database.TargetTypeLog,
+		database.TargetTypeSlack,
+	} {
+		assert.Contains(
+			t, scrape.Body.String(),
+			`webhooker_delivery_duration_seconds_count{target_type="`+
+				string(targetType)+`"} 0`,
+		)
+	}
+}

internal/server/settings_test.go

@@ -0,0 +1,24 @@
+package server_test
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSettingsPageIsBehindLogin(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	w := env.get("/settings", nil)
+	assert.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, "/pages/login?next=%2Fsettings", w.Header().Get("Location"),
+	)
+
+	w = env.get("/settings", env.authCookies(t, "id", "admin"))
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Contains(t, w.Body.String(), "WEBHOOKER_ENVIRONMENT")
+}

script/test

@@ -28,10 +28,12 @@
 # -race every test binary and every link costs a few hundred MB, so the
 # defaults (one per core) add up to several GB on a many-core host.
 #
-# No -v: the Docker build cuts each step's log off at 2 MiB, and verbose output
-# from the whole suite passes that before a failure is printed. Without it, go
-# test prints one result line per package and, for a package that fails,
-# everything its tests wrote, application log lines included.
+# The first run has no -v: go test then prints one result line per package,
+# with its coverage, and for a package that fails, everything its tests wrote,
+# application log lines included. Verbose output from the whole suite passes
+# the 2 MiB at which the Docker build cuts off each step's log, so on a failure
+# only the tests that failed run again, with -v. The script exits 1 after that
+# rerun whatever its result: the first run already showed the suite is broken.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -39,7 +41,34 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 main() {
     cd "$ROOT"
     "$ROOT/script/assets"
-    go test -race -p 4 -parallel 8 -timeout 90s ./...
+
+    log="$(mktemp -t webhooker-test.XXXXXXXX)"
+    rcfile="$(mktemp -t webhooker-test-rc.XXXXXXXX)"
+    trap 'rm -f "$log" "$rcfile"' EXIT INT TERM
+
+    # The pipeline's status is tee's, and POSIX sh has no pipefail, so go
+    # test's status travels via a file. Output still streams live.
+    {
+        go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 \
+            && echo 0 >"$rcfile" || echo $? >"$rcfile"
+    } | tee "$log"
+    if [ "$(cat "$rcfile")" -eq 0 ]; then
+        return
+    fi
+
+    # go test reports a failed test as a line starting "--- FAIL: TestName"
+    # (a failed subtest's line is indented, and reruns with its parent), and
+    # a failed package as "FAIL<tab>package/path<tab>...". A failure that
+    # names no test, such as a build error or a timeout, is already shown in
+    # full above, so there is nothing to rerun.
+    tests="$(awk '/^--- FAIL: / { print $3 }' "$log" | paste -s -d '|' -)"
+    packages="$(awk '/^FAIL\t/ { print $2 }' "$log")"
+    if [ -n "$tests" ]; then
+        echo "--- Rerunning the failed tests with -v for details ---"
+        go test -race -v -p 4 -parallel 8 -timeout 90s \
+            -run "^($tests)\$" $packages || true
+    fi
+    exit 1
 }
 
 main "$@"

templates/navbar.html

@@ -19,6 +19,7 @@
         <div class="hidden md:flex items-center gap-4">
             {{if .User}}
             <a href="/hooks" class="btn-text">Webhooks</a>
+            <a href="/settings" class="btn-text">Settings</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"/>
@@ -43,6 +44,7 @@
         <div class="flex flex-col gap-2">
             {{if .User}}
             <a href="/hooks" class="btn-text w-full text-left">Webhooks</a>
+            <a href="/settings" class="btn-text w-full text-left">Settings</a>
             <a href="/user/{{.User.Username}}" class="btn-text w-full text-left">Profile</a>
             {{if .CSRFToken}}
             <form method="POST" action="/pages/logout">

templates/settings.html

@@ -0,0 +1,30 @@
+{{template "base" .}}
+
+{{define "title"}}Settings - Webhooker{{end}}
+
+{{define "content"}}
+<div class="max-w-6xl mx-auto px-6 py-8">
+    <h1 class="text-2xl font-medium text-gray-900">Settings</h1>
+    <p class="text-sm text-gray-500 mt-1 mb-6">The configuration this server started with. It is set in the server's environment and cannot be changed here.</p>
+
+    <div class="card">
+        <div class="divide-y divide-gray-100">
+            {{range .Settings}}
+            <div class="p-4">
+                <!-- A value too wide to sit beside its name moves to the
+                     next line, where a list breaks only at the spaces
+                     between its entries. overflow-wrap: anywhere breaks
+                     inside a value only when it alone is wider than the
+                     line; an inline style, because the committed
+                     tailwind.css has no class for it. -->
+                <div class="flex flex-wrap justify-between items-start gap-4">
+                    <code class="text-sm font-medium text-gray-900">{{.Name}}</code>
+                    <code class="text-sm text-gray-900" style="overflow-wrap: anywhere">{{.Value}}</code>
+                </div>
+                <p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
+            </div>
+            {{end}}
+        </div>
+    </div>
+</div>
+{{end}}