fix: populate feature flags before auth notification [IDE-1901] by nick-y-snyk · Pull Request #1194 · snyk/snyk-ls

nick-y-snyk · 2026-04-07T10:06:58Z

Summary

Fix race condition where scans after login fail with "Snyk Code is not enabled" because the IDE triggers a scan before SAST settings are cached
postCredentialUpdateHook runs populateAllFolderConfigs between credential storage and auth notification, ensuring feature flags and SAST settings are available before the IDE reacts
sendFolderConfigs made synchronous in login command

Review feedback addressed

Use unenriched folder config (no git enrichment needed for feature flag fetch)
Loop only on trusted folders
Remove unnecessary Clone() — PopulateFolderConfig only reads org from config
Set ConfigResolver on folder config so SetFeatureFlag persists to GAF (was silently no-op without it)
Case-insensitive error string matching in shouldCauseLogout
Extract isTransientNetworkError helper to reduce cyclomatic complexity

Test plan

TestLoginCommand_Execute_FeatureFlagsPopulatedBeforeAuthNotification — regression test verifying feature flags populate BEFORE $/snyk.hasAuthenticated notification (fails when hook is removed)
Test_PostCredentialUpdateHook_CalledBeforeNotification — hook runs between token storage and notification
Test_shouldCauseLogout — all 13 cases pass including expired refresh token and transient network errors
TestIsAuthenticated_ConcurrentCallsSendOnlyOneNotification — concurrent dedup still works
All existing TestLoginCommand_* tests pass
Lint clean, zero issues

snyk-io · 2026-04-07T10:07:11Z

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

acke · 2026-04-08T07:57:36Z

domain/ide/command/login.go

 		// canceled (e.g. when the IDE cancels the snyk.login request after auth completes).
 		cmd.ldxSyncService.RefreshConfigFromLdxSync(context.Background(), conf, cmd.engine, logger, config.GetWorkspace(conf).Folders(), cmd.notifier)
-		go sendFolderConfigs(conf, cmd.engine, logger, cmd.notifier, cmd.featureFlagService, cmd.configResolver)
+		sendFolderConfigs(conf, cmd.engine, logger, cmd.notifier, cmd.featureFlagService, cmd.configResolver)


Ideally, if a user has large workspaces with 100+ projects, we want a separation between the login flow and the setup folders. With clear indications for the user that they are logged in, but snyk is not ready to scan until all configs are ready.

This can still run in a go routine, since it will get to this point after the postAuth hook

acke · 2026-04-08T08:14:51Z

infrastructure/authentication/auth_service_impl.go

+	// Expired or revoked refresh tokens cause the OAuth2 token endpoint to reject
+	// the request. http.Client wraps this in url.Error, so detect it before the
+	// generic transport-error guard.
+	errMsg := err.Error()


Could we user a re.Response.StatusCode == 401 or 400 to catch expired/invalid API credentials?

https://github.com/snyk/sweater-comb/blob/main/docs/standards/rest.md#401---unauthorized

Needs to be verified for both token an oauth.

acke · 2026-04-08T08:16:58Z

infrastructure/authentication/auth_service_impl.go

 	if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
 		return false
 	}
+	// Expired or revoked refresh tokens cause the OAuth2 token endpoint to reject


Should the change here be submitted as a separate PR for for changelog clarity?

ShawkyZ · 2026-04-09T14:05:56Z

domain/ide/command/folder_handler.go

