diff --git a/.gitignore b/.gitignore index e3d190b76..6ca5e9fb0 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,6 @@ reports/* # OS detritus *.DS_Store + +# Virtual environment +/venv/ diff --git a/Makefile b/Makefile index be3cf39a6..1e4fc11e8 100644 --- a/Makefile +++ b/Makefile @@ -24,3 +24,8 @@ upgrade: ## update the requirements/*.txt files with the latest packages satisfy pip-compile --upgrade -o requirements/tox.txt requirements/tox.in pip-compile --upgrade -o requirements/testing.txt requirements/testing.in pip-compile --upgrade -o requirements/sandbox.txt requirements/sandbox.in + +quality: ## check coding style with pycodestyle and pylint + pycodestyle codejail *.py + isort --check-only --diff --recursive codejail *.py + pylint codejail *.py diff --git a/codejail/django_integration.py b/codejail/django_integration.py index b75be1083..f5140edcb 100644 --- a/codejail/django_integration.py +++ b/codejail/django_integration.py @@ -4,8 +4,10 @@ """ -from django.core.exceptions import MiddlewareNotUsed +# pylint: skip-file + from django.conf import settings +from django.core.exceptions import MiddlewareNotUsed from django.utils.deprecation import MiddlewareMixin from . import django_integration_utils diff --git a/codejail/jail_code.py b/codejail/jail_code.py index 64b9def06..d7fff4f6f 100644 --- a/codejail/jail_code.py +++ b/codejail/jail_code.py @@ -1,13 +1,11 @@ """Run code in a jail.""" -from __future__ import absolute_import import logging import os import os.path import resource import shutil import sys - from builtins import bytes from .proxy import run_subprocess_through_proxy @@ -16,7 +14,7 @@ log = logging.getLogger("codejail") -# TODO: limit too much stdout data? +# TODO: limit too much stdout data? # pylint: disable=fixme # Configure the commands @@ -60,12 +58,15 @@ def is_configured(command): """ return command in COMMANDS + # 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 # the equality of non-empty sys.base_prefix with sys.prefix covers venv -running_in_virtualenv = (hasattr(sys, 'real_prefix') or - (hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix)) +running_in_virtualenv = ( + hasattr(sys, 'real_prefix') or + (hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix) +) if running_in_virtualenv: # On jenkins @@ -176,7 +177,7 @@ def override_limit(limit_name, value, limit_overrides_context): LIMIT_OVERRIDES[limit_overrides_context][limit_name] = value -class JailResult(object): +class JailResult: """ A passive object for us to return from jail_code. """ diff --git a/codejail/proxy.py b/codejail/proxy.py index ffc5e0400..750d96d29 100644 --- a/codejail/proxy.py +++ b/codejail/proxy.py @@ -1,6 +1,5 @@ """A proxy subprocess-making process for CodeJail.""" -from __future__ import absolute_import import ast import logging import os @@ -8,10 +7,11 @@ import subprocess import sys import time + import six +from six.moves import range from .subproc import run_subprocess -from six.moves import range log = logging.getLogger("codejail") @@ -47,7 +47,7 @@ def deserialize_out(bstr): ## -## Client code, runs in the parent CodeJail process. +# Client code, runs in the parent CodeJail process. ## def run_subprocess_through_proxy(*args, **kwargs): @@ -58,7 +58,7 @@ def run_subprocess_through_proxy(*args, **kwargs): """ last_exception = None - for tries in range(3): + for _tries in range(3): try: proxy = get_proxy() @@ -79,7 +79,7 @@ def run_subprocess_through_proxy(*args, **kwargs): for level, msg, args in log_calls: log.log(level, msg, *args) return status, stdout, stderr - except Exception as e: + except Exception: # pylint: disable=broad-except log.exception("Proxy process failed") # Give the proxy process a chance to die completely if it is dying. time.sleep(.001) @@ -94,8 +94,10 @@ def run_subprocess_through_proxy(*args, **kwargs): # There is one global proxy process. PROXY_PROCESS = None + def get_proxy(): - global PROXY_PROCESS + # pylint: disable=missing-function-docstring + global PROXY_PROCESS # pylint: disable=global-statement # If we had a proxy process, but it died, clean up. if PROXY_PROCESS is not None: @@ -132,7 +134,7 @@ def get_proxy(): return PROXY_PROCESS ## -## Proxy process code +# Proxy process code ## @@ -145,6 +147,9 @@ class CapturingHandler(logging.Handler): the caller, the current exception, the current time, etc. """ + # pylint wants us to override emit(). + # pylint: disable=abstract-method + def __init__(self): super(CapturingHandler, self).__init__() self.log_calls = [] @@ -156,6 +161,7 @@ def handle(self, record): self.log_calls.append((record.levelno, record.msg, record.args)) def get_log_calls(self): + # pylint: disable=missing-function-docstring retval = self.log_calls self.log_calls = [] return retval @@ -194,17 +200,20 @@ def proxy_main(argv): try: while True: stdin = sys.stdin.readline() - log.debug("proxy stdin: %r" % stdin) + log.debug("proxy stdin: %r", stdin) if not stdin: break args, kwargs = deserialize_in(stdin.rstrip()) status, stdout, stderr = run_subprocess(*args, **kwargs) - log.debug("run_subprocess result: status=%r\nstdout=%r\nstderr=%r" % (status, stdout, stderr)) + log.debug( + "run_subprocess result: status=%r\nstdout=%r\nstderr=%r", + status, stdout, stderr, + ) log_calls = capture_log.get_log_calls() stdout = serialize_out((status, stdout, stderr, log_calls)) sys.stdout.write(stdout+"\n") sys.stdout.flush() - except Exception: + except Exception: # pylint: disable=broad-except # Note that this log message will not get back to the parent, because # we are dying and not communicating back to the parent. This will be # useful only if you add another handler at the top of this function. diff --git a/codejail/safe_exec.py b/codejail/safe_exec.py index 015da0e85..6cfec7e19 100644 --- a/codejail/safe_exec.py +++ b/codejail/safe_exec.py @@ -1,21 +1,22 @@ """Safe execution of untrusted Python code.""" -from __future__ import absolute_import +import inspect import logging import os.path import shutil import sys import textwrap + import six -import inspect + +from codejail import jail_code +from codejail.util import change_directory, temp_directory try: import simplejson as json except ImportError: import json -from codejail import jail_code -from codejail.util import temp_directory, change_directory log = logging.getLogger("codejail") @@ -36,11 +37,17 @@ class SafeExecException(Exception): contain the original exception message. """ - pass -def safe_exec(code, globals_dict, files=None, python_path=None, - limit_overrides_context=None, slug=None, extra_files=None): +def safe_exec( + code, + globals_dict, + files=None, + python_path=None, + limit_overrides_context=None, + slug=None, + extra_files=None, +): """ Execute code as "exec" does, but safely. @@ -170,8 +177,9 @@ def json_safe(d): Used to emulate reading data through a serialization straw. """ + # pylint: disable=invalid-name - # six.binary_type is here because bytes are sometimes ok if they represent valid utf8 + # six.binary_type is here because bytes are sometimes ok if they represent valid utf8 # so we consider them valid for now and try to decode them with decode_object. If that # doesn't work they'll get dropped later in the process. ok_types = (type(None), int, float, six.binary_type, six.text_type, list, tuple, dict) @@ -190,21 +198,20 @@ def decode_object(obj): """ if isinstance(obj, bytes): return obj.decode('utf-8') - elif isinstance(obj, (list,tuple)): + if isinstance(obj, (list, tuple)): new_list = [] for i in obj: new_obj = decode_object(i) new_list.append(new_obj) return new_list - elif isinstance(obj, dict): + if isinstance(obj, dict): new_dict = {} - for k,v in six.iteritems(obj): + for k, v in six.iteritems(obj): new_key = decode_object(k) new_value = decode_object(v) new_dict[new_key] = new_value return new_dict - else: - return obj + return obj bad_keys = ("__builtins__",) jd = {} @@ -223,16 +230,22 @@ def decode_object(obj): # Also ensure that the keys encode/decode correctly k = json.loads(json.dumps(decode_object(k))) - except Exception: + except Exception: # pylint: disable=broad-except continue else: jd[k] = v return json.loads(json.dumps(jd)) -def not_safe_exec(code, globals_dict, files=None, python_path=None, - limit_overrides_context=None, # pylint: disable=unused-argument - slug=None, extra_files=None): +def not_safe_exec( + code, + globals_dict, + files=None, + python_path=None, + limit_overrides_context=None, # pylint: disable=unused-argument + slug=None, # pylint: disable=unused-argument + extra_files=None, +): """ Another implementation of `safe_exec`, but not safe. @@ -248,6 +261,7 @@ def not_safe_exec(code, globals_dict, files=None, python_path=None, with temp_directory() as tmpdir: with change_directory(tmpdir): + # pylint: disable=invalid-name # Copy the files here. for filename in files or (): dest = os.path.join(tmpdir, os.path.basename(filename)) @@ -261,7 +275,7 @@ def not_safe_exec(code, globals_dict, files=None, python_path=None, if python_path: sys.path.extend(python_path) try: - exec(code, g_dict) + exec(code, g_dict) # pylint: disable=exec-used except Exception as e: # Wrap the exception in a SafeExecException, but we don't # try here to include the traceback, since this is just a diff --git a/codejail/subproc.py b/codejail/subproc.py index b5db051b9..efce1005e 100644 --- a/codejail/subproc.py +++ b/codejail/subproc.py @@ -1,6 +1,5 @@ """Subprocess helpers for CodeJail.""" -from __future__ import absolute_import import functools import logging import os @@ -13,8 +12,8 @@ def run_subprocess( - cmd, stdin=None, cwd=None, env=None, rlimits=None, realtime=None, - slug=None, + cmd, stdin=None, cwd=None, env=None, rlimits=None, realtime=None, + slug=None, ): """ A helper to make a limited subprocess. @@ -37,11 +36,11 @@ def run_subprocess( the stdout and stderr of the process, as strings. """ - subproc = subprocess.Popen( + subproc = subprocess.Popen( # pylint: disable=subprocess-popen-preexec-fn cmd, cwd=cwd, env=env, preexec_fn=functools.partial(set_process_limits, rlimits or ()), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - ) + ) if slug: log.info("Executed jailed code %s in %s, with PID %s", slug, cwd, subproc.pid) diff --git a/codejail/tests/doit.py b/codejail/tests/doit.py index d23eb6d2c..291456ee4 100644 --- a/codejail/tests/doit.py +++ b/codejail/tests/doit.py @@ -1,5 +1,6 @@ -from __future__ import absolute_import -from __future__ import print_function +# pylint: skip-file isort:skip_file +from __future__ import absolute_import, print_function + import sys print("This is doit.py!") diff --git a/codejail/tests/test_django_integration_utils.py b/codejail/tests/test_django_integration_utils.py index 5e691cb79..e999977bf 100644 --- a/codejail/tests/test_django_integration_utils.py +++ b/codejail/tests/test_django_integration_utils.py @@ -4,7 +4,6 @@ from .. import jail_code from ..django_integration_utils import apply_django_settings - from .util import ResetJailCodeStateMixin diff --git a/codejail/tests/test_jail_code.py b/codejail/tests/test_jail_code.py index bbbc5f399..5cc4ffd41 100644 --- a/codejail/tests/test_jail_code.py +++ b/codejail/tests/test_jail_code.py @@ -1,28 +1,25 @@ """Test jail_code.py""" -from __future__ import absolute_import -from __future__ import print_function -import json import logging import os import os.path import shutil import signal -import textwrap import tempfile +import textwrap import time +from builtins import bytes from unittest import SkipTest, TestCase import mock import six -from builtins import bytes +from six.moves import range -from codejail.jail_code import jail_code, is_configured, set_limit, LIMITS from codejail import proxy -from six.moves import range +from codejail.jail_code import LIMITS, is_configured, jail_code, set_limit -def jailpy(code=None, *args, **kwargs): +def jailpy(code=None, *args, **kwargs): # pylint: disable=keyword-arg-before-vararg """Run `jail_code` on Python.""" if code: code = textwrap.dedent(code) @@ -44,21 +41,21 @@ def text_of_logs(mock_calls): def test_with_log_messages(self, log_log): do_something_that_makes_log_messages() log_text = text_of_logs(log_log.mock_calls) - self.assertRegexpMatches(log_text, r"INFO: Something cool happened") + self.assertRegex(log_text, r"INFO: Something cool happened") """ text = "" - for m in mock_calls: - level, msg, args = m[1] + for call in mock_calls: + level, msg, args = call[1] msg_formatted = msg % args text += "%s: %s\n" % (logging.getLevelName(level), msg_formatted) return text -class JailCodeHelpers(object): +class JailCodeHelpersMixin: """Assert helpers for jail_code tests.""" def setUp(self): - super(JailCodeHelpers, self).setUp() + super(JailCodeHelpersMixin, self).setUp() if not is_configured("python"): raise SkipTest @@ -70,7 +67,7 @@ def assertResultOk(self, res): self.assertEqual(res.status, 0) # pylint: disable=E1101 -class TestFeatures(JailCodeHelpers, TestCase): +class TestFeatures(JailCodeHelpersMixin, TestCase): """Test features of how `jail_code` runs Python.""" def test_hello_world(self): @@ -258,17 +255,17 @@ def test_we_can_remove_tmp_files(self): @mock.patch("codejail.subproc.log._log") def test_slugs_get_logged(self, log_log): - res = jailpy( + jailpy( code=""" from __future__ import print_function; print('Hello, world!') """, slug="HELLO" ) log_text = text_of_logs(log_log.mock_calls) - self.assertRegexpMatches(log_text, r"INFO: Executed jailed code HELLO in .*, with PID .*") + self.assertRegex(log_text, r"INFO: Executed jailed code HELLO in .*, with PID .*") -class TestLimits(JailCodeHelpers, TestCase): +class TestLimits(JailCodeHelpersMixin, TestCase): """Tests of the resource limits, and changing them.""" def setUp(self): @@ -351,7 +348,7 @@ def test_cant_use_too_much_time(self, log_log): # Make sure we log that we are killing the process. log_text = text_of_logs(log_log.mock_calls) - self.assertRegexpMatches(log_text, r"WARNING: Killing process \d+") + self.assertRegex(log_text, r"WARNING: Killing process \d+") def test_changing_realtime_limit(self): # Change time limit to 2 seconds, sleeping for 1.5 will be fine. @@ -439,6 +436,7 @@ def test_cant_write_many_small_temp_files(self): # We would like this to fail, but there's nothing that checks total # file size written, so the sandbox does not prevent it yet. raise SkipTest("There's nothing checking total file size yet.") + # pylint: disable=unreachable set_limit('FSIZE', 1000) res = jailpy(code="""\ from __future__ import print_function @@ -550,7 +548,7 @@ def test_reading_dev_random(self): self.assertNotEqual(res.status, 0) -class TestSymlinks(JailCodeHelpers, TestCase): +class TestSymlinks(JailCodeHelpersMixin, TestCase): """Testing symlink behavior.""" def setUp(self): @@ -607,7 +605,7 @@ def test_symlinks_wont_copy_data(self): self.assertIn(b"ermission denied", res.stderr) -class TestMalware(JailCodeHelpers, TestCase): +class TestMalware(JailCodeHelpersMixin, TestCase): """Tests that attempt actual malware against the interpreter or system.""" def test_crash_cpython(self): @@ -663,7 +661,7 @@ def test_find_other_sandboxes(self): self.assertEqual(res.stdout, b"Done.\n") -class TestProxyProcess(JailCodeHelpers, TestCase): +class TestProxyProcess(JailCodeHelpersMixin, TestCase): """Tests of the proxy process.""" def setUp(self): diff --git a/codejail/tests/test_json_safe.py b/codejail/tests/test_json_safe.py index d6e42d070..0c0d1ee5e 100644 --- a/codejail/tests/test_json_safe.py +++ b/codejail/tests/test_json_safe.py @@ -2,12 +2,13 @@ Test JSON serialization straw """ -from __future__ import absolute_import import unittest -from codejail.safe_exec import json_safe + from six import unichr from six.moves import range +from codejail.safe_exec import json_safe + class JsonSafeTest(unittest.TestCase): """ diff --git a/codejail/tests/test_safe_exec.py b/codejail/tests/test_safe_exec.py index 34dbb997b..a29c0a509 100644 --- a/codejail/tests/test_safe_exec.py +++ b/codejail/tests/test_safe_exec.py @@ -1,26 +1,25 @@ """Test safe_exec.py""" -from __future__ import absolute_import import os.path import textwrap import zipfile +from builtins import bytes +from io import BytesIO from unittest import SkipTest, TestCase import six -from builtins import bytes - -try: - from cStringIO import StringIO -except ImportError: - from io import StringIO -from io import BytesIO from codejail import safe_exec from codejail.jail_code import set_limit +try: + from cStringIO import StringIO +except ImportError: + from io import StringIO # pylint: disable=ungrouped-imports class TestJsonSafe(TestCase): + # pylint: disable=missing-class-docstring def test_decodable_dict(self): test_dict = {1: bytes('a', 'utf8'), 2: 'b', 3: {1: bytes('b', 'utf8'), 2: (1, bytes('a', 'utf8'))}} cleaned_dict = safe_exec.json_safe(test_dict) @@ -124,7 +123,7 @@ def test_raising_exceptions(self): msg = str(what_happened.exception) # The result may be repr'd or not, so the backslash needs to be # optional in this match. - self.assertRegexpMatches( + self.assertRegex( msg, r"ValueError: That\\?'s not how you pour soup!" ) diff --git a/codejail/util.py b/codejail/util.py index f0a194ad4..214e16358 100644 --- a/codejail/util.py +++ b/codejail/util.py @@ -1,6 +1,5 @@ """Helpers for codejail.""" -from __future__ import absolute_import import contextlib import os import shutil diff --git a/memory_stress.py b/memory_stress.py index 955ccdc79..8be0b6d50 100644 --- a/memory_stress.py +++ b/memory_stress.py @@ -1,13 +1,14 @@ """Memory-stress a long-running CodeJail-using process.""" -from __future__ import absolute_import -from __future__ import print_function -from codejail import safe_exec from six.moves import range +from codejail import safe_exec + GOBBLE_CHUNK = int(1e7) + def main(): + # pylint: disable=missing-function-docstring gobble = [] for i in range(int(1e7)): print(i) @@ -17,5 +18,6 @@ def main(): gobble.append("x"*GOBBLE_CHUNK) + if __name__ == "__main__": main() diff --git a/proxy_main.py b/proxy_main.py index c61f0fd17..fd1e3f0fd 100644 --- a/proxy_main.py +++ b/proxy_main.py @@ -1,6 +1,5 @@ """The main program for the proxy process.""" -from __future__ import absolute_import import sys from codejail.proxy import proxy_main diff --git a/pylintrc b/pylintrc index efabe2ecc..960603a0f 100644 --- a/pylintrc +++ b/pylintrc @@ -34,10 +34,9 @@ load-plugins= # multiple time (only on the command line, not in the configuration file where # it should appear only once). disable= + # Never going to use these -# W0142: Used * or ** magic -# W0141: Used builtin function 'map' -# I0011: 36,0: Locally disabling E1101 +# C0301: Line too long; pycodestyle handles this for us # Might use these when the code is in better shape # C0302: Too many lines in module @@ -50,7 +49,7 @@ disable= # R0912: Too many branches # R0913: Too many arguments # R0914: Too many local variables - I0011,C0302,W0141,W0142,R0201,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914 + C0301,C0302,R0201,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914 [REPORTS] diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 000000000..8ef6e185d --- /dev/null +++ b/setup.cfg @@ -0,0 +1,9 @@ +[isort] +include_trailing_comma = True +indent = ' ' +line_length = 120 +multi_line_output = 3 + +[pycodestyle] +exclude = .git,.tox +max-line-length = 120 diff --git a/tox.ini b/tox.ini index e11343452..52a90c89c 100644 --- a/tox.ini +++ b/tox.ini @@ -18,3 +18,11 @@ commands = make test_no_proxy make clean make test_proxy + +[testenv:quality] +whitelist_externals = + make +deps = + -rrequirements/testing.txt +commands = + make quality