From 48d59e9e9cc378de8c5c79268428c733ab3ebfaf Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 9 May 2025 17:14:52 -0400 Subject: [PATCH 1/4] 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. --- README.rst | 10 ++++--- codejail/safe_exec.py | 48 ++++++++++++++++++-------------- codejail/tests/test_safe_exec.py | 4 +-- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/README.rst b/README.rst index 6a6fe362..987bf7e0 100644 --- a/README.rst +++ b/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 diff --git a/codejail/safe_exec.py b/codejail/safe_exec.py index 18e034c3..7a397c68 100644 --- a/codejail/safe_exec.py +++ b/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) diff --git a/codejail/tests/test_safe_exec.py b/codejail/tests/test_safe_exec.py index 97e09af2..37887a5e 100644 --- a/codejail/tests/test_safe_exec.py +++ b/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): From 5882ed625ec3669a6bc3af89f54f8ced598e8b61 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Tue, 13 May 2025 17:10:10 -0400 Subject: [PATCH 2/4] test: Fixes from review Code: - Make log message scarier Docs: - Remove now-incorrect description of falling back to unsafe mode - Add bold warning - Expand on AppArmor requirements Tests: - Add tests for code changes. - Remove auto-skip based on ALWAYS_BE_UNSAFE, which no longer gets computed at module load time; it will always be false unless explicitly set. Also, add some docs around how testing works. --- README.rst | 15 ++++---------- codejail/jail_code.py | 2 +- codejail/safe_exec.py | 2 +- codejail/tests/README.rst | 10 ++++++++++ codejail/tests/test_safe_exec.py | 34 ++++++++++++++++++++++++++------ 5 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 codejail/tests/README.rst diff --git a/README.rst b/README.rst index 987bf7e0..23267b67 100644 --- a/README.rst +++ b/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: @@ -71,7 +64,7 @@ CodeJail can execute Python code safely. However, it is also possible to set 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. +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 diff --git a/codejail/jail_code.py b/codejail/jail_code.py index b00d6164..33b2a7fe 100644 --- a/codejail/jail_code.py +++ b/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: diff --git a/codejail/safe_exec.py b/codejail/safe_exec.py index 7a397c68..2aac4766 100644 --- a/codejail/safe_exec.py +++ b/codejail/safe_exec.py @@ -280,7 +280,7 @@ def not_safe_exec( # 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) + log.warning("DANGER: Executing code with `not_safe_exec` for %s", slug) g_dict = json_safe(globals_dict) diff --git a/codejail/tests/README.rst b/codejail/tests/README.rst new file mode 100644 index 00000000..3e69af5e --- /dev/null +++ b/codejail/tests/README.rst @@ -0,0 +1,10 @@ +Unit tests +========== + +Some of these tests are dependent on environment variables, AppArmor, sudoers, another system account, and additional virtualenvs. You may prefer to just rely on the GitHub Actions CI on your PR rather than setting up your local machine correctly. + +There are not yet directions for local setup, but here are some breadcrumbs to follow: + +- ``.github/workflows/ci.yml`` installs an AppArmor profile (this has to be done outside of any Docker container), builds a Dockerimage (see ``Dockerfile``), and runs tests via Makefile commands. +- The Make targets set environment variables for testing proxy vs. non-proxy setups, but otherwise run pytest directly. +- Codejail "magically" detects the presence of a sandbox virtualenv based on directory naming and auto-configures itself, but can fall back to environment variables ``CODEJAIL_TEST_USER`` and ``CODEJAIL_TEST_VENV``. See code around ``running_in_virtualenv`` in ``codejail/jail_code.py``. diff --git a/codejail/tests/test_safe_exec.py b/codejail/tests/test_safe_exec.py index 37887a5e..c3a1e043 100644 --- a/codejail/tests/test_safe_exec.py +++ b/codejail/tests/test_safe_exec.py @@ -5,6 +5,9 @@ import zipfile from io import BytesIO from unittest import SkipTest, 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 will actually just call not_safe_exec, then there's no - # point running these tests. - if safe_exec.ALWAYS_BE_UNSAFE: # pragma: no cover - raise SkipTest - def safe_exec(self, *args, **kwargs): safe_exec.not_safe_exec(*args, **kwargs) From 57c8498cdc5f4e73abcbab80d6bd9fde8497a297 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Tue, 13 May 2025 17:18:42 -0400 Subject: [PATCH 3/4] fixup! Clarify testing instructions; fix lint --- README.rst | 9 +++------ codejail/tests/README.rst | 10 ---------- codejail/tests/test_safe_exec.py | 2 +- 3 files changed, 4 insertions(+), 17 deletions(-) delete mode 100644 codejail/tests/README.rst diff --git a/README.rst b/README.rst index 23267b67..1290899a 100644 --- a/README.rst +++ b/README.rst @@ -207,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= $ export CODEJAIL_TEST_VENV= @@ -217,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 ------ diff --git a/codejail/tests/README.rst b/codejail/tests/README.rst deleted file mode 100644 index 3e69af5e..00000000 --- a/codejail/tests/README.rst +++ /dev/null @@ -1,10 +0,0 @@ -Unit tests -========== - -Some of these tests are dependent on environment variables, AppArmor, sudoers, another system account, and additional virtualenvs. You may prefer to just rely on the GitHub Actions CI on your PR rather than setting up your local machine correctly. - -There are not yet directions for local setup, but here are some breadcrumbs to follow: - -- ``.github/workflows/ci.yml`` installs an AppArmor profile (this has to be done outside of any Docker container), builds a Dockerimage (see ``Dockerfile``), and runs tests via Makefile commands. -- The Make targets set environment variables for testing proxy vs. non-proxy setups, but otherwise run pytest directly. -- Codejail "magically" detects the presence of a sandbox virtualenv based on directory naming and auto-configures itself, but can fall back to environment variables ``CODEJAIL_TEST_USER`` and ``CODEJAIL_TEST_VENV``. See code around ``running_in_virtualenv`` in ``codejail/jail_code.py``. diff --git a/codejail/tests/test_safe_exec.py b/codejail/tests/test_safe_exec.py index c3a1e043..d7eb5813 100644 --- a/codejail/tests/test_safe_exec.py +++ b/codejail/tests/test_safe_exec.py @@ -4,7 +4,7 @@ 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 51540bc06c75fb4c30e8b5d980ce04896f609592 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Thu, 15 May 2025 23:00:37 -0400 Subject: [PATCH 4/4] fixup! wording tweak --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 1290899a..d18b983a 100644 --- a/README.rst +++ b/README.rst @@ -252,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.