diff --git a/src/rcc/engine/container.py b/src/rcc/engine/container.py index ab7f174..466e9d0 100644 --- a/src/rcc/engine/container.py +++ b/src/rcc/engine/container.py @@ -27,6 +27,22 @@ # it never blocks process exit. CONTAINER_LOG_READER_JOIN_TIMEOUT = 5.0 +# Hardening applied to every submission container. None of the submitted +# scripts legitimately need network access (compilation and test execution +# are both local), and none need any Linux capability beyond what a plain +# process needs to run - so both can be dropped unconditionally with no +# effect on well-behaved submissions. +# +# Per-language memory/CPU/pids ceilings (compiler-monitor-oportunidades.md, +# items S1/E4) are intentionally NOT added here yet: they need a limit +# that is configurable per language (Java's JVM and MPI-based jobs expect +# very different budgets than a plain C binary) and that work is tracked +# separately so a single hardcoded number does not silently break a class +# of submissions. +CONTAINER_NETWORK_MODE = "none" +CONTAINER_CAP_DROP = ["ALL"] +CONTAINER_SECURITY_OPT = ["no-new-privileges"] + class ContainerLogReader: """Bridge between docker-py's blocking log generator and asyncio. @@ -173,6 +189,9 @@ async def run( detach=True, remove=False, volumes=volumes, + network_mode=CONTAINER_NETWORK_MODE, + cap_drop=CONTAINER_CAP_DROP, + security_opt=CONTAINER_SECURITY_OPT, ) log_reader = ContainerLogReader( diff --git a/src/rcc/engine/grading.py b/src/rcc/engine/grading.py index 200d747..796b5d1 100644 --- a/src/rcc/engine/grading.py +++ b/src/rcc/engine/grading.py @@ -47,10 +47,29 @@ def process_test_results( user_err_fname = os.path.join(base_dir, f"{test_case.id}.error") run_info_fname = os.path.join(base_dir, f"{test_case.id}.monitor_out") run_info = configparser.ConfigParser(allow_no_value=True) - with open(run_info_fname) as run_info_file: - run_info.read_file(it.chain(("[info]",), run_info_file)) + try: + with open(run_info_fname) as run_info_file: + run_info.read_file(it.chain(("[info]",), run_info_file)) + run_signal = run_info["info"]["signal"] + run_time = run_info["info"]["time"] + except (OSError, KeyError) as exc: + # The monitor binary did not run, or crashed before writing a + # well-formed `monitor_out` (missing file, or missing/malformed + # keys). This is a sandbox/infra failure, not a judgement on the + # submission: fail only this test case instead of letting the + # exception abort every other test case in the batch. + logger.error( + f"[{commit.id}] ({test_case.id}) Unreadable monitor output '{run_info_fname}': {exc}" + ) + return TestCaseResult( + commit.id, + test_case.id, + -1, + TestCaseResult.STATUS_INCORRECT, + "-1", + ) user_err_stat = os.stat(user_err_fname) - if len(run_info["info"]["signal"]) != 0 or user_err_stat.st_size != 0: + if len(run_signal) != 0 or user_err_stat.st_size != 0: test_status = TestCaseResult.STATUS_INCORRECT else: test_out_fname = os.path.join(base_dir, f"{test_case.id}.out") @@ -67,9 +86,9 @@ def process_test_results( return TestCaseResult( commit.id, test_case.id, - run_info["info"]["time"], + run_time, test_status, - run_info["info"]["signal"], + run_signal, ) diff --git a/src/rcc/tests/test_container_hardening.py b/src/rcc/tests/test_container_hardening.py new file mode 100644 index 0000000..07686cc --- /dev/null +++ b/src/rcc/tests/test_container_hardening.py @@ -0,0 +1,128 @@ +"""Unit tests for the container-hardening flags added to `run()` in +container.py (compiler-monitor-oportunidades.md, items S1/S2/S10): +`network_mode="none"`, `cap_drop=["ALL"]`, `security_opt=["no-new-privileges"]`. + +These flags only constrain what the container is *allowed* to do; they do +not touch the image, volumes, or anything else that determines what +actually runs inside it. A full end-to-end run (actually executing a +submission inside the hardened container) is exercised by the existing +`rcc.tests.engine.test_process_commit_hello` suite, which this change does +not otherwise modify. This test instead pins down, at the unit level, that +the Docker SDK is called with exactly the previous arguments plus the new +hardening ones -- so a reviewer does not have to trust "it still works", +they can see the call surface is additive. +""" + +import asyncio +import datetime +import logging +import unittest +from typing import override +from unittest.mock import MagicMock, patch + +from rcc.config import Config +from rcc.engine import container +from rcc.languages import Language +from rcc.model import Commit +from rcc.provider.data import DataProvider + + +def build_commit() -> Commit: + return Commit( + 1, + "user@example.com", + 1, + 1, + Commit.STATUS_RUNNING, + "", + 0, + 0, + False, + "", + datetime.datetime.now(tz=datetime.UTC), + None, + None, + None, + "", + None, + "hello.c", + 1, + 1, + 1, + "hello.c", + Language("C", ["c", "h"]), + ) + + +async def _async_noop(*_args: object, **_kwargs: object) -> None: + return None + + +class TestContainerHardening(unittest.TestCase): + cfg: Config = Config({"base_exec_timeout": 1.0}) + commit: Commit = build_commit() + fake_container: MagicMock = MagicMock() + fake_client: MagicMock = MagicMock() + data_provider: MagicMock = MagicMock() + + @override + def setUp(self) -> None: + self.cfg = Config({"base_exec_timeout": 1.0}) + self.commit = build_commit() + self.fake_container = MagicMock() + self.fake_container.logs.return_value = iter([]) + self.fake_client = MagicMock() + self.fake_client.containers.run.return_value = self.fake_container + self.data_provider = MagicMock(spec=DataProvider) + self.data_provider.update_commit = _async_noop + logging.getLogger("run.codes").addHandler(logging.NullHandler()) + + def test_run_passes_hardening_flags_without_changing_prior_arguments( + self, + ) -> None: + with patch.object(container.docker, "from_env", return_value=self.fake_client): + + async def go() -> None: + try: + await container.run( + self.cfg, + self.data_provider, + self.commit, + [], + "/tmp/base", + "/tmp/remote", + ) + except Exception as exc: # noqa: BLE001 + # The fake log stream ends immediately, so the + # compilation/run handshake raises downstream; this test + # only cares about how containers.run() was invoked. + logging.getLogger(__name__).debug( + "expected downstream failure from the fake log stream: %s", + exc, + ) + + asyncio.run(go()) + + self.fake_client.containers.run.assert_called_once() + call = self.fake_client.containers.run.call_args + args = call.args + kwargs = call.kwargs + language = self.commit.language + assert language is not None + + self.assertEqual(args[0], language.image) + self.assertTrue(kwargs["detach"]) + self.assertFalse(kwargs["remove"]) + self.assertEqual( + kwargs["volumes"], + {"/tmp/remote": {"bind": "/root", "mode": "rw"}}, + ) + + # The hardening flags themselves. + self.assertEqual(kwargs["network_mode"], "none") + self.assertEqual(kwargs["cap_drop"], ["ALL"]) + self.assertEqual(kwargs["security_opt"], ["no-new-privileges"]) + + +if __name__ == "__main__": + _ = unittest.main() diff --git a/src/rcc/tests/test_grading.py b/src/rcc/tests/test_grading.py new file mode 100644 index 0000000..46586a5 --- /dev/null +++ b/src/rcc/tests/test_grading.py @@ -0,0 +1,143 @@ +"""Unit tests for grading.process_test_results, in particular its handling +of a missing/malformed `monitor_out` file (compiler-monitor-oportunidades.md, +item B6): a sandbox-level failure should fail only the affected test case, +not raise and abort the whole batch. +""" + +import datetime +import os +import tempfile +import unittest +from typing import override +from unittest.mock import MagicMock + +from rcc.engine.grading import process_test_results +from rcc.model import Commit, TestCase, TestCaseResult + + +def build_test_case(test_case_id: int = 1) -> TestCase: + return TestCase( + test_case_id, + 1, + TestCase.IO_TYPE_TEXT, + TestCase.IO_TYPE_TEXT, + False, + False, + 0, + 5, + 0, + False, + 0, + None, + None, + ) + + +def build_commit() -> Commit: + return Commit( + 1, + "user@example.com", + 1, + 1, + Commit.STATUS_RUNNING, + "", + 0, + 0, + False, + "", + datetime.datetime.now(tz=datetime.UTC), + None, + None, + None, + "", + None, + "hello.c", + 1, + 1, + 1, + "hello.c", + None, + ) + + +class TestProcessTestResultsRunInfoHandling(unittest.TestCase): + tmpdir: tempfile.TemporaryDirectory[str] = tempfile.TemporaryDirectory() + base_dir: str = "" + storage: MagicMock = MagicMock() + commit: Commit = build_commit() + test_case: TestCase = build_test_case(1) + + @override + def setUp(self) -> None: + self.tmpdir = tempfile.TemporaryDirectory() + self.base_dir = self.tmpdir.name + self.storage = MagicMock() + self.commit = build_commit() + self.test_case = build_test_case(1) + # process_test_results always stats the `.error` file. + with open(os.path.join(self.base_dir, "1.error"), "w") as f: + _ = f.write("") + + @override + def tearDown(self) -> None: + self.tmpdir.cleanup() + + def test_missing_monitor_out_does_not_raise(self) -> None: + # No `1.monitor_out` written at all (monitor never ran / container + # setup failed before it could). + result = process_test_results( + self.storage, self.commit, self.test_case, self.base_dir + ) + self.assertIsInstance(result, TestCaseResult) + self.assertEqual(result.status, TestCaseResult.STATUS_INCORRECT) + self.storage.fetch_test_case_output_file.assert_not_called() + + def test_empty_monitor_out_does_not_raise(self) -> None: + # `monitor_out` exists but is empty (e.g. the monitor binary itself + # segfaulted before printing anything, per B4 in the audit doc). + with open(os.path.join(self.base_dir, "1.monitor_out"), "w") as f: + _ = f.write("") + result = process_test_results( + self.storage, self.commit, self.test_case, self.base_dir + ) + self.assertIsInstance(result, TestCaseResult) + self.assertEqual(result.status, TestCaseResult.STATUS_INCORRECT) + self.storage.fetch_test_case_output_file.assert_not_called() + + def test_malformed_monitor_out_does_not_raise(self) -> None: + # Present but missing the `time=` key entirely. + with open(os.path.join(self.base_dir, "1.monitor_out"), "w") as f: + _ = f.write("exit_status=0\nsignal=\n") + result = process_test_results( + self.storage, self.commit, self.test_case, self.base_dir + ) + self.assertIsInstance(result, TestCaseResult) + self.assertEqual(result.status, TestCaseResult.STATUS_INCORRECT) + + def test_well_formed_monitor_out_still_works(self) -> None: + # Control case: a normal, well-formed monitor_out must still be + # processed exactly as before (no change in behavior for the + # common path). + with open(os.path.join(self.base_dir, "1.monitor_out"), "w") as f: + _ = f.write("exit_status=0\nsignal=\ntime=0.0123\n") + with open(os.path.join(self.base_dir, "1.output"), "w") as f: + _ = f.write("Hello, run.codes!\n") + with open(os.path.join(self.base_dir, "1.out"), "w") as f: + _ = f.write("Hello, run.codes!\n") + + def fake_fetch(_test_case: TestCase, destination: str) -> None: + with open(destination, "w") as out: + _ = out.write("Hello, run.codes!\n") + + self.storage.fetch_test_case_output_file.side_effect = fake_fetch + + result = process_test_results( + self.storage, self.commit, self.test_case, self.base_dir + ) + self.assertEqual(result.status, TestCaseResult.STATUS_CORRECT) + self.assertEqual(result.cpu_time, "0.0123") + self.assertEqual(result.error, "") + + +if __name__ == "__main__": + _ = unittest.main()