From 8370952a005efb8f4e16da6d6c30cab0d6c77d3e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 12 Oct 2022 09:56:02 +1100 Subject: [PATCH] Display error message when running kernels from untrusted locations (#11624) * Display error message when running kernels from untrusted locations * Update change log and version --- CHANGELOG.md | 22 ++++++ build/azure-pipeline.stable.yml | 4 +- package-lock.json | 4 +- package.json | 2 +- package.nls.json | 3 +- .../debugger/jupyter/debuggingManager.ts | 14 +++- src/kernels/errors/kernelErrorHandler.ts | 11 ++- .../errors/kernelSpecNotTrustedError.ts | 27 +++++++ src/kernels/hiddenKernelNotification.node.ts | 58 -------------- src/kernels/raw/finder/jupyterPaths.node.ts | 4 +- .../raw/finder/localKernelFinder.node.ts | 10 +-- .../finder/localKernelSpecFinderBase.node.ts | 7 +- .../localKnownPathKernelSpecFinder.node.ts | 6 +- ...ndRelatedNonPythonKernelSpecFinder.node.ts | 6 +- ...ths.node.ts => trustedKernelPaths.node.ts} | 0 .../raw/finder/trustedKernelPaths.web.ts | 15 ++++ src/kernels/serviceRegistry.node.ts | 7 +- src/kernels/serviceRegistry.web.ts | 3 + src/notebooks/controllers/kernelConnector.ts | 15 ++++ .../controllers/vscodeNotebookController.ts | 6 +- src/notebooks/debugger/debuggingManager.ts | 14 +++- .../debugger/debuggingManagerBase.ts | 30 +++++--- src/platform/common/utils/localize.ts | 12 ++- src/platform/errors/errorUtils.ts | 2 +- src/platform/errors/errorUtils.unit.test.ts | 43 +++++++++++ .../localKernelFinder.unit.test.ts | 12 +-- .../raw/finder/jupyterPaths.node.unit.test.ts | 2 +- .../finder/trustedKernelPaths.unit.test.ts | 13 +++- .../finder/trustedKernrelPaths.unit.test.ts | 77 +++++++++++++++++++ .../webview-side/error-renderer/index.ts | 23 ++++-- 30 files changed, 311 insertions(+), 141 deletions(-) create mode 100644 src/kernels/errors/kernelSpecNotTrustedError.ts delete mode 100644 src/kernels/hiddenKernelNotification.node.ts rename src/kernels/raw/finder/{trustedKernelSpecPaths.node.ts => trustedKernelPaths.node.ts} (100%) create mode 100644 src/kernels/raw/finder/trustedKernelPaths.web.ts create mode 100644 src/platform/errors/errorUtils.unit.test.ts create mode 100644 src/test/kernels/raw/finder/trustedKernrelPaths.unit.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 757fb8d3700..56e8f23954d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## 2022.9.120 (11 October 2022) +### Enhancements +1. Display an error message (with instructions to resolve the issue) in the cell output when attempting to run a cell against a kernel from an untrusted location. + ([#11622](https://github.com/Microsoft/vscode-jupyter/issues/11622)) + +### Thanks + +Thanks to the following projects which we fully rely on to provide some of +our features: + +- [Python Extension](https://marketplace.visualstudio.com/items?itemName=ms-python.python) +- [debugpy](https://pypi.org/project/debugpy/) + +Also thanks to the various projects we provide integrations with which help +make this extension useful: + +- [Jupyter](https://jupyter.org/): + [Notebooks](https://jupyter-notebook.readthedocs.io/en/latest/?badge=latest), + [JupyterHub](https://jupyterhub.readthedocs.io/en/stable/), + [ipywidgets](https://ipywidgets.readthedocs.io/en/latest/), + [nbconvert](https://nbconvert.readthedocs.io/en/latest/) + ## 2022.9.110 (11 October 2022) ### Fixes 1. Fixed vulnerability described in [CVE-2022-41083](https://msrc.microsoft.com/update-guide/vulnerability/CVE-2022-41083) diff --git a/build/azure-pipeline.stable.yml b/build/azure-pipeline.stable.yml index f6ecb12377d..53a16010131 100644 --- a/build/azure-pipeline.stable.yml +++ b/build/azure-pipeline.stable.yml @@ -39,8 +39,8 @@ extends: - script: python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r ./requirements.txt displayName: Install Python libs - # - script: npm run updateBuildNumber - # displayName: Update build number + - script: npm run updateBuildNumber + displayName: Update build number - script: npm run prePublishBundleStable displayName: Build diff --git a/package-lock.json b/package-lock.json index fe5ed481cd0..4caaf5855ef 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "jupyter", - "version": "2022.9.1100000000", + "version": "2022.9.120", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "jupyter", - "version": "2022.9.1100000000", + "version": "2022.9.120", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 0aa09390680..098eb7127b1 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "jupyter", "displayName": "Jupyter", - "version": "2022.9.1100000000", + "version": "2022.9.120", "description": "Jupyter notebook support, interactive programming and computing that supports Intellisense, debugging and more.", "publisher": "ms-toolsai", "author": { diff --git a/package.nls.json b/package.nls.json index c0da70f97e8..af6462ba204 100644 --- a/package.nls.json +++ b/package.nls.json @@ -844,8 +844,7 @@ "message": "Failed to interrupt the Kernel.", "comment": ["{Locked='Kernel'}"] }, - "DataScience.updateSettingToTrustKernelSpecs": "Update setting to trust kernels", - "DataScience.untrustedKernelSpecsHidden": "Kernels found in an insecure location have not been loaded.", + "DataScience.failedToStartAnUntrustedKernelSpec": "The kernel '{0}' was not started as it is located in an insecure location '{1}'. \nClick [here](https://aka.ms/JupyterTrustedKernelPaths) for further details, optionally update the setting [jupyter.kernels.trusted](command:workbench.action.openSettings?[\"jupyter.kernels.trusted\"]) to trust the kernel.", "jupyter.configuration.jupyter.kernels.trusted.markdownDescription": "Enter fully qualified paths to Kernel specification files that are to be trusted. E.g. 'C:\\Program Data\\Jupyter\\kernels\\python3\\kernel.json'. \n**Note**: Kernels can execute code with user privileges. Click [here](https://aka.ms/JupyterTrustedKernelPaths) for further details.", "DataScience.kernelDied": { "message": "The kernel died. View Jupyter [log](command:jupyter.viewOutput) for further details. \nError: {0}...", diff --git a/src/interactive-window/debugger/jupyter/debuggingManager.ts b/src/interactive-window/debugger/jupyter/debuggingManager.ts index 58a4d1eb554..b30ca7a08f9 100644 --- a/src/interactive-window/debugger/jupyter/debuggingManager.ts +++ b/src/interactive-window/debugger/jupyter/debuggingManager.ts @@ -41,6 +41,7 @@ import { buildSourceMap } from '../helper'; import { noop } from '../../../platform/common/utils/misc'; import { IInteractiveWindowDebuggingManager } from '../../types'; import { IControllerLoader, IControllerSelection } from '../../../notebooks/controllers/types'; +import { IServiceContainer } from '../../../platform/ioc/types'; /** * The DebuggingManager maintains the mapping between notebook documents and debug sessions. @@ -61,9 +62,18 @@ export class InteractiveWindowDebuggingManager @inject(IDebugLocationTrackerFactory) private readonly debugLocationTrackerFactory: IDebugLocationTrackerFactory, @inject(IConfigurationService) private readonly configService: IConfigurationService, - @inject(IDebugService) private readonly debugService: IDebugService + @inject(IDebugService) private readonly debugService: IDebugService, + @inject(IServiceContainer) serviceContainer: IServiceContainer ) { - super(kernelProvider, controllerLoader, controllerSelection, commandManager, appShell, vscNotebook); + super( + kernelProvider, + controllerLoader, + controllerSelection, + commandManager, + appShell, + vscNotebook, + serviceContainer + ); } public override async activate(): Promise { diff --git a/src/kernels/errors/kernelErrorHandler.ts b/src/kernels/errors/kernelErrorHandler.ts index ef7cf6f919f..15cd50f0b12 100644 --- a/src/kernels/errors/kernelErrorHandler.ts +++ b/src/kernels/errors/kernelErrorHandler.ts @@ -453,12 +453,21 @@ function getUserFriendlyErrorMessage(error: Error | string, errorContext?: Kerne return getCombinedErrorMessage(errorPrefix, errorMessage); } } +function doesErrorHaveMarkdownLinks(message: string) { + const markdownLinks = new RegExp(/\[([^\[]+)\]\((.*)\)/); + return (markdownLinks.exec(message)?.length ?? 0) > 0; +} function getCombinedErrorMessage(prefix?: string, message?: string) { const errorMessage = [prefix || '', message || ''] .map((line) => line.trim()) .filter((line) => line.length > 0) .join(' \n'); - if (errorMessage.length && errorMessage.indexOf('command:jupyter.viewOutput') === -1) { + + if ( + !doesErrorHaveMarkdownLinks(errorMessage) && + errorMessage.length && + errorMessage.indexOf('command:jupyter.viewOutput') === -1 + ) { return `${ errorMessage.endsWith('.') ? errorMessage : errorMessage + '.' } \n${DataScience.viewJupyterLogForFurtherInfo()}`; diff --git a/src/kernels/errors/kernelSpecNotTrustedError.ts b/src/kernels/errors/kernelSpecNotTrustedError.ts new file mode 100644 index 00000000000..af0aea662dd --- /dev/null +++ b/src/kernels/errors/kernelSpecNotTrustedError.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Uri } from 'vscode'; +import { getDisplayPath } from '../../platform/common/platform/fs-paths'; +import { DataScience } from '../../platform/common/utils/localize'; +import { getDisplayNameOrNameOfKernelConnection } from '../helpers'; +import { LocalKernelConnectionMetadata } from '../types'; +import { WrappedKernelError } from './types'; + +/** + * Thrown when we attempt to start a kernel that is not trusted. + */ +export class KernelSpecNotTrustedError extends WrappedKernelError { + constructor(kernelConnectionMetadata: LocalKernelConnectionMetadata) { + super( + DataScience.failedToStartAnUntrustedKernelSpec().format( + getDisplayNameOrNameOfKernelConnection(kernelConnectionMetadata), + kernelConnectionMetadata.kernelSpec.specFile + ? getDisplayPath(Uri.file(kernelConnectionMetadata.kernelSpec.specFile)) + : '' + ), + undefined, + kernelConnectionMetadata + ); + } +} diff --git a/src/kernels/hiddenKernelNotification.node.ts b/src/kernels/hiddenKernelNotification.node.ts deleted file mode 100644 index 1940870b78c..00000000000 --- a/src/kernels/hiddenKernelNotification.node.ts +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -'use strict'; -import { inject, injectable, named } from 'inversify'; -import { commands, Memento } from 'vscode'; -import { IExtensionSyncActivationService } from '../platform/activation/types'; -import { IApplicationShell } from '../platform/common/application/types'; -import { GLOBAL_MEMENTO, IBrowserService, IMemento } from '../platform/common/types'; -import { Common, DataScience } from '../platform/common/utils/localize'; -import { noop } from '../platform/common/utils/misc'; -import { TrustedKernelPaths } from './raw/finder/trustedKernelSpecPaths.node'; - -const MEMENTO_KEY_NOTIFIED_ABOUT_HIDDEN_KERNEL = 'MEMENTO_KEY_NOTIFIED_ABOUT_HIDDEN_KERNEL_1'; -@injectable() -export class HiddenKernelNotification implements IExtensionSyncActivationService { - private notifiedAboutHiddenKernel?: boolean; - constructor( - @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento, - @inject(IApplicationShell) private readonly appShell: IApplicationShell, - @inject(IBrowserService) private readonly browser: IBrowserService - ) {} - - public activate(): void { - TrustedKernelPaths.IsKernelSpecHidden.promise - .then((hidden) => { - if ( - !hidden || - this.notifiedAboutHiddenKernel || - this.globalMemento.get(MEMENTO_KEY_NOTIFIED_ABOUT_HIDDEN_KERNEL, false) - ) { - return; - } - this.notifiedAboutHiddenKernel = true; - this.globalMemento.update(MEMENTO_KEY_NOTIFIED_ABOUT_HIDDEN_KERNEL, true).then(noop, noop); - this.appShell - .showWarningMessage( - DataScience.untrustedKernelSpecsHidden(), - Common.learnMore(), - DataScience.updateSettingToTrustKernelSpecs() - ) - .then((selection) => { - switch (selection) { - case Common.learnMore(): - this.browser.launch('https://aka.ms/JupyterTrustedKernelPaths'); - break; - case DataScience.updateSettingToTrustKernelSpecs(): - commands - .executeCommand('workbench.action.openSettings', 'jupyter.kernels.trusted') - .then(noop, noop); - break; - } - }) - .then(noop, noop); - }) - .catch(noop); - } -} diff --git a/src/kernels/raw/finder/jupyterPaths.node.ts b/src/kernels/raw/finder/jupyterPaths.node.ts index 89541003d20..65d766f6687 100644 --- a/src/kernels/raw/finder/jupyterPaths.node.ts +++ b/src/kernels/raw/finder/jupyterPaths.node.ts @@ -303,8 +303,8 @@ export class JupyterPaths { paths.add(winPath); } - if (process.env.ALLUSERSPROFILE) { - paths.add(Uri.file(path.join(process.env.ALLUSERSPROFILE, 'jupyter', 'kernels'))); + if (process.env.PROGRAMDATA) { + paths.add(Uri.file(path.join(process.env.PROGRAMDATA, 'jupyter', 'kernels'))); } } else { // Unix based diff --git a/src/kernels/raw/finder/localKernelFinder.node.ts b/src/kernels/raw/finder/localKernelFinder.node.ts index c95b6432526..60a91467ddf 100644 --- a/src/kernels/raw/finder/localKernelFinder.node.ts +++ b/src/kernels/raw/finder/localKernelFinder.node.ts @@ -27,7 +27,6 @@ import { debounceAsync } from '../../../platform/common/utils/decorators'; import { IPythonExtensionChecker } from '../../../platform/api/types'; import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { EnvironmentType } from '../../../platform/pythonEnvironments/info'; -import { ITrustedKernelPaths } from './types'; // This class searches for local kernels. // First it searches on a global persistent state, then on the installed python interpreters, @@ -64,8 +63,7 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSingleAc @inject(IInterpreterService) private readonly interpreters: IInterpreterService, @inject(CondaService) private readonly condaService: CondaService, @inject(IExtensions) private readonly extensions: IExtensions, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - @inject(ITrustedKernelPaths) private readonly trustedKernelSpecPaths: ITrustedKernelPaths + @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService ) { this._initializedPromise = new Promise((resolve) => { this._initializeResolve = resolve; @@ -341,12 +339,6 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSingleAc return this.fs.exists(kernel.interpreter.uri); case 'startUsingLocalKernelSpec': - if ( - kernel.kernelSpec.specFile && - !this.trustedKernelSpecPaths.isTrusted(Uri.file(kernel.kernelSpec.specFile)) - ) { - return false; - } // Spec files have to still exist and interpreters have to exist const promiseSpec = kernel.kernelSpec.specFile ? this.fs.exists(Uri.file(kernel.kernelSpec.specFile)) diff --git a/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts b/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts index a1af54e5ac7..fcf21c69eff 100644 --- a/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts +++ b/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts @@ -24,7 +24,6 @@ import { } from '../../../kernels/types'; import { JupyterKernelSpec } from '../../jupyter/jupyterKernelSpec'; import { getComparisonKey } from '../../../platform/vscode-path/resources'; -import { ITrustedKernelPaths } from './types'; // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires const flatten = require('lodash/flatten') as typeof import('lodash/flatten'); @@ -64,8 +63,7 @@ export abstract class LocalKernelSpecFinderBase { protected readonly fs: IFileSystemNode, protected readonly workspaceService: IWorkspaceService, protected readonly extensionChecker: IPythonExtensionChecker, - protected readonly globalState: Memento, - private readonly trustedKernelSpecPaths: ITrustedKernelPaths + protected readonly globalState: Memento ) {} @testOnlyMethod() @@ -139,9 +137,6 @@ export abstract class LocalKernelSpecFinderBase { globalSpecRootPath?: Uri, cancelToken?: CancellationToken ): Promise { - if (!this.trustedKernelSpecPaths.isTrusted(specPath)) { - return; - } // This is a backup folder for old kernels created by us. if (specPath.fsPath.includes(oldKernelsSpecFolderName)) { return; diff --git a/src/kernels/raw/finder/localKnownPathKernelSpecFinder.node.ts b/src/kernels/raw/finder/localKnownPathKernelSpecFinder.node.ts index 4736637b9af..4b640ed848d 100644 --- a/src/kernels/raw/finder/localKnownPathKernelSpecFinder.node.ts +++ b/src/kernels/raw/finder/localKnownPathKernelSpecFinder.node.ts @@ -21,7 +21,6 @@ import { IFileSystemNode } from '../../../platform/common/platform/types.node'; import { IMemento, GLOBAL_MEMENTO } from '../../../platform/common/types'; import { capturePerfTelemetry, Telemetry } from '../../../telemetry'; import { sendKernelSpecTelemetry } from './helper'; -import { ITrustedKernelPaths } from './types'; /** * This class searches for kernels on the file system in well known paths documented by Jupyter. @@ -35,10 +34,9 @@ export class LocalKnownPathKernelSpecFinder extends LocalKernelSpecFinderBase { @inject(IWorkspaceService) workspaceService: IWorkspaceService, @inject(JupyterPaths) private readonly jupyterPaths: JupyterPaths, @inject(IPythonExtensionChecker) extensionChecker: IPythonExtensionChecker, - @inject(IMemento) @named(GLOBAL_MEMENTO) memento: Memento, - @inject(ITrustedKernelPaths) trustedKernelSpecPaths: ITrustedKernelPaths + @inject(IMemento) @named(GLOBAL_MEMENTO) memento: Memento ) { - super(fs, workspaceService, extensionChecker, memento, trustedKernelSpecPaths); + super(fs, workspaceService, extensionChecker, memento); if (this.oldKernelSpecsFolder) { traceInfo( `Old kernelSpecs (created by Jupyter Extension) stored in directory ${this.oldKernelSpecsFolder}` diff --git a/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts b/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts index bee3977e337..397af2c3f78 100644 --- a/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts +++ b/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts @@ -33,7 +33,6 @@ import { areInterpreterPathsSame } from '../../../platform/pythonEnvironments/in import { capturePerfTelemetry, Telemetry } from '../../../telemetry'; import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; import { ResourceSet } from '../../../platform/vscode-path/map'; -import { ITrustedKernelPaths } from './types'; /** * Returns all Python kernels and any related kernels registered in the python environment. @@ -53,10 +52,9 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS @inject(IPythonExtensionChecker) extensionChecker: IPythonExtensionChecker, @inject(LocalKnownPathKernelSpecFinder) private readonly kernelSpecsFromKnownLocations: LocalKnownPathKernelSpecFinder, - @inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento, - @inject(ITrustedKernelPaths) trustedKernelSpecPaths: ITrustedKernelPaths + @inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento ) { - super(fs, workspaceService, extensionChecker, globalState, trustedKernelSpecPaths); + super(fs, workspaceService, extensionChecker, globalState); } public async listKernelSpecs(resource: Resource, ignoreCache?: boolean, cancelToken?: CancellationToken) { // Get an id for the workspace folder, if we don't have one, use the fsPath of the resource diff --git a/src/kernels/raw/finder/trustedKernelSpecPaths.node.ts b/src/kernels/raw/finder/trustedKernelPaths.node.ts similarity index 100% rename from src/kernels/raw/finder/trustedKernelSpecPaths.node.ts rename to src/kernels/raw/finder/trustedKernelPaths.node.ts diff --git a/src/kernels/raw/finder/trustedKernelPaths.web.ts b/src/kernels/raw/finder/trustedKernelPaths.web.ts new file mode 100644 index 00000000000..bfb79f4c88f --- /dev/null +++ b/src/kernels/raw/finder/trustedKernelPaths.web.ts @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +'use strict'; + +import { injectable } from 'inversify'; +import { Uri } from 'vscode'; +import { ITrustedKernelPaths } from './types'; + +@injectable() +export class TrustedKernelPaths implements ITrustedKernelPaths { + public isTrusted(_kernelPath: Uri): boolean { + return true; + } +} diff --git a/src/kernels/serviceRegistry.node.ts b/src/kernels/serviceRegistry.node.ts index 8ed1c194bba..06c5ae77499 100644 --- a/src/kernels/serviceRegistry.node.ts +++ b/src/kernels/serviceRegistry.node.ts @@ -45,9 +45,8 @@ import { KernelAutoReconnectMonitor } from './kernelAutoReConnectMonitor'; import { PythonKernelInterruptDaemon } from './raw/finder/pythonKernelInterruptDaemon.node'; import { LocalKernelFinder } from './raw/finder/localKernelFinder.node'; import { DebugStartupCodeProvider } from './debuggerStartupCodeProvider'; -import { TrustedKernelPaths } from './raw/finder/trustedKernelSpecPaths.node'; +import { TrustedKernelPaths } from './raw/finder/trustedKernelPaths.node'; import { ITrustedKernelPaths } from './raw/finder/types'; -import { HiddenKernelNotification } from './hiddenKernelNotification.node'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { serviceManager.addSingleton(IExtensionSingleActivationService, Activation); @@ -56,10 +55,6 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea IExtensionSyncActivationService, PortAttributesProviders ); - serviceManager.addSingleton( - IExtensionSyncActivationService, - HiddenKernelNotification - ); serviceManager.addSingleton( IRawNotebookSupportedService, RawNotebookSupportedService diff --git a/src/kernels/serviceRegistry.web.ts b/src/kernels/serviceRegistry.web.ts index 5c8736f4096..177de2b1169 100644 --- a/src/kernels/serviceRegistry.web.ts +++ b/src/kernels/serviceRegistry.web.ts @@ -24,6 +24,8 @@ import { CellOutputDisplayIdTracker } from './execution/cellDisplayIdTracker'; import { KernelAutoReConnectFailedMonitor } from './kernelAutoReConnectFailedMonitor'; import { KernelAutoReconnectMonitor } from './kernelAutoReConnectMonitor'; import { DebugStartupCodeProvider } from './debuggerStartupCodeProvider'; +import { TrustedKernelPaths } from './raw/finder/trustedKernelPaths.web'; +import { ITrustedKernelPaths } from './raw/finder/types'; @injectable() class RawNotebookSupportedService implements IRawNotebookSupportedService { @@ -63,6 +65,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea KernelAutoReconnectMonitor ); serviceManager.addSingleton(IKernelProvider, KernelProvider); + serviceManager.addSingleton(ITrustedKernelPaths, TrustedKernelPaths); serviceManager.addSingleton(IThirdPartyKernelProvider, ThirdPartyKernelProvider); serviceManager.addSingleton( PreferredRemoteKernelIdProvider, diff --git a/src/notebooks/controllers/kernelConnector.ts b/src/notebooks/controllers/kernelConnector.ts index 2cfab29bb56..01934cbc6f4 100644 --- a/src/notebooks/controllers/kernelConnector.ts +++ b/src/notebooks/controllers/kernelConnector.ts @@ -36,6 +36,8 @@ import { IRawNotebookProvider } from '../../kernels/raw/types'; import { IControllerSelection, IVSCodeNotebookController } from './types'; import { getDisplayNameOrNameOfKernelConnection } from '../../kernels/helpers'; import { isCancellationError } from '../../platform/common/cancellation'; +import { ITrustedKernelPaths } from '../../kernels/raw/finder/types'; +import { KernelSpecNotTrustedError } from '../../kernels/errors/kernelSpecNotTrustedError'; /** * Class used for connecting a controller to an instance of an IKernel @@ -378,6 +380,16 @@ export class KernelConnector { } } + private static async canStartKernel(metadata: KernelConnectionMetadata, serviceContainer: IServiceContainer) { + if (!isLocalConnection(metadata) || !metadata.kernelSpec.specFile) { + return; + } + const trustedKernelPaths = serviceContainer.get(ITrustedKernelPaths); + if (!trustedKernelPaths.isTrusted(Uri.file(metadata.kernelSpec.specFile))) { + throw new KernelSpecNotTrustedError(metadata); + } + } + private static async wrapKernelMethodImpl( metadata: KernelConnectionMetadata, initialContext: KernelAction, @@ -396,6 +408,9 @@ export class KernelConnector { let currentMethod = KernelConnector.convertContextToFunction(initialContext, options); let currentContext = initialContext; let controller = 'controller' in notebookResource ? notebookResource.controller : undefined; + if (initialContext === 'start') { + await KernelConnector.canStartKernel(metadata, serviceContainer); + } while (kernel === undefined) { // Try to create the kernel (possibly again) kernel = diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index 74a1f476219..122746a5de3 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -82,6 +82,7 @@ import { KernelMessage } from '@jupyterlab/services'; import { initializeInteractiveOrNotebookTelemetryBasedOnUserAction } from '../../kernels/telemetry/helper'; import { NotebookCellLanguageService } from '../languages/cellLanguageService'; import { IDataScienceErrorHandler } from '../../kernels/errors/types'; +import { ITrustedKernelPaths } from '../../kernels/raw/finder/types'; /** * Our implementation of the VSCode Notebook Controller. Called by VS code to execute cells in a notebook. Also displayed @@ -634,9 +635,12 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont return; } // Auto start the local kernels. + const trustedKernelPaths = this.serviceContainer.get(ITrustedKernelPaths); if ( !this.configuration.getSettings(undefined).disableJupyterAutoStart && - isLocalConnection(this.kernelConnection) + isLocalConnection(this.kernelConnection) && + this.kernelConnection.kernelSpec.specFile && + trustedKernelPaths.isTrusted(Uri.file(this.kernelConnection.kernelSpec.specFile)) ) { // Startup could fail due to missing dependencies or the like. this.connectToKernel(document, new DisplayOptions(true)).catch(noop); diff --git a/src/notebooks/debugger/debuggingManager.ts b/src/notebooks/debugger/debuggingManager.ts index 01a419e7c35..26c3373155e 100644 --- a/src/notebooks/debugger/debuggingManager.ts +++ b/src/notebooks/debugger/debuggingManager.ts @@ -29,6 +29,7 @@ import { IPlatformService } from '../../platform/common/platform/types'; import { IConfigurationService } from '../../platform/common/types'; import { DataScience } from '../../platform/common/utils/localize'; import { noop } from '../../platform/common/utils/misc'; +import { IServiceContainer } from '../../platform/ioc/types'; import { traceError, traceInfo, traceInfoIfCI } from '../../platform/logging'; import { ResourceSet } from '../../platform/vscode-path/map'; import * as path from '../../platform/vscode-path/path'; @@ -65,9 +66,18 @@ export class DebuggingManager @inject(IVSCodeNotebook) vscNotebook: IVSCodeNotebook, @inject(IConfigurationService) private readonly settings: IConfigurationService, @inject(IPlatformService) private readonly platform: IPlatformService, - @inject(IDebugService) private readonly debugService: IDebugService + @inject(IDebugService) private readonly debugService: IDebugService, + @inject(IServiceContainer) serviceContainer: IServiceContainer ) { - super(kernelProvider, controllerLoader, controllerSelection, commandManager, appShell, vscNotebook); + super( + kernelProvider, + controllerLoader, + controllerSelection, + commandManager, + appShell, + vscNotebook, + serviceContainer + ); this.runByLineCells = new ContextKey(EditorContexts.RunByLineCells, commandManager); this.runByLineDocuments = new ContextKey(EditorContexts.RunByLineDocuments, commandManager); this.debugDocuments = new ContextKey(EditorContexts.DebugDocuments, commandManager); diff --git a/src/notebooks/debugger/debuggingManagerBase.ts b/src/notebooks/debugger/debuggingManagerBase.ts index b710933f887..8dac8b6708e 100644 --- a/src/notebooks/debugger/debuggingManagerBase.ts +++ b/src/notebooks/debugger/debuggingManagerBase.ts @@ -27,6 +27,9 @@ import { KernelDebugAdapterBase } from './kernelDebugAdapterBase'; import { IpykernelCheckResult, isUsingIpykernel6OrLater } from './helper'; import { noop } from '../../platform/common/utils/misc'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; +import { KernelConnector } from '../controllers/kernelConnector'; +import { IServiceContainer } from '../../platform/ioc/types'; +import { DisplayOptions } from '../../kernels/displayOptions'; /** * The DebuggingManager maintains the mapping between notebook documents and debug sessions. @@ -44,7 +47,8 @@ export abstract class DebuggingManagerBase implements IDisposable { private readonly notebookControllerSelection: IControllerSelection, protected readonly commandManager: ICommandManager, protected readonly appShell: IApplicationShell, - protected readonly vscNotebook: IVSCodeNotebook + protected readonly vscNotebook: IVSCodeNotebook, + protected readonly serviceContainer: IServiceContainer ) {} public async activate() { @@ -145,19 +149,21 @@ export abstract class DebuggingManagerBase implements IDisposable { protected async ensureKernelIsRunning(doc: NotebookDocument): Promise { await this.notebookControllerLoader.loaded; const controller = this.notebookControllerSelection.getSelected(doc); - let kernel = this.kernelProvider.get(doc); - if (!kernel && controller) { - kernel = this.kernelProvider.getOrCreate(doc, { - metadata: controller.connection, - controller: controller?.controller, - resourceUri: doc.uri - }); - } - if (kernel && kernel.status === 'unknown') { - await kernel.start(); + if (controller && (!kernel || (kernel && kernel.status === 'unknown'))) { + kernel = await KernelConnector.connectToNotebookKernel( + controller.connection, + this.serviceContainer, + { + notebook: doc, + controller: controller.controller, + resource: doc.uri + }, + new DisplayOptions(false), + this.disposables, + 'jupyterExtension' + ); } - return kernel; } diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts index 1fe88ca7081..af74a4dae4c 100644 --- a/src/platform/common/utils/localize.ts +++ b/src/platform/common/utils/localize.ts @@ -537,13 +537,6 @@ export namespace DataScience { }, 'The kernel died. Error: {0}... View Jupyter [log](command:jupyter.viewOutput) for further details.' ); - export const untrustedKernelSpecsHidden = () => - localize( - 'DataScience.untrustedKernelSpecsHidden', - 'Kernels found in an insecure location have not been loaded.' - ); - export const updateSettingToTrustKernelSpecs = () => - localize('DataScience.updateSettingToTrustKernelSpecs', 'Update setting to trust kernels'); export const kernelDiedWithoutError = () => localize( { @@ -552,6 +545,11 @@ export namespace DataScience { }, "The kernel '{0}' died. Click [here](https://aka.ms/vscodeJupyterKernelCrash) for more info. View Jupyter [log](command:jupyter.viewOutput) for further details." ); + export const failedToStartAnUntrustedKernelSpec = () => + localize( + 'DataScience.failedToStartAnUntrustedKernelSpec', + "The kernel '{0}' was not started as it is located in an insecure location '{1}'. \nClick [here](https://aka.ms/JupyterTrustedKernelPaths) for further details, optionally update the setting [jupyter.kernels.trusted](command:workbench.action.openSettings?[\"jupyter.kernels.trusted\"]) to trust the kernel." + ); export const kernelDiedWithoutErrorAndAutoRestarting = () => localize( { diff --git a/src/platform/errors/errorUtils.ts b/src/platform/errors/errorUtils.ts index 2cbc5abf111..9936c3e86fc 100644 --- a/src/platform/errors/errorUtils.ts +++ b/src/platform/errors/errorUtils.ts @@ -655,7 +655,7 @@ export function createOutputWithErrorMessageForDisplay(errorMessage: string) { return; } // If we have markdown links to run a command, turn that into a link. - const regex = /\[(?.*)\]\((?command:\S*)\)/gm; + const regex = /\[([^\[\]]*)\]\((.*?)\)/gm; let matches: RegExpExecArray | undefined | null; while ((matches = regex.exec(errorMessage)) !== null) { if (matches.length === 3) { diff --git a/src/platform/errors/errorUtils.unit.test.ts b/src/platform/errors/errorUtils.unit.test.ts new file mode 100644 index 00000000000..624536a0a9a --- /dev/null +++ b/src/platform/errors/errorUtils.unit.test.ts @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { assert } from 'chai'; +import { createOutputWithErrorMessageForDisplay } from './errorUtils'; + +suite('Error Utils', () => { + suite('Markdown links to Hrefs', () => { + function getHtmlMessage(markdown: string) { + const output = createOutputWithErrorMessageForDisplay(markdown); + const { stack } = JSON.parse(Buffer.from(output!.items[0].data).toString()) as { stack: string }; + return stack.replace('\u001b[1;31m', ''); + } + test('Markdown links to Hrefs', () => { + const markdown = 'This is a [link](https://www.microsoft.com)'; + const expected = "This is a link"; + + const html = getHtmlMessage(markdown); + + assert.strictEqual(html, expected); + }); + test('Multiple Markdown links to Hrefs', () => { + const markdown = + 'This is a [link](https://www.microsoft.com) and [this](https://www.google.com) is also a link.'; + const expected = + "This is a link and this is also a link."; + + const html = getHtmlMessage(markdown); + + assert.strictEqual(html, expected); + }); + test('VS Code command links in markdown to Hrefs', () => { + const markdown = + 'This is a command [jupyter.kernels.trusted](command:workbench.action.openSettings?["jupyter.kernels.trusted"]) link.'; + const expected = + 'This is a command jupyter.kernels.trusted link.'; + + const html = getHtmlMessage(markdown); + + assert.strictEqual(html, expected); + }); + }); +}); diff --git a/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts b/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts index 683fadbe4d6..de0e5aafaa4 100644 --- a/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts +++ b/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts @@ -61,7 +61,6 @@ import { IKernelRankingHelper } from '../../../notebooks/controllers/types'; import { KernelRankingHelper } from '../../../notebooks/controllers/kernelRanking/kernelRankingHelper'; import { CondaService } from '../../../platform/common/process/condaService.node'; import { noop } from '../../../platform/common/utils/misc'; -import { ITrustedKernelPaths } from '../../../kernels/raw/finder/types'; [false, true].forEach((isWindows) => { suite(`Local Kernel Finder ${isWindows ? 'Windows' : 'Unix'}`, () => { @@ -230,15 +229,12 @@ import { ITrustedKernelPaths } from '../../../kernels/raw/finder/types'; when(fs.copy(anything(), anything())).thenResolve(); when(fs.copy(anything(), anything(), anything())).thenResolve(); when(fs.exists(anything())).thenResolve(true); - const trustedKernelSpecs = mock(); - when(trustedKernelSpecs.isTrusted(anything())).thenReturn(true); const nonPythonKernelSpecFinder = new LocalKnownPathKernelSpecFinder( instance(fs), instance(workspaceService), jupyterPaths, instance(extensionChecker), - instance(memento), - instance(trustedKernelSpecs) + instance(memento) ); when(memento.get('LOCAL_KERNEL_SPEC_CONNECTIONS_CACHE_KEY_V2', anything())).thenReturn([]); when(memento.get('JUPYTER_GLOBAL_KERNELSPECS_V2', anything())).thenReturn([]); @@ -265,8 +261,7 @@ import { ITrustedKernelPaths } from '../../../kernels/raw/finder/types'; jupyterPaths, instance(extensionChecker), nonPythonKernelSpecFinder, - instance(memento), - instance(trustedKernelSpecs) + instance(memento) ), instance(memento), instance(fs), @@ -277,8 +272,7 @@ import { ITrustedKernelPaths } from '../../../kernels/raw/finder/types'; instance(interpreterService), instance(condaService), instance(extensions), - instance(workspaceService), - instance(trustedKernelSpecs) + instance(workspaceService) ); localKernelFinder.activate().then(noop, noop); diff --git a/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts b/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts index 80533b43d76..e1c227102e3 100644 --- a/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts +++ b/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts @@ -336,7 +336,7 @@ suite('Jupyter Paths', () => { when(memento.get(CACHE_KEY_FOR_JUPYTER_KERNEL_PATHS, anything())).thenReturn([]); const jupyter_Paths = [__filename]; process.env['JUPYTER_PATH'] = jupyter_Paths.join(path.delimiter); - const allUserProfilePath = (process.env['ALLUSERSPROFILE'] = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'temp')); + const allUserProfilePath = (process.env['PROGRAMDATA'] = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'temp')); const paths = await jupyterPaths.getKernelSpecRootPaths(); const winJupyterPath = path.join('AppData', 'Roaming', 'jupyter', 'kernels'); diff --git a/src/test/kernels/raw/finder/trustedKernelPaths.unit.test.ts b/src/test/kernels/raw/finder/trustedKernelPaths.unit.test.ts index 8b3909c0706..6128525eec9 100644 --- a/src/test/kernels/raw/finder/trustedKernelPaths.unit.test.ts +++ b/src/test/kernels/raw/finder/trustedKernelPaths.unit.test.ts @@ -9,7 +9,8 @@ import { Uri, WorkspaceConfiguration } from 'vscode'; import { IPlatformService } from '../../../../platform/common/platform/types'; import { ITrustedKernelPaths } from '../../../../kernels/raw/finder/types'; import { IWorkspaceService } from '../../../../platform/common/application/types'; -import { TrustedKernelPaths } from '../../../../kernels/raw/finder/trustedKernelSpecPaths.node'; +import { TrustedKernelPaths } from '../../../../kernels/raw/finder/trustedKernelPaths.node'; +import { TrustedKernelPaths as TrustedKernelSpecPathsWeb } from '../../../../kernels/raw/finder/trustedKernelPaths.web'; suite('Trusted Kernel paths', () => { suite('Desktop', () => { @@ -74,4 +75,14 @@ suite('Trusted Kernel paths', () => { assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/b/foo.json'))); }); }); + suite('Web', () => { + const trustedKernelPaths = new TrustedKernelSpecPathsWeb(); + test('All paths are trusted on the web', () => { + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('foo'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('foo'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/Something/venv/shared/jupyter/kernels/foo.json'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/Windows/venv/shared/jupyter/kernels/foo.json'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/Program Files/jupyter/kernels/foo.json'))); + }); + }); }); diff --git a/src/test/kernels/raw/finder/trustedKernrelPaths.unit.test.ts b/src/test/kernels/raw/finder/trustedKernrelPaths.unit.test.ts new file mode 100644 index 00000000000..045d1b43e9c --- /dev/null +++ b/src/test/kernels/raw/finder/trustedKernrelPaths.unit.test.ts @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/* eslint-disable local-rules/dont-use-process */ + +import { assert } from 'chai'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { Uri, WorkspaceConfiguration } from 'vscode'; +import { IPlatformService } from '../../../../platform/common/platform/types'; +import { TrustedKernelPaths } from '../../../../kernels/raw/finder/trustedKernelPaths.node'; +import { ITrustedKernelPaths } from '../../../../kernels/raw/finder/types'; +import { IWorkspaceService } from '../../../../platform/common/application/types'; + +suite('Trusted Kernel paths', () => { + suite('Desktop', () => { + let trustedKernelPaths: ITrustedKernelPaths; + let jupyterConfig: WorkspaceConfiguration; + let platform: IPlatformService; + const oldValueForPROGRAMDATA = process.env['PROGRAMDATA']; + setup(createTrustedPathService); + function createTrustedPathService() { + jupyterConfig = mock(); + when(jupyterConfig.get('kernels.trusted', anything())).thenCall((_, defaultValue) => defaultValue); + const workspace = mock(); + when(workspace.getConfiguration('jupyter', anything())).thenReturn(instance(jupyterConfig)); + platform = mock(); + trustedKernelPaths = new TrustedKernelPaths(instance(platform), instance(workspace)); + } + teardown(() => { + process.env['PROGRAMDATA'] = oldValueForPROGRAMDATA; + }); + test('All paths are trusted on Mac', () => { + when(platform.isWindows).thenReturn(false); + when(platform.isMac).thenReturn(true); + when(platform.isLinux).thenReturn(false); + + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('foo'))); + }); + test('All paths are trusted on Linux', () => { + when(platform.isWindows).thenReturn(false); + when(platform.isMac).thenReturn(false); + when(platform.isLinux).thenReturn(true); + + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('foo'))); + }); + test('Some paths are not trusted in windows', async () => { + process.env['PROGRAMDATA'] = 'C:/ProgramData'; + + createTrustedPathService(); + when(platform.isWindows).thenReturn(true); + + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('foo'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/Something/venv/shared/jupyter/kernels/foo.json'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/Windows/venv/shared/jupyter/kernels/foo.json'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/Program Files/jupyter/kernels/foo.json'))); + + // Untrusted paths + assert.isFalse(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/a/foo.json'))); + assert.isFalse(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/b/foo.json'))); + + // Trust and check again + when(jupyterConfig.get('kernels.trusted', anything())).thenReturn([ + 'C:/ProgramData/jupyter/kernels/a/foo.json' + ]); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/a/foo.json'))); + assert.isFalse(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/b/foo.json'))); + + // Trust and check again + when(jupyterConfig.get('kernels.trusted', anything())).thenReturn([ + 'C:/ProgramData/jupyter/kernels/b/foo.json', + 'C:/ProgramData/jupyter/kernels/a/foo.json' + ]); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/a/foo.json'))); + assert.isTrue(trustedKernelPaths.isTrusted(Uri.file('C:/ProgramData/jupyter/kernels/b/foo.json'))); + }); + }); +}); diff --git a/src/webviews/webview-side/error-renderer/index.ts b/src/webviews/webview-side/error-renderer/index.ts index 94d73e8d372..38a22e93ed3 100644 --- a/src/webviews/webview-side/error-renderer/index.ts +++ b/src/webviews/webview-side/error-renderer/index.ts @@ -13,36 +13,44 @@ let Localizations = { /* eslint-disable @typescript-eslint/no-explicit-any */ -const handleInnerClick = (target: HTMLAnchorElement, context: RendererContext) => { +/** + * Handle a click on an anchor element. + * @return {boolean} `true` if the event has been handled, else `false` + */ +function handleInnerClick(target: HTMLAnchorElement, context: RendererContext) { if (target.href && context.postMessage) { if (target.href.indexOf('file') === 0) { context.postMessage({ message: 'open_link', payload: target.href }); - return; + return true; } if (target.href.indexOf('vscode-notebook-cell') === 0) { context.postMessage({ message: 'open_link', payload: target.href }); - return; + return true; + } else if (target.href.indexOf('command:workbench.action.openSettings') === 0) { + // Let the webview handle this. + return false; } else if (target.href.indexOf('command') === 0) { context.postMessage({ message: 'open_link', payload: target.href }); - return; + return true; } else if (target.href.indexOf('https://aka.ms/') === 0) { context.postMessage({ message: 'open_link', payload: target.href }); - return; + return true; } } -}; + return false; +} if (!String.prototype.format) { String.prototype.format = function (this: string) { @@ -90,10 +98,9 @@ function handleANSIOutput(context: RendererContext, converter: ansiToHtml, } tracebackElm.addEventListener('click', (e) => { const a = e.target as HTMLAnchorElement; - if (a && a.href) { + if (a && a.href && handleInnerClick(a, context)) { e.stopImmediatePropagation(); e.preventDefault(); - handleInnerClick(a, context); } }); return tracebackElm;