Refuse [::], 0.0.0.0, IPv6 multicast and documentation space (closes #341)

On Linux a connection to the unspecified address [::] or 0.0.0.0
reaches the host's own loopback, and the SSRF guard let [::] through.
Both unspecified addresses now sit in alwaysBlockedNetworks, so an
allowlist reaches loopback only through an entry that covers a
loopback address, never through one that covers only 0.0.0.0 or ::;
::/128 joins the default blocklist beside 0.0.0.0/8. IPv6 multicast
(ff00::/8) and documentation space (2001:db8::/32) are refused by
default. Every default blocklist entry gets a one-line comment, and the
README, the rules above each list, the two pinning tests and the
allowlist refusal test follow.

Model: opus-5-5

README.md

@@ -158,19 +158,20 @@ WireServer, which serves an Azure VM its credentials. Because it is a
 public address, listing it in `ALLOWED_EGRESS_CIDRS` reopens it.
 
 That is all the default blocklist covers: the IPv4 private and reserved
-ranges; of IPv6, only loopback (`::1`), unique local addresses
-(`fc00::/7`) and link-local addresses (`fe80::/10`); and certain public
-addresses. A public address belongs on the default blocklist only if it
-hands credentials, user data or bootstrap material to whatever can reach
-it, without the caller presenting anything. A provider's other public
-addresses are not refused. IBM Cloud, for example, serves its package
-mirrors, time servers and object storage on `161.26.0.0/16`, and the
-private endpoints of its own cloud services on `166.8.0.0/14`. Neither
-range hands out credentials that way: the token service among those
-endpoints issues a token only in exchange for something the caller
-presents, such as an API key. Reaching these services can be a
-legitimate delivery, and every cloud has some, so a partial list would
-promise coverage it does not give.
+ranges; of IPv6, only loopback (`::1`), the unspecified address (`::`),
+unique local addresses (`fc00::/7`), link-local addresses (`fe80::/10`),
+multicast (`ff00::/8`) and documentation space (`2001:db8::/32`); and
+certain public addresses. A public address belongs on the default
+blocklist only if it hands credentials, user data or bootstrap material
+to whatever can reach it, without the caller presenting anything. A
+provider's other public addresses are not refused. IBM Cloud, for
+example, serves its package mirrors, time servers and object storage on
+`161.26.0.0/16`, and the private endpoints of its own cloud services on
+`166.8.0.0/14`. Neither range hands out credentials that way: the token
+service among those endpoints issues a token only in exchange for
+something the caller presents, such as an API key. Reaching these
+services can be a legitimate delivery, and every cloud has some, so a
+partial list would promise coverage it does not give.
 
 That default is also inconvenient for the thing webhooker is mostly
 for: taking a public webhook and forwarding it to something on your own
@@ -210,16 +211,16 @@ Two things this setting cannot do:
   the list is always an allowlist; an empty list (the default) means
   every private and reserved range stays refused. Note that
   `0.0.0.0/0` gets you most of the way there anyway, per above.
-- **It cannot open link-local, or a cloud metadata endpoint at a
-  non-public address that discloses credentials or user data.** An
-  address is on the list below when it is not a public address and both
-  of these hold: the provider fixes it, so it cannot collide with
-  anything you run; and reaching it hands out credentials, user data or
-  bootstrap material. Those stay blocked no matter what you list,
-  including when you list them outright or list a supernet such as
-  `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as best
-  effort rather than a guarantee — it is a hand-maintained list and the
-  caveat below the table applies:
+- **It cannot open link-local, the unspecified addresses, or a cloud
+  metadata endpoint at a non-public address that discloses credentials
+  or user data.** A metadata address is on the list below when it is not
+  a public address and both of these hold: the provider fixes it, so it
+  cannot collide with anything you run; and reaching it hands out
+  credentials, user data or bootstrap material. Those stay blocked no
+  matter what you list, including when you list them outright or list a
+  supernet such as `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`.
+  Treat this as best effort rather than a guarantee — it is a
+  hand-maintained list and the caveat below the table applies:
 
   | Blocked unconditionally | What it is |
   | ----------------------- | ---------- |
@@ -233,14 +234,25 @@ Two things this setting cannot do:
   | `fd00:a9fe:a9fe::1/128` | Linode/Akamai metadata over IPv6 |
   | `100.100.100.200/32` | Alibaba Cloud metadata, inside CGNAT |
   | `192.0.0.192/32` | Oracle Cloud Classic metadata |
+  | `0.0.0.0/32` | IPv4 unspecified address, which reaches this host's loopback on Linux |
+  | `::/128` | IPv6 unspecified address, which reaches this host's loopback on Linux |
   | `::a9fe:a9fe/128` | `169.254.169.254` as an IPv4-compatible IPv6 address |
   | `64:ff9b::a9fe:a9fe/128` | `169.254.169.254` behind the NAT64 well-known prefix |
 
   The IPv4-mapped form `::ffff:169.254.169.254` is covered by the
-  `169.254.0.0/16` entry. Reaching any of these is credential or
-  user-data theft rather than delivery to an internal service. Every
-  entry outside the two link-local blocks is a single address, so
-  blocking it costs you nothing else on the network around it.
+  `169.254.0.0/16` entry. Reaching any of these but the two unspecified
+  addresses is credential or user-data theft rather than delivery to an
+  internal service. Every entry outside the two link-local blocks is a
+  single address, so blocking it costs you nothing else on the network
+  around it.
+
+  The unspecified addresses `0.0.0.0` and `::` hand out nothing
+  themselves, but no host can have either, and on Linux a connection to
+  one reaches this host's own loopback. They are listed so that an
+  allowlist reaches loopback only through an entry that covers a loopback
+  address, such as `127.0.0.0/8`, `::1` or `0.0.0.0/0`, never through one
+  that covers only `0.0.0.0` or `::`; `0.0.0.0/8`, for example, does not
+  open loopback.
 
   The six ULA entries, all inside `fd00::/8`, are why this matters in
   practice: `fd00::/8` is an ordinary block to allowlist for your own
@@ -3093,7 +3105,8 @@ check, see [The login endpoint](#the-login-endpoint).
   route through a single decision function, so they cannot disagree
   about a destination. An operator can permit specific blocks with
   [`ALLOWED_EGRESS_CIDRS`](#allowing-egress-to-your-own-network); the
-  guard cannot be switched off, and link-local plus a
+  guard cannot be switched off, and link-local, the unspecified
+  addresses `0.0.0.0` and `::`, and a
   [pinned set](#allowing-egress-to-your-own-network) of known cloud
   metadata endpoints — several of which are ULAs outside link-local —
   stay blocked whatever is listed, though listing `0.0.0.0/0` or

internal/config/config.go

@@ -196,10 +196,11 @@ type Config struct {
 	// otherwise refuse. The guard itself is always on: there is no
 	// setting that disables SSRF protection, and delivery's
 	// alwaysBlockedNetworks stays blocked no matter what is listed
-	// here. That set is link-local plus the cloud metadata
-	// endpoints outside it that disclose credentials or user data
-	// at a provider-fixed, non-public address; it is not
-	// exhaustive of every cloud's metadata address. See
+	// here. That set is link-local, the unspecified addresses
+	// 0.0.0.0 and ::, and the cloud metadata endpoints outside
+	// link-local that disclose credentials or user data at a
+	// provider-fixed, non-public address; it is not exhaustive of
+	// every cloud's metadata address. See
 	// alwaysBlockedNetworks for the authoritative list and the
 	// criterion it is built from.
 	AllowedEgressCIDRs []netip.Prefix

internal/delivery/ssrf.go

@@ -37,8 +37,8 @@ var (
 		"blocked cloud metadata address",
 	)
 	errBlockedMetadata = errors.New(
-		"blocked link-local or cloud instance metadata " +
-			"address: ALLOWED_EGRESS_CIDRS cannot open it",
+		"blocked link-local, cloud instance metadata or " +
+			"unspecified address: ALLOWED_EGRESS_CIDRS cannot open it",
 	)
 	errInvalidScheme = errors.New(
 		"only http and https are allowed",
@@ -72,14 +72,16 @@ var blockedNetworks []*net.IPNet
 var blockedPublicNetworks []*net.IPNet
 
 // alwaysBlockedNetworks are the ranges no configuration can
-// open: the link-local blocks and the cloud instance metadata
-// endpoints that live outside them. Reaching one is credential
-// or user-data theft rather than delivery to an internal
-// service, so a supplied CIDR that covers such an address still
-// leaves it blocked.
+// open: the link-local blocks, the cloud instance metadata
+// endpoints that live outside them, and the unspecified
+// addresses. Reaching a metadata endpoint is credential or
+// user-data theft rather than delivery to an internal service,
+// so a supplied CIDR that covers such an address still leaves it
+// blocked.
 //
 // Inclusion criterion — an address belongs here only if BOTH
-// hold, and every entry below satisfies both:
+// hold, and every entry below but the unspecified addresses
+// satisfies both:
 //
 //  1. It is a fixed address assigned by the provider, or a
 //     range reserved by IANA — never one the operator chose.
@@ -112,6 +114,15 @@ var blockedPublicNetworks []*net.IPNet
 // This is a criterion, not an enumeration of every metadata
 // address in existence.
 //
+// The unspecified addresses 0.0.0.0 and :: fail (2) and are here
+// anyway. No host can have either, and on Linux a connection to
+// one reaches this host's own loopback. Listing them means an
+// allowlist reaches loopback only through an entry that covers a
+// loopback address (127.0.0.0/8, ::1/128, 0.0.0.0/0), never
+// through one that covers only 0.0.0.0 or :: (0.0.0.0/8, for
+// example). Nothing else lives at either address, so refusing
+// them costs nothing.
+//
 // Every entry is either already in blockedNetworks — this list is
 // what makes it unconditional — or an alternate encoding of
 // 169.254.169.254 that Contains does not match against
@@ -131,23 +142,46 @@ var alwaysBlockedNetworks []*net.IPNet
 //nolint:gochecknoinits // init is the idiomatic way to parse CIDRs once at startup
 func init() {
 	blockedNetworks = mustParseCIDRs([]string{
+		// IPv4 loopback.
 		"127.0.0.0/8",
+		// RFC 1918 private network.
 		"10.0.0.0/8",
+		// RFC 1918 private network.
 		"172.16.0.0/12",
+		// RFC 1918 private network.
 		"192.168.0.0/16",
+		// IPv4 link-local.
 		"169.254.0.0/16",
+		// "This network", holding the IPv4 unspecified address 0.0.0.0.
 		"0.0.0.0/8",
+		// Carrier-grade NAT shared address space.
 		"100.64.0.0/10",
+		// IETF protocol assignments.
 		"192.0.0.0/24",
+		// IPv4 documentation (TEST-NET-1).
 		"192.0.2.0/24",
+		// Benchmarking.
 		"198.18.0.0/15",
+		// IPv4 documentation (TEST-NET-2).
 		"198.51.100.0/24",
+		// IPv4 documentation (TEST-NET-3).
 		"203.0.113.0/24",
+		// IPv4 multicast.
 		"224.0.0.0/4",
+		// Reserved, including the broadcast address.
 		"240.0.0.0/4",
+		// IPv6 loopback.
 		"::1/128",
+		// IPv6 unspecified address.
+		"::/128",
+		// IPv6 unique local addresses.
 		"fc00::/7",
+		// IPv6 link-local.
 		"fe80::/10",
+		// IPv6 multicast.
+		"ff00::/8",
+		// IPv6 documentation.
+		"2001:db8::/32",
 	})
 
 	blockedPublicNetworks = mustParseCIDRs([]string{
@@ -207,6 +241,14 @@ func init() {
 		// allowlist from opening it.
 		"192.0.0.192/32",
 
+		// The unspecified addresses, each of which reaches this
+		// host's loopback on Linux.
+		//
+		// IPv4 unspecified address, inside the blocked 0.0.0.0/8.
+		"0.0.0.0/32",
+		// IPv6 unspecified address.
+		"::/128",
+
 		// 169.254.169.254 as an IPv4-compatible IPv6 address.
 		"::a9fe:a9fe/128",
 		// 169.254.169.254 behind the NAT64 well-known prefix.
@@ -343,8 +385,9 @@ func (g *Guard) allows(ip net.IP) bool {
 // The order is the policy:
 //
 //  1. alwaysBlockedNetworks is refused before the allowlist is
-//     consulted, so no configured CIDR reaches link-local or a
-//     cloud metadata endpoint at a non-public address.
+//     consulted, so no configured CIDR reaches link-local, a
+//     cloud metadata endpoint at a non-public address, or an
+//     unspecified address.
 //  2. The allowlist is consulted next, so a listed private
 //     network, or a listed public address on the default
 //     blocklist, becomes reachable.

internal/delivery/ssrf_allowlist_test.go

@@ -168,12 +168,13 @@ func TestGuardAllowlist_UnlistedPrivateStillRefused(t *testing.T) {
 
 // TestGuardAllowlist_MetadataAlwaysRefused is the load-bearing
 // case: cloud instance metadata endpoints are credential theft
-// rather than delivery to an internal service, so no allowlist
-// reaches one. Every guard below names a CIDR that covers its
-// target — including 0.0.0.0/0, ::/0, and the ordinary ULA and
-// CGNAT blocks an operator would really list — and the address
-// must stay refused anyway, on both the validation and the
-// delivery path.
+// rather than delivery to an internal service, and the
+// unspecified addresses 0.0.0.0 and :: reach this host's loopback
+// on Linux, so no allowlist reaches any of them. Every guard
+// below names a CIDR that covers its target — including
+// 0.0.0.0/0, ::/0, and the ordinary ULA and CGNAT blocks an
+// operator would really list — and the address must stay
+// refused anyway, on both the validation and the delivery path.
 func TestGuardAllowlist_MetadataAlwaysRefused(t *testing.T) {
 	t.Parallel()
 
@@ -219,15 +220,17 @@ type metadataAlwaysRefusedCase struct {
 }
 
 // metadataAlwaysRefusedCases enumerates every unconditionally
-// blocked address together with an allowlist entry that would
-// otherwise reach it. Split by family of address only to stay
-// under the function-length limit.
+// blocked address (link-local, the cloud metadata endpoints and
+// the unspecified addresses) together with an allowlist entry
+// that would otherwise reach it. Split by family of address only
+// to stay under the function-length limit.
 func metadataAlwaysRefusedCases() []metadataAlwaysRefusedCase {
 	cases := linkLocalRefusedCases()
 	cases = append(cases, ulaMetadataRefusedCases()...)
 	cases = append(cases, ipv4MetadataRefusedCases()...)
+	cases = append(cases, encodedMetadataRefusedCases()...)
 
-	return append(cases, encodedMetadataRefusedCases()...)
+	return append(cases, unspecifiedRefusedCases()...)
 }
 
 // linkLocalRefusedCases covers the link-local blocks, including
@@ -367,6 +370,23 @@ func encodedMetadataRefusedCases() []metadataAlwaysRefusedCase {
 	}
 }
 
+// unspecifiedRefusedCases covers the unspecified addresses, each
+// of which reaches this host's loopback on Linux.
+func unspecifiedRefusedCases() []metadataAlwaysRefusedCase {
+	return []metadataAlwaysRefusedCase{
+		{
+			name:   "IPv4 unspecified address under 0.0.0.0/0",
+			allow:  allowAllIPv4,
+			target: "http://0.0.0.0:8080/hook",
+		},
+		{
+			name:   "IPv6 unspecified address under ::/0",
+			allow:  allowAllIPv6,
+			target: "http://[::]:8080/hook",
+		},
+	}
+}
+
 // TestGuardAllowlist_PublicUnaffected asserts the allowlist does
 // not narrow anything: public addresses were reachable before it
 // existed and stay reachable, whether or not a list is set.
@@ -524,6 +544,10 @@ func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) {
 		// Oracle Cloud Classic metadata, inside the blocked
 		// 192.0.0.0/24.
 		"192.0.0.192/32",
+		// The IPv4 and IPv6 unspecified addresses, each of
+		// which reaches this host's loopback on Linux.
+		"0.0.0.0/32",
+		"::/128",
 		// 169.254.169.254 as an IPv4-compatible IPv6 address.
 		"::a9fe:a9fe/128",
 		// 169.254.169.254 behind the NAT64 well-known prefix.
@@ -556,7 +580,8 @@ func TestDefaultBlocklist_PinnedSet(t *testing.T) {
 		{cidr: "172.16.0.0/12", reopenable: true},
 		{cidr: "192.168.0.0/16", reopenable: true},
 		{cidr: linkLocalIPv4, reopenable: false},
-		{cidr: "0.0.0.0/8", reopenable: true},
+		// Its first address, 0.0.0.0, is in the unconditional set.
+		{cidr: "0.0.0.0/8", reopenable: false},
 		{cidr: "100.64.0.0/10", reopenable: true},
 		{cidr: "192.0.0.0/24", reopenable: true},
 		{cidr: "192.0.2.0/24", reopenable: true},
@@ -566,8 +591,11 @@ func TestDefaultBlocklist_PinnedSet(t *testing.T) {
 		{cidr: "224.0.0.0/4", reopenable: true},
 		{cidr: "240.0.0.0/4", reopenable: true},
 		{cidr: "::1/128", reopenable: true},
+		{cidr: "::/128", reopenable: false},
 		{cidr: "fc00::/7", reopenable: true},
 		{cidr: "fe80::/10", reopenable: false},
+		{cidr: "ff00::/8", reopenable: true},
+		{cidr: "2001:db8::/32", reopenable: true},
 		{cidr: "168.63.129.16/32", public: true, reopenable: true},
 	}
 

internal/delivery/ssrf_test.go

@@ -101,6 +101,42 @@ func TestValidateTargetURL_Blocked(t *testing.T) {
 	}
 }
 
+// TestDefaultGuard_RefusesUnspecifiedMulticastAndDocumentation
+// covers the unspecified addresses and the IPv6 multicast and
+// documentation ranges: with no allowlist set, each is refused
+// both when a target is created and when a delivery dials it.
+func TestDefaultGuard_RefusesUnspecifiedMulticastAndDocumentation(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	guard := delivery.NewTestGuard()
+
+	targets := []string{
+		// The unspecified addresses. On Linux a connection to
+		// either reaches this host's loopback.
+		"http://0.0.0.0:8080/hook",
+		"http://[::]:8080/hook",
+		// IPv6 multicast, all nodes.
+		"http://[ff02::1]/hook",
+		// IPv6 documentation.
+		"http://[2001:db8::1]/hook",
+	}
+
+	for _, target := range targets {
+		t.Run(target, func(t *testing.T) {
+			t.Parallel()
+
+			require.Error(t,
+				guard.ValidateTargetURL(context.Background(), target),
+				"%s must be refused at target creation", target,
+			)
+
+			assertDialRefused(t, guard, target)
+		})
+	}
+}
+
 func TestValidateTargetURL_Allowed(t *testing.T) {
 	t.Parallel()
 

internal/handlers/auth.go

@@ -333,9 +333,7 @@ func (h *Handlers) HandleLogout() http.HandlerFunc {
 			)
 		}
 
-		http.Redirect(
-			w, r, withNotice("/pages/login", signedOut),
-			http.StatusSeeOther,
-		)
+		// Redirect to login page
+		http.Redirect(w, r, "/pages/login", http.StatusSeeOther)
 	}
 }

internal/handlers/delivery_replay.go

@@ -11,37 +11,72 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-// The outcomes of a replay POST, as the notice codes its redirect
-// carries. noticeFor holds the line each one shows.
+// replayOutcomeParam is the query parameter the replay POST redirects
+// with and the event log page reads its banner from.
+const replayOutcomeParam = "replay"
+
+// replayOutcomeCode is the outcome of a replay POST. The redirect
+// carries one of these fixed codes rather than a message, so nothing a
+// client submits can reach the rendered page through it.
+type replayOutcomeCode string
+
 const (
 	// replayQueued reports that a new delivery was created and handed
 	// to the delivery engine.
-	replayQueued noticeCode = "replay-queued"
+	replayQueued replayOutcomeCode = "queued"
 
 	// replayTargetDeleted reports a target that once existed and has
 	// since been deleted. Deletes are soft and deliveries carry no
 	// foreign key to the target row, so the history survives its
 	// target and this is the ordinary case for an old event.
-	replayTargetDeleted noticeCode = "replay-target-deleted"
+	replayTargetDeleted replayOutcomeCode = "target-deleted"
 
 	// replayTargetMissing reports a target id that names no row at
 	// all, deleted or otherwise.
-	replayTargetMissing noticeCode = "replay-target-missing"
+	replayTargetMissing replayOutcomeCode = "target-missing"
 
 	// replayTargetInactive reports a target the operator has
 	// deactivated. A deactivated target receives no new deliveries, so
 	// a replay to it would be a delivery they switched off.
-	replayTargetInactive noticeCode = "replay-target-inactive"
+	replayTargetInactive replayOutcomeCode = "target-inactive"
 
 	// replayNotTerminal reports a delivery the engine has not finished
 	// with.
-	replayNotTerminal noticeCode = "replay-not-terminal"
+	replayNotTerminal replayOutcomeCode = "not-terminal"
 
 	// replayInFlight reports that an earlier replay of this event to
 	// this target is still running.
-	replayInFlight noticeCode = "replay-in-flight"
+	replayInFlight replayOutcomeCode = "in-flight"
 )
 
+// replayOutcome returns the banner the event log page shows for an
+// outcome code, and whether the replay was queued. An unrecognised
+// code yields no banner.
+func replayOutcome(code string) (string, bool) {
+	switch replayOutcomeCode(code) {
+	case replayQueued:
+		return "Replay queued: a new delivery was created against " +
+			"the target's current configuration.", true
+	case replayTargetDeleted:
+		return "Not replayed: the target this delivery was for has " +
+			"been deleted. Recreate the target, then replay.", false
+	case replayTargetMissing:
+		return "Not replayed: the target this delivery was for no " +
+			"longer exists.", false
+	case replayTargetInactive:
+		return "Not replayed: the target this delivery was for is " +
+			"deactivated. Activate it, then replay.", false
+	case replayNotTerminal:
+		return "Not replayed: this delivery has not finished yet.",
+			false
+	case replayInFlight:
+		return "Not replayed: a delivery of this event to this " +
+			"target is already in flight.", false
+	default:
+		return "", false
+	}
+}
+
 // HandleDeliveryReplay re-sends a finished delivery's event to its
 // target.
 //
@@ -105,14 +140,14 @@ func (h *Handlers) replayDelivery(
 	}
 
 	if !original.Status.Terminal() {
-		redirectToEventLog(w, r, webhook, replayNotTerminal)
+		h.finishReplay(w, r, webhook, replayNotTerminal)
 
 		return
 	}
 
 	target, code := h.replayTarget(webhook.ID, original.TargetID)
 	if target == nil {
-		redirectToEventLog(w, r, webhook, code)
+		h.finishReplay(w, r, webhook, code)
 
 		return
 	}
@@ -165,7 +200,7 @@ func (h *Handlers) queueReplay(
 	}
 
 	if inFlight > 0 {
-		redirectToEventLog(w, r, webhook, replayInFlight)
+		h.finishReplay(w, r, webhook, replayInFlight)
 
 		return
 	}
@@ -203,7 +238,7 @@ func (h *Handlers) queueReplay(
 		"delivery_id", task.DeliveryID,
 	)
 
-	redirectToEventLog(w, r, webhook, replayQueued)
+	h.finishReplay(w, r, webhook, replayQueued)
 }
 
 // replayTarget loads the delivery's target as it stands now.
@@ -216,7 +251,7 @@ func (h *Handlers) queueReplay(
 // with the returned code saying why.
 func (h *Handlers) replayTarget(
 	webhookID, targetID string,
-) (*database.Target, noticeCode) {
+) (*database.Target, replayOutcomeCode) {
 	var target database.Target
 
 	err := h.db.DB().Unscoped().Where(
@@ -326,16 +361,17 @@ func replayBody(body string) *string {
 	return &body
 }
 
-// redirectToEventLog redirects a replay or resubmit back to the event
-// log it was triggered from, carrying the outcome as its notice and
-// the page number the form submitted.
-func redirectToEventLog(
+// finishReplay redirects back to the event log the replay was
+// triggered from, carrying the outcome code the page turns into a
+// banner and the page number the form submitted.
+func (h *Handlers) finishReplay(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
-	code noticeCode,
+	code replayOutcomeCode,
 ) {
-	dest := withNotice("/hook/"+webhook.ID+"/events", code)
+	dest := "/hook/" + webhook.ID + "/events?" +
+		replayOutcomeParam + "=" + string(code)
 
 	// The page is read from the form rather than the query string:
 	// this is a POST, and its query string is what logs and Referer

internal/handlers/delivery_replay_test.go

@@ -212,7 +212,7 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-queued",
+		"/hook/"+wh.ID+"/events?replay=queued",
 		w.Header().Get("Location"),
 	)
 
@@ -362,7 +362,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-target-deleted",
+		"/hook/"+wh.ID+"/events?replay=target-deleted",
 		w.Header().Get("Location"),
 	)
 
@@ -390,7 +390,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, missing.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-target-missing",
+		"/hook/"+wh.ID+"/events?replay=target-missing",
 		missing.Header().Get("Location"),
 	)
 }
@@ -431,7 +431,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, first.Code)
 	require.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-queued",
+		"/hook/"+wh.ID+"/events?replay=queued",
 		first.Header().Get("Location"),
 	)
 
@@ -439,7 +439,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, second.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-in-flight",
+		"/hook/"+wh.ID+"/events?replay=in-flight",
 		second.Header().Get("Location"),
 	)
 
@@ -465,7 +465,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, pending.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-not-terminal",
+		"/hook/"+wh.ID+"/events?replay=not-terminal",
 		pending.Header().Get("Location"),
 	)
 }
@@ -509,7 +509,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
 	assert.Contains(t, body, ">Replay<")
 
 	refused := renderSourceLogsPageWithQuery(
-		t, h, sess, wh.ID, "?notice=replay-target-deleted",
+		t, h, sess, wh.ID, "?replay=target-deleted",
 	)
 
 	assert.Contains(t, refused, "alert-error")
@@ -517,7 +517,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
 
 	// An outcome code nobody issued renders no banner at all.
 	unknown := renderSourceLogsPageWithQuery(
-		t, h, sess, wh.ID, "?notice=made-up",
+		t, h, sess, wh.ID, "?replay=made-up",
 	)
 
 	assert.NotContains(t, unknown, "alert-error")

internal/handlers/event_resubmit.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"errors"
 	"net/http"
+	"strconv"
 
 	"github.com/go-chi/chi"
 	"github.com/google/uuid"
@@ -10,19 +11,43 @@ import (
 	"sneak.berlin/go/webhooker/internal/database"
 )
 
-// The outcomes of a resubmit POST, as the notice codes its redirect
-// carries. noticeFor holds the line each one shows.
+// resubmitOutcomeParam is the query parameter the resubmit POST
+// redirects with and the event log page reads its banner from.
+const resubmitOutcomeParam = "resubmit"
+
+// resubmitOutcomeCode is the outcome of a resubmit POST. The redirect
+// carries one of these fixed codes rather than a message, so nothing a
+// client submits can reach the rendered page through it.
+type resubmitOutcomeCode string
+
 const (
 	// resubmitQueued reports that a new event was stored and its
 	// deliveries handed to the delivery engine.
-	resubmitQueued noticeCode = "resubmit-queued"
+	resubmitQueued resubmitOutcomeCode = "queued"
 
 	// resubmitNoTargets reports a source with no active targets. The
 	// new event is stored either way, exactly as a received event
 	// with no targets is.
-	resubmitNoTargets noticeCode = "resubmit-no-targets"
+	resubmitNoTargets resubmitOutcomeCode = "no-targets"
 )
 
+// resubmitOutcome returns the banner the event log page shows for an
+// outcome code, and whether the resubmit was queued. An unrecognised
+// code yields no banner.
+func resubmitOutcome(code string) (string, bool) {
+	switch resubmitOutcomeCode(code) {
+	case resubmitQueued:
+		return "Resubmitted: a new event was created from the stored " +
+			"one and queued to every active target.", true
+	case resubmitNoTargets:
+		return "Resubmitted: a new event was created, but this " +
+				"source has no active targets, so nothing was queued.",
+			true
+	default:
+		return "", false
+	}
+}
+
 // resubmitSource is the stored event a resubmit copies. Its body is
 // read as bytes rather than as a string so the copy is byte-identical
 // to what was received, whatever the payload's encoding.
@@ -220,5 +245,29 @@ func (h *Handlers) queueResubmit(
 		code = resubmitNoTargets
 	}
 
-	redirectToEventLog(w, r, webhook, code)
+	h.finishResubmit(w, r, webhook, code)
+}
+
+// finishResubmit redirects back to the event log the resubmit was
+// triggered from, carrying the outcome code the page turns into a
+// banner and the page number the form submitted.
+func (h *Handlers) finishResubmit(
+	w http.ResponseWriter,
+	r *http.Request,
+	webhook database.Webhook,
+	code resubmitOutcomeCode,
+) {
+	dest := "/hook/" + webhook.ID + "/events?" +
+		resubmitOutcomeParam + "=" + string(code)
+
+	// The page is read from the form rather than the query string:
+	// this is a POST, and its query string is what logs and Referer
+	// headers record.
+	if page := pageOrFirst(
+		r.PostFormValue("page"),
+	); page > 1 {
+		dest += "&page=" + strconv.Itoa(page)
+	}
+
+	http.Redirect(w, r, dest, http.StatusSeeOther)
 }

internal/handlers/event_resubmit_test.go

@@ -154,7 +154,7 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+		"/hook/"+wh.ID+"/events?resubmit=queued",
 		w.Header().Get("Location"),
 	)
 
@@ -282,7 +282,7 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
 		require.Equal(t, http.StatusSeeOther, w.Code)
 		assert.Equal(
 			t,
-			"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+			"/hook/"+wh.ID+"/events?resubmit=queued",
 			w.Header().Get("Location"),
 			"a resubmit must not be refused while an earlier "+
 				"one is in flight",
@@ -436,7 +436,7 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+		"/hook/"+wh.ID+"/events?resubmit=queued",
 		w.Header().Get("Location"),
 		"an inactive target is skipped, not an error",
 	)
@@ -482,7 +482,7 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-no-targets",
+		"/hook/"+wh.ID+"/events?resubmit=no-targets",
 		w.Header().Get("Location"),
 	)
 

internal/handlers/handlers.go

@@ -97,10 +97,10 @@ type Handlers struct {
 
 // parsePageTemplate parses a page-specific template set from the
 // embedded FS. Each page template is combined with the shared
-// base, htmlheader, navbar and notice templates, and with any further
-// files the page includes. The page file must be listed first so that
-// its root action ({{template "base" .}}) becomes the template set's
-// entry point.
+// base, htmlheader, and navbar templates, and with any further files
+// the page includes. The page file must be listed first so that its
+// root action ({{template "base" .}}) becomes the template set's entry
+// point.
 func parsePageTemplate(
 	pageFile string, included ...string,
 ) *template.Template {
@@ -109,7 +109,6 @@ func parsePageTemplate(
 		"base.html",
 		"htmlheader.html",
 		"navbar.html",
-		"notice.html",
 	}, included...)
 
 	return template.Must(
@@ -210,13 +209,11 @@ func (s *Handlers) renderError(
 	// served outside the routes where NoCache runs.
 	w.Header().Set("Cache-Control", "no-store")
 
-	// No notice: one would say an action worked above a page saying
-	// the request failed.
 	data := s.pageData(r, map[string]any{
 		"Status":     status,
 		"StatusText": http.StatusText(status),
 		"Message":    errorPageText(status),
-	}, nil)
+	})
 
 	var buf bytes.Buffer
 
@@ -270,7 +267,6 @@ type templateDataWrapper struct {
 	User      *UserInfo
 	CSRFToken string
 	Version   string
-	Notice    *notice
 	Data      any
 }
 
@@ -315,15 +311,12 @@ func (s *Handlers) renderTemplate(
 		return
 	}
 
-	s.executeTemplate(w, r, tmpl, s.pageData(r, data, noticeFor(r)))
+	s.executeTemplate(w, r, tmpl, s.pageData(r, data))
 }
 
 // pageData adds the fields the shared layout renders to a page's own
-// data. The layout shows the notice, when there is one, above the
-// page.
-func (s *Handlers) pageData(
-	r *http.Request, data any, pageNotice *notice,
-) any {
+// data.
+func (s *Handlers) pageData(r *http.Request, data any) any {
 	userInfo := s.getUserInfo(r)
 	csrfToken := middleware.CSRFToken(r)
 
@@ -337,7 +330,6 @@ func (s *Handlers) pageData(
 		m["User"] = userInfo
 		m["CSRFToken"] = csrfToken
 		m["Version"] = version
-		m["Notice"] = pageNotice
 
 		return m
 	}
@@ -346,7 +338,6 @@ func (s *Handlers) pageData(
 		User:      userInfo,
 		CSRFToken: csrfToken,
 		Version:   version,
-		Notice:    pageNotice,
 		Data:      data,
 	}
 }

internal/handlers/notice.go

@@ -1,109 +0,0 @@
-package handlers
-
-import "net/http"
-
-// noticeParam is the query parameter an action's redirect carries its
-// notice code in.
-const noticeParam = "notice"
-
-// noticeCode names one of the fixed lines noticeFor knows. An action
-// redirects with the code rather than the line, so nothing a client
-// puts in the URL reaches the page: a code noticeFor does not know
-// shows nothing.
-type noticeCode string
-
-// The codes of the actions on the webhook pages and of signing out.
-// Replay's codes, with the reasons a replay can be refused, and
-// resubmit's codes are defined beside those actions.
-const (
-	webhookCreated        noticeCode = "webhook-created"
-	webhookSaved          noticeCode = "webhook-saved"
-	webhookDeleted        noticeCode = "webhook-deleted"
-	entrypointAdded       noticeCode = "entrypoint-added"
-	entrypointDeleted     noticeCode = "entrypoint-deleted"
-	entrypointActivated   noticeCode = "entrypoint-activated"
-	entrypointDeactivated noticeCode = "entrypoint-deactivated"
-	targetAdded           noticeCode = "target-added"
-	targetSaved           noticeCode = "target-saved"
-	targetDeleted         noticeCode = "target-deleted"
-	targetActivated       noticeCode = "target-activated"
-	targetDeactivated     noticeCode = "target-deactivated"
-	signedOut             noticeCode = "signed-out"
-)
-
-// notice is the line templates/notice.html shows above a page to say
-// what an action did.
-type notice struct {
-	Text string
-
-	// Failed shows the line as an error: the action was refused.
-	Failed bool
-}
-
-// noticeFor returns the notice the request's URL names, or nil when it
-// names none or an unknown code.
-func noticeFor(r *http.Request) *notice {
-	n, ok := map[noticeCode]notice{
-		webhookCreated:        {Text: "Webhook created."},
-		webhookSaved:          {Text: "Webhook saved."},
-		webhookDeleted:        {Text: "Webhook deleted."},
-		entrypointAdded:       {Text: "Entrypoint added."},
-		entrypointDeleted:     {Text: "Entrypoint deleted."},
-		entrypointActivated:   {Text: "Entrypoint activated."},
-		entrypointDeactivated: {Text: "Entrypoint deactivated."},
-		targetAdded:           {Text: "Target added."},
-		targetSaved:           {Text: "Target saved."},
-		targetDeleted:         {Text: "Target deleted."},
-		targetActivated:       {Text: "Target activated."},
-		targetDeactivated:     {Text: "Target deactivated."},
-		signedOut:             {Text: "Signed out."},
-
-		replayQueued: {
-			Text: "Replay queued: a new delivery was created " +
-				"against the target's current configuration.",
-		},
-		replayTargetDeleted: {
-			Text: "Not replayed: the target this delivery was for " +
-				"has been deleted. Recreate the target, then replay.",
-			Failed: true,
-		},
-		replayTargetMissing: {
-			Text: "Not replayed: the target this delivery was for " +
-				"no longer exists.",
-			Failed: true,
-		},
-		replayTargetInactive: {
-			Text: "Not replayed: the target this delivery was for " +
-				"is deactivated. Activate it, then replay.",
-			Failed: true,
-		},
-		replayNotTerminal: {
-			Text:   "Not replayed: this delivery has not finished yet.",
-			Failed: true,
-		},
-		replayInFlight: {
-			Text: "Not replayed: a delivery of this event to this " +
-				"target is already in flight.",
-			Failed: true,
-		},
-
-		resubmitQueued: {
-			Text: "Resubmitted: a new event was created from the " +
-				"stored one and queued to every active target.",
-		},
-		resubmitNoTargets: {
-			Text: "Resubmitted: a new event was created, but this " +
-				"source has no active targets, so nothing was queued.",
-		},
-	}[noticeCode(r.URL.Query().Get(noticeParam))]
-	if !ok {
-		return nil
-	}
-
-	return &n
-}
-
-// withNotice returns path with code added as its notice.
-func withNotice(path string, code noticeCode) string {
-	return path + "?" + noticeParam + "=" + string(code)
-}

internal/handlers/source_delete_test.go

@@ -411,9 +411,7 @@ func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
 	h.HandleSourceDelete().ServeHTTP(w, req)
 
 	require.Equal(t, http.StatusSeeOther, w.Code)
-	assert.Equal(
-		t, "/hooks?notice=webhook-deleted", w.Header().Get("Location"),
-	)
+	assert.Equal(t, "/hooks", w.Header().Get("Location"))
 
 	assert.Equal(
 		t, int64(0),

internal/handlers/source_management.go

@@ -322,8 +322,7 @@ func (h *Handlers) createWebhookWithEntrypoint(
 	)
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, webhookCreated),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -580,8 +579,7 @@ func (h *Handlers) applyWebhookEdit(
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, webhookSaved),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -664,9 +662,7 @@ func (h *Handlers) deleteWebhookResources(
 		return
 	}
 
-	http.Redirect(
-		w, r, withNotice("/hooks", webhookDeleted), http.StatusSeeOther,
-	)
+	http.Redirect(w, r, "/hooks", http.StatusSeeOther)
 }
 
 // commitWebhookDeletion soft-deletes a webhook's entrypoints,
@@ -845,16 +841,31 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 			totalPages++
 		}
 
+		// The banner a replay or resubmit POST redirected back
+		// with. The message comes from a fixed set keyed by the
+		// outcome code, never from the query string itself.
+		replayMsg, replayOK := replayOutcome(
+			r.URL.Query().Get(replayOutcomeParam),
+		)
+
+		resubmitMsg, resubmitOK := resubmitOutcome(
+			r.URL.Query().Get(resubmitOutcomeParam),
+		)
+
 		data := map[string]any{
-			tmplKeyWebhook: &webhook,
-			"Events":       evts,
-			"Page":         page,
-			"TotalPages":   totalPages,
-			"TotalEvents":  total,
-			"HasPrev":      page > 1,
-			"HasNext":      page < totalPages,
-			"PrevPage":     page - 1,
-			"NextPage":     page + 1,
+			tmplKeyWebhook:    &webhook,
+			"Events":          evts,
+			"ReplayMessage":   replayMsg,
+			"ReplayQueued":    replayOK,
+			"ResubmitMessage": resubmitMsg,
+			"ResubmitQueued":  resubmitOK,
+			"Page":            page,
+			"TotalPages":      totalPages,
+			"TotalEvents":     total,
+			"HasPrev":         page > 1,
+			"HasNext":         page < totalPages,
+			"PrevPage":        page - 1,
+			"NextPage":        page + 1,
 		}
 
 		h.renderTemplate(w, r, "source_logs.html", data)
@@ -1243,8 +1254,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 		}
 
 		http.Redirect(
-			w, r, withNotice("/hook/"+webhook.ID, entrypointAdded),
-			http.StatusSeeOther,
+			w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 		)
 	}
 }
@@ -1355,8 +1365,7 @@ func (h *Handlers) processTargetCreate(
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, targetAdded),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -1560,10 +1569,11 @@ func (h *Handlers) validateTargetURL(
 		msg := "Invalid target URL: " + err.Error()
 
 		// Only a private or reserved address's refusal says how
-		// to allow it. Metadata refusals never do: link-local and
-		// the other unconditional metadata addresses cannot be
-		// opened, and the default blocklist's public addresses,
-		// which listing does open, hand out credentials.
+		// to allow it. Other refusals never do: link-local, the
+		// unspecified addresses and the other unconditional
+		// metadata addresses cannot be opened, and the default
+		// blocklist's public addresses, which listing does open,
+		// hand out credentials.
 		if errors.Is(err, delivery.ErrBlockedPrivateOrReservedIP) {
 			msg += ". Private and reserved addresses are refused " +
 				"by default; the server's ALLOWED_EGRESS_CIDRS " +
@@ -1634,7 +1644,6 @@ func (h *Handlers) HandleEntrypointDelete() http.HandlerFunc {
 		"entrypointID", &database.Entrypoint{},
 		"failed to delete entrypoint",
 		nil,
-		entrypointDeleted,
 	)
 }
 
@@ -1647,21 +1656,18 @@ func (h *Handlers) HandleTargetDelete() http.HandlerFunc {
 		"targetID", &database.Target{},
 		"failed to delete target",
 		h.evictArchiveWriterIfUnused,
-		targetDeleted,
 	)
 }
 
 // deleteChildResource returns a handler that deletes a child
 // resource (entrypoint or target) belonging to a webhook. The
 // optional afterDelete hook runs with the webhook's id once the
-// delete has succeeded, before the redirect, which carries done as
-// its notice.
+// delete has succeeded, before the redirect.
 func (h *Handlers) deleteChildResource(
 	idParam string,
 	model any,
 	errMsg string,
 	afterDelete func(webhookID string),
-	done noticeCode,
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
@@ -1703,7 +1709,7 @@ func (h *Handlers) deleteChildResource(
 
 		http.Redirect(
 			w, r,
-			withNotice("/hook/"+webhook.ID, done),
+			"/hook/"+webhook.ID,
 			http.StatusSeeOther,
 		)
 	}
@@ -1714,7 +1720,7 @@ func (h *Handlers) deleteChildResource(
 func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 	return h.toggleChildResource(
 		"entrypointID",
-		func(webhookID, childID string) (bool, error) {
+		func(webhookID, childID string) error {
 			var ep database.Entrypoint
 
 			err := h.db.DB().Where(
@@ -1722,15 +1728,14 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 				childID, webhookID,
 			).First(&ep).Error
 			if err != nil {
-				return false, err
+				return err
 			}
 
 			ep.Active = !ep.Active
 
-			return ep.Active, h.db.DB().Save(&ep).Error
+			return h.db.DB().Save(&ep).Error
 		},
 		"failed to toggle entrypoint",
-		entrypointActivated, entrypointDeactivated,
 	)
 }
 
@@ -1738,7 +1743,7 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 	return h.toggleChildResource(
 		"targetID",
-		func(webhookID, childID string) (bool, error) {
+		func(webhookID, childID string) error {
 			var tgt database.Target
 
 			err := h.db.DB().Where(
@@ -1746,27 +1751,23 @@ func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 				childID, webhookID,
 			).First(&tgt).Error
 			if err != nil {
-				return false, err
+				return err
 			}
 
 			tgt.Active = !tgt.Active
 
-			return tgt.Active, h.db.DB().Save(&tgt).Error
+			return h.db.DB().Save(&tgt).Error
 		},
 		"failed to toggle target",
-		targetActivated, targetDeactivated,
 	)
 }
 
 // toggleChildResource returns a handler that toggles the active
-// state of a child resource belonging to a webhook. toggleFn returns
-// the new state, and the redirect carries activated or deactivated as
-// its notice to match.
+// state of a child resource belonging to a webhook.
 func (h *Handlers) toggleChildResource(
 	idParam string,
-	toggleFn func(webhookID, childID string) (bool, error),
+	toggleFn func(webhookID, childID string) error,
 	errMsg string,
-	activated, deactivated noticeCode,
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
@@ -1792,21 +1793,16 @@ func (h *Handlers) toggleChildResource(
 			return
 		}
 
-		active, err := toggleFn(webhook.ID, childID)
+		err = toggleFn(webhook.ID, childID)
 		if err != nil {
 			h.serverError(w, r, errMsg, err)
 
 			return
 		}
 
-		done := deactivated
-		if active {
-			done = activated
-		}
-
 		http.Redirect(
 			w, r,
-			withNotice("/hook/"+webhook.ID, done),
+			"/hook/"+webhook.ID,
 			http.StatusSeeOther,
 		)
 	}

internal/handlers/target_edit.go

@@ -161,8 +161,7 @@ func (h *Handlers) applyTargetEdit(
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, targetSaved),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 

internal/server/error_page_test.go

@@ -83,22 +83,6 @@ func TestErrorPage_DeletedTarget(t *testing.T) {
 	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
 }
 
-// TestErrorPage_ShowsNoNotice pins that a notice code in the URL of a
-// page that fails is not shown above the error.
-func TestErrorPage_ShowsNoNotice(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "owner", "somepassword")
-	cookies := env.authCookies(t, userID, "owner")
-
-	w := env.get("/hook/no-such-webhook?notice=webhook-saved", cookies)
-
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-	assert.NotContains(t, w.Body.String(), "Webhook saved.")
-}
-
 func TestErrorPage_UnknownPath(t *testing.T) {
 	t.Parallel()
 

internal/server/routes_test.go

@@ -263,24 +263,6 @@ func (e *testEnv) urlFrom(
 	return html.UnescapeString(match[1])
 }
 
-// requireNotice requires w to redirect to dest carrying the notice
-// code, then renders that page and requires it to show text.
-func (e *testEnv) requireNotice(
-	t *testing.T,
-	w *httptest.ResponseRecorder,
-	dest, code, text string,
-	cookies []*http.Cookie,
-) {
-	t.Helper()
-
-	require.Equal(t, http.StatusSeeOther, w.Code)
-	require.Equal(t, dest+"?notice="+code, w.Header().Get("Location"))
-
-	page := e.get(w.Header().Get("Location"), cookies)
-	require.Equal(t, http.StatusOK, page.Code)
-	assert.Contains(t, page.Body.String(), text)
-}
-
 // authCookies forges an authenticated session for the given user.
 func (e *testEnv) authCookies(
 	t *testing.T,
@@ -759,31 +741,6 @@ func TestPagesLogin_ReturnsToTheRequestedPage(t *testing.T) {
 	assert.Equal(t, asked, w.Header().Get("Location"))
 }
 
-// TestPagesLogout_SaysSignedOut signs out with the navbar's form and
-// lands on the sign-in page, which says so.
-func TestPagesLogout_SaysSignedOut(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "leaver", "somepassword")
-	token, cookies := env.csrfFrom(
-		t, "/hooks", env.authCookies(t, userID, "leaver"),
-	)
-
-	form := url.Values{}
-	form.Set("csrf_token", token)
-
-	w := env.post(
-		env.urlFrom(t, "/hooks", `action="(/pages/logout)"`, cookies),
-		form, cookies,
-	)
-
-	// The sign-in page is requested without the session cookie, which
-	// the logout told the browser to delete.
-	env.requireNotice(t, w, "/pages/login", "signed-out", "Signed out.", nil)
-}
-
 // --- /user/{username} group ---
 
 // TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -901,9 +858,9 @@ func TestHooks_ListAndNewWebhookForm(t *testing.T) {
 	require.NoError(t,
 		env.db.DB().Where("name = ?", "created").First(&created).Error,
 	)
-	env.requireNotice(
-		t, w, "/hook/"+created.ID, "webhook-created", "Webhook created.",
-		cookies,
+	assert.Equal(
+		t, "/hook/"+created.ID, w.Header().Get("Location"),
+		"creating a webhook should redirect to its page",
 	)
 }
 
@@ -934,7 +891,8 @@ func TestHook_EditFormAndDelete(t *testing.T) {
 		env.urlFrom(t, editPage, `action="(/hook/[^/"]+/edit)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(t, w, page, "webhook-saved", "Webhook saved.", cookies)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, page, w.Header().Get("Location"))
 
 	var edited database.Webhook
 
