diff --git a/src/vs/workbench/api/common/extHostExtensionService.ts b/src/vs/workbench/api/common/extHostExtensionService.ts index 563438a2d93d8..b37423c2afd68 100644 --- a/src/vs/workbench/api/common/extHostExtensionService.ts +++ b/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 { + 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 { // Make sure the extension's surface is not undefined extensionModule = extensionModule || { diff --git a/src/vs/workbench/api/node/extHostExtensionService.ts b/src/vs/workbench/api/node/extHostExtensionService.ts index ab221e8aed7c7..a669575081b4e 100644 --- a/src/vs/workbench/api/node/extHostExtensionService.ts +++ b/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 = await import(module.toString(true)); } else { - r = require(module.fsPath); + r = runWithFencedProcessEnvironment(() => 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 { + return runWithFencedProcessEnvironment(() => super._callActivate(logService, extensionId, extensionModule, context, extensionInternalStore, activationTimesBuilder)); + } + protected async _loadCommonJSModule(extension: IExtensionDescription | null, module: URI, activationTimesBuilder: ExtensionActivationTimesBuilder): Promise { return this._doLoadModule(extension, module, activationTimesBuilder, 'cjs'); } diff --git a/src/vs/workbench/api/node/extHostProcessEnvironment.ts b/src/vs/workbench/api/node/extHostProcessEnvironment.ts new file mode 100644 index 0000000000000..67fcf3eb3b753 --- /dev/null +++ b/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(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); + } +} diff --git a/src/vs/workbench/api/test/node/extHostProcessEnvironment.test.ts b/src/vs/workbench/api/test/node/extHostProcessEnvironment.test.ts new file mode 100644 index 0000000000000..7361bb7e3414f --- /dev/null +++ b/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); + }); +});