Linux Snap: Prevent GDK_PIXBUF env from leaking out (#109608)

* update hygiene file

* snap: do not let GDK_PIXBUF leak out

fixes #100940

* 💄

build/hygiene.js

@@ -53,6 +53,7 @@ const indentationFilter = [
 	'!src/vs/base/node/terminateProcess.sh',
 	'!src/vs/base/node/cpuUsage.sh',
 	'!test/unit/assert.js',
+	'!resources/linux/snap/electron-launch',
 
 	// except specific folders
 	'!test/automation/out/**',
@@ -115,7 +116,6 @@ const copyrightFilter = [
 	'!**/*.js.map',
 	'!build/**/*.init',
 	'!resources/linux/snap/snapcraft.yaml',
-	'!resources/linux/snap/electron-launch',
 	'!resources/win32/bin/code.js',
 	'!resources/web/code-web.js',
 	'!resources/completions/**',

src/main.js

@@ -141,12 +141,6 @@ registerListeners();
 // Cached data
 const nodeCachedDataDir = getNodeCachedDir();
 
-// Remove env set by snap https://github.com/microsoft/vscode/issues/85344
-if (process.env['SNAP']) {
-	delete process.env['GDK_PIXBUF_MODULE_FILE'];
-	delete process.env['GDK_PIXBUF_MODULEDIR'];
-}
-
 /**
  * Support user defined locale: load it early before app('ready')
  * to have more things running in parallel.

src/vs/base/common/processes.ts

@@ -110,7 +110,8 @@ export function sanitizeProcessEnvironment(env: IProcessEnvironment, ...preserve
 		/^ELECTRON_.+$/,
 		/^GOOGLE_API_KEY$/,
 		/^VSCODE_.+$/,
-		/^SNAP(|_.*)$/
+		/^SNAP(|_.*)$/,
+		/^GDK_PIXBUF_.+$/,
 	];
 	const envKeys = Object.keys(env);
 	envKeys

src/vs/base/test/common/processes.test.ts

@@ -24,7 +24,9 @@ suite('Processes', () => {
 			VSCODE_PORTABLE: 'x',
 			VSCODE_PID: 'x',
 			VSCODE_NODE_CACHED_DATA_DIR: 'x',
-			VSCODE_NEW_VAR: 'x'
+			VSCODE_NEW_VAR: 'x',
+			GDK_PIXBUF_MODULE_FILE: 'x',
+			GDK_PIXBUF_MODULEDIR: 'x',
 		};
 		processes.sanitizeProcessEnvironment(env);
 		assert.equal(env['FOO'], 'bar');

src/vs/code/electron-main/app.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { app, ipcMain as ipc, systemPreferences, shell, contentTracing, protocol, IpcMainEvent, BrowserWindow, dialog, session } from 'electron';
+import { app, ipcMain as ipc, systemPreferences, contentTracing, protocol, IpcMainEvent, BrowserWindow, dialog, session } from 'electron';
 import { IProcessEnvironment, isWindows, isMacintosh } from 'vs/base/common/platform';
 import { WindowsMainService } from 'vs/platform/windows/electron-main/windowsMainService';
 import { IWindowOpenable } from 'vs/platform/windows/common/windows';
@@ -86,6 +86,7 @@ import { ActiveWindowManager } from 'vs/platform/windows/common/windowTracker';
 export class CodeApplication extends Disposable {
 	private windowsMainService: IWindowsMainService | undefined;
 	private dialogMainService: IDialogMainService | undefined;
+	private nativeHostMainService: INativeHostMainService | undefined;
 
 	constructor(
 		private readonly mainIpcServer: Server,
@@ -213,7 +214,9 @@ export class CodeApplication extends Disposable {
 			contents.on('new-window', (event, url) => {
 				event.preventDefault(); // prevent code that wants to open links
 
-				shell.openExternal(url);
+				if (this.nativeHostMainService) {
+					this.nativeHostMainService.openExternal(undefined, url);
+				}
 			});
 
 			session.defaultSession.setPermissionRequestHandler((webContents, permission /* 'media' | 'geolocation' | 'notifications' | 'midiSysex' | 'pointerLock' | 'fullscreen' | 'openExternal' */, callback) => {
@@ -542,8 +545,8 @@ export class CodeApplication extends Disposable {
 		const encryptionChannel = createChannelReceiver(encryptionMainService);
 		electronIpcServer.registerChannel('encryption', encryptionChannel);
 
-		const nativeHostMainService = accessor.get(INativeHostMainService);
-		const nativeHostChannel = createChannelReceiver(nativeHostMainService);
+		const nativeHostMainService = this.nativeHostMainService = accessor.get(INativeHostMainService);
+		const nativeHostChannel = createChannelReceiver(this.nativeHostMainService);
 		electronIpcServer.registerChannel('nativeHost', nativeHostChannel);
 		sharedProcessClient.then(client => client.registerChannel('nativeHost', nativeHostChannel));
 

src/vs/platform/issue/electron-main/issueMainService.ts

@@ -8,7 +8,7 @@ import * as os from 'os';
 import product from 'vs/platform/product/common/product';
 import { parseArgs, OPTIONS } from 'vs/platform/environment/node/argv';
 import { ICommonIssueService, IssueReporterData, IssueReporterFeatures, ProcessExplorerData } from 'vs/platform/issue/common/issue';
-import { BrowserWindow, ipcMain, screen, IpcMainEvent, Display, shell } from 'electron';
+import { BrowserWindow, ipcMain, screen, IpcMainEvent, Display } from 'electron';
 import { ILaunchMainService } from 'vs/platform/launch/electron-main/launchMainService';
 import { PerformanceInfo, isRemoteDiagnosticError } from 'vs/platform/diagnostics/common/diagnostics';
 import { IDiagnosticsService } from 'vs/platform/diagnostics/node/diagnosticsService';
@@ -21,6 +21,7 @@ import { IDialogMainService } from 'vs/platform/dialogs/electron-main/dialogs';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { zoomLevelToZoomFactor } from 'vs/platform/windows/common/windows';
 import { FileAccess } from 'vs/base/common/network';
+import { INativeHostMainService } from 'vs/platform/native/electron-main/nativeHostMainService';
 
 const DEFAULT_BACKGROUND_COLOR = '#1E1E1E';
 
@@ -42,7 +43,8 @@ export class IssueMainService implements ICommonIssueService {
 		@ILaunchMainService private readonly launchMainService: ILaunchMainService,
 		@ILogService private readonly logService: ILogService,
 		@IDiagnosticsService private readonly diagnosticsService: IDiagnosticsService,
-		@IDialogMainService private readonly dialogMainService: IDialogMainService
+		@IDialogMainService private readonly dialogMainService: IDialogMainService,
+		@INativeHostMainService private readonly nativeHostMainService: INativeHostMainService
 	) {
 		this.registerListeners();
 	}
@@ -155,7 +157,7 @@ export class IssueMainService implements ICommonIssueService {
 		});
 
 		ipcMain.on('vscode:openExternal', (_: unknown, arg: string) => {
-			shell.openExternal(arg);
+			this.nativeHostMainService.openExternal(undefined, arg);
 		});
 
 		ipcMain.on('vscode:closeIssueReporter', (event: IpcMainEvent) => {

src/vs/platform/menubar/electron-main/menubar.ts

@@ -6,7 +6,7 @@
 import * as nls from 'vs/nls';
 import { isMacintosh, language } from 'vs/base/common/platform';
 import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
-import { app, shell, Menu, MenuItem, BrowserWindow, MenuItemConstructorOptions, WebContents, Event, KeyboardEvent } from 'electron';
+import { app, Menu, MenuItem, BrowserWindow, MenuItemConstructorOptions, WebContents, Event, KeyboardEvent } from 'electron';
 import { getTitleBarStyle, INativeRunActionInWindowRequest, INativeRunKeybindingInWindowRequest, IWindowOpenable } from 'vs/platform/windows/common/windows';
 import { OpenContext } from 'vs/platform/windows/node/window';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@@ -813,7 +813,7 @@ export class Menubar {
 	}
 
 	private openUrl(url: string, id: string): void {
-		shell.openExternal(url);
+		this.nativeHostMainService.openExternal(undefined, url);
 		this.reportMenuActionTelemetry(id);
 	}
 

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

@@ -334,11 +334,27 @@ export class NativeHostMainService extends Disposable implements INativeHostMain
 	}
 
 	async openExternal(windowId: number | undefined, url: string): Promise<boolean> {
-		shell.openExternal(url);
+		if (isLinux && process.env.SNAP && process.env.SNAP_REVISION) {
+			NativeHostMainService._safeSnapOpenExternal(url);
+		} else {
+			shell.openExternal(url);
+		}
 
 		return true;
 	}
 
+	private static _safeSnapOpenExternal(url: string): void {
+		const gdkPixbufModuleFile = process.env['GDK_PIXBUF_MODULE_FILE'];
+		const gdkPixbufModuleDir = process.env['GDK_PIXBUF_MODULEDIR'];
+		delete process.env['GDK_PIXBUF_MODULE_FILE'];
+		delete process.env['GDK_PIXBUF_MODULEDIR'];
+
+		shell.openExternal(url);
+
+		process.env['GDK_PIXBUF_MODULE_FILE'] = gdkPixbufModuleFile;
+		process.env['GDK_PIXBUF_MODULEDIR'] = gdkPixbufModuleDir;
+	}
+
 	async moveItemToTrash(windowId: number | undefined, fullPath: string): Promise<boolean> {
 		return shell.moveItemToTrash(fullPath);
 	}

src/vs/platform/update/electron-main/updateService.linux.ts

@@ -12,8 +12,8 @@ import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/e
 import { ILogService } from 'vs/platform/log/common/log';
 import { createUpdateURL, AbstractUpdateService, UpdateNotAvailableClassification } from 'vs/platform/update/electron-main/abstractUpdateService';
 import { IRequestService, asJson } from 'vs/platform/request/common/request';
-import { shell } from 'electron';
 import { CancellationToken } from 'vs/base/common/cancellation';
+import { INativeHostMainService } from 'vs/platform/native/electron-main/nativeHostMainService';
 
 export class LinuxUpdateService extends AbstractUpdateService {
 
@@ -25,7 +25,8 @@ export class LinuxUpdateService extends AbstractUpdateService {
 		@ITelemetryService private readonly telemetryService: ITelemetryService,
 		@IEnvironmentMainService environmentService: IEnvironmentMainService,
 		@IRequestService requestService: IRequestService,
-		@ILogService logService: ILogService
+		@ILogService logService: ILogService,
+		@INativeHostMainService private readonly nativeHostMainService: INativeHostMainService
 	) {
 		super(lifecycleMainService, configurationService, environmentService, requestService, logService);
 	}
@@ -64,9 +65,9 @@ export class LinuxUpdateService extends AbstractUpdateService {
 		// Use the download URL if available as we don't currently detect the package type that was
 		// installed and the website download page is more useful than the tarball generally.
 		if (product.downloadUrl && product.downloadUrl.length > 0) {
-			shell.openExternal(product.downloadUrl);
+			this.nativeHostMainService.openExternal(undefined, product.downloadUrl);
 		} else if (state.update.url) {
-			shell.openExternal(state.update.url);
+			this.nativeHostMainService.openExternal(undefined, state.update.url);
 		}
 
 		this.setState(State.Idle(UpdateType.Archive));

src/vs/platform/update/electron-main/updateService.win32.ts

@@ -19,11 +19,11 @@ import { IRequestService, asJson } from 'vs/platform/request/common/request';
 import { checksum } from 'vs/base/node/crypto';
 import { tmpdir } from 'os';
 import { spawn } from 'child_process';
-import { shell } from 'electron';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { timeout } from 'vs/base/common/async';
 import { IFileService } from 'vs/platform/files/common/files';
 import { URI } from 'vs/base/common/uri';
+import { INativeHostMainService } from 'vs/platform/native/electron-main/nativeHostMainService';
 
 async function pollUntil(fn: () => boolean, millis = 1000): Promise<void> {
 	while (!fn()) {
@@ -66,7 +66,8 @@ export class Win32UpdateService extends AbstractUpdateService {
 		@IEnvironmentMainService environmentService: IEnvironmentMainService,
 		@IRequestService requestService: IRequestService,
 		@ILogService logService: ILogService,
-		@IFileService private readonly fileService: IFileService
+		@IFileService private readonly fileService: IFileService,
+		@INativeHostMainService private readonly nativeHostMainService: INativeHostMainService
 	) {
 		super(lifecycleMainService, configurationService, environmentService, requestService, logService);
 	}
@@ -177,7 +178,7 @@ export class Win32UpdateService extends AbstractUpdateService {
 
 	protected async doDownloadUpdate(state: AvailableForDownload): Promise<void> {
 		if (state.update.url) {
-			shell.openExternal(state.update.url);
+			this.nativeHostMainService.openExternal(undefined, state.update.url);
 		}
 		this.setState(State.Idle(getUpdateType()));
 	}