fix: address PR review feedback (Copilot + CodeRabbit)

Address four substantive review findings on the contextual-logs work.

1. log/default.go writeContext error sanitization (Copilot)
   The render-error marker piped err.Error() through fmt.Fprintf, which
   could carry CR/LF/ANSI bytes from a tag wrapping attacker-controlled
   data. Route the message through writeSanitizedString — the same
   sanitiser ${value:KEY} already uses — so a misconfigured tag cannot
   become a log-injection vector. Test_WithContextRenderError now embeds
   CR/LF in the error and asserts they are stripped.

2. docs/api/log.md KeyValueFormat consistency (Copilot)
   Both the Go-block snippet and the Context Formats table referenced
   the bare ${requestid} tag while the actual KeyValueFormat constant
   uses the dashed ${request-id} alias. Align both to the constant.

3. log/default.go newRetainedContext typed-nil handling (CodeRabbit)
   `value != nil` does not catch typed nils such as
   `(*fasthttp.RequestCtx)(nil)` stored in an `any`; the interface
   header carries a non-nil type descriptor and the renderer would
   dereference a nil receiver. Use reflect.Value.IsNil() on the
   nilable kinds (Chan, Func, Interface, Map, Pointer, Slice) to
   collapse typed nils to retainedContext{}. Reflection runs once per
   WithContext call (not per log line), so the cost is irrelevant to
   the hot path. Test_NewRetainedContext_TypedNil locks in coverage
   for untyped nil, typed-nil pointer/map/slice/chan, and non-nil
   variants of the same.

4. middleware/csrf csrf-token redaction shape (CodeRabbit)
   Switch the csrf-token tag from the package-local "[redacted]"
   constant to redact.Prefix(token), aligning the masking shape with
   keyauth and session (4-byte clear prefix + "****"). Log consumers
   now see a uniform redaction format across every security-sensitive
   tag the framework registers. csrf_test assertions move from exact
   match against redactedKey to shape-based assertions on the
   redact.Mask suffix. The package-internal redactedKey constant is
   retained for storage-manager redaction (unchanged behavior).

Also document the intentional-by-design decisions:

- middleware/basicauth basicauth.go: ${username} is written in clear
  text for audit-log use cases. Add a godoc block on
  registerLogContextTags pointing developers to UsernameFromContext if
  full usernames count as PII in their jurisdiction. Mirror the
  callout in docs/api/log.md.

Verified locally with golangci-lint v2.6.0 (CI-matching), modernize@latest,
and `go test -race -shuffle=on` on the affected packages.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: ReneWerner87

docs/api/log.md

