refactor: run cores and builds in a sandbox

core_probe starts its worker with only the --env values, the worker clears its own environment before loading the core, and a working bubblewrap runs it with a read-only tree, no network and the user's credentials masked. The record names the isolation used. A timeout kills the whole process group. campaign.py starts core_probe, git, make and romfetch with the allowlist, make inside the same sandbox. --record refuses a folder that is not a checkout root.

scripts/campaign.py

@@ -48,6 +48,7 @@ import json
 import os
 import re
 import shutil
+import signal
 import subprocess
 import sys
 import time
@@ -65,6 +66,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent))
 import autorun
 import campaign_profiles
 import campaign_rules as rules
+import core_probe
 import measurement_records as mr
 
 ROOT = Path(__file__).resolve().parent.parent
@@ -177,6 +179,27 @@ def is_bios(entry: dict) -> bool:
 # Host
 # --------------------------------------------------------------------------
 
+def run_group(argv: Sequence[str], *, timeout: float | None = None, check: bool = False,
+              capture_output: bool = False, **kwargs: object) -> subprocess.CompletedProcess:
+    """subprocess.run in a process group of its own, the whole group killed
+    on a timeout or an interruption: a hung core never outlives its run."""
+    if capture_output:
+        kwargs.setdefault("stdout", subprocess.PIPE)
+        kwargs.setdefault("stderr", subprocess.PIPE)
+    with subprocess.Popen(list(argv), start_new_session=True, **kwargs) as proc:
+        try:
+            out, err = proc.communicate(timeout=timeout)
+        except BaseException:
+            with contextlib.suppress(ProcessLookupError):
+                os.killpg(proc.pid, signal.SIGKILL)
+            proc.communicate()
+            raise
+    result = subprocess.CompletedProcess(list(argv), proc.returncode, out, err)
+    if check:
+        result.check_returncode()
+    return result
+
+
 @dataclass
 class Host:
     """What the campaign reads from the machine and the network, replaced in tests."""
@@ -187,7 +210,7 @@ class Host:
     cpu_count: int = os.cpu_count() or 1
     free_gb: Callable[[Path], float] = autorun.free_gb
     poll: float = 60.0
-    run: Callable[..., subprocess.CompletedProcess] = subprocess.run
+    run: Callable[..., subprocess.CompletedProcess] = run_group
     urlopen: Callable[..., object] = urllib.request.urlopen
 
     @property
@@ -242,6 +265,30 @@ class Paths:
                    content=main / ".cache" / "test-content", db=ROOT / "database.json")
 
 
+ROMFETCH_PASSED = ("XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME")
+
+
+def child_env(romfetch: bool = False) -> dict[str, str]:
+    """What a program the campaign starts inherits: autorun's allowlist,
+    never a token or a password. romfetch also gets its own settings
+    (ROMFETCH_*), its account staying in its config file."""
+    names: list[str] = []
+    if romfetch:
+        names = [k for k in os.environ if k.startswith("ROMFETCH_")] + list(ROMFETCH_PASSED)
+    return autorun.safe_env(names)
+
+
+@contextlib.contextmanager
+def working_directory(path: Path) -> Iterator[None]:
+    """contextlib.chdir, which Python 3.9 lacks."""
+    before = Path.cwd()
+    os.chdir(path)
+    try:
+        yield
+    finally:
+        os.chdir(before)
+
+
 @contextlib.contextmanager
 def host_lock(lock_path: Path) -> Iterator[None]:
     """The automation lock, unless the autorun job running us holds it."""
@@ -405,7 +452,7 @@ class ContentStore:
                 "--content-dir", str(self.c.paths.content)]
         for attempt, delay in enumerate((*self.c.settings.delays, None)):
             proc = self.c.host.run(argv, capture_output=True, text=True, check=False,
-                                   timeout=6 * 3600)
+                                   timeout=6 * 3600, env=child_env(romfetch=True))
             if proc.returncode == 0:
                 self.c.log(f"romfetch {content.name}: {proc.stdout.strip()[:300]}")
                 return
