Cap token-store lock budget; add persistence tests

Address the moderate findings from the OIDC token-persistence review.

getToken() runs on the latency-sensitive flush and reconnect path,
but persistence routed it through FileTokenStore's cross-process
lock, which could spin up to the acquire budget before a silent
refresh. The advanced FileTokenStore constructor accepted an
unbounded budget, so a misconfiguration could stall a flush. Cap the
acquire budget at 30s, and reconcile the HttpTokenProvider SPI and
the OidcDeviceAuth class docs, which still claimed getToken() never
blocks: it never waits behind an interactive sign-in, but a
coordinated silent refresh may briefly wait for the store lock.

Add the missing tests on the security-critical persistence branches
(CI had not covered these files): schema-version mismatch, per-field
fingerprint mismatch (including groups_in_token and the audience
nullableEquals path), the httpTimeoutMillis cap, a control-char JSON
round-trip, the 0600 lock-file permissions, getToken() degrading to
a lock-free refresh while a peer holds the lock, and a golden-hash
anchor pinning the cross-language on-disk file-name contract. Also
assert the previously-unchecked load/clear/lock store counters, the
load-once guarantee, and clearCache() not reloading a just-cleared
entry.

FileTokenStoreTest 19 -> 26, OidcDeviceAuthPersistenceTest 14 -> 18;
all green.

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

Author: glasstiger

core/src/main/java/io/questdb/client/HttpTokenProvider.java

