Skip to content

Commit

Permalink
Ignore formatting in ipynb when dealing with trust (#14333)
Browse files Browse the repository at this point in the history
  • Loading branch information
Don Jayamanne committed Oct 9, 2020
1 parent 9d7b40c commit 957b536
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
30 changes: 26 additions & 4 deletions src/client/datascience/interactive-ipynb/trustService.ts
@@ -1,7 +1,9 @@
import { createHmac } from 'crypto';
import { inject, injectable } from 'inversify';
import { EventEmitter, Uri } from 'vscode';
import { traceError } from '../../common/logger';
import { IConfigurationService } from '../../common/types';
import { sortObjectPropertiesRecursively } from '../notebookStorage/vscNotebookModel';
import { IDigestStorage, ITrustService } from '../types';

@injectable()
Expand Down Expand Up @@ -29,9 +31,14 @@ export class TrustService implements ITrustService {
if (this.alwaysTrustNotebooks) {
return true; // Skip check if user manually overrode our trust checking
}
// Compute digest and see if notebook is trusted
const digest = await this.computeDigest(notebookContents);
return this.digestStorage.containsDigest(uri, digest);
// Compute digest and see if notebook is trusted.
// Check formatted & unformatted notebook. Possible user saved nb using old extension & opening using new extension.
const [digest1, digest2] = await Promise.all([
this.computeDigest(notebookContents),
this.computeDigest(this.getFormattedContents(notebookContents))
]);

return this.digestStorage.containsDigest(uri, digest1) || this.digestStorage.containsDigest(uri, digest2);
}

/**
Expand All @@ -41,13 +48,28 @@ export class TrustService implements ITrustService {
*/
public async trustNotebook(uri: Uri, notebookContents: string) {
if (!this.alwaysTrustNotebooks) {
notebookContents = this.getFormattedContents(notebookContents);
// Only update digest store if the user wants us to check trust
const digest = await this.computeDigest(notebookContents);
await this.digestStorage.saveDigest(uri, digest);
this._onDidSetNotebookTrust.fire();
}
}

/**
* If a notebook is opened & saved in Jupyter, even without making any changes, the JSON in ipynb could be different from the format saved by VSC.
* Similarly, the JSON saved by native notebooks could be different when compared to how they are saved by standard notebooks.
* When trusting a notebook, we don't trust the raw bytes in ipynb, we trust the contents, & ipynb stores JSON,
* Hence when computing a hash we need to ensure the hash is always the same regardless of indentation of JSON & the order of properties in json.
* This method returns the contents of the ipynb in a manner thats solves formatting issues related to JSON.
*/
private getFormattedContents(notebookContents: string) {
try {
return JSON.stringify(sortObjectPropertiesRecursively(JSON.parse(notebookContents)));
} catch (ex) {
traceError('Notebook cannot be parsed into JSON', ex);
return notebookContents;
}
}
private async computeDigest(notebookContents: string) {
const hmac = createHmac('sha256', await this.digestStorage.key);
hmac.update(notebookContents);
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/notebookStorage/vscNotebookModel.ts
Expand Up @@ -19,7 +19,7 @@ import { BaseNotebookModel, getDefaultNotebookContentForNativeNotebooks } from '

// https://github.com/microsoft/vscode-python/issues/13155
// tslint:disable-next-line: no-any
function sortObjectPropertiesRecursively(obj: any): any {
export function sortObjectPropertiesRecursively(obj: any): any {
if (Array.isArray(obj)) {
return obj.map(sortObjectPropertiesRecursively);
}
Expand Down

0 comments on commit 957b536

Please sign in to comment.