Show each webhook's activity in the webhook list (closes #394)

Each entry in the webhook list now shows when the webhook's last event
arrived, or "No events yet", and how many of its deliveries failed in
the last 24 hours, in red when that is not zero. The entrypoint and
target counts say how many are inactive.

The figures come from the statistics pane's own reads: the event totals
row, which now also gives the event count instead of counting every
stored event, and the pane's query over the deliveries finished in the
last 24 hours. A webhook whose event database cannot be read says so in
its entry rather than showing zeros.

Model: opus-5-5

internal/delivery/export_test.go

@@ -40,6 +40,11 @@ const (
 	ExportPendingSweepMinAge = pendingSweepMinAge
 )
 
+// ExportIsBlockedIP exposes isBlockedIP for testing.
+func ExportIsBlockedIP(ip net.IP) bool {
+	return isBlockedIP(ip)
+}
+
 // NewTestGuard builds an SSRF Guard from an explicit egress
 // allowlist, without going through config. Passing no prefixes
 // yields the default guard, which blocks every private/reserved
@@ -65,11 +70,6 @@ func ExportBlockedNetworks() []*net.IPNet {
 	return blockedNetworks
 }
 
-// ExportBlockedPublicNetworks exposes blockedPublicNetworks.
-func ExportBlockedPublicNetworks() []*net.IPNet {
-	return blockedPublicNetworks
-}
-
 // ExportIsForwardableHeader exposes isForwardableHeader.
 func ExportIsForwardableHeader(name string) bool {
 	return isForwardableHeader(name)

internal/delivery/ssrf.go

@@ -25,16 +25,8 @@ var (
 	errNoIPs      = errors.New(
 		"hostname resolved to no IP addresses",
 	)
-	// ErrBlockedPrivateOrReservedIP reports an address in the
+	errBlockedIP = errors.New(
-	// default blocklist's private and reserved ranges,
+		"blocked private, reserved or cloud metadata address",
-	// blockedNetworks.
-	ErrBlockedPrivateOrReservedIP = errors.New(
-		"blocked private or reserved address",
-	)
-	// errBlockedPublicMetadata reports a public address on the
-	// default blocklist, one in blockedPublicNetworks.
-	errBlockedPublicMetadata = errors.New(
-		"blocked cloud metadata address",
 	)
 	errBlockedMetadata = errors.New(
 		"blocked link-local or cloud instance metadata " +
@@ -45,31 +37,21 @@ var (
 	)
 )
 
-// blockedNetworks and blockedPublicNetworks together are the
+// blockedNetworks is the default blocklist: the private and
-// default blocklist: the private and reserved IP ranges, plus
+// reserved IP ranges, plus the public cloud metadata addresses,
-// the public cloud metadata addresses, that are blocked to
+// that are blocked to prevent SSRF attacks. An operator can
-// prevent SSRF attacks. An operator can permit specific blocks
+// permit specific blocks out of this set with
-// out of this set with ALLOWED_EGRESS_CIDRS; see Guard.
+// ALLOWED_EGRESS_CIDRS; see Guard.
-//
-// blockedNetworks holds the private and reserved IP ranges.
-//
-//nolint:gochecknoglobals // package-level network list is appropriate here
-var blockedNetworks []*net.IPNet
-
-// blockedPublicNetworks holds the default blocklist's public
-// addresses, kept apart from blockedNetworks so that they are
-// refused as cloud metadata addresses, never as private or
-// reserved ones.
 //
 // 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; it goes
+// can reach it, without the caller presenting anything. A
-// in this list. A provider's other public addresses are not
+// provider's other public addresses are not refused, since
-// refused, since reaching them can be legitimate and no list of
+// reaching them can be legitimate and no list of them could be
-// them could be complete.
+// complete.
 //
 //nolint:gochecknoglobals // package-level network list is appropriate here
-var blockedPublicNetworks []*net.IPNet
+var blockedNetworks []*net.IPNet
 
 // alwaysBlockedNetworks are the ranges no configuration can
 // open: the link-local blocks and the cloud instance metadata
@@ -106,8 +88,8 @@ var blockedPublicNetworks []*net.IPNet
 // when it clears both halves. Nothing in this list can be
 // reopened, so putting a public address here leaves the operator
 // no escape hatch at all — the condition ALLOWED_EGRESS_CIDRS
-// exists to remove. Default-block it in blockedPublicNetworks
+// exists to remove. Default-block it in blockedNetworks instead,
-// instead, which an allowlist can override.
+// which an allowlist can override.
 //
 // This is a criterion, not an enumeration of every metadata
 // address in existence.
@@ -148,9 +130,6 @@ func init() {
 		"::1/128",
 		"fc00::/7",
 		"fe80::/10",
-	})
-
-	blockedPublicNetworks = mustParseCIDRs([]string{
 		// Azure WireServer, a public address that serves VM credentials.
 		"168.63.129.16/32",
 	})
@@ -246,6 +225,13 @@ func matchesAny(networks []*net.IPNet, ip net.IP) bool {
 	return false
 }
 
+// isBlockedIP checks whether an IP address falls within
+// the default blocklist, before any operator allowlist is
+// considered.
+func isBlockedIP(ip net.IP) bool {
+	return matchesAny(blockedNetworks, ip)
+}
+
 // Guard makes every SSRF decision in the process.
 //
 // It holds the operator's ALLOWED_EGRESS_CIDRS allowlist and
@@ -346,8 +332,7 @@ func (g *Guard) allows(ip net.IP) bool {
 //     consulted, so no configured CIDR reaches link-local or a
 //     cloud metadata endpoint at a non-public address.
 //  2. The allowlist is consulted next, so a listed private
-//     network, or a listed public address on the default
+//     network becomes reachable.
-//     blocklist, becomes reachable.
 //  3. Everything else keeps the default blocklist's answer.
 func (g *Guard) checkIP(ip net.IP) error {
 	if matchesAny(alwaysBlockedNetworks, ip) {
@@ -360,15 +345,9 @@ func (g *Guard) checkIP(ip net.IP) error {
 		return nil
 	}
 
-	if matchesAny(blockedNetworks, ip) {
+	if isBlockedIP(ip) {
 		return fmt.Errorf(
-			"target IP %s: %w", ip, ErrBlockedPrivateOrReservedIP,
+			"target IP %s: %w", ip, errBlockedIP,
-		)
-	}
-
-	if matchesAny(blockedPublicNetworks, ip) {
-		return fmt.Errorf(
-			"target IP %s: %w", ip, errBlockedPublicMetadata,
 		)
 	}
 

internal/delivery/ssrf_allowlist_test.go

@@ -23,10 +23,6 @@ const (
 	metadataIP  = "169.254.169.254"
 	metadataURL = "http://" + metadataIP + "/latest/meta-data/"
 
-	// linkLocalIPv4 is the IPv4 link-local block, which holds
-	// metadataIP.
-	linkLocalIPv4 = "169.254.0.0/16"
-
 	// loopbackHookURL is a target on this host: blocked by
 	// default, reachable only once an operator allowlists
 	// loopback.
@@ -241,7 +237,7 @@ func linkLocalRefusedCases() []metadataAlwaysRefusedCase {
 		},
 		{
 			name:   "whole link-local block",
-			allow:  linkLocalIPv4,
+			allow:  "169.254.0.0/16",
 			target: metadataURL,
 		},
 		{
@@ -416,9 +412,6 @@ func TestGuardAllowlist_AzureWireServerReopenable(t *testing.T) {
 		"WireServer must be refused by the default blocklist, "+
 			"which an allowlist can override",
 	)
-	require.NotErrorIs(t, err, delivery.ErrBlockedPrivateOrReservedIP,
-		"WireServer is public, not private or reserved",
-	)
 
 	assertDialRefused(t, defaultGuard, target)
 
@@ -503,7 +496,7 @@ func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) {
 	want := []string{
 		// IPv4 link-local: the 169.254.169.254 metadata
 		// service on AWS, Azure and others.
-		linkLocalIPv4,
+		"169.254.0.0/16",
 		// IPv6 link-local.
 		"fe80::/10",
 		// AWS IPv6 IMDS, inside the ULA space an operator may
@@ -533,90 +526,6 @@ func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) {
 	assert.Equal(t, want, got)
 }
 
-// TestDefaultBlocklist_PinnedSet pins each list of the default
-// blocklist on its own, the private and reserved ranges in
-// blockedNetworks and the public addresses in
-// blockedPublicNetworks, so moving an entry from one list to the
-// other fails it. For the first address of each entry it then
-// checks that the default guard refuses it, and that listing the
-// entry in ALLOWED_EGRESS_CIDRS opens it unless the unconditional
-// set holds that address.
-func TestDefaultBlocklist_PinnedSet(t *testing.T) {
-	t.Parallel()
-
-	// public marks an entry of blockedPublicNetworks; every other
-	// entry belongs in blockedNetworks.
-	tests := []struct {
-		cidr       string
-		public     bool
-		reopenable bool
-	}{
-		{cidr: "127.0.0.0/8", reopenable: true},
-		{cidr: "10.0.0.0/8", reopenable: true},
-		{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},
-		{cidr: "100.64.0.0/10", reopenable: true},
-		{cidr: "192.0.0.0/24", reopenable: true},
-		{cidr: "192.0.2.0/24", reopenable: true},
-		{cidr: "198.18.0.0/15", reopenable: true},
-		{cidr: "198.51.100.0/24", reopenable: true},
-		{cidr: "203.0.113.0/24", reopenable: true},
-		{cidr: "224.0.0.0/4", reopenable: true},
-		{cidr: "240.0.0.0/4", reopenable: true},
-		{cidr: "::1/128", reopenable: true},
-		{cidr: "fc00::/7", reopenable: true},
-		{cidr: "fe80::/10", reopenable: false},
-		{cidr: "168.63.129.16/32", public: true, reopenable: true},
-	}
-
-	wantPrivate := make([]string, 0, len(tests))
-	wantPublic := make([]string, 0, len(tests))
-
-	for _, tt := range tests {
-		if tt.public {
-			wantPublic = append(wantPublic, tt.cidr)
-		} else {
-			wantPrivate = append(wantPrivate, tt.cidr)
-		}
-	}
-
-	gotPrivate := make([]string, 0, len(tests))
-	for _, n := range delivery.ExportBlockedNetworks() {
-		gotPrivate = append(gotPrivate, n.String())
-	}
-
-	gotPublic := make([]string, 0, len(tests))
-	for _, n := range delivery.ExportBlockedPublicNetworks() {
-		gotPublic = append(gotPublic, n.String())
-	}
-
-	assert.ElementsMatch(t, wantPrivate, gotPrivate, "blockedNetworks")
-	assert.ElementsMatch(t, wantPublic, gotPublic, "blockedPublicNetworks")
-
-	for _, tt := range tests {
-		t.Run(tt.cidr, func(t *testing.T) {
-			t.Parallel()
-
-			prefix := netip.MustParsePrefix(tt.cidr)
-			ip := net.IP(prefix.Addr().AsSlice())
-
-			require.Error(t,
-				delivery.NewTestGuard().ExportCheckIP(ip),
-				"the default guard must refuse %s", ip,
-			)
-
-			err := delivery.NewTestGuard(prefix).ExportCheckIP(ip)
-			if tt.reopenable {
-				assert.NoError(t, err, "listing %s must open it", tt.cidr)
-			} else {
-				assert.Error(t, err, "listing %s must not open it", tt.cidr)
-			}
-		})
-	}
-}
-
 // requireLoopback fails the test unless rawURL's host is a
 // loopback address, so the allowlist test cannot silently stop
 // exercising a blocked range.

internal/delivery/ssrf_test.go

@@ -10,7 +10,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-func TestGuardCheckIP_PrivateRanges(t *testing.T) {
+func TestIsBlockedIP_PrivateRanges(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -56,14 +56,12 @@ func TestGuardCheckIP_PrivateRanges(t *testing.T) {
 				"failed to parse IP %s", tt.ip,
 			)
 
-			refused := delivery.NewTestGuard().ExportCheckIP(ip) != nil
-
 			assert.Equal(t,
 				tt.blocked,
-				refused,
+				delivery.ExportIsBlockedIP(ip),
-				"default guard refuses %s = %v, want %v",
+				"isBlockedIP(%s) = %v, want %v",
 				tt.ip,
-				refused,
+				delivery.ExportIsBlockedIP(ip),
 				tt.blocked,
 			)
 		})

internal/handlers/source_list_test.go

@@ -14,7 +14,6 @@ import (
 	"gorm.io/gorm/clause"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/logger"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
@@ -175,8 +174,8 @@ func lastEventText(ev database.Event) string {
 }
 
 // TestSourceList_ShowsActivityOfEachWebhook checks the figures the list
-// shows for a webhook with recent failures, a healthy one, a new one
+// shows for a webhook with recent failures, a healthy one, and one that
-// that has received no event, and one without an event database.
+// has never received an event.
 func TestSourceList_ShowsActivityOfEachWebhook(t *testing.T) {
 	t.Parallel()
 
@@ -194,11 +193,6 @@ func TestSourceList_ShowsActivityOfEachWebhook(t *testing.T) {
 
 	failing, failingNewest := seedFailingWebhook(t, h, db, dbMgr)
 	healthy, healthyNewest := seedHealthyWebhook(t, h, db, dbMgr)
-
-	// Creating a webhook creates its event database.
-	fresh := listWebhook(t, db, "fresh")
-	require.NoError(t, dbMgr.CreateDB(fresh.ID))
-
 	quiet := listWebhook(t, db, "quiet")
 
 	page := renderWebhookList(t, h, sess)
@@ -206,7 +200,7 @@ func TestSourceList_ShowsActivityOfEachWebhook(t *testing.T) {
 	card, text := listCard(t, page, failing.ID)
 	assert.Contains(t, text, "2 entrypoints, 1 inactive "+
 		"4 targets, 1 inactive "+
-		"3 events within retention "+
+		"3 events "+
 		"Last event "+lastEventText(failingNewest)+" "+
 		"2 failed deliveries in the last 24 hours")
 	assert.Contains(t, card,
@@ -215,24 +209,16 @@ func TestSourceList_ShowsActivityOfEachWebhook(t *testing.T) {
 	card, text = listCard(t, page, healthy.ID)
 	assert.Contains(t, text, "1 entrypoint "+
 		"1 target "+
-		"2 events within retention "+
+		"2 events "+
 		"Last event "+lastEventText(healthyNewest)+" "+
 		"0 failed deliveries in the last 24 hours")
 	assert.NotContains(t, text, "inactive")
 	assert.NotContains(t, card, failedHighlight)
 
-	card, text = listCard(t, page, fresh.ID)
-	assert.Contains(t, text, "0 entrypoints "+
-		"0 targets "+
-		"0 events within retention "+
-		"No events yet "+
-		"0 failed deliveries in the last 24 hours")
-	assert.NotContains(t, card, failedHighlight)
-
 	card, text = listCard(t, page, quiet.ID)
 	assert.Contains(t, text, "0 entrypoints "+
 		"0 targets "+
-		"0 events within retention "+
+		"0 events "+
 		"No events yet "+
 		"0 failed deliveries in the last 24 hours")
 	assert.NotContains(t, card, failedHighlight)
@@ -240,42 +226,6 @@ func TestSourceList_ShowsActivityOfEachWebhook(t *testing.T) {
 		"showing the list must not create an event database")
 }
 
-// TestSourceList_CountsOnlyEventsWithinRetention checks that once
-// retention has removed one of a webhook's three events, the list
-// counts the two still stored.
-func TestSourceList_CountsOnlyEventsWithinRetention(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-		log   *logger.Logger
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr, &log)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := &database.Webhook{
-		UserID: deleteTestUserID, Name: "pruned", RetentionDays: 14,
-	}
-	require.NoError(t, db.DB().Omit(clause.Associations).Create(wh).Error)
-
-	path := statsEntrypoint(t, db, wh.ID, true)
-	webhookDB, events := receiveEvents(t, h, dbMgr, wh.ID, path, 3)
-
-	statsAge(t, webhookDB, events[0].ID, time.Now().Add(-15*24*time.Hour))
-	statsPrune(t, db, dbMgr, log, webhookDB)
-	require.Len(t, listEvents(t, webhookDB), 2)
-
-	_, text := listCard(t, renderWebhookList(t, h, sess), wh.ID)
-	assert.Contains(t, text,
-		"1 entrypoint 0 targets 2 events within retention")
-}
-
 // TestSourceList_UnreadableEventDatabase checks that a webhook whose
 // event database cannot be read says so in its entry instead of
 // showing zeros, and that the rest of the list is still shown.

internal/handlers/source_management.go

@@ -1681,22 +1681,11 @@ func (h *Handlers) validateTargetURL(
 			"url", delivery.MaskURL(targetURL),
 			"error", err,
 		)
-
+		http.Error(
-		msg := "Invalid target URL: " + err.Error()
+			w,
-
+			"Invalid target URL: "+err.Error(),
-		// Only a private or reserved address's refusal says how
+			http.StatusBadRequest,
-		// 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.
-		if errors.Is(err, delivery.ErrBlockedPrivateOrReservedIP) {
-			msg += ". Private and reserved addresses are refused " +
-				"by default; the server's ALLOWED_EGRESS_CIDRS " +
-				"setting allows named networks (see \"Allowing " +
-				"egress to your own network\" in the README)."
-		}
-
-		http.Error(w, msg, http.StatusBadRequest)
 
 		return err
 	}

internal/handlers/target_private_refusal_test.go

@@ -1,116 +0,0 @@
-package handlers_test
-
-import (
-	"net/http"
-	"net/url"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// privateRefusalHint is the sentence that tells an operator a private
-// destination is refused on purpose, and how to allow one.
-const privateRefusalHint = "Private and reserved addresses are " +
-	"refused by default; the server's ALLOWED_EGRESS_CIDRS setting " +
-	"allows named networks (see \"Allowing egress to your own " +
-	"network\" in the README)."
-
-// TestTargetRefusal_PrivateDestinationSaysHowToAllowIt covers both
-// target types that take a URL, on add and on edit.
-func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	targetTypes := []database.TargetType{
-		database.TargetTypeHTTP,
-		database.TargetTypeSlack,
-	}
-
-	for _, targetType := range targetTypes {
-		t.Run(string(targetType), func(t *testing.T) {
-			t.Parallel()
-
-			webhook := seedWebhookWithRetention(t, env.db, 30)
-			targetsPath := "/hook/" + webhook.ID + "/targets"
-
-			form := url.Values{}
-			form.Set("name", "private")
-			form.Set("type", string(targetType))
-			form.Set("url", editBlockedURL)
-
-			added := serveTarget(
-				env, http.MethodPost, targetsPath, form,
-			)
-			assert.Equal(t, http.StatusBadRequest, added.Code)
-			assert.Contains(
-				t, added.Body.String(), privateRefusalHint,
-			)
-
-			form.Set("url", editOriginalURL)
-
-			created := serveTarget(
-				env, http.MethodPost, targetsPath, form,
-			)
-			require.Equal(
-				t, http.StatusSeeOther, created.Code,
-				created.Body.String(),
-			)
-
-			targets := targetsForWebhook(t, env.db, webhook.ID)
-			require.Len(t, targets, 1)
-
-			form.Set("url", editBlockedURL)
-
-			edited := submitTargetEdit(
-				env, webhook.ID, targets[0].ID, form,
-			)
-			assert.Equal(t, http.StatusBadRequest, edited.Code)
-			assert.Contains(
-				t, edited.Body.String(), privateRefusalHint,
-			)
-		})
-	}
-}
-
-// TestTargetRefusal_MetadataDestinationDoesNotSayHowToAllowIt: no
-// setting opens a link-local address, and Azure's WireServer hands out
-// VM credentials, so neither refusal points at the setting.
-func TestTargetRefusal_MetadataDestinationDoesNotSayHowToAllowIt(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	metadataURLs := map[string]string{
-		"link-local": "http://169.254.169.254/latest/meta-data/",
-		"wireserver": "http://168.63.129.16/?comp=versions",
-	}
-
-	for name, metadataURL := range metadataURLs {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			webhook := seedWebhookWithRetention(t, env.db, 30)
-
-			form := url.Values{}
-			form.Set("name", "metadata")
-			form.Set("type", string(database.TargetTypeHTTP))
-			form.Set("url", metadataURL)
-
-			w := serveTarget(
-				env, http.MethodPost,
-				"/hook/"+webhook.ID+"/targets", form,
-			)
-			assert.Equal(t, http.StatusBadRequest, w.Code)
-			assert.NotContains(
-				t, w.Body.String(), privateRefusalHint,
-			)
-		})
-	}
-}

templates/sources_list.html

@@ -33,7 +33,7 @@
                 {{if .EventsUnreadable}}
                 <span class="text-red-600">The event figures could not be read.</span>
                 {{else}}
-                <span>{{.EventCount}} event{{if ne .EventCount 1}}s{{end}} within retention</span>
+                <span>{{.EventCount}} event{{if ne .EventCount 1}}s{{end}}</span>
                 <span>{{with .LastEventAt}}Last event {{.UTC.Format "2006-01-02 15:04:05 UTC"}}{{else}}No events yet{{end}}</span>
                 <span class="{{if .FailedLast24Hours}}font-medium text-red-600{{end}}">{{.FailedLast24Hours}} failed deliver{{if eq .FailedLast24Hours 1}}y{{else}}ies{{end}} in the last 24 hours</span>
                 {{end}}