[Detail Bug] Auth logging: API key identifier can appear in error logs during Clerk transport failures

[detail-app]

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_653a7de6-fa97-4b1d-9c49-dca3e383407f

Introduced in ce8b434 by @WilliamAGH on Sep 1, 2026

Summary

• Context: Non-security. No measurable security impact. The commit ce8b434c ("fix(auth): log unavailable API-key revocations") added the log.error call at AuthenticatedUserController.java:73 and a regression test that explicitly asserts the API key identifier should not appear in the log event.
• Bug: AuthenticatedUserController logs an ApiKeyOperationUnavailableException with its cause chain. On Clerk transport failures, the cause is a Spring ResourceAccessException whose message includes the populated revoke URL https://api.clerk.com/v1/api_keys/{apiKeyId}/revoke, so the API key id renders into the log via the stack trace.
• Actual vs. expected: Actual: rendered CONSOLE output includes the API key id in a Caused by: ResourceAccessException ... "https://api.clerk.com/v1/api_keys/ak_.../revoke" line. Expected (per the commit’s own test intent): the API key id should not appear in the log event/output.
• Impact: API key ids (non-secret identifiers) can be written to operator-visible console logs during degraded Clerk connectivity. The existing regression test passes vacuously and does not detect the leak.

Code with Bug

} catch (ApiKeyOperationUnavailableException unavailableClerk) {
    log.error(API_KEY_REVOCATION_UNAVAILABLE_LOG_MESSAGE, unavailableClerk); // <-- BUG 🔴 logs throwable; cause chain can include revoke URL with apiKeyId
    return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE)
            .body(ApiErrorResponse.error("API key revocation is temporarily unavailable. Please retry."));
}

} catch (RestClientException revocationFailure) {
    throw new ApiKeyOperationUnavailableException("Clerk API key revocation failed", revocationFailure);
}

Explanation

• ClerkApiKeyVerifier.revoke(apiKeyId) builds the revoke URI using the provided apiKeyId. On transport-layer failures, Spring throws ResourceAccessException with a message that embeds the full request URI (including the interpolated apiKeyId).
• The controller logs the exception object (log.error(msg, throwable)), and Logback’s console output includes the full stack trace and Caused by: chain, so the id appears even though the log message itself is constant.
• The integration test added in the same commit asserts only on revocationLogEvent.getFormattedMessage() (which excludes throwable rendering) and uses a causeless ApiKeyOperationUnavailableException, so it never exercises/observes the leaking ResourceAccessException path and passes regardless of the real behavior.

Codebase Inconsistency

The same commit that added the logging also added assertions that the API key id must not appear in the log event:

assertFalse(revocationLogEvent.getFormattedMessage().contains(CLERK_API_KEY_SECRET));
assertFalse(revocationLogEvent.getFormattedMessage().contains(CLERK_API_KEY_ID));
assertFalse(revocationLogEvent.getFormattedMessage().contains(CLERK_USER_ID));

These assertions demonstrate intent, but they check the wrong surface (formatted message only) and miss the throwable output where the id actually appears.

Failing Test

src/test/java/com/williamcallahan/javachat/web/RevokeApiKeyIdLogLeakTest.java includes an acceptance test that fails when enabled:

• renderedRevocationLogExcludesApiKeyId (@Disabled by default) asserts !renderedLine.contains(id) for a production-shaped transport failure; it fails on current code because the id appears in the rendered stack trace.

Witness output excerpt (from the failing assertion):

ERROR c.w.j.w.AuthenticatedUserController - Clerk API key revocation was unavailable
...
Caused by: org.springframework.web.client.ResourceAccessException: I/O error on POST request for "https://api.clerk.com/v1/api_keys/ak_0123456789abcdef0123456789abcdef/revoke": Connection refused
...

Recommended Fix

Implement log redaction for throwable rendering so stack traces remain available without leaking ak_... identifiers.

• Add RedactingThrowableProxyConverter (regex redact ak_[A-Za-z0-9]+ → ak_***).
• Update logback-spring.xml CONSOLE pattern to use the redacting converter instead of implicit throwable appending.
• Enable the currently @Disabled acceptance test to prevent regressions.

History

This bug was introduced in commit ce8b434c. The change added log.error(..., unavailableClerk) to make revocation failures diagnosable, but passing the full throwable causes Logback to render a ResourceAccessException message that contains the populated revoke URI (and thus the API key id). The included regression test missed this because it used a causeless exception and asserted only on getFormattedMessage().