From 3a9b4367e60ebbefdc2813b5587947abd02f01c0 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 18 Apr 2025 11:22:43 -0400 Subject: [PATCH] fix: Call json_safe on globals in codejail remote_exec (#36542) We need to make globals JSON-friendly before sending them across the network. Addresses https://github.com/edx/edx-arch-experiments/issues/1016 --- xmodule/capa/safe_exec/remote_exec.py | 18 +++++++++-- .../capa/safe_exec/tests/test_remote_exec.py | 32 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 xmodule/capa/safe_exec/tests/test_remote_exec.py diff --git a/xmodule/capa/safe_exec/remote_exec.py b/xmodule/capa/safe_exec/remote_exec.py index 5b231eb156..fb086b1064 100644 --- a/xmodule/capa/safe_exec/remote_exec.py +++ b/xmodule/capa/safe_exec/remote_exec.py @@ -7,7 +7,7 @@ import logging from importlib import import_module import requests -from codejail.safe_exec import SafeExecException +from codejail.safe_exec import SafeExecException, json_safe from django.conf import settings from edx_toggles.toggles import SettingToggle from requests.exceptions import RequestException, HTTPError @@ -90,7 +90,21 @@ def send_safe_exec_request_v0(data): extra_files = data.pop("extra_files") codejail_service_endpoint = get_codejail_rest_service_endpoint() - payload = json.dumps(data) + + # In rare cases an XBlock might introduce `bytes` objects (or other + # non-JSON-serializable objects) into the globals dict. The codejail service + # (via the codejail library) will call `json_safe` on the globals before + # JSON-encoding for the sandbox input, but here we need to call it earlier + # in the process so we can even transport the globals *to* the codejail + # service. Otherwise, we may get a TypeError when constructing the payload. + # + # This is a lossy operation (non-serializable objects will be dropped, and + # bytes converted to strings) but it is the same lossy operation that + # codejail will perform anyhow -- and it should be idempotent. + data_send = {**data} + data_send['globals_dict'] = json_safe(data_send['globals_dict']) + + payload = json.dumps(data_send) try: response = requests.post( diff --git a/xmodule/capa/safe_exec/tests/test_remote_exec.py b/xmodule/capa/safe_exec/tests/test_remote_exec.py new file mode 100644 index 0000000000..ee1ee49383 --- /dev/null +++ b/xmodule/capa/safe_exec/tests/test_remote_exec.py @@ -0,0 +1,32 @@ +""" +Tests for remote codejail execution. +""" + +import json +from unittest import TestCase +from unittest.mock import patch + +from django.test import override_settings + +from xmodule.capa.safe_exec.remote_exec import get_remote_exec + + +class TestRemoteExec(TestCase): + """Tests for remote_exec.""" + + @override_settings( + ENABLE_CODEJAIL_REST_SERVICE=True, + CODE_JAIL_REST_SERVICE_HOST='http://localhost', + ) + @patch('requests.post') + def test_json_encode(self, mock_post): + get_remote_exec({ + 'code': "out = 1 + 1", + 'globals_dict': {'some_data': b'bytes', 'unusable': object()}, + 'extra_files': None, + }) + + mock_post.assert_called_once() + data_arg = mock_post.call_args_list[0][1]['data'] + payload = json.loads(data_arg['payload']) + assert payload['globals_dict'] == {'some_data': 'bytes'}