@@ -36,8 +36,10 @@
  * <p>
  * {@link #getToken()} runs on the sender's flush and reconnect paths: it must return promptly and must
  * not block on interactive input. A quick silent token refresh is fine, but it must not start an
- * interactive sign-in. An exception from {@link #getToken()} fails the in-flight flush (HTTP) or the
- * connection attempt (WebSocket).
+ * interactive sign-in; a provider that coordinates a shared token store across processes (for example
+ * {@code OidcDeviceAuth} with a {@code FileTokenStore}) may add a brief, bounded wait to acquire that
+ * store's cross-process lock before such a refresh, which still counts as a quick silent refresh. An
+ * exception from {@link #getToken()} fails the in-flight flush (HTTP) or the connection attempt (WebSocket).
  *
  * @see Sender.LineSenderBuilder#httpTokenProvider(HttpTokenProvider)
  */

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

@@ -105,6 +105,12 @@ public final class FileTokenStore implements TokenStore {
     // reject a token file larger than this; a real entry is a few KB even with a group-laden id token, so
     // anything past this is corrupt or hostile and is not read into memory
     private static final long MAX_FILE_BYTES = 1 << 20;
+    // upper bound on the configurable lock acquire budget. getToken() can take this lock on the
+    // latency-sensitive flush path, so a caller-supplied budget is kept short: a real peer refresh is
+    // sub-second, and bounding the wait stops a misconfigured budget from stalling a flush before it degrades
+    // to a lock-free refresh (Layer 1 still guards integrity). Stays well below DEFAULT_LOCK_STALE_MILLIS so a
+    // waiter degrades long before it could begin stealing live locks.
+    private static final long MAX_LOCK_ACQUIRE_BUDGET_MILLIS = 30_000L;
     private static final int SCHEMA_VERSION = 1;
     // set once if the platform cannot enforce owner-only POSIX permissions on the token files (e.g. Windows),
     // so the at-rest protection falls back to the directory's inherited ACL; warns the user once
@@ -124,7 +130,9 @@ public FileTokenStore(Path directory) {
      *
      * @param directory               the directory to hold the token files
      * @param lockAcquireBudgetMillis  how long {@code inLock} waits to acquire a peer's lock before degrading
-     *                                 to a lock-free refresh rather than stalling a sign-in
+     *                                 to a lock-free refresh rather than stalling a sign-in. Must be positive
+     *                                 and at most 30_000 (30s): {@code getToken()} can wait it out on the
+     *                                 latency-sensitive flush path, so it is kept short
      * @param lockStaleMillis          a lock older than this is treated as abandoned by a crashed holder and
      *                                 stolen. It MUST exceed the longest a live holder can hold the lock: one
      *                                 refresh under the lock runs send + await + parse plus a body drain, each
@@ -142,6 +150,12 @@ public FileTokenStore(Path directory, long lockAcquireBudgetMillis, long lockSta
         if (lockAcquireBudgetMillis <= 0) {
             throw new OidcAuthException("the token store lockAcquireBudgetMillis must be positive");
         }
+        if (lockAcquireBudgetMillis > MAX_LOCK_ACQUIRE_BUDGET_MILLIS) {
+            // getToken() can wait out this budget on the latency-sensitive flush path, so an unbounded value
+            // would let a misconfiguration stall a flush; keep it short - it degrades to a lock-free refresh
+            throw new OidcAuthException()
+                    .put("the token store lockAcquireBudgetMillis must not exceed ").put(MAX_LOCK_ACQUIRE_BUDGET_MILLIS);
+        }
         // a non-positive staleness window makes every freshly created lock look abandoned, so acquirers would
         // steal each other's live locks; keep it well above one refresh round-trip (see the default)
         if (lockStaleMillis <= 0) {

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

@@ -85,8 +85,10 @@
  * exists, otherwise re-runs the interactive flow. An instance lock serializes calls, so two
  * sign-ins never start at once. A sign-in waiting for the user holds that lock for the device code
  * lifetime (up to 30 minutes), so a concurrent {@link #signIn()} or {@link #clearCache()} blocks
- * behind it - but {@link #getToken()} never waits: it fails fast with an
- * {@link OidcAuthException} so a request/flush path never stalls. To abort a waiting sign-in, call
+ * behind it - but {@link #getToken()} never waits behind an interactive sign-in: it fails fast with an
+ * {@link OidcAuthException} rather than stall a request/flush path (a needed silent refresh still runs,
+ * bounded by {@link Builder#httpTimeoutMillis(int)} plus, with a coordinating {@link TokenStore}, a brief
+ * cross-process lock wait - see {@link #getToken()}). To abort a waiting sign-in, call
  * {@link #close()} from another thread; it signals the flow to stop, which then fails with an
  * {@link OidcAuthException} rather than polling until the device code expires. Cancellation is seen
  * between polls (within ~100ms while waiting out an interval); a poll already in flight is not

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

@@ -85,6 +85,53 @@ public void testAdvancedConstructorRejectsNonPositiveTimings() throws Exception
         });
     }
 
+    @Test
+    public void testAdvancedConstructorRejectsOverCapAcquireBudget() throws Exception {
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            // the acquire budget is capped: getToken() can wait it out on the latency-sensitive flush path, so
+            // an unbounded budget would let a misconfiguration stall a flush; the cap also keeps a waiter
+            // degrading well before it could begin stealing live locks
+            try {
+                new FileTokenStore(dir, 30_001, 600_000);
+                Assert.fail("an over-cap lock acquire budget must be rejected");
+            } catch (OidcAuthException expected) {
+                Assert.assertTrue(expected.getMessage(), expected.getMessage().contains("lockAcquireBudgetMillis"));
+            }
+            // the cap boundary itself is accepted
+            new FileTokenStore(dir, 30_000, 600_000);
+        });
+    }
+
+    @Test
+    public void testAudienceNullVersusEmptyFingerprint() throws Exception {
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            FileTokenStore store = new FileTokenStore(dir);
+            TokenStoreKey nullAud = new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                    "https://idp.example.com:443/device", "openid", null, false);
+            TokenStoreKey withAud = new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                    "https://idp.example.com:443/device", "openid", "api://billing", false);
+
+            // a null audience round-trips: the writer omits the member, and nullableEquals matches an absent
+            // file value against a null key audience
+            store.save(nullAud, sampleToken("ACCESS-1", "REFRESH-1"));
+            Assert.assertNotNull(store.load(nullAud));
+            byte[] nullAudBytes = Files.readAllBytes(tokenFile(dir, nullAud));
+
+            store.save(withAud, sampleToken("ACCESS-2", "REFRESH-2"));
+            byte[] withAudBytes = Files.readAllBytes(tokenFile(dir, withAud));
+
+            // place each file under the *other* key's name to isolate the in-file audience fingerprint check
+            // from the hash-based file naming: a recorded audience must not match a null-audience key, and an
+            // absent audience must not match an audience-bearing key
+            Files.write(tokenFile(dir, nullAud), withAudBytes);
+            Assert.assertNull("a recorded audience must not match a null-audience key", store.load(nullAud));
+            Files.write(tokenFile(dir, withAud), nullAudBytes);
+            Assert.assertNull("an absent audience must not match an audience-bearing key", store.load(withAud));
+        });
+    }
+
     @Test
     public void testClearDeletesFile() throws Exception {
         assertMemoryLeak(() -> {
@@ -102,6 +149,26 @@ public void testClearDeletesFile() throws Exception {
         });
     }
 
+    @Test
+    public void testControlCharactersRoundTrip() throws Exception {
+        assertMemoryLeak(() -> {
+            FileTokenStore store = new FileTokenStore(storeDir());
+            TokenStoreKey key = sampleKey();
+            // a refresh token carrying every control-escape branch of the JSON writer - the short escapes
+            // (\b \f \n \r \t) and the \\u00XX arm - plus a quote and a backslash must round-trip byte for byte;
+            // the served-token char check lives in OidcDeviceAuth, so the store itself must preserve these
+            String refresh = "R\b\f\n\r\t
\"\\Z";
+            // also exercise the hex-escape branch: control chars below 0x20 that are not one of the short escapes
+            refresh = refresh + (char) 0x01 + (char) 0x1f;
+            store.save(key, new PersistedToken("ACCESS-1", null, refresh, 1L, 1000L));
+
+            PersistedToken loaded = store.load(key);
+            Assert.assertNotNull(loaded);
+            Assert.assertEquals("ACCESS-1", loaded.getAccessToken());
+            Assert.assertEquals(refresh, loaded.getRefreshToken());
+        });
+    }
+
     @Test
     public void testCorruptFileReturnsNull() throws Exception {
         assertMemoryLeak(() -> {
@@ -163,6 +230,34 @@ public void testFingerprintMismatchReturnsNull() throws Exception {
         });
     }
 
+    @Test
+    public void testHashMatchesFrozenCrossLanguageContract() throws Exception {
+        assertMemoryLeak(() -> {
+            // the file name is a frozen cross-language contract (the Python client mirrors it byte for byte):
+            // lowercase-hex SHA-256 of "questdb-oidc-token-v1" and the six identity fields, NUL-separated, a
+            // null audience rendered as "" and groups_in_token as '1'/'0'. Pin it to golden values so a change
+            // to the prefix, separator, field order, or null/boolean encoding that would silently stop two
+            // clients sharing one file is caught here.
+            TokenStoreKey withAudience = new TokenStoreKey("questdb",
+                    "https://idp.example.com:443/as/token", "https://idp.example.com:443/as/device",
+                    "openid", "api://billing", false);
+            Assert.assertEquals("eee1a742a27499d176bcdaed8635c14a3edbdef1d68b61c05c3c2158a5bfbcca", withAudience.hash());
+
+            // a null audience hashes as an empty field, not the literal "null"
+            TokenStoreKey nullAudience = new TokenStoreKey("questdb",
+                    "https://idp.example.com:443/as/token", "https://idp.example.com:443/as/device",
+                    "openid", null, false);
+            Assert.assertEquals("1dca0e8192ae529b94c1ac5493f09f8a45e641e4e0ec316333c0cbfeeccfef0e", nullAudience.hash());
+
+            // groups_in_token participates in the identity, so it flips the hash to a different file
+            TokenStoreKey groups = new TokenStoreKey("questdb",
+                    "https://idp.example.com:443/as/token", "https://idp.example.com:443/as/device",
+                    "openid", "api://billing", true);
+            Assert.assertEquals("5193f668130b28cd9430f5271011f1044b3b1c1e78bfc4f45d7688a3d9b1ceb0", groups.hash());
+            Assert.assertNotEquals(withAudience.hash(), groups.hash());
+        });
+    }
+
     @Test
     public void testInLockDegradesWhenDirectoryUnusable() throws Exception {
         assertMemoryLeak(() -> {
@@ -340,6 +435,29 @@ public void testLoadMissingReturnsNull() throws Exception {
         });
     }
 
+    @Test
+    public void testLockFilePermissionsOwnerOnly() throws Exception {
+        Assume.assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix"));
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            FileTokenStore store = new FileTokenStore(dir);
+            TokenStoreKey key = sampleKey();
+            Path lock = lockFile(dir, key);
+            // the lock file is created owner-only too: it briefly records pid@host and sits beside the 0600
+            // token file, so it must not widen the directory's exposure. Assert while the lock is held; inLock
+            // deletes it on return and propagates a thrown AssertionError after releasing it.
+            store.inLock(key, () -> {
+                try {
+                    Assert.assertEquals("the lock file must be owner-only (0600)",
+                            PosixFilePermissions.fromString("rw-------"), Files.getPosixFilePermissions(lock));
+                } catch (java.io.IOException e) {
+                    throw new AssertionError(e);
+                }
+                return true;
+            });
+        });
+    }
+
     @Test
     public void testNoLeftoverTempFileAfterSave() throws Exception {
         assertMemoryLeak(() -> {
@@ -376,6 +494,39 @@ public void testOversizedFileReturnsNull() throws Exception {
         });
     }
 
+    @Test
+    public void testPerFieldFingerprintMismatchReturnsNull() throws Exception {
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            FileTokenStore store = new FileTokenStore(dir);
+            TokenStoreKey saved = sampleKey();
+            store.save(saved, sampleToken("ACCESS-1", "REFRESH-1"));
+            byte[] bytes = Files.readAllBytes(tokenFile(dir, saved));
+
+            // each key differs from the saved fingerprint in exactly one field; writing the saved bytes under
+            // the differing key's file name isolates the in-file fingerprint re-check (the file is found, but
+            // its recorded identity does not match), so a hash collision or a copied file never serves another
+            // identity's token. groups_in_token (id-token vs access-token credential) and audience (the
+            // distinct nullableEquals path) are the riskiest fields.
+            TokenStoreKey[] mismatches = {
+                    new TokenStoreKey("questdb", "https://idp.example.com:443/OTHER-token",
+                            "https://idp.example.com:443/device", "openid", null, false),
+                    new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                            "https://idp.example.com:443/OTHER-device", "openid", null, false),
+                    new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                            "https://idp.example.com:443/device", "openid groups", null, false),
+                    new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                            "https://idp.example.com:443/device", "openid", "api://other", false),
+                    new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                            "https://idp.example.com:443/device", "openid", null, true),
+            };
+            for (TokenStoreKey other : mismatches) {
+                Files.write(tokenFile(dir, other), bytes);
+                Assert.assertNull("a fingerprint mismatch must be rejected: " + other.hash(), store.load(other));
+            }
+        });
+    }
+
     @Test
     public void testPermissionsOwnerOnly() throws Exception {
         Assume.assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix"));
@@ -409,6 +560,31 @@ public void testSaveThenLoadRoundTrip() throws Exception {
         });
     }
 
