From 2a30b4955890244821e21e330bad621a45196e7b Mon Sep 17 00:00:00 2001 From: Paulo Alvarado Date: Tue, 1 Sep 2026 15:10:27 +0200 Subject: [PATCH 1/2] Run gateway sessions as the configured sandbox user A gateway session entered as root, because `sbx exec` without --user is root and the gateway path ignored DOCKER_SBX_SSH_USERNAME. Agent CLIs refuse to run as root, so a caller's run died at startup. Now one sbx invocation still enters as root to make and own the per-host home, then drops to DOCKER_SBX_SSH_USERNAME with `su -m`, which keeps the exported HOME. The root default is byte-identical to before. The SFTP backing shell goes through the same open(), thus it drops to the user too. --- docs/deploy.md | 6 ++ src/providers/docker_sbx/process.py | 28 +++++++++- .../docker_sbx/tests/test_process.py | 56 +++++++++++++++++-- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 489d8f0..6ab128d 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -217,6 +217,12 @@ first. Thus a caller writes files there — for example a `.gitconfig` or credential files — and a later command finds them by `$HOME`. SFTP relative paths also resolve against this home. +A session runs as the sandbox user `DOCKER_SBX_SSH_USERNAME` (default +`root`). The gateway prepares the home as root, then drops to this user. +Some agent CLIs refuse to run as root. Such a deployment sets +`DOCKER_SBX_SSH_USERNAME` to a non-root user that the template image +contains. + The gateway is a requirement for gateway providers: `POST /hosts` for `docker-sbx` fails without `GATEWAY_SSH_HOST`. Set it to the address callers use. The response then carries the gateway coordinates: diff --git a/src/providers/docker_sbx/process.py b/src/providers/docker_sbx/process.py index 48ea024..1803d3f 100644 --- a/src/providers/docker_sbx/process.py +++ b/src/providers/docker_sbx/process.py @@ -3,22 +3,43 @@ import fcntl import os import pty +import shlex import struct import termios from providers.base import SandboxProcess, TerminalSize from providers.exceptions import ProviderTransportError +from .settings import DockerSbxSettings + def _set_terminal_size(descriptor: int, size: TerminalSize) -> None: winsize = struct.pack("HHHH", size.rows, size.columns, 0, 0) fcntl.ioctl(descriptor, termios.TIOCSWINSZ, winsize) -def _session_script(name: str, command: str | None) -> str: +def _session_script(name: str, command: str | None, user: str) -> str: + """Build the shell for one gateway session. + + The exec enters as root. The script makes the per-host home + /home/, moves into it, and exports HOME. For the root user it + stops there, thus the default install keeps today's session exactly. + + For any other user it also gives that user the home and drops to the + user with `su -m`. `-m` keeps the caller environment, thus the exported + HOME stays the per-host home. `su` runs a PAM session, thus + /etc/environment — the sandbox env the bootstrap writes — still reaches + the session. The name is server-generated, thus the home path is a safe + shell token; the user and the payload are quoted. + """ home = f"/home/{name}" payload = command if command is not None else "exec bash -l" - return f"mkdir -p {home} && cd {home} && export HOME={home}\n{payload}" + prepare = f"mkdir -p {home} && cd {home} && export HOME={home}" + if user == "root": + return f"{prepare}\n{payload}" + owner = shlex.quote(user) + drop = f"exec su -m {owner} -s /bin/bash -c {shlex.quote(payload)}" + return f"{prepare} && chown {owner} {home}\n{drop}" class SbxExecProcess(SandboxProcess): @@ -48,10 +69,11 @@ async def open( command: str | None, terminal: TerminalSize | None, ) -> "SbxExecProcess": + user = DockerSbxSettings().ssh_username argv = ["sbx", "exec", "--interactive"] if terminal: argv.append("--tty") - argv.extend([name, "bash", "-l", "-c", _session_script(name, command)]) + argv.extend([name, "bash", "-l", "-c", _session_script(name, command, user)]) environment = {**os.environ, "SBX_NO_TELEMETRY": "1"} try: diff --git a/src/providers/docker_sbx/tests/test_process.py b/src/providers/docker_sbx/tests/test_process.py index 8c5bff9..96f71e7 100644 --- a/src/providers/docker_sbx/tests/test_process.py +++ b/src/providers/docker_sbx/tests/test_process.py @@ -1,5 +1,6 @@ import asyncio import os +import shlex import pytest @@ -48,7 +49,7 @@ async def test_exec_session_bridges_pipes_in_both_directions(monkeypatch): "-l", "-c", ) - assert captured["argv"][7] == _session_script("sb-test", "true") + assert captured["argv"][7] == _session_script("sb-test", "true", "root") assert output == b"round-trip" assert status == 5 @@ -85,7 +86,7 @@ async def test_terminal_request_allocates_a_real_pty_at_the_requested_size(monke assert "--tty" in captured["argv"] assert captured["argv"][-4:-1] == ("bash", "-l", "-c") - assert captured["argv"][-1] == _session_script("sb-test", None) + assert captured["argv"][-1] == _session_script("sb-test", None, "root") assert b"ISTTY" in output assert b"42 101" in output assert status == 3 @@ -109,7 +110,7 @@ async def test_aclose_releases_the_terminal_descriptor(monkeypatch): def test_session_prepares_the_per_host_home_before_the_command(): - script = _session_script("sb-abc", "git status") + script = _session_script("sb-abc", "git status", "root") assert "mkdir -p /home/sb-abc" in script assert "export HOME=/home/sb-abc" in script # The home exists and HOME is set before the command runs. @@ -118,11 +119,58 @@ def test_session_prepares_the_per_host_home_before_the_command(): def test_session_without_a_command_runs_a_login_shell_in_the_home(): - script = _session_script("sb-abc", None) + script = _session_script("sb-abc", None, "root") assert "mkdir -p /home/sb-abc" in script assert script.endswith("exec bash -l") +def test_root_session_is_byte_identical_to_the_plain_home_setup(): + # The default install must not change: no chown, no su. + assert _session_script("sb-abc", "git status", "root") == ( + "mkdir -p /home/sb-abc && cd /home/sb-abc && export HOME=/home/sb-abc\ngit status" + ) + assert "su" not in _session_script("sb-abc", None, "root") + + +def test_non_root_session_chowns_the_home_and_drops_to_the_user(): + script = _session_script("sb-abc", "agent run --flag", "druks") + # Root prepares and gives the user the home before it drops. + assert "chown druks /home/sb-abc" in script + assert script.index("mkdir -p /home/sb-abc") < script.index("su -m druks") + assert script.index("chown druks") < script.index("su -m druks") + # su -m keeps the exported HOME; the payload is quoted for /bin/bash -c. + assert f"exec su -m druks -s /bin/bash -c {shlex.quote('agent run --flag')}" in script + + +def test_non_root_interactive_session_drops_to_a_login_shell(): + script = _session_script("sb-abc", None, "druks") + assert f"exec su -m druks -s /bin/bash -c {shlex.quote('exec bash -l')}" in script + + +def test_non_root_sftp_backing_shell_runs_as_the_user(): + # The SFTP backend passes its server command through the same open(); + # thus its backing shell drops to the user too. + script = _session_script("sb-abc", "exec /usr/lib/openssh/sftp-server", "druks") + assert "exec su -m druks -s /bin/bash -c" in script + assert shlex.quote("exec /usr/lib/openssh/sftp-server") in script + + +async def test_open_runs_the_session_as_the_configured_user(monkeypatch): + monkeypatch.setenv("DOCKER_SBX_SSH_USERNAME", "druks") + captured: dict = {} + monkeypatch.setattr( + "providers.docker_sbx.process.asyncio.create_subprocess_exec", + _stub_sbx_with("exit 0", captured), + ) + + session = await SbxExecProcess.open("sb-test", command="id -un", terminal=None) + await session.wait() + await session.aclose() + + assert captured["argv"][7] == _session_script("sb-test", "id -un", "druks") + assert "exec su -m druks" in captured["argv"][7] + + async def test_missing_sbx_binary_maps_to_transport_error(monkeypatch): async def fail_exec(*argv, **kwargs): raise FileNotFoundError("sbx") From f835f0ecca12a5607333a2dae7fa1bb6fddbf94b Mon Sep 17 00:00:00 2001 From: Paulo Alvarado Date: Tue, 1 Sep 2026 15:12:54 +0200 Subject: [PATCH 2/2] Trim the session-user docstring and deploy note --- docs/deploy.md | 6 ------ src/providers/docker_sbx/process.py | 16 +++------------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 6ab128d..489d8f0 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -217,12 +217,6 @@ first. Thus a caller writes files there — for example a `.gitconfig` or credential files — and a later command finds them by `$HOME`. SFTP relative paths also resolve against this home. -A session runs as the sandbox user `DOCKER_SBX_SSH_USERNAME` (default -`root`). The gateway prepares the home as root, then drops to this user. -Some agent CLIs refuse to run as root. Such a deployment sets -`DOCKER_SBX_SSH_USERNAME` to a non-root user that the template image -contains. - The gateway is a requirement for gateway providers: `POST /hosts` for `docker-sbx` fails without `GATEWAY_SSH_HOST`. Set it to the address callers use. The response then carries the gateway coordinates: diff --git a/src/providers/docker_sbx/process.py b/src/providers/docker_sbx/process.py index 1803d3f..19ec186 100644 --- a/src/providers/docker_sbx/process.py +++ b/src/providers/docker_sbx/process.py @@ -19,19 +19,9 @@ def _set_terminal_size(descriptor: int, size: TerminalSize) -> None: def _session_script(name: str, command: str | None, user: str) -> str: - """Build the shell for one gateway session. - - The exec enters as root. The script makes the per-host home - /home/, moves into it, and exports HOME. For the root user it - stops there, thus the default install keeps today's session exactly. - - For any other user it also gives that user the home and drops to the - user with `su -m`. `-m` keeps the caller environment, thus the exported - HOME stays the per-host home. `su` runs a PAM session, thus - /etc/environment — the sandbox env the bootstrap writes — still reaches - the session. The name is server-generated, thus the home path is a safe - shell token; the user and the payload are quoted. - """ + # The exec enters as root and prepares the per-host home. For a non-root + # user it then drops with `su -m`, which keeps the exported HOME and, + # through PAM, still applies /etc/environment. home = f"/home/{name}" payload = command if command is not None else "exec bash -l" prepare = f"mkdir -p {home} && cd {home} && export HOME={home}"