diff --git a/common/djangoapps/util/sandboxing.py b/common/djangoapps/util/sandboxing.py index 2024f8fa27..e3ec2eef23 100644 --- a/common/djangoapps/util/sandboxing.py +++ b/common/djangoapps/util/sandboxing.py @@ -16,7 +16,12 @@ def can_execute_unsafe_code(course_id): # a list of regexes configured on the server. # If this is not defined in the environment variables then default to the most restrictive, which # is 'no unsafe courses' + # TODO: This should be a database configuration, where we can mark individual courses as being + # safe/unsafe. Someone in the future should switch us over to that rather than using regexes + # in a settings file + # To others using this: the code as-is is brittle and likely to be changed in the future, + # as per the TODO, so please consider carefully before adding more values to COURSES_WITH_UNSAFE_CODE for regex in getattr(settings, 'COURSES_WITH_UNSAFE_CODE', []): - if re.match(regex, course_id): + if re.match(regex, course_id.to_deprecated_string()): return True return False diff --git a/common/djangoapps/util/tests/test_sandboxing.py b/common/djangoapps/util/tests/test_sandboxing.py index c76132696a..55daa6681c 100644 --- a/common/djangoapps/util/tests/test_sandboxing.py +++ b/common/djangoapps/util/tests/test_sandboxing.py @@ -5,6 +5,7 @@ Tests for sandboxing.py in util app from django.test import TestCase from util.sandboxing import can_execute_unsafe_code from django.test.utils import override_settings +from xmodule.modulestore.locations import SlashSeparatedCourseKey class SandboxingTest(TestCase): @@ -16,19 +17,19 @@ class SandboxingTest(TestCase): """ Test to make sure that a non-match returns false """ - self.assertFalse(can_execute_unsafe_code('edX/notful/empty')) + self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'notful', 'empty'))) @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/full/.*']) def test_sandbox_inclusion(self): """ Test to make sure that a match works across course runs """ - self.assertTrue(can_execute_unsafe_code('edX/full/2012_Fall')) - self.assertTrue(can_execute_unsafe_code('edX/full/2013_Spring')) + self.assertTrue(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2012_Fall'))) + self.assertTrue(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2013_Spring'))) def test_courses_with_unsafe_code_default(self): """ Test that the default setting for COURSES_WITH_UNSAFE_CODE is an empty setting, e.g. we don't use @override_settings in these tests """ - self.assertFalse(can_execute_unsafe_code('edX/full/2012_Fall')) - self.assertFalse(can_execute_unsafe_code('edX/full/2013_Spring')) + self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2012_Fall'))) + self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2013_Spring')))