Check marking schemes for attachments to prevent leakage

nbgrader/preprocessors/clearmarkingscheme.py

@@ -14,12 +14,12 @@ class ClearMarkScheme(NbGraderPreprocessor):
 
     begin_mark_scheme_delimeter = Unicode(
         "BEGIN MARK SCHEME",
-        help="The delimiter marking the beginning of hidden tests cases"
+        help="The delimiter marking the beginning of a marking scheme region"
     ).tag(config=True)
 
     end_mark_scheme_delimeter = Unicode(
         "END MARK SCHEME",
-        help="The delimiter marking the end of hidden tests cases"
+        help="The delimiter marking the end of a marking scheme region"
     ).tag(config=True)
 
     enforce_metadata = Bool(
@@ -35,6 +35,16 @@ class ClearMarkScheme(NbGraderPreprocessor):
         )
     ).tag(config=True)
 
+    check_attachment_leakage = Bool(
+        True,
+        help=dedent(
+            """
+            Whether or not to check if a marking scheme region contains an attachment,
+            in order to prevent leakage to student version of notebooks.
+            """
+        )
+    ).tag(config=True)
+
     def _remove_mark_scheme_region(self, cell: NotebookNode) -> bool:
         """Find a region in the cell that is delimeted by
         `self.begin_mark_scheme_delimeter` and `self.end_mark_scheme_delimeter` (e.g.  ###
@@ -50,6 +60,7 @@ def _remove_mark_scheme_region(self, cell: NotebookNode) -> bool:
         new_lines = []
         in_ms = False
         removed_ms = False
+        attachment_regex = r"!\[.*\]\(attachment:.+?\)"
 
         for line in lines:
             # begin the test area
@@ -67,6 +78,18 @@ def _remove_mark_scheme_region(self, cell: NotebookNode) -> bool:
             elif self.end_mark_scheme_delimeter in line:
                 in_ms = False
 
+            elif self.check_attachment_leakage and in_ms and re.search(attachment_regex, line):
+                raise RuntimeError(
+                    """
+                    Encountered an attachment in a marking scheme.
+                    This can leak to student notebooks. 
+                    You might want to embed your image instead, like here:
+                    https://github.com/jupyter/nbgrader/issues/1782#issuecomment-1541493629.
+                    You can suppress this check with ClearMarkScheme.check_attachment_leakage=False.
+                    For more info: https://github.com/jupyter/nbgrader/issues/1782
+                    """
+                )
+
             # add lines as long as it's not in the hidden tests region
             elif not in_ms:
                 new_lines.append(line)

nbgrader/tests/preprocessors/test_clearmarkscheme.py

@@ -108,3 +108,65 @@ def test_preprocess_notebook(self, preprocessor):
         """Is the test notebook processed without error?"""
         nb = self._read_nb(os.path.join("files", "test_taskcell.ipynb"))
         preprocessor.preprocess(nb, {})
+
+    # attachment detection tests
+    def test_attachment_in_mark_scheme(self, preprocessor):
+        """Is an error raised when there is an attachment in the marking scheme?"""
+        source = dedent(
+            """
+            assert True
+            ### BEGIN MARK SCHEME
+            ![](attachment:image.png)
+            ### END MARK SCHEME
+            """
+        ).strip()
+        cell = create_task_cell(source, 'markdown', 'some-task-id', 2)
+        with pytest.raises(RuntimeError):
+            preprocessor._remove_mark_scheme_region(cell)
+
+        source = dedent(
+            """
+            assert True
+            ### BEGIN MARK SCHEME
+            Text text text text text.
+            ![](attachment:image1.jpg)
+            Grade grade grade.
+            ![](attachment:image2.png)
+            Mark mark mark mark.
+            ### END MARK SCHEME
+            """
+        ).strip()
+        cell = create_task_cell(source, 'markdown', 'some-task-id', 2)
+        with pytest.raises(RuntimeError):
+            preprocessor._remove_mark_scheme_region(cell)
+
+    def test_attachment_suppress_error(self, preprocessor):
+        """Can the error be suppressed?"""
+        source = dedent(
+            """
+            assert True
+            ### BEGIN MARK SCHEME
+            ![](attachment:image.png)
+            ### END MARK SCHEME
+            """
+        ).strip()
+        cell = create_task_cell(source, 'markdown', 'some-task-id', 2)
+        preprocessor.check_attachment_leakage = False
+        removed_test = preprocessor._remove_mark_scheme_region(cell)
+        assert removed_test
+        assert cell.source == "assert True"
+
+    def test_attachment_not_in_mark_scheme(self, preprocessor):
+        """Attachments outside of marking schemes shouldn't be touched"""
+        source = dedent(
+            """
+            ![](attachment:image.png)
+            ### BEGIN MARK SCHEME
+            assert True
+            ### END MARK SCHEME
+            """
+        ).strip()
+        cell = create_task_cell(source, 'markdown', 'some-task-id', 2)
+        removed_test = preprocessor._remove_mark_scheme_region(cell)
+        assert removed_test
+        assert cell.source == "![](attachment:image.png)"