[Detail Bug] CLI auth: malformed Clerk verify response missing id/subject returns HTTP 500 instead of 503

[detail-app]

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_a2ecddd7-b2ed-4efe-80ed-a573eedf7e31

Introduced in #178 by @WilliamAGH on Aug 22, 2026

Summary

• Context: The JavaChat CLI authenticates every request by presenting a personal Clerk API key as a Bearer ak_… token. ClerkApiKeyAuthenticationFilter calls ClerkApiKeyVerifier.verify on every such request to validate the key against the Clerk Backend API, then installs a ClerkApiKeyAuthenticationToken. The two endpoints in AuthenticatedUserController (GET /api/me, DELETE /api/me/api-key) — both consumed by the CLI — sit downstream of this filter.
• Bug: ClerkApiKeyVerifier.verify translates every malformed Clerk reply into a clean application outcome — Optional.empty() for rejected/revoked/expired keys, ApiKeyOperationUnavailableException (→ HTTP 503) for empty responses, incomplete lifecycle state, transport failures, and non-credential HTTP errors. The one case it fails to translate is a 200 whose identity fields (id, subject) are null or blank: it passes those straight into new VerifiedApiKey(verification.id(), verification.subject()), whose invariant guard throws IllegalArgumentException. That throw happens outside verify's try/catch (the try block only wraps the REST call), so the exception is not wrapped into ApiKeyOperationUnavailableException. The filter only catches ApiKeyOperationUnavailableException, so the IllegalArgumentException escapes OncePerRequestFilter, the servlet container dispatches it to CustomErrorController, and the client receives HTTP 500 with body {"status":"error","message":"Internal Server Error","details":"IllegalArgumentException"}.
• Actual vs. expected: A malformed Clerk 200 missing revoked/expired is handled as 503 ("Clerk returned incomplete API key lifecycle state") — a retryable, diagnosable provider failure. The structurally parallel case — a malformed Clerk 200 missing subject/id — is an unhandled 500 that signals a server crash and leaks the internal exception class name. verify's own code establishes that incomplete Clerk responses belong in the 503 bucket; it simply omits the identity fields from that treatment. The VerifiedApiKey record's IllegalArgumentException is correct value-object design (an invariant assertion); the defect is verify not handling that assertion at the provider boundary, inconsistent with every other malformed-response branch it implements.
• Impact:
  • Wrong error class for a provider issue: HTTP 500 signals "the server crashed". This malformed-200 condition is a provider-boundary issue (Clerk returned an incomplete response) which verify otherwise classifies as 503.
  • Exception class name leaked to the client: The 500 body includes "details":"IllegalArgumentException"; other verification failures return 503 with a human message and no exception type.
  • On the active CLI path: Every CLI request passes through ClerkApiKeyAuthenticationFilter.verify; a key whose Clerk verify reply omits subject (or id) makes CLI commands return 500 until Clerk’s reply shape is corrected.

Code with Bug

// ClerkApiKeyVerifier.java — verify(...)
if (verification == null) {
    throw new ApiKeyOperationUnavailableException("Clerk returned an empty verification response");
}
if (verification.revoked() == null || verification.expired() == null) {
    throw new ApiKeyOperationUnavailableException("Clerk returned incomplete API key lifecycle state");
}
markApiKeyFeatureEnabled();
if (verification.revoked() || verification.expired()) {
    return Optional.empty();
}
VerifiedApiKey verifiedApiKey = new VerifiedApiKey(verification.id(), verification.subject());             // <-- BUG 🔴 missing/blank id/subject throws IllegalArgumentException that is not translated
return Optional.of(verifiedApiKey);

// ClerkApiKeyAuthenticationFilter.java — doFilterInternal(...)
Optional<VerifiedApiKey> verifiedKey;
try {
    verifiedKey = apiKeyLifecycle.verify(presentedSecret.get());
} catch (ApiKeyOperationUnavailableException verificationFailure) {    // <-- BUG 🔴 only this exception is caught; IllegalArgumentException escapes to 500
    writeError(response, HttpStatus.SERVICE_UNAVAILABLE, VERIFICATION_UNAVAILABLE_MESSAGE);
    return;
}

Explanation

ClerkApiKeyVerifier.verify is the provider-boundary translator: it converts upstream Clerk failures/malformed responses into either Optional.empty() (invalid/revoked/expired → 401) or ApiKeyOperationUnavailableException (provider unavailable/malformed → 503). For malformed 200 responses, it explicitly guards lifecycle fields (revoked/expired) and throws ApiKeyOperationUnavailableException when missing, but it does not guard identity fields (id/subject). When those are missing/blank, new VerifiedApiKey(...) throws IllegalArgumentException, which is not caught/translated by verify and is not caught by ClerkApiKeyAuthenticationFilter (which only catches ApiKeyOperationUnavailableException). The exception reaches the servlet error handler and becomes a 500 response exposing the exception class name.

Recommended Fix

Extend verify’s incomplete-response checks to include id/subject before constructing VerifiedApiKey, and throw ApiKeyOperationUnavailableException when they are null/blank so the client receives a consistent 503 for malformed Clerk 200 responses.

History

This bug was introduced in commit abcacee. The initial feature commit added ClerkApiKeyVerifier.verify with lifecycle guards (revoked/expired) but no parallel identity-field guard before constructing VerifiedApiKey, so IllegalArgumentException could escape verify from the start; subsequent hardening commits did not change the identity-construction path.