fix(auth): redact Clerk API key id from revocation error logs

[detail-app]

Detail bug report: View on Detail

Closes #246

Bug

On a Clerk transport-layer failure (connection refused, DNS, timeout, TLS), AuthenticatedUserController.revokeCurrentApiKey logs the caught ApiKeyOperationUnavailableException via log.error(message, throwable). The exception's cause chain carries a Spring ResourceAccessException whose message embeds the resolved revoke URI — https://api.clerk.com/v1/api_keys/ak_…/revoke — so the ak_… key id renders into the CONSOLE log's Caused by: line. The regression test added alongside the log.error call (commit ce8b434c) passed vacuously: it threw a causeless exception and asserted only on getFormattedMessage() (the constant "Clerk API key revocation was unavailable"), which excludes throwable rendering, so it never exercised the leaking channel. The commit's own assertFalse(...contains(CLERK_API_KEY_ID)) assertion stated the id should not appear, yet it did.

Fix

Trace-preserving Logback redaction (the bug report's recommended Option A):

• RedactingThrowableProxyConverter extends Logback's ThrowableProxyConverter and overrides throwableProxyToString to replace ak_[A-Za-z0-9_]+ with ak_*** on the rendered stack/cause chain. It is a genuine Logback pattern-converter SPI implementation (the *Converter suffix is the framework's own naming for ThrowableProxyConverter/ExtendedThrowableProxyConverter), not a data-reshaping adapter. The regex covers both the non-secret id (ak_<id>) and the secret token form (ak_secret_<material>).
• logback-spring.xml registers the converter as %redactedEx and appends it to the CONSOLE pattern. Because the converter extends ThrowableHandlingConverter, Logback's EnsureExceptionHandling detects it and does not also append a second, unredacted default throwable converter — so the stack trace renders exactly once with the id redacted (no re-leak), and the full exception type, Caused by: chain, frames, host, and path remain for operator triage.

Rendered output after the fix: Caused by: org.springframework.web.client.ResourceAccessException: I/O error on POST request for "https://api.clerk.com/v1/api_keys/ak_***/revoke": Connection refused — only the ak_ token is removed; diagnosability is intact. The controller and verifier code are left untouched; HTTP behavior is unchanged (still 503 with the same error body, still one ERROR event).

Testing

• Unit guard (RedactingThrowableProxyConverterTest) — four cases pinning the converter's durable contract: the id is redacted to ak_***; the secret form is also redacted; the stack trace and full Caused by: chain are preserved; and Caused by: appears exactly once (proves Logback doesn't double-append an unredacted copy once %redactedEx is present).
• Acceptance guard (RevokeApiKeyIdLogLeakTest.renderedRevocationLogExcludesApiKeyId) — boots @SpringBootTest so the live logback-spring.xml CONSOLE pattern is under test, drives DELETE /api/me/api-key with a verified ak_ bearer whose mocked revoke throws an exception built by running a real RestClient against a connection-refusing transport (so the cause is a genuine Spring ResourceAccessException whose message embeds the revoke URI, exactly as in production). It asserts the rendered CONSOLE line excludes the id, contains ak_***, and still contains the outer exception, the Caused by: ResourceAccessException line, and Connection refused. This is the non-vacuous regression guard the original commit lacked — it asserts on the rendered line (not getFormattedMessage()) and drives a production-shaped cause.
• Non-vacuousness witness (not committed) — to confirm the acceptance test guards the right surface, I temporarily reverted the CONSOLE pattern to the pre-fix form and re-ran the acceptance test: it failed with the expected witness (Caused by: … ResourceAccessException: "…/api_keys/ak_0123456789abcdef0123456789abcdef/revoke" and expected: <false> but was: <true>); restoring %redactedEx made it pass again. This was a manual probe, not a committed test.
• I also wrote two reproduction tests during the fix (a real-ClerkApiKeyVerifier.revoke witness confirming Spring embeds the id on the wrapping ResourceAccessException, and a shared transport-failure stub) but excluded them from the commit — they assert the bug exists (a probe of the mechanism) rather than guard against a future regression, so per the repo's probe-vs-test policy they were deleted before committing; the production-shaped exception is now inlined in the acceptance test.

Routine checks (all green): ./gradlew compileJava compileTestJava, spotlessCheck, pmdMain, pmdTest, spotbugsMain (with FindSecBugs), spotbugsTest, ast-grep scan src/main/java/ (naming/type rules — no violation from the new *Converter SPI class), the full unit suite (1168 tests, 0 failures, 0 errors, 1 pre-existing environment-dependent skip unrelated to this change), and the existing AuthenticatedUserEndpointSecurityIntegrationTest (9/9, including the previously-vacuous revocation test, still passing) and CustomErrorControllerTest (13/13, which renders through the same live CONSOLE encoder — confirms non-throwable events are unchanged). make lint-ast required ast-grep and ruby, which were missing from the sandbox; I installed ast-grep 0.45.3 via npm install @ast-grep/cli and ruby 3.2.3 via apt-get install -y ruby, after which the lane passed (3/3 ast-grep snapshot tests, scan clean, chat-model SSOT self-test + match OK). git diff --check is clean.

---

Automatic Fixes PRs can be configured here.