fix: Require an explicit opt in to unsafety; defer decision to call time

Codejail currently makes a decision at module load time of whether it
should run all code safely or unsafely, and defaults to unsafely. This
causes several problems:

- Any misconfiguration of codejail (such as a missing Django setting or
  middleware) results in the application becoming immediately and entirely
  vulnerable to anyone who can submit code.
- Codejail's behavior changes depending on when the `codejail.safe_exec`
  module is loaded during application initialization. This causes unstable
  behavior and is confusing for developers.

This change switches the `ALWAYS_BE_UNSAFE` check to occur only at the time
that `safe_exec` is actually called, rather than at module load time.

The check for whether codejail is configured for Python is also moved to
call time, but no longer automatically switches codejail to unsafe mode.
Instead, it raises an exception to notify the user of their error.

Author: timmc-edx

README.rst

@@ -66,10 +66,12 @@ Installation
 ------------
 
 These instructions detail how to configure your operating system so that
-CodeJail can execute Python code safely.  You can run CodeJail without these
-steps, and you will have an unsafe CodeJail.  This is fine for developers'
-machines who are unconcerned with security, and simplifies the integration of
-CodeJail into your project.
+CodeJail can execute Python code safely. However, it is also possible to set
+``codejail.safe_exec.ALWAYS_BE_UNSAFE = True`` and execute submitted Python
+directly on the machine, with no security whatsoever. This may be fine for
+developers' machines who are unconcerned with security, and allows testing
+an integration with CodeJail's API. It must not be used if any input is coming
+from untrusted sources, however.
 
 To secure Python execution, you'll be creating a new virtualenv.  This means
 you'll have two: the main virtualenv for your project, and the new one for

codejail/safe_exec.py

@@ -23,7 +23,13 @@
 
 # Set this to True to log all the code and globals being executed.
 LOG_ALL_CODE = False
-# Set this to True to use the unsafe code, so that you can debug it.
+
+# Set this to True to run submitted code with no confinement and no sandbox.
+#
+# WARNING: This is deeply dangerous; anyone who can submit code can take
+# over the computer immediately and entirely.
+#
+# The only purpose of this setting is for local debugging.
 ALWAYS_BE_UNSAFE = False
 
 
@@ -80,8 +86,22 @@ def safe_exec(
     the code raises an exception, this function will raise `SafeExecException`
     with the stderr of the sandbox process, which usually includes the original
     exception message and traceback.
-
     """
+    if ALWAYS_BE_UNSAFE:
+        not_safe_exec(
+            code,
+            globals_dict,
+            files=files,
+            python_path=python_path,
+            limit_overrides_context=limit_overrides_context,
+            slug=slug,
+            extra_files=extra_files,
+        )
+        return
+
+    if not jail_code.is_configured('python'):
+        raise RuntimeError("safe_exec has not been configured for Python")
+
     the_code = []
 
     files = list(files or ())
@@ -257,6 +277,11 @@ def not_safe_exec(
     Note that `limit_overrides_context` is ignored here, because resource limits
     are not applied.
     """
+    # Because it would be bad if this function were used in production,
+    # let's log a warning when it is used.  Developers can live with
+    # one more log line.
+    log.warning("Using codejail/safe_exec.py:not_safe_exec for %s", slug)
+
     g_dict = json_safe(globals_dict)
 
     with temp_directory() as tmpdir:
@@ -286,22 +311,3 @@ def not_safe_exec(
                 sys.path = original_path
 
     globals_dict.update(json_safe(g_dict))
-
-
-# If the developer wants us to be unsafe (ALWAYS_BE_UNSAFE), or if there isn't
-# a configured jail for Python, then we'll be UNSAFE.
-UNSAFE = ALWAYS_BE_UNSAFE or not jail_code.is_configured("python")
-
-if UNSAFE:   # pragma: no cover
-    # Make safe_exec actually call not_safe_exec, but log that we're doing so.
-
-    def safe_exec(*args, **kwargs):                 # pylint: disable=E0102
-        """An actually-unsafe safe_exec, that warns it's being used."""
-
-        # Because it would be bad if this function were used in production,
-        # let's log a warning when it is used.  Developers can live with
-        # one more log line.
-        slug = kwargs.get('slug', None)
-        log.warning("Using codejail/safe_exec.py:not_safe_exec for %s", slug)
-
-        return not_safe_exec(*args, **kwargs)

codejail/tests/test_safe_exec.py

@@ -183,9 +183,9 @@ class TestNotSafeExec(SafeExecTests, TestCase):
     __test__ = True
 
     def setUp(self):
-        # If safe_exec is actually an alias to not_safe_exec, then there's no
+        # If safe_exec will actually just call not_safe_exec, then there's no
         # point running these tests.
-        if safe_exec.UNSAFE:                    # pragma: no cover
+        if safe_exec.ALWAYS_BE_UNSAFE:                    # pragma: no cover
             raise SkipTest
 
     def safe_exec(self, *args, **kwargs):