Say the query logger drops bound values (closes #535)

The comment on MaxAccessLogLineBytes said the GORM adapter logs each
statement with the client's parameter written into it. Its
ParamsFilter drops every bound value, so the statement is logged with
its placeholders. The comment now says so and keeps the parts that
still hold: the widest line spends two budgets and the adapter's test
checks it against this constant.

The assertBounded doc no longer claims each line was cut, and the
postReceiver doc no longer explains itself by a log size test that is
gone.

Model: opus-5-5

README.md

@@ -2494,9 +2494,6 @@ are both taken unconditionally, the body precisely because these handlers call
 body with `(redacted)`, drops cookies and the remote-address environment, and
 reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
 `Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and `X-Request-Id`.
-`Referer` is sent as its scheme and host only, because its path can be an
-entrypoint URL; `internal/referer.SchemeAndHost` does that reduction for both
-the access log's `referer` field and this hook.
 
 The same hook rewrites the request URL. The SDK builds it as
 `scheme://host/path` from the concrete path, which on the receiver route is
@@ -2567,9 +2564,8 @@ receiver route. What survives still names the failing route — scheme, host,
 route pattern, method — and `X-Request-Id` ties the event to the local access
 log line that holds the rest. Nothing dropped is needed for the likeliest use,
 debugging a CSRF rejection. Its three inputs are the TLS decision, `Origin` and
-`Referer`; the latter two are kept (`Referer` as its scheme and host, which is
-all the check compares), and the first is the scheme of the retained URL,
-because the SDK derives that scheme from
+`Referer`; the latter two are kept, and the first is the scheme of the retained
+URL, because the SDK derives that scheme from
 `r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"`. That predicate
 is the SDK's own and is stricter than `internal/reqtls.IsTLS`, which this
 service now uses everywhere it decides transport: the SDK reports `http` for the
@@ -3101,8 +3097,6 @@ webhooker/
 │   │   ├── ratelimit.go       # Per-IP rate limiting middleware (go-chi/httprate)
 │   │   ├── loginguard.go      # Login failure counters and the Argon2id verification semaphore
 │   │   └── middlewaretest/    # Test support: a Middleware for tests in other packages
-│   ├── referer/
-│   │   └── referer.go         # SchemeAndHost: cuts a Referer down to its scheme and host, for the access log and the Sentry hook
 │   ├── reqtls/
 │   │   └── reqtls.go          # IsTLS: the one TLS predicate, r.TLS or X-Forwarded-Proto
 │   ├── server/

internal/gormlog/gormlog_test.go

@@ -146,7 +146,7 @@ func openDB(
 }
 
 // assertBounded holds every line the adapter wrote to the stated
-// ceiling and proves each was cut rather than merely short.
+// ceiling and proves no line carried a whole client value.
 func assertBounded(t *testing.T, out string) {
 	t.Helper()
 

internal/handlers/logbound_test.go

@@ -191,13 +191,12 @@ func receiverRouter(h *handlers.Handlers) *chi.Mux {
 	return router
 }
 
-// postReceiver sends one POST at /h/<segment>.
+// postReceiver sends one POST with an empty body at /h/<segment>
+// through router and returns the response status.
 //
 // RawPath is cleared after parsing so chi routes on the decoded path
 // and the handler sees the raw bytes rather than their percent-escaped
-// spelling. That is the harder case for the budget: the escaped
-// spelling is plain ASCII, which costs one byte per byte, while the
-// decoded bytes are what the log handler has to escape.
+// spelling.
 func postReceiver(
 	t *testing.T, router *chi.Mux, segment string,
 ) int {

internal/middleware/middleware.go

@@ -22,7 +22,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/logfield"
-	"sneak.berlin/go/webhooker/internal/referer"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
@@ -49,6 +48,10 @@ const (
 	// pick the size of the line it writes.
 	redactedQuery = "?(redacted)"
 
+	// redactedReferer stands in for a Referer the access log cannot
+	// reduce to its scheme and host; see accessLogReferer.
+	redactedReferer = "(redacted)"
+
 	// maxLogRequestIDBytes bounds the request id, which is also
 	// client-supplied: chi's RequestID middleware passes an inbound
 	// X-Request-Id header through verbatim. Its generated form is an
@@ -115,13 +118,13 @@ const (
 	// a caller on a parameterised route would supply, which is the
 	// only way those caps can be pinned at all.
 	//
-	// One writer in that set is not a handler's own slog call. The
-	// GORM adapter in internal/gormlog logs the statement with the
-	// client-chosen parameter already interpolated into it, which on
-	// the receiver and login lookups is exactly the value the budgets
-	// above exist for. Its widest line spends two logfield.MaxBytes
-	// budgets — the statement and the driver error — against a fixed
-	// portion smaller than this one's, and
+	// The GORM adapter in internal/gormlog writes through slog as well,
+	// though not from a handler. It logs each statement with its
+	// placeholders rather than the values bound to them (see its
+	// ParamsFilter), so the key a client sends to the receiver or login
+	// lookup does not reach its lines. Its widest line spends two
+	// logfield.MaxBytes budgets — the statement and the driver error —
+	// against a fixed portion smaller than this one's, and
 	// internal/gormlog/gormlog_test.go asserts every line it emits
 	// against this constant directly rather than leaving it as
 	// arithmetic.
@@ -339,6 +342,27 @@ func accessLogURL(r *http.Request, status int) string {
 	return pattern
 }
 
+// accessLogReferer returns the value for the access log's referer
+// field: the Referer's scheme and host, never its path or query. The
+// path can be an entrypoint URL, which carries the UUID: a sender that
+// follows a redirect from http:// to https:// sends the URL it was
+// redirected from, and a browser sends the 404 page's URL under /h/
+// when it fetches that page's stylesheet. A Referer without both a
+// scheme and a host is logged as redactedReferer.
+func accessLogReferer(r *http.Request) string {
+	referer := r.Referer()
+	if referer == "" {
+		return ""
+	}
+
+	parsed, err := url.Parse(referer)
+	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+		return redactedReferer
+	}
+
+	return parsed.Scheme + "://" + parsed.Host
+}
+
 // Logging returns middleware that logs each HTTP request with
 // timing and metadata.
 func (s *Middleware) Logging() func(http.Handler) http.Handler {
@@ -389,8 +413,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
 						requestID, maxLogRequestIDBytes,
 					),
 					"referer", logfield.Truncate(
-						referer.SchemeAndHost(r.Referer()),
-						logfield.MaxBytes,
+						accessLogReferer(r), logfield.MaxBytes,
 					),
 					"proto", r.Proto,
 					"remoteIP", RemoteIP(r),

internal/referer/referer.go

@@ -1,32 +0,0 @@
-// Package referer reduces a Referer header to its scheme and host, so
-// that a URL path, which on the receiver route carries the entrypoint
-// UUID, never reaches a log line or an error report.
-package referer
-
-import "net/url"
-
-// redacted stands in for a Referer that cannot be reduced to its
-// scheme and host.
-const redacted = "(redacted)"
-
-// SchemeAndHost returns a Referer's scheme and host, never its path or
-// query. The path can be an entrypoint URL, which carries the UUID: a
-// sender that follows a redirect from http:// to https:// sends the
-// URL it was redirected from, and a browser sends the 404 page's URL
-// under /h/ when it fetches that page's stylesheet. A Referer without
-// both a scheme and a host comes back as "(redacted)".
-//
-// It is the access log's referer field, and the Referer header the
-// Sentry hook sends.
-func SchemeAndHost(referer string) string {
-	if referer == "" {
-		return ""
-	}
-
-	parsed, err := url.Parse(referer)
-	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
-		return redacted
-	}
-
-	return parsed.Scheme + "://" + parsed.Host
-}

internal/referer/referer_test.go

@@ -1,67 +0,0 @@
-package referer_test
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"sneak.berlin/go/webhooker/internal/referer"
-)
-
-// entrypointUUID is the secret part of an entrypoint URL; no case may
-// return it.
-const entrypointUUID = "0b6f7d3e-6a39-4e3c-9f0a-2c4d8e1b5a77"
-
-// redacted is what SchemeAndHost returns for a Referer it cannot
-// reduce to a scheme and host.
-const redacted = "(redacted)"
-
-func TestSchemeAndHost(t *testing.T) {
-	t.Parallel()
-
-	for _, tc := range []struct {
-		name    string
-		referer string
-		want    string
-	}{
-		{
-			name:    "no Referer",
-			referer: "",
-			want:    "",
-		},
-		{
-			name: "entrypoint URL with a query",
-			referer: "http://webhooker.example/h/" + entrypointUUID +
-				"?src=ci",
-			want: "http://webhooker.example",
-		},
-		{
-			name:    "host with a port",
-			referer: "https://webhooker.example:8443/h/" + entrypointUUID,
-			want:    "https://webhooker.example:8443",
-		},
-		{
-			name:    "path without a scheme or host",
-			referer: "/h/" + entrypointUUID,
-			want:    redacted,
-		},
-		{
-			name:    "host without a scheme",
-			referer: "webhooker.example/h/" + entrypointUUID,
-			want:    redacted,
-		},
-		{
-			name:    "not a URL",
-			referer: "http://[::1/h/" + entrypointUUID,
-			want:    redacted,
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			got := referer.SchemeAndHost(tc.referer)
-
-			assert.Equal(t, tc.want, got)
-			assert.NotContains(t, got, entrypointUUID)
-		})
-	}
-}

internal/server/sentry.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/getsentry/sentry-go"
 	"github.com/go-chi/chi"
-	"sneak.berlin/go/webhooker/internal/referer"
 )
 
 // sentryRedacted stands in for a withheld field on every event shipped
