Tighten OIDC token-kind and status validation

Three minor follow-ups to the device-flow review:

- storeTokens validated both the access_token and the id_token for
  control/non-ASCII chars, but getToken() only ever serves (and sends)
  one of them. A stray char in the unused kind - which never reaches a
  header or a PG-wire password - aborted an otherwise-usable grant.
  Validate only the served kind (groupsInToken ? idToken : accessToken);
  the served kind is still strictly checked.

- The HTTP status classifiers keyed off the first digit with only a
  length > 0 guard, so a malformed short all-digit status like "2" or
  "5" could be read as a 2xx/5xx. readResponse already guarantees bare
  digits; require exactly 3 of them, so a malformed-length status falls
  through to the terminal reject path instead of being trusted.

- getTokenSilently's javadoc claimed it "never blocks". It does not wait
  on interactive input or behind another thread's sign-in, but it can
  make one synchronous refresh round-trip bounded by httpTimeoutMillis.
  Reword to say so, matching the HttpTokenProvider contract.

Tests (both proven to fail without their fix):
- a control char in the unused id_token (groupsInToken=false) no longer
  aborts the grant;
- a 1-digit "2" status from the token endpoint is rejected, not accepted
  as success, and the smuggled token never surfaces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

core/src/main/java/io/questdb/client/cutlass/auth/OidcDeviceAuth.java

