fix(api): invalidate auth cache on API key deletion

[tvi]

Summary

Fixes EN-1874: deleted API keys kept authenticating for up to ~5 minutes while the dashboard promises keys are disabled immediately.

API-key auth results are cached in the shared Redis auth cache (auth:team:<sha256-of-key>, 5-minute TTL), but the key-deletion handlers never invalidated the entry. The existing InvalidateTeamCache helper couldn't be reused as-is because it enumerates the team's current key hashes from the DB — a just-deleted key is no longer returned.

Changes

• DeleteTeamAPIKey SQL query now does RETURNING api_key_hash instead of id (the hash is exactly the auth cache key); sqlc code regenerated.
• New InvalidateAPIKeyCache(ctx, hashedKey) on the auth Service interface deletes the cached auth entry.
• team.DeleteAPIKey invalidates the returned hash after a successful DB delete, covering both callers (user-facing DELETE /api-keys/{id} and admin DELETE /admin/teams/{teamID}/api-keys/{id}). The cache is Redis-backed and shared, so one delete revokes the key across all API instances.
• GetTeamByHashedAPIKey now reads from the primary DB instead of the read replica, so a cache miss racing replication lag can't re-cache a just-deleted key for another full TTL (same read-after-write reasoning as the existing OIDC identity lookup). Extra primary load is bounded by the cache miss + 1-minute refresh rates.
• InvalidateAPIKeyCache runs on a bounded context detached from request cancellation, so a client disconnect after the DB delete commits can't skip the invalidation.
• RedisCache.Delete now waits for the per-key writer lock (up to the lock TTL plus a margin) instead of giving up after 5s and deleting anyway — otherwise a cache writer stalled on the DB just before the deletion could SET the pre-delete value after the DEL, resurrecting the revoked key for another full TTL.

Constraints / residual risk

• Delete lock-wait: the ordering guarantee holds because writer callbacks are capped at RefreshTimeout (30s), strictly less than the lock TTL the delete waits for. If the lock cannot be obtained at all (Redis/lock-service errors or ctx expiry — no longer plain writer contention), Delete degrades to a best-effort DEL, logs the stale-repopulation risk, and a concurrent writer may repopulate the entry until its TTL expires. Fully closing that path would need a tombstone/version mechanism in RedisCache; deferred unless it proves necessary.
• Worst-case delete latency: the key-deletion request blocks on the invalidation for up to 45s (invalidateTimeout) if a concurrent refresh of the same key is wedged near the full 30s refresh timeout. This requires a multi-second DB stall on a single indexed point query; the typical case is milliseconds.
• Primary load: API-key cache-miss fills and the 1-minute refresh-ahead reads now hit the primary instead of the replica (accepted tradeoff for read-after-write-safe revocation; see review threads).
• No end-to-end integration test: the integration harness has no admin-token or auth-provider-JWT plumbing for the key-deletion endpoints. The handler-level test runs against a real Postgres and asserts the deleted key's hash is invalidated.

Test plan

• Extended TestDeleteAdminTeamsTeamIDApiKeysDeletesTeamKey to assert the deleted key's cache entry is invalidated (hash computed from the raw created key).
• New TestRedisCache_DeleteWaitsForInflightWriter holds the write lock past the old 5s acquire cap and asserts the DEL lands after the writer's SET (verified to fail against the previous Delete behavior).
• go test -race passes for packages/shared/pkg/cache, packages/api, packages/auth, packages/dashboard-api; golangci-lint clean.

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

[linear-code]

EN-1874

[cursor]

PR Summary

Medium Risk
Touches authentication revocation, synchronous cache invalidation (up to ~45s in edge cases), and moves API-key cache fills to the primary DB.

Overview
Deleted API keys could keep working until the Redis auth cache expired because deletes never cleared the cached entry. After a successful delete, the flow now drops the cache entry for the returned key hash via a new auth invalidation hook on both user and admin delete paths. API-key lookups on cache miss use the primary database so replication lag cannot re-cache a removed key, and cache deletes wait on the per-key writer lock so a concurrent refresh cannot write stale auth data back after revocation.

