From 5531c0df8217c8865b523d85448f3fda1d7478bf Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 29 Apr 2026 16:16:43 +1000 Subject: [PATCH] feat(docker): run container as host user to avoid root-owned bind mounts Add opt-in terminal.docker_run_as_host_user config flag that passes --user $(id -u):$(id -g) to the Docker backend so files written into bind-mounted directories (/workspace, /root, docker_volumes entries) are owned by the host user instead of root. When enabled on POSIX platforms, also drops SETUID/SETGID caps since the container no longer needs gosu/su to switch users. Falls back cleanly on platforms without os.getuid (e.g. native Windows Docker) with a warning. Wired through all three config.yaml -> TERMINAL_* env-var bridges: - cli.py env_mappings (CLI + TUI startup) - gateway/run.py _terminal_env_map (gateway / messaging platforms) - hermes_cli/config.py _config_to_env_sync (`hermes config set`) Also fixes docker_mount_cwd_to_workspace silently failing in gateway mode -- it was missing from gateway/run.py's _terminal_env_map. Adds tests/tools/test_terminal_config_env_sync.py to guard against future drift between the three bridges (same bug class shipped twice in one month). Bundled Hermes image won't work with this flag since its entrypoint expects to start as root for the usermod/gosu hermes flow; works with the default nikolaik/python-nodejs image and plain Debian/Ubuntu. --- cli-config.yaml.example | 5 + cli.py | 1 + gateway/run.py | 2 + hermes_cli/config.py | 11 + tests/tools/test_docker_environment.py | 122 ++++++++++- tests/tools/test_terminal_config_env_sync.py | 210 +++++++++++++++++++ tools/code_execution_tool.py | 1 + tools/environments/docker.py | 71 ++++++- tools/file_tools.py | 1 + tools/terminal_tool.py | 3 + 10 files changed, 412 insertions(+), 15 deletions(-) create mode 100644 tests/tools/test_terminal_config_env_sync.py diff --git a/cli-config.yaml.example b/cli-config.yaml.example index b2a6868604..ac0f1588ab 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -180,6 +180,11 @@ terminal: # lifetime_seconds: 300 # docker_image: "nikolaik/python-nodejs:python3.11-nodejs20" # docker_mount_cwd_to_workspace: true # Explicit opt-in: mount your launch cwd into /workspace +# # Optional: run the container as your host user's uid:gid so files written +# # into bind-mounted dirs are owned by you, not root. Drops SETUID/SETGID +# # caps too since no gosu privilege drop is needed. Leave off if your +# # chosen docker_image expects to start as root. +# docker_run_as_host_user: true # # Optional: explicitly forward selected env vars into Docker. # # These values come from your current shell first, then ~/.hermes/.env. # # Warning: anything forwarded here is visible to commands run in the container. diff --git a/cli.py b/cli.py index 33a4f585e2..a3caf5ac89 100644 --- a/cli.py +++ b/cli.py @@ -563,6 +563,7 @@ def load_cli_config() -> Dict[str, Any]: "container_persistent": "TERMINAL_CONTAINER_PERSISTENT", "docker_volumes": "TERMINAL_DOCKER_VOLUMES", "docker_mount_cwd_to_workspace": "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", + "docker_run_as_host_user": "TERMINAL_DOCKER_RUN_AS_HOST_USER", "sandbox_dir": "TERMINAL_SANDBOX_DIR", # Persistent shell (non-local backends) "persistent_shell": "TERMINAL_PERSISTENT_SHELL", diff --git a/gateway/run.py b/gateway/run.py index c759cb4d3f..6171cbc6f2 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -274,6 +274,8 @@ if _config_path.exists(): "container_disk": "TERMINAL_CONTAINER_DISK", "container_persistent": "TERMINAL_CONTAINER_PERSISTENT", "docker_volumes": "TERMINAL_DOCKER_VOLUMES", + "docker_mount_cwd_to_workspace": "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", + "docker_run_as_host_user": "TERMINAL_DOCKER_RUN_AS_HOST_USER", "sandbox_dir": "TERMINAL_SANDBOX_DIR", "persistent_shell": "TERMINAL_PERSISTENT_SHELL", } diff --git a/hermes_cli/config.py b/hermes_cli/config.py index ad3cd23b53..9792efbd89 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -515,6 +515,16 @@ DEFAULT_CONFIG = { # Explicit opt-in: mount the host cwd into /workspace for Docker sessions. # Default off because passing host directories into a sandbox weakens isolation. "docker_mount_cwd_to_workspace": False, + # Explicit opt-in: run the Docker container as the host user's uid:gid + # (via `--user`). When enabled, files written into bind-mounted dirs + # (docker_volumes, the persistent workspace, or the auto-mounted cwd) + # are owned by your host user instead of root, which avoids needing + # `sudo chown` after container runs. Default off to preserve behavior + # for images whose entrypoints expect to start as root (e.g. the + # bundled Hermes image, which drops to the `hermes` user via gosu). + # When on, SETUID/SETGID caps are omitted from the container since + # no privilege drop is needed. + "docker_run_as_host_user": False, # Persistent shell — keep a long-lived bash shell across execute() calls # so cwd/env vars/shell variables survive between commands. # Enabled by default for non-local backends (SSH); local is always opt-in @@ -4292,6 +4302,7 @@ def set_config_value(key: str, value: str): "terminal.modal_image": "TERMINAL_MODAL_IMAGE", "terminal.daytona_image": "TERMINAL_DAYTONA_IMAGE", "terminal.docker_mount_cwd_to_workspace": "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", + "terminal.docker_run_as_host_user": "TERMINAL_DOCKER_RUN_AS_HOST_USER", "terminal.cwd": "TERMINAL_CWD", "terminal.timeout": "TERMINAL_TIMEOUT", "terminal.sandbox_dir": "TERMINAL_SANDBOX_DIR", diff --git a/tests/tools/test_docker_environment.py b/tests/tools/test_docker_environment.py index 62b8b83df1..cd3b7aae6f 100644 --- a/tests/tools/test_docker_environment.py +++ b/tests/tools/test_docker_environment.py @@ -45,6 +45,7 @@ def _make_dummy_env(**kwargs): host_cwd=kwargs.get("host_cwd"), auto_mount_cwd=kwargs.get("auto_mount_cwd", False), env=kwargs.get("env"), + run_as_host_user=kwargs.get("run_as_host_user", False), ) @@ -384,9 +385,10 @@ def test_normalize_env_dict_rejects_complex_values(): assert result == {"GOOD": "string"} -def test_security_args_include_setuid_setgid_for_gosu_drop(): - """_SECURITY_ARGS must include SETUID and SETGID so the image entrypoint - can drop from root to the non-root `hermes` user via gosu. +def test_security_args_include_setuid_setgid_for_gosu_drop(monkeypatch): + """The default (run_as_host_user=False) invocation must include SETUID and + SETGID caps so the image entrypoint can drop from root to the non-root + `hermes` user via gosu. Without these caps gosu exits with ``error: failed switching to 'hermes': operation not permitted`` @@ -396,17 +398,117 @@ def test_security_args_include_setuid_setgid_for_gosu_drop(): after the drop — the drop is a one-way transition performed before the `no_new_privs` bit is enforced on the exec boundary. """ - args = docker_env._SECURITY_ARGS + monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") + calls = _mock_subprocess_run(monkeypatch) + + _make_dummy_env() + + run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"] + assert run_calls, "docker run should have been called" + run_args = run_calls[0][0] - # Flatten to set of added caps for clarity. added = { - args[i + 1] - for i, flag in enumerate(args[:-1]) + run_args[i + 1] + for i, flag in enumerate(run_args[:-1]) if flag == "--cap-add" } assert "SETUID" in added, "SETUID cap missing — gosu drop in entrypoint will fail" assert "SETGID" in added, "SETGID cap missing — gosu drop in entrypoint will fail" - # Sanity: the hardening posture is still in place. - assert "--cap-drop" in args and "ALL" in args - assert "--security-opt" in args and "no-new-privileges" in args + +# ── run_as_host_user tests ──────────────────────────────────────── + + +def test_run_as_host_user_passes_uid_gid(monkeypatch): + """With run_as_host_user=True, --user : is added to docker run.""" + monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") + monkeypatch.setattr(docker_env.os, "getuid", lambda: 1234, raising=False) + monkeypatch.setattr(docker_env.os, "getgid", lambda: 5678, raising=False) + calls = _mock_subprocess_run(monkeypatch) + + _make_dummy_env(run_as_host_user=True) + + run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"] + assert run_calls, "docker run should have been called" + run_args = run_calls[0][0] + + # --user must be present and must be paired with "1234:5678" + assert "--user" in run_args, f"--user flag missing from docker run args: {run_args}" + idx = run_args.index("--user") + assert run_args[idx + 1] == "1234:5678", ( + f"expected --user 1234:5678, got --user {run_args[idx + 1]}" + ) + + +def test_run_as_host_user_drops_setuid_setgid_caps(monkeypatch): + """When --user is passed, the container never needs gosu, so SETUID/SETGID + caps are omitted for a tighter security posture.""" + monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") + monkeypatch.setattr(docker_env.os, "getuid", lambda: 1000, raising=False) + monkeypatch.setattr(docker_env.os, "getgid", lambda: 1000, raising=False) + calls = _mock_subprocess_run(monkeypatch) + + _make_dummy_env(run_as_host_user=True) + + run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"] + run_args = run_calls[0][0] + + added = { + run_args[i + 1] + for i, flag in enumerate(run_args[:-1]) + if flag == "--cap-add" + } + assert "SETUID" not in added, ( + "SETUID cap should be dropped when running as host user — no gosu drop is needed" + ) + assert "SETGID" not in added, ( + "SETGID cap should be dropped when running as host user — no gosu drop is needed" + ) + # Core non-privilege-drop caps must still be there (pip/npm/apt need them). + assert "DAC_OVERRIDE" in added + assert "CHOWN" in added + assert "FOWNER" in added + + +def test_run_as_host_user_default_off(monkeypatch): + """Without the opt-in, no --user flag is emitted — preserving existing behavior.""" + monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") + calls = _mock_subprocess_run(monkeypatch) + + _make_dummy_env() # run_as_host_user defaults to False + + run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"] + run_args = run_calls[0][0] + assert "--user" not in run_args, ( + f"--user should not be in docker run args when opt-in is off: {run_args}" + ) + + +def test_run_as_host_user_warns_and_skips_when_no_posix_ids(monkeypatch, caplog): + """On platforms without POSIX getuid/getgid, log a warning and leave the + container at its image default user (no --user flag, full cap set).""" + monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") + # Simulate a platform where os.getuid is absent (e.g. Windows host). + monkeypatch.delattr(docker_env.os, "getuid", raising=False) + monkeypatch.delattr(docker_env.os, "getgid", raising=False) + calls = _mock_subprocess_run(monkeypatch) + + with caplog.at_level(logging.WARNING): + _make_dummy_env(run_as_host_user=True) + + run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"] + run_args = run_calls[0][0] + + assert "--user" not in run_args + # Fall back to the full cap set since the container still starts as root. + added = { + run_args[i + 1] + for i, flag in enumerate(run_args[:-1]) + if flag == "--cap-add" + } + assert "SETUID" in added + assert "SETGID" in added + assert any( + "does not expose POSIX uid/gid" in rec.getMessage() + for rec in caplog.records + ), "expected a warning when POSIX ids are unavailable" diff --git a/tests/tools/test_terminal_config_env_sync.py b/tests/tools/test_terminal_config_env_sync.py new file mode 100644 index 0000000000..892062fae7 --- /dev/null +++ b/tests/tools/test_terminal_config_env_sync.py @@ -0,0 +1,210 @@ +"""Regression tests for terminal config -> env-var bridging. + +terminal_tool._get_env_config() reads ALL terminal settings from os.environ +(TERMINAL_*). config.yaml values therefore have to be bridged into env vars +at startup, by THREE separate code paths: + + 1. cli.py -> ``env_mappings`` dict (CLI / TUI startup) + 2. gateway/run.py -> ``_terminal_env_map`` dict (gateway / messaging + platforms) + 3. hermes_cli/config.py:save_config_value + -> ``_config_to_env_sync`` dict (one-shot when the + user runs ``hermes config set …``) + +If any one of these is missing a key, the corresponding config.yaml setting +silently does nothing for that entry-point. This bug already shipped once +for ``docker_run_as_host_user`` (gateway and CLI maps) and once for +``docker_mount_cwd_to_workspace`` (gateway map). + +This test guards against future drift by extracting all three maps via source +inspection and asserting they all bridge the same set of writable +``terminal.*`` keys. Source inspection (rather than importing the live +dicts) keeps the test independent of the user's ~/.hermes/config.yaml and +mirrors the pattern used in tests/hermes_cli/test_config_drift.py. +""" + +import ast +import inspect + + +def _extract_dict_values(source: str, dict_name: str) -> set[str]: + """Return the set of *value* strings in `dict_name = { "k": "VALUE", ... }`. + + We parse the source with ast (so multi-line dicts and comments are + handled) instead of regex. The first matching assignment wins. + """ + tree = ast.parse(source) + for node in ast.walk(tree): + if not isinstance(node, ast.Assign): + continue + targets = [t for t in node.targets if isinstance(t, ast.Name)] + if not any(t.id == dict_name for t in targets): + continue + if not isinstance(node.value, ast.Dict): + continue + out: set[str] = set() + for k, v in zip(node.value.keys, node.value.values): + if isinstance(k, ast.Constant) and isinstance(v, ast.Constant): + if isinstance(v.value, str): + out.add(v.value) + return out + raise AssertionError(f"Could not find `{dict_name} = {{...}}` literal in source") + + +def _extract_dict_keys(source: str, dict_name: str) -> set[str]: + """Return the set of *key* strings in `dict_name = { "KEY": "v", ... }`.""" + tree = ast.parse(source) + for node in ast.walk(tree): + if not isinstance(node, ast.Assign): + continue + targets = [t for t in node.targets if isinstance(t, ast.Name)] + if not any(t.id == dict_name for t in targets): + continue + if not isinstance(node.value, ast.Dict): + continue + out: set[str] = set() + for k in node.value.keys: + if isinstance(k, ast.Constant) and isinstance(k.value, str): + out.add(k.value) + return out + raise AssertionError(f"Could not find `{dict_name} = {{...}}` literal in source") + + +def _cli_env_map_keys() -> set[str]: + """terminal config keys bridged by cli.load_cli_config().""" + import cli + source = inspect.getsource(cli.load_cli_config) + return _extract_dict_keys(source, "env_mappings") + + +def _gateway_env_map_keys() -> set[str]: + """terminal config keys bridged by gateway/run.py at module load.""" + # gateway/run.py builds the dict at module top-level (not inside a + # function), so inspect the whole module source. + import gateway.run as gr + source = inspect.getsource(gr) + return _extract_dict_keys(source, "_terminal_env_map") + + +def _save_config_env_sync_keys() -> set[str]: + """terminal config keys bridged by ``hermes config set foo bar``.""" + from hermes_cli import config as hc_config + source = inspect.getsource(hc_config.set_config_value) + keys = _extract_dict_keys(source, "_config_to_env_sync") + # set_config_value uses fully-qualified ``terminal.foo`` keys; strip the + # prefix so we can compare against the other two maps which use bare + # leaf keys. + return {k.split(".", 1)[1] for k in keys if k.startswith("terminal.")} + + +# Keys present in cli.py env_mappings but intentionally absent from +# gateway/run.py or set_config_value. Each entry must be justified. +_CLI_ONLY_OK = frozenset({ + # `env_type` is a legacy YAML key alias for `backend` that cli.py + # accepts for backwards-compat with older cli-config.yaml. The + # gateway path normalizes on the canonical `backend` key, which is + # also in the map and handles the same bridging. See cli.py ~line 515. + "env_type", + # sudo_password is not a terminal-backend option — it's a credential + # used across backends, bridged to $SUDO_PASSWORD (not TERMINAL_*). + # Treating it as terminal-only would be misleading. + "sudo_password", +}) + + +def _terminal_tool_env_var_names() -> set[str]: + """All TERMINAL_* env vars actually consumed by terminal_tool.""" + import tools.terminal_tool as tt + source = inspect.getsource(tt) + # Naive scan: every os.getenv("TERMINAL_X", ...) and _parse_env_var("TERMINAL_X", ...). + import re + pat = re.compile(r'["\'](TERMINAL_[A-Z0-9_]+)["\']') + return set(pat.findall(source)) + + +def test_cli_and_gateway_env_maps_agree(): + """cli.py and gateway/run.py must bridge the same set of terminal keys. + + Both feed the same downstream consumer (terminal_tool). Drift between + them means a config.yaml setting that "works in CLI mode but not gateway + mode" (or vice-versa) — the bug class that shipped twice already. + """ + cli_keys = _cli_env_map_keys() - _CLI_ONLY_OK + gw_keys = _gateway_env_map_keys() + + # Normalize the legacy `env_type` alias: cli.py accepts both `env_type` + # and `backend` as source keys for TERMINAL_ENV; gateway only accepts + # `backend`. Since cli.py copies `backend` → `env_type` before the + # lookup, they're equivalent. Remove `backend` from the gateway side + # to avoid a spurious "backend missing from cli" failure. + gw_keys = gw_keys - {"backend"} + + missing_in_gateway = cli_keys - gw_keys + missing_in_cli = gw_keys - cli_keys + + assert not missing_in_gateway, ( + f"Keys in cli.py env_mappings but missing from gateway/run.py " + f"_terminal_env_map: {sorted(missing_in_gateway)}. Add them to " + f"both maps (same bug class as docker_run_as_host_user shipping " + f"wired in cli but not gateway in April 2026)." + ) + assert not missing_in_cli, ( + f"Keys in gateway/run.py _terminal_env_map but missing from cli.py " + f"env_mappings: {sorted(missing_in_cli)}. Add them to both maps." + ) + + +def test_save_config_set_supports_critical_bridged_keys(): + """``hermes config set terminal.X true`` must propagate to .env for + known-critical keys. This used to be an all-keys invariant but several + pre-existing terminal keys (ssh_*, docker_forward_env, docker_volumes) + aren't in _config_to_env_sync and are instead handled via the separate + api_keys TERMINAL_SSH_* fallback path or user-edits-yaml-directly. + + Until those gaps are audited and fixed, pin the specific keys that are + load-bearing for the docker backend's ownership flag so the bug we just + fixed cannot silently regress. + """ + save_keys = _save_config_env_sync_keys() + required = { + "docker_run_as_host_user", + "docker_mount_cwd_to_workspace", + "backend", + "docker_image", + "container_cpu", + "container_memory", + "container_disk", + "container_persistent", + } + missing = required - save_keys + assert not missing, ( + f"`hermes config set terminal.X` doesn't sync these load-bearing " + f"keys to .env: {sorted(missing)}. Add them to _config_to_env_sync " + f"in hermes_cli/config.py:set_config_value." + ) + + +def test_docker_run_as_host_user_is_bridged_everywhere(): + """Explicit pin for the bug we just fixed. + + docker_run_as_host_user was added to terminal_tool._get_env_config and + DockerEnvironment but NOT to cli.py's env_mappings or gateway/run.py's + _terminal_env_map, so ``terminal.docker_run_as_host_user: true`` in + config.yaml had no effect at runtime. This guard makes the regression + impossible to reintroduce silently. + """ + assert "docker_run_as_host_user" in _cli_env_map_keys() + assert "docker_run_as_host_user" in _gateway_env_map_keys() + assert "docker_run_as_host_user" in _save_config_env_sync_keys() + assert "TERMINAL_DOCKER_RUN_AS_HOST_USER" in _terminal_tool_env_var_names() + + +def test_docker_mount_cwd_to_workspace_is_bridged_everywhere(): + """Same regression class — docker_mount_cwd_to_workspace was missing from + gateway/run.py's _terminal_env_map until the docker_run_as_host_user + audit caught it. + """ + assert "docker_mount_cwd_to_workspace" in _cli_env_map_keys() + assert "docker_mount_cwd_to_workspace" in _gateway_env_map_keys() + assert "docker_mount_cwd_to_workspace" in _save_config_env_sync_keys() + assert "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE" in _terminal_tool_env_var_names() diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index db706e6a4c..fc32e34742 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -488,6 +488,7 @@ def _get_or_create_env(task_id: str): "container_disk": config.get("container_disk", 51200), "container_persistent": config.get("container_persistent", True), "docker_volumes": config.get("docker_volumes", []), + "docker_run_as_host_user": config.get("docker_run_as_host_user", False), } ssh_config = None diff --git a/tools/environments/docker.py b/tools/environments/docker.py index 65c33b349c..06d8154872 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -151,16 +151,16 @@ def find_docker() -> Optional[str]: # SETUID/SETGID - the image entrypoint drops from root to the 'hermes' # user via `gosu`, which requires these caps. Combined with # `no-new-privileges`, gosu still cannot escalate back to root after -# the drop, so the security posture is preserved. +# the drop, so the security posture is preserved. Omitted entirely +# when the container starts as a non-root user via --user, since +# no gosu drop is needed in that mode. # Block privilege escalation and limit PIDs. # /tmp is size-limited and nosuid but allows exec (needed by pip/npm builds). -_SECURITY_ARGS = [ +_BASE_SECURITY_ARGS = [ "--cap-drop", "ALL", "--cap-add", "DAC_OVERRIDE", "--cap-add", "CHOWN", "--cap-add", "FOWNER", - "--cap-add", "SETUID", - "--cap-add", "SETGID", "--security-opt", "no-new-privileges", "--pids-limit", "256", "--tmpfs", "/tmp:rw,nosuid,size=512m", @@ -168,6 +168,39 @@ _SECURITY_ARGS = [ "--tmpfs", "/run:rw,noexec,nosuid,size=64m", ] +# Extra caps needed when the container starts as root and an entrypoint +# must drop privileges via gosu/su. Skipped when --user is passed because +# the container already starts unprivileged and never needs to switch. +_GOSU_CAP_ARGS = [ + "--cap-add", "SETUID", + "--cap-add", "SETGID", +] + + +def _build_security_args(run_as_host_user: bool) -> list[str]: + """Return the security/cap/tmpfs args tailored to the privilege mode.""" + if run_as_host_user: + return list(_BASE_SECURITY_ARGS) + return list(_BASE_SECURITY_ARGS) + list(_GOSU_CAP_ARGS) + + +def _resolve_host_user_spec() -> Optional[str]: + """Return ``:`` for the current host user, or ``None`` on platforms + where this is not meaningful (e.g. Windows without posix ids). + + We intentionally read ``os.getuid()``/``os.getgid()`` directly rather than + going through ``getpass``/``pwd`` so this stays cheap and never raises on + nameless UIDs (nss lookups can fail inside sandboxed launchers). + """ + get_uid = getattr(os, "getuid", None) + get_gid = getattr(os, "getgid", None) + if get_uid is None or get_gid is None: + return None + try: + return f"{get_uid()}:{get_gid()}" + except Exception: # pragma: no cover - defensive + return None + _storage_opt_ok: Optional[bool] = None # cached result across instances @@ -266,6 +299,7 @@ class DockerEnvironment(BaseEnvironment): network: bool = True, host_cwd: str = None, auto_mount_cwd: bool = False, + run_as_host_user: bool = False, ): if cwd == "~": cwd = "/root" @@ -421,8 +455,35 @@ class DockerEnvironment(BaseEnvironment): for key in sorted(self._env): env_args.extend(["-e", f"{key}={self._env[key]}"]) + # Optional: run the container as the host user so files written into + # bind-mounted dirs (/workspace, /root, docker_volumes entries) are + # owned by that user on the host instead of by root. Skip cleanly on + # platforms without POSIX uid/gid (e.g. native Windows Docker). + user_args: list[str] = [] + if run_as_host_user: + user_spec = _resolve_host_user_spec() + if user_spec is not None: + user_args = ["--user", user_spec] + logger.info("Docker: running container as host user %s", user_spec) + else: + logger.warning( + "docker_run_as_host_user is enabled but this platform does " + "not expose POSIX uid/gid; container will start as its " + "image default user." + ) + # Fall back to the full cap set — without --user, an image's + # entrypoint may still need gosu/su to drop privileges. + security_args = _build_security_args(run_as_host_user and bool(user_args)) + logger.info(f"Docker volume_args: {volume_args}") - all_run_args = list(_SECURITY_ARGS) + writable_args + resource_args + volume_args + env_args + all_run_args = ( + security_args + + user_args + + writable_args + + resource_args + + volume_args + + env_args + ) logger.info(f"Docker run_args: {all_run_args}") # Resolve the docker executable once so it works even when diff --git a/tools/file_tools.py b/tools/file_tools.py index 7d81cd8f8e..f157c7b665 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -389,6 +389,7 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: "docker_volumes": config.get("docker_volumes", []), "docker_mount_cwd_to_workspace": config.get("docker_mount_cwd_to_workspace", False), "docker_forward_env": config.get("docker_forward_env", []), + "docker_run_as_host_user": config.get("docker_run_as_host_user", False), } ssh_config = None diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 24a6bff40e..a926f78de3 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -978,6 +978,7 @@ def _get_env_config() -> Dict[str, Any]: "container_disk": _parse_env_var("TERMINAL_CONTAINER_DISK", "51200"), # MB (default 50GB) "container_persistent": os.getenv("TERMINAL_CONTAINER_PERSISTENT", "true").lower() in ("true", "1", "yes"), "docker_volumes": _parse_env_var("TERMINAL_DOCKER_VOLUMES", "[]", json.loads, "valid JSON"), + "docker_run_as_host_user": os.getenv("TERMINAL_DOCKER_RUN_AS_HOST_USER", "false").lower() in ("true", "1", "yes"), } @@ -1033,6 +1034,7 @@ def _create_environment(env_type: str, image: str, cwd: str, timeout: int, auto_mount_cwd=cc.get("docker_mount_cwd_to_workspace", False), forward_env=docker_forward_env, env=docker_env, + run_as_host_user=cc.get("docker_run_as_host_user", False), ) elif env_type == "singularity": @@ -1661,6 +1663,7 @@ def terminal_tool( "docker_mount_cwd_to_workspace": config.get("docker_mount_cwd_to_workspace", False), "docker_forward_env": config.get("docker_forward_env", []), "docker_env": config.get("docker_env", {}), + "docker_run_as_host_user": config.get("docker_run_as_host_user", False), } local_config = None