From 86717c78e6b542a11713112e3d5a78158c44284a Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 16 Jul 2026 15:11:37 +0200 Subject: [PATCH 1/2] fix: redact sensitive response headers --- internal/debugmiddleware/debug_middleware.go | 66 +++++++++---------- .../debugmiddleware/debug_middleware_test.go | 35 ++++++++++ 2 files changed, 67 insertions(+), 34 deletions(-) diff --git a/internal/debugmiddleware/debug_middleware.go b/internal/debugmiddleware/debug_middleware.go index 64e270f..8a95b47 100644 --- a/internal/debugmiddleware/debug_middleware.go +++ b/internal/debugmiddleware/debug_middleware.go @@ -17,8 +17,8 @@ type ( const redactedPlaceholder = "" -// Headers known to contain sensitive information like an API key. Note that this exclude `Authorization`, -// which is handled specially in `redactRequest` below. +// Headers known to contain sensitive information like an API key. Authorization +// headers are handled separately so their authentication scheme remains visible. var sensitiveHeaders = []string{ "api-key", "x-api-key", @@ -55,7 +55,10 @@ func (m *RequestLogger) Middleware() Middleware { return resp, err } - if respBytes, err := httputil.DumpResponse(resp, false); err == nil { + loggedResponse := new(http.Response) + *loggedResponse = *resp + loggedResponse.Header = m.redactHeaders(resp.Header) + if respBytes, err := httputil.DumpResponse(loggedResponse, false); err == nil { m.logger.Printf("Response Content:\n%s\n", respBytes) } @@ -68,43 +71,38 @@ func (m *RequestLogger) Middleware() Middleware { // the original and that clone is returned. As a small optimization, the // original is request is returned unchanged if no redaction is necessary. func (m *RequestLogger) redactRequest(req *http.Request) (*http.Request, error) { - redactedHeaders := req.Header.Clone() + redactedHeaders := m.redactHeaders(req.Header) + if reflect.DeepEqual(req.Header, redactedHeaders) { + return req, nil + } - // Notably, the clauses below are written so they can redact multiple - // headers of the same name if necessary. - if values := redactedHeaders.Values("Authorization"); len(values) > 0 { - redactedHeaders.Del("Authorization") + redacted := req.Clone(req.Context()) + redacted.Header = redactedHeaders + return redacted, nil +} - for _, value := range values { - // In case we're using something like a bearer token (e.g. `Bearer - // `), keep the `Bearer` part for more debugging - // information. - if authKind, _, ok := strings.Cut(value, " "); ok { - redactedHeaders.Add("Authorization", authKind+" "+redactedPlaceholder) - } else { - redactedHeaders.Add("Authorization", redactedPlaceholder) +func (m *RequestLogger) redactHeaders(headers http.Header) http.Header { + redacted := headers.Clone() + for header, values := range redacted { + if strings.EqualFold(header, "Authorization") || strings.EqualFold(header, "Proxy-Authorization") { + for i, value := range values { + if authKind, _, ok := strings.Cut(value, " "); ok { + values[i] = authKind + " " + redactedPlaceholder + } else { + values[i] = redactedPlaceholder + } } - } - } - - for _, header := range m.sensitiveHeaders { - values := redactedHeaders.Values(header) - if len(values) == 0 { continue } - redactedHeaders.Del(header) - - for range values { - redactedHeaders.Add(header, redactedPlaceholder) + for _, sensitiveHeader := range m.sensitiveHeaders { + if strings.EqualFold(header, sensitiveHeader) { + for i := range values { + values[i] = redactedPlaceholder + } + break + } } } - - if reflect.DeepEqual(req.Header, redactedHeaders) { - return req, nil - } - - redacted := req.Clone(req.Context()) - redacted.Header = redactedHeaders - return redacted, nil + return redacted } diff --git a/internal/debugmiddleware/debug_middleware_test.go b/internal/debugmiddleware/debug_middleware_test.go index 11f8920..265469c 100644 --- a/internal/debugmiddleware/debug_middleware_test.go +++ b/internal/debugmiddleware/debug_middleware_test.go @@ -200,6 +200,41 @@ func TestDebugMiddleware(t *testing.T) { require.NotContains(t, logBuf.String(), bodyContent) }) + t.Run("RedactsSensitiveResponseHeaders", func(t *testing.T) { + t.Parallel() + + middleware, logBuf := setup() + responseHeaders := http.Header{ + "sEt-CoOkIe": {"session=" + secretToken + "1", "csrf=" + secretToken + "2"}, + "aUtHoRiZaTiOn": {"Bearer " + secretToken + "3"}, + "pRoXy-AuThOrIzAtIoN": {"Basic " + secretToken + "4"}, + "X-Api-Key": {secretToken + "5"}, + "X-Request-Id": {"request-id"}, + } + originalHeaders := responseHeaders.Clone() + + req := httptest.NewRequest("GET", "https://example.com", nil) + resp, err := middleware.Middleware()(req, func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Status: "200 OK", + Header: responseHeaders, + Body: http.NoBody, + }, nil + }) + require.NoError(t, err) + + logged := logBuf.String() + for _, suffix := range []string{"1", "2", "3", "4", "5"} { + require.NotContains(t, logged, secretToken+suffix) + } + require.Equal(t, 5, strings.Count(logged, redactedPlaceholder)) + require.Contains(t, logged, "Bearer "+redactedPlaceholder) + require.Contains(t, logged, "Basic "+redactedPlaceholder) + require.Contains(t, logged, "X-Request-Id: request-id") + require.Equal(t, originalHeaders, resp.Header) + }) + t.Run("DoesNotLogOrConsumeResponseBody", func(t *testing.T) { t.Parallel() From 5318021418a6273492f907479c83fbf515ae6432 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 16 Jul 2026 15:16:07 +0200 Subject: [PATCH 2/2] test: verify response identity is preserved --- internal/debugmiddleware/debug_middleware.go | 6 +++--- internal/debugmiddleware/debug_middleware_test.go | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/debugmiddleware/debug_middleware.go b/internal/debugmiddleware/debug_middleware.go index 8a95b47..9b2575a 100644 --- a/internal/debugmiddleware/debug_middleware.go +++ b/internal/debugmiddleware/debug_middleware.go @@ -55,10 +55,9 @@ func (m *RequestLogger) Middleware() Middleware { return resp, err } - loggedResponse := new(http.Response) - *loggedResponse = *resp + loggedResponse := *resp loggedResponse.Header = m.redactHeaders(resp.Header) - if respBytes, err := httputil.DumpResponse(loggedResponse, false); err == nil { + if respBytes, err := httputil.DumpResponse(&loggedResponse, false); err == nil { m.logger.Printf("Response Content:\n%s\n", respBytes) } @@ -81,6 +80,7 @@ func (m *RequestLogger) redactRequest(req *http.Request) (*http.Request, error) return redacted, nil } +// redactHeaders returns an independent copy with sensitive values removed. func (m *RequestLogger) redactHeaders(headers http.Header) http.Header { redacted := headers.Clone() for header, values := range redacted { diff --git a/internal/debugmiddleware/debug_middleware_test.go b/internal/debugmiddleware/debug_middleware_test.go index 265469c..3cec690 100644 --- a/internal/debugmiddleware/debug_middleware_test.go +++ b/internal/debugmiddleware/debug_middleware_test.go @@ -212,15 +212,16 @@ func TestDebugMiddleware(t *testing.T) { "X-Request-Id": {"request-id"}, } originalHeaders := responseHeaders.Clone() + response := &http.Response{ + StatusCode: http.StatusOK, + Status: "200 OK", + Header: responseHeaders, + Body: http.NoBody, + } req := httptest.NewRequest("GET", "https://example.com", nil) resp, err := middleware.Middleware()(req, func(req *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: http.StatusOK, - Status: "200 OK", - Header: responseHeaders, - Body: http.NoBody, - }, nil + return response, nil }) require.NoError(t, err) @@ -232,6 +233,7 @@ func TestDebugMiddleware(t *testing.T) { require.Contains(t, logged, "Bearer "+redactedPlaceholder) require.Contains(t, logged, "Basic "+redactedPlaceholder) require.Contains(t, logged, "X-Request-Id: request-id") + require.Same(t, response, resp) require.Equal(t, originalHeaders, resp.Header) })