server: report HTTP handler panics to Sentry (closes #107)

DNSWATCHER_SENTRY_DSN was read but never used. This ports the Sentry
integration from gohttpserver with sentry-go v0.49.0. The server's start
hook calls sentry.Init when the DSN is set; a DSN Sentry cannot parse
fails the hook, so startup stops with the parse error. sentryhttp, with
Repanic, reports handler panics and passes them on to chi's Recoverer.
Shutdown sends queued reports once the HTTP server has stopped. The
client uses the older transport (DisableTelemetryBuffer): with the
default one, Flush can return before sending a report made just before
it. Client reports are off, so only panics are sent. The DSN is checked
at server start, not in config, so the config test is unchanged.
sentry-go raises several golang.org/x modules and moves go-spew and
go-difflib to untagged commits.

Model: opus-5-5

README.md

@@ -347,9 +347,9 @@ minute, failed logins included; beyond that it answers `429 Too Many Requests`
 without checking the password. A Prometheus server scraping every 15 seconds
 sends 4 a minute. IPv6 addresses in one /64 count as one client. When the
 request comes from a private or loopback address, such as a reverse proxy's,
-the client address is taken from the `X-Real-IP` or `X-Forwarded-For` header
-the proxy sets; a proxy that sets neither makes all its clients share one
-allowance.
+the client address is taken from the `X-Real-IP` header the proxy sets, or else
+from `X-Forwarded-For`, as the last address in it that is not private or
+loopback. A proxy that sets neither makes all its clients share one allowance.
 
 **`DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL`** take a positive
 duration: a number followed by a unit such as `s`, `m` or `h`, for example
@@ -625,7 +625,8 @@ repository's `Dockerfile` and runs it. The app needs:
    abandoned, and the number abandoned is logged at warn level rather
    than dropped silently. Notifications generated after shutdown has
    begun are refused and logged, so a late burst cannot extend the
-   shutdown.
+   shutdown. A DNS lookup, port check or TLS check that shutdown cuts
+   short saves nothing and sends no notification.
 
 ---
 

TODO.md

@@ -21,6 +21,10 @@ nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105
 
 - 2026-10-01: `DNSWATCHER_SENTRY_DSN` reports panics in HTTP handlers to Sentry,
   and a DSN Sentry cannot parse stops startup (closes #107).
+- 2026-10-01: a port or TLS check that shutdown cuts short saves nothing and
+  sends no notification, as a cut-short DNS lookup already did (closes #185).
+- 2026-10-01: the client address from `X-Forwarded-For` is the last entry that
+  is not a trusted proxy, not the first, which the client sets (closes #181).
 - 2026-10-01: a nameserver that does not answer is saved as `error` with the
   reason, and NS failure and NS recovery are notified (closes #104).
 - 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is

internal/middleware/export_test.go

@@ -1,6 +1,9 @@
 package middleware
 
-import "time"
+import (
+	"net/http"
+	"time"
+)
 
 // The /metrics rate limit, exported so the tests can count requests
 // against it.
@@ -8,3 +11,9 @@ const (
 	MetricsRequestLimit                = metricsRequestLimit
 	MetricsRequestWindow time.Duration = metricsRequestWindow
 )
+
+// RealIP is realIP, exported so the tests can check which address it
+// takes as the client's.
+func RealIP(r *http.Request) string {
+	return realIP(r)
+}

internal/middleware/middleware.go

@@ -209,6 +209,12 @@ func isTrustedProxy(ip net.IP) bool {
 
 // realIP extracts the client's real IP address from the request.
 // Proxy headers are only trusted from RFC1918/loopback addresses.
+//
+// Each proxy adds to the end of X-Forwarded-For the address it got the
+// request from, so the client can write every entry before the one the
+// first trusted proxy added. The client address is therefore the
+// rightmost entry that is not a trusted proxy, or the leftmost entry
+// when they all are.
 func realIP(r *http.Request) string {
 	addr := ipFromHostPort(r.RemoteAddr)
 	remoteIP := net.ParseIP(addr)
@@ -223,14 +229,24 @@ func realIP(r *http.Request) string {
 		return ip
 	}
 
-	if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
-		if parts := strings.SplitN(
-			xff, ",", 2, //nolint:mnd
-		); len(parts) > 0 {
-			if ip := strings.TrimSpace(parts[0]); ip != "" {
-				return ip
+	// A proxy may add its entry as a header line of its own instead of
+	// appending to the line the client sent, so all lines form one list.
+	entries := strings.Split(
+		strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
+	)
+	client := strings.TrimSpace(entries[0])
+
+	for i := len(entries) - 1; i > 0; i-- {
+		entry := strings.TrimSpace(entries[i])
+		if !isTrustedProxy(net.ParseIP(entry)) {
+			client = entry
+
+			break
 		}
 	}
+
+	if client != "" {
+		return client
 	}
 
 	return addr

internal/middleware/middleware_test.go

@@ -342,9 +342,9 @@ func TestDashboardRendersWithSecurityHeaders(t *testing.T) {
 	}
 }
 
-// Addresses for the rate limit tests: a client connecting directly, a
-// trusted proxy, and a client behind that proxy as its X-Real-IP
-// header names it.
+// Addresses for the rate limit and realIP tests: a client connecting
+// directly, a trusted proxy, and a client behind that proxy as the
+// proxy's X-Real-IP or X-Forwarded-For header names it.
 const (
 	directClient  = "198.51.100.1:4000"
 	trustedProxy  = "10.0.0.1:4000"
@@ -482,3 +482,84 @@ func TestMetricsRateLimitKeysOnClientAddress(t *testing.T) {
 		})
 	}
 }
+
+// TestRealIP checks which address realIP takes as the client's. Each
+// element of forwardedFor is sent as an X-Forwarded-For header line of
+// its own, and 198.51.100.9 is always an entry the client wrote itself.
+func TestRealIP(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name         string
+		remoteAddr   string
+		xRealIP      string
+		forwardedFor []string
+		want         string
+	}{
+		{
+			"untrusted peer, both headers ignored",
+			directClient, proxiedClient, []string{"198.51.100.9"},
+			"198.51.100.1",
+		},
+		{
+			"X-Real-IP from a trusted proxy wins",
+			trustedProxy, proxiedClient, []string{"203.0.113.8"},
+			proxiedClient,
+		},
+		{
+			"client's own entry, then the one the proxy added",
+			trustedProxy, "", []string{"198.51.100.9, 203.0.113.1"},
+			proxiedClient,
+		},
+		{
+			"several trusted proxies",
+			trustedProxy, "",
+			[]string{"198.51.100.9, 203.0.113.1, 10.0.0.3, 10.0.0.2"},
+			proxiedClient,
+		},
+		{
+			"proxy adds a header line of its own",
+			trustedProxy, "", []string{"198.51.100.9", proxiedClient},
+			proxiedClient,
+		},
+		{
+			"every entry a trusted proxy",
+			trustedProxy, "", []string{"10.0.0.3, 10.0.0.2"},
+			"10.0.0.3",
+		},
+		{
+			"empty where the client address belongs",
+			trustedProxy, "", []string{"203.0.113.1, , 10.0.0.2"},
+			"10.0.0.1",
+		},
+		{
+			"no headers from a trusted proxy",
+			trustedProxy, "", nil,
+			"10.0.0.1",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			req := httptest.NewRequestWithContext(
+				t.Context(), http.MethodGet, "/", nil,
+			)
+			req.RemoteAddr = tt.remoteAddr
+
+			if tt.xRealIP != "" {
+				req.Header.Set("X-Real-IP", tt.xRealIP)
+			}
+
+			for _, line := range tt.forwardedFor {
+				req.Header.Add("X-Forwarded-For", line)
+			}
+
+			got := middleware.RealIP(req)
+			if got != tt.want {
+				t.Errorf("realIP = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}

internal/server/sentry_test.go

@@ -106,7 +106,8 @@ func TestSentryUnsetDoesNothing(t *testing.T) {
 
 // TestSentryReportsHandlerPanic checks that with a valid DSN a panic in
 // a handler is reported to Sentry, still reaches chimw.Recoverer, and
-// has been sent by the time Shutdown returns.
+// has been sent by the time Shutdown returns, and that nothing else is
+// sent to Sentry.
 func TestSentryReportsHandlerPanic(t *testing.T) {
 	standIn := newSentryStandIn(t)
 
@@ -137,6 +138,12 @@ func TestSentryReportsHandlerPanic(t *testing.T) {
 		},
 	)
 
+	// An ordinary request first: with client reports on, Sentry would
+	// add a count of its dropped transaction to the panic report.
+	serve(srv, httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/.well-known/healthcheck", nil,
+	))
+
 	rec := serve(srv, httptest.NewRequestWithContext(
 		t.Context(), http.MethodGet, "/panic", nil,
 	))
@@ -155,6 +162,10 @@ func TestSentryReportsHandlerPanic(t *testing.T) {
 	if !standIn.received(panicMessage) {
 		t.Error("the panic had not been sent to Sentry when Shutdown returned")
 	}
+
+	if standIn.received("client_report") {
+		t.Error("Sentry was sent a client report, not only the panic")
+	}
 }
 
 func TestSentryInvalidDSNStopsStartup(t *testing.T) {

internal/server/server.go

@@ -216,6 +216,9 @@ func (s *Server) enableSentry() error {
 		// the default one, Flush can return before sending a report made
 		// just before it, such as one from the last request at shutdown.
 		DisableTelemetryBuffer: true,
+		// Send panic reports only, not Sentry's counts of what it dropped,
+		// such as the transaction it starts for every request.
+		DisableClientReports: true,
 	})
 	if err != nil {
 		return fmt.Errorf("invalid DNSWATCHER_SENTRY_DSN: %w", err)

internal/watcher/cancelled_test.go

@@ -0,0 +1,81 @@
+package watcher_test
+
+import (
+	"context"
+	"log/slog"
+	"reflect"
+	"testing"
+
+	"sneak.berlin/go/dnswatcher/internal/portcheck"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
+	"sneak.berlin/go/dnswatcher/internal/state"
+	"sneak.berlin/go/dnswatcher/internal/tlscheck"
+	"sneak.berlin/go/dnswatcher/internal/watcher"
+)
+
+// TestCancelledCheckSavesNothing runs a check with its context already
+// cancelled, which is how the rest of a check runs once shutdown cuts it
+// short. The real resolver drops the DNS lookup without sending a query,
+// and the real port and TLS checkers fail without connecting. The port
+// and certificate state the last check saved must stay as it was, and
+// nothing may be notified.
+func TestCancelledCheckSavesNothing(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaultTestConfig(t)
+	cfg.Hostnames = []string{host}
+
+	// newTestWatcher's watcher has stand-in checkers. This one, on the
+	// same state and notifier, has the real ones.
+	_, deps := newTestWatcher(t, cfg)
+	w := watcher.NewForTest(
+		cfg,
+		deps.state,
+		resolver.NewFromLogger(slog.Default()),
+		portcheck.NewStandalone(),
+		tlscheck.NewStandalone(),
+		deps.notifier,
+	)
+
+	// The last check found host at a local address, with both ports
+	// open and a good certificate.
+	const localIP = "127.0.0.1"
+
+	deps.state.SetHostnameState(host, hostnameState(
+		map[string]map[string][]string{nsA: {"A": {localIP}}},
+	))
+
+	ports := map[string]*state.PortState{
+		localIP + ":80":  {Open: true, Hostnames: []string{host}},
+		localIP + ":443": {Open: true, Hostnames: []string{host}},
+	}
+	for key, ps := range ports {
+		deps.state.SetPortState(key, ps)
+	}
+
+	certKey := localIP + ":443:" + host
+	cert := &state.CertificateState{CommonName: host, Status: "ok"}
+	deps.state.SetCertificateState(certKey, cert)
+
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel()
+
+	w.RunOnce(ctx)
+
+	for key, want := range ports {
+		got, _ := deps.state.GetPortState(key)
+		if !reflect.DeepEqual(got, want) {
+			t.Errorf("port %s saved as %+v, want %+v", key, got, want)
+		}
+	}
+
+	got, _ := deps.state.GetCertificateState(certKey)
+	if !reflect.DeepEqual(got, cert) {
+		t.Errorf("certificate saved as %+v, want %+v", got, cert)
+	}
+
+	notifications := deps.notifier.getNotifications()
+	if len(notifications) != 0 {
+		t.Errorf("sent %v, want no notifications", notifications)
+	}
+}

internal/watcher/watcher.go

@@ -658,6 +658,13 @@ func (w *Watcher) checkSinglePort(
 	hostnames []string,
 ) {
 	result, err := w.portCheck.CheckPort(ctx, ip, port)
+
+	// A check the context cut short says nothing about the port, so it
+	// is neither saved nor notified.
+	if ctx.Err() != nil {
+		return
+	}
+
 	if err != nil {
 		w.log.Error(
 			"port check failed",
@@ -733,6 +740,13 @@ func (w *Watcher) checkTLSCert(
 	hostname string,
 ) {
 	cert, err := w.tlsCheck.CheckCertificate(ctx, ip, hostname)
+
+	// A check the context cut short says nothing about the certificate,
+	// so it is neither saved nor notified.
+	if ctx.Err() != nil {
+		return
+	}
+
 	certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname)
 	now := time.Now().UTC()
 	prev, hasPrev := w.state.GetCertificateState(certKey)