Skip to content

Commit 79f8b17

Browse files
fix: honour the stored setup_script_path when executing automations
- Problem: setup_script_path is validated, stored and echoed back by the API, but both executor paths run a hardcoded root setup.sh, so a tarball naming any other setup script silently skips its setup step and fails later in the entrypoint (issue #343). - Fix: thread automation.setup_script_path from the dispatcher into execute_in_context (and add the same parameter to run_automation for the blocking path), interpolating the value with the existing _shell_quote helper and keeping the setup.sh default when unset. - Verification: uv run pytest tests/ -q --ignore=tests/integration (1460 passed); uv run pre-commit run --files <changed> (ruff, pycodestyle, pyright all passed). Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent 7e9b89a commit 79f8b17

3 files changed

Lines changed: 133 additions & 6 deletions

File tree

openhands/automation/dispatcher.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ async def _fail(
395395
env_vars=env_vars,
396396
timeout=effective_timeout,
397397
run_id=run_id,
398+
setup_script_path=automation.setup_script_path,
398399
sandbox_id=ctx.sandbox_id,
399400
)
400401
except PermanentDispatchError as exc:

openhands/automation/execution.py

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,14 +341,15 @@ async def execute_in_context(
341341
timeout: int | None = None,
342342
run_id: str | None = None,
343343
sandbox_id: str | None = None,
344+
setup_script_path: str | None = None,
344345
) -> DispatchResult:
345346
"""Execute automation code in an existing execution context.
346347
347348
This is the core execution logic used by both Cloud and Local modes.
348349
The context (agent_url, session_key) is obtained from the backend.
349350
350351
1. Get tarball into environment (upload bytes OR download from URL).
351-
2. Extract it, run ``setup.sh`` (if present), then start *entrypoint*.
352+
2. Extract it, run the setup script (if present), then start *entrypoint*.
352353
3. Return immediately without waiting for the entrypoint to complete.
353354
354355
Args:
@@ -364,6 +365,10 @@ async def execute_in_context(
364365
path (/tmp/automation-<run_id>.tar.gz) that prevents collisions
365366
when concurrent runs share the same filesystem (sandboxless mode)
366367
sandbox_id: Sandbox ID for logging (Cloud mode only)
368+
setup_script_path: Path to the setup script inside the extracted
369+
tarball (default: ``setup.sh``). The value is validated by the
370+
request layer (relative, no traversal, no shell metacharacters)
371+
before it is stored on the automation.
367372
368373
Returns:
369374
DispatchResult with success status
@@ -408,12 +413,13 @@ def _log_ctx() -> dict[str, Any]:
408413
)
409414
env_prefix = _env_command_prefix(env_path)
410415

416+
setup_cmd = _shell_quote(setup_script_path or "setup.sh")
411417
cmd = (
412418
f"{env_prefix}mkdir -p {work_dir}"
413419
f" && tar xzf {tarball_path} -C {work_dir}"
414420
f" && rm -f {tarball_path}"
415421
f" && cd {work_dir}"
416-
f" && ([ ! -f setup.sh ] || bash setup.sh)"
422+
f" && ([ ! -f {setup_cmd} ] || bash {setup_cmd})"
417423
f" && {entrypoint}"
418424
)
419425

@@ -484,6 +490,7 @@ async def run_automation(
484490
run_id: str | None = None,
485491
keep_sandbox: bool = False,
486492
work_dir: str = DEFAULT_WORK_DIR,
493+
setup_script_path: str | None = None,
487494
) -> AutomationResult:
488495
"""Execute an automation end-to-end in a fresh sandbox (blocking).
489496
@@ -492,16 +499,17 @@ async def run_automation(
492499
493500
1. Create sandbox and wait until RUNNING.
494501
2. Get tarball into sandbox (upload bytes OR download from URL).
495-
3. Extract it, run ``setup.sh`` (if present), then run *entrypoint*.
502+
3. Extract it, run the setup script (if present), then run *entrypoint*.
496503
4. Wait for completion and return the result.
497504
5. Delete the sandbox (unless *keep_sandbox* is True).
498505
499506
*tarball_source*: Either raw bytes (uploaded to sandbox) or a URL string
500507
(downloaded directly inside sandbox via curl). URLs avoid downloading
501508
untrusted/large files on the automation service.
502509
503-
*env_vars* are exported before setup.sh and the entrypoint run,
504-
so setup.sh can consume injected values such as ``AUTOMATION_API_URL``.
510+
*env_vars* are exported before the setup script and the entrypoint run,
511+
so the setup script can consume injected values such as
512+
``AUTOMATION_API_URL``.
505513
The sandbox identity env vars (``SANDBOX_ID``, ``SESSION_API_KEY``) are
506514
**always** injected so the SDK's ``local_agent_server_mode`` works.
507515
If *callback_url* / *run_id* are set they are injected as
@@ -510,6 +518,10 @@ async def run_automation(
510518
511519
*work_dir* is the working directory for tarball extraction
512520
(default: /workspace/project).
521+
522+
*setup_script_path* is the path of the setup script inside the extracted
523+
tarball (default: ``setup.sh``). Validated by the request layer before it
524+
is stored on the automation.
513525
"""
514526
timeout = resolve_automation_timeout_seconds(timeout)
515527
http_timeout = get_config().http.http_long_timeout
@@ -574,11 +586,12 @@ def _log_ctx() -> dict[str, Any]:
574586
)
575587
env_prefix = _env_command_prefix(env_path)
576588

589+
setup_cmd = _shell_quote(setup_script_path or "setup.sh")
577590
cmd = (
578591
f"{env_prefix}mkdir -p {work_dir}"
579592
f" && tar xzf {TARBALL_PATH} -C {work_dir}"
580593
f" && cd {work_dir}"
581-
f" && ([ ! -f setup.sh ] || bash setup.sh)"
594+
f" && ([ ! -f {setup_cmd} ] || bash {setup_cmd})"
582595
f" && {entrypoint}"
583596
)
584597

tests/test_execution.py

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,119 @@ async def test_success_returns_dispatch_result(self, mock_start_bash, mock_uploa
326326
assert result.sandbox_id == "test-sandbox-id"
327327

328328

329+
class TestSetupScriptPath:
330+
"""The stored ``setup_script_path`` must reach the executed command.
331+
332+
The field is validated, persisted and echoed back by the API, but the
333+
executor historically ran a hardcoded root ``setup.sh`` — a tarball
334+
naming anything else silently skipped its setup step (issue #343).
335+
"""
336+
337+
@pytest.mark.asyncio
338+
@patch("openhands.automation.execution._start_bash", new_callable=AsyncMock)
339+
@patch("openhands.automation.execution._upload", new_callable=AsyncMock)
340+
async def test_custom_path_is_used_in_command(self, mock_upload, mock_start_bash):
341+
"""execute_in_context runs the stored setup script path, quoted."""
342+
mock_start_bash.return_value = "cmd-1"
343+
344+
result = await execute_in_context(
345+
client=AsyncMock(),
346+
agent_url="https://agent.example.com",
347+
session_key="session-key",
348+
entrypoint="python main.py",
349+
tarball_source=b"fake tarball bytes",
350+
work_dir=DEFAULT_WORK_DIR,
351+
setup_script_path="scripts/setup.sh",
352+
)
353+
354+
assert result.success is True
355+
command = mock_start_bash.await_args.args[3]
356+
assert "([ ! -f 'scripts/setup.sh' ] || bash 'scripts/setup.sh')" in command
357+
358+
@pytest.mark.asyncio
359+
@patch("openhands.automation.execution._start_bash", new_callable=AsyncMock)
360+
@patch("openhands.automation.execution._upload", new_callable=AsyncMock)
361+
async def test_default_setup_sh_when_unset(self, mock_upload, mock_start_bash):
362+
"""Without a stored path the root setup.sh convention is kept."""
363+
mock_start_bash.return_value = "cmd-1"
364+
365+
result = await execute_in_context(
366+
client=AsyncMock(),
367+
agent_url="https://agent.example.com",
368+
session_key="session-key",
369+
entrypoint="python main.py",
370+
tarball_source=b"fake tarball bytes",
371+
work_dir=DEFAULT_WORK_DIR,
372+
)
373+
374+
assert result.success is True
375+
command = mock_start_bash.await_args.args[3]
376+
assert "([ ! -f 'setup.sh' ] || bash 'setup.sh')" in command
377+
378+
@pytest.mark.asyncio
379+
@patch("openhands.automation.execution._start_bash", new_callable=AsyncMock)
380+
@patch("openhands.automation.execution._upload", new_callable=AsyncMock)
381+
async def test_path_with_single_quote_is_escaped(
382+
self, mock_upload, mock_start_bash
383+
):
384+
"""A quote in the path (allowed by the validator) stays literal."""
385+
mock_start_bash.return_value = "cmd-1"
386+
387+
result = await execute_in_context(
388+
client=AsyncMock(),
389+
agent_url="https://agent.example.com",
390+
session_key="session-key",
391+
entrypoint="python main.py",
392+
tarball_source=b"fake tarball bytes",
393+
work_dir=DEFAULT_WORK_DIR,
394+
setup_script_path="scripts/se'tup.sh",
395+
)
396+
397+
assert result.success is True
398+
command = mock_start_bash.await_args.args[3]
399+
assert (
400+
"([ ! -f 'scripts/se'\\''tup.sh' ] || bash 'scripts/se'\\''tup.sh')"
401+
in command
402+
)
403+
404+
@pytest.mark.asyncio
405+
@patch("openhands.automation.execution._bash", new_callable=AsyncMock)
406+
@patch("openhands.automation.execution._upload", new_callable=AsyncMock)
407+
@patch("openhands.automation.execution._create_and_wait", new_callable=AsyncMock)
408+
@patch("openhands.automation.execution.httpx.AsyncClient")
409+
async def test_blocking_path_uses_custom_setup_script(
410+
self,
411+
mock_async_client,
412+
mock_create_and_wait,
413+
mock_upload,
414+
mock_bash,
415+
):
416+
"""run_automation (blocking mode) also honours the stored path."""
417+
context_manager = MagicMock()
418+
context_manager.__aenter__ = AsyncMock(return_value=AsyncMock())
419+
context_manager.__aexit__ = AsyncMock(return_value=None)
420+
mock_async_client.return_value = context_manager
421+
mock_create_and_wait.return_value = (
422+
"sandbox-1",
423+
"session-key",
424+
"https://agent.example.com",
425+
)
426+
mock_bash.return_value = (0, "", "")
427+
428+
result = await run_automation(
429+
api_url="https://api.example.com",
430+
api_key="api-key",
431+
entrypoint="python main.py",
432+
tarball_source=b"fake tarball bytes",
433+
keep_sandbox=True,
434+
setup_script_path="scripts/setup.sh",
435+
)
436+
437+
assert result.success is True
438+
command = mock_bash.await_args.args[3]
439+
assert "([ ! -f 'scripts/setup.sh' ] || bash 'scripts/setup.sh')" in command
440+
441+
329442
class TestPrivateEnvironmentInjection:
330443
def test_env_file_is_loaded_and_removed(self, tmp_path):
331444
env_path = tmp_path / "private.env"

0 commit comments

Comments
 (0)