+	}
+	log := logger.With().Str("method", "populateAllFolderConfigs").Logger()
+	for _, folder := range ws.Folders() {
+		fc, err := folderconfig.GetOrCreateFolderConfig(conf, folder.Path(), &log)


Use Unenriched variant

ShawkyZ · 2026-04-09T14:06:32Z

domain/ide/command/folder_handler.go

+		if fc == nil {
+			continue
+		}
+		featureFlagService.PopulateFolderConfig(fc.Clone())


why the clone ?

ShawkyZ · 2026-04-09T14:07:35Z

domain/ide/command/folder_handler.go

+		return
+	}
+	log := logger.With().Str("method", "populateAllFolderConfigs").Logger()
+	for _, folder := range ws.Folders() {


loop on trusted folders

ShawkyZ · 2026-04-09T14:17:11Z

infrastructure/authentication/auth_service_impl.go

+	// the request. http.Client wraps this in url.Error, so detect it before the
+	// generic transport-error guard.
+	errMsg := err.Error()
+	if strings.Contains(errMsg, "oauth2/token") && strings.Contains(errMsg, "authentication failed") {


nitpick: make the comparison case insensitive

Race condition: after login, IDE receives $/snyk.hasAuthenticated and triggers scan before SAST settings are cached → "Snyk Code not enabled". Fix: postCredentialUpdateHook runs populateAllFolderConfigs between credential storage and auth notification, ensuring feature flags and SAST settings are available before the IDE reacts. Review feedback addressed: - Use unenriched folder config (no git enrichment for feature flag fetch) - Loop only on trusted folders - Remove unnecessary Clone() - Set ConfigResolver on folder config so SetFeatureFlag persists to GAF - Make sendFolderConfigs synchronous in login command - Case-insensitive error string matching in shouldCauseLogout - Extract isTransientNetworkError to reduce cyclomatic complexity Includes regression test verifying feature flags populate before auth notification is sent (fails when hook is removed).

- Synchronize SetPostCredentialUpdateHook with a.m mutex to prevent data race - Skip hook when token is empty (logout path) to avoid wasteful API calls - Wrap hook invocation in recover() to prevent LSP server crash on panic

ShawkyZ · 2026-04-14T16:32:57Z

infrastructure/authentication/auth_service_impl.go

+		func() {
+			defer func() {
+				if r := recover(); r != nil {
+					a.engine.GetLogger().Error().Interface("panic", r).Msg("postCredentialUpdateHook panicked")


why are you expecting it to panic?

I didn't. just a double safety (ai suggestion to prevent ls crash). Do you want this removed?

I also added an actual test which fails without other changes. But haven't tested manually yet

we have a global recovery that should catch it if it happens, I think this is not necessary here.

…DE-1901-sast-not-enabled-after-login

snyk-pr-review-bot · 2026-04-14T16:58:45Z

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Blocking Global Lock 🔴 [critical] The `updateCredentials` function invokes the `postCredentialUpdateHook` (line 248) while holding the global `a.m` mutex lock. The registered hook in `login.go` calls `populateAllFolderConfigs`, which performs network requests to fetch feature flags and SAST settings via `featureFlagService.PopulateFolderConfig`. Holding a global mutex across sequential network calls will block all other authentication-related operations—including high-frequency calls like `IsAuthenticated()` from the status bar or scanner—making the Language Server unresponsive during the entire feature flag fetch sequence. if a.postCredentialUpdateHook != nil && newToken != "" { func() { defer func() { if r := recover(); r != nil { a.engine.GetLogger().Error().Interface("panic", r).Msg("postCredentialUpdateHook panicked") } }() a.postCredentialUpdateHook() }() } Regression in Dynamic Population 🟠 [major] Removing `PopulateFolderConfig` from `buildLspFolderConfigs` (lines 115-117 in the old hunk) breaks the 'on-demand' population of feature flags. The new mechanism relies on explicit calls in `HandleFolders` and the login hook. However, `HandleFolders` calls `populateAllFolderConfigs` before handling untrusted folders. If a user trusts a folder via the trust dialog, that folder will never have its feature flags populated because `buildLspFolderConfigs` no longer performs this check, leading to 'Snyk Code is not enabled' errors for newly trusted folders. func buildLspFolderConfigs(conf configuration.Configuration, engine workflow.Engine, logger zerolog.Logger, featureFlagService featureflag.Service, configResolver types.ConfigResolverInterface) []types.LspFolderConfig { ws := config.GetWorkspace(conf) if ws == nil { return nil } log := logger.With().Str("method", "buildLspFolderConfigs").Logger() engineConfig := conf var lspFolderConfigs []types.LspFolderConfig for _, folder := range ws.Folders() { fc, err := folderconfig.GetOrCreateFolderConfig(engineConfig, folder.Path(), &log) if err != nil { log.Err(err).Msg("unable to load folderConfig") continue } if fc == nil { log.Warn().Str("path", string(folder.Path())).Msg("folder config is nil, skipping") continue } folderConfig := fc.Clone() // AutoDeterminedOrg is written to FolderMetadataKey by LDX-Sync (SetAutoDeterminedOrg); // no separate cache lookup is needed here. folderConfig.ConfigResolver = configResolver lspConfig := folderConfig.ToLspFolderConfig() if lspConfig != nil { lspFolderConfigs = append(lspFolderConfigs, lspConfig)
📚 Repository Context Analyzed This review considered 25 relevant code sections from 12 files (average relevance: 0.97) infrastructure/featureflag/featureflag.go (lines 1-241) infrastructure/authentication/oauth_provider.go (lines 1-92) infrastructure/authentication/auth_provider.go (lines 1-61) infrastructure/authentication/auth_configuration.go (lines 1-132) application/server/configuration.go (lines 1-189) infrastructure/code/code.go (lines 1-160) application/di/test_init.go (lines 1-117) ... and 5 more file(s)

nick-y-snyk temporarily deployed to Standard April 7, 2026 10:07 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT3 April 7, 2026 10:07 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT1 April 7, 2026 10:07 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT3 April 7, 2026 11:17 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT1 April 7, 2026 11:17 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to Standard April 7, 2026 11:17 — with GitHub Actions Inactive

acke reviewed Apr 8, 2026

View reviewed changes

ShawkyZ reviewed Apr 9, 2026

View reviewed changes

ShawkyZ added the stacked on central config label Apr 14, 2026

ShawkyZ temporarily deployed to Standard April 14, 2026 13:12 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to MT1 April 14, 2026 13:12 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to MT3 April 14, 2026 13:12 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to MT1 April 14, 2026 13:12 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to MT3 April 14, 2026 13:12 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to Standard April 14, 2026 13:12 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to MT3 April 14, 2026 13:28 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to Standard April 14, 2026 13:28 — with GitHub Actions Inactive

ShawkyZ temporarily deployed to MT1 April 14, 2026 13:28 — with GitHub Actions Inactive

ShawkyZ marked this pull request as ready for review April 14, 2026 13:49

ShawkyZ requested review from a team as code owners April 14, 2026 13:49

This comment has been minimized.

Sign in to view

nick-y-snyk force-pushed the fix/IDE-1901-sast-not-enabled-after-login branch from 166ec3f to ce97199 Compare April 14, 2026 15:16

nick-y-snyk temporarily deployed to MT1 April 14, 2026 15:16 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT3 April 14, 2026 15:16 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to Standard April 14, 2026 15:16 — with GitHub Actions Inactive

This comment has been minimized.

Sign in to view

nick-y-snyk changed the title ~~fix: call sendFolderConfigs synchronously after login [IDE-1901]~~ fix: populate feature flags before auth notification [IDE-1901] Apr 14, 2026

fix: harden postCredentialUpdateHook safety [IDE-1901]

4ebb2cc

- Synchronize SetPostCredentialUpdateHook with a.m mutex to prevent data race - Skip hook when token is empty (logout path) to avoid wasteful API calls - Wrap hook invocation in recover() to prevent LSP server crash on panic

nick-y-snyk temporarily deployed to MT3 April 14, 2026 15:54 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to Standard April 14, 2026 15:54 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT1 April 14, 2026 15:54 — with GitHub Actions Inactive

This comment has been minimized.

Sign in to view

ShawkyZ reviewed Apr 14, 2026

View reviewed changes

Merge branch 'refactor/IDE-1786_folder-config-refactoring' into fix/I…

ed3d3e2

…DE-1901-sast-not-enabled-after-login

nick-y-snyk temporarily deployed to Standard April 14, 2026 16:56 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT3 April 14, 2026 16:56 — with GitHub Actions Inactive

nick-y-snyk temporarily deployed to MT1 April 14, 2026 16:56 — with GitHub Actions Inactive

Conversation

nick-y-snyk commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review feedback addressed

Test plan

Uh oh!

snyk-io bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShawkyZ Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

nick-y-snyk Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snyk-pr-review-bot bot commented Apr 14, 2026

PR Reviewer Guide 🔍

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nick-y-snyk commented Apr 7, 2026 •

edited

Loading

snyk-io bot commented Apr 7, 2026 •

edited

Loading

ShawkyZ Apr 9, 2026 •

edited

Loading

nick-y-snyk Apr 14, 2026 •

edited

Loading