@@ -569,17 +616,23 @@ class Cores:
             if not self.c.settings.network:
                 raise Unavailable("core: not on the buildbot, offline, nothing built yet")
             try:
-                built = [self.compile(source, commit, out)]
+                so, command = self.compile(source, commit, out)
             except Unavailable as exc:
                 self.c.state["builds"][emulator] = {"commit": commit, "error": str(exc), "at": now_iso()}
                 raise
+            built = [so]
+            record["command"] = command
         command = record.get("command") or "make"
         self.c.state["builds"][emulator] = {"commit": commit, "command": command,
                                             "so": built[0].name, "at": now_iso()}
         return Core(built[0].name.removesuffix("_libretro.so"), built[0], file_sha256(built[0]),
                     repo=source, commit=commit, build_command=command)
 
-    def compile(self, source: str, commit: str, out: Path) -> Path:
+    def compile(self, source: str, commit: str, out: Path) -> tuple[Path, str]:
+        """Clone with git (trusted, networked), then run the project's make
+        under bubblewrap: no network, a read-only tree with the source
+        writable, the user's credentials masked. Returns the .so and the
+        command, with how it was isolated."""
         src = self.c.paths.home / "src" / slug(source)
         log = self.c.paths.logs / "builds" / f"{slug(source)}-{commit[:12]}.log"
         log.parent.mkdir(parents=True, exist_ok=True)
