From 818c72b85e2be9f5e36f49877bee82a588bb80cb Mon Sep 17 00:00:00 2001 From: Zhang Handuo Date: Tue, 6 Oct 2026 17:32:24 +0800 Subject: [PATCH 1/4] test(sandbox): make a skipped bwrap isolation test fail instead of passing quietly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #58. Three problems, one theme — the jail test could be skipped and nobody would know: - Its guard probed with `bwrap --ro-bind / / true`, which fails on a host where the jail works fine: bwrap is not setuid, so without `--unshare-user` it cannot get a mount namespace. The guard therefore "confirmed" unavailability everywhere and could never catch a silent skip. (It also made me report the isolation as unverified when it had in fact been running and passing all along.) - `bwrap_probe()` is split out of `bwrap_available()` and returns `(usable, why_not)` using the sandbox's own argument set, so a test reports the real reason instead of re-probing with its own flags. - `FRONTIER_AGENT_REQUIRE_BWRAP=1` (set in CI, which installs bubblewrap) turns the skip into a failure naming why the jail was unusable. Ubuntu 24.04 restricts unprivileged user namespaces through AppArmor, so this is a live possibility on the runner rather than a hypothetical. - CI runs `pytest -q -rs`: without it a skip is invisible, since the pass/skip counts come out identical whether the jail test ran or not. Verified: the jail test runs and passes here (and fails when the mount is reverted to the whole spill root, so it is not vacuous); with the probe forced to report failure and the flag set, the run fails instead of skipping. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 11 ++++- CHANGELOG.md | 9 ++++ plugins/tools/_sandbox.py | 39 ++++++++++----- tests/test_spill_scope_recovery.py | 76 +++++++++++++++++++++++++----- 4 files changed, 111 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee767e8..a8093b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,4 +91,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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 044ceb9..099148d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,15 @@ 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). - 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 diff --git a/plugins/tools/_sandbox.py b/plugins/tools/_sandbox.py index 82b25ef..94ce38f 100644 --- a/plugins/tools/_sandbox.py +++ b/plugins/tools/_sandbox.py @@ -841,15 +841,20 @@ 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"], @@ -857,12 +862,22 @@ def bwrap_available() -> bool: 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 diff --git a/tests/test_spill_scope_recovery.py b/tests/test_spill_scope_recovery.py index f41eecd..0d341f1 100644 --- a/tests/test_spill_scope_recovery.py +++ b/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 From d199e0e7a1ed8a53174bab3a2e14c14a54d90965 Mon Sep 17 00:00:00 2001 From: Zhang Handuo Date: Tue, 6 Oct 2026 17:44:35 +0800 Subject: [PATCH 2/4] ci: let bubblewrap run on the runner so the jail tests actually execute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The REQUIRE_BWRAP flag did its job on its own first CI run: it failed with bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted so `bwrap_available()` has been false on the runner all along and every real-jail assertion was silently skipped — the hypothesis the flag existed to test, now recorded in the log. The cause is `--unshare-net`: the runner will not let bwrap configure loopback inside a fresh network namespace. `SANDBOX_UNSHARE_NET=0` is the escape hatch `_bwrap_base_args` already documents for exactly this ("an outer container that rejects net-namespace creation"), and it feeds the probe, so the backend becomes available again. What CI verifies changes for the better: mount isolation (what the spill scope tests assert) now runs, where previously nothing did. Network isolation is not exercised here — it was not being exercised before either, since bwrap was unavailable outright. Confirmed with net unsharing off: 4433 passed, 1 skipped (the root-only test_userenv case), and the jail test still fails when the mount is reverted to the whole spill root, so it is not vacuous. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 9 +++++++++ CHANGELOG.md | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8093b3..d4a6a9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -97,6 +97,15 @@ jobs: # was unusable (Ubuntu 24.04, for instance, restricts unprivileged # user namespaces through AppArmor). FRONTIER_AGENT_REQUIRE_BWRAP: "1" + # The runner refuses to bring up loopback inside a fresh network + # namespace ("bwrap: loopback: Failed RTM_NEWADDR: Operation not + # permitted"), which made ``bwrap_available()`` false and silently + # removed ALL real-jail coverage. This is the escape hatch the + # sandbox already documents for an outer container that rejects + # net-namespace creation. Mount isolation — what the spill tests + # check — is unaffected; network isolation is simply not exercised + # here, as it was not being exercised before either. + SANDBOX_UNSHARE_NET: "0" # ``-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". diff --git a/CHANGELOG.md b/CHANGELOG.md index 099148d..62fdfd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,7 +73,13 @@ Initial open-source release of FrontierAgent. 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). + (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 From aaf99d46bc1288846835931583ee01a659ac4f3c Mon Sep 17 00:00:00 2001 From: Zhang Handuo Date: Tue, 6 Oct 2026 17:50:47 +0800 Subject: [PATCH 3/4] ci: allow unprivileged user namespaces so bwrap can build its uid map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the loopback problem out of the way the runner reported the next one: `bwrap: setting up uid map: Permission denied`. Ubuntu 24.04 — the runner image — restricts unprivileged user namespaces through AppArmor, which is exactly what bwrap needs. Both sysctls are set best-effort (the knob differs by kernel and is absent on some), and the step now prints whether bwrap is usable on this host, so the next person debugging this reads it from the log instead of inferring it from a test count. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d4a6a9d..4f44f5f 100644 --- a/.github/workflows/ci.yml +++ b/.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 From 0f04551780cd315722157713f8e712b9cc0c759c Mon Sep 17 00:00:00 2001 From: Zhang Handuo Date: Tue, 6 Oct 2026 19:40:39 +0800 Subject: [PATCH 4/4] ci: keep network isolation on and let the sysctl do the work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Testing whether the AppArmor restriction was the root cause of BOTH runner failures: the loopback RTM_NEWADDR error may have been a symptom of the user namespace never being created properly, not an independent limitation. If this run is green, bwrap works on the runner with its full argument set and nothing has to be weakened — which is the better outcome, and the one ApodexHarness's CI explicitly asks for: disabling --unshare-net would make a network-boundary test green but hollow. Frontier has no such test today, but there is no reason to configure CI as though it might. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4f44f5f..dc25b63 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,15 +112,6 @@ jobs: # was unusable (Ubuntu 24.04, for instance, restricts unprivileged # user namespaces through AppArmor). FRONTIER_AGENT_REQUIRE_BWRAP: "1" - # The runner refuses to bring up loopback inside a fresh network - # namespace ("bwrap: loopback: Failed RTM_NEWADDR: Operation not - # permitted"), which made ``bwrap_available()`` false and silently - # removed ALL real-jail coverage. This is the escape hatch the - # sandbox already documents for an outer container that rejects - # net-namespace creation. Mount isolation — what the spill tests - # check — is unaffected; network isolation is simply not exercised - # here, as it was not being exercised before either. - SANDBOX_UNSHARE_NET: "0" # ``-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".