@@ -271,7 +271,7 @@ type Buffer        = logtemplate.Buffer
 const (
     DefaultFormat   = ""
     RequestIDFormat = "[${requestid}] "
-    KeyValueFormat  = "request-id=${requestid} username=${username} api-key=${api-key} csrf-token=${csrf-token} session-id=${session-id} "
+    KeyValueFormat  = "request-id=${request-id} username=${username} api-key=${api-key} csrf-token=${csrf-token} session-id=${session-id} "
     TagContextValue = "value:"
 )
 ```
@@ -282,16 +282,16 @@ const (
 | :-- | :-- | :-- |
 | `DefaultFormat` | `""` | Disables contextual fields. |
 | `RequestIDFormat` | `"[${requestid}] "` | Prepends the request ID when the requestid middleware is used. |
-| `KeyValueFormat` | `"request-id=${requestid} username=${username} api-key=${api-key} csrf-token=${csrf-token} session-id=${session-id} "` | Prepends common middleware context values as key/value fields. Sensitive values are redacted by the registering middleware. |
+| `KeyValueFormat` | `"request-id=${request-id} username=${username} api-key=${api-key} csrf-token=${csrf-token} session-id=${session-id} "` | Prepends common middleware context values as key/value fields. Sensitive values are redacted by the registering middleware. |
 
 ### Context Tags
 
 | Tag | Source |
 | :-- | :-- |
 | `${requestid}` / `${request-id}` | `requestid` middleware |
-| `${username}` | `basicauth` middleware |
+| `${username}` | `basicauth` middleware — written **in clear text** for audit-log use cases. Avoid this tag if your usernames are PII. |
 | `${api-key}` | `keyauth` middleware, redacted to a 4-byte prefix |
-| `${csrf-token}` | `csrf` middleware, redacted to a constant placeholder |
+| `${csrf-token}` | `csrf` middleware, redacted to a 4-byte prefix |
 | `${session-id}` | `session` middleware, redacted to a 4-byte prefix |
 | `${value:key}` | Any bound value with `Value(key)` or `UserValue(key)` lookup methods |
 

log/default.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"log"
 	"os"
+	"reflect"
 
 	"github.com/gofiber/utils/v2"
 	"github.com/valyala/bytebufferpool"
@@ -28,11 +29,28 @@ type retainedContext struct {
 	ok    bool
 }
 
+// newRetainedContext wraps value, treating both untyped nil and typed-nil
+// pointers/interfaces/maps/slices/channels/funcs as "no context". A typed nil
+// (e.g. (*fasthttp.RequestCtx)(nil) stored in an `any`) compares non-nil under
+// `value != nil` because the interface header carries a non-nil type
+// descriptor; passing it through to a tag renderer would cause the renderer
+// to dereference a nil receiver. Reflection here is one-shot per WithContext
+// call — far off the per-log-line hot path.
 func newRetainedContext(value any) retainedContext {
-	return retainedContext{
-		value: value,
-		ok:    value != nil,
+	if value == nil {
+		return retainedContext{}
 	}
+	switch rv := reflect.ValueOf(value); rv.Kind() {
+	case reflect.Chan, reflect.Func, reflect.Interface,
+		reflect.Map, reflect.Pointer, reflect.Slice:
+		if rv.IsNil() {
+			return retainedContext{}
+		}
+	default:
+		// Non-nilable kinds (struct, int, string, ...) cannot be typed-nil;
+		// fall through to wrap the value as-is.
+	}
+	return retainedContext{value: value, ok: true}
 }
 
 // privateLog logs a message at a given level log the default logger.
@@ -259,7 +277,13 @@ func (l *defaultLogger) writeContext(buf Buffer) {
 	defer bytebufferpool.Put(scratch)
 
 	if err := tmpl.Execute(scratch, l.ctx.value, &ContextData{}); err != nil {
-		_, _ = fmt.Fprintf(buf, "[ctx-render-error: %s] ", err.Error())
+		// The error string can carry CR/LF/ANSI bytes derived from request data
+		// (e.g. when a tag wraps an upstream parsing error around a header). Run
+		// it through the same sanitiser the ${value:KEY} renderer uses so a
+		// misconfigured tag cannot become a log-injection vector.
+		_, _ = buf.WriteString("[ctx-render-error: ") //nolint:errcheck // best-effort marker
+		_, _ = writeSanitizedString(buf, err.Error()) //nolint:errcheck // best-effort marker
+		_, _ = buf.WriteString("] ")                  //nolint:errcheck // best-effort marker
 		return
 	}
 

log/default_test.go

@@ -8,6 +8,7 @@ import (
 	"log"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -96,14 +97,17 @@ func Test_WithContextNilCaller(t *testing.T) {
 
 // Test_WithContextRenderError locks in M8: a misconfigured context tag must
 // not silently drop context — the failure should leave a visible marker in
-// the log line so operators notice. Calls initDefaultLogger up front because
-// under -shuffle=on this test may run before any other log test, in which
-// case the package-global logger.stdlog could still be nil.
+// the log line so operators notice. The error message is also sanitized for
+// control bytes so a tag that wraps attacker-controlled data in its error
+// cannot inject CR/LF into the log stream. Calls initDefaultLogger up front
+// because under -shuffle=on this test may run before any other log test, in
+// which case the package-global logger.stdlog could still be nil.
 func Test_WithContextRenderError(t *testing.T) {
 	initDefaultLogger()
 	t.Cleanup(initDefaultLogger)
 
-	templateErr := errors.New("tag boom")
+	// Embed CR/LF in the error to exercise the sanitiser.
+	templateErr := errors.New("tag\r\nboom")
 	require.NoError(t, SetContextTemplate(ContextConfig{
 		Format: "[${broken}] ",
 		CustomTags: map[string]ContextTagFunc{
@@ -122,6 +126,40 @@ func Test_WithContextRenderError(t *testing.T) {
 	out := string(w.b)
 	require.Contains(t, out, "ctx-render-error", "expected render-error marker, got %q", out)
 	require.Contains(t, out, "payload", "expected payload to still be emitted, got %q", out)
+	require.NotContains(t, out, "\r", "CR in error message must be sanitized")
+	// One trailing newline from log.Output is expected; reject any embedded ones.
+	require.Equal(t, 1, strings.Count(out, "\n"), "embedded LF in error message must be sanitized; got %q", out)
+}
+
+// Test_NewRetainedContext_TypedNil locks in the reflect-based nil detection
+// in newRetainedContext: a typed-nil pointer wrapped in `any` must be treated
+// as "no context" so tag renderers don't fault on a nil receiver.
+func Test_NewRetainedContext_TypedNil(t *testing.T) {
+	t.Parallel()
+
+	type fakeCtx struct{}
+	var typedNil *fakeCtx
+
+	tests := []struct {
+		value any
+		name  string
+		want  bool
+	}{
+		{name: "untyped nil", value: nil, want: false},
+		{name: "typed nil pointer", value: typedNil, want: false},
+		{name: "typed nil map", value: map[string]string(nil), want: false},
+		{name: "typed nil slice", value: []byte(nil), want: false},
+		{name: "typed nil chan", value: chan int(nil), want: false},
+		{name: "non-nil pointer", value: &fakeCtx{}, want: true},
+		{name: "string", value: "context", want: true},
+		{name: "non-nil map", value: map[string]string{"k": "v"}, want: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tt.want, newRetainedContext(tt.value).ok)
+		})
+	}
 }
 
 func Test_DefaultLogger(t *testing.T) {

middleware/basicauth/basicauth.go

@@ -116,6 +116,17 @@ func New(config ...Config) fiber.Handler {
 	}
 }
 
+// registerLogContextTags exposes the authenticated username under the
+// ${username} tag for both middleware/logger access logs and fiberlog
+// WithContext lines. The full clear-text username is intentionally written
+// (not redacted) because the primary use case for ${username} is auditability:
+// who hit which endpoint, when. Username strings have already passed
+// containsCTL stripping, so they are safe with respect to log injection.
+//
+// If full usernames are PII in your jurisdiction (GDPR, CCPA, etc.), do not
+// include ${username} in your log format and obtain the value via
+// UsernameFromContext at the application layer where you can hash, prefix,
+// or otherwise minimize it before emitting.
 func registerLogContextTags() {
 	logger.RegisterContextTag("username", UsernameFromContext)
 }

middleware/csrf/csrf.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/gofiber/fiber/v3"
 	"github.com/gofiber/fiber/v3/extractors"
+	"github.com/gofiber/fiber/v3/internal/redact"
 	"github.com/gofiber/fiber/v3/middleware/logger"
 )
 
@@ -229,10 +230,10 @@ func New(config ...Config) fiber.Handler {
 
 func registerLogContextTags() {
 	logger.RegisterContextTag("csrf-token", func(ctx any) string {
-		if TokenFromContext(ctx) == "" {
-			return ""
-		}
-		return redactedKey
+		// redact.Prefix matches the masking shape used by keyauth and session
+		// (4-byte clear prefix + "****"), so log consumers see a uniform
+		// redaction format across the framework's security-sensitive tags.
+		return redact.Prefix(TokenFromContext(ctx))
 	})
 }
 

middleware/csrf/csrf_test.go

@@ -7,13 +7,16 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/gofiber/fiber/v3"
 	"github.com/gofiber/fiber/v3/extractors"
 	"github.com/gofiber/fiber/v3/internal/loggertest"
+	"github.com/gofiber/fiber/v3/internal/redact"
 	fiberlog "github.com/gofiber/fiber/v3/log"
 	"github.com/gofiber/fiber/v3/middleware/logger"
 	"github.com/gofiber/fiber/v3/middleware/session"
@@ -652,7 +655,11 @@ func Test_CSRFLoggerTagRedactsToken(t *testing.T) {
 	resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
 	require.NoError(t, err)
 	require.Equal(t, fiber.StatusOK, resp.StatusCode)
-	require.Equal(t, redactedKey, buf.String())
+	// CSRF tokens are randomly generated per request, so assert on the
+	// redaction shape (4-byte prefix + Mask) rather than a fixed value.
+	got := buf.String()
+	require.Len(t, got, redact.PrefixLength+len(redact.Mask))
+	require.True(t, strings.HasSuffix(got, redact.Mask), "expected suffix %q in %q", redact.Mask, got)
 }
 
 // Test_CSRFLogContextTagRedactsToken runs serially because it mutates
@@ -670,7 +677,7 @@ func Test_CSRFLogContextTagRedactsToken(t *testing.T) {
 	resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
 	require.NoError(t, err)
 	require.Equal(t, fiber.StatusOK, resp.StatusCode)
-	require.Contains(t, buf.String(), "[Info] csrf-token="+redactedKey+" start")
+	require.Regexp(t, `\[Info\] csrf-token=.{`+strconv.Itoa(redact.PrefixLength)+`}`+regexp.QuoteMeta(redact.Mask)+` start`, buf.String())
 }
 
 func Test_CSRF_From_Form(t *testing.T) {