Skip to content

fix: redact Authorization header in LogInterceptor [AIS-247] #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Tyler Pina (tylerpina) merged 3 commits into from
Jul 21, 2026
+98 −1
Merged

fix: redact Authorization header in LogInterceptor [AIS-247] #212

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb968c9
fix: redact Authorization header in LogInterceptor [AIS-247]
tylerpina Jul 21, 2026
02ace95
test: add LogInterceptor redaction tests, reuse HEADER_NAME [AIS-247]
tylerpina Jul 21, 2026
b1031c4
test: assert redactHeaders directly, note okhttp masking [AIS-247]
tylerpina Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/main/java/com/contentful/java/cma/interceptor/LogInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import java.io.IOException;

import okhttp3.Headers;
import okhttp3.Interceptor;
import okhttp3.Request;
import okhttp3.Response;
Expand Down Expand Up @@ -46,7 +47,7 @@ public LogInterceptor(Logger logger) {

long t1 = System.nanoTime();
logger.log(String.format("Sending request %s on %s%n%s",
request.url(), chain.connection(), request.headers()));
request.url(), chain.connection(), redactHeaders(request.headers())));

Response response = chain.proceed(request);

Expand All @@ -56,4 +57,17 @@ public LogInterceptor(Logger logger) {

return response;
}

private static Headers redactHeaders(Headers headers) {

@t-col Tyler Collins (t-col) Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on coverage — there's no existing test file for LogInterceptor, so this redaction doesn't have anything guarding it going forward. A quick test that mocks the logger and asserts the token never shows up unredacted in the logged string could be cheap insurance for a fix like this.

@tylerpina Tyler Pina (tylerpina) Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added LogInterceptorTests.kt under interceptor/. It drives a real request through the interceptor via MockWebServer with a capturing Logger:

  • testAuthorizationHeaderIsRedacted — asserts the raw token never appears and Bearer [REDACTED] does.
  • testOtherHeadersAreStillLogged — asserts a non-auth header (X-Custom-Header) is still present, so redaction is surgical.

Heads up: I could not run the suite locally (no JDK on my machine at the moment), so I am leaning on CI to confirm it goes green — the test mirrors the existing MockWebServer pattern used across the Kotlin tests, so I would expect it to, but flagging that I have not executed it myself.

@tylerpina Tyler Pina (tylerpina) Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran the suite locally after all (installed a JDK) — and it caught something worth flagging.

The end-to-end test I first wrote asserted Bearer [REDACTED] appears in the log. It failed: okhttp >= 4.10 already masks Authorization to ██ in Headers.toString() by header name, regardless of value. So on our okhttp (4.12):

  • the raw token was never in the log even before this fix (okhttp masks it), and
  • our injected Bearer [REDACTED] gets overwritten to ██ too, so it is not observable end-to-end.

Two takeaways:

  1. redactHeaders is defense-in-depth here, not the thing preventing the leak on 4.12 — it guarantees the value is stripped independent of okhttp's internal masking (which is an undocumented debug convenience, not a security guarantee, and would disappear if okhttp were pinned < 4.10). Added a comment saying so.
  2. An end-to-end "token absent" test would pass even if redactHeaders were deleted, so it does not actually guard the code. Reworked the tests to assert redactHeaders directly (Authorization value → Bearer [REDACTED], case-insensitive match, other headers untouched) plus keep one end-to-end raw-token-absent invariant.

3/3 green locally (mvn -Dtest=LogInterceptorTests test). Pushed in b1031c4.

Headers.Builder redacted = new Headers.Builder();
for (int i = 0; i < headers.size(); i++) {
String name = headers.name(i);
if ("Authorization".equalsIgnoreCase(name)) {

@t-col Tyler Collins (t-col) Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small one: AuthorizationHeaderInterceptor.HEADER_NAME already holds this exact string a few files over. Might be worth reusing it here instead of the literal, just so we don't end up with two sources of truth for the header name.

@tylerpina Tyler Pina (tylerpina) Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — redactHeaders now compares against AuthorizationHeaderInterceptor.HEADER_NAME instead of the string literal, so the header name has a single source of truth. Same package, so no new import.

redacted.add(name, "Bearer [REDACTED]");
} else {
redacted.add(name, headers.value(i));
}
}
return redacted.build();
}
}
Loading