@@ -948,17 +906,16 @@ func TestHook_EditFormAndDelete(t *testing.T) {
 		env.urlFrom(t, page, `action="(/hook/[^/"]+/delete)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(
-		t, w, "/hooks", "webhook-deleted", "Webhook deleted.", cookies,
-	)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, "/hooks", w.Header().Get("Location"))
 	assert.Equal(
 		t, http.StatusNotFound, env.get(page, cookies).Code,
 		"a deleted webhook's page should be gone",
 	)
 }
 
-// TestHook_EntrypointActions adds, deactivates, activates and deletes
-// an entrypoint with the forms on the webhook page, each submitted to
+// TestHook_EntrypointActions adds, deactivates and deletes an
+// entrypoint with the forms on the webhook page, each submitted to
 // the action and with the token the page rendered.
 func TestHook_EntrypointActions(t *testing.T) {
 	t.Parallel()
@@ -976,19 +933,16 @@ func TestHook_EntrypointActions(t *testing.T) {
 	form.Set("csrf_token", token)
 
 	// submit posts the webhook page's form whose action pattern
-	// captures, and requires the redirect back to that page with the
-	// notice code, and the page to show text.
-	submit := func(pattern, code, text string) {
+	// captures, and requires the redirect back to that page.
+	submit := func(pattern string) {
 		t.Helper()
 
 		w := env.post(env.urlFrom(t, page, pattern, cookies), form, cookies)
-		env.requireNotice(t, w, page, code, text, cookies)
+		require.Equal(t, http.StatusSeeOther, w.Code)
+		require.Equal(t, page, w.Header().Get("Location"))
 	}
 
-	toggle := `action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`
-
-	submit(`action="(/hook/[^/"]+/entrypoints)"`,
-		"entrypoint-added", "Entrypoint added.")
+	submit(`action="(/hook/[^/"]+/entrypoints)"`)
 
 	var added database.Entrypoint
 
