feat(core): OIDC sign-in via device flow (RFC 8628)

[glasstiger]

Adds interactive OIDC sign-in to the Java client using the OAuth 2.0 Device Authorization Grant (RFC 8628). A process with no local browser — a remote notebook kernel, a container, a headless job — can sign a human in against QuestDB Enterprise: the user authorizes on any device (laptop or phone) while the process only makes outbound calls to the identity provider.

On first use it prints a verification URL and a short code (and, by default, also tries to open the URL in a local browser); once the user authorizes, the token is cached in memory and refreshed silently on later calls.

import io.questdb.client.Sender;
import io.questdb.client.cutlass.auth.OidcDeviceAuth;

// Discover the client id, scope and endpoints from the QuestDB server's /settings:
try (OidcDeviceAuth auth = OidcDeviceAuth.fromQuestDB("https://questdb.example.com:9000")) {
    auth.signIn(); // sign in once: prompts on first use, then caches and refreshes

    // Pass a token provider, not a fixed string: the sender pulls a freshly refreshed token on each
    // request, so a long-lived sender keeps working as the token rotates. getToken() refreshes
    // silently and never prompts on the flush path.
    try (Sender sender = Sender.builder(Sender.Transport.HTTP)
            .address("questdb.example.com:9000")
            .enableTls()
            .httpTokenProvider(auth::getToken)
            .build()) {
        sender.table("trades")
                .symbol("symbol", "ETH-USD")
                .doubleColumn("price", 2615.54)
                .atNow();
    }
}

What's new

OidcDeviceAuth (io.questdb.client.cutlass.auth) — runs the flow and owns the token:

• OidcDeviceAuth.fromQuestDB(url) discovers the client id, scope, audience and IdP endpoints from the server's unauthenticated /settings; OidcDeviceAuth.fromQuestDB(url, DiscoveryOptions) adds an identity-provider pin (.issuer(...)), a TLS config, an allowInsecureTransport opt-in, and the prompt (see Discovery and trust below); OidcDeviceAuth.builder() configures the identity provider explicitly.
• signIn() signs in interactively on first use, then serves a cached token and refreshes it silently; getToken() never prompts and never blocks (safe on a request/flush path); getAuthorizationHeaderValue() returns the full Bearer … value; clearCache() drops the cached token so the next signIn() re-signs-in; close() cancels an in-flight sign-in (observed between polls, so it can take up to one HTTP request timeout to return). Calls are serialized by a ReentrantLock; getToken() uses tryLock and fails fast rather than wait behind an interactive sign-in. Token state lives in memory only and does not survive a process restart.

Sender integration — new HttpTokenProvider interface and Sender.builder(...).httpTokenProvider(auth::getToken). The sender pulls a freshly refreshed token on every request, so a long-lived sender keeps working as the token rotates — unlike a fixed httpToken(...), which is captured once and eventually starts returning 401s. Mutually exclusive with httpToken/httpUsernamePassword. Supported over HTTP and WebSocket transport (a WebSocket sender re-queries the provider on every (re)connect/upgrade); rejected for TCP and UDP. The two transports diverge on a sustained token outage: over HTTP a failed pull leaves the request token-pending and is retried on the next row, whereas over WebSocket a pull that keeps failing past the sender's reconnect budget terminates the sender for good, like any persistent reconnect failure. The first pull is deferred off the build path to the first row, so the documented construct → signIn() → send ordering works and a provider that throws leaves the request retriable instead of corrupting the sender.

QWP egress query client — QwpQueryClient.withBearerTokenProvider(HttpTokenProvider) accepts the same on-demand provider, so OidcDeviceAuth::getToken plugs into the egress query path as well as ingress. The provider is queried at every WebSocket upgrade — the initial connect() and each failover reconnect — so a long-lived query client follows token rotation; each returned token is validated before it reaches the header, and a provider that throws fails that connection attempt (matching the ingress sender). Mutually exclusive with withBearerToken/withBasicAuth.

DeviceCodePrompt / DeviceAuthorizationChallenge — how the verification URL and user code are shown. The default, DeviceCodePrompt.openBrowser(), prints the instructions to System.out and also tries to open the verification URL in the local default browser; the browser open is best-effort (skipped on a headless JVM, without the java.desktop module, or for a non-http(s) URL, and disabled by -Dquestdb.client.oidc.open.browser=false) and never blocks or fails sign-in. Use DeviceCodePrompt.SYSTEM_OUT to print only, or supply your own to render a clickable link or a QR code, e.g. in a notebook.

audience — builder().audience(...) / discovered from acl.oidc.audience. When set, the audience parameter is sent on the device-authorization and refresh requests, for providers that require it to stamp the aud claim QuestDB expects.

