Reject QTI items with entity references instead of raising

contentcuration/contentcuration/tests/test_exportchannel.py

@@ -39,6 +39,7 @@
 from .testdata import thumbnail_bytes
 from .testdata import tree
 from .utils.qti.test_validation import _item_xml
+from .utils.qti.test_validation import ENTITY_CHOICE_ITEM
 from .utils.qti.test_validation import VALID_CHOICE_ITEM
 from .utils.restricted_filesystemstorage import RestrictedFileSystemStorage
 from contentcuration import models as cc
@@ -332,6 +333,28 @@ def setUp(self):
             randomize=False,
         )
 
+        native_qti_entity_exercise = create_node(
+            {
+                "kind_id": "exercise",
+                "title": "Native QTI Entity Exercise",
+                "extra_fields": qti_extra_fields,
+            }
+        )
+        native_qti_entity_exercise.complete = True
+        native_qti_entity_exercise.parent = current_exercise.parent
+        native_qti_entity_exercise.save()
+        cc.AssessmentItem.objects.create(
+            contentnode=native_qti_entity_exercise,
+            assessment_id=uuid.uuid4().hex,
+            type=exercises.QTI,
+            question="",
+            answers="[]",
+            hints="[]",
+            raw_data=ENTITY_CHOICE_ITEM,
+            order=1,
+            randomize=False,
+        )
+
         # Only legacy structured-field items, no perseus_question -> must now route to QTI (was Perseus)
         legacy_no_perseus_exercise = create_node(
             {
@@ -1015,6 +1038,14 @@ def test_native_qti_unsupported_interaction_publishes_qti_only(self):
         self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists())
         self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists())
 
+    def test_native_qti_entity_item_is_excluded_from_qti_only_package(self):
+        node = cc.ContentNode.objects.get(title="Native QTI Entity Exercise")
+        self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists())
+        qti_file = node.files.get(preset_id=format_presets.QTI_ZIP)
+        with qti_file.file_on_disk.open("rb") as file_handle:
+            names = zipfile.ZipFile(file_handle).namelist()
+        self.assertEqual([name for name in names if name.startswith("items/")], [])
+
     def test_legacy_items_without_perseus_question_route_to_qti_packaging(self):
         node = cc.ContentNode.objects.get(title="Legacy No Perseus Exercise")
         self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists())

contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py

@@ -4,6 +4,7 @@
 from le_utils.constants import exercises
 
 from contentcuration.tests.utils.qti.test_validation import _item_xml
+from contentcuration.tests.utils.qti.test_validation import ENTITY_CHOICE_ITEM
 from contentcuration.tests.utils.qti.test_validation import HINTED_EDITOR_ITEM
 from contentcuration.utils.assessment.qti.perseus_derive import derive_perseus_item
 from contentcuration.utils.assessment.qti.perseus_derive import is_perseus_derivable
@@ -279,6 +280,7 @@ def test_response_identifier_mismatch_degrades_to_qti_only():
         pytest.param(TWO_INTERACTION_ITEM, id="two_interactions"),
         pytest.param(EXTENDED_TEXT_ITEM, id="extended_text"),
         pytest.param(MALFORMED_XML, id="malformed_xml"),
+        pytest.param(ENTITY_CHOICE_ITEM, id="doctype_entity"),
     ],
 )
 def test_not_derivable(raw_data):

contentcuration/contentcuration/tests/utils/qti/test_validation.py

@@ -47,6 +47,14 @@ def _item_xml(identifier, title, response_declaration, item_body):
     "</qti-choice-interaction>",
 )
 
+_XML_DECLARATION = '<?xml version="1.0" encoding="UTF-8"?>'
+
+_ENTITY_DOCTYPE = '<!DOCTYPE qti-assessment-item [<!ENTITY e "hi">]>'
+
+ENTITY_CHOICE_ITEM = VALID_CHOICE_ITEM.replace(
+    _XML_DECLARATION, _XML_DECLARATION + _ENTITY_DOCTYPE
+).replace("Option A", "Option &e;")
+
 
 class ValidateQTIItemTests(unittest.TestCase):
     def test_accepts_valid_item(self):
@@ -96,6 +104,46 @@ def test_does_not_resolve_external_entities(self):
         serialized = " ".join(e.message for e in result.errors)
         self.assertNotIn("super-secret-value", serialized)
 
