fix(sandbox): stop with-skill builds poisoning no-skill rollouts

src/benchflow/agents/install.py

@@ -365,3 +365,49 @@ async def deploy_skills(
         label = agent_cfg.name if agent_cfg else "oracle"
         if count:
             logger.info(f"Skills distributed to {count} paths for {label}")
+
+
+async def assert_no_skill_isolation(env, agent_cfg, sandbox_user: str | None) -> None:
+    """Fail a no-skill rollout that can still reach skills.
+
+    The no-skill condition is the primary capability signal for skill
+    benchmarks, and until now nothing verified it: benchflow could resolve a
+    rollout as no-skill, correctly skip every deployment path, and still start
+    the agent in an image another rollout had baked skills into. The run then
+    reported a normal pass and no counter disagreed — ``total_skill_invocations``
+    stays at 0 whether or not the agent read a SKILL.md off disk.
+
+    Check the agent's own discovery paths plus the canonical mount point, and
+    raise rather than let a contaminated run be recorded as a result.
+    """
+    home = f"/home/{sandbox_user}" if sandbox_user else "/root"
+    candidates = ["/skills"]
+    if agent_cfg is not None:
+        for skill_path in agent_cfg.skill_paths:
+            candidates.append(skill_path.replace("$HOME", home))
+
+    quoted = " ".join(shlex.quote(path) for path in dict.fromkeys(candidates))
+    # -L so a symlinked discovery path is followed to the real tree.
+    probe = (
+        f"for p in {quoted}; do "
+        'if [ -e "$p" ]; then find -L "$p" -name SKILL.md 2>/dev/null; fi; '
+        "done"
+    )
+    result = await env.exec(probe, timeout_sec=20)
+    # Only absolute paths to a SKILL.md count. The probe's stdout can also carry
+    # shell noise, and a substring match would turn that into a false positive.
+    found = [
+        line.strip()
+        for line in (result.stdout or "").splitlines()
+        if line.strip().startswith("/") and line.strip().endswith("/SKILL.md")
+    ]
+    if not found:
+        return
+
+    raise RuntimeError(
+        "no-skill rollout is contaminated: skills are reachable inside the "
+        f"sandbox at {', '.join(found[:5])}"
+        + (f" (+{len(found) - 5} more)" if len(found) > 5 else "")
+        + ". The agent can read mentor guidance the condition is meant to "
+        "withhold, so the result would not measure no-skill capability."
+    )

src/benchflow/contracts/planes.py

@@ -71,6 +71,10 @@ async def seed_verifier_workspace(
         self, env: Any, *, workspace: str, sandbox_user: str | None
     ) -> None: ...
     async def deploy_skills(self, *args: Any, **kwargs: Any) -> None: ...
+
+    async def assert_no_skill_isolation(
+        self, *args: Any, **kwargs: Any
+    ) -> None: ...
     async def lockdown_paths(self, env: Any, locked_paths: list[str]) -> None: ...
     async def install_agent(
         self,

src/benchflow/rollout/__init__.py

@@ -1150,6 +1150,12 @@ async def install_agent(self) -> None:
             self._agent_cwd,
             skills_sandbox_dir=self._effective_skills_sandbox_dir,
         )
+        if cfg.recorded_skill_mode == SKILL_MODE_NO_SKILL:
+            # Verify the condition rather than assume it: a stale image can
+            # carry skills that no deployment path in this rollout put there.
+            await self._planes.assert_no_skill_isolation(
+                self._env, self._agent_cfg, cfg.sandbox_user
+            )
         if cfg.export_generated_skills_to:
             await _ensure_sandbox_dir(
                 self._env, cfg.generated_skills_root, cfg.sandbox_user

src/benchflow/rollout_planes.py

@@ -20,6 +20,7 @@
 from benchflow.agents.install import (
     _link_skill_paths,
     apply_web_tool_policy,
+    assert_no_skill_isolation,
     deploy_skills,
     install_agent,
 )
@@ -146,6 +147,9 @@ async def seed_verifier_workspace(
     async def deploy_skills(self, *args: Any, **kwargs: Any) -> None:
         await deploy_skills(*args, **kwargs)
 
+    async def assert_no_skill_isolation(self, *args: Any, **kwargs: Any) -> None:
+        await assert_no_skill_isolation(*args, **kwargs)
+
     async def lockdown_paths(self, env: Any, locked_paths: list[str]) -> None:
         await lockdown_paths(env, locked_paths)
 

src/benchflow/sandbox/docker.py

@@ -8,6 +8,7 @@
 import asyncio
 import asyncio.subprocess
 import contextlib
+import hashlib
 import json
 import logging
 import os
@@ -69,6 +70,36 @@ def _sanitize_docker_image_name(name: str) -> str:
     return name
 
 
+def _build_context_fingerprint(environment_dir: Path) -> str:
+    """Short digest of the inputs that decide what the built image contains.
+
+    The image tag must distinguish builds whose contents differ. The Dockerfile
+    is rewritten in place when skills are baked in (``--skills-dir`` appends
+    ``COPY _deps/skills`` plus symlinks into every agent skill path), so a tag
+    derived from the task name alone is shared by a with-skill and a no-skill
+    build of the same task. Whichever built last wins, and the other rollout
+    starts from it — a no-skill rollout then runs in an image that already
+    contains ``/skills`` and the agent-side symlinks, silently invalidating the
+    no-skill condition.
+
+    Hashing the Dockerfile, and the presence of the staged skills payload,
+    gives the two builds different tags.
+    """
+    hasher = hashlib.sha256()
+    dockerfile = environment_dir / "Dockerfile"
+    if dockerfile.is_file():
+        hasher.update(dockerfile.read_bytes())
+    # The staged payload is what COPY pulls in; include its layout so a change
+    # of skills — not just of the Dockerfile line — yields a different tag.
+    skills_root = environment_dir / "_deps" / "skills"
+    if skills_root.is_dir():
+        for path in sorted(skills_root.rglob("*")):
+            hasher.update(str(path.relative_to(skills_root)).encode())
+            if path.is_file():
+                hasher.update(str(path.stat().st_size).encode())
+    return hasher.hexdigest()[:12]
+
+
 def _sanitize_docker_compose_project_name(name: str) -> str:
     name = name.lower()
     if not re.match(r"^[a-z0-9]", name):
@@ -191,7 +222,10 @@ def __init__(
         )
 
         self._env_vars = DockerSandboxEnvVars(
-            main_image_name=_sanitize_docker_image_name(f"bf__{environment_name}"),
+            main_image_name=_sanitize_docker_image_name(
+                f"bf__{environment_name}__"
+                f"{_build_context_fingerprint(self.environment_dir)}"
+            ),
             context_dir=str(self.environment_dir.resolve().absolute()),
             host_verifier_logs_path=verifier_dir,
             host_agent_logs_path=agent_dir,

tests/test_no_skill_image_isolation.py

@@ -0,0 +1,118 @@
+"""Regression tests for no-skill rollout isolation.
+
+A with-skill build rewrites the task Dockerfile in place (``COPY _deps/skills``
+plus symlinks into every agent skill path). While the image tag was derived from
+the task name alone, that build and a no-skill build of the same task shared one
+tag, so whichever ran last decided what both started from — and a no-skill
+rollout could silently run inside an image containing ``/skills``.
+"""
+
+from __future__ import annotations
+
+from typing import ClassVar
+
+import pytest
+
+from benchflow.agents.install import assert_no_skill_isolation
+from benchflow.sandbox.docker import _build_context_fingerprint
+
+
+def _write_env(tmp_path, dockerfile_body: str, skills: dict[str, str] | None = None):
+    env_dir = tmp_path / "environment"
+    env_dir.mkdir(parents=True, exist_ok=True)
+    (env_dir / "Dockerfile").write_text(dockerfile_body)
+    if skills:
+        for rel, content in skills.items():
+            target = env_dir / "_deps" / "skills" / rel
+            target.parent.mkdir(parents=True, exist_ok=True)
+            target.write_text(content)
+    return env_dir
+
+
+BASE_DOCKERFILE = "FROM python:3.12-slim\nCOPY payload.stl /root/payload.stl\n"
+SKILL_INJECTED = BASE_DOCKERFILE + (
+    "\n# Skills directory (injected by benchflow --skills-dir)\n"
+    "COPY _deps/skills /skills/\n"
+    "RUN mkdir -p /home/agent/.opencode && ln -sf /skills /home/agent/.opencode/skills\n"
+)
+
+
+def test_fingerprint_is_stable_for_identical_context(tmp_path):
+    a = _write_env(tmp_path / "a", BASE_DOCKERFILE)
+    b = _write_env(tmp_path / "b", BASE_DOCKERFILE)
+    assert _build_context_fingerprint(a) == _build_context_fingerprint(b)
+
+
+def test_skill_injection_changes_the_fingerprint(tmp_path):
+    """The core regression: the two builds must not share an image tag."""
+    clean = _write_env(tmp_path / "clean", BASE_DOCKERFILE)
+    injected = _write_env(
+        tmp_path / "injected",
+        SKILL_INJECTED,
+        skills={"mentor/SKILL.md": "# mentor\n"},
+    )
+    assert _build_context_fingerprint(clean) != _build_context_fingerprint(injected)
+
+
+def test_changing_skill_payload_changes_the_fingerprint(tmp_path):
+    one = _write_env(
+        tmp_path / "one", SKILL_INJECTED, skills={"mentor/SKILL.md": "# mentor\n"}
+    )
+    two = _write_env(
+        tmp_path / "two",
+        SKILL_INJECTED,
+        skills={"mentor/SKILL.md": "# mentor\n", "second/SKILL.md": "# second\n"},
+    )
+    assert _build_context_fingerprint(one) != _build_context_fingerprint(two)
+
+
+class _FakeEnv:
+    """Minimal env double returning canned `find` output."""
+
+    def __init__(self, stdout: str):
+        self._stdout = stdout
+        self.commands: list[str] = []
+
+    async def exec(self, cmd: str, timeout_sec: int = 20):
+        self.commands.append(cmd)
+
+        class _Result:
+            stdout = self._stdout
+            stderr = ""
+            return_code = 0
+
+        return _Result()
+
+
+class _FakeAgentCfg:
+    skill_paths: ClassVar[list[str]] = ["$HOME/.opencode/skills"]
+
+
+@pytest.mark.asyncio
+async def test_isolation_check_passes_when_no_skills_present():
+    env = _FakeEnv("")
+    await assert_no_skill_isolation(env, _FakeAgentCfg(), "agent")
+    assert "/home/agent/.opencode/skills" in env.commands[0]
+    assert "/skills" in env.commands[0]
+
+
+@pytest.mark.asyncio
+async def test_isolation_check_raises_when_skills_are_reachable():
+    env = _FakeEnv("/skills/ion-shuttling-mentor/SKILL.md\n")
+    with pytest.raises(RuntimeError, match="no-skill rollout is contaminated"):
+        await assert_no_skill_isolation(env, _FakeAgentCfg(), "agent")
+
+
+@pytest.mark.asyncio
+async def test_isolation_check_handles_missing_agent_config():
+    """The oracle path passes no agent config; the mount point is still checked."""
+    env = _FakeEnv("")
+    await assert_no_skill_isolation(env, None, None)
+    assert "/skills" in env.commands[0]
+
+
+@pytest.mark.asyncio
+async def test_isolation_check_ignores_non_skill_output():
+    """Shell noise on stdout must not be mistaken for a contaminated sandbox."""
+    env = _FakeEnv("/workspace\nsome unrelated line\n")
+    await assert_no_skill_isolation(env, _FakeAgentCfg(), "agent")