diff --git a/docs/containment.md b/docs/containment.md index edbe708..d713eb1 100644 --- a/docs/containment.md +++ b/docs/containment.md @@ -119,8 +119,19 @@ as the entrypoint of the image, so it must resolve inside the image (e.g. timed-out result: the parser may still be running with its output directory mounted writable, so nothing in that directory is collected or sealed (`exc.artifact_dir` names it). +- **Statuses 125–127.** The engines use them for their own failures, and a + parser may exit with them too. A status counts as an `engine_error` only + when stderr ends with the engine's or OCI runtime's message (a failed exec + of the command, an unknown image or runtime, an `Error response from + daemon`, or podman's/docker's `Error: `/`docker: ` prefix at 125); + otherwise it is the parser's own `exit_status`. A parser that prints the + engine's exact wording and exits 125 is still taken for an engine error. +- **Scratch.** The `/tmp` and other tmpfs mounts are writable by default + (`writable_scratch=True`). With `writable_scratch=False` they are mounted + read-only (still empty) on top of the read-only root filesystem, so + `/stele/output` is the only writable path. - **Known limits.** Mount paths containing commas or quotes are refused rather - than escaped. An engine error and a parser exiting 125 are indistinguishable. + than escaped. SELinux-enforcing hosts may need volume relabeling, which is not done yet. If the Stele process is killed, a running Docker container is not stopped (Podman gets `--timeout` as a backstop). diff --git a/stele/containment/oci.py b/stele/containment/oci.py index e74fba3..cde7c32 100644 --- a/stele/containment/oci.py +++ b/stele/containment/oci.py @@ -379,9 +379,13 @@ def build_argv(self, config: SandboxConfig, *, container_name: str, user: tuple[ if self.gpu: argv += list(info.gpu_flags) - # Ephemeral scratch space, mirroring the bubblewrap layout. + # Ephemeral scratch space, mirroring the bubblewrap layout. Without + # writable_scratch the same (empty) mounts are read-only, so + # /stele/output is the only writable path, as bubblewrap's Landlock + # rules make it there. + access = "rw" if config.writable_scratch else "ro" for path in ["/tmp", *_EPHEMERAL_TMPFS]: - argv += ["--tmpfs", f"{path}:rw,nosuid,nodev,size={self.tmpfs_size},mode=1777"] + argv += ["--tmpfs", f"{path}:{access},nosuid,nodev,size={self.tmpfs_size},mode=1777"] # Writable artifact output: the ONLY path that survives the container. config.artifact_dir.mkdir(parents=True, exist_ok=True) @@ -527,7 +531,7 @@ def _classify( f"killed by SIGKILL at the {self.memory} container memory limit", exit_code=exit_code, signal=9, ) - if exit_code in _ENGINE_ERRORS: + if exit_code in _ENGINE_ERRORS and _engine_failed(exit_code, stderr): last = _last_line(stderr) return RunFailure( FailureReason.ENGINE_ERROR, @@ -547,6 +551,31 @@ def _classify( } _SIGKILL_EXIT = 128 + 9 +# The same statuses are ordinary exit codes for the parser itself, so they are +# an engine error only when the engine or OCI runtime says so at the end of stderr. A +# failed exec of the parser command (126/127) is always reported by the +# runtime ("OCI runtime ... executable file not found", "unable to start +# container process"); for 125 the engines' own "Error: ..." (podman) or +# "docker: ..." prefix is enough, since the engine exits 125 for many +# different messages. A parser can still imitate these lines, but then it +# only misreports its own failure. +_RUNTIME_ERROR_RE = re.compile( + r"OCI runtime|OCI permission denied|unable to start container process" + r"|Error response from daemon|executable file not found in \$PATH" +) +_ENGINE_PREFIX_RE = re.compile(r"^(?:Error: |docker: )") +_ENGINE_TAIL_LINES = 3 + + +def _engine_failed(exit_code: int, stderr: str) -> bool: + # Docker follows its error with "Run 'docker run --help' for more + # information", so look at the last few lines, not only the last. + lines = [line.strip() for line in stderr.splitlines() if line.strip()] + tail = lines[-_ENGINE_TAIL_LINES:] + if any(_RUNTIME_ERROR_RE.search(line) for line in tail): + return True + return exit_code == 125 and any(_ENGINE_PREFIX_RE.match(line) for line in tail) + def _last_line(text: str) -> str: lines = [line.strip() for line in text.splitlines() if line.strip()] diff --git a/tests/test_oci_backend.py b/tests/test_oci_backend.py index 403e04b..66991ab 100644 --- a/tests/test_oci_backend.py +++ b/tests/test_oci_backend.py @@ -38,6 +38,7 @@ parse_engine_info, ) from stele.containment.capture import BoundedRun +from stele.containment.telemetry import FailureReason from stele.containment.runner import run_in_sandbox from stele.containment.sandbox import SandboxConfig @@ -222,6 +223,16 @@ def test_layout_matches_bubblewrap(self, tmp_path: Path, monkeypatch) -> None: assert _pairs(argv, "--workdir") == ["/stele/output"] tmpfs = [t.split(":")[0] for t in _pairs(argv, "--tmpfs")] assert tmpfs == ["/tmp", "/home", "/mnt", "/root", "/run", "/var"] + assert all(t.split(":")[1].startswith("rw,") for t in _pairs(argv, "--tmpfs")) + + def test_no_writable_scratch_makes_every_tmpfs_read_only(self, tmp_path: Path, monkeypatch) -> None: + # Issue #38: /stele/output must then be the only writable path. + _fake_engine(monkeypatch) + argv = _argv(OciBackend(), _config(tmp_path, writable_scratch=False)) + tmpfs = _pairs(argv, "--tmpfs") + assert [t.split(":")[0] for t in tmpfs] == ["/tmp", "/home", "/mnt", "/root", "/run", "/var"] + assert all(t.split(":")[1].startswith("ro,") for t in tmpfs) + assert "--read-only" in argv def test_runs_exact_command_on_inspected_image_id(self, tmp_path: Path, monkeypatch) -> None: _fake_engine(monkeypatch) @@ -438,6 +449,49 @@ def test_gvisor_and_gpu_are_never_combined(self, monkeypatch) -> None: # Execution, timeout, registry # --------------------------------------------------------------------------- +class TestExitClassification: + """Issue #38: 125-127 are the engine's only when the engine says so.""" + + RUNC_NOT_FOUND = ( + 'Error: runc: runc create failed: unable to start container process: error during ' + 'container init: exec: "nosuch": executable file not found in $PATH: OCI runtime ' + "attempted to invoke a command that was not found" + ) + DOCKER_NOT_FOUND = ( + "docker: Error response from daemon: failed to create task for container: failed to " + "create shim task: OCI runtime create failed: runc create failed: unable to start " + 'container process: exec: "/no/such": stat /no/such: no such file or directory: unknown.' + ) + + @pytest.mark.parametrize("code, stderr", [ + (127, RUNC_NOT_FOUND), + (127, DOCKER_NOT_FOUND), + (127, DOCKER_NOT_FOUND + "\n\nRun 'docker run --help' for more information"), + (125, "docker: Error response from daemon: pull access denied.\n\n" + "Run 'docker run --help' for more information"), + (126, 'Error: runc: ... exec: "/etc/passwd": permission denied: OCI permission denied'), + (125, "Error: nosuchimage:1: image not known"), + (125, 'Error: default OCI runtime "nosuch" not found: invalid argument'), + (125, "docker: Error response from daemon: Unknown runtime specified nosuch."), + ]) + def test_engine_failures(self, code: int, stderr: str) -> None: + failure = OciBackend()._classify(code, "parser noise\n" + stderr + "\n", 0.1, 60, False) + assert failure.reason is FailureReason.ENGINE_ERROR, failure + assert failure.exit_code == code + + @pytest.mark.parametrize("code, stderr", [ + (126, "Error: parser says no"), + (127, "Traceback ...\nSystemExit: 127"), + (125, "the parser gave up"), + (126, "Error: noise\n" + "x\n" * 5), # an engine-looking line, long before the end + (126, ""), + ]) + def test_the_parsers_own_exit_status(self, code: int, stderr: str) -> None: + failure = OciBackend()._classify(code, stderr, 0.1, 60, False) + assert failure.reason is FailureReason.EXIT_STATUS, failure + assert failure.exit_code == code + + class TestExecution: def test_timeout_force_removes_container(self, tmp_path: Path, monkeypatch) -> None: diff --git a/tests/test_run_faults.py b/tests/test_run_faults.py index 6e0bed9..0597990 100644 --- a/tests/test_run_faults.py +++ b/tests/test_run_faults.py @@ -224,6 +224,47 @@ def test_container_engine_error(tmp_path: Path, backend: OciBackend) -> None: _assert_clean(result, backend) +@pytest.mark.parametrize("backend", _LIMITED, ids=[_proof_id(b) for b in _LIMITED]) +@pytest.mark.parametrize("code", [125, 126, 127]) +def test_parser_exit_125_to_127_is_its_own(tmp_path: Path, backend: OciBackend, code: int) -> None: + """Issue #38: a parser exiting 125-127 is not an engine error.""" + if not backend.available(): + pytest.skip(f"{backend.name} backend unavailable here: {backend.unavailable_reason()}") + # Neutral wording: a parser that copies the engine's own "Error: ..." line at + # exit 125 cannot be told apart from the engine (see oci._engine_failed). + result = _run(tmp_path, "python3", f"import sys; print('parser gave up', file=sys.stderr); " + f"sys.exit({code})", backend=backend) + assert result.failure.reason is FailureReason.EXIT_STATUS, result.failure + assert result.failure.exit_code == code + _assert_clean(result, backend) + + +@pytest.mark.parametrize("backend", _LIMITED, ids=[_proof_id(b) for b in _LIMITED]) +def test_no_writable_scratch_in_a_container(tmp_path: Path, backend: OciBackend) -> None: + """Issue #38: with writable_scratch=False only /stele/output is writable.""" + if not backend.available(): + pytest.skip(f"{backend.name} backend unavailable here: {backend.unavailable_reason()}") + code = ( + "import os, sys\n" + "blocked = []\n" + "for d in ('/tmp', '/var', '/run', '/home', '/root', '/mnt', '/etc', '/'):\n" + " try:\n" + " open(os.path.join(d, 'stele-probe'), 'w').close()\n" + " print('WROTE', d, file=sys.stderr)\n" + " except OSError:\n" + " blocked.append(d)\n" + "open('/stele/output/ok.txt', 'w').write(' '.join(blocked))\n" + ) + config = SandboxConfig(command=["python3", "-c", code], artifact_dir=tmp_path / "out", + writable_scratch=False) + result = run_in_sandbox(config, backend=backend) + assert result.exit_code == 0, result.stderr + assert "WROTE" not in result.stderr, result.stderr + assert (tmp_path / "out" / "ok.txt").read_text().split() == [ + "/tmp", "/var", "/run", "/home", "/root", "/mnt", "/etc", "/", + ] + + def test_blocked_syscall_is_reported(tmp_path: Path) -> None: from stele.containment import seccomp from stele.containment.backend import BubblewrapBackend