Skip to content

Commit 42457bd

Browse files
deepak1556deepak1556
authored
fix: snap env variables leaking into terminal env (microsoft#185834)
* fix: snap env variables leaking into terminal env * chore: add spec
1 parent 982869f commit 42457bd

File tree

5 files changed

+200
-39
lines changed

5 files changed

+200
-39
lines changed

‎resources/linux/snap/electron-launch

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ function append_dir() {
4747
fi
4848
}
4949

50+
function copy_env_variable() {
51+
local -n var="$1"
52+
if [[ "+$var" ]]; then
53+
export "${!var}_VSCODE_SNAP_ORIG=${var}"
54+
else
55+
export "${!var}_VSCODE_SNAP_ORIG=''"
56+
fi
57+
}
58+
5059
# shellcheck source=/dev/null
5160
source "$SNAP_USER_DATA/.last_revision" 2>/dev/null || true
5261
if [ "$SNAP_DESKTOP_LAST_REVISION" = "$SNAP_VERSION" ]; then
@@ -84,6 +93,19 @@ function can_open_file() {
8493
[ -f "$1" ] && [ -r "$1" ]
8594
}
8695

96+
# Preserve system variables that get modified below
97+
copy_env_variable XDG_CONFIG_DIRS
98+
copy_env_variable XDG_DATA_DIRS
99+
copy_env_variable LOCPATH
100+
copy_env_variable GIO_MODULE_DIR
101+
copy_env_variable GSETTINGS_SCHEMA_DIR
102+
copy_env_variable GDK_PIXBUF_MODULE_FILE
103+
copy_env_variable GDK_PIXBUF_MODULEDIR
104+
copy_env_variable GDK_BACKEND
105+
copy_env_variable GTK_PATH
106+
copy_env_variable GTK_EXE_PREFIX
107+
copy_env_variable GTK_IM_MODULE_FILE
108+
87109
# XDG Config
88110
prepend_dir XDG_CONFIG_DIRS "$SNAP/etc/xdg"
89111

‎src/vs/platform/environment/electron-main/environmentMainService.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import { memoize } from 'vs/base/common/decorators';
77
import { join } from 'vs/base/common/path';
8+
import { isLinux } from 'vs/base/common/platform';
89
import { createStaticIPCHandle } from 'vs/base/parts/ipc/node/ipc.net';
910
import { IEnvironmentService, INativeEnvironmentService } from 'vs/platform/environment/common/environment';
1011
import { NativeEnvironmentService } from 'vs/platform/environment/node/environmentService';
@@ -34,10 +35,15 @@ export interface IEnvironmentMainService extends INativeEnvironmentService {
3435

3536
// --- config
3637
readonly disableUpdates: boolean;
38+
39+
unsetSnapExportedVariables(): void;
40+
restoreSnapExportedVariables(): void;
3741
}
3842

3943
export class EnvironmentMainService extends NativeEnvironmentService implements IEnvironmentMainService {
4044

45+
private _snapEnv: Record<string, string> = {};
46+
4147
@memoize
4248
get cachedLanguagesPath(): string { return join(this.userDataPath, 'clp'); }
4349

@@ -64,4 +70,39 @@ export class EnvironmentMainService extends NativeEnvironmentService implements
6470

6571
@memoize
6672
get useCodeCache(): boolean { return !!this.codeCachePath; }
73+
74+
unsetSnapExportedVariables() {
75+
if (!isLinux) {
76+
return;
77+
}
78+
for (const key in process.env) {
79+
if (key.endsWith('_VSCODE_SNAP_ORIG')) {
80+
const originalKey = key.slice(0, -17); // Remove the _VSCODE_SNAP_ORIG suffix
81+
if (this._snapEnv[originalKey]) {
82+
continue;
83+
}
84+
// Preserve the original value in case the snap env is re-entered
85+
if (process.env[originalKey]) {
86+
this._snapEnv[originalKey] = process.env[originalKey]!;
87+
}
88+
// Copy the original value from before entering the snap env if available,
89+
// if not delete the env variable.
90+
if (process.env[key]) {
91+
process.env[originalKey] = process.env[key];
92+
} else {
93+
delete process.env[originalKey];
94+
}
95+
}
96+
}
97+
}
98+
99+
restoreSnapExportedVariables() {
100+
if (!isLinux) {
101+
return;
102+
}
103+
for (const key in this._snapEnv) {
104+
process.env[key] = this._snapEnv[key];
105+
delete this._snapEnv[key];
106+
}
107+
}
67108
}

‎src/vs/platform/environment/test/electron-main/environmentMainService.test.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as assert from 'assert';
7+
import { EnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
8+
import product from 'vs/platform/product/common/product';
9+
import { isLinux } from 'vs/base/common/platform';
10+
11+
suite('EnvironmentMainService', () => {
12+
13+
test('can unset and restore snap env variables', () => {
14+
const service = new EnvironmentMainService({ '_': [] }, { '_serviceBrand': undefined, ...product });
15+
16+
process.env['TEST_ARG1_VSCODE_SNAP_ORIG'] = 'original';
17+
process.env['TEST_ARG1'] = 'modified';
18+
process.env['TEST_ARG2_SNAP'] = 'test_arg2';
19+
process.env['TEST_ARG3_VSCODE_SNAP_ORIG'] = '';
20+
process.env['TEST_ARG3'] = 'test_arg3_non_empty';
21+
22+
// Unset snap env variables
23+
service.unsetSnapExportedVariables();
24+
if (isLinux) {
25+
assert.strictEqual(process.env['TEST_ARG1'], 'original');
26+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
27+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
28+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
29+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
30+
assert.strictEqual(process.env['TEST_ARG3'], undefined);
31+
} else {
32+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
33+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
34+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
35+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
36+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
37+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
38+
}
39+
40+
// Restore snap env variables
41+
service.restoreSnapExportedVariables();
42+
if (isLinux) {
43+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
44+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
45+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
46+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
47+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
48+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
49+
} else {
50+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
51+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
52+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
53+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
54+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
55+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
56+
}
57+
});
58+
59+
test('can invoke unsetSnapExportedVariables and restoreSnapExportedVariables multiple times', () => {
60+
const service = new EnvironmentMainService({ '_': [] }, { '_serviceBrand': undefined, ...product });
61+
// Mock snap environment
62+
process.env['SNAP'] = '1';
63+
process.env['SNAP_REVISION'] = 'test_revision';
64+
65+
process.env['TEST_ARG1_VSCODE_SNAP_ORIG'] = 'original';
66+
process.env['TEST_ARG1'] = 'modified';
67+
process.env['TEST_ARG2_SNAP'] = 'test_arg2';
68+
process.env['TEST_ARG3_VSCODE_SNAP_ORIG'] = '';
69+
process.env['TEST_ARG3'] = 'test_arg3_non_empty';
70+
71+
// Unset snap env variables
72+
service.unsetSnapExportedVariables();
73+
service.unsetSnapExportedVariables();
74+
service.unsetSnapExportedVariables();
75+
if (isLinux) {
76+
assert.strictEqual(process.env['TEST_ARG1'], 'original');
77+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
78+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
79+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
80+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
81+
assert.strictEqual(process.env['TEST_ARG3'], undefined);
82+
} else {
83+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
84+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
85+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
86+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
87+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
88+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
89+
}
90+
91+
// Restore snap env variables
92+
service.restoreSnapExportedVariables();
93+
service.restoreSnapExportedVariables();
94+
if (isLinux) {
95+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
96+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
97+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
98+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
99+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
100+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
101+
} else {
102+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
103+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
104+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
105+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
106+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
107+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
108+
}
109+
110+
// Unset snap env variables
111+
service.unsetSnapExportedVariables();
112+
if (isLinux) {
113+
assert.strictEqual(process.env['TEST_ARG1'], 'original');
114+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
115+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
116+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
117+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
118+
assert.strictEqual(process.env['TEST_ARG3'], undefined);
119+
} else {
120+
assert.strictEqual(process.env['TEST_ARG1'], 'modified');
121+
assert.strictEqual(process.env['TEST_ARG2'], undefined);
122+
assert.strictEqual(process.env['TEST_ARG1_VSCODE_SNAP_ORIG'], 'original');
123+
assert.strictEqual(process.env['TEST_ARG2_SNAP'], 'test_arg2');
124+
assert.strictEqual(process.env['TEST_ARG3_VSCODE_SNAP_ORIG'], '');
125+
assert.strictEqual(process.env['TEST_ARG3'], 'test_arg3_non_empty');
126+
}
127+
});
128+
});

‎src/vs/platform/native/electron-main/nativeHostMainService.ts

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { Emitter, Event } from 'vs/base/common/event';
1212
import { Disposable } from 'vs/base/common/lifecycle';
1313
import { Schemas } from 'vs/base/common/network';
1414
import { dirname, join, resolve } from 'vs/base/common/path';
15-
import { isLinux, isLinuxSnap, isMacintosh, isWindows } from 'vs/base/common/platform';
15+
import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform';
1616
import { AddFirstParameterToFunctions } from 'vs/base/common/types';
1717
import { URI } from 'vs/base/common/uri';
1818
import { realpath } from 'vs/base/node/extpath';
@@ -433,43 +433,11 @@ export class NativeHostMainService extends Disposable implements INativeHostMain
433433
}
434434

435435
async openExternal(windowId: number | undefined, url: string): Promise<boolean> {
436-
if (isLinuxSnap) {
437-
this.safeSnapOpenExternal(url);
438-
} else {
439-
shell.openExternal(url);
440-
}
441-
442-
return true;
443-
}
444-
445-
private safeSnapOpenExternal(url: string): void {
446-
447-
// Remove some environment variables before opening to avoid issues...
448-
const gdkPixbufModuleFile = process.env['GDK_PIXBUF_MODULE_FILE'];
449-
const gdkPixbufModuleDir = process.env['GDK_PIXBUF_MODULEDIR'];
450-
const gtkIMModuleFile = process.env['GTK_IM_MODULE_FILE'];
451-
const gdkBackend = process.env['GDK_BACKEND'];
452-
const gioModuleDir = process.env['GIO_MODULE_DIR'];
453-
const gtkExePrefix = process.env['GTK_EXE_PREFIX'];
454-
const gsettingsSchemaDir = process.env['GSETTINGS_SCHEMA_DIR'];
455-
delete process.env['GDK_PIXBUF_MODULE_FILE'];
456-
delete process.env['GDK_PIXBUF_MODULEDIR'];
457-
delete process.env['GTK_IM_MODULE_FILE'];
458-
delete process.env['GDK_BACKEND'];
459-
delete process.env['GIO_MODULE_DIR'];
460-
delete process.env['GTK_EXE_PREFIX'];
461-
delete process.env['GSETTINGS_SCHEMA_DIR'];
462-
436+
this.environmentMainService.unsetSnapExportedVariables();
463437
shell.openExternal(url);
438+
this.environmentMainService.restoreSnapExportedVariables();
464439

465-
// ...but restore them after
466-
process.env['GDK_PIXBUF_MODULE_FILE'] = gdkPixbufModuleFile;
467-
process.env['GDK_PIXBUF_MODULEDIR'] = gdkPixbufModuleDir;
468-
process.env['GTK_IM_MODULE_FILE'] = gtkIMModuleFile;
469-
process.env['GDK_BACKEND'] = gdkBackend;
470-
process.env['GIO_MODULE_DIR'] = gioModuleDir;
471-
process.env['GTK_EXE_PREFIX'] = gtkExePrefix;
472-
process.env['GSETTINGS_SCHEMA_DIR'] = gsettingsSchemaDir;
440+
return true;
473441
}
474442

475443
moveItemToTrash(windowId: number | undefined, fullPath: string): Promise<void> {

‎src/vs/platform/terminal/electron-main/electronPtyHostStarter.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { IEnvironmentService, INativeEnvironmentService } from 'vs/platform/environment/common/environment';
6+
import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
77
import { parsePtyHostDebugPort } from 'vs/platform/environment/node/environmentService';
88
import { ILifecycleMainService } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
99
import { ILogService } from 'vs/platform/log/common/log';
@@ -31,7 +31,7 @@ export class ElectronPtyHostStarter implements IPtyHostStarter {
3131
constructor(
3232
private readonly _reconnectConstants: IReconnectConstants,
3333
@IConfigurationService private readonly _configurationService: IConfigurationService,
34-
@IEnvironmentService private readonly _environmentService: INativeEnvironmentService,
34+
@IEnvironmentMainService private readonly _environmentMainService: IEnvironmentMainService,
3535
@ILifecycleMainService private readonly _lifecycleMainService: ILifecycleMainService,
3636
@ILogService private readonly _logService: ILogService
3737
) {
@@ -43,7 +43,7 @@ export class ElectronPtyHostStarter implements IPtyHostStarter {
4343
start(lastPtyId: number): IPtyHostConnection {
4444
this.utilityProcess = new UtilityProcess(this._logService, NullTelemetryService, this._lifecycleMainService);
4545

46-
const inspectParams = parsePtyHostDebugPort(this._environmentService.args, this._environmentService.isBuilt);
46+
const inspectParams = parsePtyHostDebugPort(this._environmentMainService.args, this._environmentMainService.isBuilt);
4747
const execArgv = inspectParams.port ? [
4848
'--nolazy',
4949
`--inspect${inspectParams.break ? '-brk' : ''}=${inspectParams.port}`
@@ -75,6 +75,7 @@ export class ElectronPtyHostStarter implements IPtyHostStarter {
7575
}
7676

7777
private _createPtyHostConfiguration(lastPtyId: number) {
78+
this._environmentMainService.unsetSnapExportedVariables();
7879
const config: { [key: string]: string } = {
7980
...deepClone(process.env),
8081
VSCODE_LAST_PTY_ID: String(lastPtyId),
@@ -93,6 +94,7 @@ export class ElectronPtyHostStarter implements IPtyHostStarter {
9394
if (startupDelay && typeof startupDelay === 'number') {
9495
config.VSCODE_STARTUP_DELAY = String(startupDelay);
9596
}
97+
this._environmentMainService.restoreSnapExportedVariables();
9698
return config;
9799
}
98100

0 commit comments

Comments
 (0)