@@ -440,16 +440,18 @@ public String getToken() {
     }
 
     /**
-     * Like {@link #getToken()} but never starts the interactive device flow and never blocks: returns
-     * the cached token while valid, silently refreshes when a refresh token is available, otherwise
-     * throws. Designed for the request/flush path of a long-lived client, for example
-     * {@code Sender.builder(...).httpTokenProvider(auth::getTokenSilently)}, where an interactive prompt
-     * is inappropriate and a stalled flush unacceptable. Call {@link #getToken()} once to sign in first.
+     * Like {@link #getToken()} but never starts the interactive device flow, never prompts, and never waits
+     * on interactive input: returns the cached token while valid, silently refreshes when a refresh token is
+     * available, otherwise throws. Designed for the request/flush path of a long-lived client, for example
+     * {@code Sender.builder(...).httpTokenProvider(auth::getTokenSilently)}, where an interactive prompt is
+     * inappropriate. Call {@link #getToken()} once to sign in first.
      * <p>
-     * To keep the flush path responsive it returns or throws promptly - it never waits for an interactive
-     * {@link #getToken()} on another thread (which would stall the flush for the whole device-code
-     * lifetime). While such a sign-in runs there is no token to return anyway, so it throws and the caller
-     * should retry once the sign-in completes.
+     * It does not wait behind an interactive {@link #getToken()} running on another thread (which would stall
+     * the flush for the whole device-code lifetime): if such a sign-in holds the lock it fails fast, and the
+     * caller should retry once the sign-in completes. It is not, however, instantaneous - when the cached
+     * token has expired it makes one synchronous refresh round-trip to the token endpoint, bounded by
+     * {@link Builder#httpTimeoutMillis(int)} (30s by default). That is the "quick silent refresh" the
+     * {@code HttpTokenProvider} contract permits on the flush path, not an unbounded interactive wait.
      *
      * @return a non-null, non-empty token
      * @throws OidcAuthException if no token has been obtained yet, if the cached token expired and could
@@ -979,19 +981,23 @@ private HttpClient httpClient(boolean isTls) {
     }
 
     private boolean isHttpStatusSuccess() {
-        // responseStatus is the numeric HTTP status captured by readResponse; a 2xx starts with '2'
-        return responseStatus.length() > 0 && responseStatus.charAt(0) == '2';
+        // responseStatus is the bare-digit HTTP status captured by readResponse. A real status is exactly 3
+        // digits, so require that before reading the leading digit: a malformed short status such as "2" must
+        // not be mistaken for a 2xx success and accepted as a grant.
+        return responseStatus.length() == 3 && responseStatus.charAt(0) == '2';
     }
 
     private boolean isHttpStatusTerminal4xx() {
-        // a 4xx other than 429 is a terminal client-error rejection (429 is a transient rate-limit)
-        return responseStatus.length() > 0 && responseStatus.charAt(0) == '4' && !Chars.equals(HTTP_STATUS_TOO_MANY_REQUESTS, responseStatus);
+        // a 4xx other than 429 is a terminal client-error rejection (429 is a transient rate-limit); require a
+        // full 3-digit status so a malformed short "4" is not classified as a terminal 4xx
+        return responseStatus.length() == 3 && responseStatus.charAt(0) == '4' && !Chars.equals(HTTP_STATUS_TOO_MANY_REQUESTS, responseStatus);
     }
 
     private boolean isHttpStatusTransient() {
         // a 5xx server error or a 429 rate-limit is transient - keep polling; any other non-2xx (a 4xx
-        // rejection) is terminal. Mirrors the Python client's _http_status_is_transient.
-        return responseStatus.length() > 0 && (responseStatus.charAt(0) == '5' || Chars.equals(HTTP_STATUS_TOO_MANY_REQUESTS, responseStatus));
+        // rejection) is terminal. Mirrors the Python client's _http_status_is_transient. Require a full
+        // 3-digit status so a malformed short "5" is not classified as a transient 5xx.
+        return responseStatus.length() == 3 && (responseStatus.charAt(0) == '5' || Chars.equals(HTTP_STATUS_TOO_MANY_REQUESTS, responseStatus));
     }
 
     private void pollForToken(String deviceCode, int expiresInSeconds, int intervalSeconds) {
@@ -1241,11 +1247,16 @@ private void sleepBetweenPolls(long millis) {
     }
 
     private void storeTokens(TokenResponseParser parser) {
-        // reject a token with control or non-ASCII chars before caching: getToken() serves it verbatim as
-        // an HTTP Authorization header value and a PG-wire password, where a decoded CR/LF would inject
-        // into the request line sent to the trusted QuestDB server
-        validateTokenChars(parser.accessToken, "access_token");
-        validateTokenChars(parser.idToken, "id_token");
+        // reject a token with control or non-ASCII chars before caching: getToken() serves it verbatim as an
+        // HTTP Authorization header value and a PG-wire password, where a decoded CR/LF would inject into the
+        // request line sent to the trusted QuestDB server. Validate only the kind getToken() actually serves
+        // (the one that reaches the wire); the other kind is cached but never sent, so a stray char in it must
+        // not abort an otherwise-usable grant.
+        if (groupsInToken) {
+            validateTokenChars(parser.idToken, "id_token");
+        } else {
+            validateTokenChars(parser.accessToken, "access_token");
+        }
         accessToken = parser.accessToken.length() > 0 ? parser.accessToken.toString() : null;
         idToken = parser.idToken.length() > 0 ? parser.idToken.toString() : null;
         // a refresh response usually omits a new refresh token; keep the current one in that case

core/src/test/java/io/questdb/client/test/cutlass/auth/OidcDeviceAuthTest.java

@@ -3176,6 +3176,69 @@ private static OidcDeviceAuth.DiscoveryOptions insecure() {
         return new OidcDeviceAuth.DiscoveryOptions().allowInsecureTransport(true).prompt(noopPrompt());
     }
 
+    @Test(timeout = 30_000)
+    public void testControlCharInUnusedTokenKindDoesNotAbortGrant() throws Exception {
+        assertMemoryLeak(() -> {
+            // groupsInToken=false, so getToken() serves and sends only the access_token; the id_token is
+            // cached but never placed in a header or a PG-wire password. A control char in that unused id_token
+            // must not reject an otherwise-usable grant - only the served kind is validated for wire safety
+            MockOidcServer.Handler handler = (method, path, body) -> {
+                if (DEVICE_PATH.equals(path)) {
+                    return MockOidcServer.json(200, "{"
+                            + "\"device_code\":\"DEV\","
+                            + "\"user_code\":\"WDJB-MJHT\","
+                            + "\"verification_uri\":\"https://verify.example/device\","
+                            + "\"expires_in\":300,"
+                            + "\"interval\":1"
+                            + "}");
+                }
+                // a clean access_token (the served kind) alongside an id_token carrying a decoded control char
+                return MockOidcServer.json(200, tokenJson("CLEAN-ACCESS", "bad" + jsonUnicodeEscape(0x0001) + "id", null, 3600));
+            };
+            try (MockOidcServer server = new MockOidcServer(handler);
+                 OidcDeviceAuth auth = newAuth(server, false, noopPrompt())) {
+                Assert.assertEquals("CLEAN-ACCESS", auth.getToken());
+            }
+        });
+    }
+
+    @Test(timeout = 30_000)
+    public void testShortAllDigitStatusIsNotTreatedAsSuccess() throws Exception {
+        assertMemoryLeak(() -> {
+            // a real HTTP status is exactly 3 digits; a malformed 1-digit "2" (all digits, so readResponse
+            // accepts it) must not be classified as a 2xx success by its leading digit and accepted as a grant
+            String tokenBody = tokenJson("SHOULD-NOT-ACCEPT", null, null, 3600);
+            String rawToken = "HTTP/1.1 2 OK\r\n"
+                    + "Content-Type: application/json\r\n"
+                    + "Transfer-Encoding: chunked\r\n\r\n"
+                    + Integer.toHexString(tokenBody.length()) + "\r\n" + tokenBody + "\r\n"
+                    + "0\r\n\r\n";
+            MockOidcServer.Handler handler = (method, path, body) -> {
+                if (DEVICE_PATH.equals(path)) {
+                    return MockOidcServer.json(200, "{"
+                            + "\"device_code\":\"DEV\","
+                            + "\"user_code\":\"WDJB-MJHT\","
+                            + "\"verification_uri\":\"https://verify.example/device\","
+                            + "\"expires_in\":300,"
+                            + "\"interval\":1"
+                            + "}");
+                }
+                return MockOidcServer.raw(rawToken);
+            };
+            try (MockOidcServer server = new MockOidcServer(handler);
+                 OidcDeviceAuth auth = newAuth(server, false, noopPrompt())) {
+                try {
+                    auth.getToken();
+                    Assert.fail("expected a malformed 1-digit status to be rejected, not accepted as success");
+                } catch (OidcAuthException e) {
+                    String msg = e.getMessage();
+                    Assert.assertTrue(msg, msg.contains("rejected the request") || msg.contains("refusing to keep polling"));
+                    Assert.assertFalse("the unaccepted token must not leak: " + msg, msg.contains("SHOULD-NOT-ACCEPT"));
+                }
+            }
+        });
+    }
+
     // Forces the cached access/id token to look expired WITHOUT dropping the refresh token, so the next
     // getToken()/getTokenSilently() takes the silent-refresh (or interactive re-sign-in) path. Reflection
     // because the field is private and there is no configurable clock skew to lean on anymore; the client is