From 3c7e5b038abd43c16cf604aafbfdf0269849a8f8 Mon Sep 17 00:00:00 2001 From: Tomas Virgl <739690+tvi@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:21:55 +0200 Subject: [PATCH 1/4] fix(api): invalidate auth cache on API key deletion [EN-1874] Deleted API keys kept authenticating for up to 5 minutes because the Redis-backed auth cache entry (keyed by the key's SHA-256 hash) was never invalidated on deletion. DeleteTeamAPIKey now returns the deleted key's hash, and team.DeleteAPIKey invalidates the corresponding auth cache entry via the new AuthService.InvalidateAPIKeyCache, so revocation takes effect immediately across all API instances (the cache is shared in Redis). --- .../api/internal/handlers/admin_api_keys.go | 2 +- .../internal/handlers/admin_api_keys_test.go | 25 ++++++++++++++++--- packages/api/internal/handlers/apikey.go | 2 +- packages/api/internal/team/apikeys.go | 13 +++++++--- packages/auth/internal/service/service.go | 8 ++++++ .../internal/handlers/team_handlers_test.go | 2 ++ .../auth/queries/delete_team_api_key.sql.go | 12 ++++----- .../api_keys/delete_team_api_key.sql | 2 +- 8 files changed, 51 insertions(+), 15 deletions(-) diff --git a/packages/api/internal/handlers/admin_api_keys.go b/packages/api/internal/handlers/admin_api_keys.go index 78d956e4c6..76ebdce00f 100644 --- a/packages/api/internal/handlers/admin_api_keys.go +++ b/packages/api/internal/handlers/admin_api_keys.go @@ -89,7 +89,7 @@ func (a *APIStore) DeleteAdminTeamsTeamIDApiKeysApiKeyID(c *gin.Context, teamID return } - deleted, err := team.DeleteAPIKey(ctx, a.authDB, teamID, apiKeyUUID) + deleted, err := team.DeleteAPIKey(ctx, a.authDB, a.authService, teamID, apiKeyUUID) if err != nil { a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when deleting API key: %s", err)) diff --git a/packages/api/internal/handlers/admin_api_keys_test.go b/packages/api/internal/handlers/admin_api_keys_test.go index 9359bb3bfd..d59cdf34ed 100644 --- a/packages/api/internal/handlers/admin_api_keys_test.go +++ b/packages/api/internal/handlers/admin_api_keys_test.go @@ -18,6 +18,7 @@ import ( authtypes "github.com/e2b-dev/infra/packages/auth/pkg/types" authqueries "github.com/e2b-dev/infra/packages/db/pkg/auth/queries" "github.com/e2b-dev/infra/packages/db/pkg/testutils" + sharedkeys "github.com/e2b-dev/infra/packages/shared/pkg/keys" ) func TestPostAdminTeamsTeamIDApiKeysCreatesTeamKey(t *testing.T) { @@ -195,9 +196,13 @@ func TestDeleteAdminTeamsTeamIDApiKeysDeletesTeamKey(t *testing.T) { createCtx.Request = httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/admin/teams/"+teamID.String()+"/api-keys", strings.NewReader(`{"name":"Admin integration"}`)) createCtx.Request.Header.Set("Content-Type", "application/json") + var invalidatedHashes []string store := &APIStore{ - authDB: testDB.AuthDB, - authService: fakeAPIKeyAuthService{team: &authtypes.Team{Team: &authqueries.Team{ID: teamID}}}, + authDB: testDB.AuthDB, + authService: fakeAPIKeyAuthService{ + team: &authtypes.Team{Team: &authqueries.Team{ID: teamID}}, + invalidatedAPIKeyHashes: &invalidatedHashes, + }, } store.PostAdminTeamsTeamIDApiKeys(createCtx, teamID) if createRecorder.Code != http.StatusCreated { @@ -225,6 +230,12 @@ func TestDeleteAdminTeamsTeamIDApiKeysDeletesTeamKey(t *testing.T) { if len(keys) != 0 { t.Fatalf("expected API key to be deleted, got %d keys", len(keys)) } + + deletedKeyHash, err := sharedkeys.VerifyKey(sharedkeys.ApiKeyPrefix, created.Key) + if err != nil { + t.Fatalf("failed to hash created API key: %v", err) + } + require.Equal(t, []string{deletedKeyHash}, invalidatedHashes, "expected the deleted key's auth cache entry to be invalidated") } func TestDeleteAdminTeamsTeamIDApiKeysRejectsMissingKey(t *testing.T) { @@ -233,7 +244,7 @@ func TestDeleteAdminTeamsTeamIDApiKeysRejectsMissingKey(t *testing.T) { testDB := testutils.SetupDatabase(t) teamID := testutils.CreateTestTeam(t, testDB) - store := &APIStore{authDB: testDB.AuthDB} + store := &APIStore{authDB: testDB.AuthDB, authService: fakeAPIKeyAuthService{}} recorder := httptest.NewRecorder() ctx, _ := gin.CreateTestContext(recorder) missingKeyID := uuid.New() @@ -249,6 +260,8 @@ func TestDeleteAdminTeamsTeamIDApiKeysRejectsMissingKey(t *testing.T) { type fakeAPIKeyAuthService struct { team *authtypes.Team err error + + invalidatedAPIKeyHashes *[]string } func (f fakeAPIKeyAuthService) ValidateAPIKey(context.Context, *gin.Context, string) (*authtypes.Team, *sharedauth.APIError) { @@ -277,6 +290,12 @@ func (f fakeAPIKeyAuthService) GetTeamByID(context.Context, uuid.UUID) (*authtyp func (f fakeAPIKeyAuthService) InvalidateTeamMemberCache(context.Context, uuid.UUID, string) {} +func (f fakeAPIKeyAuthService) InvalidateAPIKeyCache(_ context.Context, hashedKey string) { + if f.invalidatedAPIKeyHashes != nil { + *f.invalidatedAPIKeyHashes = append(*f.invalidatedAPIKeyHashes, hashedKey) + } +} + func (f fakeAPIKeyAuthService) InvalidateTeamCache(context.Context, uuid.UUID) error { return nil } diff --git a/packages/api/internal/handlers/apikey.go b/packages/api/internal/handlers/apikey.go index 21d1e30a32..71c39a6992 100644 --- a/packages/api/internal/handlers/apikey.go +++ b/packages/api/internal/handlers/apikey.go @@ -118,7 +118,7 @@ func (a *APIStore) DeleteApiKeysApiKeyID(c *gin.Context, apiKeyID string) { teamID := auth.MustGetTeamID(c) - deleted, err := team.DeleteAPIKey(ctx, a.authDB, teamID, apiKeyIDParsed) + deleted, err := team.DeleteAPIKey(ctx, a.authDB, a.authService, teamID, apiKeyIDParsed) if err != nil { a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when deleting API key: %s", err)) diff --git a/packages/api/internal/team/apikeys.go b/packages/api/internal/team/apikeys.go index e075a4ed54..f99d75183d 100644 --- a/packages/api/internal/team/apikeys.go +++ b/packages/api/internal/team/apikeys.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" + sharedauth "github.com/e2b-dev/infra/packages/auth/pkg/auth" "github.com/e2b-dev/infra/packages/db/pkg/auth" "github.com/e2b-dev/infra/packages/db/pkg/auth/queries" "github.com/e2b-dev/infra/packages/shared/pkg/keys" @@ -48,8 +49,8 @@ func CreateAPIKey(ctx context.Context, authDB *authdb.Client, teamID uuid.UUID, }, nil } -func DeleteAPIKey(ctx context.Context, authDB *authdb.Client, teamID uuid.UUID, apiKeyID uuid.UUID) (bool, error) { - ids, err := authDB.Write.DeleteTeamAPIKey(ctx, authqueries.DeleteTeamAPIKeyParams{ +func DeleteAPIKey(ctx context.Context, authDB *authdb.Client, authService sharedauth.Service, teamID uuid.UUID, apiKeyID uuid.UUID) (bool, error) { + hashes, err := authDB.Write.DeleteTeamAPIKey(ctx, authqueries.DeleteTeamAPIKeyParams{ ID: apiKeyID, TeamID: teamID, }) @@ -59,5 +60,11 @@ func DeleteAPIKey(ctx context.Context, authDB *authdb.Client, teamID uuid.UUID, return false, fmt.Errorf("error when deleting API key: %w", err) } - return len(ids) > 0, nil + // Invalidate the auth cache so the deleted key stops authenticating + // immediately instead of after the cache TTL expires. + for _, hash := range hashes { + authService.InvalidateAPIKeyCache(ctx, hash) + } + + return len(hashes) > 0, nil } diff --git a/packages/auth/internal/service/service.go b/packages/auth/internal/service/service.go index 9d1522b51a..b17fc35240 100644 --- a/packages/auth/internal/service/service.go +++ b/packages/auth/internal/service/service.go @@ -43,6 +43,7 @@ type Service interface { GetTeamByID(ctx context.Context, teamID uuid.UUID) (*types.Team, error) InvalidateTeamMemberCache(ctx context.Context, userID uuid.UUID, teamID string) InvalidateTeamCache(ctx context.Context, teamID uuid.UUID) error + InvalidateAPIKeyCache(ctx context.Context, hashedKey string) Close(ctx context.Context) error } @@ -277,6 +278,13 @@ func (s *AuthService) InvalidateTeamCache(ctx context.Context, teamID uuid.UUID) return nil } +// InvalidateAPIKeyCache removes the cached auth entry for a specific hashed API key. +// This should be called when the key is deleted so revocation takes effect immediately +// instead of after the cache TTL expires. +func (s *AuthService) InvalidateAPIKeyCache(ctx context.Context, hashedKey string) { + s.teamCache.Invalidate(ctx, hashedKey) +} + func teamMemberCacheKey(userID uuid.UUID, teamID string) string { return fmt.Sprintf("%s-%s", userID.String(), strings.ToLower(teamID)) } diff --git a/packages/dashboard-api/internal/handlers/team_handlers_test.go b/packages/dashboard-api/internal/handlers/team_handlers_test.go index 5946968786..3ca1b895ba 100644 --- a/packages/dashboard-api/internal/handlers/team_handlers_test.go +++ b/packages/dashboard-api/internal/handlers/team_handlers_test.go @@ -847,6 +847,8 @@ func (noopAuthService) GetTeamByID(context.Context, uuid.UUID) (*authtypes.Team, func (noopAuthService) InvalidateTeamMemberCache(context.Context, uuid.UUID, string) {} +func (noopAuthService) InvalidateAPIKeyCache(context.Context, string) {} + func (noopAuthService) InvalidateTeamCache(context.Context, uuid.UUID) error { return nil } diff --git a/packages/db/pkg/auth/queries/delete_team_api_key.sql.go b/packages/db/pkg/auth/queries/delete_team_api_key.sql.go index 49dd634e59..e019ab3501 100644 --- a/packages/db/pkg/auth/queries/delete_team_api_key.sql.go +++ b/packages/db/pkg/auth/queries/delete_team_api_key.sql.go @@ -14,7 +14,7 @@ import ( const deleteTeamAPIKey = `-- name: DeleteTeamAPIKey :many DELETE FROM "public"."team_api_keys" WHERE id = $1 AND team_id = $2 -RETURNING id +RETURNING api_key_hash ` type DeleteTeamAPIKeyParams struct { @@ -22,19 +22,19 @@ type DeleteTeamAPIKeyParams struct { TeamID uuid.UUID } -func (q *Queries) DeleteTeamAPIKey(ctx context.Context, arg DeleteTeamAPIKeyParams) ([]uuid.UUID, error) { +func (q *Queries) DeleteTeamAPIKey(ctx context.Context, arg DeleteTeamAPIKeyParams) ([]string, error) { rows, err := q.db.Query(ctx, deleteTeamAPIKey, arg.ID, arg.TeamID) if err != nil { return nil, err } defer rows.Close() - var items []uuid.UUID + var items []string for rows.Next() { - var id uuid.UUID - if err := rows.Scan(&id); err != nil { + var api_key_hash string + if err := rows.Scan(&api_key_hash); err != nil { return nil, err } - items = append(items, id) + items = append(items, api_key_hash) } if err := rows.Err(); err != nil { return nil, err diff --git a/packages/db/pkg/auth/sql_queries/api_keys/delete_team_api_key.sql b/packages/db/pkg/auth/sql_queries/api_keys/delete_team_api_key.sql index d38b25f205..91ab08edb4 100644 --- a/packages/db/pkg/auth/sql_queries/api_keys/delete_team_api_key.sql +++ b/packages/db/pkg/auth/sql_queries/api_keys/delete_team_api_key.sql @@ -1,4 +1,4 @@ -- name: DeleteTeamAPIKey :many DELETE FROM "public"."team_api_keys" WHERE id = @id AND team_id = @team_id -RETURNING id; +RETURNING api_key_hash; From ff1dcaf3acf183482fe7c49bae4ec4abe2d7b20a Mon Sep 17 00:00:00 2001 From: Tomas Virgl <739690+tvi@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:15:38 +0200 Subject: [PATCH 2/4] fix(auth): read API-key lookup from primary to make revocation read-after-write safe After a key deletion invalidates the cache entry, a cache miss that read through the read replica could race replication lag, find the just-deleted key still present, and re-cache it for the full 5-minute TTL. Reading the API-key auth lookup from the primary closes that window (same approach as the OIDC identity lookup). --- packages/auth/internal/service/store.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/auth/internal/service/store.go b/packages/auth/internal/service/store.go index 74fac0b5e5..becd205460 100644 --- a/packages/auth/internal/service/store.go +++ b/packages/auth/internal/service/store.go @@ -31,7 +31,11 @@ func (s *authStoreImpl) GetTeamByHashedAPIKey(ctx context.Context, hashedKey str ctx, span := tracer.Start(ctx, "get team auth") defer span.End() - result, err := s.authDB.Read.GetTeamWithTierByAPIKey(ctx, hashedKey) + // Deleting an API key invalidates its cache entry; reading through the + // read replica here races replication lag and could re-cache a + // just-deleted key for the full cache TTL, so key revocation must be + // read-after-write safe. + result, err := s.authDB.Write.GetTeamWithTierByAPIKey(ctx, hashedKey) if err != nil { return nil, fmt.Errorf("failed to get team from API key: %w", err) } From e3f119ab87acd4ca39cb94a716925e131d64ceea Mon Sep 17 00:00:00 2001 From: Tomas Virgl <739690+tvi@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:25:45 +0200 Subject: [PATCH 3/4] fix(auth): detach API-key cache invalidation from request cancellation The invalidation runs after the key's DB delete has committed. If the client disconnected mid-request, the canceled request context would make the Redis delete silently fail and the revoked key would keep authenticating until the cache TTL expires. Run the invalidation on a bounded context detached from the request. --- packages/auth/internal/service/cache.go | 1 + packages/auth/internal/service/service.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/packages/auth/internal/service/cache.go b/packages/auth/internal/service/cache.go index 124562afca..7bf1a980c0 100644 --- a/packages/auth/internal/service/cache.go +++ b/packages/auth/internal/service/cache.go @@ -14,6 +14,7 @@ const ( authInfoExpiration = 5 * time.Minute refreshInterval = 1 * time.Minute refreshTimeout = 30 * time.Second + invalidateTimeout = 10 * time.Second authCacheRedisPrefix = "auth:team" ) diff --git a/packages/auth/internal/service/service.go b/packages/auth/internal/service/service.go index b17fc35240..b68dc62d26 100644 --- a/packages/auth/internal/service/service.go +++ b/packages/auth/internal/service/service.go @@ -282,6 +282,12 @@ func (s *AuthService) InvalidateTeamCache(ctx context.Context, teamID uuid.UUID) // This should be called when the key is deleted so revocation takes effect immediately // instead of after the cache TTL expires. func (s *AuthService) InvalidateAPIKeyCache(ctx context.Context, hashedKey string) { + // The invalidation runs after the key's DB delete has committed; if it were + // skipped because the client disconnected, the revoked key would keep + // authenticating until the cache TTL expires. + ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), invalidateTimeout) + defer cancel() + s.teamCache.Invalidate(ctx, hashedKey) } From bf7ad2b8a86f94f9dcb49b70ef472665fbcc9569 Mon Sep 17 00:00:00 2001 From: Tomas Virgl <739690+tvi@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:44:44 +0200 Subject: [PATCH 4/4] fix(cache): order Delete after in-flight writers to prevent stale repopulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RedisCache writers (GetOrSet backfill, background refresh) hold the per-key lock across their SET, but Delete gave up on the lock after 5s and deleted anyway — a writer stalled on the backing store could then SET a pre-delete value after the DEL, repopulating stale data for a full TTL. For the auth cache this meant a revoked API key could be resurrected for another 5 minutes. Delete now waits for the lock up to the lock TTL plus a margin (bounded by the caller's ctx), so a healthy writer — whose callback is capped at RefreshTimeout, strictly less than the lock TTL — always lands its SET before the DEL. The auth invalidation timeout grows to 45s to cover that wait; lock failure still degrades to a best-effort DEL, now logged with the stale-repopulation risk. Constraints are documented on Delete and the auth invalidation path. --- packages/auth/internal/service/cache.go | 8 +++- packages/auth/internal/service/service.go | 6 +++ packages/shared/pkg/cache/redis.go | 38 ++++++++++++++---- packages/shared/pkg/cache/redis_test.go | 47 +++++++++++++++++++++++ 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/packages/auth/internal/service/cache.go b/packages/auth/internal/service/cache.go index 7bf1a980c0..1856c9988b 100644 --- a/packages/auth/internal/service/cache.go +++ b/packages/auth/internal/service/cache.go @@ -14,7 +14,13 @@ const ( authInfoExpiration = 5 * time.Minute refreshInterval = 1 * time.Minute refreshTimeout = 30 * time.Second - invalidateTimeout = 10 * time.Second + // invalidateTimeout must exceed the cache's write-lock wait so an + // invalidation can outwait any in-flight cache writer instead of degrading + // to a best-effort delete that a stale write could overwrite. The wait is + // bounded by the lock TTL (refreshTimeout + 2x the 2s default Redis + // timeout = 34s) plus the 5s lock-acquire margin; 45s adds headroom for + // the DEL itself. + invalidateTimeout = 45 * time.Second authCacheRedisPrefix = "auth:team" ) diff --git a/packages/auth/internal/service/service.go b/packages/auth/internal/service/service.go index b68dc62d26..c1e4344f33 100644 --- a/packages/auth/internal/service/service.go +++ b/packages/auth/internal/service/service.go @@ -281,6 +281,12 @@ func (s *AuthService) InvalidateTeamCache(ctx context.Context, teamID uuid.UUID) // InvalidateAPIKeyCache removes the cached auth entry for a specific hashed API key. // This should be called when the key is deleted so revocation takes effect immediately // instead of after the cache TTL expires. +// +// The call is synchronous and waits for any in-flight cache writer on the key +// (see RedisCache.Delete), so the caller's request can block for up to +// invalidateTimeout in the worst case — only reached when a concurrent +// refresh of the same key is wedged near the full refresh timeout, which +// requires a multi-second DB stall; the typical case returns in milliseconds. func (s *AuthService) InvalidateAPIKeyCache(ctx context.Context, hashedKey string) { // The invalidation runs after the key's DB delete has committed; if it were // skipped because the client disconnected, the revoked key would keep diff --git a/packages/shared/pkg/cache/redis.go b/packages/shared/pkg/cache/redis.go index 9ce4983b94..cd2f9186ce 100644 --- a/packages/shared/pkg/cache/redis.go +++ b/packages/shared/pkg/cache/redis.go @@ -116,7 +116,7 @@ func (rc *RedisCache[V]) GetOrSet(ctx context.Context, key string, dataCallback ctx := context.WithoutCancel(ctx) // Acquire distributed lock if enabled - lock, lockErr := rc.acquireLock(ctx, key, redislock.LinearBackoff(rc.config.LockRetryInterval)) + lock, lockErr := rc.acquireLock(ctx, key, redislock.LinearBackoff(rc.config.LockRetryInterval), acquireLockTimeout) defer rc.releaseLock(ctx, lock, key) // We want to get the results even without the lock to prevent failing all the waiting requests @@ -163,10 +163,33 @@ func (rc *RedisCache[V]) Set(ctx context.Context, key string, value V) { } // Delete removes a value from Redis. +// +// Writers (the GetOrSet backfill and the background refresh) hold the per-key +// lock across their SET, so Delete waits for that lock to guarantee the DEL +// is ordered after any in-flight write; without this, a writer that read the +// backing store just before the caller's mutation could repopulate the entry +// with stale data for a full TTL after a fire-and-forget delete. +// +// The guarantee comes with constraints the caller must respect: +// - The wait is bounded by LockTTL (RefreshTimeout + 2*RedisTimeout) plus a +// margin, and by ctx — pass a context that survives at least that long +// (and detached from request cancellation when the delete must not be +// skipped), otherwise Delete degrades to the best-effort behavior below. +// - If the lock still cannot be obtained (Redis/lock-service errors, or the +// ctx expiring), Delete falls back to a best-effort DEL and a concurrent +// writer may repopulate the entry with stale data until its TTL expires. +// - Healthy writers always finish inside the wait window: their data +// callback is capped at RefreshTimeout, which is strictly less than the +// lock TTL the wait is derived from. func (rc *RedisCache[V]) Delete(ctx context.Context, key string) { - lock, err := rc.acquireLock(ctx, key, redislock.LinearBackoff(rc.config.LockRetryInterval)) + // Wait past a wedged writer's lock auto-expiry (LockTTL) so lock + // acquisition can only fail on Redis/lock-service errors or ctx expiry, + // not on writer contention. + lock, err := rc.acquireLock(ctx, key, redislock.LinearBackoff(rc.config.LockRetryInterval), rc.config.LockTTL+acquireLockTimeout) if err != nil { - logger.L().Warn(ctx, "RedisCache - Delete: failed to acquire lock", zap.String("key", key)) + logger.L().Warn(ctx, "RedisCache - Delete: failed to acquire lock, deleting best-effort; a concurrent writer may repopulate stale data", + zap.String("key", key), + zap.Error(err)) // Continue without the lock to remove the stale data // In that case it's just a best effort, the data may get repopulated with stale data } @@ -299,7 +322,7 @@ func (rc *RedisCache[V]) getFromRedis(ctx context.Context, key string) (V, time. func (rc *RedisCache[V]) refreshRedis(ctx context.Context, key string, dataCallback DataCallback[V]) { rc.redisRefresh.Do(key, func() (any, error) { // Acquire lock without retry — if another instance is refreshing, skip. - lock, lockErr := rc.acquireLock(ctx, key, redislock.NoRetry()) + lock, lockErr := rc.acquireLock(ctx, key, redislock.NoRetry(), acquireLockTimeout) if errors.Is(lockErr, redislock.ErrNotObtained) { logger.L().Debug(ctx, "RedisCache: skipping refresh, lock held by another instance", zap.String("key", key)) @@ -350,10 +373,11 @@ func (rc *RedisCache[V]) releaseLock(ctx context.Context, lock redis_utils.Lock, } } -// acquireLock attempts to acquire a distributed lock for the given key. +// acquireLock attempts to acquire a distributed lock for the given key, +// retrying per the strategy until maxWait or ctx expires, whichever is first. // Always returns a non-nil Lock (NoopLock on failure) so callers can defer Release unconditionally. -func (rc *RedisCache[V]) acquireLock(ctx context.Context, key string, retry redislock.RetryStrategy) (redis_utils.Lock, error) { - ctx, cancel := context.WithTimeout(ctx, acquireLockTimeout) +func (rc *RedisCache[V]) acquireLock(ctx context.Context, key string, retry redislock.RetryStrategy, maxWait time.Duration) (redis_utils.Lock, error) { + ctx, cancel := context.WithTimeout(ctx, maxWait) defer cancel() lockKey := redis_utils.GetLockKey(rc.RedisKey(key)) diff --git a/packages/shared/pkg/cache/redis_test.go b/packages/shared/pkg/cache/redis_test.go index b4b62c3dd5..2cbd2351e9 100644 --- a/packages/shared/pkg/cache/redis_test.go +++ b/packages/shared/pkg/cache/redis_test.go @@ -148,6 +148,53 @@ func TestRedisCache_Delete(t *testing.T) { assert.ErrorIs(t, err, redis.Nil) } +// A writer (GetOrSet backfill) holds the per-key lock across its SET. Delete +// must wait for that lock — even past the 5s acquire timeout used by writers — +// so the DEL is ordered after the in-flight write and stale data cannot be +// repopulated for a full TTL (e.g. a revoked API key resurrected into the auth +// cache). +func TestRedisCache_DeleteWaitsForInflightWriter(t *testing.T) { + t.Parallel() + redisClient := redis_utils.SetupInstance(t) + rc := newTestRedisCache(t, redisClient) + defer rc.Close(t.Context()) + + key := "key1" + callbackStarted := make(chan struct{}) + writerDone := make(chan struct{}) + + var eg errgroup.Group + eg.Go(func() error { + defer close(writerDone) + _, err := rc.GetOrSet(t.Context(), key, func(_ context.Context, _ string) (testValue, error) { + close(callbackStarted) + // Hold the write lock longer than the acquire timeout writers use, + // simulating a callback stalled on a slow backing store. + time.Sleep(acquireLockTimeout + time.Second) + + return testValue{ID: "9", Name: "stale"}, nil + }) + + return err + }) + + <-callbackStarted + rc.Delete(t.Context(), key) + + // Delete must have waited for the writer's lock, so by the time it + // returns the writer's SET has already happened and been removed. + select { + case <-writerDone: + default: + t.Fatal("Delete returned while the writer still held the lock; the writer's SET could repopulate stale data") + } + + require.NoError(t, eg.Wait()) + + _, err := redisClient.Get(t.Context(), rc.RedisKey(key)).Result() + assert.ErrorIs(t, err, redis.Nil, "the DEL must be ordered after the in-flight writer's SET") +} + func TestRedisCache_SetWritesRedis(t *testing.T) { t.Parallel() redisClient := redis_utils.SetupInstance(t)