Skip to content

fix: pass trusted folders through didChangeConfiguration [IDE-1893]#1189

Open
nick-y-snyk wants to merge 2 commits intorefactor/IDE-1786_folder-config-refactoringfrom
fix/IDE-1786-trusted-folders-in-didChangeConfiguration
Open

fix: pass trusted folders through didChangeConfiguration [IDE-1893]#1189
nick-y-snyk wants to merge 2 commits intorefactor/IDE-1786_folder-config-refactoringfrom
fix/IDE-1786-trusted-folders-in-didChangeConfiguration

Conversation

@nick-y-snyk
Copy link
Copy Markdown
Contributor

Description

The IDE-1786 refactoring moved TrustedFolders into InitializationOptions as init-only metadata, but on main it was updatable at runtime via writeSettings/updateTrustedFolders. This caused trusted folder changes sent via workspace/didChangeConfiguration to be silently dropped.

Changes:

  • Add TrustedFolders []string field to LspConfigurationParam (matching the field on InitializationOptions)
  • Wire handlePushModel and handlePullModel to call applyTrustedFolders with the new field
  • Update push/pull model entry conditions to also check for TrustedFolders
  • Add test for trusted folders via didChangeConfiguration push model
  • Add js-tests/fixtures/*.html to .gitignore (generated test artifacts)

Checklist

  • Tests added and all succeed
  • Regenerated mocks, etc. (make generate)
  • Linted (make lint-fix)
  • README.md updated, if user-facing
  • License file updated, if new 3rd-party dependency is introduced

The IDE-1786 refactoring moved TrustedFolders into InitializationOptions
as init-only metadata, but on main it was updatable at runtime via
writeSettings/updateTrustedFolders. This caused trusted folder changes
sent via workspace/didChangeConfiguration to be silently dropped.

Add TrustedFolders field to LspConfigurationParam (matching
InitializationOptions) and wire it through handlePushModel and
handlePullModel to applyTrustedFolders. Also add js-tests/fixtures/*.html
to .gitignore as generated test artifacts.
@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Apr 1, 2026

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.

@nick-y-snyk nick-y-snyk marked this pull request as ready for review April 1, 2026 09:23
@nick-y-snyk nick-y-snyk requested review from a team as code owners April 1, 2026 09:23
@snyk-pr-review-bot

This comment has been minimized.

@nick-y-snyk nick-y-snyk changed the title fix: pass trusted folders through didChangeConfiguration [IDE-1786] fix: pass trusted folders through didChangeConfiguration [IDE-1893] Apr 1, 2026
…DE-1786-trusted-folders-in-didChangeConfiguration

Fix trustedFolders nil vs empty: use \!= nil check instead of len > 0
so that an empty TrustedFolders slice (clear all) is handled correctly.
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Stale Trust State 🟠 [major]

In handlePushModel and handlePullModel, UpdateSettings is called before applyTrustedFolders. UpdateSettings internally calls processFolderConfigs, which invokes validateLockedFields. This validation checks if settings are locked by organization policy. If the incoming payload contains both a new trusted folder and a setting for that folder that would be 'unlocked' by that trust, the validation will fail because it evaluates against the stale trust state from before applyTrustedFolders is executed.

UpdateSettings(conf, engine, logger, params.Settings, params.FolderConfigs, triggerSource, di.ConfigResolver())
applyTrustedFolders(conf, engine, logger, params.TrustedFolders, triggerSource, di.ConfigResolver())
API Compatibility 🟡 [minor]

Adding TrustedFolders to LspConfigurationParam changes the wire format for workspace/didChangeConfiguration. While the field is omitempty, clients that do not strictly follow the Snyk-specific LSP extensions might send an empty array if they 'zero-value' their internal representation. In application/server/configuration.go, applyTrustedFolders treats an empty (non-nil) slice as a command to clear all trusted folders, which could lead to unintended security prompts for users if their client clears the list during a routine configuration sync.

TrustedFolders []string                  `json:"trustedFolders,omitempty"`
📚 Repository Context Analyzed

This review considered 8 relevant code sections from 4 files (average relevance: 0.96)

defer logger.Info().Str("method", "WorkspaceDidChangeConfiguration").Msg("DONE")

if len(params.Settings.Settings) > 0 || len(params.Settings.FolderConfigs) > 0 {
if len(params.Settings.Settings) > 0 || len(params.Settings.FolderConfigs) > 0 || params.Settings.TrustedFolders != nil {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a string slice, check for len

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.

2 participants