openedx / openedx/codejail

Stop using global state to store configuration options

Open
#159 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

code health
Dominant language
Python
Stars
479
Forks
84
Avg merge
16h 34m
Merged PRs (30d)
2

Description

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(...)
    

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading codejail.jail_code.COMMANDS and configure, then inspect the codejail.django_integration middleware that populates the configuration. Decide on and implement a non-global configuration design, with repeated configuration independent of unit-test order and behavior consistent across Python processes.

Written by the indexing model from the issue text.

Assessment

Tech stack
django, python
Domain
backend, security
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.