@@ -997,7 +951,7 @@ func TestHook_EntrypointActions(t *testing.T) {
 	)
 	require.True(t, added.Active)
 
-	submit(toggle, "entrypoint-deactivated", "Entrypoint deactivated.")
+	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`)
 
 	var toggled database.Entrypoint
 
@@ -1006,10 +960,7 @@ func TestHook_EntrypointActions(t *testing.T) {
 	)
 	assert.False(t, toggled.Active, "the toggle should deactivate it")
 
-	submit(toggle, "entrypoint-activated", "Entrypoint activated.")
-
-	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`,
-		"entrypoint-deleted", "Entrypoint deleted.")
+	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`)
 
 	var left int64
 
@@ -1020,8 +971,8 @@ func TestHook_EntrypointActions(t *testing.T) {
 
 // TestHook_TargetActions adds a target with the form on the webhook
 // page, follows its Edit link to the target edit form and submits
-// it, then deactivates, activates and deletes it, every URL and token
-// taken from the rendered pages.
+// it, then deactivates and deletes it, every URL and token taken from
+// the rendered pages.
 func TestHook_TargetActions(t *testing.T) {
 	t.Parallel()
 
@@ -1036,29 +987,27 @@ func TestHook_TargetActions(t *testing.T) {
 
 	// submit posts form, with the token, to the action pattern
 	// captures on the page at from, and requires the redirect back to
-	// the webhook page with the notice code, and that page to show
-	// text.
-	submit := func(from, pattern string, form url.Values, code, text string) {
+	// the webhook page.
+	submit := func(from, pattern string, form url.Values) {
 		t.Helper()
 
 		form.Set("csrf_token", token)
 
 		w := env.post(env.urlFrom(t, from, pattern, cookies), form, cookies)
-		env.requireNotice(t, w, page, code, text, cookies)
+		require.Equal(t, http.StatusSeeOther, w.Code)
+		require.Equal(t, page, w.Header().Get("Location"))
 	}
 
-	toggle := `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`
-
 	submit(page, `action="(/hook/[^/"]+/targets)"`, url.Values{
 		"name": {"added"},
 		"type": {string(database.TargetTypeLog)},
