fix: redact credential headers from the request log - #1747
Merged
Conversation
The request logger attached the full inbound header map to its record, so x-api-key was written verbatim to application-%DATE%.log and to stdout on every request. Both transports are pinned to 'debug', so APP.LOG_LEVEL did not suppress it. Redact at the call site and again in the shared winston format chain, so a future record that carries a header map cannot reintroduce the leak. Authorization, proxy-authorization, cookie and set-cookie are covered too; their values no longer appear in verbose request logs.
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
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.
Problem
The request logger middleware attached the entire inbound header map to its log record:
Every authenticated request therefore wrote the plaintext
x-api-keytoapplication-%DATE%.logand, in production, to stdout — which is what log shippersforward to aggregators.
APP.LOG_LEVELdid not suppress this. In winston 3 a transport's own level overridesthe logger's level rather than being capped by it, and both relevant transports in
src/config/logger.jsare pinned tolevel: 'debug', so theverboserecord wasemitted even at
LOG_LEVEL: info. Verified against the installed winston 3.19.0: atransport at
debugreceivesverboserecords while the logger level isinfo.Change
src/utils/log-redaction.jswith aredactHeadershelper that returns a copywith sensitive values replaced by
[REDACTED]. The header name is preserved, sologs still show that a credential was supplied.
src/middleware.jspassesredactHeaders(req.headers)— this closes the leak.src/config/logger.jsadds the same redaction as a winston format at the front ofthe shared chain, so a future record elsewhere that carries a header map is scrubbed
too. It handles both
info.headersandinfo.metadata.headersso reordering thechain later cannot silently reopen the hole.
Scope note for reviewers
The redaction set is deliberately wider than
x-api-key:authorization,proxy-authorization,cookieandset-cookieare also redacted. Since the requestlogger logs the full header map on every request, this means those values no longer
appear in verbose request logs or in the log artifacts CI uploads. That is an
intentional trade-off — flagging it in case anyone relies on those fields for auth
debugging.
Redaction covers the top-level
headersfield only; a header map nested deeper in alog record still reaches the log stream. The limitation is documented in the code.
Operational follow-up (not addressed by this PR)
Deployments whose logs were already shipped should rotate
CADT_API_KEYand purge theexisting
application-*.logfiles, including the gzipped archives, since the rotatedlogs retain 30 days of history.
Test plan
tests/integration/log-redaction.spec.js: 10 tests covering the helper, thewinston format at every log level, non-mutation of caller-supplied objects, and an
end-to-end request through the app asserting the key never reaches the output.
hands the logger a record that is already redacted; reverting the format failsthe logger-level tests; reverting the metadata copy fails the non-mutation test.
npm run test:v1— 163 passing, 5 pendingeslintandprettier --checkclean on all touched files. Notesrc/middleware.jshas pre-existing
no-unused-varserrors and Prettier drift on the base branch;both reproduce on the unmodified file and were left alone to keep this diff small.
Note
Low Risk
Defensive logging change that reduces credential exposure; no auth or request-handling logic changes beyond what gets logged.
Overview
Stops plaintext API keys and other credentials from being written to application logs and production stdout on every authenticated request.
The request logger previously attached the full
req.headersmap to verbose records; with transports pinned todebug, those records still shipped even whenAPP.LOG_LEVELwasinfo. This PR addsredactHeadersinlog-redaction.js(sensitive names includex-api-key,authorization, cookies) and uses it in middleware at the call site. A Winston format at the start of the shared logger chain also scrubsinfo.headersandinfo.metadata.headersso other log sites are covered.Header names stay in logs with values replaced by
[REDACTED]. Redaction applies only to top-levelheadersshapes documented in code. Integration tests cover the helper, all log levels, non-mutation of caller metadata, and end-to-end requests.Reviewed by Cursor Bugbot for commit 1af85c7. Bugbot is set up for automated code reviews on this repo. Configure here.