Skip to content

Commit

Permalink
Port trust fixes (#12929)
Browse files Browse the repository at this point in the history
* 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)
  • Loading branch information
joyceerhl committed Jul 13, 2020
1 parent f0bad89 commit 656fb61
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 18 deletions.
26 changes: 22 additions & 4 deletions src/client/datascience/interactive-ipynb/digestStorage.ts
Expand Up @@ -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);
}
}
}

Expand All @@ -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<string> {
const normalizedName = os.platform() === 'win32' ? uri.fsPath.toLowerCase() : uri.fsPath;
const hashedName = createHash('sha256').update(normalizedName).digest('hex');
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/interactive-ipynb/nativeEditor.ts
Expand Up @@ -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
Expand Down
33 changes: 20 additions & 13 deletions src/client/datascience/interactive-ipynb/trustCommandHandler.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
}
15 changes: 15 additions & 0 deletions src/datascience-ui/native-editor/nativeCell.tsx
Expand Up @@ -281,6 +281,9 @@ export class NativeCell extends React.Component<INativeCellProps> {

// 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':
Expand Down Expand Up @@ -886,3 +889,15 @@ export class NativeCell extends React.Component<INativeCellProps> {
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'
);
}

0 comments on commit 656fb61

Please sign in to comment.