feat: clear global overrides on Project Defaults reset [IDE-2149]

[nick-y-snyk]

What

Implements the Eclipse side of IDE-2149 — global ("Project Defaults") reset overrides in the HTML settings page (sibling of the per-folder reset, IDE-1945).

Honors inbound global resets from the LS: when $/snyk.configuration sends a global setting as {value:null, changed:true}, Eclipse drops the persisted preference value and clears explicit-change tracking, so the override is not re-asserted on the next outbound sync/reconnect.

Changes

• SnykExtendedLanguageClient.persistGlobalSettings — on a null + changed:true reset, removePref(prefKey) + clearExplicitlyChanged(prefKey) instead of skipping; non-null persist behavior unchanged. (Inbound still never marks settings as explicitly changed.)
• Preferences.removePref — remove a persisted value (falls back to default).
• HTMLSettingsPreferencePage — form-save null path also removes the persisted value so the next outbound sends changed:false.
• InMemoryPreferenceStore (test fake) — implement remove.
• Tests — inbound null reset clears value + explicit-changed; null+changed:false ignored; re-push guard in LsConfigurationUpdaterTest (changed:false after reset).

Notes

• Mockito-based tests (SnykExtendedLanguageClientTest, LsConfigurationUpdaterTest) could not run locally — the inline mock-maker fails under JDK 21 inside the Tycho/Equinox surefire runtime. They will run in CI. PMD and compilation are clean locally; non-mock tests (HTMLSettingsPreferencePageTest 25/25, PreferencesTest, FolderConfigSettingsTest) pass.
• Requires the snyk-ls IDE-2149 PR for end-to-end behavior; safe to merge independently.

🤖 Generated with Claude Code

[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 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Restricted Reset Capability 🟡 [minor] The removePref method explicitly returns early for encryptedPreferenceKeys (like AUTH_TOKEN_KEY). While the PR notes state these are handled separately, this creates an inconsistency where a global reset signal for the token via the $/snyk.configuration notification or the HTML settings form will be a no-op at the storage level for the token, whereas other settings will revert to defaults. public final void removePref(String key) { if (key == null || encryptedPreferenceKeys.contains(key)) { return; } insecurePreferences.remove(key); }

📚 Repository Context Analyzed This review considered 22 relevant code sections from 8 files (average relevance: 0.99) plugin/src/main/java/io/snyk/languageserver/protocolextension/SnykExtendedLanguageClient.java (6 segments, lines 401-500) plugin/src/main/java/io/snyk/eclipse/plugin/preferences/HTMLSettingsPreferencePage.java (3 segments, lines 1-100) plugin/src/main/java/io/snyk/eclipse/plugin/preferences/Preferences.java (lines 1-100) plugin/src/main/java/io/snyk/eclipse/plugin/preferences/InMemorySecurePreferenceStore.java (lines 1-100) plugin/src/main/java/io/snyk/eclipse/plugin/preferences/SecurePreferenceStore.java (lines 1-100) plugin/src/main/java/io/snyk/languageserver/LsSettingsRegistry.java (lines 101-200) plugin/src/main/java/io/snyk/languageserver/LsConfigurationUpdater.java (lines 1-80) ... and 1 more file(s)

[basti-snyk]

Should Fix — two reset paths can be silently undone. This is the core reset logic; two interactions can defeat it:

1. Severity-filter partial reset re-asserts siblings. The four severity-filter keys share an OR'd outbound changed flag in LsConfigurationUpdater/LsSettingsRegistry. If the LS pushes {value:null, changed:true} for only a subset of the four, this loop clears just those keys, but the un-reset siblings keep their persisted value and explicit-changed entry — and because the outbound changed is OR'd across the co-key group, all four are still sent changed=true on the next sync, re-asserting the overrides the reset was meant to clear. Worth confirming what subset the LS can emit, and handling the co-key group atomically if it can send a strict subset.

2. Env-var reseed resurrects reset endpoint/org. ENDPOINT/ORGANIZATION are non-encrypted registry entries reachable by this reset path, but the Preferences constructor unconditionally re-seeds them from SNYK_API/SNYK_ORG on every restart. So a reset of those keys is silently undone on the next plugin start when those env vars are set.

— AI review

[basti-snyk]

Suggestion — code and comment disagree about the trigger. The branch fires purely on value==null && changed==true, but the comment (and removePref's javadoc) describe this as an "org-scope global" reset. ConfigSetting.originScope is deserialized from the wire yet never read here, so any {value:null, changed:true} — regardless of scope — performs the destructive override removal. Either gate on "org".equals(setting.getOriginScope()), or drop the "org-scope" wording so the code and its docs agree about what triggers the removal. — AI review

[basti-snyk]

ℹ️ Automated AI review summary — non-blocking. Two inline comments posted (one Should-Fix, one Suggestion). No approval is given; that's left to a human.

Verdict: clean, surgical, well-tested change. The shared removePref + clear-tracking primitive is correctly factored, both reset entry points reuse it, and the negative case (value:null, changed:false ignored) is explicitly tested. No security findings (no dependency/manifest changes). The InMemoryPreferenceStore.remove fix (replacing a TODO stub) is a real fix that also makes the test double faithful.

Should Fix (inline)

• Two ways the reset can be silently undone: (1) severity-filter partial reset re-asserts the co-key siblings because the outbound changed flag is OR'd across the group; (2) endpoint/org reset is resurrected on restart by the unconditional SNYK_API/SNYK_ORG env reseed in the Preferences constructor.

Suggestions

• originScope is deserialized but never enforced, while comments claim "org-scope" — align code and docs (inline).
• The inbound loop calls the flushing clearExplicitlyChanged per key (N whole-node flushes for N keys); the form path batches with clearExplicitlyChangedNoFlush + one trailing flushExplicitChanges(). Mirroring that collapses N flushes to 1 and avoids partial-flush on a mid-loop exception.
• The reset idiom (removePref + clear-tracking) is open-coded at both call sites and has already drifted on flush behaviour. A single Preferences.resetPref(key) / resetPrefNoFlush(key) would keep both paths provably identical.
• Coverage is unit-only with fakes/mocks; the headline guarantee ("override removed → next sync emits changed=false") is never exercised end-to-end against a real store. One integration test would close that gap.
• For a human: confirm the HTML/JS that emits value:null per global key (AC Win fix #3) lands here or in a sibling PR — this diff implements only the Java receive side.

— AI review