fix: Prevent hundreds of keys from being created with eventually consistent storage

[swalker326]

Fixes #322

Problem

When deploying to Cloudflare Workers with an empty KV store, the initial request to /.well-known/jwks.json would create hundreds of signing and encryption keys, causing subsequent requests to take 15-20 seconds and eventually crash.

This issue was triggered by Cloudflare's KVs eventual consistency behavior.

Root Cause

The signingKeys() and encryptionKeys() functions had a race condition with eventually consistent storage:

// Old code
await Storage.set(storage, ["signing:key", serialized.id], serialized)
return signingKeys(storage)  // ❌ Recursive call

The recursive flow:
1. Generate key → Save to storage
2. Recursively call signingKeys() to load it back
3. Scan storage for keys
4. Due to eventual consistency, the just-written key isn't visible yet
5. Function thinks there are no keys, so it generates another
6. Loop continues, creating hundreds of keys

Solution

Return the newly created key directly instead of recursively scanning storage:

// New code
await Storage.set(storage, ["signing:key", serialized.id], serialized)

// Return the key directly - no recursive call
const jwk = await exportJWK(key.publicKey)
jwk.kid = serialized.id
jwk.use = "sig"
return [
  ...results,
  {
    id: serialized.id,
    alg: signingAlg,
    created: new Date(serialized.created),
    expired: undefined,
    public: key.publicKey,
    private: key.privateKey,
    jwk,
  },
]

Why this works:

• Eliminates the recursive call that was causing the infinite loop
• Returns the in-memory key object we just generated (cryptographically identical to deserializing it)
• The key is still persisted to storage for future loads
• No functionality is lost - subsequent calls still load keys from storage normally

Testing

Added comprehensive tests in test/keys.test.ts that:

1. Simulate eventual consistency with a mock storage adapter where writes complete but scans don't immediately see new values
2. Verify the bug existed by testing with the old recursive code (created 10+ keys)
3. Verify the fix works by testing with the new code (creates exactly 1 key)
4. Confirm keys are persisted to storage for future loads

Test results:

• ✅ Old code: Created 10+ keys with eventual consistency (bug confirmed)
• ✅ New code: Creates exactly 1 key with eventual consistency (fix verified)
• ✅ All existing tests pass

Trade-offs

Potential for 2 keys with concurrent requests:
if two requests hit an empty storage at exactly the same time, both might create a key (2 total instead of 1). This is acceptable because:

• Extremely rare edge case (first deployment + precise timing)
• System is designed for multiple keys (key rotation)
• Functionally identical to having 1 key
• Much better than the original bug (hundreds of keys)

Migration Notes

No breaking changes. Existing deployments with keys in storage will continue to work normally. Only affects the initial key generation when storage is empty.

[beeirl]

looks good! there's still an edge case where multiple keys could be generated under concurrent calls, but that's probably negligible. this is likely the best solution we can get for eventually consistent storage adapters without introducing distributed locking.

[grunklejp]

Would it be simpler to just make key generation an explicit admin step? Tripping on the issue this PR addresses, I was kind of surprised key generation was done at runtime at all.

Not sure what the admin interface would be, but a production auth solution at least needs a clear path on how to roll keys.