fix(http): remove never-functional Authorization header capture from request logger - #4437
Open
stevenvegt wants to merge 3 commits into
Open
fix(http): remove never-functional Authorization header capture from request logger#4437stevenvegt wants to merge 3 commits into
stevenvegt wants to merge 3 commits into
Conversation
The request logger middleware listed the Authorization header in LogHeaders, to be copied into request log entries at debug verbosity. Bearer tokens must never reach logs, so the capture is removed entirely instead of filtered. The removal changes no observable behavior: the debug gate read Entry.Level, which is only set on the entry copy used during an actual log call, so the condition was always false and headers were never logged by any released version. A pinning test now asserts that no headers field is logged, even at debug verbosity. Assisted-by: AI
stevenvegt
requested review from
Dirklectisch,
JorisHeadease,
gerardsn,
reinkrul and
woutslakhorst
as code owners
July 30, 2026 12:39
Contributor
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (2)
🛟 Help
|
The http.log flag description stated that authorization headers are logged at debug verbosity. The header capture is removed, and it never fired in any released version to begin with. Assisted-by: AI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the header capture (
AuthorizationandDPoP) from the HTTP request logger middleware. The capture has never worked: no released version has ever written these headers to a log, so no operator action is required. It is removed rather than repaired becauseAuthorizationcarries bearer tokens, which must not end up in logs.Details: since its introduction in #3033 the gate read
logger.Levelon a*logrus.Entry, which is the per-message level that is only set on the entry copy used during an actual log call, never on the long-lived entry held by the middleware. The condition was therefore always0 >= 5, i.e. false, and theheadersfield was never added to a log entry.Rather than repairing the gate and filtering out
Authorization, this removes the header capture entirely, so that a future innocent fix of the level check cannot silently start logging bearer tokens.Testing:
Test_requestLoggerMiddlewaregains a pinning subtestit does not log headers, even on debugthat setsAuthorizationandDPoPon the request, runs the middleware with the logger at debug verbosity, and asserts the resulting log entry has noheadersfield. It passes both before and after the removal, which is what confirms the capture never fired.