fix(telemetry): scope consent cache by auth token#751
Conversation
Prevent users from inheriting each other's cached telemetry consent by scoping the cache key to a hash of the current auth token. This ensures that switching users (via login/logout) always results in a cache miss, eliminating the race condition where a detached flush worker could write consent for the wrong user. The cache now uses keys like 'telemetryConsent:a1b2c3d4e5f6' (with the token hash as a suffix), so each user has an isolated cache entry. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Claude finished @rexxars's task in 1m 5s —— View job Review
Cache entry accumulation ( Each unique token creates a new config store entry under Deduplication comment is slightly inaccurate ( The comment says deduplication "does not work across concurrent calls to
The function is exported for testing, but callers outside the module have no legitimate use for it — they'd need to replicate No bugs, security issues, or logic errors found. |
Add a clear() method to the in-memory config store and use it in beforeEach to completely clear the cache between tests. This prevents cache pollution from tests that use different tokens (token-a, token-b) from affecting subsequent tests. Previously, only the base key and test-token key were cleared, leaving entries from other tokens in the store. While tests still passed due to unique cache keys per token, this was brittle.
Remove mockGetCliToken.mockResolvedValue('test-token') from the first two
tests since it's already set in beforeEach. Tests that need different tokens
(token-a, token-b, undefined) still set them explicitly.
Coverage Delta
Comparing 1 changed file against main @ Overall Coverage
|
Remove vi.spyOn that re-implements getCliToken with a custom function. Instead, use vi.resetModules() + dynamic import in beforeEach to get a fresh module and reset the module-level cachedToken between tests. This tests the actual implementation rather than a spy, and each test gets a clean slate for the token cache.
…ade-off Remove @internal from getTelemetryConsentCacheKey since it's exported for test use. Add comment documenting that in-flight request deduplication doesn't work across concurrent calls to fetchTelemetryConsent() since createExpiringConfig is instantiated on every call. This is a pre-existing design trade-off (favoring simpler code over deduplication) that's worth documenting.
1 participant
Scope the telemetry consent cache key to the current auth token, ensuring each user gets an isolated cache entry. This prevents one user from inheriting another user's cached consent status when tokens change due to login/logout.
The cache now uses token-scoped keys like 'telemetryConsent:a1b2c3d4e5f6' (token hash suffix), eliminating the race condition where detached flush workers could write consent for the wrong user.
Includes comprehensive tests verifying cache isolation between different tokens and correct behavior when logged out.