The token can be presented to QuestDB over any auth path the server already validates:

• REST / ingestion — Authorization: Bearer <token>.
• PG-wire — connect as _sso with the token as the password (requires acl.oidc.pg.token.as.password.enabled=true on the server).

Discovery and trust

fromQuestDB(...) takes the IdP endpoints from the server's unauthenticated /settings, so by default it trusts that server to designate where the user signs in: a spoofed, compromised, or man-in-the-middled server could otherwise redirect the sign-in — and the long-lived refresh token — to an attacker-controlled identity provider. An optional DiscoveryOptions.issuer(...) pin addresses this, and also covers servers that do not advertise a device-authorization endpoint. The pin separates two sources of endpoints and trusts them differently — an endpoint the untrusted /settings advertised is constrained to the issuer, while an endpoint read from the identity provider's own .well-known is trusted wherever the provider hosts it:

• .well-known discovery fallback. Current servers do not advertise the device-authorization endpoint. When it (and/or the token endpoint) is missing, a pinned issuer reads it from {issuer}/.well-known/openid-configuration. The discovery origin comes only from the caller-supplied issuer, never from a /settings-supplied value, so a tampered /settings cannot choose where discovery — and the credential POSTs it resolves — are aimed. Without a pin, discovery is refused rather than guessed.
• Co-location pin. validateEndpointOrigins, enforced on every construction path (discovery and the explicit builder()), requires the token and device-authorization endpoints to share one origin (RFC 8628 co-locates them on a single authorization server), so a tampered /settings or discovery document cannot siphon one of the two credential POSTs off to a different origin.
• /settings-advertised endpoints are pinned to the issuer. An endpoint the untrusted /settings response supplied must sit on the pinned issuer's origin, and — when the issuer has a path — under that path (compared segment by segment, rejecting ./.., percent-encoded traversal, and a percent-encoded path separator such as %2f or %5c, at every decode level). The path check matters for a path-based provider that shares one origin per tenant (e.g. a Keycloak realm path /realms/<realm>), where the origin check alone cannot stop a tampered /settings from steering credentials to a sibling tenant. The issuer is supplied out of band and cannot be forged.
• IdP-discovered endpoints are trusted where the issuer hosts them. An endpoint read from the issuer's own .well-known is neither origin-pinned nor path-scoped: that document is fetched from the pinned issuer origin and is authoritative for wherever the provider hosts its endpoints. This is deliberate — some providers (e.g. Google, Azure AD) serve their token and device endpoints from a different origin or path than the issuer, and discovery against them signs in normally. The co-location pin above still applies.
• Plaintext-channel pin. A /settings response fetched over plaintext http to a non-loopback host (only reachable with allowInsecureTransport) is MITM-able, so its advertised endpoints are not trusted to route credentials without an issuer pin.

Without a pin, the behaviour against an https server that advertises its endpoints is unchanged: that server is trusted, as before.

Security

