From 6c54972604682699950d3f884f902c9dc0632de0 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 27 Apr 2026 20:46:57 -0700 Subject: [PATCH 1/3] Prevent writing settings to global scope for environment and package managers --- docs/managing-python-projects.md | 2 + src/features/settings/settingHelpers.ts | 30 ------- .../settings/settingHelpers.unit.test.ts | 84 ++----------------- 3 files changed, 10 insertions(+), 106 deletions(-) diff --git a/docs/managing-python-projects.md b/docs/managing-python-projects.md index 645b3a1a..000715c2 100644 --- a/docs/managing-python-projects.md +++ b/docs/managing-python-projects.md @@ -211,6 +211,8 @@ You can set default managers that apply to all projects without explicit overrid } ``` +> **Important**: The extension never writes settings to the User (global) scope. All extension-managed settings are written at the Workspace or Workspace Folder level only. This prevents the extension from setting values that persist across unrelated projects and cause unexpected interference (see [#1468](https://github.com/microsoft/vscode-python-environments/issues/1468)). If a user want a user-level default, they can set it manually in their User `settings.json`. + ## Working with Multi-Root Workspaces Multi-root workspaces contain multiple top-level folders. The extension handles these seamlessly: diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 4ef9f9ab..84e72944 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -195,18 +195,6 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr } }); - const config = workspaceApis.getConfiguration('python-envs', undefined); - edits - .filter((e) => !e.project) - .forEach((e) => { - if (config.get('defaultEnvManager') !== e.envManager) { - promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global)); - } - if (config.get('defaultPackageManager') !== e.packageManager) { - promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global)); - } - }); - await Promise.all(promises); } @@ -269,15 +257,6 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr } }); - const config = workspaceApis.getConfiguration('python-envs', undefined); - edits - .filter((e) => !e.project) - .forEach((e) => { - if (config.get('defaultEnvManager') !== e.envManager) { - promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global)); - } - }); - await Promise.all(promises); } @@ -343,15 +322,6 @@ export async function setPackageManager(edits: EditPackageManagerSettings[]): Pr } }); - const config = workspaceApis.getConfiguration('python-envs', undefined); - edits - .filter((e) => !e.project) - .forEach((e) => { - if (config.get('defaultPackageManager') !== e.packageManager) { - promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global)); - } - }); - await Promise.all(promises); } diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index 1cb3a0c1..20b1f317 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -108,7 +108,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { } suite('setAllManagerSettings - Global Settings', () => { - test('should write global defaultEnvManager when value differs from current', async () => { + test('should NOT write global defaultEnvManager even when value differs from current', async () => { const mockConfig = createMockConfig({ currentEnvManager: VENV_MANAGER_ID, }); @@ -125,31 +125,10 @@ suite('Setting Helpers - Settings Write Behavior', () => { const envManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual(envManagerUpdates.length, 1, 'Should write global defaultEnvManager when value differs'); - assert.strictEqual(envManagerUpdates[0].value, SYSTEM_MANAGER_ID); + assert.strictEqual(envManagerUpdates.length, 0, 'Should never write to user/global settings'); }); - test('should NOT write global defaultEnvManager when value is same as current', async () => { - const mockConfig = createMockConfig({ - currentEnvManager: SYSTEM_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setAllManagerSettings([ - { - project: undefined, - envManager: SYSTEM_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - }, - ]); - - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual(envManagerUpdates.length, 0, 'Should NOT write when value is same as current'); - }); - - test('should write global defaultPackageManager when value differs from current', async () => { + test('should NOT write global defaultPackageManager even when value differs from current', async () => { const mockConfig = createMockConfig({ currentEnvManager: VENV_MANAGER_ID, currentPkgManager: PIP_MANAGER_ID, @@ -167,17 +146,12 @@ suite('Setting Helpers - Settings Write Behavior', () => { const pkgManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual( - pkgManagerUpdates.length, - 1, - 'Should write global defaultPackageManager when value differs', - ); - assert.strictEqual(pkgManagerUpdates[0].value, CONDA_MANAGER_ID); + assert.strictEqual(pkgManagerUpdates.length, 0, 'Should never write to user/global settings'); }); }); suite('setEnvironmentManager - Global Settings', () => { - test('should write when value differs from current', async () => { + test('should NOT write to global even when value differs from current', async () => { const mockConfig = createMockConfig({ currentEnvManager: VENV_MANAGER_ID, }); @@ -193,31 +167,12 @@ suite('Setting Helpers - Settings Write Behavior', () => { const envManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual(envManagerUpdates.length, 1, 'Should write global defaultEnvManager when value differs'); - }); - - test('should NOT write when value is same as current', async () => { - const mockConfig = createMockConfig({ - currentEnvManager: SYSTEM_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setEnvironmentManager([ - { - project: undefined, - envManager: SYSTEM_MANAGER_ID, - }, - ]); - - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual(envManagerUpdates.length, 0, 'Should NOT write when value is same'); + assert.strictEqual(envManagerUpdates.length, 0, 'Should never write to user/global settings'); }); }); suite('setPackageManager - Global Settings', () => { - test('should write when value differs from current', async () => { + test('should NOT write to global even when value differs from current', async () => { const mockConfig = createMockConfig({ currentPkgManager: PIP_MANAGER_ID, }); @@ -233,30 +188,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { const pkgManagerUpdates = updateCalls.filter( (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, ); - assert.strictEqual( - pkgManagerUpdates.length, - 1, - 'Should write global defaultPackageManager when value differs', - ); - }); - - test('should NOT write when value is same as current', async () => { - const mockConfig = createMockConfig({ - currentPkgManager: PIP_MANAGER_ID, - }); - sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); - - await setPackageManager([ - { - project: undefined, - packageManager: PIP_MANAGER_ID, - }, - ]); - - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, - ); - assert.strictEqual(pkgManagerUpdates.length, 0, 'Should NOT write when value is same'); + assert.strictEqual(pkgManagerUpdates.length, 0, 'Should never write to user/global settings'); }); }); }); From 0a3ab621af0dee199267f8c623cd5c216ff55075 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Tue, 28 Apr 2026 14:32:17 -0700 Subject: [PATCH 2/3] Update docs/managing-python-projects.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- docs/managing-python-projects.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/managing-python-projects.md b/docs/managing-python-projects.md index 000715c2..9f64f773 100644 --- a/docs/managing-python-projects.md +++ b/docs/managing-python-projects.md @@ -211,7 +211,7 @@ You can set default managers that apply to all projects without explicit overrid } ``` -> **Important**: The extension never writes settings to the User (global) scope. All extension-managed settings are written at the Workspace or Workspace Folder level only. This prevents the extension from setting values that persist across unrelated projects and cause unexpected interference (see [#1468](https://github.com/microsoft/vscode-python-environments/issues/1468)). If a user want a user-level default, they can set it manually in their User `settings.json`. +> **Important**: The extension never writes settings to the User (global) scope. All extension-managed settings are written at the Workspace or Workspace Folder level only. This prevents the extension from setting values that persist across unrelated projects and cause unexpected interference (see [#1468](https://github.com/microsoft/vscode-python-environments/issues/1468)). If a user wants a user-level default, they can set it manually in their User `settings.json`. ## Working with Multi-Root Workspaces From d2f08e206ab0a46fff61ff64e06701ee3c0ae09c Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Tue, 28 Apr 2026 14:40:10 -0700 Subject: [PATCH 3/3] address comments --- src/features/settings/settingHelpers.ts | 92 ++++++++++--------- .../settings/settingHelpers.unit.test.ts | 58 +++++++----- 2 files changed, 83 insertions(+), 67 deletions(-) diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 84e72944..0ee67396 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -2,7 +2,7 @@ import * as path from 'path'; import { ConfigurationScope, ConfigurationTarget, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { PythonProject } from '../../api'; import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID } from '../../common/constants'; -import { traceError, traceInfo, traceWarn } from '../../common/logging'; +import { traceError, traceInfo, traceVerbose, traceWarn } from '../../common/logging'; import * as workspaceApis from '../../common/workspace.apis'; import { PythonProjectManager, PythonProjectSettings } from '../../internal.api'; @@ -81,8 +81,16 @@ export function getDefaultPkgManagerSetting( return defaultManager; } +function traceIgnoredGlobalManagerEdits(functionName: string, count: number): void { + if (count > 0) { + traceVerbose( + `[${functionName}] Ignoring ${count} edit(s) without a project because python-envs does not persist manager defaults to User/global settings.`, + ); + } +} + export interface EditAllManagerSettings { - // undefined means global + // Edits without a project are ignored; python-envs does not persist manager defaults to User/global settings. project?: PythonProject; envManager: string; packageManager: string; @@ -95,24 +103,25 @@ interface EditAllManagerSettingsInternal { export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Promise { const noWorkspace: EditAllManagerSettingsInternal[] = []; const workspaces = new Map(); - edits - .filter((e) => !!e.project) - .map((e) => e as EditAllManagerSettingsInternal) - .forEach((e) => { - const w = workspaceApis.getWorkspaceFolder(e.project.uri); - if (w) { - workspaces.set(w, [ - ...(workspaces.get(w) || []), - { project: e.project, envManager: e.envManager, packageManager: e.packageManager }, - ]); - } else { - noWorkspace.push({ project: e.project, envManager: e.envManager, packageManager: e.packageManager }); - } - }); + const projectEdits = edits.filter((e): e is EditAllManagerSettingsInternal => !!e.project); + traceIgnoredGlobalManagerEdits('setAllManagerSettings', edits.length - projectEdits.length); + projectEdits.forEach((e) => { + const w = workspaceApis.getWorkspaceFolder(e.project.uri); + if (w) { + workspaces.set(w, [ + ...(workspaces.get(w) || []), + { project: e.project, envManager: e.envManager, packageManager: e.packageManager }, + ]); + } else { + noWorkspace.push({ project: e.project, envManager: e.envManager, packageManager: e.packageManager }); + } + }); noWorkspace.forEach((e) => { if (e.project) { - traceInfo(`Unable to find workspace for ${e.project.uri.fsPath}, will use global settings for this.`); + traceInfo( + `Unable to find workspace for ${e.project.uri.fsPath}, skipping settings update because no User/global fallback is written.`, + ); } }); @@ -199,7 +208,7 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr } export interface EditEnvManagerSettings { - // undefined means global + // Edits without a project are ignored; python-envs does not persist manager defaults to User/global settings. project?: PythonProject; envManager: string; } @@ -210,17 +219,16 @@ interface EditEnvManagerSettingsInternal { export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Promise { const noWorkspace: EditEnvManagerSettingsInternal[] = []; const workspaces = new Map(); - edits - .filter((e) => !!e.project) - .map((e) => e as EditEnvManagerSettingsInternal) - .forEach((e) => { - const w = workspaceApis.getWorkspaceFolder(e.project.uri); - if (w) { - workspaces.set(w, [...(workspaces.get(w) || []), { project: e.project, envManager: e.envManager }]); - } else { - noWorkspace.push({ project: e.project, envManager: e.envManager }); - } - }); + const projectEdits = edits.filter((e): e is EditEnvManagerSettingsInternal => !!e.project); + traceIgnoredGlobalManagerEdits('setEnvironmentManager', edits.length - projectEdits.length); + projectEdits.forEach((e) => { + const w = workspaceApis.getWorkspaceFolder(e.project.uri); + if (w) { + workspaces.set(w, [...(workspaces.get(w) || []), { project: e.project, envManager: e.envManager }]); + } else { + noWorkspace.push({ project: e.project, envManager: e.envManager }); + } + }); noWorkspace.forEach((e) => { if (e.project) { @@ -261,7 +269,7 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr } export interface EditPackageManagerSettings { - // undefined means global + // Edits without a project are ignored; python-envs does not persist manager defaults to User/global settings. project?: PythonProject; packageManager: string; } @@ -272,20 +280,16 @@ interface EditPackageManagerSettingsInternal { export async function setPackageManager(edits: EditPackageManagerSettings[]): Promise { const noWorkspace: EditPackageManagerSettingsInternal[] = []; const workspaces = new Map(); - edits - .filter((e) => !!e.project) - .map((e) => e as EditPackageManagerSettingsInternal) - .forEach((e) => { - const w = workspaceApis.getWorkspaceFolder(e.project.uri); - if (w) { - workspaces.set(w, [ - ...(workspaces.get(w) || []), - { project: e.project, packageManager: e.packageManager }, - ]); - } else { - noWorkspace.push({ project: e.project, packageManager: e.packageManager }); - } - }); + const projectEdits = edits.filter((e): e is EditPackageManagerSettingsInternal => !!e.project); + traceIgnoredGlobalManagerEdits('setPackageManager', edits.length - projectEdits.length); + projectEdits.forEach((e) => { + const w = workspaceApis.getWorkspaceFolder(e.project.uri); + if (w) { + workspaces.set(w, [...(workspaces.get(w) || []), { project: e.project, packageManager: e.packageManager }]); + } else { + noWorkspace.push({ project: e.project, packageManager: e.packageManager }); + } + }); noWorkspace.forEach((e) => { if (e.project) { diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index 20b1f317..40233435 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -3,6 +3,7 @@ import * as assert from 'assert'; import * as path from 'path'; import * as sinon from 'sinon'; import { ConfigurationTarget, Uri, WorkspaceFolder } from 'vscode'; +import * as logging from '../../../common/logging'; import * as workspaceApis from '../../../common/workspace.apis'; import { addPythonProjectSetting, @@ -22,12 +23,8 @@ function getTestWorkspacePath(): string { } /** - * These tests verify that settings ARE written when the value changes, - * regardless of whether it's the default/system manager or not. - * - * Note: These tests focus on the global settings path (project=undefined) because - * workspace-scoped tests would require mocking workspace.getWorkspaceFolder which - * cannot be easily stubbed in unit tests. + * These tests verify that manager edits without a project do not write settings + * and are logged explicitly as ignored global edits. */ suite('Setting Helpers - Settings Write Behavior', () => { const SYSTEM_MANAGER_ID = 'ms-python.python:system'; @@ -35,7 +32,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { const PIP_MANAGER_ID = 'ms-python.python:pip'; const CONDA_MANAGER_ID = 'ms-python.python:conda'; - let updateCalls: Array<{ key: string; value: unknown; target: ConfigurationTarget }>; + let updateCalls: Array<{ key: string; value: unknown; target: boolean | ConfigurationTarget | undefined }>; setup(() => { updateCalls = []; @@ -46,7 +43,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { }); /** - * Creates a mock WorkspaceConfiguration that tracks update calls + * Creates a mock WorkspaceConfiguration that tracks update calls. */ function createMockConfig(options: { defaultEnvManagerGlobalValue?: string; @@ -99,7 +96,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { updateCalls.push({ key: section, value, - target: configurationTarget as ConfigurationTarget, + target: configurationTarget, }); return Promise.resolve(); }; @@ -113,6 +110,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { currentEnvManager: VENV_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + const traceVerboseStub = sinon.stub(logging, 'traceVerbose'); await setAllManagerSettings([ { @@ -122,10 +120,12 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, + const envManagerUpdates = updateCalls.filter((c) => c.key === 'defaultEnvManager'); + assert.strictEqual(envManagerUpdates.length, 0, 'Should never write defaultEnvManager for global edits'); + sinon.assert.calledWithMatch( + traceVerboseStub, + '[setAllManagerSettings] Ignoring 1 edit(s) without a project because python-envs does not persist manager defaults to User/global settings.', ); - assert.strictEqual(envManagerUpdates.length, 0, 'Should never write to user/global settings'); }); test('should NOT write global defaultPackageManager even when value differs from current', async () => { @@ -143,10 +143,12 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, + const pkgManagerUpdates = updateCalls.filter((c) => c.key === 'defaultPackageManager'); + assert.strictEqual( + pkgManagerUpdates.length, + 0, + 'Should never write defaultPackageManager for global edits', ); - assert.strictEqual(pkgManagerUpdates.length, 0, 'Should never write to user/global settings'); }); }); @@ -156,6 +158,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { currentEnvManager: VENV_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + const traceVerboseStub = sinon.stub(logging, 'traceVerbose'); await setEnvironmentManager([ { @@ -164,10 +167,12 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const envManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultEnvManager' && c.target === ConfigurationTarget.Global, + const envManagerUpdates = updateCalls.filter((c) => c.key === 'defaultEnvManager'); + assert.strictEqual(envManagerUpdates.length, 0, 'Should never write defaultEnvManager for global edits'); + sinon.assert.calledWithMatch( + traceVerboseStub, + '[setEnvironmentManager] Ignoring 1 edit(s) without a project because python-envs does not persist manager defaults to User/global settings.', ); - assert.strictEqual(envManagerUpdates.length, 0, 'Should never write to user/global settings'); }); }); @@ -177,6 +182,7 @@ suite('Setting Helpers - Settings Write Behavior', () => { currentPkgManager: PIP_MANAGER_ID, }); sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + const traceVerboseStub = sinon.stub(logging, 'traceVerbose'); await setPackageManager([ { @@ -185,10 +191,16 @@ suite('Setting Helpers - Settings Write Behavior', () => { }, ]); - const pkgManagerUpdates = updateCalls.filter( - (c) => c.key === 'defaultPackageManager' && c.target === ConfigurationTarget.Global, + const pkgManagerUpdates = updateCalls.filter((c) => c.key === 'defaultPackageManager'); + assert.strictEqual( + pkgManagerUpdates.length, + 0, + 'Should never write defaultPackageManager for global edits', + ); + sinon.assert.calledWithMatch( + traceVerboseStub, + '[setPackageManager] Ignoring 1 edit(s) without a project because python-envs does not persist manager defaults to User/global settings.', ); - assert.strictEqual(pkgManagerUpdates.length, 0, 'Should never write to user/global settings'); }); }); }); @@ -210,7 +222,7 @@ suite('Setting Helpers - Empty Path Bug Fix', () => { index: 0, }; - let updateCalls: Array<{ key: string; value: unknown; target: ConfigurationTarget }>; + let updateCalls: Array<{ key: string; value: unknown; target: boolean | ConfigurationTarget | undefined }>; setup(() => { updateCalls = []; @@ -248,7 +260,7 @@ suite('Setting Helpers - Empty Path Bug Fix', () => { updateCalls.push({ key: section, value, - target: configurationTarget as ConfigurationTarget, + target: configurationTarget, }); return Promise.resolve(); };