diff --git a/src/agent_env/a2a_agent/a2a_agent.py b/src/agent_env/a2a_agent/a2a_agent.py index b8c8be4d..4801db1f 100644 --- a/src/agent_env/a2a_agent/a2a_agent.py +++ b/src/agent_env/a2a_agent/a2a_agent.py @@ -13,7 +13,7 @@ from agent_env.config import get_config from agent_env.entity_refs import EntityRef -from agent_env.providers.sandbox_providers.sandbox import NetworkPolicy, NetworkPolicyUnsupportedError +from agent_env.providers.sandbox_providers.sandbox import NetworkPolicy, NetworkPolicyUnsupportedError, port_bindings from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.a2a_agent.object_transfer import ( TRANSFER_TIMEOUT_SECONDS, @@ -533,12 +533,13 @@ async def _run_container(self, image_name: str, a2a_port: int, merged_env: dict[ escaped_value = value.replace("'", "'\\''") env_flags.append(f"-e {key}='{escaped_value}'") env_str = " \\\n ".join(env_flags) + publish = " ".join(f"-p {spec}" for spec in port_bindings(self._sandbox.host_ips, a2a_port, a2a_port)) run_script = f"""#!/bin/bash set -e {setup_script}docker run -d \\ --name {self._sandbox.container_name} \\ - -p {a2a_port}:{a2a_port} \\ + {publish} \\ {network_flag}{env_str} \\ {image_name} > /dev/null sleep 2 diff --git a/src/agent_env/env/envs/multi_env.py b/src/agent_env/env/envs/multi_env.py index 7fa4bd2d..1bf7ab88 100644 --- a/src/agent_env/env/envs/multi_env.py +++ b/src/agent_env/env/envs/multi_env.py @@ -578,6 +578,7 @@ async def _load_from_snapshot(self, snapshot) -> None: gateway_mode=self._gateway_mode, state_provider=gw._state_provider, # local-only, set above state_instance=gw._state_instance, + host_ips=self._sandbox.host_ips, mcp_server_name=self._mcp_server_name or self.name, ) compose_content = compose_content.replace( diff --git a/src/agent_env/providers/env_providers/env_gateway_provider.py b/src/agent_env/providers/env_providers/env_gateway_provider.py index 26e6f74f..d5ca45b5 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -34,7 +34,7 @@ ) from agent_env.providers.env_providers.env_provider import _SandboxEnvironmentProvider, _size_kwargs, _tool_names from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider -from agent_env.providers.sandbox_providers.sandbox import Sandbox, VmSandbox +from agent_env.providers.sandbox_providers.sandbox import Sandbox, VmSandbox, port_bindings from agent_env.providers.sandbox_providers.sandbox_provider import SANDBOX_MODE_CONTAINER, SandboxProvider if TYPE_CHECKING: @@ -210,6 +210,7 @@ def create_docker_compose( state_provider: "DatabaseStateProvider", state_instance: "EnvStateInstance", host_port: Optional[Callable[[int], int]] = None, + host_ips: tuple[str, ...] = (), mcp_server_name: str | None = None, ) -> str: """Generate docker-compose.yml content for a gateway deployment onto a VM/laptop/arbitrary machine. @@ -316,7 +317,7 @@ def create_docker_compose( lines.extend( state_provider.render_compose_containers( - environment_names, instance=state_instance, host_port=host_port + environment_names, instance=state_instance, host_port=host_port, host_ips=host_ips ) ) store_dep_lines = state_provider.docker_service_dependency() @@ -417,7 +418,7 @@ def create_docker_compose( if expose_gateway_port: exposed_gateway_ports.extend([ " ports:", - f' - "{publish(gateway_port)}:{gateway_port}"', + *(f' - "{spec}"' for spec in port_bindings(host_ips, publish(gateway_port), gateway_port)), ]) lines.extend([ " environment:", @@ -486,7 +487,7 @@ def create_docker_compose( lines.append(f" - {key}={value}") lines.extend([ " ports:", - f' - "{publish(sc.host_port)}:{sc.container_port}"', + *(f' - "{spec}"' for spec in port_bindings(host_ips, publish(sc.host_port), sc.container_port)), f" restart: {sc.restart}", " networks:", " - env-network", @@ -706,6 +707,7 @@ async def _stand_up(_spec): state_provider=self._state_provider, state_instance=state_instance, host_port=sandbox.host_port, + host_ips=sandbox.host_ips, mcp_server_name=mcp_server_name, ) logger.info(f"Generated docker-compose.yml:\n{_redact_compose_secrets(compose_content)}") diff --git a/src/agent_env/providers/env_state/env_state_provider.py b/src/agent_env/providers/env_state/env_state_provider.py index 7a3d25d9..f0e1ff93 100644 --- a/src/agent_env/providers/env_state/env_state_provider.py +++ b/src/agent_env/providers/env_state/env_state_provider.py @@ -201,6 +201,7 @@ def render_sidecar_containers( environment_names: list[str], *, host_port: Optional[Callable[[int], int]] = None, + host_ips: tuple[str, ...] = (), ) -> list[str]: """Extra co-deployed compose services beyond the readiness one (e.g. pgweb / db-mcp browse UIs). Optional: a backend may have none, in which case this returns an empty list.""" @@ -212,16 +213,17 @@ def render_compose_containers( *, instance: EnvStateInstance | None = None, host_port: Optional[Callable[[int], int]] = None, + host_ips: tuple[str, ...] = (), ) -> list[str]: """All co-deployed compose services for this backend — the mandatory readiness service plus any sidecars — spliced into the gateway's docker-compose doc. ``host_port`` maps a container port to the host port to publish it on; defaults - to identity. + to identity. ``host_ips`` are the host IPs to publish on; none means every interface. """ return [ *self.render_healthcheck_service(environment_names, instance=instance), - *self.render_sidecar_containers(environment_names, host_port=host_port), + *self.render_sidecar_containers(environment_names, host_port=host_port, host_ips=host_ips), ] def docker_service_dependency(self) -> list[str]: diff --git a/src/agent_env/providers/env_state/local_postgres.py b/src/agent_env/providers/env_state/local_postgres.py index 50af58b8..b324910a 100644 --- a/src/agent_env/providers/env_state/local_postgres.py +++ b/src/agent_env/providers/env_state/local_postgres.py @@ -30,6 +30,7 @@ PGWEB_SERVICE_NAME, SERVICE_DB_PORT, ) +from agent_env.providers.sandbox_providers.sandbox import port_bindings from agent_env.providers.env_state.env_state_provider import ( DatabaseStateProvider, DEFAULT_STATE_TTL_SECONDS, @@ -258,9 +259,12 @@ def render_sidecar_containers( environment_names: list[str], *, host_port: Optional[Callable[[int], int]] = None, + host_ips: tuple[str, ...] = (), ) -> list[str]: """The pgweb + db-mcp direct-SQL browse UIs (VM/compose path), each gated on the servicedb's health. Beyond the mandatory readiness service.""" + # Opened from the host only, so kept off the bridge address containers reach the host through. + host_ips = host_ips[:1] publish = host_port or (lambda port: port) cfg = self.service_db_config lines: list[str] = [] @@ -279,7 +283,7 @@ def render_sidecar_containers( # session and 500'd the changelog export on Save & Next). " - PGWEB_LOCK_SESSION=true", " ports:", - f' - "{publish(DB_WEB_PORT)}:{DB_WEB_PORT}"', + *(f' - "{spec}"' for spec in port_bindings(host_ips, publish(DB_WEB_PORT), DB_WEB_PORT)), " networks:", " - env-network", "", @@ -296,7 +300,7 @@ def render_sidecar_containers( f" - DATABASE_URI={self._pg_url(DATABASE_SERVICE_NAME, scheme='postgresql')}", " command: --transport=streamable-http --streamable-http-host=0.0.0.0 --access-mode=unrestricted", " ports:", - f' - "{publish(DB_MCP_PORT)}:{DB_MCP_CONTAINER_PORT}"', + *(f' - "{spec}"' for spec in port_bindings(host_ips, publish(DB_MCP_PORT), DB_MCP_CONTAINER_PORT)), " networks:", " - env-network", "", diff --git a/src/agent_env/providers/sandbox_providers/local_sandbox.py b/src/agent_env/providers/sandbox_providers/local_sandbox.py index cce1988e..ad5f14e6 100644 --- a/src/agent_env/providers/sandbox_providers/local_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/local_sandbox.py @@ -8,6 +8,7 @@ import asyncio import contextlib +import functools import glob import logging import os @@ -113,8 +114,9 @@ def __init__(self, exposed_ports: list[int] | None = None, work_dir: Path | None port: port for port in (exposed_ports or []) } # Keyed by container port: callers index this with the well-known constants. + # 127.0.0.1, not localhost: ports are published on IPv4 loopback, and a client may try ::1 first. self.tunnel_urls = { - container: f"http://localhost:{host}" for container, host in self._port_map.items() + container: f"http://127.0.0.1:{host}" for container, host in self._port_map.items() } self.vnc_url = None self.mode = SANDBOX_MODE_VM @@ -126,6 +128,11 @@ def host_port(self, port: int) -> int: """The allocated host port for a published container port (identity if unmapped).""" return self._port_map.get(port, port) + @property + def host_ips(self) -> tuple[str, ...]: + """Loopback, so a local deploy is not reachable from the network (see ``_host_ips``).""" + return _host_ips() + @classmethod def find_work_dir(cls, sandbox_id: str) -> Path | None: """Find the work directory for a previously created local sandbox. @@ -406,3 +413,26 @@ def get_external_url(cls, url: str) -> str: inside a container: ``host.docker.internal``. Docker Desktop and Rancher Desktop (macOS/Windows) resolve this themselves; on Linux ``EXTRA_CONTAINER_RUN_ARGS`` maps it via ``--add-host …:host-gateway``.""" return url.replace("localhost", "host.docker.internal").replace("127.0.0.1", "host.docker.internal") + + +@functools.cache +def _host_ips() -> tuple[str, ...]: + """Loopback, plus on Linux the bridge gateway ``host-gateway`` resolves to, where containers reach the host.""" + # Docker Desktop publishes through a host-side proxy, which can't bind the bridge address inside its VM. + if platform.system() != "Linux" or _docker("info", "--format", "{{.OperatingSystem}}") == "Docker Desktop": + return ("127.0.0.1",) + # Listing, unlike inspecting, answers a missing network with nothing rather than an error. + if not _docker("network", "ls", "--quiet", "--filter", "name=^bridge$"): + return ("127.0.0.1",) + return ("127.0.0.1", _docker("network", "inspect", "bridge", "--format", "{{(index .IPAM.Config 0).Gateway}}")) + + +def _docker(*args: str) -> str: + """A docker CLI query's output. A failure raises, so the cache above never keeps it.""" + run = subprocess.run(["docker", *args], capture_output=True, text=True, timeout=_DOCKER_QUERY_SECONDS) + if run.returncode: + raise RuntimeError(f"docker {' '.join(args)} failed: {run.stderr.strip()}") + return run.stdout.strip() + + +_DOCKER_QUERY_SECONDS = 30 diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index 9fa2c6e2..fd2e9391 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -120,6 +120,8 @@ class Sandbox(ABC): vnc_url: str | None # Effective, as applied by the backend; None when it cannot tell us. network_policy: NetworkPolicy | None = None + # Host IPs published ports bind to; empty binds every interface. + host_ips: tuple[str, ...] = () _VM_READY_TIMEOUT = 1200 # wait_for_vm wall-clock budget (s) _VM_READY_POLL_INTERVAL = 30 # sparse polling (s) @@ -385,3 +387,8 @@ async def write_file_from_text(self, content: str, destination_path: str) -> Non await self._copy_into_container(vm_path, destination_path) finally: await self._remove_vm_temp_file(vm_path, f"{vm_path}.b64") + + +def port_bindings(host_ips: Iterable[str], host_port: int, container_port: int) -> list[str]: + """Docker publish specs for one port: one per host IP, or a bare one (every interface) when there are none.""" + return [f"{ip}:{host_port}:{container_port}" for ip in host_ips] or [f"{host_port}:{container_port}"] diff --git a/src/agent_env/providers/sandbox_providers/sandbox_provider.py b/src/agent_env/providers/sandbox_providers/sandbox_provider.py index b72b6ab1..bfd46cdb 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/sandbox_provider.py @@ -20,6 +20,7 @@ NetworkPolicyUnsupportedError, Sandbox, VmSandbox, + port_bindings, ) if TYPE_CHECKING: @@ -113,9 +114,9 @@ async def create_container( f"-e {shlex.quote(k)}={shlex.quote(v)}" for k, v in env.items() ) extra_args = f"{self.EXTRA_CONTAINER_RUN_ARGS} " if self.EXTRA_CONTAINER_RUN_ARGS else "" - host_side = sandbox.host_port(port) + publish = " ".join(f"-p {spec}" for spec in port_bindings(sandbox.host_ips, sandbox.host_port(port), port)) await sandbox.exec_script( - f"docker run -d --name {shlex.quote(sandbox.container_name)} -p {host_side}:{port} {extra_args}\\\n " + f"docker run -d --name {shlex.quote(sandbox.container_name)} {publish} {extra_args}\\\n " f"{env_flags} \\\n {shlex.quote(image_name)} > /dev/null" ) sandbox.mode = SANDBOX_MODE_CONTAINER diff --git a/src/agent_env/task_step/task_steps/run_docker_container.py b/src/agent_env/task_step/task_steps/run_docker_container.py index f7e45a8a..0f826bff 100644 --- a/src/agent_env/task_step/task_steps/run_docker_container.py +++ b/src/agent_env/task_step/task_steps/run_docker_container.py @@ -11,6 +11,7 @@ from agent_env.task_step.context import TaskStepContext from agent_env.entity_refs import EntityRef +from agent_env.providers.sandbox_providers.sandbox import port_bindings from agent_env.task_step.task_step import TaskStep, TaskStepDependency from agent_env.utils.paths import validate_relative_filename @@ -206,7 +207,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: if self.network: network_flag = f"--network {await self._ensure_network(sandbox, self.network)} " - port_flags = " ".join(f"-p {p}:{p}" for p in (self.ports or [])) + port_flags = " ".join(f"-p {spec}" for p in (self.ports or []) for spec in port_bindings(sandbox.host_ips, p, p)) env_flag_parts = [] if self.env_vars: for k, v in self.env_vars.items(): diff --git a/tst/integration/env/envs/server_env_local_test.py b/tst/integration/env/envs/server_env_local_test.py index 72d27920..217a5e27 100644 --- a/tst/integration/env/envs/server_env_local_test.py +++ b/tst/integration/env/envs/server_env_local_test.py @@ -97,7 +97,7 @@ async def test_a_server_env_deploys_restores_loads_and_tears_down_on_local_conta container = f"agent-{deployed.sandbox_id}" local_stack.append(container) assert type(deployed) is DeployedSandboxEnv and deployed.sandbox_type == "local" - assert deployed.environment_card["name"] == "items" and deployed.mcp_url.startswith("http://localhost:") + assert deployed.environment_card["name"] == "items" and deployed.mcp_url.startswith("http://127.0.0.1:") restored = await Env.from_instance_id(deployed.instance_id) await restored.load_environment_artifact(_items_artifact(uid)) diff --git a/tst/integration/store/local_task_lifecycle_test.py b/tst/integration/store/local_task_lifecycle_test.py index 0fb06948..fa299e60 100644 --- a/tst/integration/store/local_task_lifecycle_test.py +++ b/tst/integration/store/local_task_lifecycle_test.py @@ -201,7 +201,7 @@ async def test_task_lifecycle_all_local(all_local): # is not identity — take the URL the deploy reported. Keyed by CONTAINER port # (deploy_sandbox stringifies the keys), which is what the caller asked for. served_url = deployed.tunnel_urls[str(app_port)] - assert re.fullmatch(r"http://localhost:\d+", served_url), served_url + assert re.fullmatch(r"http://127\.0\.0\.1:\d+", served_url), served_url # the pulled image is actually running and serving on the published port assert _wait_ready(f"{served_url}/", (200,)), \ diff --git a/tst/unit/env/envs/test_multi_env_snapshot_reload_name.py b/tst/unit/env/envs/test_multi_env_snapshot_reload_name.py index b4b637b9..274c1e7f 100644 --- a/tst/unit/env/envs/test_multi_env_snapshot_reload_name.py +++ b/tst/unit/env/envs/test_multi_env_snapshot_reload_name.py @@ -1,4 +1,4 @@ -"""The snapshot reload regenerates the compose; it must reuse the name the running gateway already presents.""" +"""The snapshot reload regenerates the compose; it must reuse the name and host IPs the running gateway already has.""" from __future__ import annotations @@ -20,7 +20,7 @@ class _Stop(Exception): async def test_snapshot_reload_keeps_the_env_name(declared, deployed_as, expected): env = MultiEnv(id="crm-suite", version=1, mcp_server_envs=[], name=declared) env._mcp_server_name = deployed_as - env._sandbox = MagicMock(mode="vm", load_docker_images=AsyncMock()) + env._sandbox = MagicMock(mode="vm", load_docker_images=AsyncMock(), host_ips=("127.0.0.1",)) db_image = MagicMock(spec=DockerImageArtifact, image_name="snap:1") snapshot = MagicMock(db_image_artifact_id="snap", db_image_artifact_version=1) gateway_env, service_db = MagicMock(), MagicMock() @@ -34,3 +34,4 @@ async def test_snapshot_reload_keeps_the_env_name(declared, deployed_as, expecte await env._load_from_snapshot(snapshot) assert compose.call_args.kwargs["mcp_server_name"] == expected + assert compose.call_args.kwargs["host_ips"] == ("127.0.0.1",) diff --git a/tst/unit/providers/compose_host_port_publish_test.py b/tst/unit/providers/compose_host_port_publish_test.py index dafa60d1..c463bac0 100644 --- a/tst/unit/providers/compose_host_port_publish_test.py +++ b/tst/unit/providers/compose_host_port_publish_test.py @@ -2,8 +2,8 @@ LocalSandbox shares the host network namespace, so two concurrent deployments contend for any fixed host port. Each deployment is therefore given its own host port and the compose -publishes ``resolved_host:container``. Container ports are untouched -- they are the env's -internal contract, and callers read the host side back out of ``tunnel_urls``. +publishes ``resolved_host:container``, on the sandbox's host IPs. Container ports are untouched -- +they are the env's internal contract, and callers read the host side back out of ``tunnel_urls``. Coverage is two layers. The source guard holds for every renderer in the tree, including env types added later. The rendered-document checks then exercise the config axes that @@ -15,6 +15,7 @@ import pytest +from agent_env.env.gateway import AGENT_ENV_GATEWAY_MCP_PORT from agent_env.providers.env_providers import EnvironmentGatewayProvider, MCPServerConfig, SidecarConfig from agent_env.env.envs.service_db import DB_MCP_CONTAINER_PORT, DB_MCP_PORT, DB_WEB_PORT from agent_env.providers.env_state.local_postgres import LocalPostgresStateProvider @@ -28,22 +29,30 @@ # --- source guard: exhaustive over the tree --------------------------------- -# A compose port mapping. Excludes volume mounts and image refs (which contain "/" or -# "://") and docstring illustrations written as . -_PORT_PAIR = re.compile(r'-\s+"([^":/]+):([^":/]+)"') +# A compose port mapping, ``[ip:]host:container``, double-quoted, single-quoted or bare. Excludes +# volume mounts and image refs (which contain "/" or "://") and docstring illustrations written as +# . +_PORT_PAIR = re.compile(r'-\s+["\']?(?:([^"\':/\s]+):)?([^"\':/\s]+):([^"\':/\s]+)') + +# A compose port entry rendered from ``port_bindings(host_ips, host_port, container_port)``. +_PUBLISH_SITE = re.compile(r'-\s+"\{(\w+)\}"\' for \1 in port_bindings\([^,]+, (.+), [^,]+\)') def _is_port_expression(text: str) -> bool: return text.isdigit() or (text.startswith("{") and text.endswith("}")) -def _publish_sites(): - """Every ``host:container`` publication written anywhere under src/.""" +def _source_lines(): for path in sorted(SRC.rglob("*.py")): for lineno, line in enumerate(path.read_text().splitlines(), 1): - for host, container in _PORT_PAIR.findall(line): - if _is_port_expression(host) and _is_port_expression(container): - yield path, lineno, line.strip(), host + yield path, lineno, line + + +def _publish_sites(): + """Every compose port entry written anywhere under src/, with its host-port expression.""" + for path, lineno, line in _source_lines(): + for _, host in _PUBLISH_SITE.findall(line): + yield path, lineno, line.strip(), host def _bypasses_resolver(host: str) -> bool: @@ -66,6 +75,25 @@ def test_no_renderer_publishes_a_fixed_host_port(): assert not violations, "host ports published without the resolver:\n" + "\n".join(violations) +def test_no_renderer_writes_a_port_mapping_by_hand(): + """A mapping written out skips ``port_bindings``, so on local it would publish on every interface.""" + by_hand = [ + f"{path.relative_to(SRC)}:{lineno}: {line.strip()}" + for path, lineno, line in _source_lines() + for _, host, container in _PORT_PAIR.findall(line) + if _is_port_expression(host) and _is_port_expression(container) + ] + assert not by_hand, "port mappings written without port_bindings:\n" + "\n".join(by_hand) + + +def test_the_mapping_guard_reads_every_quoting(): + """Compose accepts a mapping in any of the three forms, so a hand-written one can't slip past as another.""" + for entry in ('- "8080:8080"', "- '8080:8080'", "- 8080:8080"): + assert _PORT_PAIR.findall(entry) == [("", "8080", "8080")] + # With an IP in front, the IP is its own field and the host port is still the second. + assert _PORT_PAIR.findall('- "127.0.0.1:18768:8000"') == [("127.0.0.1", "18768", "8000")] + + def test_source_guard_flags_a_fixed_host_port(): """The guard above only means something if a constant actually trips it.""" assert _bypasses_resolver("{DB_WEB_PORT}") @@ -81,7 +109,7 @@ def _published_pairs(compose: str) -> list[tuple[int, int]]: """Every numeric ``host:container`` pair in a rendered compose.""" pairs = [] for line in compose.splitlines(): - for host, container in _PORT_PAIR.findall(line.strip()): + for _, host, container in _PORT_PAIR.findall(line.strip()): if host.isdigit() and container.isdigit(): pairs.append((int(host), int(container))) return pairs @@ -213,3 +241,26 @@ def test_postgres_sidecar_fragment_defaults_to_identity(stub_postgres): assert f'"{DB_WEB_PORT}:{DB_WEB_PORT}"' in lines assert f'"{DB_MCP_PORT}:{DB_MCP_CONTAINER_PORT}"' in lines + + +@pytest.mark.parametrize("host_ips", [("127.0.0.1",), ("127.0.0.1", "172.17.0.1")], ids=["loopback", "loopback-and-bridge"]) +def test_gateway_compose_publishes_only_on_the_host_ips(stub_postgres, host_ips): + """Each port once per host IP, and none on every interface. pgweb and db-mcp, opened only from the + host, stay off the bridge address.""" + compose = EnvironmentGatewayProvider.create_docker_compose( + EnvironmentGatewayProvider.__new__(EnvironmentGatewayProvider), + mcp_servers=[MCPServerConfig(image="slack:test", environment_name="slack")], + gateway_image="gateway:test", + sidecars=[_sidecar("relay", 18768)], + state_provider=stub_postgres, + state_instance=_StateInstance(), + host_port=lambda port: port + OFFSET, + host_ips=host_ips, + ) + + everywhere = [(AGENT_ENV_GATEWAY_MCP_PORT, AGENT_ENV_GATEWAY_MCP_PORT), (18768, 8000)] + host_only = [(DB_WEB_PORT, DB_WEB_PORT), (DB_MCP_PORT, DB_MCP_CONTAINER_PORT)] + assert sorted(_PORT_PAIR.findall(compose)) == sorted( + [(ip, str(host + OFFSET), str(container)) for ip in host_ips for host, container in everywhere] + + [(host_ips[0], str(host + OFFSET), str(container)) for host, container in host_only] + ) diff --git a/tst/unit/providers/env_providers/env_gateway_provider_test.py b/tst/unit/providers/env_providers/env_gateway_provider_test.py index 91e130b9..880abe09 100644 --- a/tst/unit/providers/env_providers/env_gateway_provider_test.py +++ b/tst/unit/providers/env_providers/env_gateway_provider_test.py @@ -1336,7 +1336,7 @@ async def test_deploy_returns_the_gateway_record_for_the_env(caplog): caplog.set_level(logging.INFO, logger="agent_env.providers.env_providers") gp, external = EnvironmentGatewayProvider(), MagicMock(state_type="remote", instance_id="st-ext") state_provider = MagicMock(spec=DatabaseStateProvider, prepare=AsyncMock(), teardown=AsyncMock()) - vm = MagicMock(sandbox_id="vm-1", type="modal_vm", tunnel_urls={18765: "https://vm.example"}, host_port=lambda port: port, + vm = MagicMock(sandbox_id="vm-1", type="modal_vm", tunnel_urls={18765: "https://vm.example"}, host_port=lambda port: port, host_ips=(), load_docker_images=AsyncMock(), exec_script=AsyncMock(), exec_with_output=AsyncMock(return_value=(0, "gateway", ""))) env = MagicMock(id="crm-env", version=3, environment_name="slack", docker_image_artifact=MagicMock(image_name="mcp-slack")) card = {"name": "env1234", "children_environments": []} @@ -1359,6 +1359,7 @@ async def test_deploy_returns_the_gateway_record_for_the_env(caplog): [server] = compose.call_args.kwargs["mcp_servers"] assert (server.image, server.environment_name, compose.call_args.kwargs["gateway_mode"]) == ("mcp-slack", "slack", GatewayMode.CONSISTENT) assert re.fullmatch(r"env\d{4}", compose.call_args.kwargs["mcp_server_name"]) + assert compose.call_args.kwargs["host_ips"] == () assert dataclasses.asdict(record) == dataclasses.asdict(DeployedGatewayEnv( env_id="crm-env", env_version=3, gateway_url="https://vm.example", mcp_url="https://vm.example/mcp", db_web_url=None, sandbox_id="vm-1", sandbox_type="modal_vm", sandbox_ids={}, db_mcp_url=None, diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_host_ports_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_host_ports_test.py index 9f108f4d..a0905443 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_host_ports_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_host_ports_test.py @@ -15,6 +15,10 @@ from agent_env.providers.sandbox_providers import local_sandbox as local_sandbox_module from agent_env.env.envs.service_db import DB_MCP_PORT, DB_WEB_PORT from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider +from agent_env.providers.sandbox_providers.e2b import E2BSandbox +from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandbox +from agent_env.providers.sandbox_providers.modal_vm_sandbox import ModalVmSandbox +from agent_env.providers.sandbox_providers.sandbox import port_bindings RESERVED = [AGENT_ENV_GATEWAY_MCP_PORT, DB_WEB_PORT, DB_MCP_PORT] @@ -37,6 +41,12 @@ def test_base_sandbox_host_port_is_identity(): assert Sandbox.host_port(None, 18765) == 18765 +@pytest.mark.parametrize("sandbox_class", [ModalVmSandbox, ModalSandbox, E2BSandbox]) +def test_remote_sandboxes_publish_on_every_interface(sandbox_class): + """Their tunnels reach a published port from outside the VM, so it stays on every interface.""" + assert port_bindings(sandbox_class.host_ips, 18765, 18765) == ["18765:18765"] + + def test_unmapped_ports_pass_through(): assert LocalSandbox().host_port(18765) == 18765 @@ -81,4 +91,4 @@ async def test_tunnel_urls_stay_keyed_by_container_port(fake_ports): assert set(sandbox.tunnel_urls) == set(RESERVED) for port in RESERVED: - assert sandbox.tunnel_urls[port] == f"http://localhost:{sandbox.host_port(port)}" + assert sandbox.tunnel_urls[port] == f"http://127.0.0.1:{sandbox.host_port(port)}" diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index eb90d393..99abbf38 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -8,12 +8,19 @@ import pytest import agent_env.providers.sandbox_providers.local_sandbox as ls +from agent_env.a2a_agent.a2a_agent import A2AAgent from agent_env.config import reset_config, set_object_store from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider from agent_env.providers.sandbox_providers.sandbox_provider import SANDBOX_MODE_CONTAINER from agent_env.store import LocalFilesystemObjectStore +@pytest.fixture(autouse=True) +def loopback_host_ips(monkeypatch): + """On Linux the host IPs take a docker call; the tests of that lookup opt back in with ``real_host_ips``.""" + monkeypatch.setattr(ls, "_host_ips", lambda: ("127.0.0.1",)) + + def test_rewrite_app_arg_only_rewrites_app_prefix(): sandbox = LocalSandbox(work_dir=Path("/tmp/agent-env-work")) @@ -421,16 +428,16 @@ async def test_create_vm_publishes_on_an_allocated_host_port_not_the_requested_o assert sandbox.host_port(8000) == 41001 assert sandbox.host_port(9001) == 41002 - assert sandbox.tunnel_urls[8000] == "http://localhost:41001" - assert sandbox.tunnel_urls[9001] == "http://localhost:41002" + assert sandbox.tunnel_urls[8000] == "http://127.0.0.1:41001" + assert sandbox.tunnel_urls[9001] == "http://127.0.0.1:41002" # An unexposed port has nothing published, so it falls back to identity. assert sandbox.host_port(7777) == 7777 @pytest.mark.asyncio async def test_create_container_publishes_the_allocated_host_port(monkeypatch, tmp_path): - """The `docker run -p` left-hand side is the allocated host port, not the container port, - so what `tunnel_urls` advertises is what Docker actually published.""" + """The `docker run -p` left-hand side is the allocated host port, not the container port, on + each host IP, so what `tunnel_urls` advertises is what Docker actually published.""" import agent_env.config as cfg class _StubImageStore: @@ -443,6 +450,7 @@ def get_image_store(self): monkeypatch.setattr(cfg, "get_config", lambda: _StubConfig()) monkeypatch.setattr(ls, "_free_host_port", lambda: 41337) + monkeypatch.setattr(ls, "_host_ips", lambda: ("127.0.0.1", "172.17.0.1")) recorded: list[str] = [] made: list[LocalSandbox] = [] @@ -464,7 +472,96 @@ async def _rec(script, *, max_retries=0): assert made[0].host_port(8000) == 41337 run_cmd = next(s for s in recorded if "docker run" in s) - assert "-p 41337:8000" in run_cmd + assert "-p 127.0.0.1:41337:8000 -p 172.17.0.1:41337:8000 " in run_cmd + + +@pytest.mark.asyncio +async def test_an_agent_placed_on_a_local_vm_publishes_only_on_the_host_ips(monkeypatch, tmp_path): + monkeypatch.setattr(ls, "_host_ips", lambda: ("127.0.0.1", "172.17.0.1")) + agent = A2AAgent(id="a", version=None, docker_image_artifact=SimpleNamespace(image_name="img:1")) + agent._sandbox = _RecordingLocalSandbox(work_dir=tmp_path) + + await agent._run_container("img:1", 8000, {}) + + [script] = agent._sandbox.scripts + assert "-p 127.0.0.1:8000:8000 -p 172.17.0.1:8000:8000 " in script + + +@pytest.fixture +def real_host_ips(monkeypatch): + monkeypatch.setattr(ls, "_host_ips", _HOST_IPS) + _HOST_IPS.cache_clear() + yield + _HOST_IPS.cache_clear() + + +def test_host_ips_are_loopback_off_linux(monkeypatch, real_host_ips, tmp_path): + """Docker Desktop and Rancher route host.docker.internal to the host's loopback.""" + monkeypatch.setattr(ls.platform, "system", lambda: "Darwin") + monkeypatch.setattr(ls.subprocess, "run", lambda *a, **k: pytest.fail("no docker call is needed")) + + assert LocalSandbox(work_dir=tmp_path).host_ips == ("127.0.0.1",) + + +def test_host_ips_add_the_bridge_gateway_on_linux(monkeypatch, real_host_ips, tmp_path): + """On Linux host.docker.internal is the bridge gateway (host-gateway), which cannot reach a loopback-only port.""" + runs = _fake_docker(monkeypatch, {**_ENGINE}) + sandbox = LocalSandbox(work_dir=tmp_path) + + assert sandbox.host_ips == ("127.0.0.1", "172.17.0.1") + assert sandbox.host_ips == ("127.0.0.1", "172.17.0.1") + assert [_query(args) for args, _ in runs] == ["info", "network ls", "network inspect"] + assert all(kwargs["timeout"] for _, kwargs in runs) + + +def test_host_ips_are_loopback_on_docker_desktop_for_linux(monkeypatch, real_host_ips, tmp_path): + """Its proxy can't bind the bridge address, which lives in its VM (docker/desktop-feedback#485).""" + runs = _fake_docker(monkeypatch, {**_ENGINE, "info": "Docker Desktop"}) + + assert LocalSandbox(work_dir=tmp_path).host_ips == ("127.0.0.1",) + assert [_query(args) for args, _ in runs] == ["info"] + + +def test_host_ips_are_loopback_without_a_default_bridge(monkeypatch, real_host_ips, tmp_path): + runs = _fake_docker(monkeypatch, {**_ENGINE, "network ls": ""}) + + assert LocalSandbox(work_dir=tmp_path).host_ips == ("127.0.0.1",) + assert [_query(args) for args, _ in runs] == ["info", "network ls"] + + +@pytest.mark.parametrize("failing", ["info", "network ls", "network inspect"]) +def test_a_daemon_that_fails_once_is_asked_again(monkeypatch, real_host_ips, tmp_path, failing): + """Caching a failed lookup would keep later agents off the bridge, and so away from their gateways.""" + outputs = {**_ENGINE, failing: None} + _fake_docker(monkeypatch, outputs) + + with pytest.raises(RuntimeError, match="Cannot connect to the Docker daemon"): + LocalSandbox(work_dir=tmp_path).host_ips + outputs[failing] = _ENGINE[failing] + assert LocalSandbox(work_dir=tmp_path).host_ips == ("127.0.0.1", "172.17.0.1") + + +_ENGINE = {"info": "Ubuntu 24.04.3 LTS", "network ls": "38411e70d045", "network inspect": "172.17.0.1"} + + +def _query(args): + return "info" if args[1] == "info" else " ".join(args[1:3]) + + +def _fake_docker(monkeypatch, outputs): + """``docker `` answers ``outputs[query]``; ``None`` fails the call, as a daemon that is down would.""" + runs = [] + + def run(args, **kwargs): + runs.append((args, kwargs)) + out = outputs[_query(args)] + if out is None: + return SimpleNamespace(returncode=1, stdout="", stderr="Cannot connect to the Docker daemon at unix:///var/run/docker.sock.\n") + return SimpleNamespace(returncode=0, stdout=f"{out}\n", stderr="") + + monkeypatch.setattr(ls.platform, "system", lambda: "Linux") + monkeypatch.setattr(ls.subprocess, "run", run) + return runs class _CapturedProcess: @@ -604,3 +701,6 @@ async def slow_spawn(*args, **kwargs): with pytest.raises(ProcessLookupError): os.kill(int(pid_file.read_text()), 0) + + +_HOST_IPS = ls._host_ips