+    @Test
+    public void testSchemaVersionMismatchReturnsNull() throws Exception {
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            FileTokenStore store = new FileTokenStore(dir);
+            TokenStoreKey key = sampleKey();
+            Files.createDirectories(dir);
+            // a future schema version with an otherwise-matching fingerprint must be ignored, not served: the
+            // version is the forward-compat guard the frozen cross-language contract rests on
+            String v2 = "{\"v\":2,\"client_id\":\"questdb\","
+                    + "\"token_endpoint\":\"https://idp.example.com:443/token\","
+                    + "\"device_authorization_endpoint\":\"https://idp.example.com:443/device\","
+                    + "\"scope\":\"openid\",\"groups_in_token\":false,"
+                    + "\"access_token\":\"ACCESS-1\",\"refresh_token\":\"REFRESH-1\","
+                    + "\"expires_at_millis\":1730000000000,\"token_ttl_millis\":300000}";
+            Files.write(tokenFile(dir, key), v2.getBytes(StandardCharsets.UTF_8));
+            Assert.assertNull("a future schema version must be rejected", store.load(key));
+
+            // sanity: the identical body at the live version IS accepted, proving the rejection is the version
+            // and not a malformed document
+            Files.write(tokenFile(dir, key), v2.replace("\"v\":2", "\"v\":1").getBytes(StandardCharsets.UTF_8));
+            Assert.assertNotNull("the same body at the live schema version must load", store.load(key));
+        });
+    }
+
     @Test
     public void testSpecialCharactersAndNullsRoundTrip() throws Exception {
         assertMemoryLeak(() -> {