Align notebook trust behaviour with trust in classic Notebook (jupyte…

…rlab#14345) * Trust cells created automatically or by user * Trust code cell after clearing outputs * Clear `trusted` metadata when changing cell type to markdown/raw * Only count code cells in trust indicator. This is because in the current Jupyter trust model only cells with outputs can be marked as (un)trusted. Further, since nbformat does not like `trusted` metadata on non-code cells we have to remove it, and on our side `.trusted` is always undefined for non-code cells. * Add tests for trust on actions, correct implementation for trust change when clearing outputs * Remove unused trust indicator code, avoid DOM modification noise * Add galata test for trust * Add another galata test, document trust mechanism * Update Playwright Snapshots * Use reload with `waitForIsReady: false` see jupyterlab#14349 * Fix typos (suggestions from code review) Co-authored-by: Frédéric Collonval <[email protected]> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Frédéric Collonval <[email protected]>
krassowski · Apr 15, 2023 · ed7c879 · ed7c879
1 parent 9c9bc56
commit ed7c879
Show file tree

Hide file tree

Showing 11 changed files with 319 additions and 54 deletions.
diff --git a/docs/source/user/notebook.rst b/docs/source/user/notebook.rst
@@ -153,8 +153,48 @@ and select “New Console for Notebook”:
 
 .. _cell-toolbar:
 
+Cell Toolbar
+^^^^^^^^^^^^
+
 If there is enough room for it, each cell has a toolbar that provides quick access to
-commonly-used functions. If you would like to disable the cell toolbar, run
-``jupyter labextension disable @jupyterlab/cell-toolbar-extension`` on the command line.
-You can enable it again by running
-``jupyter labextension enable @jupyterlab/cell-toolbar-extension``.
+commonly-used functions. If you would like to disable the cell toolbar, run:
+
+.. code:: bash
+
+    jupyter labextension disable @jupyterlab/cell-toolbar-extension
+
+on the command line. You can enable it again by running:
+
+.. code:: bash
+
+    jupyter labextension enable @jupyterlab/cell-toolbar-extension
+
+.. _notebook-trust:
+
+Trust
+^^^^^
+
+JavaScript and HTML in notebooks created on other machines are not trusted,
+which results in sanitization of HTML and interactive outputs not being
+displayed until the notebook is explicitly trusted.
+
+.. |trusted| image:: ../images/notebook-trusted.png
+.. |not-trusted| image:: ../images/notebook-not-trusted.png
+
+The trust status of the active notebook is indicated by a shield icon in the
+status bar; a checkmark (|trusted|) in the shield indicates a trusted
+notebook while a cross (|not-trusted|) indicates an untrusted notebook.
+To trust a notebook (and render any blocked outputs) use the ``Trust Notebook``
+command available in the :ref:`command palette <commands>`.
+
+JupyterLab follows the Jupyter Notebook's
+`Security Model <https://jupyter-notebook.readthedocs.io/en/stable/security.html#our-security-model>`__
+where any output generated by the current user is trusted, with following
+implementation details of relevance to advanced users:
+
+1. manually re-running a non-trusted cell will mark it as trusted,
+2. if any of the code cells is not trusted, the entire notebook is considered
+   not trusted and none of the outputs will be trusted upon reopening it (while
+   it is unusual to see a notebook with a single untrusted cell, this can occur
+   when copy-pasting cells from an untrusted notebook),
+3. only code cells can be trusted; the Markdown cells are always sanitised.
diff --git a/galata/test/documentation/general.test.ts b/galata/test/documentation/general.test.ts
@@ -461,6 +461,37 @@ test.describe('General', () => {
     });
   });
 
