diff --git a/tests/tools/test_terminal_tool.py b/tests/tools/test_terminal_tool.py index b17fc332c49..fe2f5e3f514 100644 --- a/tests/tools/test_terminal_tool.py +++ b/tests/tools/test_terminal_tool.py @@ -168,3 +168,46 @@ def test_validate_workdir_blocks_shell_metacharacters_in_windows_paths(): assert terminal_tool._validate_workdir(r"C:\Users\Alice\project; rm -rf /") assert terminal_tool._validate_workdir(r"C:\Users\Alice\project$(whoami)") assert terminal_tool._validate_workdir("C:\\Users\\Alice\\project\nwhoami") + + +def test_get_env_config_ignores_bad_docker_json_for_local_backend(monkeypatch): + """Docker-only JSON env vars must not break the default local backend.""" + monkeypatch.setenv("TERMINAL_ENV", "local") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", "None") + monkeypatch.setenv("TERMINAL_DOCKER_ENV", "not-json") + monkeypatch.setenv("TERMINAL_DOCKER_FORWARD_ENV", "not-json") + monkeypatch.setenv("TERMINAL_DOCKER_EXTRA_ARGS", "not-json") + + config = terminal_tool._get_env_config() + + assert config["env_type"] == "local" + assert config["docker_volumes"] == [] + assert config["docker_env"] == {} + assert config["docker_forward_env"] == [] + assert config["docker_extra_args"] == [] + + +def test_get_env_config_ignores_bad_docker_json_for_ssh_backend(monkeypatch): + """Non-container remote backends should also ignore Docker-only JSON.""" + monkeypatch.setenv("TERMINAL_ENV", "ssh") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", "None") + monkeypatch.setenv("TERMINAL_DOCKER_ENV", "not-json") + + config = terminal_tool._get_env_config() + + assert config["env_type"] == "ssh" + assert config["docker_volumes"] == [] + assert config["docker_env"] == {} + + +def test_get_env_config_still_rejects_bad_docker_json_for_docker_backend(monkeypatch): + """Selecting Docker should keep the existing actionable config error.""" + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", "None") + + try: + terminal_tool._get_env_config() + except ValueError as exc: + assert "TERMINAL_DOCKER_VOLUMES" in str(exc) + else: + raise AssertionError("Docker backend must validate TERMINAL_DOCKER_VOLUMES") diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index e859cf10cad..d9edd7a5d5d 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -1030,7 +1030,7 @@ def _resolve_container_task_id(task_id: Optional[str]) -> str: # Configuration from environment variables -def _parse_env_var(name: str, default: str, converter=int, type_label: str = "integer"): +def _parse_env_var(name: str, default: str, converter: Any = int, type_label: str = "integer"): """Parse an environment variable with *converter*, raising a clear error on bad values. Without this wrapper, a single malformed env var (e.g. TERMINAL_TIMEOUT=5m) @@ -1067,6 +1067,32 @@ def _get_env_config() -> Dict[str, Any]: env_type = os.getenv("TERMINAL_ENV", "local") mount_docker_cwd = os.getenv("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", "false").lower() in {"true", "1", "yes"} + container_backend = env_type in {"docker", "singularity", "modal", "daytona"} + docker_backend = env_type == "docker" + + # Docker/container-only env vars may be bridged from config.yaml even when + # the active backend is local/ssh. Do not parse their JSON/numeric payloads + # until a backend that can consume them is selected; a stale or invalid + # Docker value should not make local terminal/execute_code unusable. + if container_backend: + container_cpu = _parse_env_var("TERMINAL_CONTAINER_CPU", "1", float, "number") + container_memory = _parse_env_var("TERMINAL_CONTAINER_MEMORY", "5120") + container_disk = _parse_env_var("TERMINAL_CONTAINER_DISK", "51200") + else: + container_cpu = 1.0 + container_memory = 5120 + container_disk = 51200 + + if docker_backend: + docker_forward_env = _parse_env_var("TERMINAL_DOCKER_FORWARD_ENV", "[]", json.loads, "valid JSON") + docker_volumes = _parse_env_var("TERMINAL_DOCKER_VOLUMES", "[]", json.loads, "valid JSON") + docker_env = _parse_env_var("TERMINAL_DOCKER_ENV", "{}", json.loads, "valid JSON") + docker_extra_args = _parse_env_var("TERMINAL_DOCKER_EXTRA_ARGS", "[]", json.loads, "valid JSON") + else: + docker_forward_env = [] + docker_volumes = [] + docker_env = {} + docker_extra_args = [] # Default cwd: local uses the host's current directory, ssh uses the # remote home, and everything else starts in the backend's default @@ -1110,7 +1136,7 @@ def _get_env_config() -> Dict[str, Any]: "env_type": env_type, "modal_mode": coerce_modal_mode(os.getenv("TERMINAL_MODAL_MODE", "auto")), "docker_image": os.getenv("TERMINAL_DOCKER_IMAGE", default_image), - "docker_forward_env": _parse_env_var("TERMINAL_DOCKER_FORWARD_ENV", "[]", json.loads, "valid JSON"), + "docker_forward_env": docker_forward_env, "singularity_image": os.getenv("TERMINAL_SINGULARITY_IMAGE", f"docker://{default_image}"), "modal_image": os.getenv("TERMINAL_MODAL_IMAGE", default_image), "daytona_image": os.getenv("TERMINAL_DAYTONA_IMAGE", default_image), @@ -1134,14 +1160,14 @@ def _get_env_config() -> Dict[str, Any]: "local_persistent": os.getenv("TERMINAL_LOCAL_PERSISTENT", "false").lower() in {"true", "1", "yes"}, # Container resource config (applies to docker, singularity, modal, # daytona -- ignored for local/ssh) - "container_cpu": _parse_env_var("TERMINAL_CONTAINER_CPU", "1", float, "number"), - "container_memory": _parse_env_var("TERMINAL_CONTAINER_MEMORY", "5120"), # MB (default 5GB) - "container_disk": _parse_env_var("TERMINAL_CONTAINER_DISK", "51200"), # MB (default 50GB) + "container_cpu": container_cpu, + "container_memory": container_memory, # MB (default 5GB) + "container_disk": container_disk, # 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_env": _parse_env_var("TERMINAL_DOCKER_ENV", "{}", json.loads, "valid JSON"), + "docker_volumes": docker_volumes, + "docker_env": docker_env, "docker_run_as_host_user": os.getenv("TERMINAL_DOCKER_RUN_AS_HOST_USER", "false").lower() in {"true", "1", "yes"}, - "docker_extra_args": _parse_env_var("TERMINAL_DOCKER_EXTRA_ARGS", "[]", json.loads, "valid JSON"), + "docker_extra_args": docker_extra_args, # Cross-process container reuse (issue #20561). The docs claim # "ONE long-lived container shared across sessions" — this toggle # makes that real by probing for a labeled container at startup and