fix(terminal): lazy-parse docker env config (#42733)

Co-authored-by: BROCCOLO1D <279959838+BROCCOLO1D@users.noreply.github.com>
This commit is contained in:
BROCCOLO1D 2026-06-10 11:04:27 +10:00 committed by GitHub
parent 8b84d82227
commit 29036155ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 77 additions and 8 deletions

View file

@ -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")

View file

@ -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