Skip to content

Commit

Permalink
Disable env activation in PowerShell using command prompt (#2892)
Browse files Browse the repository at this point in the history
* Disable powershell activation using command prompt
* Change to cmd prompt and fix code review comments
* Change news entry
* Fix tests
  • Loading branch information
DonJayamanne authored Oct 16, 2018
1 parent a339d2f commit 542ae64
Show file tree
Hide file tree
Showing 38 changed files with 692 additions and 189 deletions.
1 change: 1 addition & 0 deletions news/2 Fixes/2827.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Disable activation of conda environments in PowerShell.
79 changes: 79 additions & 0 deletions src/client/application/diagnostics/checks/powerShellActivation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable } from 'inversify';
import { DiagnosticSeverity } from 'vscode';
import '../../../common/extensions';
import { error } from '../../../common/logger';
import { useCommandPromptAsDefaultShell } from '../../../common/terminal/commandPrompt';
import { IConfigurationService, ICurrentProcess } from '../../../common/types';
import { IServiceContainer } from '../../../ioc/types';
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
import { IDiagnosticsCommandFactory } from '../commands/types';
import { DiagnosticCodes } from '../constants';
import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../promptHandler';
import { DiagnosticScope, IDiagnostic, IDiagnosticHandlerService } from '../types';

const PowershellActivationNotSupportedWithBatchFilesMessage = 'Activation of the selected Python environment is not supported in PowerShell. Consider changing your shell to Command Prompt.';

export class PowershellActivationNotAvailableDiagnostic extends BaseDiagnostic {
constructor() {
super(DiagnosticCodes.EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic,
PowershellActivationNotSupportedWithBatchFilesMessage,
DiagnosticSeverity.Warning, DiagnosticScope.Global);
}
}

export const PowerShellActivationHackDiagnosticsServiceId = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic';

@injectable()
export class PowerShellActivationHackDiagnosticsService extends BaseDiagnosticsService {
protected readonly messageService: IDiagnosticHandlerService<MessageCommandPrompt>;
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
super([DiagnosticCodes.EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic], serviceContainer);
this.messageService = serviceContainer.get<IDiagnosticHandlerService<MessageCommandPrompt>>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerServiceId);
}
public async diagnose(): Promise<IDiagnostic[]> {
return [];
}
public async handle(diagnostics: IDiagnostic[]): Promise<void> {
// This class can only handle one type of diagnostic, hence just use first item in list.
if (diagnostics.length === 0 || !this.canHandle(diagnostics[0])) {
return;
}
const diagnostic = diagnostics[0];
if (await this.filterService.shouldIgnoreDiagnostic(diagnostic.code)) {
return;
}
const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
const currentProcess = this.serviceContainer.get<ICurrentProcess>(ICurrentProcess);
const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
const options = [
{
prompt: 'Use Command Prompt',
// tslint:disable-next-line:no-object-literal-type-assertion
command: {
diagnostic, invoke: async (): Promise<void> => {
useCommandPromptAsDefaultShell(currentProcess, configurationService)
.catch(ex => error('Use Command Prompt as default shell', ex));
}
}
},
{
prompt: 'Ignore'
},
{
prompt: 'Always Ignore',
command: commandFactory.createCommand(diagnostic, { type: 'ignore', options: DiagnosticScope.Global })
},
{
prompt: 'More Info',
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://aka.ms/Niq35h' })
}
];

await this.messageService.handle(diagnostic, { commandPrompts: options });
}
}
3 changes: 2 additions & 1 deletion src/client/application/diagnostics/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ export enum DiagnosticCodes {
NoPythonInterpretersDiagnostic = 'NoPythonInterpretersDiagnostic',
MacInterpreterSelectedAndNoOtherInterpretersDiagnostic = 'MacInterpreterSelectedAndNoOtherInterpretersDiagnostic',
MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic = 'MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic',
InvalidPythonPathInDebuggerDiagnostic = 'InvalidPythonPathInDebuggerDiagnostic'
InvalidPythonPathInDebuggerDiagnostic = 'InvalidPythonPathInDebuggerDiagnostic',
EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic'
}
2 changes: 2 additions & 0 deletions src/client/application/diagnostics/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { IServiceManager } from '../../ioc/types';
import { EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId } from './checks/envPathVariable';
import { InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId } from './checks/invalidDebuggerType';
import { InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId } from './checks/invalidPythonPathInDebugger';
import { PowerShellActivationHackDiagnosticsService, PowerShellActivationHackDiagnosticsServiceId } from './checks/powerShellActivation';
import { InvalidPythonInterpreterService, InvalidPythonInterpreterServiceId } from './checks/pythonInterpreter';
import { DiagnosticsCommandFactory } from './commands/factory';
import { IDiagnosticsCommandFactory } from './commands/types';
Expand All @@ -21,5 +22,6 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidPythonInterpreterService, InvalidPythonInterpreterServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, PowerShellActivationHackDiagnosticsService, PowerShellActivationHackDiagnosticsServiceId);
serviceManager.addSingleton<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory, DiagnosticsCommandFactory);
}
25 changes: 17 additions & 8 deletions src/client/common/configuration/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ export class ConfigurationService implements IConfigurationService {
return PythonSettings.getInstance(resource);
}

