From 4446646e5084063686984b029961ec274b4ccf10 Mon Sep 17 00:00:00 2001 From: Bingran You Date: Tue, 14 Jul 2026 22:15:51 -0700 Subject: [PATCH 1/2] Fix OpenHands direct execution outside root workdirs --- src/benchflow/acp/runtime.py | 66 ++++++++++++++++++++++++++++++++++++ tests/test_acp.py | 42 +++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/src/benchflow/acp/runtime.py b/src/benchflow/acp/runtime.py index 2555ea89..95ba22b2 100644 --- a/src/benchflow/acp/runtime.py +++ b/src/benchflow/acp/runtime.py @@ -67,6 +67,66 @@ _ACP_CONNECT_BASE_DELAY = 2.0 _PROMPT_CANCEL_DRAIN_TIMEOUT_SEC = 0.25 _ACP_HANDSHAKE_TIMEOUT_SEC = 60 +_OPENHANDS_DISABLE_SUBAGENTS_ENV = "BENCHFLOW_OPENHANDS_DISABLE_SUBAGENTS" + + +async def _prepare_openhands_direct_execution( + env, + *, + agent: str, + agent_env: dict[str, str], +) -> dict[str, str]: + """Patch OpenHands as root before a sandbox-user privilege drop. + + OpenHands is installed under ``/root/.local/share/uv/tools`` and exposed to + the sandbox user through a read-only symlink. Running the opt-in patch from + the launch command only worked for tasks whose workspace was ``/root``, + because sandbox-user setup happened to chown that whole directory. Tasks + rooted elsewhere (for example ``/app``) exited before ACP initialization. + + Apply the same narrow patch through the environment's root execution plane, + then disable the duplicate launch-time patch for this process. The caller's + environment mapping is copied so recorded run provenance still reflects the + requested opt-in value. + """ + if agent != "openhands" or agent_env.get(_OPENHANDS_DISABLE_SUBAGENTS_ENV) != "1": + return agent_env + + patch_cmd = ( + 'export PATH="$HOME/.local/bin:$PATH"; ' + 'OH_BIN="$(command -v openhands)"; ' + '[ -n "$OH_BIN" ] || { echo "Cannot locate OpenHands executable" >&2; exit 127; }; ' + 'OH_PY="$(dirname "$(readlink -f "$OH_BIN")")/python"; ' + '[ -x "$OH_PY" ] || { ' + 'echo "Cannot locate OpenHands tool interpreter" >&2; exit 127; }; ' + '"$OH_PY" -c \'from pathlib import Path; ' + "import openhands_cli.utils as u; " + "p=Path(u.__file__); s=p.read_text(); " + 'old=" Tool(name=task_tool_name),\\n"; ' + 'new=" # BenchFlow: delegation disabled for this run.\\n"; ' + "assert old in s or new in s; " + "p.write_text(s.replace(old,new,1)) if old in s else None; " + "assert new in p.read_text()'" + ) + result = await env.exec(patch_cmd, timeout_sec=30) + return_code = getattr( + result, + "return_code", + getattr(result, "exit_code", 1), + ) + if return_code != 0: + stdout = (getattr(result, "stdout", "") or "").strip() + stderr = (getattr(result, "stderr", "") or "").strip() + detail = stderr or stdout or "no output" + raise RuntimeError( + "Failed to prepare OpenHands direct-execution mode as root " + f"(exit code {return_code}): {detail[:2000]}" + ) + + launch_env = dict(agent_env) + launch_env[_OPENHANDS_DISABLE_SUBAGENTS_ENV] = "0" + logger.info("Prepared OpenHands direct-execution mode before privilege drop") + return launch_env async def _wait_for_acp_handshake(awaitable, *, phase: str): @@ -478,6 +538,12 @@ async def connect_acp( Retries with exponential backoff on ConnectionError (Daytona SSH storms). """ + agent_env = await _prepare_openhands_direct_execution( + env, + agent=agent, + agent_env=agent_env, + ) + # Resolve agent binary path for non-docker environments if environment != "docker": which_result = await env.exec( diff --git a/tests/test_acp.py b/tests/test_acp.py index 175f9ba4..f819c0c7 100644 --- a/tests/test_acp.py +++ b/tests/test_acp.py @@ -1064,6 +1064,48 @@ async def test_openhands_skips_set_model(self, tmp_path): mock_acp.set_model.assert_not_awaited() + @pytest.mark.asyncio + async def test_openhands_direct_execution_patches_before_privilege_drop( + self, tmp_path + ): + """Guards the PR #921 follow-up for OpenHands tasks rooted outside /root.""" + from benchflow.acp.runtime import connect_acp + + mock_acp = self._make_mocks() + mock_env = AsyncMock() + mock_env.exec.return_value = MagicMock(return_code=0, stdout="", stderr="") + + with ( + patch( + "benchflow.acp.runtime.DockerProcess.from_sandbox_env", + return_value=MagicMock(), + ), + patch( + "benchflow.acp.runtime.ContainerTransport", + return_value=MagicMock(), + ) as mock_transport, + patch("benchflow.acp.runtime.ACPClient", return_value=mock_acp), + ): + await connect_acp( + env=mock_env, + agent="openhands", + agent_launch="openhands acp --always-approve --override-with-envs", + agent_env={"BENCHFLOW_OPENHANDS_DISABLE_SUBAGENTS": "1"}, + sandbox_user="agent", + model=None, + rollout_dir=tmp_path, + environment="docker", + agent_cwd="/app", + ) + + patch_call = mock_env.exec.await_args_list[0] + assert "openhands_cli.utils" in patch_call.args[0] + assert patch_call.kwargs == {"timeout_sec": 30} + transport_env = mock_transport.call_args.kwargs["env"] + assert transport_env["BENCHFLOW_OPENHANDS_DISABLE_SUBAGENTS"] == "0" + transport_command = mock_transport.call_args.kwargs["command"] + assert "--reuid=agent" in transport_command + @pytest.mark.asyncio async def test_codex_uses_session_advertised_model_id(self, tmp_path): """Guards commit 81ff286 against codex-acp rejecting bare set_model IDs.""" From 832d0a20f5f107a94e31b22648901e158a5ab083 Mon Sep 17 00:00:00 2001 From: Bingran You Date: Tue, 14 Jul 2026 22:22:43 -0700 Subject: [PATCH 2/2] Run OpenHands prelaunch patch explicitly as root --- src/benchflow/acp/runtime.py | 2 +- tests/test_acp.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/benchflow/acp/runtime.py b/src/benchflow/acp/runtime.py index 95ba22b2..7fb733d3 100644 --- a/src/benchflow/acp/runtime.py +++ b/src/benchflow/acp/runtime.py @@ -108,7 +108,7 @@ async def _prepare_openhands_direct_execution( "p.write_text(s.replace(old,new,1)) if old in s else None; " "assert new in p.read_text()'" ) - result = await env.exec(patch_cmd, timeout_sec=30) + result = await env.exec(patch_cmd, user="root", timeout_sec=30) return_code = getattr( result, "return_code", diff --git a/tests/test_acp.py b/tests/test_acp.py index f819c0c7..55949dbb 100644 --- a/tests/test_acp.py +++ b/tests/test_acp.py @@ -1100,7 +1100,7 @@ async def test_openhands_direct_execution_patches_before_privilege_drop( patch_call = mock_env.exec.await_args_list[0] assert "openhands_cli.utils" in patch_call.args[0] - assert patch_call.kwargs == {"timeout_sec": 30} + assert patch_call.kwargs == {"user": "root", "timeout_sec": 30} transport_env = mock_transport.call_args.kwargs["env"] assert transport_env["BENCHFLOW_OPENHANDS_DISABLE_SUBAGENTS"] == "0" transport_command = mock_transport.call_args.kwargs["command"]