From edb255ca5e255a98507ebf0a8b2b742b11ef3392 Mon Sep 17 00:00:00 2001 From: MarkKoz <1515135+MarkKoz@users.noreply.github.com> Date: Fri, 3 Jun 2022 15:08:04 -0700 Subject: Update pre-commit hooks and flake8 plugins --- .pre-commit-config.yaml | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6c56f43..5700645 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v3.4.0 + rev: v4.2.0 hooks: - id: check-merge-conflict - id: check-toml @@ -9,26 +9,25 @@ repos: - id: trailing-whitespace args: [--markdown-linebreak-ext=md] - repo: https://github.com/pre-commit/pygrep-hooks - rev: v1.6.0 + rev: v1.9.0 hooks: - id: python-check-blanket-noqa - repo: https://github.com/PyCQA/flake8 - rev: &flake8_version 3.8.4 + rev: &flake8_version 4.0.1 hooks: - &flake8_hook id: flake8 additional_dependencies: - - flake8-annotations~=2.5.0 - - flake8-bugbear~=20.11 - - flake8-docstrings~=1.5 - - flake8-formatter-junit-xml~=0.0.6 - - flake8-import-order~=0.18.1 - - flake8-quotes~=3.2 + - flake8-annotations~=2.7 + - flake8-bugbear==22.4.25 + - flake8-docstrings~=1.4 + - flake8-import-order~=0.18,!=0.18.0 + - flake8-quotes~=3.3 - flake8-string-format~=0.3.0 - - flake8-tidy-imports~=4.2 + - flake8-tidy-imports~=4.5 - flake8-todo~=0.7 - - pep8-naming~=0.11.1 - - pydocstyle~=5.1 + - pep8-naming~=0.12.1 + - pydocstyle~=6.1,!=6.1.0 - repo: https://github.com/PyCQA/flake8 rev: *flake8_version hooks: -- cgit v1.2.3 From aad2a6db38a8300c9db74f215d8c6e562e502586 Mon Sep 17 00:00:00 2001 From: MarkKoz <1515135+MarkKoz@users.noreply.github.com> Date: Fri, 3 Jun 2022 15:54:35 -0700 Subject: Add black and isort --- .flake8 | 5 +-- .pre-commit-config.yaml | 12 ++++-- pyproject.toml | 17 ++++++-- snekbox/__main__.py | 2 +- snekbox/api/resources/eval.py | 20 ++-------- snekbox/nsjail.py | 28 +++++++------- snekbox/utils/cgroup.py | 2 +- snekbox/utils/gunicorn.py | 1 + snekbox/utils/logging.py | 3 +- tests/api/__init__.py | 5 +-- tests/api/test_eval.py | 10 ++--- tests/gunicorn_utils.py | 4 +- tests/test_integration.py | 1 - tests/test_main.py | 18 +++------ tests/test_nsjail.py | 90 ++++++++++++++++++++++++------------------- 15 files changed, 111 insertions(+), 107 deletions(-) diff --git a/.flake8 b/.flake8 index 25c431c..0f42e66 100644 --- a/.flake8 +++ b/.flake8 @@ -1,15 +1,12 @@ [flake8] allow-untyped-nested = true -application_import_names = snekbox docstring-convention = all -import-order-style = pycharm -inline-quotes = " max-line-length = 100 exclude = __pycache__,.cache,user_base,venv,.venv,snekbox/config_pb2.py ignore = - W503, + E203, W503, # Missing Docstrings D100,D104,D105,D107, # Docstring Whitespace diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5700645..620c5be 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,6 +12,15 @@ repos: rev: v1.9.0 hooks: - id: python-check-blanket-noqa + - repo: https://github.com/PyCQA/isort + rev: 5.10.1 + hooks: + - id: isort + - repo: https://github.com/psf/black + rev: 22.3.0 + hooks: + - id: black + language_version: "3.10" - repo: https://github.com/PyCQA/flake8 rev: &flake8_version 4.0.1 hooks: @@ -21,10 +30,7 @@ repos: - flake8-annotations~=2.7 - flake8-bugbear==22.4.25 - flake8-docstrings~=1.4 - - flake8-import-order~=0.18,!=0.18.0 - - flake8-quotes~=3.3 - flake8-string-format~=0.3.0 - - flake8-tidy-imports~=4.5 - flake8-todo~=0.7 - pep8-naming~=0.12.1 - pydocstyle~=6.1,!=6.1.0 diff --git a/pyproject.toml b/pyproject.toml index 08aeb8c..7a13370 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,8 +59,17 @@ exclude_lines = [ branch = true data_file = "${COVERAGE_DATAFILE-.coverage}" include = ["snekbox/*"] -omit = [ - "snekbox/api/app.py", - "snekbox/config_pb2.py" -] +omit = ["snekbox/config_pb2.py"] relative_files = true + +[tool.black] +line-length = 100 +target-version = ["py310"] +force-exclude = ["snekbox/config_pb2.py"] + +[tool.isort] +line_length = 100 +profile = "black" +skip_gitignore = true +src_paths = ["snekbox"] +extend_skip = ["snekbox/config_pb2.py"] diff --git a/snekbox/__main__.py b/snekbox/__main__.py index 7ac10e9..2382c4c 100644 --- a/snekbox/__main__.py +++ b/snekbox/__main__.py @@ -27,7 +27,7 @@ def parse_args() -> argparse.Namespace: # Can't use double dash because that has special semantics for argparse already. split = unknown.index("---") args.nsjail_args = unknown[:split] - args.py_args = unknown[split + 1:] + args.py_args = unknown[split + 1 :] except ValueError: args.nsjail_args = unknown diff --git a/snekbox/api/resources/eval.py b/snekbox/api/resources/eval.py index 0a59f2e..7c23a52 100644 --- a/snekbox/api/resources/eval.py +++ b/snekbox/api/resources/eval.py @@ -23,19 +23,10 @@ class EvalResource: REQ_SCHEMA = { "type": "object", "properties": { - "input": { - "type": "string" - }, - "args": { - "type": "array", - "items": { - "type": "string" - } - } + "input": {"type": "string"}, + "args": {"type": "array", "items": {"type": "string"}}, }, - "required": [ - "input" - ] + "required": ["input"], } def __init__(self): @@ -91,7 +82,4 @@ class EvalResource: log.exception("An exception occurred while trying to process the request") raise falcon.HTTPInternalServerError - resp.media = { - "stdout": result.stdout, - "returncode": result.returncode - } + resp.media = {"stdout": result.stdout, "returncode": result.returncode} diff --git a/snekbox/nsjail.py b/snekbox/nsjail.py index 1aca637..a6b202e 100644 --- a/snekbox/nsjail.py +++ b/snekbox/nsjail.py @@ -133,11 +133,7 @@ class NsJail: return "".join(output) def python3( - self, - code: str, - *, - nsjail_args: Iterable[str] = (), - py_args: Iterable[str] = ("-c",) + self, code: str, *, nsjail_args: Iterable[str] = (), py_args: Iterable[str] = ("-c",) ) -> CompletedProcess: """ Execute Python 3 code in an isolated environment and return the completed process. @@ -153,17 +149,26 @@ class NsJail: if self.ignore_swap_limits: nsjail_args = ( - "--cgroup_mem_memsw_max", "0", "--cgroup_mem_swap_max", "-1", *nsjail_args + "--cgroup_mem_memsw_max", + "0", + "--cgroup_mem_swap_max", + "-1", + *nsjail_args, ) with NamedTemporaryFile() as nsj_log: args = ( self.nsjail_binary, - "--config", NSJAIL_CFG, - "--log", nsj_log.name, + "--config", + NSJAIL_CFG, + "--log", + nsj_log.name, *nsjail_args, "--", - self.config.exec_bin.path, *self.config.exec_bin.arg, *py_args, code + self.config.exec_bin.path, + *self.config.exec_bin.arg, + *py_args, + code, ) msg = "Executing code..." @@ -173,10 +178,7 @@ class NsJail: try: nsjail = subprocess.Popen( - args, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True + args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True ) except ValueError: return CompletedProcess(args, None, "ValueError: embedded null byte", None) diff --git a/snekbox/utils/cgroup.py b/snekbox/utils/cgroup.py index cd515ab..b06cdfa 100644 --- a/snekbox/utils/cgroup.py +++ b/snekbox/utils/cgroup.py @@ -19,7 +19,7 @@ def get_version(config: NsJailConfig) -> int: config.cgroup_mem_mount, config.cgroup_pids_mount, config.cgroup_net_cls_mount, - config.cgroup_cpu_mount + config.cgroup_cpu_mount, ) v1_exists = any(Path(mount).exists() for mount in cgroup_mounts) diff --git a/snekbox/utils/gunicorn.py b/snekbox/utils/gunicorn.py index 68e3ed9..96d2e02 100644 --- a/snekbox/utils/gunicorn.py +++ b/snekbox/utils/gunicorn.py @@ -4,6 +4,7 @@ from gunicorn import glogging from gunicorn.config import Config from snekbox import DEBUG + from .logging import FORMAT __all__ = ("GunicornLogger",) diff --git a/snekbox/utils/logging.py b/snekbox/utils/logging.py index a73db2d..0bbc5e3 100644 --- a/snekbox/utils/logging.py +++ b/snekbox/utils/logging.py @@ -5,7 +5,6 @@ import warnings from falcon.util.deprecation import DeprecatedWarning - __all__ = ("FORMAT", "init_logger", "init_sentry") FORMAT = "%(asctime)s | %(process)5s | %(name)30s | %(levelname)8s | %(message)s" @@ -38,5 +37,5 @@ def init_sentry(version: str) -> None: dsn=os.environ.get("SNEKBOX_SENTRY_DSN", ""), integrations=[FalconIntegration()], send_default_pii=True, - release=f"snekbox@{version}" + release=f"snekbox@{version}", ) diff --git a/tests/api/__init__.py b/tests/api/__init__.py index a900316..3f7d250 100644 --- a/tests/api/__init__.py +++ b/tests/api/__init__.py @@ -14,10 +14,7 @@ class SnekAPITestCase(testing.TestCase): self.patcher = mock.patch("snekbox.api.resources.eval.NsJail", autospec=True) self.mock_nsjail = self.patcher.start() self.mock_nsjail.return_value.python3.return_value = CompletedProcess( - args=[], - returncode=0, - stdout="output", - stderr="error" + args=[], returncode=0, stdout="output", stderr="error" ) self.addCleanup(self.patcher.stop) diff --git a/tests/api/test_eval.py b/tests/api/test_eval.py index 24e673a..976970e 100644 --- a/tests/api/test_eval.py +++ b/tests/api/test_eval.py @@ -20,15 +20,13 @@ class TestEvalResource(SnekAPITestCase): expected = { "title": "Request data failed validation", - "description": "'input' is a required property" + "description": "'input' is a required property", } self.assertEqual(expected, result.json) def test_post_invalid_data_400(self): - bodies = ( - {"input": 400}, {"input": "", "args": [400]} - ) + bodies = ({"input": 400}, {"input": "", "args": [400]}) for body in bodies: with self.subTest(): @@ -38,7 +36,7 @@ class TestEvalResource(SnekAPITestCase): expected = { "title": "Request data failed validation", - "description": "400 is not of type 'string'" + "description": "400 is not of type 'string'", } self.assertEqual(expected, result.json) @@ -52,7 +50,7 @@ class TestEvalResource(SnekAPITestCase): expected = { "title": "415 Unsupported Media Type", - "description": "application/xml is an unsupported media type." + "description": "application/xml is an unsupported media type.", } self.assertEqual(expected, result.json) diff --git a/tests/gunicorn_utils.py b/tests/gunicorn_utils.py index f5dae7a..5c077aa 100644 --- a/tests/gunicorn_utils.py +++ b/tests/gunicorn_utils.py @@ -29,12 +29,14 @@ class _StandaloneApplication(WSGIApplication): def _proc_target(config_path: str, event: multiprocessing.Event, **kwargs) -> None: """Run a Gunicorn app with the given config and set `event` when Gunicorn is ready.""" + def when_ready(_): event.set() app = _StandaloneApplication(config_path, when_ready=when_ready, **kwargs) import logging + logging.disable(logging.INFO) app.run() @@ -62,7 +64,7 @@ def run_gunicorn(config_path: str = "config/gunicorn.conf.py", **kwargs) -> Iter concurrent.futures.wait( [executor.submit(proc.join), executor.submit(event.wait)], timeout=60, - return_when=concurrent.futures.FIRST_COMPLETED + return_when=concurrent.futures.FIRST_COMPLETED, ) # Can't use the context manager cause wait=False needs to be set. executor.shutdown(wait=False, cancel_futures=True) diff --git a/tests/test_integration.py b/tests/test_integration.py index b76b005..7c5db2b 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -21,7 +21,6 @@ def run_code_in_snekbox(code: str) -> tuple[str, int]: class IntegrationTests(unittest.TestCase): - def test_memory_limit_separate_per_process(self): """ Each NsJail process should have its own memory limit. diff --git a/tests/test_main.py b/tests/test_main.py index 5b5bfcb..77b3130 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -12,30 +12,24 @@ import snekbox.__main__ as snekbox_main class ArgParseTests(unittest.TestCase): def test_parse_args(self): subtests = ( - ( - ["", "code"], - Namespace(code="code", nsjail_args=[], py_args=["-c"]) - ), + (["", "code"], Namespace(code="code", nsjail_args=[], py_args=["-c"])), ( ["", "code", "--time_limit", "0"], - Namespace(code="code", nsjail_args=["--time_limit", "0"], py_args=["-c"]) + Namespace(code="code", nsjail_args=["--time_limit", "0"], py_args=["-c"]), ), ( ["", "code", "---", "-m", "timeit"], - Namespace(code="code", nsjail_args=[], py_args=["-m", "timeit"]) + Namespace(code="code", nsjail_args=[], py_args=["-m", "timeit"]), ), ( ["", "code", "--time_limit", "0", "---", "-m", "timeit"], - Namespace(code="code", nsjail_args=["--time_limit", "0"], py_args=["-m", "timeit"]) + Namespace(code="code", nsjail_args=["--time_limit", "0"], py_args=["-m", "timeit"]), ), ( ["", "code", "--time_limit", "0", "---"], - Namespace(code="code", nsjail_args=["--time_limit", "0"], py_args=[]) + Namespace(code="code", nsjail_args=["--time_limit", "0"], py_args=[]), ), - ( - ["", "code", "---"], - Namespace(code="code", nsjail_args=[], py_args=[]) - ) + (["", "code", "---"], Namespace(code="code", nsjail_args=[], py_args=[])), ) for argv, expected in subtests: diff --git a/tests/test_nsjail.py b/tests/test_nsjail.py index 989cc31..a3632e6 100644 --- a/tests/test_nsjail.py +++ b/tests/test_nsjail.py @@ -5,7 +5,7 @@ import unittest import unittest.mock from textwrap import dedent -from snekbox.nsjail import NsJail, OUTPUT_MAX, READ_CHUNK_SIZE +from snekbox.nsjail import OUTPUT_MAX, READ_CHUNK_SIZE, NsJail class NsJailTests(unittest.TestCase): @@ -23,10 +23,7 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_timeout_returns_137(self): - code = dedent(""" - while True: - pass - """).strip() + code = "while True: pass" with self.assertLogs(self.logger) as log: result = self.nsjail.python3(code) @@ -38,9 +35,7 @@ class NsJailTests(unittest.TestCase): def test_memory_returns_137(self): # Add a kilobyte just to be safe. - code = dedent(f""" - x = ' ' * {self.nsjail.config.cgroup_mem_max + 1000} - """).strip() + code = f"x = ' ' * {self.nsjail.config.cgroup_mem_max + 1000}" result = self.nsjail.python3(code) self.assertEqual(result.returncode, 137) @@ -48,7 +43,8 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_subprocess_resource_unavailable(self): - code = dedent(""" + code = dedent( + """ import subprocess # Max PIDs is 5. @@ -60,7 +56,8 @@ class NsJailTests(unittest.TestCase): 'import time; time.sleep(1)' ], ).pid) - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertEqual(result.returncode, 1) @@ -68,7 +65,8 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_multiprocess_resource_limits(self): - code = dedent(""" + code = dedent( + """ import time from multiprocessing import Process @@ -87,7 +85,8 @@ class NsJailTests(unittest.TestCase): proc_2.join() print(proc_1.exitcode, proc_2.exitcode) - """) + """ + ) result = self.nsjail.python3(code) @@ -98,10 +97,12 @@ class NsJailTests(unittest.TestCase): def test_read_only_file_system(self): for path in ("/", "/etc", "/lib", "/lib64", "/snekbox", "/usr"): with self.subTest(path=path): - code = dedent(f""" + code = dedent( + f""" with open('{path}/hello', 'w') as f: f.write('world') - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertEqual(result.returncode, 1) @@ -109,11 +110,13 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_forkbomb_resource_unavailable(self): - code = dedent(""" + code = dedent( + """ import os while 1: os.fork() - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertEqual(result.returncode, 1) @@ -121,10 +124,12 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_sigsegv_returns_139(self): # In honour of Juan. - code = dedent(""" + code = dedent( + """ import ctypes ctypes.string_at(0) - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertEqual(result.returncode, 139) @@ -144,12 +149,16 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_unicode_env_erase_escape_fails(self): - result = self.nsjail.python3(dedent(""" - import os - import sys - os.unsetenv('PYTHONIOENCODING') - os.execl(sys.executable, 'python', '-c', 'print(chr(56550))') - """).strip()) + result = self.nsjail.python3( + dedent( + """ + import os + import sys + os.unsetenv('PYTHONIOENCODING') + os.execl(sys.executable, 'python', '-c', 'print(chr(56550))') + """ + ).strip() + ) self.assertEqual(result.returncode, None) self.assertEqual(result.stdout, "UnicodeDecodeError: invalid Unicode in output pipe") self.assertEqual(result.stderr, None) @@ -168,7 +177,7 @@ class NsJailTests(unittest.TestCase): "cmdline::setupArgv(nsjconf_t*, int, char**, int)():316 No command-line provided", "[F][2019-06-22T20:07:00+0000][16] int main(int, char**)():204 " "Couldn't parse cmdline options", - "Invalid Line" + "Invalid Line", ) with self.assertLogs(self.logger, logging.DEBUG) as log: @@ -181,16 +190,18 @@ class NsJailTests(unittest.TestCase): self.assertIn("WARNING:snekbox.nsjail:This is a warning!", log.output) self.assertIn( "INFO:snekbox.nsjail:pid=20 ([STANDALONE MODE]) exited with status: 2, (PIDs left: 0)", - log.output + log.output, ) def test_shm_and_tmp_not_mounted(self): for path in ("/dev/shm", "/run/shm", "/tmp"): with self.subTest(path=path): - code = dedent(f""" + code = dedent( + f""" with open('{path}/test', 'wb') as file: file.write(bytes([255])) - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertEqual(result.returncode, 1) @@ -198,13 +209,15 @@ class NsJailTests(unittest.TestCase): self.assertEqual(result.stderr, None) def test_multiprocessing_shared_memory_disabled(self): - code = dedent(""" + code = dedent( + """ from multiprocessing.shared_memory import SharedMemory try: SharedMemory('test', create=True, size=16) except FileExistsError: pass - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertEqual(result.returncode, 1) @@ -220,26 +233,25 @@ class NsJailTests(unittest.TestCase): def test_output_order(self): stdout_msg = "greetings from stdout!" stderr_msg = "hello from stderr!" - code = dedent(f""" + code = dedent( + f""" print({stdout_msg!r}) raise ValueError({stderr_msg!r}) - """).strip() + """ + ).strip() result = self.nsjail.python3(code) self.assertLess( result.stdout.find(stdout_msg), result.stdout.find(stderr_msg), - msg="stdout does not come before stderr" + msg="stdout does not come before stderr", ) self.assertEqual(result.stderr, None) def test_stdout_flood_results_in_graceful_sigterm(self): - stdout_flood = dedent(""" - while True: - print('abcdefghij') - """).strip() + code = "while True: print('abcdefghij')" - result = self.nsjail.python3(stdout_flood) + result = self.nsjail.python3(code) self.assertEqual(result.returncode, 143) def test_large_output_is_truncated(self): @@ -260,7 +272,7 @@ class NsJailTests(unittest.TestCase): result = self.nsjail.python3("", nsjail_args=args) end = result.args.index("--") - self.assertEqual(result.args[end - len(args):end], args) + self.assertEqual(result.args[end - len(args) : end], args) def test_py_args(self): args = ("-m", "timeit") -- cgit v1.2.3 From 556358e56990dcc9b4acfff12d1bf0dc269db479 Mon Sep 17 00:00:00 2001 From: Mark <1515135+MarkKoz@users.noreply.github.com> Date: Sat, 4 Jun 2022 11:55:14 -0700 Subject: Docker: never pull image when using Docker Compose Force the image to be built if it doesn't exist. --- docker-compose.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/docker-compose.yml b/docker-compose.yml index aa1a0f5..07548cd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,6 +6,7 @@ services: hostname: snekbox_dev privileged: true image: ghcr.io/python-discord/snekbox${IMAGE_SUFFIX:--venv:dev} + pull_policy: never ports: - 8060:8060 init: true -- cgit v1.2.3 From e0ac266adacf65f040a291f16b6205863e640751 Mon Sep 17 00:00:00 2001 From: Mark <1515135+MarkKoz@users.noreply.github.com> Date: Sat, 4 Jun 2022 12:01:01 -0700 Subject: CI: use GH's Ubuntu 22.04 runner instead of the self-hosted one --- .github/workflows/test.yaml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 30e6ba3..51eb0f8 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -14,7 +14,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04, self-hosted] + os: [ubuntu-20.04, ubuntu-22.04] steps: - name: Download image artifact @@ -53,17 +53,9 @@ jobs: path: .coverage.* retention-days: 1 - # Self-hosted runner needs containers, images, networks, volumes, etc. - # removed because they persist across runs and may interfere. - - name: Docker cleanup - if: matrix.os == 'self-hosted' && always() - run: | - export IMAGE_SUFFIX='-venv:${{ inputs.version }}' - docker-compose down --rmi all --remove-orphans -v -t 0 - report: name: Report coverage - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest needs: test steps: -- cgit v1.2.3