public async updateSettingAsync(setting: string, value?: {}, resource?: Uri, configTarget?: ConfigurationTarget): Promise<void> {
const settingsInfo = PythonSettings.getSettingsUriAndTarget(resource);
public async updateSectionSetting(section: string, setting: string, value?: {}, resource?: Uri, configTarget?: ConfigurationTarget): Promise<void> {
const settingsInfo = section === 'python' ?
PythonSettings.getSettingsUriAndTarget(resource) :
{
uri: resource,
target: configTarget ? configTarget : ConfigurationTarget.WorkspaceFolder
};

const pythonConfig = workspace.getConfiguration('python', settingsInfo.uri);
const currentValue = pythonConfig.inspect(setting);
const configSection = workspace.getConfiguration(section, settingsInfo.uri);
const currentValue = configSection.inspect(setting);

if (currentValue !== undefined &&
((settingsInfo.target === ConfigurationTarget.Global && currentValue.globalValue === value) ||
Expand All @@ -25,19 +30,23 @@ export class ConfigurationService implements IConfigurationService {
return;
}

await pythonConfig.update(setting, value, settingsInfo.target);
await this.verifySetting(pythonConfig, settingsInfo.target, setting, value);
await configSection.update(setting, value, settingsInfo.target);
await this.verifySetting(configSection, settingsInfo.target, setting, value);
}

public async updateSetting(setting: string, value?: {}, resource?: Uri, configTarget?: ConfigurationTarget): Promise<void> {
return this.updateSectionSetting('python', setting, value, resource, configTarget);
}

public isTestExecution(): boolean {
return process.env.VSC_PYTHON_CI_TEST === '1';
}

private async verifySetting(pythonConfig: WorkspaceConfiguration, target: ConfigurationTarget, settingName: string, value?: {}): Promise<void> {
private async verifySetting(configSection: WorkspaceConfiguration, target: ConfigurationTarget, settingName: string, value?: {}): Promise<void> {
if (this.isTestExecution()) {
let retries = 0;
do {
const setting = pythonConfig.inspect(settingName);
const setting = configSection.inspect(settingName);
if (!setting && value === undefined) {
break; // Both are unset
}
Expand Down
2 changes: 1 addition & 1 deletion src/client/common/installer/productInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export class FormatterInstaller extends BaseInstaller {
const formatterName = ProductNames.get(formatter)!;

if (item.endsWith(formatterName)) {
await this.configService.updateSettingAsync('formatting.provider', formatterName, resource);
await this.configService.updateSetting('formatting.provider', formatterName, resource);
return this.install(formatter, resource);
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/client/common/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import { PersistentStateFactory } from './persistentState';
import { IS_64_BIT, IS_WINDOWS } from './platform/constants';
import { PathUtils } from './platform/pathUtils';
import { CurrentProcess } from './process/currentProcess';
import { TerminalActivator } from './terminal/activator';
import { PowershellTerminalActivationFailedHandler } from './terminal/activator/powershellFailedHandler';
import { Bash } from './terminal/environmentActivationProviders/bash';
import {
CommandPromptAndPowerShell
Expand All @@ -37,7 +39,7 @@ import { TerminalServiceFactory } from './terminal/factory';
import { TerminalHelper } from './terminal/helper';
import {
ITerminalActivationCommandProvider,
ITerminalHelper, ITerminalServiceFactory
ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, ITerminalServiceFactory
} from './terminal/types';
import {
IBrowserService, IConfigurationService,
Expand Down Expand Up @@ -71,6 +73,8 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<IHttpClient>(IHttpClient, HttpClient);
serviceManager.addSingleton<IEditorUtils>(IEditorUtils, EditorUtils);
serviceManager.addSingleton<INugetService>(INugetService, NugetService);
serviceManager.addSingleton<ITerminalActivator>(ITerminalActivator, TerminalActivator);
serviceManager.addSingleton<ITerminalActivationHandler>(ITerminalActivationHandler, PowershellTerminalActivationFailedHandler);

serviceManager.addSingleton<ITerminalHelper>(ITerminalHelper, TerminalHelper);
serviceManager.addSingleton<ITerminalActivationCommandProvider>(
Expand Down
38 changes: 38 additions & 0 deletions src/client/common/terminal/activator/base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { Terminal, Uri } from 'vscode';
import { sleep } from '../../utils/async';
import { ITerminalActivator, ITerminalHelper, TerminalShellType } from '../types';

export class BaseTerminalActivator implements ITerminalActivator {
private readonly activatedTerminals: Set<Terminal> = new Set<Terminal>();
constructor(private readonly helper: ITerminalHelper) { }
public async activateEnvironmentInTerminal(terminal: Terminal, resource: Uri | undefined, preserveFocus: boolean = true) {
if (this.activatedTerminals.has(terminal)) {
return false;
}
this.activatedTerminals.add(terminal);
const shellPath = this.helper.getTerminalShellPath();
const terminalShellType = !shellPath || shellPath.length === 0 ? TerminalShellType.other : this.helper.identifyTerminalShell(shellPath);

const activationCommamnds = await this.helper.getEnvironmentActivationCommands(terminalShellType, resource);
if (activationCommamnds) {
for (const command of activationCommamnds!) {
terminal.show(preserveFocus);
terminal.sendText(command);
await this.waitForCommandToProcess(terminalShellType);
}
return true;
} else {
return false;
}
}
protected async waitForCommandToProcess(shell: TerminalShellType) {
// Give the command some time to complete.
// Its been observed that sending commands too early will strip some text off in VS Code Terminal.
await sleep(500);
}
}
26 changes: 26 additions & 0 deletions src/client/common/terminal/activator/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable, multiInject } from 'inversify';
import { Terminal, Uri } from 'vscode';
import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper } from '../types';
import { BaseTerminalActivator } from './base';

@injectable()
export class TerminalActivator implements ITerminalActivator {
protected baseActivator!: ITerminalActivator;
constructor(@inject(ITerminalHelper) readonly helper: ITerminalHelper,
@multiInject(ITerminalActivationHandler) private readonly handlers: ITerminalActivationHandler[]) {
this.initialize();
}
public async activateEnvironmentInTerminal(terminal: Terminal, resource: Uri | undefined, preserveFocus: boolean = true) {
const activated = await this.baseActivator.activateEnvironmentInTerminal(terminal, resource, preserveFocus);
this.handlers.forEach(handler => handler.handleActivation(terminal, resource, preserveFocus, activated).ignoreErrors());
return activated;
}
protected initialize() {
this.baseActivator = new BaseTerminalActivator(this.helper);
}
}
35 changes: 35 additions & 0 deletions src/client/common/terminal/activator/powershellFailedHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable, named } from 'inversify';
import { Terminal, Uri } from 'vscode';
import { PowerShellActivationHackDiagnosticsServiceId, PowershellActivationNotAvailableDiagnostic } from '../../../application/diagnostics/checks/powerShellActivation';
import { IDiagnosticsService } from '../../../application/diagnostics/types';
import { IPlatformService } from '../../platform/types';
import { ITerminalActivationHandler, ITerminalHelper, TerminalShellType } from '../types';

@injectable()
export class PowershellTerminalActivationFailedHandler implements ITerminalActivationHandler {
constructor(@inject(ITerminalHelper) private readonly helper: ITerminalHelper,
@inject(IPlatformService) private readonly platformService: IPlatformService,
@inject(IDiagnosticsService) @named(PowerShellActivationHackDiagnosticsServiceId) private readonly diagnosticService: IDiagnosticsService) {
}
public async handleActivation(_terminal: Terminal, resource: Uri | undefined, _preserveFocus: boolean, activated: boolean) {
if (activated || !this.platformService.isWindows) {
return;
}
const shell = this.helper.identifyTerminalShell(this.helper.getTerminalShellPath());
if (shell !== TerminalShellType.powershell && shell !== TerminalShellType.powershellCore) {
return;
}
// Check if we can activate in Command Prompt.
const activationCommands = await this.helper.getEnvironmentActivationCommands(TerminalShellType.commandPrompt, resource);
if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) {
return;
}
this.diagnosticService.handle([new PowershellActivationNotAvailableDiagnostic()]).ignoreErrors();
}

}
23 changes: 23 additions & 0 deletions src/client/common/terminal/commandPrompt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import * as path from 'path';
import { ConfigurationTarget } from 'vscode';
import { IConfigurationService, ICurrentProcess } from '../types';

export function getCommandPromptLocation(currentProcess: ICurrentProcess) {
// https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts#L218
// Determine the correct System32 path. We want to point to Sysnative
// when the 32-bit version of VS Code is running on a 64-bit machine.
// The reason for this is because PowerShell's important PSReadline
// module doesn't work if this is not the case. See #27915.
const is32ProcessOn64Windows = currentProcess.env.hasOwnProperty('PROCESSOR_ARCHITEW6432');
const system32Path = path.join(currentProcess.env.windir!, is32ProcessOn64Windows ? 'Sysnative' : 'System32');
return path.join(system32Path, 'cmd.exe');
}
export async function useCommandPromptAsDefaultShell(currentProcess: ICurrentProcess, configService: IConfigurationService) {
const cmdPromptLocation = getCommandPromptLocation(currentProcess);
await configService.updateSectionSetting('terminal', 'integrated.shell.windows', cmdPromptLocation, undefined, ConfigurationTarget.Global);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { inject, injectable } from 'inversify';
import * as path from 'path';
import { IServiceContainer } from '../../../ioc/types';
import '../../extensions';
import { IPlatformService } from '../../platform/types';
import { TerminalShellType } from '../types';
import { BaseActivationCommandProvider } from './baseActivationProvider';

Expand Down Expand Up @@ -34,18 +33,7 @@ export class CommandPromptAndPowerShell extends BaseActivationCommandProvider {
// lets not try to run the powershell file from command prompt (user may not have powershell)
return [];
} else {
// This means we're in powershell and we have a .bat file.
if (this.serviceContainer.get<IPlatformService>(IPlatformService).isWindows) {
// On windows, the solution is to go into cmd, then run the batch (.bat) file and go back into powershell.
const powershellExe = targetShell === TerminalShellType.powershell ? 'powershell' : 'pwsh';
const activationCmd = scriptFile.fileToCommandArgument();
return [
`& cmd /k "${activationCmd.replace(/"/g, '""')} & ${powershellExe}"`
];
} else {
// Powershell on non-windows os, we cannot execute the batch file.
return;
}
return;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { injectable } from 'inversify';
import * as path from 'path';
import { Uri } from 'vscode';
Expand Down Expand Up @@ -105,19 +107,7 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
envName: string,
targetShell: TerminalShellType
): Promise<string[] | undefined> {
// https://github.com/conda/conda/issues/626
// On windows, the solution is to go into cmd, then run the batch (.bat) file and go back into powershell.
const powershellExe = targetShell === TerminalShellType.powershell ? 'powershell' : 'pwsh';
const activateCmd = await this.getWindowsActivateCommand();

let cmdStyleCmd = `${activateCmd} ${envName.toCommandArgument()}`;
// we need to double-quote any cmd quotes as we will wrap them
// in another layer of quotes for powershell:
cmdStyleCmd = cmdStyleCmd.replace(/"/g, '""');

return [
`& cmd /k "${cmdStyleCmd} & ${powershellExe}"`
];
return;
}

public async getFishCommands(
Expand Down
Loading

0 comments on commit 542ae64

Please sign in to comment.