fix: the sandbox stops running as root

ROADMAP.md

@@ -413,12 +413,23 @@ assertion that needs no daemon: that the policy is where the code looks for it.
 `clock_gettime64` was missing and has been added on that basis alone — there is
 no 32-bit host here to run it on.
 
-*Open:* the command runs as **root inside the container**. There is no `--user`
-flag, though `--tmpfs=…,uid=1000` says someone intended one. With no
-capabilities, no new privileges, a read-only root and a deny-by-default filter
-this is heavily defanged, but it is weaker than the argv implies, and closing it
-could break a diagnostic that expects to read something root-only. That is a
-decision, not an oversight to fix quietly.
+**And it no longer runs as root.** There was no `--user` flag, though
+`--tmpfs=…,uid=1000` said someone had intended one. "Root with no capabilities"
+sounds equivalent to unprivileged and is not: DAC grants access on an *ownership
+match*, with no capability involved, so the process read root-owned files that
+`CapEff=0` did nothing about. Measured, same image, same argv otherwise:
+
+| | as root | as uid 1000 |
+| --- | --- | --- |
+| read `/etc/shadow` (0640 root:shadow) | yes | denied |
+| list `/root` (0700 root:root) | yes | denied |
+
+`--user=1000:1000` now rides alongside a tmpfs owned `uid=1000,gid=1000`, from
+one constant rather than two literals that have to agree — which is how most of
+this file's defects started. The feared cost did not materialise: pipelines,
+`ps`, `df`, `date` and the session's own diagnostics all still run, and `/work`
+stays writable because it is owned by the user now instead of merely mounted for
+them.
 
 **Behaviour gate** — `make test-behaviour`. Five of this product's behaviours are
 produced by a *prompt*, not by code: an injected Memory constraint changing an

src/opspilot/sandbox/docker_l2.py

@@ -29,6 +29,10 @@
 # made that silent.
 _SECCOMP_PROFILE: Final[Path] = _SPECS_DIR / "sandbox" / "policies" / "seccomp.template.json"
 _DEFAULT_IMAGE = "alpine:3.19"
+# The process runs as this id and the tmpfs is owned by it. One constant for
+# both: the argv already carried `uid=1000` while the container ran as root,
+# and two literals that have to agree is how most of this file's defects began.
+_SANDBOX_UID: Final[int] = 1000
 
 
 def _mem_to_docker(mem: str) -> str:
@@ -56,7 +60,8 @@ def _build_docker_args(request: ActionRequest, image: str, runtime: str | None =
         "run",
         "--rm",
         "--read-only",
-        f"--tmpfs={workdir}:size={disk},uid=1000",
+        f"--tmpfs={workdir}:size={disk},uid={_SANDBOX_UID},gid={_SANDBOX_UID}",
+        f"--user={_SANDBOX_UID}:{_SANDBOX_UID}",
         "--cap-drop=ALL",
         "--security-opt=no-new-privileges",
         f"--memory={_mem_to_docker(p.resource.memory)}",

tests/test_sandbox_containment.py

@@ -118,6 +118,25 @@ def test_a_subprocess_can_be_created() -> None:
     assert "one" in stdout
 
 
+@needs_docker
+def test_the_process_is_not_root() -> None:
+    stdout, _, _ = _run("id -u")
+    assert stdout.strip() == "1000", stdout
+
+
+@needs_docker
+def test_a_root_owned_file_cannot_be_read() -> None:
+    """Dropping capabilities is not the same as not being root.
+
+    DAC grants access on an ownership match, with no capability involved, so a
+    root process reads root-owned files even with `CapEff=0`. The argv carried
+    `uid=1000` on the tmpfs while the process ran as root, and `/etc/shadow`
+    (0640 root:shadow) was readable.
+    """
+    stdout, _, _ = _run("cat /etc/shadow >/dev/null 2>&1 && echo READ || echo denied")
+    assert "denied" in stdout, stdout
+
+
 @needs_docker
 def test_the_process_holds_no_capabilities() -> None:
     stdout, _, _ = _run("grep -E '^(CapEff|CapBnd):' /proc/self/status")