From 656fb618e7376ec638a0d82519fa709d1617c8be Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Mon, 13 Jul 2020 13:45:32 -0700 Subject: [PATCH] Port trust fixes (#12929) * Fix regressions in trusted notebooks (#12902) * Handle trustAllNotebooks selection * Fix bug where after trusting, UI didn't update * Recover from ENOENT due to missing parent directory when trusting notebook (#12913) * Disable keydown on native cells in untrusted notebooks (#12914) --- .../interactive-ipynb/digestStorage.ts | 26 ++++++++++++--- .../interactive-ipynb/nativeEditor.ts | 2 +- .../interactive-ipynb/trustCommandHandler.ts | 33 +++++++++++-------- .../native-editor/nativeCell.tsx | 15 +++++++++ 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/client/datascience/interactive-ipynb/digestStorage.ts b/src/client/datascience/interactive-ipynb/digestStorage.ts index 0db3feed5fa5..620953bff53d 100644 --- a/src/client/datascience/interactive-ipynb/digestStorage.ts +++ b/src/client/datascience/interactive-ipynb/digestStorage.ts @@ -26,10 +26,20 @@ export class DigestStorage implements IDigestStorage { public async saveDigest(uri: Uri, signature: string) { const fileLocation = await this.getFileLocation(uri); // Since the signature is a hex digest, the character 'z' is being used to delimit the start and end of a single digest - await this.fs.appendFile(fileLocation, `z${signature}z\n`); - if (!this.loggedFileLocations.has(fileLocation)) { - traceInfo(`Wrote trust for ${uri.toString()} to ${fileLocation}`); - this.loggedFileLocations.add(fileLocation); + try { + await this.saveDigestInner(uri, fileLocation, signature); + } catch (err) { + // The nbsignatures dir is only initialized on extension activation. + // If the user deletes it to reset trust, the next attempt to trust + // an untrusted notebook in the same session will fail because the parent + // directory does not exist. + if (isFileNotFoundError(err)) { + // Gracefully recover from such errors by reinitializing directory and retrying + await this.initDir(); + await this.saveDigestInner(uri, fileLocation, signature); + } else { + traceError(err); + } } } @@ -46,6 +56,14 @@ export class DigestStorage implements IDigestStorage { } } + private async saveDigestInner(uri: Uri, fileLocation: string, signature: string) { + await this.fs.appendFile(fileLocation, `z${signature}z\n`); + if (!this.loggedFileLocations.has(fileLocation)) { + traceInfo(`Wrote trust for ${uri.toString()} to ${fileLocation}`); + this.loggedFileLocations.add(fileLocation); + } + } + private async getFileLocation(uri: Uri): Promise { const normalizedName = os.platform() === 'win32' ? uri.fsPath.toLowerCase() : uri.fsPath; const hashedName = createHash('sha256').update(normalizedName).digest('hex'); diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 0ae442b19afb..7eeb259016fc 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -252,7 +252,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Sign up for dirty events model.changed(this.modelChanged.bind(this)); - this.previouslyNotTrusted = model.isTrusted; + this.previouslyNotTrusted = !model.isTrusted; } // tslint:disable-next-line: no-any diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts index f7834234477f..6d44adda8a91 100644 --- a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -4,7 +4,7 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; +import { commands, Uri } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; import { IApplicationShell, ICommandManager } from '../../common/application/types'; import { ContextKey } from '../../common/contextKey'; @@ -57,18 +57,25 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { DataScience.doNotTrustNotebook(), DataScience.trustAllNotebooks() ); - if (selection !== DataScience.trustNotebook() || model.isTrusted) { - return; + + switch (selection) { + case DataScience.trustAllNotebooks(): + commands.executeCommand('workbench.action.openSettings', 'python.dataScience.alwaysTrustNotebooks'); + break; + case DataScience.trustNotebook(): + // Update model trust + model.update({ + source: 'user', + kind: 'updateTrust', + oldDirty: model.isDirty, + newDirty: model.isDirty, + isNotebookTrusted: true + }); + const contents = model.getContent(); + await this.trustService.trustNotebook(model.file, contents); + break; + default: + break; } - // Update model trust - model.update({ - source: 'user', - kind: 'updateTrust', - oldDirty: model.isDirty, - newDirty: model.isDirty, - isNotebookTrusted: true - }); - const contents = model.getContent(); - await this.trustService.trustNotebook(model.file, contents); } } diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 39f97b3f7090..77fe2ad356db 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -281,6 +281,9 @@ export class NativeCell extends React.Component { // tslint:disable-next-line: cyclomatic-complexity max-func-body-length private keyDownInput = (cellId: string, e: IKeyboardEvent) => { + if (!this.isNotebookTrusted() && !isCellNavigationKeyboardEvent(e)) { + return; + } const isFocusedWhenNotSuggesting = this.isFocused() && e.editorInfo && !e.editorInfo.isSuggesting; switch (e.code) { case 'ArrowUp': @@ -886,3 +889,15 @@ export class NativeCell extends React.Component { export function getConnectedNativeCell() { return connect(null, actionCreators)(NativeCell); } + +function isCellNavigationKeyboardEvent(e: IKeyboardEvent) { + return ( + e.code === 'Enter' || + e.code === 'NumpadEnter' || + e.code === 'ArrowUp' || + e.code === 'k' || + e.code === 'ArrowDown' || + e.code === 'j' || + e.code === 'Escape' + ); +}