From 447fd0b6cbbb5a517d3490fee36a1c901c7e8a4d Mon Sep 17 00:00:00 2001 From: Tim McCormack <59623490+timmc-edx@users.noreply.github.com> Date: Tue, 17 Jun 2025 09:33:52 -0400 Subject: [PATCH] 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 --- .../tests/test_submitting_problems.py | 2 ++ .../instructor_task/tests/test_integration.py | 2 ++ requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 3 ++- requirements/edx/testing.txt | 2 +- .../capa/safe_exec/tests/test_safe_exec.py | 4 +++ xmodule/capa/tests/test_capa_problem.py | 3 +++ xmodule/capa/tests/test_html_render.py | 2 ++ xmodule/capa/tests/test_responsetypes.py | 10 +++++++ xmodule/capa/tests/test_util.py | 27 +++++++++++++++++++ xmodule/tests/test_capa_block.py | 2 ++ 13 files changed, 58 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 4d55645d7a..4d04204078 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -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. diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index 004cba1cda..267f8021cd 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -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. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index b33ae20909..c1809ae42d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -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 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 472e969eb0..b928da2b66 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -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 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index dd850e55db..b5413f24f4 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.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 diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index caec5c8c04..fcc502755c 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -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 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 9f13477279..8e0301ac7e 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -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 diff --git a/xmodule/capa/safe_exec/tests/test_safe_exec.py b/xmodule/capa/safe_exec/tests/test_safe_exec.py index d09f8c9d9b..d7679b66aa 100644 --- a/xmodule/capa/safe_exec/tests/test_safe_exec.py +++ b/xmodule/capa/safe_exec/tests/test_safe_exec.py @@ -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("""\ diff --git a/xmodule/capa/tests/test_capa_problem.py b/xmodule/capa/tests/test_capa_problem.py index 74cf4d096f..5f42b91849 100644 --- a/xmodule/capa/tests/test_capa_problem.py +++ b/xmodule/capa/tests/test_capa_problem.py @@ -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 """ diff --git a/xmodule/capa/tests/test_html_render.py b/xmodule/capa/tests/test_html_render.py index 0af5f1198e..46ad47d79a 100644 --- a/xmodule/capa/tests/test_html_render.py +++ b/xmodule/capa/tests/test_html_render.py @@ -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. diff --git a/xmodule/capa/tests/test_responsetypes.py b/xmodule/capa/tests/test_responsetypes.py index fa0c97fb15..ca9f5eba59 100644 --- a/xmodule/capa/tests/test_responsetypes.py +++ b/xmodule/capa/tests/test_responsetypes.py @@ -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. diff --git a/xmodule/capa/tests/test_util.py b/xmodule/capa/tests/test_util.py index 92bc039cfa..3176ff9b9a 100644 --- a/xmodule/capa/tests/test_util.py +++ b/xmodule/capa/tests/test_util.py @@ -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 diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index b48aa10ef8..21582e55ea 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -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.