Skip to content

fix: prevent stale async credential clears from overwriting a freshly-set token [IDE-2106]#1353

Open
basti-snyk wants to merge 1 commit into
mainfrom
fix/IDE-2106-flake
Open

fix: prevent stale async credential clears from overwriting a freshly-set token [IDE-2106]#1353
basti-snyk wants to merge 1 commit into
mainfrom
fix/IDE-2106-flake

Conversation

@basti-snyk

Copy link
Copy Markdown
Contributor

Summary

  • Fixes the flaky Test_E2E_AuthenticationMethodChangeClearsIncompatibleToken (Windows CI, IDE-2106), where switching auth method and setting a token in one didChangeConfiguration sometimes left the token empty.
  • Root cause: on that path applyAuthenticationMethod runs before applyToken. ConfigureProviders/logout enqueue empty-token clears through the async OAuth storage bridge (QueueCredentialUpdate("")). The single credentialUpdateWorker could drain those stale clears after applyToken's synchronous UpdateCredentials wrote the real token, wiping it.
  • Fix: a monotonic syncGeneration counter on AuthenticationServiceImpl. The increment lives in the private updateCredentials, so every synchronous writer (UpdateCredentials, authenticate, logout) supersedes earlier-queued async updates; the worker discards any update whose generation predates the latest synchronous write — a strict happens-before. The worker now holds a.m only for the generation check + token mutation and releases it before running the postCredentialUpdateHook, removing a reentrancy deadlock hazard.
  • Adds regression tests for the authenticate, logout, and rapid-rotation paths plus a worker-hook no-deadlock test.

Test plan

  • go test ./infrastructure/authentication/... -race green
  • Test_E2E_AuthenticationMethodChangeClearsIncompatibleToken proven RED before fix, 200/200 GREEN after (-race, -cpu=1,2,4)
  • New unit tests RED-without-fix / GREEN-with-fix for authenticate + logout paths
  • make test (full unit suite) green at this commit
  • CI integration + smoke suites (run on PR)

This fix was produced by an automated flake-fix loop.

…-set token [IDE-2106]

Test_E2E_AuthenticationMethodChangeClearsIncompatibleToken flaked on
Windows CI: after switching auth method + setting a token in one
didChangeConfiguration, the token sometimes read back empty.

Root cause: on that path applyAuthenticationMethod runs before applyToken.
ConfigureProviders/logout enqueue empty-token clears through the async
OAuth storage bridge (QueueCredentialUpdate("")). The single
credentialUpdateWorker could drain those stale clears AFTER applyToken's
synchronous UpdateCredentials wrote the real token, wiping it.

Fix: a monotonic syncGeneration counter on AuthenticationServiceImpl.
The increment lives in the private updateCredentials, so every
synchronous writer (UpdateCredentials, authenticate, logout) supersedes
earlier-queued async updates; the worker discards any update whose
generation predates the latest synchronous write, giving a strict
happens-before. The worker now holds a.m only for the generation check
and token mutation and releases it before running the
postCredentialUpdateHook, eliminating a reentrancy deadlock hazard.

Adds regression tests for the authenticate, logout, and rapid-rotation
paths plus a worker-hook no-deadlock test.

Produced by an automated flake-fix loop.
@snyk-io

snyk-io Bot commented Jun 23, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected
📚 Repository Context Analyzed

This review considered 19 relevant code sections from 13 files (average relevance: 1.00)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant