From 5bd8695ab894b1b618ded27a5b60dbf7b393d031 Mon Sep 17 00:00:00 2001 From: shanemort1982 <156683457+shanemort1982@users.noreply.github.com> Date: Mon, 5 Jan 2026 03:26:16 +0000 Subject: [PATCH] feat: Add configurable sandbox host_port and container_url_pattern for remote access (#12255) Co-authored-by: openhands Co-authored-by: Tim O'Farrell --- openhands/app_server/config.py | 15 ++- .../sandbox/docker_sandbox_service.py | 19 ++- .../app_server/test_docker_sandbox_service.py | 113 ++++++++++++++++++ 3 files changed, 143 insertions(+), 4 deletions(-) diff --git a/openhands/app_server/config.py b/openhands/app_server/config.py index 3c40806af0..a11db78ea3 100644 --- a/openhands/app_server/config.py +++ b/openhands/app_server/config.py @@ -67,7 +67,8 @@ def get_default_persistence_dir() -> Path: def get_default_web_url() -> str | None: """Get legacy web host parameter. - If present, we assume we are running under https.""" + If present, we assume we are running under https. + """ web_host = os.getenv('WEB_HOST') if not web_host: return None @@ -175,7 +176,17 @@ def config_from_env() -> AppServerConfig: elif os.getenv('RUNTIME') in ('local', 'process'): config.sandbox = ProcessSandboxServiceInjector() else: - config.sandbox = DockerSandboxServiceInjector() + # Support legacy environment variables for Docker sandbox configuration + docker_sandbox_kwargs: dict = {} + if os.getenv('SANDBOX_HOST_PORT'): + docker_sandbox_kwargs['host_port'] = int( + os.environ['SANDBOX_HOST_PORT'] + ) + if os.getenv('SANDBOX_CONTAINER_URL_PATTERN'): + docker_sandbox_kwargs['container_url_pattern'] = os.environ[ + 'SANDBOX_CONTAINER_URL_PATTERN' + ] + config.sandbox = DockerSandboxServiceInjector(**docker_sandbox_kwargs) if config.sandbox_spec is None: if os.getenv('RUNTIME') == 'remote': diff --git a/openhands/app_server/sandbox/docker_sandbox_service.py b/openhands/app_server/sandbox/docker_sandbox_service.py index 2969a2ebbd..26cc2c9861 100644 --- a/openhands/app_server/sandbox/docker_sandbox_service.py +++ b/openhands/app_server/sandbox/docker_sandbox_service.py @@ -426,8 +426,23 @@ class DockerSandboxService(SandboxService): class DockerSandboxServiceInjector(SandboxServiceInjector): """Dependency injector for docker sandbox services.""" - container_url_pattern: str = 'http://localhost:{port}' - host_port: int = 3000 + container_url_pattern: str = Field( + default='http://localhost:{port}', + description=( + 'URL pattern for exposed sandbox ports. Use {port} as placeholder. ' + 'For remote access, set to your server IP (e.g., http://192.168.1.100:{port}). ' + 'Configure via OH_SANDBOX_CONTAINER_URL_PATTERN environment variable.' + ), + ) + host_port: int = Field( + default=3000, + description=( + 'The port on which the main OpenHands app server is running. ' + 'Used for webhook callbacks from agent-server containers. ' + 'If running OpenHands on a non-default port, set this to match. ' + 'Configure via OH_SANDBOX_HOST_PORT environment variable.' + ), + ) container_name_prefix: str = 'oh-agent-server-' max_num_sandboxes: int = Field( default=5, diff --git a/tests/unit/app_server/test_docker_sandbox_service.py b/tests/unit/app_server/test_docker_sandbox_service.py index b6b6774de0..714d030685 100644 --- a/tests/unit/app_server/test_docker_sandbox_service.py +++ b/tests/unit/app_server/test_docker_sandbox_service.py @@ -973,3 +973,116 @@ class TestExposedPort: port = ExposedPort(name='test', description='Test port') with pytest.raises(ValueError): # Should raise validation error port.name = 'new_name' + + +class TestDockerSandboxServiceInjector: + """Test cases for DockerSandboxServiceInjector configuration.""" + + def test_default_values(self): + """Test default configuration values.""" + from openhands.app_server.sandbox.docker_sandbox_service import ( + DockerSandboxServiceInjector, + ) + + injector = DockerSandboxServiceInjector() + assert injector.host_port == 3000 + assert injector.container_url_pattern == 'http://localhost:{port}' + + def test_custom_host_port(self): + """Test custom host_port configuration.""" + from openhands.app_server.sandbox.docker_sandbox_service import ( + DockerSandboxServiceInjector, + ) + + injector = DockerSandboxServiceInjector(host_port=4000) + assert injector.host_port == 4000 + + def test_custom_container_url_pattern(self): + """Test custom container_url_pattern configuration.""" + from openhands.app_server.sandbox.docker_sandbox_service import ( + DockerSandboxServiceInjector, + ) + + injector = DockerSandboxServiceInjector( + container_url_pattern='http://192.168.1.100:{port}' + ) + assert injector.container_url_pattern == 'http://192.168.1.100:{port}' + + def test_custom_configuration_combined(self): + """Test combined custom configuration for remote access.""" + from openhands.app_server.sandbox.docker_sandbox_service import ( + DockerSandboxServiceInjector, + ) + + injector = DockerSandboxServiceInjector( + host_port=4000, + container_url_pattern='http://192.168.1.100:{port}', + ) + assert injector.host_port == 4000 + assert injector.container_url_pattern == 'http://192.168.1.100:{port}' + + +class TestDockerSandboxServiceInjectorFromEnv: + """Test cases for DockerSandboxServiceInjector environment variable configuration.""" + + def test_config_from_env_with_sandbox_host_port(self): + """Test that SANDBOX_HOST_PORT environment variable is respected.""" + import os + from unittest.mock import patch + + env_vars = { + 'SANDBOX_HOST_PORT': '4000', + } + + with patch.dict(os.environ, env_vars, clear=False): + # Clear the global config to force reload + import openhands.app_server.config as config_module + from openhands.app_server.config import config_from_env + + config_module._global_config = None + + config = config_from_env() + assert config.sandbox is not None + assert config.sandbox.host_port == 4000 + + def test_config_from_env_with_sandbox_container_url_pattern(self): + """Test that SANDBOX_CONTAINER_URL_PATTERN environment variable is respected.""" + import os + from unittest.mock import patch + + env_vars = { + 'SANDBOX_CONTAINER_URL_PATTERN': 'http://192.168.1.100:{port}', + } + + with patch.dict(os.environ, env_vars, clear=False): + # Clear the global config to force reload + import openhands.app_server.config as config_module + from openhands.app_server.config import config_from_env + + config_module._global_config = None + + config = config_from_env() + assert config.sandbox is not None + assert config.sandbox.container_url_pattern == 'http://192.168.1.100:{port}' + + def test_config_from_env_with_both_sandbox_vars(self): + """Test that both SANDBOX_HOST_PORT and SANDBOX_CONTAINER_URL_PATTERN work together.""" + import os + from unittest.mock import patch + + env_vars = { + 'SANDBOX_HOST_PORT': '4000', + 'SANDBOX_CONTAINER_URL_PATTERN': 'http://192.168.1.100:{port}', + } + + with patch.dict(os.environ, env_vars, clear=False): + # Clear the global config to force reload + import openhands.app_server.config as config_module + from openhands.app_server.config import config_from_env + + config_module._global_config = None + + config = config_from_env() + assert config.sandbox is not None + assert config.sandbox.host_port == 4000 + assert config.sandbox.container_url_pattern == 'http://192.168.1.100:{port}'