Swallow potential exceptions from showtraceback() (#13934)

Carreau · web-flow · commit e548ee23ac46 · 2023-02-10T10:14:21.000+01:00
The nbgrader project is aware of a form of cheating where students disrupt `InteractiveShell.showtraceback` in hopes of hiding exceptions to avoid losing points. They have implemented a solution to prevent this cheating from working on the client side, and have some tests to demonstrate this technique: https://github.com/jupyter/nbgrader/blob/main/nbgrader/tests/apps/files/submitted-cheat-attempt.ipynb https://github.com/jupyter/nbgrader/blob/main/nbgrader/tests/apps/files/submitted-cheat-attempt-alternative.ipynb In essence, these attacks import the interactive shell and erase the traceback handler so that their failing tests won't report failures. ```python import IPython.core.interactiveshell IPython.core.interactiveshell.InteractiveShell.showtraceback = None ``` The problem is that this causes an exception inside the kernel, leading to a stalled execution. The kernel has stopped working, but the client continues to wait for messages. So far, nbgrader's solution to this is to require a timeout value so the client can eventually decide it is done. This prevents allowing a value of `None` for `Execute.timeout` because this would cause a test case to infinitely hang. This commit addresses the problem by making `InteractiveShell._run_cell` a little more protective around it's call to `showtraceback()`. There is already a try/except block around running the cell. This commit adds a finally clause so that the method will _always_ return an `ExecutionResult`, even if a new exception is thrown within the except clause. For the record, the exception thrown is: TypeError: 'NoneType' object is not callable Accepting this change will allow nbgrader to update `nbgrader.preprocessors.Execute` to support a type of `Integer(allow_none=True)` as the parent `NotebookClient` intended. Discussion about this is ongoing in jupyter/nbgrader#1690.
diff --git a/IPython/core/interactiveshell.py b/IPython/core/interactiveshell.py
@@ -3013,14 +3013,15 @@ def _run_cell(
             runner = _pseudo_sync_runner
 
         try:
-            return runner(coro)
+            result = runner(coro)
         except BaseException as e:
             info = ExecutionInfo(
                 raw_cell, store_history, silent, shell_futures, cell_id
             )
             result = ExecutionResult(info)
             result.error_in_exec = e
             self.showtraceback(running_compiled_code=True)
+        finally:
             return result
 
     def should_run_async(
diff --git a/IPython/core/tests/test_interactiveshell.py b/IPython/core/tests/test_interactiveshell.py
@@ -1127,3 +1127,49 @@ def foo(*args, **kwargs):
 
     # clean up
     ip.Completer.custom_matchers.pop()
+
+
+class TestShowTracebackAttack(unittest.TestCase):
+    """Test that the interactive shell is resilient against the client attack of
+    manipulating the showtracebacks method. These attacks shouldn't result in an
+    unhandled exception in the kernel."""
+
+    def setUp(self):
+        self.orig_showtraceback = interactiveshell.InteractiveShell.showtraceback
+
+    def tearDown(self):
+        interactiveshell.InteractiveShell.showtraceback = self.orig_showtraceback
+
+    def test_set_show_tracebacks_none(self):
+        """Test the case of the client setting showtracebacks to None"""
+
+        result = ip.run_cell(
+            """
+            import IPython.core.interactiveshell
+            IPython.core.interactiveshell.InteractiveShell.showtraceback = None
+
+            assert False, "This should not raise an exception"
+        """
+        )
+        print(result)
+
+        assert result.result is None
+        assert isinstance(result.error_in_exec, TypeError)
+        assert str(result.error_in_exec) == "'NoneType' object is not callable"
+
+    def test_set_show_tracebacks_noop(self):
+        """Test the case of the client setting showtracebacks to a no op lambda"""
+
+        result = ip.run_cell(
+            """
+            import IPython.core.interactiveshell
+            IPython.core.interactiveshell.InteractiveShell.showtraceback = lambda *args, **kwargs: None
+
+            assert False, "This should not raise an exception"
+        """
+        )
+        print(result)
+
+        assert result.result is None
+        assert isinstance(result.error_in_exec, AssertionError)
+        assert str(result.error_in_exec) == "This should not raise an exception"
