Fence process.env mutations during extension startup

src/vs/workbench/api/common/extHostExtensionService.ts

@@ -515,7 +515,7 @@ export abstract class AbstractExtHostExtensionService extends Disposable impleme
 			this._loadExtensionContext(extensionDescription, extensionInternalStore)
 		]).then(values => {
 			performance.mark(`code/extHost/willActivateExtension/${extensionDescription.identifier.value}`);
-			return AbstractExtHostExtensionService._callActivate(this._logService, extensionDescription.identifier, values[0], values[1], extensionInternalStore, activationTimesBuilder);
+			return this._callActivate(this._logService, extensionDescription.identifier, values[0], values[1], extensionInternalStore, activationTimesBuilder);
 		}).then((activatedExtension) => {
 			performance.mark(`code/extHost/didActivateExtension/${extensionDescription.identifier.value}`);
 			return activatedExtension;
@@ -596,6 +596,10 @@ export abstract class AbstractExtHostExtensionService extends Disposable impleme
 		});
 	}
 
+	protected _callActivate(logService: ILogService, extensionId: ExtensionIdentifier, extensionModule: IExtensionModule, context: vscode.ExtensionContext, extensionInternalStore: IDisposable, activationTimesBuilder: ExtensionActivationTimesBuilder): Promise<ActivatedExtension> {
+		return AbstractExtHostExtensionService._callActivate(logService, extensionId, extensionModule, context, extensionInternalStore, activationTimesBuilder);
+	}
+
 	private static _callActivate(logService: ILogService, extensionId: ExtensionIdentifier, extensionModule: IExtensionModule, context: vscode.ExtensionContext, extensionInternalStore: IDisposable, activationTimesBuilder: ExtensionActivationTimesBuilder): Promise<ActivatedExtension> {
 		// Make sure the extension's surface is not undefined
 		extensionModule = extensionModule || {

src/vs/workbench/api/node/extHostExtensionService.ts

@@ -7,13 +7,13 @@ import * as performance from '../../../base/common/performance.js';
 import type * as vscode from 'vscode';
 import { createApiFactoryAndRegisterActors } from '../common/extHost.api.impl.js';
 import { INodeModuleFactory, RequireInterceptor } from '../common/extHostRequireInterceptor.js';
-import { ExtensionActivationTimesBuilder } from '../common/extHostExtensionActivator.js';
+import { ActivatedExtension, ExtensionActivationTimesBuilder, IExtensionModule } from '../common/extHostExtensionActivator.js';
 import { connectProxyResolver } from './proxyResolver.js';
 import { AbstractExtHostExtensionService } from '../common/extHostExtensionService.js';
 import { ExtHostDownloadService } from './extHostDownloadService.js';
 import { URI } from '../../../base/common/uri.js';
 import { Schemas } from '../../../base/common/network.js';
-import { IExtensionDescription } from '../../../platform/extensions/common/extensions.js';
+import { ExtensionIdentifier, IExtensionDescription } from '../../../platform/extensions/common/extensions.js';
 import { ExtensionRuntime } from '../common/extHostTypes.js';
 import { CLIServer } from './extHostCLIServer.js';
 import { realpathSync } from '../../../base/node/pfs.js';
@@ -23,7 +23,9 @@ import nodeModule from 'node:module';
 import { assertType } from '../../../base/common/types.js';
 import { generateUuid } from '../../../base/common/uuid.js';
 import { BidirectionalMap } from '../../../base/common/map.js';
-import { DisposableStore, toDisposable } from '../../../base/common/lifecycle.js';
+import { DisposableStore, IDisposable, toDisposable } from '../../../base/common/lifecycle.js';
+import { runWithFencedProcessEnvironment } from './extHostProcessEnvironment.js';
+import { ILogService } from '../../../platform/log/common/log.js';
 const require = nodeModule.createRequire(import.meta.url);
 
 class NodeModuleRequireInterceptor extends RequireInterceptor {
@@ -204,7 +206,7 @@ export class ExtHostExtensionService extends AbstractExtHostExtensionService {
 			if (mode === 'esm') {
 				r = <T>await import(module.toString(true));
 			} else {
-				r = <T>require(module.fsPath);
+				r = runWithFencedProcessEnvironment(() => <T>require(module.fsPath));
 			}
 		} finally {
 			if (extensionId) {
@@ -215,6 +217,10 @@ export class ExtHostExtensionService extends AbstractExtHostExtensionService {
 		return r;
 	}
 
+	protected override _callActivate(logService: ILogService, extensionId: ExtensionIdentifier, extensionModule: IExtensionModule, context: vscode.ExtensionContext, extensionInternalStore: IDisposable, activationTimesBuilder: ExtensionActivationTimesBuilder): Promise<ActivatedExtension> {
+		return runWithFencedProcessEnvironment(() => super._callActivate(logService, extensionId, extensionModule, context, extensionInternalStore, activationTimesBuilder));
+	}
+
 	protected async _loadCommonJSModule<T>(extension: IExtensionDescription | null, module: URI, activationTimesBuilder: ExtensionActivationTimesBuilder): Promise<T> {
 		return this._doLoadModule<T>(extension, module, activationTimesBuilder, 'cjs');
 	}

src/vs/workbench/api/node/extHostProcessEnvironment.ts

@@ -0,0 +1,19 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+/** Runs a callback without allowing synchronous environment mutations to escape. */
+export function runWithFencedProcessEnvironment<T>(callback: () => T): T {
+	const environment = { ...process.env };
+	try {
+		return callback();
+	} finally {
+		for (const key of Object.keys(process.env)) {
+			if (!Object.hasOwn(environment, key)) {
+				delete process.env[key];
+			}
+		}
+		Object.assign(process.env, environment);
+	}
+}

src/vs/workbench/api/test/node/extHostProcessEnvironment.test.ts

@@ -0,0 +1,59 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
+import { runWithFencedProcessEnvironment } from '../../node/extHostProcessEnvironment.js';
+
+suite('ExtHostProcessEnvironment', () => {
+	ensureNoDisposablesAreLeakedInTestSuite();
+
+	const addedKey = 'VSCODE_TEST_FENCED_ENV_ADDED';
+	const changedKey = 'VSCODE_TEST_FENCED_ENV_CHANGED';
+	const deletedKey = 'VSCODE_TEST_FENCED_ENV_DELETED';
+
+	setup(() => {
+		delete process.env[addedKey];
+		process.env[changedKey] = 'before';
+		process.env[deletedKey] = 'before';
+	});
+
+	teardown(() => {
+		delete process.env[addedKey];
+		delete process.env[changedKey];
+		delete process.env[deletedKey];
+	});
+
+	test('restores variables changed by a callback', () => {
+		const result = runWithFencedProcessEnvironment(() => {
+			process.env[addedKey] = 'inside';
+			process.env[changedKey] = 'inside';
+			delete process.env[deletedKey];
+			return 'result';
+		});
+
+		assert.deepStrictEqual({
+			result,
+			added: process.env[addedKey],
+			changed: process.env[changedKey],
+			deleted: process.env[deletedKey]
+		}, {
+			result: 'result',
+			added: undefined,
+			changed: 'before',
+			deleted: 'before'
+		});
+	});
+
+	test('restores variables when a callback throws', () => {
+		const error = new Error('expected');
+
+		assert.throws(() => runWithFencedProcessEnvironment(() => {
+			process.env[addedKey] = 'inside';
+			throw error;
+		}), candidate => candidate === error);
+		assert.strictEqual(process.env[addedKey], undefined);
+	});
+});