Verifier hardening does not check post-kill quiescence before verifier startup

[kywch]

Summary

When sandbox_user is configured, BenchFlow's pre-verifier hardening tries to kill that user's processes, then continues toward verifier startup without checking a final post-kill observation that no process remains.

The hardening contract says it will "assert all sandbox-user processes are dead," but _kill_sandbox_user_procs() does not make a checked final quiescence observation:

• the helper issues two unchecked env.exec() calls;
• the hardening docstring promises an assertion, then continues after the helper returns; and
• _verify_rollout() starts the verifier immediately after hardening returns.

The second shell command probes before its last kill attempt, but never probes afterward. Its subshell also ends with sleep 1, so that sleep can supply a successful shell status even if the preceding pkill did not establish quiescence. BenchFlow ignores both exec results in any case.

Reproduction

At 74016df5aa8a87fe49ef8804442dc532bbf3d6d4 (current main while filing), add this regression test to tests/test_sandbox_hardening.py:

@pytest.mark.asyncio
async def test_process_kill_failure_is_not_accepted() -> None:
    from benchflow.sandbox.lockdown import _kill_sandbox_user_procs

    env = MagicMock()
    env.exec = AsyncMock(
        side_effect=[
            # Initial TERM/KILL command reported success.
            MagicMock(stdout="", stderr="", return_code=0),
            # Follow-up command could not establish the claimed state.
            MagicMock(stdout="1234\n", stderr="pgrep/kill failed", return_code=1),
        ]
    )

    with pytest.raises(RuntimeError, match="process"):
        await _kill_sandbox_user_procs(env, "agent")

Run:

uv run --extra dev pytest -q tests/test_sandbox_hardening.py::test_process_kill_failure_is_not_accepted

Actual result:

Failed: DID NOT RAISE <class 'RuntimeError'>

The helper returns normally despite the failed follow-up. harden_before_verify() therefore has no result from this helper that would stop the normal path toward verifier invocation.

This test deterministically covers the unchecked exec-result seam. It does not create a live surviving process or demonstrate an end-to-end verifier invocation. A separate integration regression test should model a process still present (or respawning) after the last kill and require a checked final zero-survivor observation before verifier invocation; checking only that pkill was attempted does not establish the observed post-kill state.

Expected behavior

For profiles that opt into sandbox_user, verifier execution should begin only after a checked post-kill observation reports that no process owned by that user remains. If BenchFlow cannot obtain that observation within a bounded interval, the rollout should expose an explicit verifier-hardening failure and should not emit a trusted reward.

Actual behavior

Exec results are ignored, and there is no checked probe after the final kill attempt. Hardening can proceed toward verifier setup and scoring without a successful post-kill zero-survivor observation.

Impact

This weakens the agent/verifier trust boundary. A surviving agent-owned process may race pre-verifier cleanup or retain an already-open writable file descriptor while the workspace and evidence are being frozen. Whether a specific task is exploitable depends on its filesystem layout and verifier inputs; this issue does not claim universal reward forgery or that a process probe eliminates all races. It means this hardening path currently lacks its documented checked post-kill observation.

Scope

• Applies to the main-container hardening path when sandbox_user is set.
• The required postcondition is zero processes owned by the designated sandbox user, not zero processes in the whole sandbox.
• sandbox_user=None has no separate-UID process-quiescence identity. Existing profiles using it should not be rejected as a side effect of this fix; any stronger policy for those profiles needs a separate contract.
• Target/database service processes in multi-container tasks are out of scope. Current hardening intentionally operates on the main agent container.
• This is a general BenchFlow verifier-integrity gap found during BenchGuard work, not a BenchGuard-specific artifact rule.

Acceptance signals

• A failed final zero-survivor observation, or an observation showing survivors, prevents verifier execution. Intermediate kill failures may be tolerated only if final quiescence is established.
• A sandbox-user process observed after the last kill prevents verifier execution.
• The final successful zero-survivor observation occurs after the final kill attempt.
• Failure is bounded and recorded as a clear verifier-hardening error; no trusted reward is retained for that attempt.
• A normal sandbox_user rollout with confirmed quiescence still reaches the verifier.
• Existing sandbox_user=None behavior is unchanged unless its contract is addressed separately.
• Tests assert ordering: kill attempts -> final observation -> verifier.

Possible direction (non-prescriptive)

One option is a bounded kill-and-probe loop followed by a checked, root-run zero-survivor probe. The exec return code must be normalized through the existing checked-exec path. Verifier startup should depend on a successful final observation, not merely on having sent pkill. This narrows the race window but is not an absolute proof against process creation after the probe.

Other implementations are fine if they provide equivalent, testable evidence and preserve supported profile behavior.

PR provenance

PR #1051, created from my agent-assisted BenchGuard work, exposed this gap. Commit ddab74c added a checked final probe, but bundled it with timeout terminalization and broader verifier-policy changes.

That prototype is useful evidence, not a merge-ready fix: its hard gate affects every rollout with a sandbox user, while other changes in the PR also reject or suppress evidence for profiles without one. This issue isolates the mainline quiescence bug so failure policy, backend support, and compatibility can be reviewed independently.

[tulerfeng]

Thank you for catching this important issue. I opened PR #1088 for it.

I ran into one thing that goes past the report, and it changed how I fixed it. Checking the return value alone wouldn't have helped, because the command ends on sleep 1 — a shell takes its status from the last thing it ran, so it came back 0 no matter what the pkill before it did. I had to move the check into the command itself rather than just read what it returned.

I also looked at whether the workspace chown already covers this, since it describes itself as "belt-and-suspenders against any zombie sandbox-user process that survived the pkill". It doesn't hold up. POSIX checks permissions at open(), so a descriptor the process is already holding keeps writing straight through the ownership change — I watched a sandbox-user process take a root-owned, non-writable file from 3 lines to 10.

I verified the fix against real containers. The same live agent process that main waves through now raises and names the pid, and a run whose kill succeeded still passes. A full bench eval run on hello-world-task came out 1/1, reward 1.00.