^{Reviewed by Cursor Bugbot for commit bf7ad2b. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 47.36842% with 10 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
packages/api/internal/team/apikeys.go	0.00%	6 Missing ⚠️
packages/shared/pkg/cache/redis.go	72.72%	3 Missing ⚠️
packages/api/internal/handlers/apikey.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[gemini-code-assist]

Code Review

This pull request implements immediate cache invalidation when an API key is deleted. It updates the database query to return the deleted key's hash and introduces an InvalidateAPIKeyCache method to the AuthService interface. Feedback suggests adding a defensive nil check for the authService parameter in DeleteAPIKey to prevent potential runtime panics.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59f199fc18

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

No bugs found, but this PR touches the API-key authentication/revocation cache path, so I'm deferring for a human look rather than auto-approving.

Reviewed the delete-key flow end-to-end: the SQL query change (RETURNING api_key_hash instead of id), the new InvalidateAPIKeyCache plumbed through both the user-facing and admin delete handlers, and the AuthService interface/impl change. Checked the async background cache-refresh path noted by the bug hunter (teamCache's refresh-ahead goroutine) for resurrecting a just-deleted key's cache entry — the PR's own notes already call out the residual replica-lag race, and it's bounded to seconds rather than the original 5-minute TTL. Test coverage (admin_api_keys_test.go) asserts the deleted key's hash is passed to InvalidateAPIKeyCache.

Extended reasoning...

Overview

The PR fixes delayed API-key revocation: deleting a team API key now returns the key's api_key_hash from the DB (via an updated sqlc query) and invalidates the corresponding Redis auth-cache entry through a new InvalidateAPIKeyCache method on the auth Service interface. Both the user-facing (DELETE /api-keys/{id}) and admin (DELETE /admin/teams/{teamID}/api-keys/{id}) delete paths are updated to call this. Generated sqlc code and the raw SQL file were regenerated to match.

Security risks

This is directly on the authentication/authorization path (API key validation and cache invalidation), so it warrants care even though the change itself is narrow. The main risk class is incomplete invalidation (revoked keys continuing to authenticate) rather than a new vulnerability being introduced. The PR description is transparent about a residual replica-lag race (a request reading from the DB read replica could theoretically re-populate the cache for a just-deleted key before replication catches up), which is a pre-existing architectural property of the read-replica + cache design, not something this PR introduces or worsens — it shrinks the exposure window from ~5 minutes to a replication-lag-scale window.

Level of scrutiny

Given this sits on an auth revocation path, I lean toward the 'do not approve' criterion for security-sensitive code even in the absence of concrete bugs. The change is small and mechanical (return a hash instead of an id, call an invalidate method), and is well-tested, but the surface (auth cache correctness) is the kind of thing where a human sign-off is worth the small extra cost.

Other factors

Test coverage is solid: the admin delete test was extended to assert the cache invalidation call with the correct hash, and a fake auth service implementation was added/extended for both the admin and dashboard-api test suites to satisfy the new interface method. No integration test was added, but the PR description explains why (the integration harness lacks admin-token/JWT plumbing for these endpoints) and the handler-level test already exercises real Postgres.

[tvi]

Addressed the P1 in b66fcaa: GetTeamByHashedAPIKey now reads from the primary (authDB.Write) instead of the read replica, so a cache miss racing replication lag can no longer re-cache a just-deleted key for a fresh 5-minute TTL. This follows the existing precedent in NewAuthService, where the OIDC identity lookup already uses the primary for the same read-after-write reason.

Considered a tombstone/negative-cache for deleted hashes as the alternative (would keep this query on the replica), but went with the primary read for simplicity — the extra primary load is bounded by the cache-miss + 1-minute refresh rates.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b66fcaafd9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cursor]

Stale comment

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 386c2b1e07

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jakubno]

👍🏻