codejail
codejail copied to clipboard
Stop using global state to store configuration options
Codejail has a global variable at codejail.jail_code.COMMANDS
. It's a dictionary that maps command names (like "python"
) to safe shell commands. An example value is:
{'python': {'cmdline_start': ['/edx/app/edxapp/venvs/edxapp-sandbox/bin/python', '-E', '-B'], 'user': 'sandbox'}}
The dictionary is populated by calling codejail.jail_code.configure
, which is generally done via the codejail.django_integration
middleware.
This all works fine, but the fact that COMMANDS
is a global variable is not idiomatic Python. More concretely, it's a pain in the neck for unit tests: the order and timing with which unit tests are run will affect whether .configure
has been called or not. This could also cause problems across Python processes in production, leading to situations where codejail is configured in one process but effectively disabled in another process without anyone knowing. Not good.
This stateful system also forces us to use a middleware in order to configure codejail in Django--that's another layer where something could go wrong.
I recommend replacing COMMANDS
with either:
- a stateless function named
load_commands()
, which is safe to call over and over again, which does something like this:try: from django.conf import settings except ImportError, AttributeError settings = {} if settings.get('CODE_JAIL'): return _load_command_from_django_settings(settings) else: # .... load alternative/fallback configuration options, if we're not using Django
- putting the entire Codejail interface behind a class, so clients would need to pass the settings into it, like this:
from django.conf import settings codejail = CodeJail(some_setting=settings.CODEJAIL[...], etc..) codejail.safe_exec(...)