@@ -589,7 +642,7 @@ class Cores:
                 handle.write(f"$ {' '.join(argv)}\n")
                 handle.flush()
                 proc = self.c.host.run(argv, cwd=cwd, stdout=handle, stderr=subprocess.STDOUT,
-                                       check=False, timeout=timeout)
+                                       check=False, timeout=timeout, env=child_env())
             if proc.returncode != 0:
                 tail = log.read_text(encoding="utf-8", errors="replace").strip().splitlines()[-3:]
                 raise Unavailable(f"core: {argv[0]} {argv[1] if len(argv) > 1 else ''} exited "
@@ -603,13 +656,14 @@ class Cores:
         makefile = "Makefile.libretro" if (src / "Makefile.libretro").is_file() else "Makefile"
         jobs = str(max(1, (os.cpu_count() or 2) // 2))
         command = ["make", "-f", makefile, f"-j{jobs}", "platform=unix"]
-        step(command, cwd=src, timeout=BUILD_TIMEOUT)
+        prefix, isolation = core_probe.sandbox_argv([src], private_tmp=True)
+        step(prefix + command, cwd=src, timeout=BUILD_TIMEOUT)
         built = sorted(src.rglob("*_libretro.so"), key=lambda p: p.stat().st_mtime)
         if not built:
             raise Unavailable(f"core: {' '.join(command)} produced no *_libretro.so (log {log})")
         out.mkdir(parents=True, exist_ok=True)
         shutil.copy2(built[-1], out / built[-1].name)
-        return out / built[-1].name
+        return out / built[-1].name, f"{' '.join(command)} (isolation: {isolation})"
 
 
 # --------------------------------------------------------------------------
@@ -699,7 +753,7 @@ class Campaign:
 
         hint = entry.get("path") if isinstance(entry.get("path"), str) else ""
         # The database names files relative to the checkout it describes.
-        with contextlib.chdir(self.paths.root):
+        with working_directory(self.paths.root):
             path, status = resolve_local_file(entry, self.db, dest_hint=hint or "")
         if not path or status in ("not_found", "hash_mismatch"):
             return None
@@ -754,7 +808,7 @@ class Campaign:
             self.guard()
             try:
                 proc = self.host.run(command, capture_output=True, text=True, check=False,
-                                     timeout=budget, cwd=self.paths.root)
+                                     timeout=budget, cwd=self.paths.root, env=child_env())
             except subprocess.TimeoutExpired:
                 with log.open("a", encoding="utf-8") as handle:
                     handle.write(f"$ {' '.join(command)}\nkilled after {budget} s\n")
@@ -1492,6 +1546,11 @@ def main(argv: Sequence[str] | None = None) -> int:
         campaign.run(only, args.system, dry_run=True)
         return 0
     campaign.dirty_at_start = set(autorun.changed_files(paths.root)) if args.commit else set()
+
+    def stop(signum: int, frame: object) -> None:
+        # A timer's TimeoutStartSec ends the pass like Ctrl-C: state saved, pass line written.
+        raise KeyboardInterrupt
+    signal.signal(signal.SIGTERM, stop)
     try:
         with host_lock(campaign.lock_path):
             current = campaign.run(only, args.system, commit_every=args.commit_every,

scripts/core_probe.py

@@ -18,6 +18,7 @@ reproduce it (core build, frontend commit, host, inputs by hash).
 from __future__ import annotations
 
 import argparse
+import contextlib
 import ctypes
 import datetime as dt
 import hashlib
@@ -26,6 +27,7 @@ import os
 import platform
 import re
 import shutil
+import signal
 import statistics
 import struct
 import subprocess
@@ -470,6 +472,9 @@ class Frontend:
 
 def run_worker(spec: dict) -> dict:
     """Load the core in this process and run it. Returns the measured result."""
+    # Nothing of the host's environment reaches the core: only what --env gave.
+    os.environ.clear()
+    os.environ.update(spec["env"])
     core = ctypes.CDLL(spec["core"])
     front = Frontend(spec)
     for name, fn in zip(("retro_set_environment", "retro_set_video_refresh", "retro_set_audio_sample",
@@ -646,6 +651,59 @@ def host_paths_hidden(text: str, places: Sequence[tuple[str, str]]) -> str:
     return text
 
 
+_BWRAP_WORKS: dict[str, bool] = {}
+
+
+def sandbox_argv(writable: Sequence[Path], private_tmp: bool = False) -> tuple[list[str], str]:
+    """The bubblewrap prefix a third-party program runs under, and its name.
+
+    The whole tree is read-only, the network and the other namespaces are
+    unshared, the user's credentials (~/.config, ~/.ssh, ~/.gnupg,
+    ~/.git-credentials, ~/.netrc) are masked, and only `writable` can be
+    written. Without a working bwrap the program still runs, with an empty
+    environment, and the returned name says so.
+    """
+    bwrap = shutil.which("bwrap")
+    if not bwrap:
+        return [], "environment only: bwrap not found"
+    base = [bwrap, "--ro-bind", "/", "/", "--dev", "/dev", "--tmpfs", "/dev/shm", "--proc", "/proc",
+            "--unshare-all", "--die-with-parent", "--new-session"]
+    if bwrap not in _BWRAP_WORKS:
+        probe_run = subprocess.run(base + ["true"], capture_output=True, check=False, env={})
+        _BWRAP_WORKS[bwrap] = probe_run.returncode == 0
+    if not _BWRAP_WORKS[bwrap]:
+        return [], "environment only: bwrap cannot create a sandbox here"
+    home = Path.home()
+    argv = list(base) + (["--tmpfs", "/tmp"] if private_tmp else [])
+    for hidden in (home / ".config", home / ".ssh", home / ".gnupg"):
+        if hidden.is_dir():
+            argv += ["--tmpfs", str(hidden)]
+    for secret in (home / ".git-credentials", home / ".netrc"):
+        if secret.exists():
+            argv += ["--ro-bind", "/dev/null", str(secret)]
+    for path in writable:
+        resolved = Path(path).resolve()
+        resolved.mkdir(parents=True, exist_ok=True)
+        argv += ["--bind", str(resolved), str(resolved)]
+    return argv, "bwrap"
+
+
+def run_group(command: list[str], timeout: float, env: dict[str, str]) -> subprocess.CompletedProcess:
+    """Run the worker in a process group of its own and kill the whole group
+    when it outlives the timeout. Killing strace alone left a hung core
+    spinning, detached from anything that would ever stop it."""
+    with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
+                          env=env, start_new_session=True) as proc:
+        try:
+            out, err = proc.communicate(timeout=timeout)
+        except BaseException:
+            with contextlib.suppress(ProcessLookupError):
+                os.killpg(proc.pid, signal.SIGKILL)
+            proc.communicate()
+            raise
+    return subprocess.CompletedProcess(command, proc.returncode, out, err)
+
+
 def run_once(args: argparse.Namespace, system_dir: Path, scratch: Path, state_out: Path | None,
              png_dir: Path | None) -> dict:
     save_dir = scratch / "save"
@@ -664,6 +722,7 @@ def run_once(args: argparse.Namespace, system_dir: Path, scratch: Path, state_ou
         "joypad_ports": args.joypad_ports,
         "png_dir": str(png_dir) if png_dir else None,
         "state_out": str(state_out) if state_out else None,
+        "env": dict(args.env),
     }
     spec_path, out_path, trace_path = scratch / "spec.json", scratch / "result.json", scratch / "strace.txt"
     spec_path.write_text(json.dumps(spec), encoding="utf-8")
@@ -671,9 +730,10 @@ def run_once(args: argparse.Namespace, system_dir: Path, scratch: Path, state_ou
     strace = shutil.which("strace") if not args.no_trace else None
     if strace:
         command = [strace, "-f", "-qq", "-s", "4096", "-e", "trace=openat,open", "-o", str(trace_path)] + command
-    env = {**os.environ, **dict(args.env)}
-    proc = subprocess.run(command, check=False, capture_output=True, text=True,
-                          timeout=args.timeout, env=env)
+    writable = [scratch, system_dir] + [p for p in (png_dir, state_out.parent if state_out else None) if p]
+    prefix, isolation = sandbox_argv(writable)
+    # The core sees the variables it was given and nothing of the host's.
+    proc = run_group(prefix + command, args.timeout, dict(args.env))
     if proc.returncode != 0 or not out_path.exists():
         raise ProbeError(f"worker exited {proc.returncode}: {proc.stderr.strip()[-2000:]}")
     result = json.loads(out_path.read_text(encoding="utf-8"))
@@ -697,6 +757,7 @@ def run_once(args: argparse.Namespace, system_dir: Path, scratch: Path, state_ou
     if result.get("wall_s") and result.get("frames"):
         result["ms_per_frame"] = result["wall_s"] * 1000 / result["frames"]
         result["fps"] = result["frames"] / result["wall_s"]
+    result["isolation"] = isolation
     return result
 
 
@@ -824,6 +885,20 @@ def build_parser() -> argparse.ArgumentParser:
     return parser
 
 
+def not_a_checkout_root(directory: Path) -> str | None:
+    """Why --record DIR is refused, or None. DIR is the checkout, never its
+    measurements/ folder: that wrote measurements/measurements/<emulator>.jsonl."""
+    form = "--record names the root of a git checkout (the folder holding measurements/), e.g. --record ."
+    if not (directory / ".git").exists():
+        return f"--record {directory}: not a git checkout root. {form}"
+    proc = subprocess.run(["git", "-C", str(directory), "rev-parse", "--show-toplevel"],
+                          capture_output=True, text=True, check=False)
+    top = proc.stdout.strip()
+    if proc.returncode != 0 or not top or Path(top).resolve() != directory.resolve():
+        return f"--record {directory}: not a git checkout root (its root is {top or 'unknown'}). {form}"
+    return None
+
+
 def validate_args(args: argparse.Namespace) -> list[str]:
     problems: list[str] = []
     if not Path(args.core).is_file():
@@ -838,6 +913,9 @@ def validate_args(args: argparse.Namespace) -> list[str]:
     if args.png_dir and not args.frame_hash_every:
         problems.append("--png-dir needs --frame-hash-every")
     if args.record:
+        wrong = not_a_checkout_root(Path(args.record))
+        if wrong:
+            problems.append(wrong)
         if not args.emulator and Path(args.core).is_file():
             try:
                 default_emulator(Path(args.core), None)