@@ -254,7 +253,6 @@ func sentryTransactionName(name, pattern string) string {
 // gorilla/csrf accepts in place of the form field — and the shared
 // secrets senders put on the receiver route (X-Gitlab-Token and the
 // per-provider signature headers) would otherwise ship verbatim.
-// The Referer it keeps is cut down to its scheme and host.
 func keptSentryHeaders(headers map[string]string) map[string]string {
 	if len(headers) == 0 {
 		return headers
@@ -263,15 +261,9 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
 	kept := make(map[string]string, len(headers))
 
 	for name, value := range headers {
-		if !sentryKeepsHeader(name) {
-			continue
+		if sentryKeepsHeader(name) {
+			kept[name] = value
 		}
-
-		if http.CanonicalHeaderKey(name) == "Referer" {
-			value = referer.SchemeAndHost(value)
-		}
-
-		kept[name] = value
 	}
 
 	return kept
@@ -279,14 +271,11 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
 
 // sentryKeepsHeader reports whether a request header is routing or
 // content metadata rather than client-chosen payload. Referer is kept
-// only as its scheme and host, through referer.SchemeAndHost, which
-// the access log uses too: its path can be an entrypoint URL, which
-// carries the UUID, as when a sender follows a redirect from http://
-// to https:// or a browser fetches the 404 page's stylesheet from
-// under /h/. The scheme and host are all gorilla/csrf compares a
-// Referer by, so a CSRF rejection is still readable from the event.
-// X-Request-Id ties the event to the local access log line, which
-// holds the rest of the detail.
+// on the reasoning that it is browser-set, that the only query
+// parameters in this service's own URLs are the login page's `next`,
+// `notice` and the event log's `show`, and that Referrer-Policy is set to
+// strict-origin-when-cross-origin. X-Request-Id ties the event to the
+// local access log line, which holds the rest of the detail.
 func sentryKeepsHeader(name string) bool {
 	switch http.CanonicalHeaderKey(name) {
 	case "Accept",

internal/server/sentry_test.go

@@ -325,37 +325,6 @@ func TestSentryScrub_ReplacesTheCapabilityPathWithTheRoutePattern(
 	)
 }
 
-// TestSentryScrub_KeepsOnlyTheRefererSchemeAndHost sends a receiver
-// request whose Referer is the entrypoint URL, as a sender's is after
-// it follows the README's redirect from http:// to https://. The event
-// keeps the Referer's scheme and host, and carries the UUID nowhere.
-func TestSentryScrub_KeepsOnlyTheRefererSchemeAndHost(t *testing.T) {
-	t.Parallel()
-
-	event := onlyEvent(t, sentryCase{
-		scrub:  true,
-		panics: true,
-		request: func(client *sentry.Client) *http.Request {
-			req := sentryReceiverRequest(client)
-			req.Header.Set(
-				"Referer",
-				"http://webhooker.example/h/"+sentryReceiverUUID,
-			)
-
-			return req
-		},
-	}.capture(t))
-
-	assert.NotContains(
-		t, marshalEvent(t, event), sentryReceiverUUID,
-	)
-	assert.Equal(
-		t,
-		"http://webhooker.example",
-		event.Request.Headers["Referer"],
-	)
-}
-
 // TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost
 // the debugging signal: the route, its scheme and host, the method and
 // the metadata headers still identify what failed. On a static route