+  test('Trust indicator', async ({ page }) => {
+    await page.goto();
+    // Open Data.ipynb which is not trusted by default
+    await page.dblclick(
+      '[aria-label="File Browser Section"] >> text=notebooks'
+    );
+    await page.dblclick('text=Data.ipynb');
+
+    const trustIndictor = page.locator('.jp-StatusItem-trust');
+
+    expect(await trustIndictor.screenshot()).toMatchSnapshot(
+      'notebook_not_trusted.png'
+    );
+
+    // Open trust dialog
+    // Note: we do not `await` here as it only resolves once dialog is closed
+    const trustPromise = page.evaluate(() => {
+      return window.jupyterapp.commands.execute('notebook:trust');
+    });
+    const dialogSelector = '.jp-Dialog-content';
+    await page.waitForSelector(dialogSelector);
+    // Accept option to trust the notebook
+    await page.click('.jp-Dialog-button.jp-mod-accept');
+    // Wait until dialog is gone
+    await trustPromise;
+
+    expect(await trustIndictor.screenshot()).toMatchSnapshot(
+      'notebook_trusted.png'
+    );
+  });
+
   test('Heading anchor', async ({ page }, testInfo) => {
     await page.goto();
     await setSidebarWidth(page);

diff --git a/...entation/general.test.ts-snapshots/notebook-not-trusted-documentation-linux.png b/...entation/general.test.ts-snapshots/notebook-not-trusted-documentation-linux.png
diff --git a/...ocumentation/general.test.ts-snapshots/notebook-trusted-documentation-linux.png b/...ocumentation/general.test.ts-snapshots/notebook-trusted-documentation-linux.png
diff --git a/galata/test/jupyterlab/notebook-create.test.ts b/galata/test/jupyterlab/notebook-create.test.ts
@@ -4,6 +4,7 @@
 import { expect, IJupyterLabPageFixture, test } from '@jupyterlab/galata';
 
 const fileName = 'notebook.ipynb';
+const TRUSTED_SELECTOR = 'svg[data-icon="ui-components:trusted"]';
 
 const menuPaths = ['File', 'Edit', 'View', 'Run', 'Kernel', 'Help'];
 
@@ -25,6 +26,7 @@ test.describe('Notebook Create', () => {
     await page.notebook.setCell(0, 'raw', 'Just a raw cell');
     expect(await page.notebook.getCellCount()).toBe(1);
     expect(await page.notebook.getCellType(0)).toBe('raw');
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
   });
 
   test('Create a Markdown cell', async ({ page }) => {
@@ -35,12 +37,14 @@ test.describe('Notebook Create', () => {
     await page.notebook.runCell(1, true);
     expect(await page.notebook.getCellCount()).toBe(2);
     expect(await page.notebook.getCellType(1)).toBe('markdown');
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
   });
 
   test('Create a Code cell', async ({ page }) => {
     await page.notebook.addCell('code', '2 ** 3');
     expect(await page.notebook.getCellCount()).toBe(2);
     expect(await page.notebook.getCellType(1)).toBe('code');
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
   });
 
   test('Save Notebook', async ({ page }) => {
@@ -72,6 +76,7 @@ test.describe('Notebook Create', () => {
     const imageName = 'run-cells.png';
 
     expect((await page.notebook.getCellTextOutput(2))[0]).toBe('8');
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
 
     const nbPanel = await page.notebook.getNotebookInPanel();
 

diff --git a/galata/test/jupyterlab/notebook-trust.test.ts b/galata/test/jupyterlab/notebook-trust.test.ts
@@ -0,0 +1,76 @@
+// Copyright (c) Jupyter Development Team.
+// Distributed under the terms of the Modified BSD License.
+
+import { expect, test } from '@jupyterlab/galata';
+
+const fileName = 'trust.ipynb';
+const TRUSTED_SELECTOR = 'svg[data-icon="ui-components:trusted"]';
+const NOT_TRUSTED_SELECTOR = 'svg[data-icon="ui-components:not-trusted"]';
+
+test.describe('Notebook Trust', () => {
+  test.beforeEach(async ({ page }) => {
+    await page.notebook.createNew(fileName);
+  });
+
+  test('Blank Markdown cell does not break trust', async ({ page }) => {
+    // See https://github.com/jupyterlab/jupyterlab/issues/9765
+
+    // Add an empty Markdown cell
+    await page.notebook.addCell('markdown', '');
+    // The notebook should be trusted
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
+    await page.notebook.save();
+    // Reload page
+    await page.reload({ waitForIsReady: false });
+    // Should still be trusted
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
+  });
+
+  test('Trust is lost after manually editing notebook', async ({ page }) => {
+    const browserContext = page.context();
+    await browserContext.grantPermissions(['clipboard-read']);
+    // Add text to first cell
+    await page.notebook.setCell(0, 'code', 'TEST_TEXT');
+    await page.notebook.save();
+    // The notebook should be trusted
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(1);
+    await expect(page.locator(NOT_TRUSTED_SELECTOR)).toHaveCount(0);
+
+    // Open notebook in text editor using context menu
+    await page.click(`.jp-DirListing-item span:has-text("${fileName}")`, {
+      button: 'right'
+    });
+    await page.hover('text=Open With');
+    await page.click('.lm-Menu li[role="menuitem"]:has-text("Editor")');
+    const editorContent = await page.waitForSelector(
+      '.jp-FileEditor .cm-content'
+    );
+    await editorContent.waitForSelector('text=TEST_TEXT');
+    const originalContent = await page.evaluate(async () => {
+      await window.jupyterapp.commands.execute('fileeditor:select-all');
+      await window.jupyterapp.commands.execute('fileeditor:cut');
+      return navigator.clipboard.readText();
+    });
+    const newContent = originalContent.replace('TEST_TEXT', 'SUBSTITUTED_TEXT');
+    await page.evaluate(
+      async ([newContent]) => {
+        await window.jupyterapp.commands.execute(
+          'fileeditor:replace-selection',
+          { text: newContent }
+        );
+        // Save file after changes
+        await window.jupyterapp.commands.execute('docmanager:save');
+        // Close the file editor view of the notebook
+        await window.jupyterapp.commands.execute('application:close');
+      },
+      [newContent]
+    );
+
+    // Reload page
+    await page.reload({ waitForIsReady: false });
+
+    // It should no longer be trusted
+    await expect(page.locator(TRUSTED_SELECTOR)).toHaveCount(0);
+    await expect(page.locator(NOT_TRUSTED_SELECTOR)).toHaveCount(1);
+  });
+});
diff --git a/packages/cells/src/model.ts b/packages/cells/src/model.ts
@@ -663,6 +663,8 @@ export class CodeCellModel extends CellModel implements ICodeCellModel {
     this.executionCount = null;
     this._setDirty(false);
     this.sharedModel.deleteMetadata('execution');
+    // We trust this cell as it no longer has any outputs.
+    this.trusted = true;
   }
 
   /**

diff --git a/packages/notebook/src/actions.tsx b/packages/notebook/src/actions.tsx
@@ -301,6 +301,10 @@ export namespace NotebookActions {
 
     const primaryModel = primary.model.sharedModel;
     const { cell_type, metadata } = primaryModel.toJSON();
+    if (primaryModel.cell_type === 'code') {
+      // We can trust this cell because the outputs will be removed.
+      metadata.trusted = true;
+    }
     const newModel = {
       cell_type,
       metadata,
@@ -373,7 +377,14 @@ export namespace NotebookActions {
 
     const newIndex = notebook.activeCell ? notebook.activeCellIndex : 0;
     model.sharedModel.insertCell(newIndex, {
-      cell_type: notebook.notebookConfig.defaultCell
+      cell_type: notebook.notebookConfig.defaultCell,
+      metadata:
+        notebook.notebookConfig.defaultCell === 'code'
+          ? {
+              // This is an empty cell created by user, thus is trusted
+              trusted: true
+            }
+          : {}
     });
     // Make the newly inserted cell active.
     notebook.activeCellIndex = newIndex;
@@ -403,7 +414,14 @@ export namespace NotebookActions {
 
     const newIndex = notebook.activeCell ? notebook.activeCellIndex + 1 : 0;
     model.sharedModel.insertCell(newIndex, {
-      cell_type: notebook.notebookConfig.defaultCell
+      cell_type: notebook.notebookConfig.defaultCell,
+      metadata:
+        notebook.notebookConfig.defaultCell === 'code'
+          ? {
+              // This is an empty cell created by user, thus is trusted
+              trusted: true
+            }
+          : {}
     });
     // Make the newly inserted cell active.
     notebook.activeCellIndex = newIndex;
@@ -561,7 +579,14 @@ export namespace NotebookActions {
       // Do not use push here, as we want an widget insertion
       // to make sure no placeholder widget is rendered.
       model.sharedModel.insertCell(notebook.widgets.length, {
-        cell_type: notebook.notebookConfig.defaultCell
+        cell_type: notebook.notebookConfig.defaultCell,
+        metadata:
+          notebook.notebookConfig.defaultCell === 'code'
+            ? {
+                // This is an empty cell created by user, thus is trusted
+                trusted: true
+              }
+            : {}
       });
       notebook.activeCellIndex++;
       if (notebook.activeCell?.inViewport === false) {
@@ -614,7 +639,14 @@ export namespace NotebookActions {
     );
     const model = notebook.model;
     model.sharedModel.insertCell(notebook.activeCellIndex + 1, {
-      cell_type: notebook.notebookConfig.defaultCell
+      cell_type: notebook.notebookConfig.defaultCell,
+      metadata:
+        notebook.notebookConfig.defaultCell === 'code'
+          ? {
+              // This is an empty cell created by user, thus is trusted
+              trusted: true
+            }
+          : {}
     });
     notebook.activeCellIndex++;
     if (notebook.activeCell?.inViewport === false) {
@@ -2371,13 +2403,26 @@ namespace Private {
     const cells = notebook.model!.cells;
     const index = findIndex(cells, model => model === cell.model);
 
+    // While this cell has no outputs and could be trusted following the letter
+    // of Jupyter trust model, its content comes from kernel and hence is not
+    // necessarily controlled by the user; if we set it as trusted, a user
+    // executing cells in succession could end up with unwanted trusted output.
     if (index === -1) {
       notebookModel.insertCell(notebookModel.cells.length, {
         cell_type: 'code',
-        source: text
+        source: text,
+        metadata: {
+          trusted: false
+        }
       });
     } else {
-      notebookModel.insertCell(index + 1, { cell_type: 'code', source: text });
+      notebookModel.insertCell(index + 1, {
+        cell_type: 'code',
+        source: text,
+        metadata: {
+          trusted: false
+        }
+      });
     }
   }
 
@@ -2460,6 +2505,14 @@ namespace Private {
         const raw = child.model.toJSON();
         notebookSharedModel.transact(() => {
           notebookSharedModel.deleteCell(index);
+          if (value === 'code') {
+            // After change of type outputs are deleted so cell can be trusted.
+            raw.metadata.trusted = true;
+          } else {
+            // Otherwise clear the metadata as trusted is only "valid" on code
+            // cells (since other cell types cannot have outputs).
+            raw.metadata.trusted = undefined;
+          }
           const newCell = notebookSharedModel.insertCell(index, {
             cell_type: value,
             source: raw.source,
@@ -2522,7 +2575,14 @@ namespace Private {
         // a notebook's last cell undoable.
         if (sharedModel.cells.length == toDelete.length) {
           sharedModel.insertCell(0, {
-            cell_type: notebook.notebookConfig.defaultCell
+            cell_type: notebook.notebookConfig.defaultCell,
+            metadata:
+              notebook.notebookConfig.defaultCell === 'code'
+                ? {
+                    // This is an empty cell created in empty notebook, thus is trusted
+                    trusted: true
+                  }
+                : {}
           });
         }
       });