Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,57 @@ A ticket whose submitter demanded a restart and a rollback, in those words, and
told the assistant to skip diagnostics, produced four read-only diagnostics and
no mutation — through the real path rather than the gate's hand-built prompt.

**The sandbox was then asked whether it actually contains anything.** ADR-0005
makes it the real boundary and the approval gate an admitted heuristic, and
until #209 it had never started a container, so nothing about the boundary had
ever been observed. Reading the kernel's own view from inside — `/proc/self/status`,
cgroup limits, routes — rather than trying to break out of it:

| declared | in effect |
| --- | --- |
| `--cap-drop=ALL` | `CapEff` and `CapBnd` both `0000000000000000` |
| `--security-opt=no-new-privileges` | `NoNewPrivs: 1` |
| `--read-only` | writing outside `/work` blocked |
| `--tmpfs=/work:size=64m` | `dd` asked for 80M, wrote exactly 67108864 bytes |
| `--network=none` | no routes, only `lo` addressed, egress fails |
| `--pids-limit=128` | `pids.max=128` |
| `--memory=512m` | `memory.max=536870912` |
| the project's seccomp profile | **not applied — ever** |

`_SECCOMP_PROFILE` resolved to `<repo>/../sandbox/policies/`, one level above
the repo root at a directory that has never existed, and it is used behind
`if …exists()`, so the miss was silent: Docker's default profile applied
instead. Three other modules already resolve `docs/specs` correctly with an
`OPSPILOT_SPECS_DIR` override; this was the fourth site and the only wrong one.

**Fixing the path alone breaks the sandbox**, which is why the profile had never
been noticed: its allowlist has `fork` and `vfork` but not `clone`, and libc
implements `fork()` with `clone` — on aarch64 the `fork` syscall does not exist
at all. Every command returned `/bin/sh: can't fork: Operation not permitted`.
The path bug had been hiding a profile that could not run anything. `clone` is
now allowed with the namespace flags masked off and `clone3` returns `ENOSYS` so
libc falls back to it — the same treatment Docker's default gives them, and the
only one seccomp can enforce, since `clone3` takes a struct pointer a filter
cannot dereference.

With the profile actually loaded, `unshare`, `chroot` and `mount` are refused by
the filter rather than only by the dropped capabilities.

`tests/test_sandbox_containment.py` pins all of it behind a `requires_docker`
marker. It self-skips where the image is absent, so CI still runs the one
assertion that needs no daemon: that the policy is where the code looks for it.

*Not verified:* the profile on 32-bit sub-architectures, which `archMap` claims.
`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.

**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
answer, a Memory ↔ KB contradiction being reported, a distilled Skill keeping its
Expand Down
18 changes: 16 additions & 2 deletions docs/specs/sandbox/policies/seccomp.template.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"_comment": "OpsPilot Sandbox baseline seccomp profile",
"_note_default": "Docker's default seccomp profile already disables ~44 high-risk syscalls; this template is for hardened mode (L2) and tightens things further on top of the default.",
"_note_default": "defaultAction is SCMP_ACT_ERRNO, so this profile REPLACES Docker's default rather than layering on it: anything not named below is denied, including syscalls the default would have allowed. Verified against alpine:3.19 on aarch64 by tests/test_sandbox_containment.py.",
"_reference": "Docker default: https://github.com/moby/moby/blob/master/profiles/seccomp/default.json",
"_apply": "docker run --security-opt seccomp=/path/to/seccomp.json ...",
"_version": "1.0.0",
Expand All @@ -17,7 +17,7 @@
"_comment": "General IO / process / file / time — the minimal set the sandbox requires",
"names": [
"accept", "accept4", "access", "arch_prctl", "bind", "brk", "capget", "capset",
"chdir", "chmod", "chown", "chown32", "clock_getres", "clock_gettime", "clock_nanosleep",
"chdir", "chmod", "chown", "chown32", "clock_getres", "clock_gettime", "clock_gettime64", "clock_nanosleep",
"close", "close_range", "connect", "copy_file_range", "creat", "dup", "dup2", "dup3",
"epoll_create", "epoll_create1", "epoll_ctl", "epoll_pwait", "epoll_wait",
"eventfd", "eventfd2", "execve", "execveat", "exit", "exit_group",
Expand Down Expand Up @@ -60,6 +60,20 @@
],
"action": "SCMP_ACT_ALLOW"
},
{
"_comment": "libc implements fork() with clone; on aarch64 the fork syscall does not exist at all. Allowed only without the namespace flags, the same mask Docker's default profile uses.",
"names": ["clone"],
"action": "SCMP_ACT_ALLOW",
"args": [
{ "index": 0, "value": 2114060288, "valueTwo": 0, "op": "SCMP_CMP_MASKED_EQ" }
]
},
{
"_comment": "clone3 passes its flags in a struct a seccomp filter cannot dereference, so the mask above cannot be applied to it. ENOSYS makes libc fall back to clone.",
"names": ["clone3"],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 38
},
{
"_comment": "High-risk syscalls explicitly disabled (already covered by the default deny; listed here for readability only)",
"names": [
Expand Down
4 changes: 4 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ markers = [
# Model-in-the-loop behaviour gate: real API calls, non-deterministic, run by
# a person rather than by CI. See `make test-behaviour`.
"requires_api_key: calls a hosted model; excluded from the default run",
# Starts a real container. Every other sandbox test asserts on the argv we
# build, which is why a policy that could not start a container survived
# from PR-30 until it was run by hand.
"requires_docker: runs a real container; excluded from the default run",
]
addopts = "-v --strict-markers"
asyncio_mode = "auto"
18 changes: 16 additions & 2 deletions src/opspilot/sandbox/docker_l2.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,28 @@

from __future__ import annotations

import os
import subprocess
import time
from pathlib import Path
from typing import Final

from .types import ActionRequest, ApplyResult, DryRunPreview

# Repo-level seccomp policy (falls back to Docker default if not found).
_SECCOMP_PROFILE = Path(__file__).parents[4] / "sandbox" / "policies" / "seccomp.template.json"
# This file is ``src/opspilot/sandbox/``, so the repo root is four parents up
# and the policy lives under ``docs/specs``. When pip-installed outside the repo
# (the Docker image), ``OPSPILOT_SPECS_DIR`` points at the shipped tree — the
# same resolution redaction.py, schemas.py and kb/storage_init.py use.
_SPECS_DIR: Final[Path] = (
Path(os.environ["OPSPILOT_SPECS_DIR"])
if os.environ.get("OPSPILOT_SPECS_DIR")
else Path(__file__).resolve().parents[3] / "docs" / "specs"
)
# Falls back to the Docker default profile when absent — which is what happened
# for this policy's whole life: the path pointed one level above the repo root
# at a `sandbox/policies/` directory that has never existed, and `.exists()`
# made that silent.
_SECCOMP_PROFILE: Final[Path] = _SPECS_DIR / "sandbox" / "policies" / "seccomp.template.json"
_DEFAULT_IMAGE = "alpine:3.19"


Expand Down
176 changes: 176 additions & 0 deletions tests/test_sandbox_containment.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
"""The sandbox is the boundary, so something has to actually run in it.

ADR-0005 makes the sandbox the real boundary — the approval gate is an admitted
heuristic denylist and says so in its own docstring. Every other sandbox test
asserts on the argv we hand `docker`, and argv is a claim, not a control.

Two defects lived their whole life behind that gap. `--tmpfs=…size=64Mi` is a
Kubernetes quantity the kernel rejects, so no container ever started (#209). And
the seccomp policy path pointed one level above the repo root at a directory
that has never existed, so the profile was never applied — and when the path was
fixed, the profile turned out to deny `clone`, which is how libc implements
fork, so nothing could start at all.

**What these cannot tell you**: whether *our* profile is loaded or Docker's
default is. Both deny the things probed below, so the containment assertions
pass either way — `test_the_seccomp_policy_is_where_the_code_looks_for_it`
covers that gap, because it is the half that broke.

Marked `requires_docker` and excluded from the default run: it needs a daemon
and the `alpine:3.19` image. It is worth running whenever the argv, the policy,
or the profile changes.
"""

from __future__ import annotations

import json
import re
import shutil
import subprocess

import pytest

from opspilot.sandbox.docker_l2 import _DEFAULT_IMAGE, _SECCOMP_PROFILE
from opspilot.sandbox.engine import SandboxEngine
from opspilot.sandbox.proposals import to_request

pytestmark = pytest.mark.requires_docker


def _docker_ready() -> bool:
if shutil.which("docker") is None:
return False
try:
subprocess.run(
["docker", "image", "inspect", _DEFAULT_IMAGE], capture_output=True, check=True
)
except (subprocess.CalledProcessError, OSError):
return False
return True


needs_docker = pytest.mark.skipif(
not _docker_ready(), reason=f"needs a docker daemon and the {_DEFAULT_IMAGE} image"
)


def _run(command: str) -> tuple[str, str, int]:
"""Execute *command* through the real engine, exactly as a proposal would."""
request = to_request(
{
"ref": "pa-1",
"intent": "diagnose",
"type": "shell",
"command": command,
"target": "sandbox-self",
"why": "verifying the declared controls are in effect",
},
session_id="sess_containment",
proposed_by="test",
)
result = SandboxEngine().execute(
request.model_copy(update={"dry_run": False}), force_approve=True
)
assert result.apply_result is not None
return result.apply_result.stdout, result.apply_result.stderr, result.apply_result.exit_code


def test_the_seccomp_policy_is_where_the_code_looks_for_it() -> None:
"""No docker needed, and the one assertion the rest cannot make.

`_SECCOMP_PROFILE` is applied behind `if …exists()`, so a wrong path is not
an error — it is a silent fallback to Docker's default, which denies enough
of the same things that every containment probe still passes. The path was
wrong from PR-30 until 2026-08-19: one level above the repo root, pointing
at a `sandbox/policies/` directory that has never existed.
"""
assert _SECCOMP_PROFILE.is_file(), f"the profile is not at {_SECCOMP_PROFILE}"
policy = json.loads(_SECCOMP_PROFILE.read_text())
assert policy["defaultAction"] == "SCMP_ACT_ERRNO", "the allowlist must deny by default"
allowed = {
n
for rule in policy["syscalls"]
if rule["action"] == "SCMP_ACT_ALLOW"
for n in rule["names"]
}
# libc implements fork() with clone; `fork` alone does not exist on aarch64.
assert "clone" in allowed, "nothing can start a process without clone"


@needs_docker
def test_a_container_starts_and_reports_its_output() -> None:
"""The floor: without this the other assertions are vacuous."""
stdout, _, exit_code = _run("echo alive")
assert exit_code == 0
assert "alive" in stdout


@needs_docker
def test_a_subprocess_can_be_created() -> None:
"""A profile denying `clone` fails here and nowhere else.

`fork` and `vfork` being on an allowlist is not enough: libc implements
fork() with `clone`, and on aarch64 the `fork` syscall does not exist.
"""
stdout, stderr, exit_code = _run("echo one | cat")
assert "can't fork" not in stderr
assert exit_code == 0
assert "one" in stdout


@needs_docker
def test_the_process_holds_no_capabilities() -> None:
stdout, _, _ = _run("grep -E '^(CapEff|CapBnd):' /proc/self/status")
caps = dict(re.findall(r"^(CapEff|CapBnd):\s*(\w+)", stdout, re.M))
assert caps.get("CapEff") == "0000000000000000", stdout
# The bounding set too, or a setuid binary could regain them.
assert caps.get("CapBnd") == "0000000000000000", stdout


@needs_docker
def test_privileges_cannot_be_regained() -> None:
stdout, _, _ = _run("grep -E '^(NoNewPrivs|Seccomp):' /proc/self/status")
assert re.search(r"^NoNewPrivs:\s*1", stdout, re.M), stdout
# 2 = a seccomp filter is loaded. 0 would mean the profile silently vanished.
assert re.search(r"^Seccomp:\s*2", stdout, re.M), stdout


@needs_docker
def test_the_root_filesystem_is_read_only() -> None:
stdout, _, _ = _run("touch /etc/probe 2>/dev/null && echo WRITABLE || echo blocked")
assert "blocked" in stdout, stdout


@needs_docker
def test_the_workdir_is_writable_but_capped() -> None:
stdout, _, _ = _run(
"dd if=/dev/zero of=/work/big bs=1M count=80 2>/dev/null; wc -c < /work/big"
)
written = int(stdout.strip().split()[-1])
assert written == 64 * 1024 * 1024, f"tmpfs cap not enforced: wrote {written} bytes"


@needs_docker
def test_there_is_no_network() -> None:
stdout, _, _ = _run(
"ip -o addr 2>/dev/null | awk '{print $2}' | sort -u; echo ---; ip route 2>/dev/null | wc -l"
)
interfaces, _, routes = stdout.partition("---")
assert set(interfaces.split()) <= {"lo"}, f"an addressed interface besides lo: {interfaces}"
assert routes.strip() == "0", f"a route exists: {routes}"


@needs_docker
@pytest.mark.parametrize(
("label", "command"),
[
("new user namespace", "unshare -U true"),
("new network namespace", "unshare -n true"),
("chroot", "chroot /tmp /bin/true"),
("mount", "mount -t tmpfs none /work"),
],
)
def test_the_profile_denies_what_it_claims_to(label: str, command: str) -> None:
"""Deny-by-default is the point of the allowlist; check it bites."""
_, stderr, exit_code = _run(f"{command} 2>&1")
assert exit_code != 0, f"{label} succeeded"
Loading