feat: Upgrade to codejail 4.0.0 (#36916)
This brings an important security improvement -- codejail won't default to running in unsafe mode, which can happen if certain configuration errors are present. Properly configured installations shouldn't be affected. We just need to adjust some unit tests to opt into unsafe mode. Changes: - Update `edx-codejail` dependency to [version 4.0.0](https://github.com/openedx/codejail/blob/master/CHANGELOG.rst#400---2025-06-13) - Define a `use_unsafe_codejail` decorator that allows running a unit test (or entire TestCase class) in unsafe mode - Use that decorator as needed, based on which tests started failing
This commit is contained in:
@@ -28,6 +28,7 @@ from xmodule.capa.tests.response_xml_factory import (
|
||||
OptionResponseXMLFactory,
|
||||
SchematicResponseXMLFactory
|
||||
)
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
from xmodule.capa.xqueue_interface import XQueueInterface
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
from lms.djangoapps.courseware.models import BaseStudentModuleHistory, StudentModule
|
||||
@@ -810,6 +811,7 @@ class ProblemWithUploadedFilesTest(TestSubmittingProblems):
|
||||
self.assertEqual(list(kwargs['files'].keys()), filenames.split())
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class TestPythonGradedResponse(TestSubmittingProblems):
|
||||
"""
|
||||
Check that we can submit a schematic and custom response, and it answers properly.
|
||||
|
||||
@@ -22,6 +22,7 @@ from django.urls import reverse
|
||||
|
||||
from xmodule.capa.responsetypes import StudentInputError
|
||||
from xmodule.capa.tests.response_xml_factory import CodeResponseXMLFactory, CustomResponseXMLFactory
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
from lms.djangoapps.courseware.model_data import StudentModule
|
||||
from lms.djangoapps.grades.api import CourseGradeFactory
|
||||
from lms.djangoapps.instructor_task.api import (
|
||||
@@ -71,6 +72,7 @@ class TestIntegrationTask(InstructorTaskModuleTestCase):
|
||||
|
||||
@ddt.ddt
|
||||
@override_settings(RATELIMIT_ENABLE=False)
|
||||
@use_unsafe_codejail()
|
||||
class TestRescoringTask(TestIntegrationTask):
|
||||
"""
|
||||
Integration-style tests for rescoring problems in a background task.
|
||||
|
||||
@@ -421,7 +421,7 @@ edx-celeryutils==1.4.0
|
||||
# -r requirements/edx/kernel.in
|
||||
# edx-name-affirmation
|
||||
# super-csv
|
||||
edx-codejail==3.5.2
|
||||
edx-codejail==4.0.0
|
||||
# via -r requirements/edx/kernel.in
|
||||
edx-completion==4.9
|
||||
# via -r requirements/edx/kernel.in
|
||||
|
||||
@@ -691,7 +691,7 @@ edx-celeryutils==1.4.0
|
||||
# -r requirements/edx/testing.txt
|
||||
# edx-name-affirmation
|
||||
# super-csv
|
||||
edx-codejail==3.5.2
|
||||
edx-codejail==4.0.0
|
||||
# via
|
||||
# -r requirements/edx/doc.txt
|
||||
# -r requirements/edx/testing.txt
|
||||
|
||||
@@ -505,7 +505,7 @@ edx-celeryutils==1.4.0
|
||||
# -r requirements/edx/base.txt
|
||||
# edx-name-affirmation
|
||||
# super-csv
|
||||
edx-codejail==3.5.2
|
||||
edx-codejail==4.0.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-completion==4.9
|
||||
# via -r requirements/edx/base.txt
|
||||
|
||||
@@ -66,7 +66,8 @@ edx-celeryutils
|
||||
edx-completion
|
||||
edx-django-release-util # Release utils for the edx release pipeline
|
||||
edx-django-sites-extensions
|
||||
edx-codejail
|
||||
# Codejail 4 brings important safety improvements (no unsafe mode by default)
|
||||
edx-codejail>=4.0.0
|
||||
# edx-django-utils 5.14.1 adds FrontendMonitoringMiddleware
|
||||
edx-django-utils>=5.14.1 # Utilities for cache, monitoring, and plugins
|
||||
edx-drf-extensions
|
||||
|
||||
@@ -530,7 +530,7 @@ edx-celeryutils==1.4.0
|
||||
# -r requirements/edx/base.txt
|
||||
# edx-name-affirmation
|
||||
# super-csv
|
||||
edx-codejail==3.5.2
|
||||
edx-codejail==4.0.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-completion==4.9
|
||||
# via -r requirements/edx/base.txt
|
||||
|
||||
@@ -24,8 +24,10 @@ from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
from xmodule.capa.safe_exec import safe_exec, update_hash
|
||||
from xmodule.capa.safe_exec.remote_exec import is_codejail_in_darklaunch, is_codejail_rest_service_enabled
|
||||
from xmodule.capa.safe_exec.safe_exec import emsg_normalizers, normalize_error_message
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class TestSafeExec(unittest.TestCase): # lint-amnesty, pylint: disable=missing-class-docstring
|
||||
def test_set_values(self):
|
||||
g = {}
|
||||
@@ -530,6 +532,7 @@ class DictCache(object):
|
||||
self.cache[key] = value
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class TestSafeExecCaching(unittest.TestCase):
|
||||
"""Test that caching works on safe_exec."""
|
||||
|
||||
@@ -654,6 +657,7 @@ class TestUpdateHash(unittest.TestCase):
|
||||
assert h1 == h2
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class TestRealProblems(unittest.TestCase): # lint-amnesty, pylint: disable=missing-class-docstring
|
||||
def test_802x(self):
|
||||
code = textwrap.dedent("""\
|
||||
|
||||
@@ -15,6 +15,7 @@ from markupsafe import Markup
|
||||
from xmodule.capa.correctmap import CorrectMap
|
||||
from xmodule.capa.responsetypes import LoncapaProblemError
|
||||
from xmodule.capa.tests.helpers import new_loncapa_problem
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
from openedx.core.djangolib.markup import HTML
|
||||
|
||||
|
||||
@@ -23,6 +24,7 @@ FEATURES_WITH_GRADING_METHOD_IN_PROBLEMS['ENABLE_GRADING_METHOD_IN_PROBLEMS'] =
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@use_unsafe_codejail()
|
||||
class CAPAProblemTest(unittest.TestCase):
|
||||
""" CAPA problem related tests"""
|
||||
|
||||
@@ -424,6 +426,7 @@ class CAPAProblemTest(unittest.TestCase):
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@use_unsafe_codejail()
|
||||
class CAPAMultiInputProblemTest(unittest.TestCase):
|
||||
""" TestCase for CAPA problems with multiple inputtypes """
|
||||
|
||||
|
||||
@@ -11,12 +11,14 @@ from unittest import mock
|
||||
import ddt
|
||||
from lxml import etree
|
||||
from xmodule.capa.tests.helpers import new_loncapa_problem, mock_capa_system
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
from openedx.core.djangolib.markup import HTML
|
||||
|
||||
from .response_xml_factory import CustomResponseXMLFactory, StringResponseXMLFactory
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@use_unsafe_codejail()
|
||||
class CapaHtmlRenderTest(unittest.TestCase):
|
||||
"""
|
||||
CAPA HTML rendering tests class.
|
||||
|
||||
@@ -37,6 +37,7 @@ from xmodule.capa.tests.response_xml_factory import (
|
||||
SymbolicResponseXMLFactory,
|
||||
TrueFalseResponseXMLFactory
|
||||
)
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
from xmodule.capa.util import convert_files_to_filenames
|
||||
from xmodule.capa.xqueue_interface import dateformat
|
||||
|
||||
@@ -108,6 +109,7 @@ class ResponseTest(unittest.TestCase):
|
||||
return str(rand.randint(0, 1e9))
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class MultiChoiceResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
xml_factory_class = MultipleChoiceResponseXMLFactory
|
||||
|
||||
@@ -375,6 +377,7 @@ class SymbolicResponseTest(ResponseTest): # pylint: disable=missing-class-docst
|
||||
assert correct_map.get_correctness('1_2_1') == expected_correctness
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class OptionResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
xml_factory_class = OptionResponseXMLFactory
|
||||
|
||||
@@ -422,6 +425,7 @@ class OptionResponseTest(ResponseTest): # pylint: disable=missing-class-docstri
|
||||
assert correct_map.get_property('1_2_1', 'answervariable') == '$a'
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class FormulaResponseTest(ResponseTest):
|
||||
"""
|
||||
Test the FormulaResponse class
|
||||
@@ -571,6 +575,7 @@ class FormulaResponseTest(ResponseTest):
|
||||
assert not list(problem.responders.values())[0].validate_answer('3*y+2*x')
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class StringResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
xml_factory_class = StringResponseXMLFactory
|
||||
|
||||
@@ -1124,6 +1129,7 @@ class CodeResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
assert output[answer_id]['msg'] == 'Invalid grader reply. Please contact the course staff.'
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class ChoiceResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
xml_factory_class = ChoiceResponseXMLFactory
|
||||
|
||||
@@ -1292,6 +1298,7 @@ class ChoiceResponseTest(ResponseTest): # pylint: disable=missing-class-docstri
|
||||
self.assert_grade(problem, ['choice_1', 'choice_3'], 'incorrect')
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class NumericalResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
xml_factory_class = NumericalResponseXMLFactory
|
||||
|
||||
@@ -1680,6 +1687,7 @@ class NumericalResponseTest(ResponseTest): # pylint: disable=missing-class-docs
|
||||
assert not responder.validate_answer('fish')
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class CustomResponseTest(ResponseTest): # pylint: disable=missing-class-docstring
|
||||
xml_factory_class = CustomResponseXMLFactory
|
||||
|
||||
@@ -2399,6 +2407,7 @@ class CustomResponseTest(ResponseTest): # pylint: disable=missing-class-docstri
|
||||
assert correct_map.get_msg('1_2_11') == '11'
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class SchematicResponseTest(ResponseTest):
|
||||
"""
|
||||
Class containing setup and tests for Schematic responsetype.
|
||||
@@ -2488,6 +2497,7 @@ class AnnotationResponseTest(ResponseTest): # lint-amnesty, pylint: disable=mis
|
||||
assert expected_points == actual_points, ('%s should have %d points' % (answer_id, expected_points))
|
||||
|
||||
|
||||
@use_unsafe_codejail()
|
||||
class ChoiceTextResponseTest(ResponseTest):
|
||||
"""
|
||||
Class containing setup and tests for ChoiceText responsetype.
|
||||
|
||||
@@ -6,7 +6,9 @@ Tests capa util
|
||||
|
||||
import unittest
|
||||
|
||||
import codejail.safe_exec
|
||||
import ddt
|
||||
from django.test.utils import TestContextDecorator
|
||||
from lxml import etree
|
||||
|
||||
from xmodule.capa.tests.helpers import mock_capa_system
|
||||
@@ -167,3 +169,28 @@ class UtilTest(unittest.TestCase):
|
||||
expected_text = '$あなたあなたあなたあなた あなたhi'
|
||||
contextual_text = contextualize_text(text, context)
|
||||
assert expected_text == contextual_text
|
||||
|
||||
|
||||
class use_unsafe_codejail(TestContextDecorator):
|
||||
"""
|
||||
Tell codejail to run in unsafe mode for the scope of the decorator.
|
||||
Use this as a decorator on Django TestCase classes or methods.
|
||||
|
||||
This is needed because codejail has significant OS-level setup requirements
|
||||
which we don't even attempt to fulfill for unit testing purposes. Running
|
||||
tests in unsafe mode (that is, running code executions in-process, with no
|
||||
sandboxing) is only safe because we control the contents of the unit tests.
|
||||
It's not a perfect replica of how safe mode operates but it's generally good
|
||||
enough for testing the integration and overall behavior.
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
self.old_be_unsafe = None
|
||||
super().__init__()
|
||||
|
||||
def enable(self):
|
||||
self.old_be_unsafe = codejail.safe_exec.ALWAYS_BE_UNSAFE
|
||||
codejail.safe_exec.ALWAYS_BE_UNSAFE = True
|
||||
|
||||
def disable(self):
|
||||
codejail.safe_exec.ALWAYS_BE_UNSAFE = self.old_be_unsafe
|
||||
|
||||
@@ -37,6 +37,7 @@ from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, Stude
|
||||
from xmodule.capa.xqueue_interface import XQueueInterface
|
||||
from xmodule.capa_block import ComplexEncoder, ProblemBlock
|
||||
from xmodule.tests import DATA_DIR
|
||||
from xmodule.capa.tests.test_util import use_unsafe_codejail
|
||||
|
||||
from ..capa_block import RANDOMIZATION, SHOWANSWER
|
||||
from . import get_test_system
|
||||
@@ -3635,6 +3636,7 @@ class ComplexEncoderTest(unittest.TestCase): # lint-amnesty, pylint: disable=mi
|
||||
|
||||
|
||||
@skip_unless_lms
|
||||
@use_unsafe_codejail()
|
||||
class ProblemCheckTrackingTest(unittest.TestCase):
|
||||
"""
|
||||
Ensure correct tracking information is included in events emitted during problem checks.
|
||||
|
||||
Reference in New Issue
Block a user