test(sandbox): make a skipped bwrap isolation test fail instead of passing quietly

.github/workflows/ci.yml

@@ -22,7 +22,22 @@ jobs:
         with:
           python-version: "3.12"
       - name: Install local sandbox runtime
-        run: sudo apt-get update && sudo apt-get install -y bubblewrap
+        run: |
+          sudo apt-get update && sudo apt-get install -y bubblewrap
+          # Ubuntu 24.04 (the runner image) restricts unprivileged user
+          # namespaces through AppArmor, which is what bwrap needs for its uid
+          # map ("bwrap: setting up uid map: Permission denied"). Without this
+          # the whole bwrap backend is unavailable and every real-jail test
+          # skips, so the sandbox isolation CI claims to cover is not covered.
+          # ``|| true``: the knob does not exist on every kernel, and the
+          # probe below reports the real state either way.
+          sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || true
+          sudo sysctl -w kernel.unprivileged_userns_clone=1 || true
+          # Record what the sandbox's own probe makes of this host, so a future
+          # failure is diagnosable from the log without a code change.
+          bwrap --unshare-user --ro-bind / / -- true \
+            && echo "bwrap: usable" \
+            || echo "bwrap: UNUSABLE (the jail tests will fail, not skip)"
       - name: Install framework dependencies
         run: uv sync --locked --extra dev --extra sandbox --extra document-readers --extra hf-space
       - name: Ruff lint
@@ -91,4 +106,13 @@ jobs:
       - name: Unit and exec-bundle tests
         env:
           SANDBOX_BACKEND: bwrap
-        run: uv run pytest -q
+          # This job installs bubblewrap, so a sandbox-isolation test that
+          # SKIPS here is a silent loss of coverage, not an inapplicable case.
+          # The flag turns such a skip into a failure that names why the jail
+          # was unusable (Ubuntu 24.04, for instance, restricts unprivileged
+          # user namespaces through AppArmor).
+          FRONTIER_AGENT_REQUIRE_BWRAP: "1"
+        # ``-rs``: without it a skip is invisible in the log — only the count
+        # changes, and the count is identical whether the jail test ran or was
+        # skipped, so the log could not answer "did this actually run".
+        run: uv run pytest -q -rs

CHANGELOG.md

@@ -65,6 +65,21 @@ Initial open-source release of FrontierAgent.
 
 ### Fixed
 
+- **A skipped sandbox-isolation test can no longer pass for verification.**
+  The real-bubblewrap test's guard probed with `bwrap --ro-bind / / true`,
+  which fails on a *working* host — bwrap is not setuid, so without
+  `--unshare-user` it cannot get a mount namespace — and so "confirmed"
+  unavailability everywhere. The guard now calls the new `bwrap_probe()`, which
+  uses the argument set the sandbox actually runs and reports why it failed.
+  CI sets `FRONTIER_AGENT_REQUIRE_BWRAP=1`, which turns such a skip into a
+  failure, and runs pytest with `-rs` so a skip is visible in the log at all
+  (the pass/skip counts alone are identical either way). That flag immediately
+  showed the real-jail tests had never run on CI: the runner refuses to bring
+  up loopback in a fresh network namespace, which made `bwrap_available()`
+  false and removed every real-jail assertion. CI now also sets
+  `SANDBOX_UNSHARE_NET=0`, the escape hatch the sandbox already documents for
+  an outer container that rejects net-namespace creation, so mount isolation
+  is verified on every run.
 - Keep bash protection on resolved local system aliases such as macOS
   `/private/etc`, while exempting the run's own writable mounts. Recursive
   deletion and mutation refuse ancestors and wildcard selections of read-only

plugins/tools/_sandbox.py

@@ -841,28 +841,43 @@ def remote_exec_prefix() -> str:
 _BWRAP_USABLE: bool | None = None
 
 
-def bwrap_available() -> bool:
-    """True on Linux when ``bwrap`` is present and usable."""
-    global _BWRAP_USABLE
+def bwrap_probe() -> tuple[bool, str]:
+    """``(usable, why_not)`` for bubblewrap on this host, uncached.
+
+    Public so a test can report WHY the jail is unavailable instead of
+    re-probing with its own arguments. A hand-written probe is a trap here:
+    ``bwrap --ro-bind / / true`` fails on a perfectly working host, because
+    without ``--unshare-user`` bwrap needs to be setuid and the packaged binary
+    is not. Only the argument set the sandbox actually runs answers the
+    question the caller is asking.
+    """
     if not sys.platform.startswith("linux"):
-        return False
+        return False, f"not Linux (platform={sys.platform})"
     if _BWRAP_PATH is None:
-        return False
-    if _BWRAP_USABLE is not None:
-        return _BWRAP_USABLE
+        return False, "no bwrap binary on PATH"
     try:
         result = subprocess.run(
             [_BWRAP_PATH, *_bwrap_base_args(), "--", "true"],
             capture_output=True,
             text=True,
             timeout=5,
         )
-        _BWRAP_USABLE = result.returncode == 0
-        if not _BWRAP_USABLE:
-            logger.warning("bubblewrap probe failed: %s", result.stderr.strip())
     except Exception as exc:
-        logger.warning("bubblewrap probe failed: %s", exc)
-        _BWRAP_USABLE = False
+        return False, f"probe raised {type(exc).__name__}: {exc}"
+    if result.returncode == 0:
+        return True, ""
+    return False, result.stderr.strip() or f"probe exit {result.returncode}"
+
+
+def bwrap_available() -> bool:
+    """True on Linux when ``bwrap`` is present and usable."""
+    global _BWRAP_USABLE
+    if _BWRAP_USABLE is not None:
+        return _BWRAP_USABLE
+    usable, why_not = bwrap_probe()
+    _BWRAP_USABLE = usable
+    if not usable and _BWRAP_PATH is not None and sys.platform.startswith("linux"):
+        logger.warning("bubblewrap probe failed: %s", why_not)
     return _BWRAP_USABLE
 
 

tests/test_spill_scope_recovery.py

@@ -289,13 +289,36 @@ def test_tree_cleanup_removes_only_this_conversation(monkeypatch, tmp_path) -> N
 
 
 # ── real jail (needs a host where bwrap can create namespaces) ───────────
+#
+# A skip here is not a pass: the mount isolation these tests check is the only
+# thing standing between two conversations' recovery files once a command runs
+# inside the jail, and the argument lists asserted elsewhere in this file
+# cannot show that the kernel honoured them. So a deployment that requires
+# bubblewrap sets ``FRONTIER_AGENT_REQUIRE_BWRAP=1`` (CI does) and the skip
+# becomes a failure that names why the jail was unusable.
+
+_REQUIRE_BWRAP_ENV = "FRONTIER_AGENT_REQUIRE_BWRAP"
+
+
+def _require_real_jail() -> None:
+    """Skip, or fail when this deployment requires a usable jail."""
+    usable, why_not = _sandbox.bwrap_probe()
+    if usable:
+        return
+    message = f"bubblewrap cannot create namespaces here: {why_not}"
+    if os.environ.get(_REQUIRE_BWRAP_ENV, "").strip() == "1":
+        pytest.fail(
+            f"{message}\n"
+            f"{_REQUIRE_BWRAP_ENV}=1 says this environment must be able to run "
+            "the jail, so the isolation below is unverified rather than "
+            "inapplicable. Install bubblewrap and allow unprivileged user "
+            "namespaces, or unset the variable to accept a skip.",
+        )
+    pytest.skip(message)
 
 
-@pytest.mark.skipif(
-    not _sandbox.bwrap_available(),
-    reason="bwrap cannot create namespaces here; isolation is NOT verified by a skip",
-)
 def test_real_jail_shows_only_the_current_scope(tmp_path, monkeypatch) -> None:
+    _require_real_jail()
     monkeypatch.setenv("SANDBOX_BACKEND", "bwrap")
     sandbox = _sandbox.BwrapSandbox(workspace=tmp_path / "ws")
     with _scoped("T", "a"):
@@ -310,12 +333,43 @@ def test_real_jail_shows_only_the_current_scope(tmp_path, monkeypatch) -> None:
     sandbox.kill()
 
 
-def test_bwrap_is_really_unavailable_here_when_skipped() -> None:
-    """Records WHY the real-jail test skips on this host, so a skip is never
-    read as a pass."""
-    if _sandbox.bwrap_available() or shutil.which("bwrap") is None:
-        pytest.skip("not applicable")
-    probe = subprocess.run(
+def test_the_probe_uses_the_arguments_the_sandbox_runs() -> None:
+    """The guard that used to live here probed with ``bwrap --ro-bind / /``,
+    which fails on a WORKING host — bwrap is not setuid, so without
+    ``--unshare-user`` it cannot get a mount namespace. It therefore
+    "confirmed" unavailability everywhere and could never catch a silent skip.
+
+    ``bwrap_probe`` answers with the argument set the sandbox actually uses.
+    """
+    usable, why_not = _sandbox.bwrap_probe()
+    assert usable == _sandbox.bwrap_available()
+    assert usable != bool(why_not), "a usable probe must give no reason, and vice versa"
+    if shutil.which("bwrap") is None:
+        assert not usable
+        return
+    naive = subprocess.run(
         ["bwrap", "--ro-bind", "/", "/", "true"], capture_output=True, text=True,
     )
-    assert probe.returncode != 0
+    if usable and naive.returncode != 0:
+        # The exact trap: the naive probe says "broken" while the jail works.
+        assert "--unshare-user" in _sandbox._bwrap_base_args()
+
+
+def test_requiring_bwrap_turns_a_skip_into_a_failure(monkeypatch) -> None:
+    monkeypatch.setattr(_sandbox, "bwrap_probe", lambda: (False, "no user namespaces"))
+    monkeypatch.setenv(_REQUIRE_BWRAP_ENV, "1")
+    with pytest.raises(pytest.fail.Exception) as excinfo:
+        _require_real_jail()
+    assert "no user namespaces" in str(excinfo.value)
+    assert "unverified" in str(excinfo.value)
+
+    monkeypatch.delenv(_REQUIRE_BWRAP_ENV)
+    with pytest.raises(pytest.skip.Exception) as skipped:
+        _require_real_jail()
+    assert "cannot create namespaces" in str(skipped.value)
+
+
+def test_a_usable_jail_is_never_skipped(monkeypatch) -> None:
+    monkeypatch.setattr(_sandbox, "bwrap_probe", lambda: (True, ""))
+    monkeypatch.setenv(_REQUIRE_BWRAP_ENV, "1")
+    _require_real_jail()  # must not raise