+    def test_rejects_entity_reference_without_raising(self):
+        result = validate_qti_item(ENTITY_CHOICE_ITEM)
+        self.assertFalse(result.is_valid)
+        self.assertEqual(len(result.errors), 1)
+
+    def test_rejects_external_entity_reference_in_body(self):
+        with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f:
+            f.write("super-secret-value")
+            secret_path = f.name
+        self.addCleanup(os.remove, secret_path)
+        xml = VALID_CHOICE_ITEM.replace(
+            _XML_DECLARATION,
+            _XML_DECLARATION
+            + '<!DOCTYPE qti-assessment-item [<!ENTITY e SYSTEM "file://%s">]>'
+            % secret_path,
+        ).replace("Option A", "Option &e;")
+        result = validate_qti_item(xml)
+        self.assertFalse(result.is_valid)
+        self.assertNotIn(
+            "super-secret-value", " ".join(e.message for e in result.errors)
+        )
+
+    def test_accepts_entity_in_attribute_value(self):
+        xml = VALID_CHOICE_ITEM.replace(
+            _XML_DECLARATION, _XML_DECLARATION + _ENTITY_DOCTYPE
+        ).replace('title="Sample Item"', 'title="&e;"')
+        self.assertTrue(validate_qti_item(xml).is_valid)
+
+    def test_accepts_unused_entity_declaration(self):
+        xml = VALID_CHOICE_ITEM.replace(
+            _XML_DECLARATION, _XML_DECLARATION + _ENTITY_DOCTYPE
+        )
+        self.assertTrue(validate_qti_item(xml).is_valid)
+
+    def test_accepts_bare_doctype(self):
+        xml = VALID_CHOICE_ITEM.replace(
+            _XML_DECLARATION, _XML_DECLARATION + "<!DOCTYPE qti-assessment-item>"
+        )
+        self.assertTrue(validate_qti_item(xml).is_valid)
+
 
 MATCH_INTERACTION_ITEM = _item_xml(
     "item_match",

contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py

@@ -9,6 +9,7 @@
 from contentcuration.tests import testdata
 from contentcuration.tests.base import StudioAPITestCase
 from contentcuration.tests.utils.qti.test_validation import _item_xml
+from contentcuration.tests.utils.qti.test_validation import ENTITY_CHOICE_ITEM
 from contentcuration.tests.utils.qti.test_validation import VALID_CHOICE_ITEM
 from contentcuration.tests.viewsets.base import generate_create_event
 from contentcuration.tests.viewsets.base import generate_delete_event
@@ -1087,6 +1088,27 @@ def test_invalid_qti_xml_syntax_assessmentitem(self):
                 assessment_id=assessmentitem["assessment_id"]
             )
 
+    def test_qti_entity_reference_assessmentitem(self):
+        self.client.force_authenticate(user=self.user)
+        assessmentitem = self.assessmentitem_metadata
+        assessmentitem["type"] = "QTI"
+        assessmentitem["raw_data"] = ENTITY_CHOICE_ITEM
+        response = self.sync_changes(
+            [
+                generate_create_event(
+                    [assessmentitem["contentnode"], assessmentitem["assessment_id"]],
+                    ASSESSMENTITEM,
+                    assessmentitem,
+                    channel_id=self.channel.id,
+                ),
+            ],
+        )
+        self.assertTrue(response.json()["errors"][0]["errors"]["raw_data"])
+        with self.assertRaises(models.AssessmentItem.DoesNotExist):
+            models.AssessmentItem.objects.get(
+                assessment_id=assessmentitem["assessment_id"]
+            )
+
     def test_valid_answers_assessmentitem(self):
         self.client.force_authenticate(user=self.user)
         assessmentitem = self.assessmentitem_metadata

contentcuration/contentcuration/utils/assessment/qti/validation.py

@@ -55,7 +55,14 @@ def secure_parser() -> etree.XMLParser:
 
 
 def parse_qti_xml(xml: bytes) -> etree._Element:
-    return etree.parse(BytesIO(xml), parser=secure_parser())
+    doc = etree.parse(BytesIO(xml), parser=secure_parser())
+    # Unresolved entity nodes crash schema validation and derivation.
+    entity = next(doc.iter(etree.Entity), None)
+    if entity is not None:
+        raise etree.XMLSyntaxError(
+            "entity references are not supported", 0, entity.sourceline or 0, 1
+        )
+    return doc
 
 
 def validate_qti_item(xml: Union[str, bytes]) -> QTIValidationResult: