From 23cd8525daf706c7e45ed65a2b5d89fd4dc719d0 Mon Sep 17 00:00:00 2001 From: rudi193-cmd Date: Mon, 15 Jun 2026 13:43:40 -0600 Subject: [PATCH] feat: introduce CodeJailConfig class; keep module-level backward compat Addresses openedx/codejail#159. * Add class encapsulating COMMANDS, LIMITS, and LIMIT_OVERRIDES plus configure/set_limit/override_limit/ get_effective_limits methods. * Expose a module-level instance so independent test fixtures and multi-tenant callers can carry isolated state without touching global dicts. * Keep the existing module-level names (COMMANDS, LIMITS, LIMIT_OVERRIDES) as aliases pointing at the same dict objects inside _default_config, so all existing callers continue to work unchanged. * Update to accept an optional argument; falls back to when omitted. * Update to accept an optional argument with the same fallback. * Update to mutate dicts in-place instead of rebinding module-level names (required now that jail_code() reads from _default_config, not the module-level alias). * Add unit-test class with 9 tests covering isolation, configuration, limit overrides, and PROXY guard. Co-Authored-By: Claude Sonnet 4.6 --- codejail/django_integration_utils.py | 16 +- codejail/jail_code.py | 211 ++++++++++++++++++--------- codejail/tests/test_jail_code.py | 69 ++++++++- codejail/tests/util.py | 37 +++-- 4 files changed, 248 insertions(+), 85 deletions(-) diff --git a/codejail/django_integration_utils.py b/codejail/django_integration_utils.py index 2fb0b5137..3d49c288f 100644 --- a/codejail/django_integration_utils.py +++ b/codejail/django_integration_utils.py @@ -7,24 +7,30 @@ from . import jail_code -def apply_django_settings(code_jail_settings): +def apply_django_settings(code_jail_settings, config=None): """ - Apply a settings.CODE_JAIL dictionary to the `jail_code` module. + Apply a settings.CODE_JAIL dictionary to a :class:`~codejail.jail_code.CodeJailConfig`. + + ``config`` is an optional :class:`~codejail.jail_code.CodeJailConfig` instance. + When omitted the module-level default config (``jail_code._default_config``) is + used, which preserves the original behaviour. Pass an explicit instance in tests + or multi-tenant scenarios where you need isolated configuration. """ + cfg = config if config is not None else jail_code._default_config python_bin = code_jail_settings.get('python_bin') if python_bin: user = code_jail_settings['user'] - jail_code.configure("python", python_bin, user=user) + cfg.configure("python", python_bin, user=user) limits = code_jail_settings.get('limits', {}) for name, value in limits.items(): - jail_code.set_limit( + cfg.set_limit( limit_name=name, value=value, ) limit_overrides = code_jail_settings.get('limit_overrides', {}) for context, overrides in limit_overrides.items(): for name, value in overrides.items(): - jail_code.override_limit( + cfg.override_limit( limit_name=name, value=value, limit_overrides_context=context, diff --git a/codejail/jail_code.py b/codejail/jail_code.py index 33b2a7fef..f83e8ef61 100644 --- a/codejail/jail_code.py +++ b/codejail/jail_code.py @@ -15,11 +15,133 @@ # TODO: limit too much stdout data? # pylint: disable=fixme -# Configure the commands -# COMMANDS is a map from an abstract command name to a list of command-line -# pieces, such as subprocess.Popen wants. -COMMANDS = {} +# --------------------------------------------------------------------------- +# Default resource limits (used by CodeJailConfig.__init__) +# --------------------------------------------------------------------------- + +DEFAULT_LIMITS = { + # CPU seconds, defaulting to 1. + "CPU": 1, + # Real time, defaulting to 1 second. + "REALTIME": 1, + # Total process virutal memory, in bytes, defaulting to unlimited. + "VMEM": 0, + # Size of files creatable, in bytes, defaulting to nothing can be written. + "FSIZE": 0, + # The number of processes and threads to allow for the sandbox user (total + # across entire host). + "NPROC": 15, + # Whether to use a proxy process or not. None means use an environment + # variable to decide. NOTE: using a proxy process is NOT THREAD-SAFE, only + # one thread can use CodeJail at a time if you are using a proxy process. + "PROXY": None, +} + + +# --------------------------------------------------------------------------- +# CodeJailConfig — encapsulates all mutable codejail state +# --------------------------------------------------------------------------- + +class CodeJailConfig: + """ + Encapsulate codejail configuration: command paths, resource limits, and + per-context limit overrides. + + Using instances of this class instead of the module-level globals allows + independent configurations to coexist in the same process — for example, + isolated test fixtures that do not affect each other or production state. + + The module-level helper functions (``configure``, ``is_configured``, + ``set_limit``, etc.) all delegate to the module-level ``_default_config`` + instance and remain fully backward-compatible. + """ + + def __init__(self): + # Map from abstract command name → {'cmdline_start': [...], 'user': ...} + self.COMMANDS = {} + # Active resource limits for this configuration. + self.LIMITS = DEFAULT_LIMITS.copy() + # Per-context limit overrides: {context_str: {limit_name: value}} + self.LIMIT_OVERRIDES = {} + + def configure(self, command, bin_path, user=None): + """ + Configure a command for ``jail_code`` to use. + + ``command`` is the abstract command you're configuring, such as + "python" or "node". ``bin_path`` is the path to the binary. + ``user``, if provided, is the user name to run the command under. + """ + cmd_argv = [bin_path] + if command == "python": + # -E means ignore the environment variables PYTHON* + # -B means don't try to write .pyc files. + cmd_argv.extend(['-E', '-B']) + self.COMMANDS[command] = { + 'cmdline_start': cmd_argv, + 'user': user, + } + + def is_configured(self, command): + """Return True if this config has been set up for ``command``.""" + return command in self.COMMANDS + + def set_limit(self, limit_name, value): + """ + Set a resource limit on this configuration. + + See the module-level ``set_limit`` docstring for the full list of + recognised limit names and their semantics. + """ + self.LIMITS[limit_name] = value + + def get_effective_limits(self, overrides_context=None): + """ + Return the effective limits dict, merging any context-specific overrides. + """ + overrides = ( + self.LIMIT_OVERRIDES.get(overrides_context, {}) + if overrides_context + else {} + ) + return {**self.LIMITS, **overrides} + + def override_limit(self, limit_name, value, limit_overrides_context): + """ + Override a limit for a specific context on this configuration. + + See the module-level ``override_limit`` docstring for semantics. + """ + if limit_name == 'PROXY' and self.LIMITS['PROXY'] != value: + log.error( + 'Tried to override value of PROXY to %s. ' + 'Overriding PROXY on a per-context basis is not permitted. ' + 'Will use globally-configured value instead: %s.', + value, + self.LIMITS['PROXY'], + ) + return + if limit_overrides_context not in self.LIMIT_OVERRIDES: + self.LIMIT_OVERRIDES[limit_overrides_context] = {} + self.LIMIT_OVERRIDES[limit_overrides_context][limit_name] = value + + +# --------------------------------------------------------------------------- +# Module-level default config instance +# --------------------------------------------------------------------------- + +#: The default configuration used by all module-level helper functions. +#: Callers that need isolated state should create their own ``CodeJailConfig`` +#: and pass it as ``config=`` to ``jail_code()``. +_default_config = CodeJailConfig() + +# Module-level aliases kept for backward compatibility. These names point to +# the *same dict objects* held by ``_default_config``, so direct mutations via +# either path stay in sync. +COMMANDS = _default_config.COMMANDS +LIMITS = _default_config.LIMITS +LIMIT_OVERRIDES = _default_config.LIMIT_OVERRIDES def configure(command, bin_path, user=None): @@ -31,20 +153,7 @@ def configure(command, bin_path, user=None): the user name to run the command under. """ - cmd_argv = [bin_path] - - # Command-specific arguments - if command == "python": - # -E means ignore the environment variables PYTHON* - # -B means don't try to write .pyc files. - cmd_argv.extend(['-E', '-B']) - - COMMANDS[command] = { - # The start of the command line for this program. - 'cmdline_start': cmd_argv, - # The user to run this as, perhaps None. - 'user': user, - } + _default_config.configure(command, bin_path, user) def is_configured(command): @@ -55,9 +164,10 @@ def is_configured(command): in the `jail_code` function. """ - return command in COMMANDS + return _default_config.is_configured(command) +# --------------------------------------------------------------------------- # By default, look where our current Python is, and maybe there's a # python-sandbox alongside. Only do this if running in a virtualenv. # The check for sys.real_prefix covers virtualenv @@ -78,34 +188,6 @@ def is_configured(command): configure("python", sys.prefix + "-sandbox/bin/python", "sandbox") -# The resource limits that we unless otherwise configured. -DEFAULT_LIMITS = { - # CPU seconds, defaulting to 1. - "CPU": 1, - # Real time, defaulting to 1 second. - "REALTIME": 1, - # Total process virutal memory, in bytes, defaulting to unlimited. - "VMEM": 0, - # Size of files creatable, in bytes, defaulting to nothing can be written. - "FSIZE": 0, - # The number of processes and threads to allow for the sandbox user (total - # across entire host). - "NPROC": 15, - # Whether to use a proxy process or not. None means use an environment - # variable to decide. NOTE: using a proxy process is NOT THREAD-SAFE, only - # one thread can use CodeJail at a time if you are using a proxy process. - "PROXY": None, -} - -# Configured resource limits. -# Modified by calling `set_limit`. -LIMITS = DEFAULT_LIMITS.copy() - -# Map from limit_overrides_contexts (strings) to dictionaries in the shape of LIMITS. -# Modified by calling `override_limit`. -LIMIT_OVERRIDES = {} - - def set_limit(limit_name, value): """ Set a limit for `jail_code`. @@ -141,7 +223,7 @@ def set_limit(limit_name, value): Providing a limit of 0 will disable that limit, unless otherwise specified. """ - LIMITS[limit_name] = value + _default_config.set_limit(limit_name, value) def get_effective_limits(overrides_context=None): @@ -152,8 +234,7 @@ def get_effective_limits(overrides_context=None): overrides_context (str|None): Identifies which set of overrides to use. If None or missing from `LIMIT_OVERRIDES`, then just return `LIMITS` as-is. """ - overrides = LIMIT_OVERRIDES.get(overrides_context, {}) if overrides_context else {} - return {**LIMITS, **overrides} + return _default_config.get_effective_limits(overrides_context) def override_limit(limit_name, value, limit_overrides_context): @@ -166,18 +247,7 @@ def override_limit(limit_name, value, limit_overrides_context): executions of code is not supported. If one attempts to override PROXY, the override will be ignored and the globally-configured value will be used instead. """ - if limit_name == 'PROXY' and LIMITS['PROXY'] != value: - log.error( - 'Tried to override value of PROXY to %s. ' - 'Overriding PROXY on a per-context basis is not permitted. ' - 'Will use globally-configured value instead: %s.', - value, - LIMITS['PROXY'], - ) - return - if limit_overrides_context not in LIMIT_OVERRIDES: - LIMIT_OVERRIDES[limit_overrides_context] = {} - LIMIT_OVERRIDES[limit_overrides_context][limit_name] = value + _default_config.override_limit(limit_name, value, limit_overrides_context) class JailResult: @@ -190,7 +260,7 @@ def __init__(self): # pylint: disable=too-many-positional-arguments def jail_code(command, code=None, files=None, extra_files=None, argv=None, - stdin=None, limit_overrides_context=None, slug=None): + stdin=None, limit_overrides_context=None, slug=None, config=None): """ Run code in a jailed subprocess. @@ -224,6 +294,11 @@ def jail_code(command, code=None, files=None, extra_files=None, argv=None, `slug` is an arbitrary string, a description that's meaningful to the caller, that will be used in log messages. + `config` is an optional :class:`CodeJailConfig` instance. When provided, + that instance's commands and limits are used instead of the module-level + defaults. This allows isolated test fixtures and multi-tenant scenarios to + coexist without sharing global state. + Return an object with: .stdout: stdout of the program, a string @@ -233,7 +308,9 @@ def jail_code(command, code=None, files=None, extra_files=None, argv=None, """ # pylint: disable=too-many-statements - if not is_configured(command): + cfg = config if config is not None else _default_config + + if not cfg.is_configured(command): # pylint: disable=broad-exception-raised raise Exception("jail_code needs to be configured for %r" % command) @@ -281,7 +358,7 @@ def jail_code(command, code=None, files=None, extra_files=None, argv=None, rm_cmd = [] # Build the command to run. - user = COMMANDS[command]['user'] + user = cfg.COMMANDS[command]['user'] if user: # Run as the specified user cmd.extend(['sudo', '-u', user]) @@ -292,13 +369,13 @@ def jail_code(command, code=None, files=None, extra_files=None, argv=None, # Issue: https://github.com/openedx/codejail/issues/162 cmd.extend(['TMPDIR=tmp']) # Start with the command line dictated by "python" or whatever. - cmd.extend(COMMANDS[command]['cmdline_start']) + cmd.extend(cfg.COMMANDS[command]['cmdline_start']) # Add the code-specific command line pieces. cmd.extend(argv) # Determine effective resource limits. - effective_limits = get_effective_limits(limit_overrides_context) + effective_limits = cfg.get_effective_limits(limit_overrides_context) if slug: log.info( "Preparing to execute jailed code %r " diff --git a/codejail/tests/test_jail_code.py b/codejail/tests/test_jail_code.py index f11fde9ed..74ed40c30 100644 --- a/codejail/tests/test_jail_code.py +++ b/codejail/tests/test_jail_code.py @@ -11,7 +11,7 @@ from unittest import SkipTest, TestCase, mock from codejail import proxy -from codejail.jail_code import LIMITS, is_configured, jail_code, set_limit +from codejail.jail_code import LIMITS, CodeJailConfig, is_configured, jail_code, set_limit def jailpy(code=None, *args, **kwargs): # pylint: disable=keyword-arg-before-vararg @@ -659,3 +659,70 @@ def test_crash_proxy(self): pid = proxy.PROXY_PROCESS.pid self.assertNotIn(pid, pids) pids.add(pid) + + +class TestCodeJailConfig(TestCase): + """Unit tests for the CodeJailConfig class. + + These tests do not require a sandbox environment — they exercise the + configuration object in isolation without spawning any subprocesses. + """ + + def test_default_limits_are_copied(self): + # Two independent instances must not share the same LIMITS dict. + cfg1 = CodeJailConfig() + cfg2 = CodeJailConfig() + cfg1.set_limit('CPU', 999) + self.assertNotEqual(cfg2.LIMITS['CPU'], 999) + + def test_configure_and_is_configured(self): + cfg = CodeJailConfig() + self.assertFalse(cfg.is_configured('python')) + cfg.configure('python', '/usr/bin/python3', user='sandbox') + self.assertTrue(cfg.is_configured('python')) + self.assertEqual(cfg.COMMANDS['python']['user'], 'sandbox') + self.assertIn('-E', cfg.COMMANDS['python']['cmdline_start']) + self.assertIn('-B', cfg.COMMANDS['python']['cmdline_start']) + + def test_configure_non_python_no_extra_flags(self): + cfg = CodeJailConfig() + cfg.configure('node', '/usr/bin/node') + self.assertEqual(cfg.COMMANDS['node']['cmdline_start'], ['/usr/bin/node']) + + def test_set_limit_and_get_effective_limits(self): + cfg = CodeJailConfig() + cfg.set_limit('CPU', 5) + limits = cfg.get_effective_limits() + self.assertEqual(limits['CPU'], 5) + + def test_get_effective_limits_with_no_overrides_context(self): + cfg = CodeJailConfig() + limits = cfg.get_effective_limits() + self.assertEqual(set(limits.keys()), {'CPU', 'REALTIME', 'VMEM', 'FSIZE', 'NPROC', 'PROXY'}) + + def test_override_limit_applies_in_context(self): + cfg = CodeJailConfig() + cfg.set_limit('CPU', 1) + cfg.override_limit('CPU', 10, 'my_context') + self.assertEqual(cfg.get_effective_limits('my_context')['CPU'], 10) + self.assertEqual(cfg.get_effective_limits()['CPU'], 1) + + def test_override_limit_unknown_context_returns_base(self): + cfg = CodeJailConfig() + cfg.set_limit('CPU', 2) + self.assertEqual(cfg.get_effective_limits('nonexistent')['CPU'], 2) + + def test_proxy_override_is_rejected(self): + cfg = CodeJailConfig() + cfg.set_limit('PROXY', 0) + cfg.override_limit('PROXY', 1, 'ctx') + # PROXY override must be silently ignored. + self.assertNotIn('PROXY', cfg.LIMIT_OVERRIDES.get('ctx', {})) + + def test_instances_are_isolated_from_module_globals(self): + # Mutating an independent instance must not touch the module-level LIMITS. + from codejail import jail_code as jc + original_cpu = jc.LIMITS['CPU'] + cfg = CodeJailConfig() + cfg.set_limit('CPU', 42) + self.assertEqual(jc.LIMITS['CPU'], original_cpu) diff --git a/codejail/tests/util.py b/codejail/tests/util.py index c80d45f0f..4ffce87ff 100644 --- a/codejail/tests/util.py +++ b/codejail/tests/util.py @@ -7,10 +7,17 @@ class ResetJailCodeStateMixin: """ - The jail_code module has global state. + The jail_code module has global state held in ``_default_config``. - Use this mixin to reset jail_code to its initial state before running a test function, - and then restore the existing state once the test function is complete. + Use this mixin to reset codejail to its initial state before running a test + function, and then restore the existing state once the test function is + complete. + + The three module-level names ``COMMANDS``, ``LIMITS``, and + ``LIMIT_OVERRIDES`` are backward-compat aliases that reference the *same* + dict objects stored inside ``_default_config``. We therefore mutate those + dicts in-place (``clear`` + ``update``) rather than rebinding the names, so + that both access paths stay consistent throughout the test. """ def setUp(self): @@ -18,19 +25,25 @@ def setUp(self): Reset global variables back to defaults, copying and saving existing values. """ super().setUp() + cfg = jail_code._default_config # pylint: disable=invalid-name - self._COMMANDS = jail_code.COMMANDS - self._LIMITS = jail_code.LIMITS - self._LIMIT_OVERRIDES = jail_code.LIMIT_OVERRIDES - jail_code.COMMANDS = {} - jail_code.LIMITS = jail_code.DEFAULT_LIMITS.copy() - jail_code.LIMIT_OVERRIDES = {} + self._COMMANDS = dict(cfg.COMMANDS) + self._LIMITS = dict(cfg.LIMITS) + self._LIMIT_OVERRIDES = {k: dict(v) for k, v in cfg.LIMIT_OVERRIDES.items()} + cfg.COMMANDS.clear() + cfg.LIMITS.clear() + cfg.LIMITS.update(jail_code.DEFAULT_LIMITS) + cfg.LIMIT_OVERRIDES.clear() def tearDown(self): """ Restore global variables to the values they had before running the test. """ super().setUp() - jail_code.COMMANDS = self._COMMANDS - jail_code.LIMITS = self._LIMITS - jail_code.LIMIT_OVERRIDES = self._LIMIT_OVERRIDES + cfg = jail_code._default_config + cfg.COMMANDS.clear() + cfg.COMMANDS.update(self._COMMANDS) + cfg.LIMITS.clear() + cfg.LIMITS.update(self._LIMITS) + cfg.LIMIT_OVERRIDES.clear() + cfg.LIMIT_OVERRIDES.update(self._LIMIT_OVERRIDES)