fix: harden submission containers and stop grading crashes on bad monitor_out

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(

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,
     )
 
 

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

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