• https is required by default for both the QuestDB server and the IdP endpoints; http is rejected unless the caller opts in with allowInsecureTransport(true). That opt-in relaxes only the QuestDB /settings link — the IdP device-authorization and token endpoints always require https (loopback excepted), so the device code and refresh token never cross the network in cleartext (matching the Python client).
• Tokens never leak into logs or exceptions. Only the HTTP status of a token/device response is captured; the body — which carries access, id and refresh tokens — is never retained or surfaced in a message. The captured status is validated to be exactly three digits, so a malformed or hostile status line cannot splice ANSI/control bytes into a later [httpStatus=…] echo, nor can a short all-digit status (2, 5) be misread as a 2xx/5xx class — a malformed-length status falls through to the terminal reject path.
• Untrusted IdP text is sanitized before it is shown in a prompt or an exception message: per-code-point stripping of control characters, ANSI escapes, CR/LF, and bidirectional / zero-width / Unicode-format characters (including supplementary-plane "tag" characters that arrive as surrogate pairs, and unpaired surrogates), so an attacker-influenced field cannot reorder, hide, or forge what a human reads — e.g. a right-to-left override that makes the displayed verification URL differ from the one the browser opens. A user code or verification URL that is non-empty on the wire but sanitizes to nothing is rejected (or, for verification_uri_complete, treated as absent) rather than shown as a blank line or handed to the browser launcher.
• The token itself is validated before use. The token QuestDB will actually receive — the id token when the server encodes groups in the token, the access token otherwise — is rejected if it carries a control or non-ASCII character (outside 0x20–0x7e) before it is cached, placed in the Authorization: Bearer header, or used as the PG-wire password — so a tampered or hostile identity provider cannot smuggle a CR/LF into the request the client then sends to the trusted QuestDB server. Only the served kind is checked; a stray character in the unused token kind, which never reaches the wire, no longer aborts an otherwise usable grant. (The JsonLexer change below decodes JSON escapes, which is what turns a \r/\n in a token into a real byte rather than two literal characters.)
• URLs are validated up front. Endpoint.parse rejects control characters, whitespace and display-unsafe code points anywhere in the url (so a tampered endpoint cannot inject a CR/LF into the request line or a bidi char into a log line), rejects bracketed IPv6 literals rather than mis-parsing them, rejects userinfo (user@host) — which the HTTP layer would otherwise try to connect to literally — terminates the authority at the first /, ? or # so a query or fragment is never folded into the host, and range-checks the port to 1..65535.
• Bounded against a hostile or stalled server. Response reads are capped by a 4 MiB byte limit and a wall-clock deadline that bounds the whole read — covering both a chunked response whose chunk-size line is dribbled a byte at a time and a Content-Length body dribbled through stalled TLS records, either of which previously could keep a single read running well past the deadline. After such a bounded-read abort the half-read poll connection is dropped so the next poll reconnects on a clean socket, rather than the loop spinning on the stalled response's leftover bytes until the device code expires. The device-code lifetime, the poll interval, and the token TTL are all clamped (defaults applied for absent/zero values, hard caps for absurd ones). A 429 with no OAuth error is treated as a transient back-off (a 429 that also carries a terminal error such as access_denied still aborts on the error); a transient transport failure or 5xx during polling keeps polling until the device-code deadline rather than failing the sign-in (RFC 8628; matches the Python client), while a definitive OAuth error or a terminal 4xx aborts immediately. The trade-off is that a persistently flaky network is no longer cut short by a separate error budget — it polls to the device-code deadline.

Supporting changes

