Empty environment variables passed to vscode are unset

[hyangah]

This is a continuation of #174330 which is now locked and doesn't accept comments.
The original report was filed by our Go extension users who observed empty env vars were
not reflected when they run/debug tests using the extension's feature.
The issue was closed with #175682, which fixed env handling in the term env.

However, the Go extension features (run/debug tests) actually use extension host's process.env directly. So, the term environment fix doesn't help the original issue users are facing. (golang/vscode-go#2745)

I prepared a repro in microsoft/vscode-extension-samples@main...hyangah:vscode-extension-samples:main#diff-665767c0f91ad3e1e028cc5dd145783e8ecaef232aefdb222ba9590b6bd52ba1

Either use the launch config in the commit, or

1. cd getting-started-sample
2. vsce package
3. code-insiders --install-extension=getting-started-sample-0.0.1.vsix
4. EMPTY_VAR="" NONEMPTY_VAR="nonempty" code-insiders
5. Select "Getting Started Sample: Say Hello" from command palette. Check the console log, using "Developer: Toggle Developer Tools".

code-insiders (Version: 1.79.0-insider (Universal) Commit: 9084e08) gives me

You can see that EMPTY_VAR is undefined.

Is there a recommended way for extensions to get the process environment other than process.env?
Does the node debugger avoid this issue in #174330 (comment) because the debugger/debugee is launched from a terminal?

[fedorareis]

Out of curiosity, are there any estimates on when this might be worked on? I would really like to move to a VSCode version newer than 1.73.1, but I can't do that while this issue is still outstanding. I'd be happy to try to look into the issue to see if I can fix it if that would be helpful. I'm not completely sure where process.env is being overwritten though since I did some testing and both node by itself and standard electron treat EMPTY_VAR="" as an empty string and not as undefined, so something somewhere in vscode is overwriting that.

[Tyriar]

@fedorareis it's currently on the backlog which means it doesn't have any estimated time. If you want to give it a shot here is where I'd start looking into it:

Environment code in CLI:

vscode/src/vs/code/node/cli.ts

Lines 182 to 185 in 2d9cc42

	const env: IProcessEnvironment = {
	...process.env,
	'ELECTRON_NO_ATTACH_CONSOLE': '1'
	};

Where we resolve "development environment" on Linux and macOS when launched via the dock:

vscode/src/vs/platform/shell/node/shellEnv.ts

Line 123 in 2d9cc42

return new Promise<typeof process.env>((resolve, reject) => {

Starting the extension host:

vscode/src/vs/platform/extensions/electron-main/extensionHostStarter.ts

Lines 89 to 98 in 2d9cc42

	this._getExtHost(id).start({
	...opts,
	type: 'extensionHost',
	entryPoint: 'vs/workbench/api/node/extensionHostProcess',
	args: ['--skipWorkspaceStorageLock'],
	execArgv: opts.execArgv,
	allowLoadingUnsignedLibraries: true,
	forceAllocationsToV8Sandbox: true,
	correlationId: id
	});

Note also that changing process.env isn't considered public API which means it can have problems. There is this issue which is related to setting the environment on the extension host via EnvironmentVariableCollection: #152806

[NoelTautges]

I think this bug is in Electron or Chromium.

The release notes of 1.74 say that progress was made toward sandboxing VS Code, including the introduction of a new UtilityProcess API to support these changes. Since 1.73.1 is the last version where the bug doesn't exist, I think it was introduced by this change.

To confirm that the utility process fork is the thing removing the environment variable, we can log process.env at the place where the Extension Host is spawned and the entry point of the Extension Host itself:

1. Add "__A_EMPTY_VAR": "" and "__A_NONEMPTY_VAR": "" to the environment of the Launch VS Code Internal task:

vscode/.vscode/launch.json

Lines 234 to 237 in 3cd6f48

	"env": {
	"VSCODE_EXTHOST_WILL_SEND_SOCKET": null,
	"VSCODE_SKIP_PRELAUNCH": "1"
	},

2. Set a conditional breakpoint at line 237 with condition configuration.env to check the environment variables when the Extension Host gets spawned:

vscode/src/vs/platform/utilityProcess/electron-main/utilityProcess.ts

Lines 236 to 244 in 3cd6f48

	// Fork utility process
	this.process = utilityProcess.fork(modulePath, args, {
	serviceName,
	env,
	execArgv,
	allowLoadingUnsignedLibraries,
	forceAllocationsToV8Sandbox,
	stdio
	} as ForkOptions & { forceAllocationsToV8Sandbox?: Boolean });

3. Add a console.log(process.env); somewhere at the top of bootstrap-fork.js to log the environment variables at the entry point of the Extension Host:

vscode/src/bootstrap-fork.js

Lines 1 to 9 in 3cd6f48

	/*---------------------------------------------------------------------------------------------
	* Copyright (c) Microsoft Corporation. All rights reserved.
	* Licensed under the MIT License. See License.txt in the project root for license information.
	*--------------------------------------------------------------------------------------------*/
	//@ts-check
	'use strict';
	const performance = require('./vs/base/common/performance');

4. Launch the VS Code task.

5. At the breakpoint, check the env variable. Confirm that __A_EMPTY_VAR and __A_NONEMPTY_VAR are both present with the values we put in.

6. Continue.

7. Switch to the Debug Console and the "Attach to Extension Host" task. Expand the Object that got logged and confirm that __A_EMPTY_VAR has disappeared, but __A_NONEMPTY_VAR is still there.

[NoelTautges]

Hey folks, I found it, it's in Chromium.

I found a clue by accident:

// Setting values in EnvironmentMap to an empty-string will make
// sure that they get unset from the environment via AlterEnvironment().

Which led me to the cause of it all:

// Now append all modified and new values.
for (const auto& i : changes) {
  if (!i.second.empty()) {
    result_indices.push_back(value_storage.size());
    value_storage.append(i.first);
    value_storage.push_back('=');
    value_storage.append(i.second);
    value_storage.push_back(0);
  }
}

Chromium's Services API takes environment map entries with a blank value to mean that it shouldn't set the variable in a new process. It doesn't do it when you inherit the parent environment and don't pass any environment variables in because there's no need to alter the environment variables it already has.

If you build Electron with this code in that for loop before the if statement, you can confirm that it isn't setting the variable when it launches the utility process:

if (i.first == "EMPTY_VAR") {
  LOG(INFO) << "It's unsetting the empty variable";
}

Dobby has found the root cause of the bug. Dobby is free.

(This is a lie. Dobby must file an issue with Chromium... some other time.)

[Tyriar]

@deepak1556 this might be because of Chromium/Electron swallowing empty variables?

[deepak1556]

@NoelTautges findings are on point, thanks! but this is not a bug in chromium, the launch API was not intended for external use cases and their behavior cannot be changed in upstream. However we can tweak the behavior for utility process in Electron.

[Zhong-z]

+1, just to add an additional datapoint, encountered the same behaviour with vscode-python extension as well. see microsoft/vscode-python#22337