fix: never hand a completed sign-job to a new Blossom token caller

`signOnce` retired the in-flight entry from `invokeOnCompletion`, which runs
when the job ends — after `fresh.complete()` has already resumed the awaiting
caller. In that window the map still holds a *completed* deferred, so the next
caller took the leader/follower branch and was handed the token that job had
already signed instead of signing a new one.

A caller whose token has just expired does exactly that: `header()` misses the
cache, reaches `signOnce`, and gets the expired token straight back.
`BlossomReadAuthTokenProviderTest.refreshesAfterExpiry` closes that window
immediately, so it hit the bug on every run and has been failing on main.

Remove the entry before completing it. `invokeOnCompletion` keeps its
now-idempotent removal as the cancellation safety net.

The test also asserted the re-signed header differed byte-for-byte from the
first. That cannot hold: the injected `clock` only drives the cache TTL, while
BlossomAuthorizationEvent takes `created_at` from `TimeUtils.now()`, so two
signings in the same second produce identical events. Count signatures instead,
which is what "must be re-signed" actually means.
This commit is contained in:
Claude
2026-08-29 15:00:56 +00:00
parent 2f220656f8
commit 0d9b2d8d29
2 changed files with 28 additions and 4 deletions
@@ -139,6 +139,15 @@ class BlossomReadAuthTokenProvider(
if (header != null) {
cache[host] = CachedToken(header, clock() + CACHE_TTL_MS)
}
// Retire the entry *before* completing it. `invokeOnCompletion` fires when the
// job ends, which is after `complete()` resumes the awaiting caller — so a
// caller that returned from `header()` could come straight back, find this
// finished deferred still in the map, and be handed its already-signed token
// instead of signing a new one. A caller whose token has just expired does
// exactly that, and got the expired token back for as long as the window
// lasted — [refreshesAfterExpiry] closes it immediately and so hit it every run.
inFlight.remove(host, fresh)
fresh.complete(header)
}.invokeOnCompletion {
inFlight.remove(host, fresh)
@@ -35,6 +35,7 @@ import kotlinx.coroutines.withTimeout
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
@@ -114,14 +115,28 @@ class BlossomReadAuthTokenProviderTest {
fun refreshesAfterExpiry() =
runBlocking {
var now = 0L
val provider = BlossomReadAuthTokenProvider({ signer }, scope, clock = { now })
var lookups = 0
val provider =
BlossomReadAuthTokenProvider({
lookups++
signer
}, scope, clock = { now })
assertNotNull(provider.header(host))
assertEquals("the first call must sign", 1, lookups)
val first = provider.header(host)
now += 56L * 60L * 1000L
assertNull("token must be gone from the pure read once expired", provider.cachedHeader(host))
val second = provider.header(host)
assertNotEquals("an expired token must be re-signed", first, second)
// Counts signatures rather than comparing the two headers: the injected [clock] only
// drives the cache TTL, while the signed BlossomAuthorizationEvent takes its
// `created_at` from the real wall clock (TimeUtils.now()). Both signings land in the
// same second on any quick machine, so the two events — and therefore their ids, sigs
// and headers — are byte-identical, and an assertNotEquals on them fails even though
// the token was correctly re-signed. The signer is only ever consulted on a real
// signing pass (a cache hit returns before it), so this counter says exactly that.
assertNotNull(provider.header(host))
assertEquals("an expired token must be re-signed", 2, lookups)
}
@Test