• JsonLexer now resolves JSON string escape sequences (\", \\, \/, \b \f \n \r \t, \uXXXX; lenient on malformed input), so string values arrive fully decoded. This also reaches the existing ILP error-response parser, which now sees decoded message/code/line/errorId fields.
• Response.recv(int timeout) (both the Content-Length and chunked implementations) bounds the whole read to the timeout in total, not per socket read, so a server that dribbles the body — the chunk-size line of a chunked response, or Content-Length bytes behind stalled TLS records — cannot keep a single read running past the caller's deadline. A non-positive timeout keeps the legacy unbounded behaviour. The existing ILP flush reads go through the no-arg recv(), which passes the configured client timeout (positive by default), so they now bound the whole body read to that timeout instead of re-arming it per socket read: a response that completes within the timeout is unaffected, while one that legitimately dribbles a single fragment for longer than the timeout — previously tolerated as long as each socket read made progress — now aborts.
• AbstractLineHttpSender plumbs the token provider through with a deferred, retriable per-request pull (a throwing or blank-returning provider leaves the request token-pending for the next row instead of corrupting the half-built request), so the very first send already carries a provider-sourced token. Its error rendering now routes every untrusted server-supplied string through putAsPrintable — the decoded JSON error body, the [http-status=…] field, and the line-protocol-version detection probe body — escaping control and Unicode format characters (bidi overrides, zero-width joiners, the BOM), so a hostile or proxied endpoint cannot reorder, hide, or forge the text shown in a LineSenderException (or spliced into a log line or terminal).
• QwpWebSocketSender sources its auth header from the token provider too, re-querying it on every connect/reconnect so a rotating token keeps a long-lived WebSocket sender authenticated.

Tradeoffs and limitations

• The origin pin behaves differently on the two construction paths. fromQuestDB(...) discovery trusts an endpoint read from the issuer's .well-known wherever the provider hosts it, so an off-origin provider (e.g. Google) signs in normally through a pinned issuer; only an endpoint the /settings response itself advertised is held to the issuer's origin and path. The explicit builder().issuer(...) pin is stricter — a plain sanity check that both supplied endpoints sit on the issuer origin — so an off-origin provider configured that way must have its issuer omitted, or its endpoints supplied to match. This matches the Python client.
• A failed token pull is handled differently per transport: over HTTP it leaves the request token-pending and retries on the next row, but over WebSocket a pull that keeps failing past the sender's reconnect budget terminates the sender, like any persistent reconnect failure. A getToken() provider that fails only transiently recovers on HTTP; a long outage ends a WebSocket sender.
• The co-location check requires the token and device-authorization endpoints to share an origin. A test that previously pointed the token endpoint at a dead second port to simulate an unreachable endpoint was reworked to drop a co-located connection instead (MockOidcServer.dropConnection()).
• The plaintext-channel pin's firing path is exercised end to end by reaching the loopback mock through a short-form 127.x address that the loopback classifier deliberately rejects as non-loopback. That trick relies on the OS resolver expanding the short form (BSD inet_aton, on Linux/macOS), which Windows getaddrinfo does not do, so that one end-to-end test is skipped on Windows; the loopback classifier itself is covered cross-platform.

Tests & docs

OidcDeviceAuthTest (~107 cases) + MockOidcServer, BrowserLauncherTest, LineHttpSenderTokenProviderTest, WebSocketTokenProviderTest + TestWebSocketServer, SenderBuilderErrorApiTest, JsonLexerTest, LineHttpSenderErrorResponseTest, DisplaySafeTest, ChunkedResponseTest/ResponseTest, QwpQueryClientTokenProviderTest; runnable OidcDeviceFlowExample / OIDCAuthExample; and a README "OIDC Sign-In (Device Flow)" section. Coverage includes:

• .well-known discovery via a pinned issuer; a discovery document that omits the device-authorization endpoint
• the co-location check rejecting split-origin endpoints; the builder().issuer(...) origin pin rejecting off-origin endpoints; a /settings-advertised endpoint rejected when off the issuer origin; an endpoint discovered from the issuer's .well-known accepted even when off the issuer origin (the Google case)
• issuer path scoping: endpoints under the issuer path accepted; a sibling realm, percent-encoded traversal, an encoded path separator (%2f), and a percent-encoded backslash (%5c) rejected
• the plaintext-channel pin requiring an issuer pin for advertised endpoints over http (firing path skipped on Windows)
• the audience parameter discovered from /settings and sent on the device and refresh requests
• token-provider support over HTTP and WebSocket (initial upgrade and re-queried per reconnect; static token / username-password still work over WebSocket); rejected for TCP/UDP; mutual exclusion with other auth; null/empty provider token rejected; deferred build-time pull; no sender corruption when the provider throws after a flush; the QWP egress query client's token provider — header synthesis, per-resolve re-query, token validation, and mutual exclusion
• a token with a control or non-ASCII character rejected rather than sent (the served kind validated); tokens never echoed in messages
• bounded reads: a stalled body and an oversized body aborting on the deadline / 4 MiB cap; a chunked body read aborting when the chunk-size line is dribbled; a bounded-read abort dropping the dirty poll connection so the next poll reconnects and signs in
• the poll model: a 429 and a transient 5xx/transport failure keep polling to the deadline; a terminal 4xx and an OAuth error fail fast (including a 429 that also carries a terminal error); slow_down growth and the 60 s interval clamp; device-code-lifetime and clock-skew clamps
• Endpoint.parse rejecting a malformed url: userinfo (user@host), a bracketed IPv6 literal, an out-of-range port, and control/whitespace/display-unsafe characters
• a malformed HTTP status code rejected on both the device-authorization and the token-poll path: a non-numeric status, and a short all-digit status (2, 5) that must not be read as a 2xx/5xx class
• display sanitizing: bidi/zero-width, lone-surrogate and supplementary-plane format characters stripped from the challenge and OAuth error; a server's JSON error body, its HTTP status field, and the protocol-version probe body with such characters escaped, not rendered raw; a user code or verification URL that sanitizes to empty rejected, and a verification_uri_complete that sanitizes to empty treated as absent
• JsonLexer escape decoding, including a \uXXXX escape split across two parse fragments, and the lenient/exotic escape arms
• getToken() failing fast while another thread holds the lock in an interactive sign-in or a silent refresh; native-memory cleanup on the error/rejection construction paths

Tandem

OSS: questdb/questdb#7331
Ent: https://github.com/questdb/questdb-enterprise/pull/1090

[glasstiger]

Code review — OIDC device flow

Reviewed at e1f3d53 (base 84da57d1), 32 files, +8129/−121. This is a deep pass over the security-sensitive surface (token/refresh-token/device-code handling, untrusted IdP + /settings input, issuer origin/path pinning, display sanitizing, bounded reads, the lock/cancellation protocol) plus every existing out-of-diff caller of the shared infrastructure this PR changes. Findings were verified against source, several both-ways on the JVM.

Verdict: approve. No Critical or Moderate issues.

The three highest-blast-radius changes all touch existing ILP code paths and were each verified clean:

• Utf16Sink.putAsPrintable → DisplaySafe (column-name errors, ConfStringParser, ILP error rendering): no existing test pinned the old \u00XX low-byte format or a raw bidi char; legitimate non-ASCII letters (é, ā, CJK, emoji) stay unescaped. C1 controls 0x80–0x9F were emitted raw before and are now correctly escaped — a strict improvement.
• JsonLexer escape decoding reaching the ILP JsonErrorParser: the one pre-existing assertion that would have broken (a\"bc) was correctly updated, the now-redundant interop-harness unescape was removed, and the version-probe key path is covered. No parent-repo ILP test asserts escape-containing text.
• Response.recv(int) whole-call bound on the flush: the bound is per fragment (re-armed each recv() call, default 600s) and a stall becomes a retryable HttpClientException caught by flush0 — intended hardening, not a regression.

Test coverage is genuinely strong (~107 cases, a realistic socket-based MockOidcServer, both-ways regression proofs, direct native-tag leak assertions).

Minor (polish, non-blocking)

1. Per-poll re-encoding of invariant fields — pollOnce (OidcDeviceAuth.java:~1047) and tryRefresh re-run URLEncoder.encode on clientId/device_code/scope on every poll (~5s for the whole sign-in) and every silent refresh, though all are invariant for the flow. The class already precomputes GRANT_TYPE_*_ENCODED at class load — extend that pattern (precompute clientIdEncoded/scopeEncoded, encode device_code once in pollForToken). The two most-worth-doing items are this and chore(conf): update JDK version from 17 to 11 and change tag to 1.0.0 #2.

2. Native-leak regression guard is blind to the real risk — testMalformedEndpointDoesNotLeakNativeMemory feeds not-a-url, which throws at Endpoint.parse before the instance is constructed, so it passes even if build() were reordered to construct-then-validate (the actual leak path). Production ordering is correct — verified both ways on the JVM: a deliberate reorder leaks exactly 1024 bytes, the current order leaks nothing — but the guard doesn't protect it. Use a parseable but rejected endpoint instead (e.g. http://idp.example/device with allowInsecureTransport(false)).

3. Status-validation coverage asymmetry — the short all-digit status trap ("2"/"5" must not read as 2xx/5xx) is tested on the token-poll path but not the device-authorization path, though both share the classifier. A raw("HTTP/1.1 2 OK") device-step case closes it.

4. OidcAuthException static-method ordering (:59 vs :76) — oauthError precedes isUnsafeForDisplay; i should sort before o.

5. Builder.httpTimeoutMillis(int) is unvalidated — every other timing input is clamped, but a non-positive value yields an already-expired read deadline and passes ≤0 to recv(int) (treated as unbounded). Reachable only by explicit misconfiguration; a > 0 guard would be consistent.

6. Javadoc nuance — Sender.httpTokenProvider's "over WebSocket the initial handshake pulls [the token] at build()" is inaccurate for the opt-in ASYNC initial-connect mode, where the first pull is deferred to the I/O thread and a provider failure surfaces via the error inbox. (Default is OFF.)

7. groupsInToken lacks the is/has prefix — public builder API mapping to acl.oidc.groups.encoded.in.token; renaming has API cost, so low priority.

Checked and cleared (so they don't get re-raised)

• WebSocket: a throwing token provider during reconnect is correctly caught — buildAndConnect has a broad catch (Exception e) right after catch (HttpClientException e) that catches the supplier's OidcAuthException/LineSenderException, closes the client, records a transport error, and continues to the next endpoint within the reconnect budget. The code comment is accurate.
• Concurrency: single ReentrantLock; getTokenSilently uses tryLock and fails fast; close() sets the closed volatile before acquiring the lock and never frees the native lexer while a flow holds it — no use-after-free. The documented shared-instance case (a getToken() thread + a Sender flush/reconnect thread) is safe.
• Resource management: no native leak on any error path; jsonLexer is allocated last in the ctor and build() validates before constructing; fetchJson frees the transient lexer + client in finally.
• ILP flush slow-body abort is the intended documented per-fragment bound, not a regression for healthy responses.

Summary

7 confirmed Minor findings, all in-diff; 0 out-of-diff defects (the cross-context callers were walked and verified, not skipped). None blocks merge — #1 and #2 are the two worth addressing before merge. The only change since the prior review pass is the String.repeat → TestUtils.repeat Java-8 test fix in e1f3d53, confirmed correct with no other String.repeat remaining.

_{🤖 Generated with Claude Code}

[mtopolnik]

[PR Coverage check]

😍 pass : 1111 / 1170 (94.96%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/auth/BrowserLauncher.java	14	21	66.67%
🔵	io/questdb/client/cutlass/auth/DeviceCodePrompt.java	22	24	91.67%
🔵	io/questdb/client/cutlass/qwp/client/QwpQueryClient.java	21	23	91.30%
🔵	io/questdb/client/cutlass/auth/OidcAuthException.java	31	33	93.94%
🔵	io/questdb/client/cutlass/auth/OidcDeviceAuth.java	823	867	94.93%
🔵	io/questdb/client/Sender.java	29	30	96.67%
🔵	io/questdb/client/cutlass/line/http/AbstractLineHttpSender.java	35	36	97.22%
🔵	io/questdb/client/std/str/Utf16Sink.java	10	10	100.00%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	6	6	100.00%
🔵	io/questdb/client/cutlass/auth/DeviceAuthorizationChallenge.java	12	12	100.00%
🔵	io/questdb/client/cutlass/http/client/AbstractChunkedResponse.java	8	8	100.00%
🔵	io/questdb/client/cutlass/json/JsonLexer.java	65	65	100.00%
🔵	io/questdb/client/cutlass/http/client/AbstractResponse.java	8	8	100.00%
🔵	io/questdb/client/std/str/DisplaySafe.java	20	20	100.00%
🔵	io/questdb/client/HttpTokenProvider.java	7	7	100.00%

[glasstiger]

Code review — OIDC device flow (level-3 re-review)

Re-reviewed at 1b7fecd (base 84da57d), 36 files, +8546/−129. This pass focuses on the 4 commits since the prior review at e1f3d53 — the QWP egress token provider, the getToken/getTokenSilently → signIn/getToken method swap, pre-encoding of the OIDC form params, and the Java-8 build fixes — and re-verifies the full security surface (token/refresh-token/device-code handling, untrusted IdP + /settings input, issuer origin/path pinning, display sanitizing, bounded reads, lock/cancellation). Findings were verified against source; the build (JDK 25) and the relevant suites (~120 tests) were run green.

Verdict: approve. No Critical or Moderate issues.

The four highest-blast-radius changes were each verified:

• The method swap is complete and correct. Zero lingering getTokenSilently references. Every request/flush/reconnect caller (AbstractLineHttpSender:772, Sender:2936, QwpQueryClient:1812) uses the non-blocking getToken(), never the blocking signIn(); the README and examples call signIn() once first. Bodies/locking swapped correctly: signIn() = lock.lock() + runDeviceFlow, getToken() = tryLock fail-fast, never runs the flow.
• Pre-encoding is wire-byte-identical. clientIdEncoded/scopeEncoded/audienceEncoded (ctor) and deviceCodeEncoded (once in pollForToken) produce exactly the old per-call bytes. The dropped if (scope != null) guard in tryRefresh is safe — build() forces scope = DEFAULT_SCOPE, so scope is never null; audience stays null-guarded.
• The QWP egress provider is correct. resolveAuthorizationHeader() resolves once before the endpoint walk in both connect() and reconnectViaTracker(), validates the token, and propagates a provider throw with its own error (not folded into "all endpoints unreachable"). Mutual exclusion is bidirectional; withBearerTokenProvider is in the post-connect guard test.
• Java 8 floor is clean. Exhaustive scan of all 36 changed files (main + test) found no Java 9+ API/syntax; the URLEncoder String-charset revert held.

Security invariants all still hold after the follow-ups: served-kind token validation on every path, full percent-encoding of attacker-influenced form values, display sanitizing (bidi/zero-width/surrogate/tag), the 3-digit status guard on both device-auth and poll paths, endpoint/issuer origin+path pinning (no traversal bypass found under adversarial probing), bounded reads, and no token leakage into messages.

Minor (polish, non-blocking)

1. Native-leak regression guard is still blind to the real risk. OidcDeviceAuthTest.testMalformedEndpointDoesNotLeakNativeMemory feeds "not-a-url", which throws in Endpoint.parse inside build() before the constructor allocates the native lexer, so the strong NATIVE_TEXT_PARSER_RSS measurement never exercises a parseable-but-rejected endpoint — the actual construct-then-validate reorder it guards. Production ordering is correct (lexer last; build() validates first), verified both ways. Raised at e1f3d53 and still open. Fix: add a measured block using deviceAuthorizationEndpoint("http://idp.example/device") + allowInsecureTransport(false) (parses, then rejected by the https check) and assert the tag is unchanged. This is the one item most worth doing before merge.

2. Egress/ingress provider-failure asymmetry. Egress QwpQueryClient fail-fasts on a token-provider throw (resolves once before the walk, propagates the real error), but ingress QwpWebSocketSender.buildAndConnect pulls inside the per-endpoint loop and treats a throw as a transport error, retrying within the reconnect budget (catch (Exception e) — correct, and the comment is accurate). Both are correct and documented, but the two WebSocket clients diverge for an identical failure. Consider aligning QwpWebSocketSender to resolve-once-before-walk for consistent errors. (Optional.)

3. Minor test-coverage gaps (all low): no direct short-all-digit status case ("HTTP/1.1 2 OK") on the device-authorization path (the poll path has one; covered indirectly via the shared classifier); the QWP per-reconnect re-query is asserted only via the @TestOnly hook rather than a real multi-endpoint failover on the wire; withBasicAuth(...).withBearerTokenProvider(...) ordering is untested; QWP bad-token rejection is asserted via the hook, not a real connect().

4. refresh_token re-encoded per refresh. tryRefresh re-runs urlEncode(refreshToken) on each silent refresh (~once per token lifetime), while every other param is pre-encoded. Cold cadence — pre-encoding it in storeTokens would complete the pattern. Nit.

5. Comment nit. OidcDeviceAuth.java:103 describes the clock-skew/expiry logic as getToken()'s, but after the rename both getToken() and signIn() share it.

Checked and cleared (so they don't get re-raised)

• OidcAuthException static-method ordering (oauthError before isUnsafeForDisplay): correct — members are grouped by kind and visibility then alphabetical, so public-static precedes package-private-static. The earlier flag was over-strict.
• QwpWebSocketSender "caught below" comment: accurate — the provider throw is caught by the broad catch (Exception e) (it doesn't claim a specific handler), which records a transport error and continues. Not a defect.
• tryRefresh silent-refresh token validation: covered structurally — storeTokens (the single choke point) validates the served kind unconditionally on both the device-flow and refresh paths.
• groupsInToken naming: no public boolean getter exists; it's a private final field plus a fluent builder setter, matching scope(...)/audience(...).
• Concurrency / resources: single ReentrantLock; getToken() uses tryLock and fails fast; close() sets the closed volatile before acquiring the lock and never frees the native lexer while a flow holds it — no use-after-free; the jsonLexer is allocated last in the ctor and freed once.

Summary

~12 draft findings verified, ~4 dropped as false positives. 5 in-diff minor items, 0 out-of-diff defects — the cross-context pass walked every out-of-diff caller of the changed shared symbols (Sender, AbstractLineHttpSender, README, examples) and confirmed all safe. None blocks merge; #1 (strengthen the native-leak guard) is the only item worth addressing before merge.

_{🤖 Generated with Claude Code}

[glasstiger]

Code review — OIDC device flow (independent level-3 pass)

Reviewed at 1b7fecd (base 84da57d), 36 files, +8546/−129. Independent 9-dimension adversarial pass (correctness, concurrency, resources, cross-context callers, performance ×2, tests, code quality, metadata, plus a fresh-context "find what's wrong" agent), every finding verified against working-tree source — not a re-confirmation of the prior passes.

Process note for anyone re-reviewing: gh pr diff 52 currently serves a stale cached diff for this PR — it shows a removed auth::getTokenSilently reference and older Javadoc wording that do not exist at the real head 1b7fecd. The authoritative diff is git diff $(git merge-base HEAD origin/main)...HEAD; read source from the checked-out head. Findings below are against the real head.

Verdict: approve. No Critical or Moderate issues.

The three highest-blast-radius changes all touch the existing ILP path and were each verified safe at their out-of-diff callers (incl. a cross-module compile of OSS questdb/core against the changed client + 522 passing tests across the affected suites):

• Utf16Sink.putAsPrintable — for 0x00–0x1F/0x7F the new \uXXXX output is byte-identical to the old \u00XX; only C1/FORMAT/surrogate chars change from raw to escaped (strictly safer). Legitimate non-ASCII letters/emoji still pass raw. No existing caller (ConfStringParser, column-name errors, ILP error rendering) depended on the old format.
• JsonLexer escape decoding — the new hasEscape fast-path returns the raw sink unchanged for escape-free values (the common case for the ILP JsonErrorParser); decoded values are re-escaped via putAsPrintable before display; numeric fields carry no backslash and are unaffected.
• Response.recv(int) whole-call bound — active on the ILP flush (30s default, re-armed per recv() call, so a large body across many recv()s is unaffected), bites only a single fragment read that stalls ~30s, and surfaces as a retryable HttpClientException caught in flush0.

This is an unusually well-defended PR: CRLF/header injection, bidi/ANSI terminal spoofing, MITM endpoint redirection (origin + path pinning with multi-decode traversal defense), token-kind confusion, use-after-free on close(), and unbounded reads were each anticipated and hold up under adversarial tracing.

Minor (none blocking)

1. Tautological native-leak guard — OidcDeviceAuthTest.testMalformedEndpointDoesNotLeakNativeMemory (~:1993). "not-a-url" throws in Endpoint.parse before the native JsonLexer is allocated (it's the last ctor statement), so the RSS-equality assertion can never fail — it doesn't guard the construct-then-validate ordering it claims to. Production ordering is safe. Fix: use a parseable-but-rejected endpoint, e.g. deviceAuthorizationEndpoint("http://idp.example/device") + allowInsecureTransport(false). (Still open from the prior passes — the one item most worth doing before merge.)

2. Test gap: short all-digit status on the device-authorization path. The 3-digit isHttpStatusSuccess() guard is tested on the token-poll path but not at the device-auth call site (runDeviceFlow, :1195); the device-auth status tests use a non-numeric status, a different code path. Add a raw("HTTP/1.1 2 OK") device-step case.

3. Test gap: throwing httpTokenProvider on a WebSocket ingest sender reconnect. Only the QWP QueryClient initial-connect throw and the WS happy-reconnect re-query are covered. Production handling is correct — the throw (OidcAuthException/LineSenderException, both RuntimeException) is caught by catch (Exception e) at QwpWebSocketSender.java:2500, the client is closed, and it retries within the reconnect budget — but a regression narrowing that catch would ship green. (The :2456 "caught below" comment is accurate, just non-specific — not a defect.)

4. Test gap: ILP flush recv() whole-call timeout end-to-end. ResponseTest/ChunkedResponseTest cover the unit-level bound well, but no integration test drives a real Sender.flush() against a dribbling/stalled server to confirm the abort fires and is classified retryable.

5. OIDC JSON parsers track object depth only, ignoring array nesting. All four inner parsers bump depth on EVT_OBJ_START/END but ignore EVT_ARRAY_START/END, so array-wrapping ({"config":[{…}]} / [{…}]) extracts fields from RFC-malformed shapes, subverting the depth-based "only top-level config trusted" intent. No practical security/correctness impact: the /settings body is already fully attacker-controlled in the fromQuestDB threat model (array-wrapping grants no new capability), endpoints stay origin/path-pinned regardless of parse shape, a preferences value still can't be read as config, and a real QuestDB server never returns arrays there. Robustness/defense-in-depth nit — optionally track array depth or reject a non-object top level.

6. Cold-path perf nits (optional). tryRefresh re-encodes refresh_token per silent refresh (invariant per token lifetime — could pre-encode in storeTokens); the issuer-path check runs two independent multi-pass percent-decodes over the same path (endpointPathHasEncodedSeparator then decodePathSegments — fusible); percentDecodeOnce allocates a fresh StringSink per call. All run once per construction/refresh, off the data path. The per-row/per-flush path is allocation-clean and the Authorization: Bearer write is allocation-free.

7. PR description: disclose the ILP-flush timeout change under "Tradeoffs", not "Supporting changes". The Response.recv(int) whole-call bound affects existing non-OIDC senders; the repo convention is "regressions as prominently as improvements." Worth one line in Tradeoffs noting a flush aborted by the tighter deadline is subject to normal retry (the pre-existing ILP-over-HTTP at-least-once window, marginally widened). Labels enhancement+security are correct (the ILP/REST API/Core labels don't exist in this submodule repo).

8. Naming/comment nits (non-actionable). groupsInToken/allowInsecureTransport lack the is/has prefix but follow the fluent-builder idiom (mirroring scope()/audience()); the :103 clock-skew comment names only getToken() though signIn() shares the logic (accurate but incomplete).

Checked and dismissed (so they don't get re-raised)

• Utf16Sink.putAsPrintable "misses supplementary-plane tag chars" — false: the CharSequence overload scans by code point via DisplaySafe and does strip them.
• Use-after-free of the native jsonLexer on close() — refuted: close() sets the closed volatile then lock.lock(), blocking until any in-flight parse releases the lock; the lexer is never freed mid-parse.
• OOB read on a trailing backslash in JsonLexer.unescape — refuted: the lexer's ignoreNext guarantees a char follows every \ before the closing quote; the i+1>=n guard is dead-but-correct.
• OidcAuthException static-method ordering — correct (public-static before package-private-static, then alphabetical).

Summary

~8 Minor items, none blocking: 4 test-coverage/quality gaps (#1–#4), 1 robustness nit (#5), 1 optional cold-path perf cleanup (#6), 1 PR-description prominence fix (#7), naming/comment nits (#8). ~6 draft findings dropped as false positives after source verification. Every confirmed finding is in-diff (tests / new parser code / PR body); the high-blast-radius out-of-diff callers (ILP error rendering, JsonLexer consumers, recv()/createLineSender callers, getAuthorizationHeaderForTest) were walked and verified safe — 0 out-of-diff defects. The single item most worth doing before merge is #1.

_{🤖 Generated with Claude Code}