fix: release auth write lock before post-credential hook [IDE-1901]

[nick-y-snyk]

Summary

• Authenticate() held a write lock (a.m.Lock()) for the entire duration, including the postCredentialUpdateHook which does feature flag HTTP calls via populateAllFolderConfigs()
• All IsAuthenticated() callers (status bar, scanner, IDE heartbeat) blocked on RLock() for seconds → LS became unresponsive during login
• Fix: updateCredentials() now returns a post-action closure (hook + notification) that runs after the write lock is released. Ordering invariant preserved: token stored → feature flags populated → auth notification sent

Before (broken)

sequenceDiagram
    participant A as Authenticate()
    participant L as a.m (RWMutex)
    participant N as Network
    participant I as IsAuthenticated()

    A->>L: Lock() 🔒
    activate L
    Note over L: write lock held

    A->>A: authProvider.Authenticate()
    A->>A: tokenService.SetToken()
    A->>N: postCredentialUpdateHook()
    activate N
    Note over N: HTTP: feature flags<br/>HTTP: SAST API<br/>(seconds...)

    I->>L: RLock() ⏳
    Note over I,L: BLOCKED — waiting for<br/>write lock release

    N-->>A: hook done
    deactivate N
    A->>A: notifier.Send(AuthParams)
    A->>L: Unlock() 🔓
    deactivate L

    L-->>I: RLock() acquired ✓
    Note over I: finally unblocked

Loading

After (fixed)

sequenceDiagram
    participant A as Authenticate()
    participant L as a.m (RWMutex)
    participant N as Network
    participant I as IsAuthenticated()

    A->>L: Lock() 🔒
    activate L
    Note over L: write lock held

    A->>A: authProvider.Authenticate()
    A->>A: tokenService.SetToken()
    A->>A: capture hook ref
    A->>A: configureProviders()
    A->>L: Unlock() 🔓
    deactivate L

    I->>L: RLock() ✅
    Note over I: immediately unblocked,<br/>sees new token

    A->>N: postAction(): hook()
    activate N
    Note over N: HTTP: feature flags<br/>HTTP: SAST API<br/>(no lock held)
    N-->>A: hook done
    deactivate N
    A->>A: notifier.Send(AuthParams)
    Note over A: ordering preserved:<br/>token → flags → notification

Loading

Lock scope comparison

gantt
    title Write lock duration comparison
    dateFormat X
    axisFormat %s

    section Before
    Lock held (token + hook + notify)  :crit, 0, 5
    Hook network calls (blocking)      :active, 1, 4

    section After
    Lock held (token only)             :crit, 0, 1
    Hook runs (no lock)                :done, 1, 4

Loading

updateCredentials caller flow

flowchart TD
    UC["updateCredentials(token, notify, url)"]
    UC --> STORE["Store token<br/>Remove old from cache<br/>Reset notif dedup"]
    STORE --> CAPTURE["Capture hook ref"]
    CAPTURE --> RETURN["Return postAction closure"]

    RETURN --> AUTH["Authenticate()<br/>runs AFTER Unlock()"]
    RETURN --> UPCRED["UpdateCredentials()<br/>runs AFTER Unlock()"]
    RETURN --> LOGOUT["logout()<br/>runs inline<br/>(hook skipped: token='')"]
    RETURN --> CALLBACK["credentialsCallback<br/>runs in goroutine<br/>(already no lock)"]

    style AUTH fill:#2d6,stroke:#333,color:#fff
    style UPCRED fill:#2d6,stroke:#333,color:#fff
    style LOGOUT fill:#69c,stroke:#333,color:#fff
    style CALLBACK fill:#69c,stroke:#333,color:#fff

Loading

Test plan

• Test_PostCredentialUpdateHook_CalledBeforeNotification — hook before notification ordering
• TestLoginCommand_Execute_FeatureFlagsPopulatedBeforeAuthNotification — end-to-end login ordering
• Test_UpdateCredentials — all credential update variants
• Test_Authenticate — endpoint config after auth
• Race detector clean on ./infrastructure/authentication/... and ./domain/ide/command/...

[snyk-io]

✅ 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]

PR Reviewer Guide 🔍

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Deadlock Risk 🔴 [critical] The Authenticate and UpdateCredentials methods have removed the defer a.m.Unlock() call and now manually invoke a.m.Unlock() after performing core logic. If a.authenticate(ctx) (which involves external provider calls) or a.updateCredentials panics, the write lock on the mutex a.m will never be released. This would permanently deadlock all subsequent authentication-related calls, including IsAuthenticated and Logout. The lock should be managed via defer or a recovery block to ensure release on panic. a.m.Lock() a.previousAuthCtxCancelFuncMu.Lock() ctx, a.previousAuthCtxCancelFunc = context.WithCancel(ctx) a.previousAuthCtxCancelFuncMu.Unlock() defer a.previousAuthCtxCancelFunc() // need to clean up resources if we weren't interrupted, impl should ensure its safe to double call var postAction func() token, postAction, err = a.authenticate(ctx) a.m.Unlock() Concurrency Issue 🟠 [major] The credentialsUpdateCallback calls the unexported updateCredentials method from a new goroutine without acquiring the service's write lock (a.m). The documentation for this method in auth_service.go explicitly states that it 'doesn't have a mutex lock' and assumes the caller holds it. Bypassing the lock when accessing and modifying shared state (like a.postCredentialUpdateHook and a.authCache) introduces a data race. The callback should use the exported UpdateCredentials method which correctly manages the lock. postAction := authenticationService.updateCredentials(newToken, true, false) postAction()

📚 Repository Context Analyzed This review considered 15 relevant code sections from 13 files (average relevance: 0.93) domain/ide/command/login.go (lines 1-133) infrastructure/authentication/provider_fake.go (lines 1-140) infrastructure/authentication/oauth_provider.go (lines 1-92) infrastructure/authentication/auth_provider.go (lines 1-61) application/config/token_service.go (lines 1-104) domain/ide/command/apply_auth_config.go (lines 1-64) application/server/configuration.go (lines 371-542) ... and 6 more file(s)