Redact sensitive headers from debug logs by pavel-ptashyts · Pull Request #2279 · AsyncHttpClient/async-http-client

pavel-ptashyts · 2026-07-20T20:42:17Z

Summary

redact authentication and cookie values in HTTP and WebSocket debug logs
share the redaction policy through a neutral utility used by request and Netty logging
allow sensitive logging through an explicit startup-only JVM property or environment variable
warn once at startup when sensitive logging is enabled
avoid rendering complete request objects in retry and replay logs by default
add regression coverage for Netty message formatting, request output, and every startup configuration

Root cause

Debug logging passed complete Netty request and response objects to SLF4J. Their
string representations included every header value, so user-supplied
Authorization headers and other credentials could be written to application
logs.

Sensitive logging

Sensitive values remain redacted by default. Full header values can be enabled
before the sensitive logging policy is initialized with either:

JVM property: -Dorg.asynchttpclient.enableSensitiveLogging=true
environment variable: AHC_ENABLE_SENSITIVE_LOGGING=true

The JVM property takes precedence over the environment variable. The setting is
read once and remains immutable for the process. A warning is logged once when
sensitive logging is enabled because credentials and session data may be exposed.

Scope

This change addresses the header disclosure reported in #1739 and #1740.
Request form, query, multipart, and arbitrary body values do not have a reliable
generic sensitivity classification and remain outside this change.

Validation

JDK 11 focused HttpMessageFormatterTest and RequestBuilderTest: 33 tests passed, 0 skipped
JDK 11 system-property, environment-variable, and precedence paths execute in isolated JVMs
JDK 11 ./mvnw clean verify: passed, including Javadocs and Revapi
GitHub Actions matrix: passed on Ubuntu, macOS, and Windows with JDK 11, 17, 21, and 25

Closes#1739
Closes#1740

Codex on behalf of Pavel Ptashyts

Request and response debug logging rendered complete Netty messages, which exposed Authorization values and other credentials. Format logged HTTP messages with shared redaction for authentication and cookie headers. Reuse the same public predicate in DefaultRequest, and avoid rendering complete requests from retry and replay paths. Add regression coverage for HTTP message formatting and request string representations. FixesAsyncHttpClient#1739FixesAsyncHttpClient#1740 Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>

hyperxpro · 2026-07-20T20:46:08Z

I would like to have a system property / environment variable to enable non-redacted logs. It is helpful in debugging at CI pipeline.

Probably like AHC_ENABLE_SENSITIVE_LOGGING=true?

Of course feel free to choose a better key.

Redaction is secure by default, but it can prevent diagnosis of authentication failures that only occur in CI. Read a JVM property or environment variable once when the HTTP message formatter is initialized. Fold that immutable setting into isSensitiveHeader so both Netty logging and DefaultRequest use the same decision without repeated configuration lookups. Refs AsyncHttpClient#1739 Refs AsyncHttpClient#1740 Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>

pavel-ptashyts · 2026-07-20T22:07:36Z

@hyperxpro Implemented in 2a7f11b84.

Sensitive values remain redacted by default. Unredacted logging can be enabled before HttpMessageFormatter is initialized with either:

JVM property: -Dorg.asynchttpclient.enableSensitiveLogging=true
environment variable: AHC_ENABLE_SENSITIVE_LOGGING=true

The value is calculated once and remains immutable for the process. The JVM property takes precedence over the environment variable, including allowing an explicit false to keep redaction enabled. Full JDK 11 verification passes.

Thanks for the suggestion.

hyperxpro · 2026-07-23T19:52:00Z

+
+ private static final String ENABLE_SENSITIVE_LOGGING_PROPERTY = "org.asynchttpclient.enableSensitiveLogging";
+ private static final String ENABLE_SENSITIVE_LOGGING_ENVIRONMENT_VARIABLE = "AHC_ENABLE_SENSITIVE_LOGGING";
+ private static final boolean SENSITIVE_LOGGING_ENABLED = isSensitiveLoggingEnabled();


This is read once when the class loads and never rechecked. Surefire in this repo reuses a single JVM across the whole client module by default, so whichever test touches this class first locks the value for the rest of the run. That means the EnabledIfSystemProperty and EnabledIfEnvironmentVariable tests further down just get skipped in a normal build instead of actually running, so the enabled path has no real coverage right now. Might be worth resolving this per call or behind a seam that a test can override.

Kept the startup value immutable, but replaced the conditional tests with isolated JVM probes. The system-property, environment-variable, and precedence paths now run in every normal test build.

hyperxpro · 2026-07-23T19:52:34Z

+ private static StringBuilder appendHeaders(StringBuilder value, HttpHeaders headers) {
+ for (Map.Entry<String, String> header : headers) {
+ value.append('\n').append(header.getKey()).append(": ")
+ .append(isSensitiveHeader(header.getKey()) ? REDACTED : header.getValue());


This uses a colon followed by a space between header name and value, but DefaultRequest toString uses a colon with no space for the same kind of dump. Small thing, but the two renderings will drift if they stay separate.

Aligned DefaultRequest with the formatter colon-space spelling. Both paths now use the same neutral redaction policy.

hyperxpro · 2026-07-23T19:52:59Z

+ * @param name the header name
+ * @return {@code true} for authentication and cookie headers when sensitive logging is disabled
+ */
+ public static boolean isSensitiveHeader(CharSequence name) {


DefaultRequest in the base package now depends on this method and REDACTED. Since the base Request model has not reached into netty handler before, it might be cleaner to move this predicate and the redacted constant into a neutral utility package and have this class consume it instead.

Moved the shared REDACTED value and predicate to SensitiveLoggingUtils in the neutral util package. HttpMessageFormatter retains its public facade and delegates to that policy.

hyperxpro · 2026-07-23T19:53:33Z

+ return value;
+ }
+
+ private static boolean isSensitiveLoggingEnabled() {


This reads from an environment variable that can be inherited from a container or orchestration setup. Worth considering a one time warn log when this ends up enabled, so it shows up in the logs instead of only being discoverable after something has already leaked.

Added a one-time WARN during policy initialization whenever sensitive logging is enabled, regardless of whether it came from the system property or inherited environment.

hyperxpro · 2026-07-23T19:56:38Z

 import java.util.List;
 import java.util.Map;

+import static org.asynchttpclient.netty.handler.HttpMessageFormatter.REDACTED;


This pulls the base Request model into a dependency on the netty handler package, which has been treated as transport level internal up to now. See the note on HttpMessageFormatter about moving the shared predicate somewhere neutral instead.

Removed the dependency from DefaultRequest to the Netty handler package. It now consumes SensitiveLoggingUtils from the neutral util package.

hyperxpro · 2026-07-23T19:57:34Z

 sb.append(header.getKey());
 sb.append(':');
- sb.append(header.getValue());
+ sb.append(isSensitiveHeader(header.getKey()) ? REDACTED : header.getValue());


Headers are redacted here but formParams a bit further down in this same method still get appended in full. If a password ever comes through as a form field it would still show up in this output untouched, worth checking if that is in scope for the issues this closes.

Checked the scope against #1739 and #1740: both report Authorization and header disclosure. Form, query, multipart, and arbitrary body fields have no reliable generic sensitivity classification, so this PR intentionally leaves request-body rendering unchanged and now documents that scope.

hyperxpro · 2026-07-23T19:58:07Z

 future.touch();

- LOGGER.debug("\n\nReplaying Request {}\n for Future {}\n", newRequest, future);
+ LOGGER.debug("\n\nReplaying request '{}' to '{}'\n for Future {}\n", newRequest.getMethod(), newRequest.getUri(), future);


Checked this one, NettyResponseFuture toString does not render the underlying NettyRequest headers since NettyRequest has no toString override, so this falls back to identity hash and does not leak anything today.
Might be worth a short comment here noting that this is load bearing, so a future toString added to NettyRequest does not quietly reopen this.

Added the load-bearing comment directly in NettyResponseFuture.toString, where NettyRequest is rendered, explaining that NettyRequest must keep identity-only rendering so request headers are not exposed.

hyperxpro · 2026-07-23T19:58:25Z

+ }
+
+ @Test
+ @EnabledIfSystemProperty(named = "org.asynchttpclient.enableSensitiveLogging", matches = "(?i)true")


Same concern as on HttpMessageFormatter, this test and the one below it get silently skipped in a normal run rather than exercised, so the enabled path is effectively untested in CI as configured.

Replaced the skipped conditional tests with isolated JVM probes. Both enabled modes and property-over-environment precedence now execute in the regular CI test suite.

hyperxpro · 2026-07-23T19:58:50Z

Also please don't rebase your PRs as it spams my inbox :)

Move the shared redaction policy into a neutral utility and warn once when sensitive logging is enabled. Exercise each startup configuration in an isolated JVM, align request header formatting, and document why response futures do not render request headers. ClosesAsyncHttpClient#1739ClosesAsyncHttpClient#1740 Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>

pavel-ptashyts · 2026-07-25T11:20:26Z

Addressed the review feedback in 3b572a6:

moved the shared redaction policy to the neutral util package
added a one-time warning when sensitive logging is enabled
replaced skipped startup-mode tests with isolated JVM coverage
aligned request header formatting and documented the NettyRequest rendering invariant
clarified that this PR covers header disclosure, not arbitrary request-body fields

Full JDK 11 clean verify passes, including Javadocs and Revapi.

No rebase was used for this update, and I will keep future updates as regular commits.

hyperxpro · 2026-07-25T12:11:21Z

There are test failures, can you have a look?

Redirect sensitive logging probe output to temporary files so Windows pipe buffers cannot block child JVMs before the parent reads output. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>

pavel-ptashyts · 2026-07-25T12:28:49Z

Fixed the Windows CI timeouts in 12474ee.

The isolated probe JVMs wrote Logback and Netty startup output to a pipe while the parent process waited for them to exit. The smaller Windows pipe buffer filled first, which blocked each child until the 30-second timeout. Probe output is now redirected to a JUnit temporary file and read after the child exits.

Validation:

focused HttpMessageFormatterTest: 5 tests passed, 0 skipped
JDK 11 ./mvnw clean verify: passed, including Javadocs and Revapi

No rebase was used.

pavel-ptashyts · 2026-07-25T12:38:05Z

The follow-up CI run is fully green: compile-and-check plus Ubuntu, macOS, and Windows on JDK 11, 17, 21, and 25 all pass.

Motivation: Non-blocking follow-ups from the PR #2279 review: dead public API surface, an undocumented invariant, and an undocumented scope gap. Modification: Removed unused HttpMessageFormatter.REDACTED/isSensitiveHeader wrappers, added an invariant comment to NettyRequest, and documented the redaction scope in SensitiveLoggingUtils javadoc. Result: No functional change

hyperxpro requested changes Jul 23, 2026
View reviewed changes

Avoid Windows probe output deadlock
12474ee
Redirect sensitive logging probe output to temporary files so Windows pipe buffers cannot block child JVMs before the parent reads output. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>

pavel-ptashyts requested a review from hyperxpro July 25, 2026 13:43

hyperxpro approved these changes Jul 25, 2026
View reviewed changes

hyperxpro merged commit fcd54ce into AsyncHttpClient:mainJul 25, 2026
13 checks passed

pavel-ptashyts deleted the perf/redact-sensitive-log-headers branch July 25, 2026 17:39

hyperxpro mentioned this pull request Jul 25, 2026
Trim dead sensitive-logging wrappers and document scope #2290
Merged

Uh oh!

Conversation

pavel-ptashyts commented Jul 20, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Sensitive logging

Scope

Validation

Uh oh!

hyperxpro commented Jul 20, 2026

Uh oh!

pavel-ptashyts commented Jul 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyperxpro commented Jul 23, 2026

Uh oh!

pavel-ptashyts commented Jul 25, 2026

Uh oh!

hyperxpro commented Jul 25, 2026

Uh oh!

pavel-ptashyts commented Jul 25, 2026

Uh oh!

pavel-ptashyts commented Jul 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pavel-ptashyts commented Jul 20, 2026•
edited
Loading