-	}, "target-added", "Target added.")
+	})
 
 	editPage := env.urlFrom(
 		t, page, `href="(/hook/[^/"]+/targets/[^/"]+/edit)"`, cookies,
 	)
 	submit(editPage, `action="(/hook/[^/"]+/targets/[^/"]+/edit)"`,
-		url.Values{"name": {"renamed"}}, "target-saved", "Target saved.")
+		url.Values{"name": {"renamed"}})
 
 	var edited database.Target
 
@@ -1068,8 +1017,8 @@ func TestHook_TargetActions(t *testing.T) {
 	assert.Equal(t, "renamed", edited.Name)
 	require.True(t, edited.Active)
 
-	submit(page, toggle, url.Values{},
-		"target-deactivated", "Target deactivated.")
+	submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`,
+		url.Values{})
 
 	var toggled database.Target
 
@@ -1078,11 +1027,8 @@ func TestHook_TargetActions(t *testing.T) {
 	)
 	assert.False(t, toggled.Active, "the toggle should deactivate it")
 
-	submit(page, toggle, url.Values{},
-		"target-activated", "Target activated.")
-
 	submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/delete)"`,
-		url.Values{}, "target-deleted", "Target deleted.")
+		url.Values{})
 
 	var left int64
 
@@ -1118,9 +1064,9 @@ func TestHook_ResubmitFromEventLog(t *testing.T) {
 		env.urlFrom(t, logsPath, `action="(/hook/[^"]+/resubmit)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(
-		t, w, logsPath, "resubmit-no-targets",
-		"this source has no active targets", cookies,
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, logsPath+"?resubmit=no-targets", w.Header().Get("Location"),
 	)
 
 	webhookDB, err := env.dbMgr.GetDB(wh.ID)
@@ -1356,8 +1302,10 @@ func TestDeliveryReplay_PostOnlyAndCSRFProtected(t *testing.T) {
 		html.UnescapeString(action[1]), form, cookies,
 	)
 
-	env.requireNotice(
-		t, w, logsPath, "replay-queued", "Replay queued:", cookies,
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, logsPath+"?replay=queued",
+		w.Header().Get("Location"),
 	)
 	assert.Equal(
 		t, int64(2), env.countDeliveries(t, wh.ID),

script/test

@@ -28,12 +28,10 @@
 # -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.
 #
-# 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.
+# 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.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -41,34 +39,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 main() {
     cd "$ROOT"
     "$ROOT/script/assets"
-
-    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
+    go test -race -p 4 -parallel 8 -timeout 90s ./...
 }
 
 main "$@"

templates/base.html

@@ -7,7 +7,6 @@
 <body class="bg-gray-50 min-h-screen flex flex-col">
     <div class="flex-grow">
         {{template "navbar" .}}
-        {{template "notice" .}}
         {{block "content" .}}{{end}}
     </div>
     {{template "footer" .}}

templates/notice.html

@@ -1,7 +0,0 @@
-{{define "notice"}}
-{{with .Notice}}
-<div class="max-w-6xl mx-auto px-6 pt-4">
-    <div class="{{if .Failed}}alert-error{{else}}alert-success{{end}}">{{.Text}}</div>
-</div>
-{{end}}
-{{end}}

templates/source_logs.html

@@ -12,6 +12,14 @@
         </div>
     </div>
 
+    {{if .ReplayMessage}}
+    <div class="{{if .ReplayQueued}}alert-success{{else}}alert-error{{end}}">{{.ReplayMessage}}</div>
+    {{end}}
+
+    {{if .ResubmitMessage}}
+    <div class="{{if .ResubmitQueued}}alert-success{{else}}alert-error{{end}}">{{.ResubmitMessage}}</div>
+    {{end}}
+
     <div class="card">
         <div class="divide-y divide-gray-100">
             {{range .Events}}