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

README.rst

@@ -6,18 +6,11 @@ designed primarily for Python execution, but can be used for other languages as
 well.
 
 Security is enforced with AppArmor.  If your operating system doesn't support
-AppArmor, then CodeJail won't protect the execution.
+AppArmor, or if the AppArmor profile is not defined and configured correctly,
+then CodeJail will not protect the execution.
 
 CodeJail is designed to be configurable, and will auto-configure itself for
-Python execution if you install it properly.  The configuration is designed to
-be flexible: it can run in safe mode or unsafe mode.  This helps support large
-development groups where only some of the developers are involved enough with
-secure execution to configure AppArmor on their development machines.
-
-If CodeJail is not configured for safe execution, it will execution Python
-using the same API, but will not guard against malicious code.  This allows the
-same code to be used on safe-configured or non-safe-configured developer's
-machines.
+Python execution if you install it properly.
 
 A CodeJail sandbox consists of several pieces:
 
@@ -66,10 +59,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. **Do not use this option in production systems.**
 
 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
@@ -212,8 +207,8 @@ the rights to modify the files in its site-packages directory.
 Tests
 -----
 
-In order to target the sandboxed Python environment(s) you have created on your
-system, you must set the following environment variables for testing::
+To run tests, you must perform the standard installation steps. Then
+you must set the following environment variables::
 
     $ export CODEJAIL_TEST_USER=<owner of sandbox (usually 'sandbox')>
     $ export CODEJAIL_TEST_VENV=<SANDENV>
@@ -222,10 +217,7 @@ Run the tests with the Makefile::
 
     $ make tests
 
-If CodeJail is running unsafely, many of the tests will be automatically
-skipped, or will fail, depending on whether CodeJail thinks it should be in
-safe mode or not.
-
+Several proxy tests are skipped if proxy mode is not configured.
 
 Design
 ------
@@ -260,7 +252,7 @@ the subprocess as JSON.
 Limitations
 -----------
 
-* If codejail or AppArmor is not configured properly, codejail will default to
+* If codejail or AppArmor is not configured properly, codejail may default to
   running code insecurely (no sandboxing). It is not secure by default.
 * Sandbox isolation is achieved via AppArmor confinement. Codejail facilitates
   this, but cannot isolate execution without the use of AppArmor.

codejail/jail_code.py

@@ -68,7 +68,7 @@ def is_configured(command):
 )
 
 if running_in_virtualenv:
-    # On jenkins
+    # In test environment
     sandbox_user = os.getenv('CODEJAIL_TEST_USER')
     sandbox_env = os.getenv('CODEJAIL_TEST_VENV')
     if sandbox_env and sandbox_user:

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("DANGER: Executing code with `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

@@ -4,7 +4,10 @@
 import textwrap
 import zipfile
 from io import BytesIO
-from unittest import SkipTest, TestCase
+from unittest import TestCase
+from unittest.mock import patch
+
+import pytest
 
 from codejail import safe_exec
 from codejail.jail_code import set_limit
@@ -176,17 +179,36 @@ class TestSafeExec(SafeExecTests, TestCase):
     def safe_exec(self, *args, **kwargs):
         safe_exec.safe_exec(*args, **kwargs)
 
+    @patch('codejail.jail_code.is_configured', return_value=False)
+    def test_configuration(self, mock_is_configured):
+        """
+        When not configured for Python, raise an exception.
+        """
+        with pytest.raises(RuntimeError, match=r'safe_exec has not been configured for Python'):
+            self.safe_exec('out = 1 + 2', {})
+
+        mock_is_configured.assert_called_once_with('python')
+
+    @patch('codejail.safe_exec.not_safe_exec')
+    @patch('codejail.jail_code.is_configured', return_value=True)
+    def test_opt_unsafe(self, mock_is_configured, mock_not_safe_exec):
+        """
+        When ALWAYS_BE_UNSAFE enabled, go immediately to not_safe_exec.
+        """
+        with patch.object(safe_exec, 'ALWAYS_BE_UNSAFE', new=True):
+            self.safe_exec('out = 1 + 2', {})
+
+        mock_is_configured.assert_not_called()
+        mock_not_safe_exec.assert_called_once_with(
+            'out = 1 + 2', {},
+            files=None, python_path=None, limit_overrides_context=None, slug=None, extra_files=None,
+        )
+
 
 class TestNotSafeExec(SafeExecTests, TestCase):
     """Run SafeExecTests, with not_safe_exec."""
 
     __test__ = True
 
-    def setUp(self):
-        # If safe_exec is actually an alias to not_safe_exec, then there's no
-        # point running these tests.
-        if safe_exec.UNSAFE:                    # pragma: no cover
-            raise SkipTest
-
     def safe_exec(self, *args, **kwargs):
         safe_exec.not_safe_exec(*args, **kwargs)